* [PATCH 0/1] Correcting eMMC HS400 switch sequence @ 2019-02-15 14:44 Albecki, Mateusz 2019-02-15 14:44 ` [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix " Albecki, Mateusz 0 siblings, 1 reply; 3+ messages in thread From: Albecki, Mateusz @ 2019-02-15 14:44 UTC (permalink / raw) To: edk2-devel; +Cc: Albecki, Mateusz, Hao Wu SdMmcPchHcDxe used to execute SEND_STATUS with link running at incorrect clock frequency while downgrading from HS200 to high speed during HS400 switch sequence. Changing clock frequency before executing SEND_STATUS makes sure that SEND_STATUS is executed with link at correct frequency. Since new frequency might not be suitable for old bus timing this change is not sufficient for protecting against failed bus timing change but seeing as the rest of the driver is unable to mitigate failed bus timing change it is of no importance. Cc: Hao Wu <hao.a.wu@intel.com> Albecki, Mateusz (1): MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 37 +++++++++++++------------ 1 file changed, 19 insertions(+), 18 deletions(-) -- 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 recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence 2019-02-15 14:44 [PATCH 0/1] Correcting eMMC HS400 switch sequence Albecki, Mateusz @ 2019-02-15 14:44 ` Albecki, Mateusz 2019-02-18 6:00 ` Wu, Hao A 0 siblings, 1 reply; 3+ messages in thread From: Albecki, Mateusz @ 2019-02-15 14:44 UTC (permalink / raw) To: edk2-devel; +Cc: Albecki, Mateusz, Hao Wu In eMMC HS400 switch sequence flow eMMC driver attampted to execute SEND_STATUS just after switching bus timing to high speed and before downgrading clock frequency to 52MHz. Since link was at that time in incorrect state SEND_STATUS was failing which made driver think switch to HS400 failed. This change makes driver always change clock frequency after switching bus timing and before executing SEND_STATUS. Change-Id: Ib1f2c78a8693c2a4ef6e1f1b5da2f4133f6210d2 Cc: Hao Wu <hao.a.wu@intel.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Albecki Mateusz <mateusz.albecki@intel.com> --- MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 37 +++++++++++++------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c index 4ef849fd0962..22200f806f26 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c @@ -642,7 +642,7 @@ EmmcSwitchBusWidth ( } /** - Switch the clock frequency to the specified value. + Switch the bus timing and clock frequency. Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host Controller Simplified Spec 3.0 Figure 3-3 for details. @@ -660,7 +660,7 @@ EmmcSwitchBusWidth ( **/ EFI_STATUS -EmmcSwitchClockFreq ( +EmmcSwitchBusTiming ( IN EFI_PCI_IO_PROTOCOL *PciIo, IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, @@ -693,18 +693,6 @@ EmmcSwitchClockFreq ( return Status; } - Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails with %r\n", Status)); - return Status; - } - // - // Check the switch operation is really successful or not. - // - if ((DevStatus & BIT7) != 0) { - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation fails as DevStatus is 0x%08x\n", DevStatus)); - return EFI_DEVICE_ERROR; - } // // Convert the clock freq unit from MHz to KHz. // @@ -713,6 +701,19 @@ EmmcSwitchClockFreq ( return Status; } + Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails with %r\n", Status)); + return Status; + } + // + // Check the switch operation is really successful or not. + // + if ((DevStatus & BIT7) != 0) { + DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation fails as DevStatus is 0x%08x\n", DevStatus)); + return EFI_DEVICE_ERROR; + } + if (mOverride != NULL && mOverride->NotifyPhase != NULL) { Status = mOverride->NotifyPhase ( Private->ControllerHandle, @@ -799,7 +800,7 @@ EmmcSwitchToHighSpeed ( } HsTiming = 1; - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); return Status; } @@ -887,7 +888,7 @@ EmmcSwitchToHS200 ( Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl); HsTiming = 2; - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); if (EFI_ERROR (Status)) { return Status; } @@ -937,7 +938,7 @@ EmmcSwitchToHS400 ( // Set to Hight Speed timing and set the clock frequency to a value less than 52MHz. // HsTiming = 1; - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52); + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52); if (EFI_ERROR (Status)) { return Status; } @@ -957,7 +958,7 @@ EmmcSwitchToHS400 ( } HsTiming = 3; - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, 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 recipient, please contact the sender and delete all copies; any review or distribution by others is strictly prohibited. ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence 2019-02-15 14:44 ` [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix " Albecki, Mateusz @ 2019-02-18 6:00 ` Wu, Hao A 0 siblings, 0 replies; 3+ messages in thread From: Wu, Hao A @ 2019-02-18 6:00 UTC (permalink / raw) To: Albecki, Mateusz, edk2-devel@lists.01.org Hi Mateusz, I found that the patch proposed is actually handling one of the issues reported in the below BZ tracker: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 According to the discussion within the tracker, there are 2 fixes needed for the SdMmcPciHcDxe: 1. Move the clock supply before doing send status 2. More robust handle to the send status CRC error on the switch to HS200 I think the proposed patch is handling the 1st issue listed above. So could you help to add the below line: REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 at the beginning of your commit log message? Also, please see the below inline comments below: > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of > Albecki, Mateusz > Sent: Friday, February 15, 2019 10:45 PM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A > Subject: [edk2] [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC > HS400 switch sequence > > In eMMC HS400 switch sequence flow eMMC driver attampted 'attampted' -> 'attempted' > to execute SEND_STATUS just after switching bus timing to high > speed and before downgrading clock frequency to 52MHz. Since link > was at that time in incorrect state SEND_STATUS was failing which > made driver think switch to HS400 failed. > This change makes driver always change clock frequency after > switching bus timing and before executing SEND_STATUS. > > Change-Id: Ib1f2c78a8693c2a4ef6e1f1b5da2f4133f6210d2 Please help to remove the above 'Change-Id' information. > Cc: Hao Wu <hao.a.wu@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Albecki Mateusz <mateusz.albecki@intel.com> > --- > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 37 > +++++++++++++------------ > 1 file changed, 19 insertions(+), 18 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > index 4ef849fd0962..22200f806f26 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > @@ -642,7 +642,7 @@ EmmcSwitchBusWidth ( > } > > /** > - Switch the clock frequency to the specified value. > + Switch the bus timing and clock frequency. > > Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host > Controller > Simplified Spec 3.0 Figure 3-3 for details. > @@ -660,7 +660,7 @@ EmmcSwitchBusWidth ( > > **/ > EFI_STATUS > -EmmcSwitchClockFreq ( > +EmmcSwitchBusTiming ( Could you help to update the debug messages that include keyword 'EmmcSwitchClockFreq' to 'EmmcSwitchBusTiming' to align with the function name change? Best Regards, Hao Wu > IN EFI_PCI_IO_PROTOCOL *PciIo, > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > @@ -693,18 +693,6 @@ EmmcSwitchClockFreq ( > return Status; > } > > - Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails > with %r\n", Status)); > - return Status; > - } > - // > - // Check the switch operation is really successful or not. > - // > - if ((DevStatus & BIT7) != 0) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation > fails as DevStatus is 0x%08x\n", DevStatus)); > - return EFI_DEVICE_ERROR; > - } > // > // Convert the clock freq unit from MHz to KHz. > // > @@ -713,6 +701,19 @@ EmmcSwitchClockFreq ( > return Status; > } > > + Status = EmmcSendStatus (PassThru, Slot, Rca, &DevStatus); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Send status fails > with %r\n", Status)); > + return Status; > + } > + // > + // Check the switch operation is really successful or not. > + // > + if ((DevStatus & BIT7) != 0) { > + DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: The switch operation > fails as DevStatus is 0x%08x\n", DevStatus)); > + return EFI_DEVICE_ERROR; > + } > + > if (mOverride != NULL && mOverride->NotifyPhase != NULL) { > Status = mOverride->NotifyPhase ( > Private->ControllerHandle, > @@ -799,7 +800,7 @@ EmmcSwitchToHighSpeed ( > } > > HsTiming = 1; > - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, > Timing, ClockFreq); > > return Status; > } > @@ -887,7 +888,7 @@ EmmcSwitchToHS200 ( > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof > (ClockCtrl), &ClockCtrl); > > HsTiming = 2; > - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, > Timing, ClockFreq); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -937,7 +938,7 @@ EmmcSwitchToHS400 ( > // Set to Hight Speed timing and set the clock frequency to a value less than > 52MHz. > // > HsTiming = 1; > - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, > SdMmcMmcHsSdr, 52); > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, > SdMmcMmcHsSdr, 52); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -957,7 +958,7 @@ EmmcSwitchToHS400 ( > } > > HsTiming = 3; > - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, > Timing, 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 recipient, > please contact the sender and delete all copies; any review or distribution by > others is strictly prohibited. > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-02-18 6:00 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-02-15 14:44 [PATCH 0/1] Correcting eMMC HS400 switch sequence Albecki, Mateusz 2019-02-15 14:44 ` [PATCH 1/1] MdeModulePkg/SdMmcPciHcDxe Fix " Albecki, Mateusz 2019-02-18 6:00 ` Wu, Hao A
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox