From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web09.6650.1583373247759606360 for ; Wed, 04 Mar 2020 17:54:07 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Mar 2020 17:54:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.70,516,1574150400"; d="scan'208";a="439335000" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga005.fm.intel.com with ESMTP; 04 Mar 2020 17:54:07 -0800 Received: from fmsmsx162.amr.corp.intel.com (10.18.125.71) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Mar 2020 17:54:07 -0800 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by fmsmsx162.amr.corp.intel.com (10.18.125.71) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 4 Mar 2020 17:54:06 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.206]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.232]) with mapi id 14.03.0439.000; Thu, 5 Mar 2020 09:54:03 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "Wu, Hao A" , "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+0kBtLa75s5J02eygid22e4t6gspGGQgAylglA= Date: Thu, 5 Mar 2020 01:54:03 +0000 Message-ID: References: <20200225150553.6040-1-mateusz.albecki@intel.com> <20200225150553.6040-2-mateusz.albecki@intel.com> In-Reply-To: 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 > Wu, Hao A > Sent: Wednesday, February 26, 2020 8:51 AM > 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 >=20 > > -----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, > Liming > > Subject: [edk2-devel] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe: > Send > > SEND_STATUS at lower frequency > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1140 > > > > To avoid stability issues on some designs the driver > > will now send SEND_STATUS at previous, lower, frequency > > when upgrading the bus timing. >=20 >=20 > Reviewed-by: Hao A Wu > I will push this patch *after* the upcoming edk2-stable202002 tag. Patch has been pushed via commit 643623147a. Best Regards, Hao Wu >=20 > Best Regards, > Hao Wu >=20 >=20 > > > > Cc: Hao A Wu > > Cc: Marcin Wojtas > > Cc: Zhichao Gao > > Cc: Liming Gao > > > > 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(-) > > > > 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; > > } > > > > +/** > > + 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 sen= t. > > + @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. > > > > @@ -591,7 +628,6 @@ EmmcSwitchBusWidth ( > > UINT8 Index; > > UINT8 Value; > > UINT8 CmdSet; > > - UINT32 DevStatus; > > > > // > > // Write Byte, the Value field is written into the byte pointed by = Index. > > @@ -617,18 +653,10 @@ EmmcSwitchBusWidth ( > > return Status; > > } > > > > - 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; > > - } > > > > Status =3D SdMmcHcSetBusWidth (PciIo, Slot, BusWidth); > > > > @@ -669,9 +697,9 @@ EmmcSwitchBusTiming ( > > UINT8 Index; > > UINT8 Value; > > UINT8 CmdSet; > > - UINT32 DevStatus; > > SD_MMC_HC_PRIVATE_DATA *Private; > > UINT8 HostCtrl1; > > + BOOLEAN DelaySendStatus; > > > > 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; > > } > > > > @@ -724,6 +752,26 @@ EmmcSwitchBusTiming ( > > return Status; > > } > > > > + // > > + // 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 una= ble 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 t= arget > > 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 th= en > 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; > > } > > > > - 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; > > + } > > } > > > > 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 > > }; > > > > -#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; > > > > 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 ( > > } > > } > > > > + Private->Slot[Slot].CurrentFreq =3D ClockFreq; > > + > > return Status; > > } > > > > -- > > 2.14.1.windows.1 > > > > -------------------------------------------------------------------- > > > > 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. > > > > 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 = the > > sole use of the intended recipient(s). If you are not the intended rec= ipient, > > please contact the sender and delete all copies; any review or distrib= ution > by > > others is strictly prohibited. > > > > > > >=20 >=20 >=20