* [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence [not found] <cover.1550487993.git.mateusz.albecki@intel.com> @ 2019-02-18 11:11 ` Albecki, Mateusz 2019-02-20 1:11 ` Wu, Hao A 0 siblings, 1 reply; 4+ messages in thread From: Albecki, Mateusz @ 2019-02-18 11:11 UTC (permalink / raw) To: edk2-devel; +Cc: Albecki, Mateusz, Hao Wu REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 In eMMC HS400 switch sequence flow eMMC driver 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. 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 | 39 +++++++++++++------------ 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c index 4ef849fd0962..15db8a87a5c4 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, @@ -689,22 +689,10 @@ EmmcSwitchClockFreq ( Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to hstiming %d fails with %r\n", HsTiming, Status)); + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d fails with %r\n", HsTiming, Status)); 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, "EmmcSwitchBusTiming: 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, "EmmcSwitchBusTiming: 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] 4+ messages in thread
* Re: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence 2019-02-18 11:11 ` [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence Albecki, Mateusz @ 2019-02-20 1:11 ` Wu, Hao A 2019-02-21 20:57 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 4+ messages in thread From: Wu, Hao A @ 2019-02-20 1:11 UTC (permalink / raw) To: Albecki, Mateusz, edk2-devel@lists.01.org Thanks Mateusz, Reviewed-by: Hao Wu <hao.a.wu@intel.com> And pushed via commit 68c67d3a2a33261e41ff0123129b4e9759617f71. Best Regards, Hao Wu > -----Original Message----- > From: Albecki, Mateusz > Sent: Monday, February 18, 2019 7:12 PM > To: edk2-devel@lists.01.org > Cc: Albecki, Mateusz; Wu, Hao A > Subject: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 > switch sequence > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 > > In eMMC HS400 switch sequence flow eMMC driver 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. > > 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 | 39 > +++++++++++++------------ > 1 file changed, 20 insertions(+), 19 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > index 4ef849fd0962..15db8a87a5c4 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, > @@ -689,22 +689,10 @@ EmmcSwitchClockFreq ( > > Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); > if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to hstiming %d > fails with %r\n", HsTiming, Status)); > + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d > fails with %r\n", HsTiming, Status)); > 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, "EmmcSwitchBusTiming: 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, "EmmcSwitchBusTiming: 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence 2019-02-20 1:11 ` Wu, Hao A @ 2019-02-21 20:57 ` Philippe Mathieu-Daudé 2019-02-22 0:18 ` Wu, Hao A 0 siblings, 1 reply; 4+ messages in thread From: Philippe Mathieu-Daudé @ 2019-02-21 20:57 UTC (permalink / raw) To: Wu, Hao A, edk2-devel@lists.01.org Hi Hao A, On 2/20/19 2:11 AM, Wu, Hao A wrote: > Thanks Mateusz, > > Reviewed-by: Hao Wu <hao.a.wu@intel.com> > And pushed via commit 68c67d3a2a33261e41ff0123129b4e9759617f71. I think you mean "pushed 'after' commit 68c67d3a2a33261e41ff0123129b4e9759617f71". Commit 68c67d3a2a33261e41ff0123129b4e9759617f71 is: "BaseTools: Fixed a code bug for Pcd Array", while this patch is commited as 195f673f6270aaf73dd34b75f1da26451b63c316. Now about this commit 195f673f6270aaf, I think you have an issue with your configuration, as the patch author was remplaced from: Albecki, Mateusz <mateusz.albecki@intel.com> to: Albecki, Mateusz </o=Intel/ou=Exchange Administrative Group (FYDIBOHF23SPDLT)/cn=Recipients/cn=Albecki, Mateusz3be> Regards, Phil. > > Best Regards, > Hao Wu > > >> -----Original Message----- >> From: Albecki, Mateusz >> Sent: Monday, February 18, 2019 7:12 PM >> To: edk2-devel@lists.01.org >> Cc: Albecki, Mateusz; Wu, Hao A >> Subject: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 >> switch sequence >> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 >> >> In eMMC HS400 switch sequence flow eMMC driver 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. >> >> 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 | 39 >> +++++++++++++------------ >> 1 file changed, 20 insertions(+), 19 deletions(-) >> >> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c >> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c >> index 4ef849fd0962..15db8a87a5c4 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, >> @@ -689,22 +689,10 @@ EmmcSwitchClockFreq ( >> >> Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); >> if (EFI_ERROR (Status)) { >> - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to hstiming %d >> fails with %r\n", HsTiming, Status)); >> + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d >> fails with %r\n", HsTiming, Status)); >> 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, "EmmcSwitchBusTiming: 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, "EmmcSwitchBusTiming: 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 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence 2019-02-21 20:57 ` Philippe Mathieu-Daudé @ 2019-02-22 0:18 ` Wu, Hao A 0 siblings, 0 replies; 4+ messages in thread From: Wu, Hao A @ 2019-02-22 0:18 UTC (permalink / raw) To: Philippe Mathieu-Daudé, edk2-devel@lists.01.org > -----Original Message----- > From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com] > Sent: Friday, February 22, 2019 4:57 AM > To: Wu, Hao A; edk2-devel@lists.01.org > Cc: Albecki, Mateusz > Subject: Re: [edk2] [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix > eMMC HS400 switch sequence > > Hi Hao A, > > On 2/20/19 2:11 AM, Wu, Hao A wrote: > > Thanks Mateusz, > > > > Reviewed-by: Hao Wu <hao.a.wu@intel.com> > > And pushed via commit 68c67d3a2a33261e41ff0123129b4e9759617f71. > > I think you mean "pushed 'after' commit > 68c67d3a2a33261e41ff0123129b4e9759617f71". Yes, the commit should have been 195f673f6270aaf73dd34b75f1da26451b63c316. It is my mistake. > > Commit 68c67d3a2a33261e41ff0123129b4e9759617f71 is: > "BaseTools: Fixed a code bug for Pcd Array", while > this patch is commited as 195f673f6270aaf73dd34b75f1da26451b63c316. > > Now about this commit 195f673f6270aaf, I think you have an issue with > your configuration, as the patch author was remplaced from: > Albecki, Mateusz <mateusz.albecki@intel.com> > to: > Albecki, Mateusz </o=Intel/ou=Exchange Administrative Group > (FYDIBOHF23SPDLT)/cn=Recipients/cn=Albecki, Mateusz3be> You are right, there is something wrong with my local settings when abstracting the patch. Thanks for pointing out. Best Regards, Hao Wu > > Regards, > > Phil. > > > > > Best Regards, > > Hao Wu > > > > > >> -----Original Message----- > >> From: Albecki, Mateusz > >> Sent: Monday, February 18, 2019 7:12 PM > >> To: edk2-devel@lists.01.org > >> Cc: Albecki, Mateusz; Wu, Hao A > >> Subject: [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC > HS400 > >> switch sequence > >> > >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1140 > >> > >> In eMMC HS400 switch sequence flow eMMC driver 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. > >> > >> 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 | 39 > >> +++++++++++++------------ > >> 1 file changed, 20 insertions(+), 19 deletions(-) > >> > >> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > >> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > >> index 4ef849fd0962..15db8a87a5c4 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, > >> @@ -689,22 +689,10 @@ EmmcSwitchClockFreq ( > >> > >> Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); > >> if (EFI_ERROR (Status)) { > >> - DEBUG ((DEBUG_ERROR, "EmmcSwitchClockFreq: Switch to > hstiming %d > >> fails with %r\n", HsTiming, Status)); > >> + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to > hstiming %d > >> fails with %r\n", HsTiming, Status)); > >> 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, "EmmcSwitchBusTiming: 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, "EmmcSwitchBusTiming: 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 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-22 0:18 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1550487993.git.mateusz.albecki@intel.com>
2019-02-18 11:11 ` [PATCHv2 1/1] MdeModulePkg/SdMmcPciHcDxe Fix eMMC HS400 switch sequence Albecki, Mateusz
2019-02-20 1:11 ` Wu, Hao A
2019-02-21 20:57 ` Philippe Mathieu-Daudé
2019-02-22 0:18 ` 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