One small comment below.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, November 30, 2017 6:12 PM
> To: edk2-devel@lists.01.org
> Cc: leif.lindh...@linaro.org; Zeng, Star; Wu, Hao A; Tian, Feng; Ard 
> Biesheuvel
> Subject: [PATCH v2 2/2] MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities
> to be overridden
> 
> Invoke the newly introduced SD/MMC override protocol to override
> the capabilities register after reading it from the device registers,
> and to call the pre/post host init and reset hooks at the appropriate
> times.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> ---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 116
> +++++++++++++++++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   |   6 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |   2 +
>  3 files changed, 119 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index 0be8828abfcc..61f64285807d 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -60,7 +60,8 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = {
>    {                                 // MaxCurrent
>      0,
>    },
> -  0                                 // ControllerVersion
> +  0,                                // ControllerVersion
> +  NULL                              // Override
>  };
> 
>  SD_DEVICE_PATH    mSdDpTemplate = {
> @@ -213,6 +214,92 @@ Done:
>    return;
>  }
> 

Please help to add the function description comments for function:
SdMmcPciHcResetHost() & SdMmcPciHcInitHost()

> +STATIC
> +EFI_STATUS
> +SdMmcPciHcResetHost (
> +  IN  SD_MMC_HC_PRIVATE_DATA          *Private,
> +  IN  UINT8                           Slot
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  if (Private->Override != NULL &&
> +      Private->Override->InvokeHook != NULL) {
> +    Status = Private->Override->InvokeHook (
> +                                  &Private->PassThru,
> +                                  Private->ControllerHandle,
> +                                  Slot,
> +                                  SD_MMC_OVERRIDE_RESET_PRE_HOOK);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: SD/MMC pre reset hook failed - %r\n",
> +        __FUNCTION__, Status));
> +      return Status;
> +    }
> +  }
> +
> +  Status = SdMmcHcReset (Private->PciIo, Slot);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (Private->Override != NULL &&
> +      Private->Override->InvokeHook != NULL) {
> +    Status = Private->Override->InvokeHook (
> +                                  &Private->PassThru,
> +                                  Private->ControllerHandle,
> +                                  Slot,
> +                                  SD_MMC_OVERRIDE_RESET_POST_HOOK);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: SD/MMC post reset hook failed - %r\n",
> +        __FUNCTION__, Status));
> +    }
> +  }
> +  return Status;
> +}
> +
> +STATIC
> +EFI_STATUS
> +SdMmcPciHcInitHost (
> +  IN  SD_MMC_HC_PRIVATE_DATA          *Private,
> +  IN  UINT8                           Slot
> +  )
> +{
> +  EFI_STATUS    Status;
> +
> +  if (Private->Override != NULL &&
> +      Private->Override->InvokeHook != NULL) {
> +    Status = Private->Override->InvokeHook (
> +                                  &Private->PassThru,
> +                                  Private->ControllerHandle,
> +                                  Slot,
> +                                  SD_MMC_OVERRIDE_INIT_HOST_PRE_HOOK);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: SD/MMC pre init hook failed - %r\n",
> +        __FUNCTION__, Status));
> +      return Status;
> +    }
> +  }
> +
> +  Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (Private->Override != NULL &&
> +      Private->Override->InvokeHook != NULL) {
> +    Status = Private->Override->InvokeHook (
> +                                  &Private->PassThru,
> +                                  Private->ControllerHandle,
> +                                  Slot,
> +                                  SD_MMC_OVERRIDE_INIT_HOST_POST_HOOK);
> +    if (EFI_ERROR (Status)) {
> +      DEBUG ((DEBUG_WARN, "%a: SD/MMC post init hook failed - %r\n",
> +        __FUNCTION__, Status));
> +    }
> +  }
> +  return Status;
> +}
> +
>  /**
>    Sd removable device enumeration callback function when the timer event is
> signaled.
> 
> @@ -281,14 +368,14 @@ SdMmcPciHcEnumerateDevice (
>          //
>          // Reset the specified slot of the SD/MMC Pci Host Controller
>          //
> -        Status = SdMmcHcReset (Private->PciIo, Slot);
> +        Status = SdMmcPciHcResetHost (Private, Slot);
>          if (EFI_ERROR (Status)) {
>            continue;
>          }
>          //
>          // Reinitialize slot and restart identification process for the new 
> attached
> device
>          //
> -        Status = SdMmcHcInitHost (Private->PciIo, Slot, 
> Private->Capability[Slot]);
> +        Status = SdMmcPciHcInitHost (Private, Slot);
>          if (EFI_ERROR (Status)) {
>            continue;
>          }
> @@ -601,6 +688,12 @@ SdMmcPciHcDriverBindingStart (
>      goto Done;
>    }
> 
> +  Status = gBS->HandleProtocol (Controller,
> &gEdkiiSdMmcOverrideProtocolGuid,
> +                  (VOID **)&Private->Override);
> +  if (!EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_INFO, "%a: using SD/MMC override protocol\n",
> __FUNCTION__));
> +  }
> +
>    Support64BitDma = TRUE;
>    for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
>      Private->Slot[Slot].Enable = TRUE;
> @@ -609,6 +702,19 @@ SdMmcPciHcDriverBindingStart (
>      if (EFI_ERROR (Status)) {
>        continue;
>      }
> +    if (Private->Override != NULL &&
> +        Private->Override->OverrideCapability != NULL) {
> +      Status = Private->Override->OverrideCapability (
> +                                    &Private->PassThru,
> +                                    Controller,
> +                                    Slot,
> +                                    (VOID *)&Private->Capability[Slot]);
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
> +          __FUNCTION__, Status));
> +        continue;
> +      }
> +    }
>      DumpCapabilityReg (Slot, &Private->Capability[Slot]);
> 
>      Support64BitDma &= Private->Capability[Slot].SysBus64;
> @@ -627,7 +733,7 @@ SdMmcPciHcDriverBindingStart (
>      //
>      // Reset the specified slot of the SD/MMC Pci Host Controller
>      //
> -    Status = SdMmcHcReset (PciIo, Slot);
> +    Status = SdMmcPciHcResetHost (Private, Slot);
>      if (EFI_ERROR (Status)) {
>        continue;
>      }
> @@ -642,7 +748,7 @@ SdMmcPciHcDriverBindingStart (
>        continue;
>      }
> 
> -    Status = SdMmcHcInitHost (PciIo, Slot, Private->Capability[Slot]);
> +    Status = SdMmcPciHcInitHost (Private, Slot);
>      if (EFI_ERROR (Status)) {
>        continue;
>      }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index 6a2a27969936..b51e0529f885 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -35,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include <Protocol/DriverBinding.h>
>  #include <Protocol/ComponentName.h>
>  #include <Protocol/ComponentName2.h>
> +#include <Protocol/SdMmcOverride.h>
>  #include <Protocol/SdMmcPassThru.h>
> 
>  #include "SdMmcPciHci.h"
> @@ -115,6 +116,11 @@ typedef struct {
>    UINT64                              MaxCurrent[SD_MMC_HC_MAX_SLOT];
> 
>    UINT32                              ControllerVersion;
> +
> +  //
> +  // Optional protcol to deal with non-standard SDHCI implementations
> +  //
> +  SD_MMC_OVERRIDE                     *Override;
>  } SD_MMC_HC_PRIVATE_DATA;
> 
>  #define SD_MMC_HC_TRB_SIG             SIGNATURE_32 ('T', 'R', 'B', 'T')
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> index e26e6a098c17..154ce45d8223 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
> @@ -48,6 +48,7 @@ [Sources]
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> 
>  [LibraryClasses]
>    DevicePathLib
> @@ -61,6 +62,7 @@ [LibraryClasses]
>    DebugLib
> 
>  [Protocols]
> +  gEdkiiSdMmcOverrideProtocolGuid               ## SOMETIMES_CONSUMES
>    gEfiDevicePathProtocolGuid                    ## TO_START
>    gEfiPciIoProtocolGuid                         ## TO_START
>    gEfiSdMmcPassThruProtocolGuid                 ## BY_START
> --
> 2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to