public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Albecki, Mateusz" <mateusz.albecki@intel.com>
To: "Wu, Hao A" <hao.a.wu@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Subject: Re: [PATCH v3 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase
Date: Tue, 25 Jun 2019 14:15:04 +0000	[thread overview]
Message-ID: <92CF190FF2351747A47C1708F0E09C0875E7342D@IRSMSX102.ger.corp.intel.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C8F3B85@SHSMSX104.ccr.corp.intel.com>

Sorry, I must have cherry-picked this commit without the changes I made after v1 review and send it out as v2. It would explain why I still have those typos and there is no reordering. V4 will have it fixed.

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, June 24, 2019 10:15 AM
> To: Albecki, Mateusz <mateusz.albecki@intel.com>; devel@edk2.groups.io
> Subject: RE: [PATCH v3 1/2] MdeModulePkg/SdMmcOverride: Add
> GetOperatingParam notify phase
> 
> > -----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

--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


  reply	other threads:[~2019-06-25 14:15 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-21 15:12 [PATCH v3 0/2] Add GetOperatingParam notify phase to SdMmcOverride protocol Albecki, Mateusz
2019-06-21 15:12 ` [PATCH v3 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz
2019-06-24  8:14   ` Wu, Hao A
2019-06-25 14:15     ` Albecki, Mateusz [this message]
2019-06-21 15:12 ` [PATCH v3 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz
2019-06-24  8:15   ` Wu, Hao A
     [not found]   ` <B80AF82E9BFB8E4FBD8C89DA810C6A093C8F3A12@SHSMSX104.ccr.corp.intel.com>
2019-06-24  8:46     ` Wu, Hao A
2019-06-25 14:13       ` Albecki, Mateusz
2019-06-26  1:26         ` Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=92CF190FF2351747A47C1708F0E09C0875E7342D@IRSMSX102.ger.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox