From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by mx.groups.io with SMTP id smtpd.web10.632.1582678248900227016 for ; Tue, 25 Feb 2020 16:50:49 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.126, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Feb 2020 16:50:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,486,1574150400"; d="scan'208";a="231215563" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga008.jf.intel.com with ESMTP; 25 Feb 2020 16:50:48 -0800 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 25 Feb 2020 16:50:35 -0800 Received: from fmsmsx605.amr.corp.intel.com (10.18.126.85) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Tue, 25 Feb 2020 16:50:35 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx605.amr.corp.intel.com (10.18.126.85) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 25 Feb 2020 16:50:34 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.5]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.158]) with mapi id 14.03.0439.000; Wed, 26 Feb 2020 08:50:32 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Albecki, Mateusz" CC: Marcin Wojtas , "Gao, Zhichao" , "Gao, Liming" Subject: Re: [edk2-devel] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe: Send SEND_STATUS at lower frequency Thread-Topic: [edk2-devel] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe: Send SEND_STATUS at lower frequency Thread-Index: AQHV6+0kBtLa75s5J02eygid22e4t6gspGGQ Date: Wed, 26 Feb 2020 00:50:31 +0000 Message-ID: References: <20200225150553.6040-1-mateusz.albecki@intel.com> <20200225150553.6040-2-mateusz.albecki@intel.com> In-Reply-To: <20200225150553.6040-2-mateusz.albecki@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Albecki, Mateusz > Sent: Tuesday, February 25, 2020 11:06 PM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Limin= g > Subject: [edk2-devel] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe: Send > SEND_STATUS at lower frequency >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1140 >=20 > To avoid stability issues on some designs the driver > will now send SEND_STATUS at previous, lower, frequency > when upgrading the bus timing. Reviewed-by: Hao A Wu I will push this patch *after* the upcoming edk2-stable202002 tag. Best Regards, Hao Wu >=20 > Cc: Hao A Wu > Cc: Marcin Wojtas > Cc: Zhichao Gao > Cc: Liming Gao >=20 > Signed-off-by: Mateusz Albecki > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 88 > ++++++++++++++++------ > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 2 +- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 1 + > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 2 + > 4 files changed, 69 insertions(+), 24 deletions(-) >=20 > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > index 776c0e796c..8b5f8e8ee7 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > @@ -558,6 +558,43 @@ EmmcTuningClkForHs200 ( > return EFI_DEVICE_ERROR; > } >=20 > +/** > + Check the SWITCH operation status. > + > + @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @param[in] Slot The slot number on which command should be sent. > + @param[in] Rca The relative device address. > + > + @retval EFI_SUCCESS The SWITCH finished siccessfully. > + @retval others The SWITCH failed. > +**/ > +EFI_STATUS > +EmmcCheckSwitchStatus ( > + IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > + IN UINT8 Slot, > + IN UINT16 Rca > + ) > +{ > + EFI_STATUS Status; > + UINT32 DevStatus; > + > + Status =3D EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "EmmcCheckSwitchStatus: Send status fails > with %r\n", Status)); > + return Status; > + } > + > + // > + // Check the switch operation is really successful or not. > + // > + if ((DevStatus & BIT7) !=3D 0) { > + DEBUG ((DEBUG_ERROR, "EmmcCheckSwitchStatus: The switch > operation fails as DevStatus is 0x%08x\n", DevStatus)); > + return EFI_DEVICE_ERROR; > + } > + > + return EFI_SUCCESS; > +} > + > /** > Switch the bus width to specified width. >=20 > @@ -591,7 +628,6 @@ EmmcSwitchBusWidth ( > UINT8 Index; > UINT8 Value; > UINT8 CmdSet; > - UINT32 DevStatus; >=20 > // > // Write Byte, the Value field is written into the byte pointed by In= dex. > @@ -617,18 +653,10 @@ EmmcSwitchBusWidth ( > return Status; > } >=20 > - Status =3D EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); > + Status =3D EmmcCheckSwitchStatus (PassThru, Slot, Rca); > if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusWidth: Send status fails > with %r\n", Status)); > return Status; > } > - // > - // Check the switch operation is really successful or not. > - // > - if ((DevStatus & BIT7) !=3D 0) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusWidth: The switch operation > fails as DevStatus is 0x%08x\n", DevStatus)); > - return EFI_DEVICE_ERROR; > - } >=20 > Status =3D SdMmcHcSetBusWidth (PciIo, Slot, BusWidth); >=20 > @@ -669,9 +697,9 @@ EmmcSwitchBusTiming ( > UINT8 Index; > UINT8 Value; > UINT8 CmdSet; > - UINT32 DevStatus; > SD_MMC_HC_PRIVATE_DATA *Private; > UINT8 HostCtrl1; > + BOOLEAN DelaySendStatus; >=20 > Private =3D SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > // > @@ -695,7 +723,7 @@ EmmcSwitchBusTiming ( > Value =3D 0; > break; > default: > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported > BusTiming(%d\n)", BusTiming)); > + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported > BusTiming(%d)\n", BusTiming)); > return EFI_INVALID_PARAMETER; > } >=20 > @@ -724,6 +752,26 @@ EmmcSwitchBusTiming ( > return Status; > } >=20 > + // > + // For cases when we switch bus timing to higher mode from current we > want to > + // send SEND_STATUS at current, lower, frequency then the target > frequency to avoid > + // stability issues. It has been observed that some designs are unabl= e to > process the > + // SEND_STATUS at higher frequency during switch to HS200 @200MHz > irrespective of the number of retries > + // and only running the clock tuning is able to make them work at tar= get > frequency. > + // > + // For cases when we are downgrading the frequency and current high > frequency is invalid > + // we have to first change the frequency to target frequency and then= send > the SEND_STATUS. > + // > + if (Private->Slot[Slot].CurrentFreq < (ClockFreq * 1000)) { > + Status =3D EmmcCheckSwitchStatus (PassThru, Slot, Rca); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + DelaySendStatus =3D FALSE; > + } else { > + DelaySendStatus =3D TRUE; > + } > + > // > // Convert the clock freq unit from MHz to KHz. > // > @@ -732,17 +780,11 @@ EmmcSwitchBusTiming ( > return Status; > } >=20 > - Status =3D EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Send status fails > with %r\n", Status)); > - return Status; > - } > - // > - // Check the switch operation is really successful or not. > - // > - if ((DevStatus & BIT7) !=3D 0) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: The switch operation > fails as DevStatus is 0x%08x\n", DevStatus)); > - return EFI_DEVICE_ERROR; > + if (DelaySendStatus) { > + Status =3D EmmcCheckSwitchStatus (PassThru, Slot, Rca); > + if (EFI_ERROR (Status)) { > + return Status; > + } > } >=20 > return Status; > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > index b18ff3e972..57f4cf329a 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > @@ -28,7 +28,7 @@ EFI_DRIVER_BINDING_PROTOCOL > gSdMmcPciHcDriverBinding =3D { > NULL > }; >=20 > -#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, \ > +#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, 0, \ > {EDKII_SD_MMC_BUS_WIDTH_IGNORE,\ > EDKII_SD_MMC_CLOCK_FREQ_IGNORE,\ > {EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE}}} > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > index 5bc3577ba2..bb3d38482f 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > @@ -83,6 +83,7 @@ typedef struct { > BOOLEAN MediaPresent; > BOOLEAN Initialized; > SD_MMC_CARD_TYPE CardType; > + UINT64 CurrentFreq; > EDKII_SD_MMC_OPERATING_PARAMETERS OperatingParameters; > } SD_MMC_HC_SLOT; >=20 > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > index 43626fff48..7971196a25 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > @@ -931,6 +931,8 @@ SdMmcHcClockSupply ( > } > } >=20 > + Private->Slot[Slot].CurrentFreq =3D ClockFreq; > + > return Status; > } >=20 > -- > 2.14.1.windows.1 >=20 > -------------------------------------------------------------------- >=20 > Intel Technology Poland sp. z o.o. > ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII > Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957- > 07-52-316 | Kapital zakladowy 200.000 PLN. >=20 > Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego > adresata i moze zawierac informacje poufne. W razie przypadkowego > otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale > jej usuniecie; jakiekolwiek > przegladanie lub rozpowszechnianie jest zabronione. > This e-mail and any attachments may contain confidential material for th= e > sole use of the intended recipient(s). If you are not the intended recip= ient, > please contact the sender and delete all copies; any review or distribut= ion by > others is strictly prohibited. >=20 >=20 >=20