> -----Original Message----- > From: Marcin Wojtas [mailto:[email protected]] > Sent: Friday, November 02, 2018 5:39 PM > To: Wu, Hao A > Cc: edk2-devel-01; Tomasz Michalec; [email protected]; Gao, Liming; > Kinney, Michael D > Subject: Re: [edk2] [PATCH v2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add > SwitchClockFreqPost to SdMmcOverride > > Hi Hao, > > czw., 1 lis 2018 o 08:06 Wu, Hao A <[email protected]> napisał(a): > > > > > -----Original Message----- > > > From: edk2-devel [mailto:[email protected]] On Behalf Of > > > Marcin Wojtas > > > Sent: Friday, October 05, 2018 9:25 PM > > > To: [email protected] > > > Cc: Tian, Feng; [email protected]; Wu, Hao A; [email protected]; Gao, > > > Liming; Kinney, Michael D > > > Subject: [edk2] [PATCH v2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add > > > SwitchClockFreqPost to SdMmcOverride > > > > > > From: Tomasz Michalec <[email protected]> > > > > > > Some SD Host Controlers need to do additional opperations after clock > > > frequency switch. > > > > > > This patch add new callback type to NotifyPhase of the SdMmcOverride > > > protocol. It is called after EmmcSwitchClockFreq and SdMmcHcClockSupply. > > > > Hi Marcin, > > > > Just curious, I had a quick glance at the implementation of the > > XenonSwitchClockFreqPost() in your platform part changes. Are those > operations > > within the function mandatory during the HC initialization? Are they mainly > > for > > performance or stability consideration? > > As for Marvellt he Xenon controller is pretty complicated IP, which > consists of standard Sd/Mmc part and the dedicated PHY, that's > responsible for signal integrity for all bus modes. It requires > additional configuration, depending on the mode.
Got it. > > > > > I am wondering if this kind of customization is common among the SD & > eMMC devices. > > Well, in Linux this clock tuning after switching to certain bus mode. > This driver simply does it in 'set_ios' callback, whose custom > implementation (platform-specific code surrounding generic > sdhci_set_ios call) is _very_ common among all drivers/mmc/host > drivers. > > Do you have any objections to the patch itself, given above explanation? No, thanks for the detailed explanation. Best Regards, Hao Wu > > Best regards, > Marcin > > > > > Best Regards, > > Hao Wu > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.1 > > > Signed-off-by: Marcin Wojtas <[email protected]> > > > --- > > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 1 + > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 60 > > > ++++++++++++++++++++ > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++ > > > 3 files changed, 79 insertions(+) > > > > > > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > > b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > > index 25db98a..d9daada 100644 > > > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > > > @@ -33,6 +33,7 @@ typedef enum { > > > EdkiiSdMmcInitHostPre, > > > EdkiiSdMmcInitHostPost, > > > EdkiiSdMmcUhsSignaling, > > > + EdkiiSdMmcSwitchClockFreqPost, > > > } EDKII_SD_MMC_PHASE_TYPE; > > > > > > /** > > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > > index 05bd4a0..7e75283 100755 > > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > > > @@ -796,6 +796,27 @@ EmmcSwitchToHighSpeed ( > > > > > > HsTiming = 1; > > > Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, > > > ClockFreq); > > > + if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + > > > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) { > > > + Status = mOverride->NotifyPhase ( > > > + Private->ControllerHandle, > > > + Slot, > > > + EdkiiSdMmcSwitchClockFreqPost, > > > + &Timing > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG (( > > > + DEBUG_ERROR, > > > + "%a: SD/MMC switch clock freq post notifier callback failed - > > > %r\n", > > > + __FUNCTION__, > > > + Status > > > + )); > > > + return Status; > > > + } > > > + } > > > > > > return Status; > > > } > > > @@ -905,6 +926,24 @@ EmmcSwitchToHS200 ( > > > return Status; > > > } > > > > > > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) { > > > + Status = mOverride->NotifyPhase ( > > > + Private->ControllerHandle, > > > + Slot, > > > + EdkiiSdMmcSwitchClockFreqPost, > > > + &Timing > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG (( > > > + DEBUG_ERROR, > > > + "%a: SD/MMC switch clock freq post notifier callback failed - > > > %r\n", > > > + __FUNCTION__, > > > + Status > > > + )); > > > + return Status; > > > + } > > > + } > > > + > > > Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth); > > > > > > return Status; > > > @@ -989,6 +1028,27 @@ EmmcSwitchToHS400 ( > > > > > > HsTiming = 3; > > > Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, > > > ClockFreq); > > > + if (EFI_ERROR (Status)) { > > > + return Status; > > > + } > > > + > > > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) { > > > + Status = mOverride->NotifyPhase ( > > > + Private->ControllerHandle, > > > + Slot, > > > + EdkiiSdMmcSwitchClockFreqPost, > > > + &Timing > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG (( > > > + DEBUG_ERROR, > > > + "%a: SD/MMC switch clock freq post notifier callback failed - > > > %r\n", > > > + __FUNCTION__, > > > + Status > > > + )); > > > + return Status; > > > + } > > > + } > > > > > > return Status; > > > } > > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > > index 5645a71..057a4e2 100644 > > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > > @@ -887,6 +887,24 @@ SdCardSetBusMode ( > > > return Status; > > > } > > > > > > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) { > > > + Status = mOverride->NotifyPhase ( > > > + Private->ControllerHandle, > > > + Slot, > > > + EdkiiSdMmcSwitchClockFreqPost, > > > + &Timing > > > + ); > > > + if (EFI_ERROR (Status)) { > > > + DEBUG (( > > > + DEBUG_ERROR, > > > + "%a: SD/MMC switch clock freq post notifier callback failed - > > > %r\n", > > > + __FUNCTION__, > > > + Status > > > + )); > > > + return Status; > > > + } > > > + } > > > + > > > if ((AccessMode == 3) || ((AccessMode == 2) && (Capability- > > > >TuningSDR50 != 0))) { > > > Status = SdCardTuningClock (PciIo, PassThru, Slot); > > > if (EFI_ERROR (Status)) { > > > -- > > > 2.7.4 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > [email protected] > > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
