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. > > 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? 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

