Re: [PATCH v3 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase

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

> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Friday, June 21, 2019 11:12 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A
> Subject: [PATCH v3 1/2] MdeModulePkg/SdMmcOverride: Add
> GetOperatingParam notify phase
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1882
> 
> The new notify phase allows platform to configure additional
> bus paramters in addition to parameters that can already be configured
> with capability override. Specifically we allow to configure bus width,
> clock frequency and driver strength. If platform doesn't wish to configure
> some of the parameters it can left it on default values and driver will
> assume it's standard behavior with respect to those parameters.
> The definition of the SD_MMC_BUS_MODE has been extended to
> incorporate SD card default speed and high speed.


Hello Mateusz,

For "Reordered the SD_MMC_BUS_MODE to fix problem with driver never
choosing the DDR50 speed mode", it seems to me that the change is not
covered by this patch. Did I miss it?

Also, a couple of typos:
unsuportted -> unsupported
choosen -> chosen

Others look good to me.

Best Regards,
Hao Wu


> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com>
> ---
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h | 58
> ++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> index 9c8bf37efd..1cf27008ea 100644
> --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> @@ -16,14 +16,61 @@
>  #define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \
>    { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23,
> 0x23 } }
> 
> -#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION    0x2
> +#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION    0x3
> 
>  typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
> 
> -//
> -// Bus timing modes
> -//
> +#define EDKII_SD_MMC_BUS_WIDTH_IGNORE MAX_UINT8
> +#define EDKII_SD_MMC_CLOCK_FREQ_IGNORE MAX_UINT32
> +#define EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE  MAX_UINT8
> +
> +typedef enum {
> +  SdDriverStrengthTypeB        = 0,
> +  SdDriverStrengthTypeA,
> +  SdDriverStrengthTypeC,
> +  SdDriverStrengthTypeD,
> +  SdDriverStrengthIgnore = EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE
> +} SD_DRIVER_STRENGTH_TYPE;
> +
>  typedef enum {
> +  EmmcDriverStrengthType0      = 0,
> +  EmmcDriverStrengthType1,
> +  EmmcDriverStrengthType2,
> +  EmmcDriverStrengthType3,
> +  EmmcDriverStrengthType4,
> +  EmmcDriverStrengthIgnore =
> EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE
> +} EMMC_DRIVER_STRENGTH_TYPE;
> +
> +typedef union {
> +  SD_DRIVER_STRENGTH_TYPE      Sd;
> +  EMMC_DRIVER_STRENGTH_TYPE    Emmc;
> +} EDKII_SD_MMC_DRIVER_STRENGTH;
> +
> +typedef struct {
> +  //
> +  // The target width of the bus. If user tells driver to ignore it
> +  // or specifies unsuportted width driver will choose highest supported
> +  // bus width for a given mode.
> +  //
> +  UINT8                         BusWidth;
> +  //
> +  // The target clock frequency of the bus in MHz. If user tells driver to
> ignore
> +  // it or specifies unsupported frequency driver will choose highest
> supported
> +  // clock frequency for a given mode.
> +  //
> +  UINT32                        ClockFreq;
> +  //
> +  // The target driver strength of the bus. If user tells driver to
> +  // ignore it or specifies unsupported driver strength, driver will
> +  // default to Type0 for eMMC cards and TypeB for SD cards. Driver
> strength
> +  // setting is only considered if choosen bus timing supports them.
> +  //
> +  EDKII_SD_MMC_DRIVER_STRENGTH  DriverStrength;
> +} EDKII_SD_MMC_OPERATING_PARAMETERS;
> +
> +typedef enum {
> +  SdMmcSdDs,
> +  SdMmcSdHs,
>    SdMmcUhsSdr12,
>    SdMmcUhsSdr25,
>    SdMmcUhsSdr50,
> @@ -43,10 +90,10 @@ typedef enum {
>    EdkiiSdMmcInitHostPost,
>    EdkiiSdMmcUhsSignaling,
>    EdkiiSdMmcSwitchClockFreqPost,
> +  EdkiiSdMmcGetOperatingParam
>  } EDKII_SD_MMC_PHASE_TYPE;
> 
>  /**
> -
>    Override function for SDHCI capability bits
> 
>    @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
> @@ -70,7 +117,6 @@ EFI_STATUS
>    );
> 
>  /**
> -
>    Override function for SDHCI controller operations
> 
>    @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
> --
> 2.14.1.windows.1