Re: [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol

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

Except a minor comment in patch 2/2, the series looks good to me.

Hello Ard and Marcin, please let me know if you have comments on this
series.

If no other feedbacks, I plan to push the change at early next week.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Albecki, Mateusz
> Sent: Wednesday, June 26, 2019 9:10 PM
> To: devel@edk2.groups.io
> Cc: Albecki, Mateusz; Wu, Hao A
> Subject: [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision
> 3 of SdMmcOverrideProtocol
> 
> To allow platform greater control over the bus settings for SD card and
> eMMC card we have added a new notify phase to SdMmcOverrideProtocol
> called GetOperatingParam.
> This phase is signaled before SD card/eMMC initialization and allows platform
> to tweak the values in new structure called
> EDKII_SD_MMC_OPERATING_PARAMETERS which allows to configure bus
> width, clock frequency and driver strength.
> Other bus parameters can be configured by overriding host controller
> capabilities.
> 
> Changes in v2:
> - Fixed stylistic issues and documentation issues pointed out in v1 review
> - Changed bus width to be UINT8
> - Reordered the SD_MMC_BUS_MODE to fix problem with driver never
> choosing the DDR50 speed mode
> - Fixed the bug with driver not switching the controller into correct driver
> strength for SD card slots
> - Fixed bug with the driver not considering driver strength support for SD
> card slots
> - Fixed bug with driver choosing SdMmcMmcHsSdr if card doesn't support
> frequencies above 26MHz
> - Fixed bug with driver choosing driver strength for legacy speed modes
> 
> Changes in v3:
> - Changed BusWidth field of EDKII_SD_MMC_OPERATING_PARAMETERS to
> UINT16
> 
> Changes in v4
> - Fixed typos and ordering in SD_MMC_BUS_MODE(this time for real)
> - Fixed non boolean types usage in if statements
> - Fixed code that checks if there was an error in switch response for SD card.
> The code has been updated to check if switch failed due to function being
> busy
> 
> Tests:
> - OS boot from eMMC without SdMmcOverride protocol installed
> - OS boot from eMMC with SdMmcOverride installed and clock frequency
> lowered to 100MHz in HS200
> - OS boot from eMMC with driver strength changed to Type1
> - OS boot from eMMC in HS400 without override protocol installed
> - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
> - SD card enumeration in UEFI shell on default speed and high speed(non
> UHS-I) with no Override protocol(legacy card used)
> 
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> 
> 
> Albecki, Mateusz (1):
>   MdeModulePkg/SdMmcHcDxe: Implement revision 3 of
> SdMmcOverrideProtocol
> 
> Mateusz Albecki (1):
>   MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 512
> +++++++++++++++------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 410
> ++++++++++++++---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  34 ++
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  19 +
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  60 ++-
>  7 files changed, 867 insertions(+), 238 deletions(-)
> 
> --
> 2.14.1.windows.1