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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Sat, 10 Nov 2018 at 00:02, Marcin Wojtas <mw@semihalf.com> wrote:
>
> From: Tomasz Michalec <tm@semihalf.com>
>
> Some SD Host Controlers need to do additional opperations

operations

> after clock
> frequency switch.
>
> This patch add new callback type to NotifyPhase of the SdMmcOverride
> protocol. It is called after SdMmcHcClockSupply.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h   |  1 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 31 +++++++++++++++++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 18 ++++++++++++
>  3 files changed, 46 insertions(+), 4 deletions(-)
>
> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> index f948bef..6160b5b 100644
> --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> @@ -48,6 +48,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 db4e8a1..b75a9bb 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -651,6 +651,7 @@ EmmcSwitchBusWidth (
>    @param[in] Slot           The slot number of the SD card to send the command to.
>    @param[in] Rca            The relative device address to be assigned.
>    @param[in] HsTiming       The value to be written to HS_TIMING field of EXT_CSD register.
> +  @param[in] Timing         The bus mode timing indicator.
>    @param[in] ClockFreq      The max clock frequency to be set, the unit is MHz.
>
>    @retval EFI_SUCCESS       The operation is done correctly.
> @@ -664,6 +665,7 @@ EmmcSwitchClockFreq (
>    IN UINT8                              Slot,
>    IN UINT16                             Rca,
>    IN UINT8                              HsTiming,
> +  IN SD_MMC_BUS_MODE                    Timing,
>    IN UINT32                             ClockFreq
>    )
>  {
> @@ -706,6 +708,27 @@ EmmcSwitchClockFreq (
>    // Convert the clock freq unit from MHz to KHz.
>    //
>    Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
> +  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;
>  }
> @@ -775,7 +798,7 @@ EmmcSwitchToHighSpeed (
>    }
>
>    HsTiming = 1;
> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> +  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
>
>    return Status;
>  }
> @@ -863,7 +886,7 @@ EmmcSwitchToHS200 (
>    Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
>
>    HsTiming = 2;
> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> +  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> @@ -913,7 +936,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, 52);
> +  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52);
>    if (EFI_ERROR (Status)) {
>      return Status;
>    }
> @@ -933,7 +956,7 @@ EmmcSwitchToHS400 (
>    }
>
>    HsTiming = 3;
> -  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> +  Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
>
>    return Status;
>  }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 55b3564..32fd416 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -869,6 +869,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
>