Re: [PATCH 0/2] Add GetOperatingParam notify phase to SdMmcOverride protocol

["Marcin Wojtas" <mw@semihalf.com>]

[-- Attachment #1: Type: text/plain, Size: 6206 bytes --]

Hi Hao,

Please give me some time to try and review - I hope to find some slot next
week.

Best regards,
Marcin

pt., 14 cze 2019 o 03:46 Wu, Hao A <hao.a.wu@intel.com> napisał(a):

> Loop in Ard and Marcin.
>
>
>
>
>
> Hello Ard and Marcin,
>
>
>
> We are proposing to make an extension to the SDMMC Override protocol to
>
> more customization for platforms during the SD/eMMC device/controller
>
> initialization process.
>
>
>
> For convenience, the series is available at:
>
> https://github.com/hwu25/edk2/tree/sdmmc_override_extension_v1
>
>
>
> I only verified the change will not break the build for platforms
>
> (in edk2-platform repo). Could you help provide feedbacks for this series?
>
> Thanks in advance.
>
>
>
> Best Regards,
>
> Hao Wu
>
>
>
> *From:* Albecki, Mateusz
> *Sent:* Thursday, June 13, 2019 9:44 PM
> *To:* devel@edk2.groups.io
> *Cc:* Wu, Hao A
> *Subject:* RE: [PATCH 0/2] Add GetOperatingParam notify phase to
> SdMmcOverride protocol
>
>
>
> Resending to group.
>
>
>
> *From:* Wu, Hao A
> *Sent:* Monday, June 10, 2019 5:18 AM
> *To:* Albecki, Mateusz <mateusz.albecki@intel.com>
> *Subject:* RE: [PATCH 0/2] Add GetOperatingParam notify phase to
> SdMmcOverride protocol
>
>
>
> Hello Mateusz,
>
>
>
> Thanks for the patch series and sorry for the delayed response.
>
>
>
> One question first, I did not see the patches being sent to the mailing
> list
>
> 'devel@edk2.groups.io'. We have recently switched to this new mailing
> list back
>
> to early April, the old one 'edk2-devel@lists.01.org' is no longer being
> used.
>
>
>
> In case that you have not registered the new list, you can sign up at:
>
> https://edk2.groups.io/g/devel
>
> or email:
>
> devel+subscribe@edk2.groups.io
>
> to get registered in the new mailing list.
>
>
>
> Or you can send me the patch files and I will help to send them to the
> mailing list.
>
>
>
>
>
> Below are some general level feedbacks (Other patch-specific feedbacks
> will be replied in other mails):
>
> ·         *Could you help to add the below Bugzilla tracker reference in
> the commit log message?*
>
> https://bugzilla.tianocore.org/show_bug.cgi?id=1882
>
>
>
> ·         *Could you help to run the script
> ‘BaseTools/Scripts/PatchCheck.py’ and resolve its report on the commit log
> format issues?*
>
> *Checking git commit: 6e3f64aeb4*
>
> *The commit message format is not valid:*
>
> ** Contributed-under! (Note: this must be removed by the code
> contributor!)*
>
> ** Add quotes (") around name with a comma: Albecki, Mateusz*
>
> ** Email format is invalid: hao.a.wu@intel.com <hao.a.wu@intel.com>*
>
> ** Line 15 of commit message is too long.*
>
> *https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
> <https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format>*
>
> *The code passed all checks.*
>
>
>
> *Checking git commit: 91595ae911*
>
> *The commit message format is not valid:*
>
> ** Contributed-under! (Note: this must be removed by the code
> contributor!)*
>
> ** Add quotes (") around name with a comma: Albecki, Mateusz*
>
> ** Email format is invalid: hao.a.wu@intel.com <hao.a.wu@intel.com>*
>
> *https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format
> <https://github.com/tianocore/tianocore.github.io/wiki/Commit-Message-Format>*
>
> *The code passed all checks.*
>
>
>
> ·         *Could you help to resolve the typo ‘Prefered’ to ‘Preferred’
> in the naming of some variables?*
>
>
>
> ·         *Could you help to use lowercase ‘static’ (instead of using the
> uppercase one) for the new functions?*
>
>
>
>
>
> Also, I only verified the build of the patch series. Please grant me some
> time
>
> to verify on the HW on my side. Maybe your updated patches arrive first
> and I
>
> will then verify the update ones.
>
>
>
>
>
> Best Regards,
>
> Hao Wu
>
>
>
> > -----Original Message-----
>
> > From: Albecki, Mateusz
>
> > Sent: Monday, June 03, 2019 7:34 PM
>
> > Cc: Albecki, Mateusz; Wu, Hao A
>
> > Subject: [PATCH 0/2] Add GetOperatingParam notify phase to
>
> > SdMmcOverride protocol
>
> >
>
> > 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.
>
> >
>
> > 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
>
> > - SD card enumeration in UEFI shell on default speed and high speed(non
>
> > UHS-I) with SdMmcOverride installed and UHS-I disabled in capability
>
> >
>
> >
>
> > Cc: hao.a.wu@intel.com
>
> >
>
> > Albecki, Mateusz (2):
>
> >   MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
>
> >   MdeModulePkg/SdMmcHcDxe: Implement revision 3 of
>
> > SdMmcOverrideProtocol
>
> >
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 522
>
> > +++++++++++++++------
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      | 381
>
> > ++++++++++++---
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  52 +-
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |  18 +-
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |   8 +-
>
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |   8 +-
>
> >  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  58 ++-
>
> >  7 files changed, 802 insertions(+), 245 deletions(-)
>
> >
>
> > --
>
> > 2.14.1.windows.1
>
>
>

[-- Attachment #2: Type: text/html, Size: 18903 bytes --]