Re: [PATCH v4 0/4] SdMmcOverride extension

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

Hi Marcin,

The code changes look good to me.

Could you please grant me some time for some additional tests for these
patches?
I will inform you with the results sometime next week. Thanks in advance.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@semihalf.com]
> Sent: Saturday, November 10, 2018 7:01 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> Subject: [PATCH v4 0/4] SdMmcOverride extension
> 
> Hi,
> 
> Although I could've waited for Hao's remarks, I think it may
> be better if he takes a look at much cleaner code, which
> addresses v3 review comments.
> The newest version of the patchset cleans-up significantly
> patches 2&3 by removing code duplication and other minor
> improvements.
> 
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/sdmmc-override-upstream-r20181109
> 
> Please note that extending SdMmcOverride protocol was impacting
> so far the only user of it (Synquacer controller). In paralel
> edk2-platforms patchset, a patch can be found:
> ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> which adjust to the new API.
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/xenon-upstream-r20181109
> 
> I'm looking forward to the comments and remarks.
> 
> Best regards,
> Marcin
> 
> Changelog:
> v3->v4
> * 2/4:
>   - avoid duplication by calling SdMmcOverride callback in
> SdMmcHcUhsSignaling
> 
> * 3/4:
>   - avoid duplication by calling SdMmcOverride callback in
> EmmcSwitchClockFreq
> 
> * 4/4:
>   - add Ard's RB
> 
> v2->v3
> * 1/4:
>   - rename new parameter to PhaseData
>   - add Ard's RB
> 
> * 2/4:
>   - s/Controler/Controller/
>   - remove all references to MMC_SDR_50 mode
>   - rename and reorder MMC bus modes
>   - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
>     and move it to protocol header in order to drop including private one
>   - fix if condition in EmmcSwitchToHighSpeed
>   - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
>     callback, so that protocol producer can optionally modify only quirky
>     timing mode values.
> 
> *4/4
>   - bump protocol version to 2
>   - remove redundant assert from SdMmcPciHcDriverBindingStart
>     (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
>   - update comment in SdMmcHcInitClockFreq
>   - restore original DumpCapabilityReg and append
> 
> v1 -> v2
> * Rebase onto newest master
> * 1/4 [new patch] - preparation for extending NotifyPhase
> * 2/4 - UhsSignaling as a part of NotifyPhase
> * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> * 4/4 - Allow updating BaseClkFreq via Capability instead of the
>         independent callback.
> 
> 
> Marcin Wojtas (2):
>   MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
>   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
> 
> Tomasz Michalec (2):
>   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride
> protocol
>   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> SdMmcOverride
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  42 ++++++-
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  29 ++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 121 ++++++++++-
> --------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  35 ++++--
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  13 +-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 124
> +++++++++++++++++---
>  7 files changed, 280 insertions(+), 90 deletions(-)
> 
> --
> 2.7.4