Re: [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start SD clock

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

> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Saturday, December 21, 2019 1:13 AM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A; Marcin Wojtas; Gao, Zhichao; Gao, Liming
> Subject: [PATCH 2/2] MdeModulePkg/SdMmcPciHcDxe: Add function to start
> SD clock
> 
> In SD card voltage switch flow we used to redo the
> entire internal clock setup after voltage switch.
> Since internal clock has already been setup this
> is wasting time on polling the internal clock stable.
> This commit changes it to only start the SD clock.


Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Let us wait to see if other reviewers have additional comments.

Best Regards,
Hao Wu


> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Marcin Wojtas <mw@semihalf.com>
> Cc: Zhichao Gao <zhichao.gao@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> 
> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c    | 11 +++-----
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 33
> ++++++++++++++++++++----
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 15 +++++++++++
>  3 files changed, 47 insertions(+), 12 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index d63dc54e8c..b630daab76 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -1327,13 +1327,10 @@ SdCardIdentification (
>          goto Error;
>        }
> 
> -      //
> -      // Restart the clock with first time parameters.
> -      // NOTE: it is not required to actually restart the clock
> -      // and go through internal clock setup again. Some time
> -      // could be saved if we simply started the SD clock.
> -      //
> -      SdMmcHcClockSupply (Private, Slot, 0, TRUE, 400);
> +      Status = SdMmcHcStartSdClock (PciIo, Slot);
> +      if (EFI_ERROR (Status)) {
> +        goto Error;
> +      }
> 
>        gBS->Stall (1000);
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index f667264c5e..e7f2fac69b 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -758,6 +758,30 @@ SdMmcHcStopClock (
>    return Status;
>  }
> 
> +/**
> +  Start the SD clock.
> +
> +  @param[in] PciIo  The PCI IO protocol instance.
> +  @param[in] Slot   The slot number.
> +
> +  @retval EFI_SUCCESS  Succeeded to start the SD clock.
> +  @rtval  Others       Failed to start the SD clock.
> +**/
> +EFI_STATUS
> +SdMmcHcStartSdClock (
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN UINT8                Slot
> +  )
> +{
> +  UINT16                    ClockCtrl;
> +
> +  //
> +  // Set SD Clock Enable in the Clock Control register to 1
> +  //
> +  ClockCtrl = BIT2;
> +  return SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
> +}
> +
>  /**
>    SD/MMC card clock supply.
> 
> @@ -879,11 +903,10 @@ SdMmcHcClockSupply (
>      return Status;
>    }
> 
> -  //
> -  // Set SD Clock Enable in the Clock Control register to 1
> -  //
> -  ClockCtrl = BIT2;
> -  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof
> (ClockCtrl), &ClockCtrl);
> +  Status = SdMmcHcStartSdClock (PciIo, Slot);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> 
>    //
>    // We don't notify the platform on first time setup to avoid changing
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 826e851b04..4753bb6864 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -478,6 +478,21 @@ SdMmcHcStopClock (
>    IN UINT8                  Slot
>    );
> 
> +/**
> +  Start the SD clock.
> +
> +  @param[in] PciIo  The PCI IO protocol instance.
> +  @param[in] Slot   The slot number.
> +
> +  @retval EFI_SUCCESS  Succeeded to start the SD clock.
> +  @rtval  Others       Failed to start the SD clock.
> +**/
> +EFI_STATUS
> +SdMmcHcStartSdClock (
> +  IN EFI_PCI_IO_PROTOCOL  *PciIo,
> +  IN UINT8                Slot
> +  );
> +
>  /**
>    SD/MMC bus power control.
> 
> --
> 2.14.1.windows.1