Re: [PATCH v2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride

["Wu, Hao A" <hao.a.wu@intel.com>]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Marcin Wojtas
> Sent: Friday, October 05, 2018 9:25 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; tm@semihalf.com; Wu, Hao A; nadavh@marvell.com; Gao,
> Liming; Kinney, Michael D
> Subject: [edk2] [PATCH v2 3/4] MdeModulePkg/SdMmcPciHcDxe: Add
> SwitchClockFreqPost to SdMmcOverride
> 
> From: Tomasz Michalec <tm@semihalf.com>
> 
> 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?

I am wondering if this kind of customization is common among the SD & eMMC
devices.

Best Regards,
Hao Wu

> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
>  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
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel