* [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol
@ 2019-06-26 13:10 Albecki, Mateusz
2019-06-26 13:10 ` [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz
` (3 more replies)
0 siblings, 4 replies; 19+ messages in thread
From: Albecki, Mateusz @ 2019-06-26 13:10 UTC (permalink / raw)
To: devel; +Cc: Mateusz Albecki, Hao A Wu
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
--------------------------------------------------------------------
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.
^ permalink raw reply [flat|nested] 19+ messages in thread* [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase 2019-06-26 13:10 [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz @ 2019-06-26 13:10 ` Albecki, Mateusz 2019-06-27 2:49 ` [edk2-devel] " Wu, Hao A 2019-06-26 13:10 ` [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz ` (2 subsequent siblings) 3 siblings, 1 reply; 19+ messages in thread From: Albecki, Mateusz @ 2019-06-26 13:10 UTC (permalink / raw) To: devel; +Cc: Mateusz Albecki, Hao A Wu 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. Cc: Hao A Wu <hao.a.wu@intel.com> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com> --- MdeModulePkg/Include/Protocol/SdMmcOverride.h | 60 +++++++++++++++++++++++---- 1 file changed, 53 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h index 9c8bf37efd..d44027260a 100644 --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h @@ -16,19 +16,66 @@ #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 unsupported 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 chosen bus timing supports them. + // + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; +} EDKII_SD_MMC_OPERATING_PARAMETERS; + +typedef enum { + SdMmcSdDs, + SdMmcSdHs, SdMmcUhsSdr12, SdMmcUhsSdr25, SdMmcUhsSdr50, - SdMmcUhsSdr104, SdMmcUhsDdr50, + SdMmcUhsSdr104, SdMmcMmcLegacy, SdMmcMmcHsSdr, SdMmcMmcHsDdr, @@ -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. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase 2019-06-26 13:10 ` [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz @ 2019-06-27 2:49 ` Wu, Hao A 0 siblings, 0 replies; 19+ messages in thread From: Wu, Hao A @ 2019-06-27 2:49 UTC (permalink / raw) To: devel@edk2.groups.io, Albecki, Mateusz > -----Original Message----- > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > Albecki, Mateusz > Sent: Wednesday, June 26, 2019 9:10 PM > To: devel@edk2.groups.io > Cc: Albecki, Mateusz; Wu, Hao A > Subject: [edk2-devel] [PATCH v4 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. > > Cc: Hao A Wu <hao.a.wu@intel.com> > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com> > --- > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 60 > +++++++++++++++++++++++---- > 1 file changed, 53 insertions(+), 7 deletions(-) > > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > index 9c8bf37efd..d44027260a 100644 > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h > @@ -16,19 +16,66 @@ > #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 unsupported 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 chosen bus timing supports them. > + // > + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; > +} EDKII_SD_MMC_OPERATING_PARAMETERS; > + > +typedef enum { > + SdMmcSdDs, > + SdMmcSdHs, > SdMmcUhsSdr12, > SdMmcUhsSdr25, > SdMmcUhsSdr50, > - SdMmcUhsSdr104, > SdMmcUhsDdr50, > + SdMmcUhsSdr104, > SdMmcMmcLegacy, > SdMmcMmcHsSdr, > SdMmcMmcHsDdr, > @@ -43,10 +90,10 @@ typedef enum { > EdkiiSdMmcInitHostPost, > EdkiiSdMmcUhsSignaling, > EdkiiSdMmcSwitchClockFreqPost, > + EdkiiSdMmcGetOperatingParam > } EDKII_SD_MMC_PHASE_TYPE; Reviewed-by: Hao A Wu <hao.a.wu@intel.com> Best Regards, Hao Wu > > /** > - > 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. > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-26 13:10 [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz 2019-06-26 13:10 ` [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz @ 2019-06-26 13:10 ` Albecki, Mateusz 2019-06-27 2:49 ` Wu, Hao A 2019-06-27 2:49 ` [PATCH v4 0/2] " Wu, Hao A 2019-06-27 6:25 ` [edk2-devel] " Marcin Wojtas 3 siblings, 1 reply; 19+ messages in thread From: Albecki, Mateusz @ 2019-06-26 13:10 UTC (permalink / raw) To: devel; +Cc: Albecki, Mateusz, Hao A Wu From: "Albecki, Mateusz" <mateusz.albecki@intel.com> https://bugzilla.tianocore.org/show_bug.cgi?id=1882 Implement support for GetOperatingParamters notify phase in SdMmcHcDxe driver. GetOperatingParameters notify phase is signaled before we start card detection and initialization. Code has been updated for both eMMC and SD card controllers to take into consideration those new parameters. Initialization process has been divided into 2 steps. In the first step we bring the link up to the point where we can get card identification data(Extended CSD in eMMC case and SWITCH command response in SD card case). This data is later used along with controller capabilities and operating parameters passed in GetOperatingParameters phase to choose prefered bus settings in GetTargetBusSettings function. Those settings are later on to start bus training to high speeds. If user passes incompatible setting with selected bus timing driver will assume it's standard behavior with respect to that setting. For instance if HS400 has been selected as a target bus timing due to card and controller support bus width setting of 4 and 1 bit won't be respected and 8 bit setting will be choosen instead. Cc: Hao A Wu <hao.a.wu@intel.com> Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com> --- 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 + 6 files changed, 814 insertions(+), 231 deletions(-) diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c index deaf4468c9..3f4a8e5413 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c @@ -641,13 +641,13 @@ EmmcSwitchBusWidth ( Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host Controller Simplified Spec 3.0 Figure 3-3 for details. - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. - @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. - @param[in] Slot The slot number of the SD card to send the command to. - @param[in] Rca The relative device address to be assigned. - @param[in] HsTiming The value to be written to HS_TIMING field of EXT_CSD register. - @param[in] Timing The bus mode timing indicator. - @param[in] ClockFreq The max clock frequency to be set, the unit is MHz. + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. + @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. + @param[in] Slot The slot number of the SD card to send the command to. + @param[in] Rca The relative device address to be assigned. + @param[in] DriverStrength Driver strength to set for speed modes that support it. + @param[in] BusTiming The bus mode timing indicator. + @param[in] ClockFreq The max clock frequency to be set, the unit is MHz. @retval EFI_SUCCESS The operation is done correctly. @retval Others The operation fails. @@ -659,8 +659,8 @@ EmmcSwitchBusTiming ( IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, IN UINT16 Rca, - IN UINT8 HsTiming, - IN SD_MMC_BUS_MODE Timing, + IN EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength, + IN SD_MMC_BUS_MODE BusTiming, IN UINT32 ClockFreq ) { @@ -678,12 +678,29 @@ EmmcSwitchBusTiming ( // Access = 0x03; Index = OFFSET_OF (EMMC_EXT_CSD, HsTiming); - Value = HsTiming; CmdSet = 0; + switch (BusTiming) { + case SdMmcMmcHs400: + Value = (UINT8)((DriverStrength.Emmc << 4) | 3); + break; + case SdMmcMmcHs200: + Value = (UINT8)((DriverStrength.Emmc << 4) | 2); + break; + case SdMmcMmcHsSdr: + case SdMmcMmcHsDdr: + Value = 1; + break; + case SdMmcMmcLegacy: + Value = 0; + break; + default: + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported BusTiming(%d\n)", BusTiming)); + return EFI_INVALID_PARAMETER; + } Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d fails with %r\n", HsTiming, Status)); + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to bus timing %d fails with %r\n", BusTiming, Status)); return Status; } @@ -713,7 +730,7 @@ EmmcSwitchBusTiming ( Private->ControllerHandle, Slot, EdkiiSdMmcSwitchClockFreqPost, - &Timing + &BusTiming ); if (EFI_ERROR (Status)) { DEBUG (( @@ -739,10 +756,7 @@ EmmcSwitchBusTiming ( @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. @param[in] Slot The slot number of the SD card to send the command to. @param[in] Rca The relative device address to be assigned. - @param[in] ClockFreq The max clock frequency to be set. - @param[in] IsDdr If TRUE, use dual data rate data simpling method. Otherwise - use single data rate data simpling method. - @param[in] BusWidth The bus width to be set, it could be 4 or 8. + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure containing bus settings. @retval EFI_SUCCESS The operation is done correctly. @retval Others The operation fails. @@ -754,25 +768,34 @@ EmmcSwitchToHighSpeed ( IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, IN UINT16 Rca, - IN UINT32 ClockFreq, - IN BOOLEAN IsDdr, - IN UINT8 BusWidth + IN SD_MMC_BUS_SETTINGS *BusMode ) { EFI_STATUS Status; - UINT8 HsTiming; UINT8 HostCtrl1; - SD_MMC_BUS_MODE Timing; SD_MMC_HC_PRIVATE_DATA *Private; + BOOLEAN IsDdr; Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth); + if ((BusMode->BusTiming != SdMmcMmcHsSdr && BusMode->BusTiming != SdMmcMmcHsDdr) || + BusMode->ClockFreq > 52) { + return EFI_INVALID_PARAMETER; + } + + if (BusMode->BusTiming == SdMmcMmcHsDdr) { + IsDdr = TRUE; + } else { + IsDdr = FALSE; + } + + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusMode->BusWidth); if (EFI_ERROR (Status)) { return Status; } + // - // Set to Hight Speed timing + // Set to High Speed timing // HostCtrl1 = BIT2; Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1); @@ -780,37 +803,25 @@ EmmcSwitchToHighSpeed ( return Status; } - if (IsDdr) { - Timing = SdMmcMmcHsDdr; - } else if (ClockFreq == 52) { - Timing = SdMmcMmcHsSdr; - } else { - Timing = SdMmcMmcLegacy; - } - - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing); + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, BusMode->BusTiming); if (EFI_ERROR (Status)) { return Status; } - HsTiming = 1; - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); - - return Status; + return EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode->DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); } /** - Switch to the HS200 timing according to request. + Switch to the HS200 timing. This function assumes that eMMC bus is still in legacy mode. Refer to EMMC Electrical Standard Spec 5.1 Section 6.6.8 and SD Host Controller Simplified Spec 3.0 Figure 2-29 for details. - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. - @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. - @param[in] Slot The slot number of the SD card to send the command to. - @param[in] Rca The relative device address to be assigned. - @param[in] ClockFreq The max clock frequency to be set. - @param[in] BusWidth The bus width to be set, it could be 4 or 8. + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. + @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. + @param[in] Slot The slot number of the SD card to send the command to. + @param[in] Rca The relative device address to be assigned. + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure containing bus settings. @retval EFI_SUCCESS The operation is done correctly. @retval Others The operation fails. @@ -822,30 +833,25 @@ EmmcSwitchToHS200 ( IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, IN UINT16 Rca, - IN UINT32 ClockFreq, - IN UINT8 BusWidth + IN SD_MMC_BUS_SETTINGS *BusMode ) { EFI_STATUS Status; - UINT8 HsTiming; UINT16 ClockCtrl; - SD_MMC_BUS_MODE Timing; SD_MMC_HC_PRIVATE_DATA *Private; Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); - if ((BusWidth != 4) && (BusWidth != 8)) { + if (BusMode->BusTiming != SdMmcMmcHs200 || + (BusMode->BusWidth != 4 && BusMode->BusWidth != 8)) { return EFI_INVALID_PARAMETER; } - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, FALSE, BusWidth); + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, FALSE, BusMode->BusWidth); if (EFI_ERROR (Status)) { return Status; } // - // Set to HS200/SDR104 timing - // - // // Stop bus clock at first // Status = SdMmcHcStopClock (PciIo, Slot); @@ -853,9 +859,7 @@ EmmcSwitchToHS200 ( return Status; } - Timing = SdMmcMmcHs200; - - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing); + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, BusMode->BusTiming); if (EFI_ERROR (Status)) { return Status; } @@ -881,28 +885,27 @@ EmmcSwitchToHS200 ( ClockCtrl = BIT2; Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl); - HsTiming = 2; - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode->DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); if (EFI_ERROR (Status)) { return Status; } - Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth); + Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusMode->BusWidth); return Status; } /** - Switch to the HS400 timing according to request. + Switch to the HS400 timing. This function assumes that eMMC bus is still in legacy mode. Refer to EMMC Electrical Standard Spec 5.1 Section 6.6.8 and SD Host Controller Simplified Spec 3.0 Figure 2-29 for details. - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. - @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. - @param[in] Slot The slot number of the SD card to send the command to. - @param[in] Rca The relative device address to be assigned. - @param[in] ClockFreq The max clock frequency to be set. + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. + @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. + @param[in] Slot The slot number of the SD card to send the command to. + @param[in] Rca The relative device address to be assigned. + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure containing bus settings. @retval EFI_SUCCESS The operation is done correctly. @retval Others The operation fails. @@ -914,47 +917,314 @@ EmmcSwitchToHS400 ( IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, IN UINT16 Rca, - IN UINT32 ClockFreq + IN SD_MMC_BUS_SETTINGS *BusMode ) { EFI_STATUS Status; - UINT8 HsTiming; - SD_MMC_BUS_MODE Timing; SD_MMC_HC_PRIVATE_DATA *Private; + SD_MMC_BUS_SETTINGS Hs200BusMode; + UINT32 HsFreq; + + if (BusMode->BusTiming != SdMmcMmcHs400 || + BusMode->BusWidth != 8) { + return EFI_INVALID_PARAMETER; + } Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); + Hs200BusMode.BusTiming = SdMmcMmcHs200; + Hs200BusMode.BusWidth = BusMode->BusWidth; + Hs200BusMode.ClockFreq = BusMode->ClockFreq; + Hs200BusMode.DriverStrength = BusMode->DriverStrength; - Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8); + Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, &Hs200BusMode); if (EFI_ERROR (Status)) { return Status; } + // - // Set to Hight Speed timing and set the clock frequency to a value less than 52MHz. + // Set to High Speed timing and set the clock frequency to a value less than or equal to 52MHz. + // This step is necessary to be able to switch Bus into 8 bit DDR mode which is unsupported in HS200. // - HsTiming = 1; - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52); + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, SdMmcMmcHsSdr); if (EFI_ERROR (Status)) { return Status; } - // - // HS400 mode must use 8 data lines. - // - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, TRUE, 8); + + HsFreq = BusMode->ClockFreq < 52 ? BusMode->ClockFreq : 52; + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode->DriverStrength, SdMmcMmcHsSdr, HsFreq); if (EFI_ERROR (Status)) { return Status; } - Timing = SdMmcMmcHs400; + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, TRUE, BusMode->BusWidth); + if (EFI_ERROR (Status)) { + return Status; + } - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing); + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, BusMode->BusTiming); if (EFI_ERROR (Status)) { return Status; } - HsTiming = 3; - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq); + return EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode->DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); +} - return Status; +/** + Check if passed BusTiming is supported in both controller and card. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to the card's extended CSD + @param[in] BusTiming Bus timing to check + + @retval TRUE Both card and controller support given BusTiming + @retval FALSE Card or controller doesn't support given BusTiming +**/ +BOOLEAN +EmmcIsBusTimingSupported ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + BOOLEAN Supported; + SD_MMC_HC_SLOT_CAP *Capabilities; + + Capabilities = &Private->Capability[SlotIndex]; + + Supported = FALSE; + switch (BusTiming) { + case SdMmcMmcHs400: + if ((((ExtCsd->DeviceType & (BIT6 | BIT7)) != 0) && (Capabilities->Hs400 != 0)) && Capabilities->BusWidth8 != 0) { + Supported = TRUE; + } + break; + case SdMmcMmcHs200: + if ((((ExtCsd->DeviceType & (BIT4 | BIT5)) != 0) && (Capabilities->Sdr104 != 0))) { + Supported = TRUE; + } + break; + case SdMmcMmcHsDdr: + if ((((ExtCsd->DeviceType & (BIT2 | BIT3)) != 0) && (Capabilities->Ddr50 != 0))) { + Supported = TRUE; + } + break; + case SdMmcMmcHsSdr: + if ((((ExtCsd->DeviceType & BIT1) != 0) && (Capabilities->HighSpeed != 0))) { + Supported = TRUE; + } + break; + case SdMmcMmcLegacy: + if ((ExtCsd->DeviceType & BIT0) != 0) { + Supported = TRUE; + } + break; + default: + ASSERT (FALSE); + } + + return Supported; +} + +/** + Get the target bus timing to set on the link. This function + will try to select highest bus timing supported by card, controller + and the driver. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to the card's extended CSD + + @return Bus timing value that should be set on link +**/ +SD_MMC_BUS_MODE +EmmcGetTargetBusTiming ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd + ) +{ + SD_MMC_BUS_MODE BusTiming; + + // + // We start with highest bus timing that this driver currently supports and + // return as soon as we find supported timing. + // + BusTiming = SdMmcMmcHs400; + while (BusTiming > SdMmcMmcLegacy) { + if (EmmcIsBusTimingSupported (Private, SlotIndex, ExtCsd, BusTiming)) { + break; + } + BusTiming--; + } + + return BusTiming; +} + +/** + Check if the passed bus width is supported by controller and card. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] BusTiming Bus timing set on the link + @param[in] BusWidth Bus width to check + + @retval TRUE Passed bus width is supported in current bus configuration + @retval FALSE Passed bus width is not supported in current bus configuration +**/ +BOOLEAN +EmmcIsBusWidthSupported ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN SD_MMC_BUS_MODE BusTiming, + IN UINT16 BusWidth + ) +{ + if (BusWidth == 8 && Private->Capability[SlotIndex].BusWidth8 != 0) { + return TRUE; + } else if (BusWidth == 4 && BusTiming != SdMmcMmcHs400) { + return TRUE; + } else if (BusWidth == 1 && (BusTiming == SdMmcMmcHsSdr || BusTiming == SdMmcMmcLegacy)) { + return TRUE; + } + + return FALSE; +} + +/** + Get the target bus width to be set on the bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to card's extended CSD + @param[in] BusTiming Bus timing set on the bus + + @return Bus width to be set on the bus +**/ +UINT8 +EmmcGetTargetBusWidth ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + UINT8 BusWidth; + UINT8 PreferredBusWidth; + + PreferredBusWidth = Private->Slot[SlotIndex].OperatingParameters.BusWidth; + + if (PreferredBusWidth != EDKII_SD_MMC_BUS_WIDTH_IGNORE && + EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, PreferredBusWidth)) { + BusWidth = PreferredBusWidth; + } else if (EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, 8)) { + BusWidth = 8; + } else if (EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, 4)) { + BusWidth = 4; + } else { + BusWidth = 1; + } + + return BusWidth; +} + +/** + Get the target clock frequency to be set on the bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to card's extended CSD + @param[in] BusTiming Bus timing to be set on the bus + + @return Value of the clock frequency to be set on bus in MHz +**/ +UINT32 +EmmcGetTargetClockFreq ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + UINT32 PreferredClockFreq; + UINT32 MaxClockFreq; + + PreferredClockFreq = Private->Slot[SlotIndex].OperatingParameters.ClockFreq; + + switch (BusTiming) { + case SdMmcMmcHs400: + case SdMmcMmcHs200: + MaxClockFreq = 200; + break; + case SdMmcMmcHsSdr: + case SdMmcMmcHsDdr: + MaxClockFreq = 52; + break; + default: + MaxClockFreq = 26; + break; + } + + if (PreferredClockFreq != EDKII_SD_MMC_CLOCK_FREQ_IGNORE && PreferredClockFreq < MaxClockFreq) { + return PreferredClockFreq; + } else { + return MaxClockFreq; + } +} + +/** + Get the driver strength to be set on bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to card's extended CSD + @param[in] BusTiming Bus timing set on the bus + + @return Value of the driver strength to be set on the bus +**/ +EDKII_SD_MMC_DRIVER_STRENGTH +EmmcGetTargetDriverStrength ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + EDKII_SD_MMC_DRIVER_STRENGTH PreferredDriverStrength; + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; + + PreferredDriverStrength = Private->Slot[SlotIndex].OperatingParameters.DriverStrength; + DriverStrength.Emmc = EmmcDriverStrengthType0; + + if (PreferredDriverStrength.Emmc != EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE && + (ExtCsd->DriverStrength & (BIT0 << PreferredDriverStrength.Emmc))) { + DriverStrength.Emmc = PreferredDriverStrength.Emmc; + } + + return DriverStrength; +} + +/** + Get the target settings for the bus mode. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] ExtCsd Pointer to card's extended CSD + @param[out] BusMode Target configuration of the bus +**/ +VOID +EmmcGetTargetBusMode ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN EMMC_EXT_CSD *ExtCsd, + OUT SD_MMC_BUS_SETTINGS *BusMode + ) +{ + BusMode->BusTiming = EmmcGetTargetBusTiming (Private, SlotIndex, ExtCsd); + BusMode->BusWidth = EmmcGetTargetBusWidth (Private, SlotIndex, ExtCsd, BusMode->BusTiming); + BusMode->ClockFreq = EmmcGetTargetClockFreq (Private, SlotIndex, ExtCsd, BusMode->BusTiming); + BusMode->DriverStrength = EmmcGetTargetDriverStrength (Private, SlotIndex, ExtCsd, BusMode->BusTiming); } /** @@ -983,10 +1253,7 @@ EmmcSetBusMode ( EFI_STATUS Status; EMMC_CSD Csd; EMMC_EXT_CSD ExtCsd; - UINT8 HsTiming; - BOOLEAN IsDdr; - UINT32 ClockFreq; - UINT8 BusWidth; + SD_MMC_BUS_SETTINGS BusMode; SD_MMC_HC_PRIVATE_DATA *Private; Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); @@ -1004,85 +1271,30 @@ EmmcSetBusMode ( } ASSERT (Private->BaseClkFreq[Slot] != 0); + // - // Check if the Host Controller support 8bits bus width. - // - if (Private->Capability[Slot].BusWidth8 != 0) { - BusWidth = 8; - } else { - BusWidth = 4; - } - // - // Get Deivce_Type from EXT_CSD register. + // Get Device_Type from EXT_CSD register. // Status = EmmcGetExtCsd (PassThru, Slot, &ExtCsd); if (EFI_ERROR (Status)) { DEBUG ((DEBUG_ERROR, "EmmcSetBusMode: GetExtCsd fails with %r\n", Status)); return Status; } - // - // Calculate supported bus speed/bus width/clock frequency. - // - HsTiming = 0; - IsDdr = FALSE; - ClockFreq = 0; - if (((ExtCsd.DeviceType & (BIT4 | BIT5)) != 0) && (Private->Capability[Slot].Sdr104 != 0)) { - HsTiming = 2; - IsDdr = FALSE; - ClockFreq = 200; - } else if (((ExtCsd.DeviceType & (BIT2 | BIT3)) != 0) && (Private->Capability[Slot].Ddr50 != 0)) { - HsTiming = 1; - IsDdr = TRUE; - ClockFreq = 52; - } else if (((ExtCsd.DeviceType & BIT1) != 0) && (Private->Capability[Slot].HighSpeed != 0)) { - HsTiming = 1; - IsDdr = FALSE; - ClockFreq = 52; - } else if (((ExtCsd.DeviceType & BIT0) != 0) && (Private->Capability[Slot].HighSpeed != 0)) { - HsTiming = 1; - IsDdr = FALSE; - ClockFreq = 26; - } - // - // Check if both of the device and the host controller support HS400 DDR mode. - // - if (((ExtCsd.DeviceType & (BIT6 | BIT7)) != 0) && (Private->Capability[Slot].Hs400 != 0)) { - // - // The host controller supports 8bits bus. - // - ASSERT (BusWidth == 8); - HsTiming = 3; - IsDdr = TRUE; - ClockFreq = 200; - } - if ((ClockFreq == 0) || (HsTiming == 0)) { - // - // Continue using default setting. - // - return EFI_SUCCESS; - } + EmmcGetTargetBusMode (Private, Slot, &ExtCsd, &BusMode); - DEBUG ((DEBUG_INFO, "EmmcSetBusMode: HsTiming %d ClockFreq %d BusWidth %d Ddr %a\n", HsTiming, ClockFreq, BusWidth, IsDdr ? "TRUE":"FALSE")); + DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Target bus mode: timing = %d, width = %d, clock freq = %d, driver strength = %d\n", + BusMode.BusTiming, BusMode.BusWidth, BusMode.ClockFreq, BusMode.DriverStrength.Emmc)); - if (HsTiming == 3) { - // - // Execute HS400 timing switch procedure - // - Status = EmmcSwitchToHS400 (PciIo, PassThru, Slot, Rca, ClockFreq); - } else if (HsTiming == 2) { - // - // Execute HS200 timing switch procedure - // - Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, BusWidth); + if (BusMode.BusTiming == SdMmcMmcHs400) { + Status = EmmcSwitchToHS400 (PciIo, PassThru, Slot, Rca, &BusMode); + } else if (BusMode.BusTiming == SdMmcMmcHs200) { + Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, &BusMode); } else { - // - // Execute High Speed timing switch procedure - // - Status = EmmcSwitchToHighSpeed (PciIo, PassThru, Slot, Rca, ClockFreq, IsDdr, BusWidth); + Status = EmmcSwitchToHighSpeed (PciIo, PassThru, Slot, Rca, &BusMode); } - DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Switch to %a %r\n", (HsTiming == 3) ? "HS400" : ((HsTiming == 2) ? "HS200" : "HighSpeed"), Status)); + DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Switch to %a %r\n", (BusMode.BusTiming == SdMmcMmcHs400) ? "HS400" : ((BusMode.BusTiming == SdMmcMmcHs200) ? "HS200" : "HighSpeed"), Status)); return Status; } diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c index 17936a5492..88ece5256c 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -313,10 +313,6 @@ SdCardSetRca ( return Status; } - - - - /** Send command SELECT_DESELECT_CARD to the SD device to select/deselect it. @@ -472,14 +468,14 @@ SdCardSetBusWidth ( Refer to SD Physical Layer Simplified Spec 4.1 Section 4.7 for details. - @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. - @param[in] Slot The slot number of the SD card to send the command to. - @param[in] AccessMode The value for access mode group. - @param[in] CommandSystem The value for command set group. - @param[in] DriveStrength The value for drive length group. - @param[in] PowerLimit The value for power limit group. - @param[in] Mode Switch or check function. - @param[out] SwitchResp The return switch function status. + @param[in] PassThru A pointer to the EFI_SD_MMC_PASS_THRU_PROTOCOL instance. + @param[in] Slot The slot number of the SD card to send the command to. + @param[in] BusTiming Target bus timing based on which access group value will be set. + @param[in] CommandSystem The value for command set group. + @param[in] DriverStrength The value for driver strength group. + @param[in] PowerLimit The value for power limit group. + @param[in] Mode Switch or check function. + @param[out] SwitchResp The return switch function status. @retval EFI_SUCCESS The operation is done correctly. @retval Others The operation fails. @@ -489,9 +485,9 @@ EFI_STATUS SdCardSwitch ( IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, IN UINT8 Slot, - IN UINT8 AccessMode, + IN SD_MMC_BUS_MODE BusTiming, IN UINT8 CommandSystem, - IN UINT8 DriveStrength, + IN SD_DRIVER_STRENGTH_TYPE DriverStrength, IN UINT8 PowerLimit, IN BOOLEAN Mode, OUT UINT8 *SwitchResp @@ -502,6 +498,7 @@ SdCardSwitch ( EFI_SD_MMC_PASS_THRU_COMMAND_PACKET Packet; EFI_STATUS Status; UINT32 ModeValue; + UINT8 AccessMode; ZeroMem (&SdMmcCmdBlk, sizeof (SdMmcCmdBlk)); ZeroMem (&SdMmcStatusBlk, sizeof (SdMmcStatusBlk)); @@ -516,14 +513,49 @@ SdCardSwitch ( SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1; ModeValue = Mode ? BIT31 : 0; - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \ - ((DriveStrength & 0xF) << 8) | ((DriveStrength & 0xF) << 12) | \ + + switch (BusTiming) { + case SdMmcUhsDdr50: + AccessMode = 0x4; + break; + case SdMmcUhsSdr104: + AccessMode = 0x3; + break; + case SdMmcUhsSdr50: + AccessMode = 0x2; + break; + case SdMmcUhsSdr25: + case SdMmcSdHs: + AccessMode = 0x1; + break; + case SdMmcUhsSdr12: + case SdMmcSdDs: + AccessMode = 0; + break; + default: + AccessMode = 0xF; + } + + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ + ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ ModeValue; Packet.InDataBuffer = SwitchResp; Packet.InTransferLength = 64; Status = SdMmcPassThruPassThru (PassThru, Slot, &Packet, NULL); + if (EFI_ERROR (Status)) { + return Status; + } + + if (Mode) { + if ((((AccessMode & 0xF) != 0xF) && ((SwitchResp[16] & 0xF) != AccessMode) || + (((CommandSystem & 0xF) != 0xF) && (((SwitchResp[16] >> 4) & 0xF) != CommandSystem)) || + (((DriverStrength & 0xF) != 0xF) && ((SwitchResp[15] & 0xF) != DriverStrength)) || + (((PowerLimit & 0xF) != 0xF) && (((SwitchResp[15] >> 4) & 0xF) != PowerLimit)))) { + return EFI_DEVICE_ERROR; + } + } return Status; } @@ -748,6 +780,273 @@ SdCardSwitchBusWidth ( return Status; } +/** + Check if passed BusTiming is supported in both controller and card. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] CardSupportedBusTimings Bitmask indicating which bus timings are supported by card + @param[in] IsInUhsI Flag indicating if link is in UHS-I + + @retval TRUE Both card and controller support given BusTiming + @retval FALSE Card or controller doesn't support given BusTiming +**/ +BOOLEAN +SdIsBusTimingSupported ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN UINT8 CardSupportedBusTimings, + IN BOOLEAN IsInUhsI, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + SD_MMC_HC_SLOT_CAP *Capability; + + Capability = &Private->Capability[SlotIndex]; + + if (IsInUhsI) { + switch (BusTiming) { + case SdMmcUhsSdr104: + if ((Capability->Sdr104 != 0) && ((CardSupportedBusTimings & BIT3) != 0)) { + return TRUE; + } + break; + case SdMmcUhsDdr50: + if ((Capability->Ddr50 != 0) && ((CardSupportedBusTimings & BIT4) != 0)) { + return TRUE; + } + break; + case SdMmcUhsSdr50: + if ((Capability->Sdr50 != 0) && ((CardSupportedBusTimings & BIT2) != 0)) { + return TRUE; + } + break; + case SdMmcUhsSdr25: + if ((CardSupportedBusTimings & BIT1) != 0) { + return TRUE; + } + break; + case SdMmcUhsSdr12: + if ((CardSupportedBusTimings & BIT0) != 0) { + return TRUE; + } + break; + default: + break; + } + } else { + switch (BusTiming) { + case SdMmcSdHs: + if ((Capability->HighSpeed != 0) && (CardSupportedBusTimings & BIT1) != 0) { + return TRUE; + } + break; + case SdMmcSdDs: + if ((CardSupportedBusTimings & BIT0) != 0) { + return TRUE; + } + break; + default: + break; + } + } + + return FALSE; +} + +/** + Get the target bus timing to set on the link. This function + will try to select highest bus timing supported by card, controller + and the driver. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] CardSupportedBusTimings Bitmask indicating which bus timings are supported by card + @param[in] IsInUhsI Flag indicating if link is in UHS-I + + @return Bus timing value that should be set on link +**/ +SD_MMC_BUS_MODE +SdGetTargetBusTiming ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN UINT8 CardSupportedBusTimings, + IN BOOLEAN IsInUhsI + ) +{ + SD_MMC_BUS_MODE BusTiming; + + if (IsInUhsI) { + BusTiming = SdMmcUhsSdr104; + } else { + BusTiming = SdMmcSdHs; + } + + while (BusTiming > SdMmcSdDs) { + if (SdIsBusTimingSupported (Private, SlotIndex, CardSupportedBusTimings, IsInUhsI, BusTiming)) { + break; + } + BusTiming--; + } + + return BusTiming; +} + +/** + Get the target bus width to be set on the bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] BusTiming Bus timing set on the bus + + @return Bus width to be set on the bus +**/ +UINT8 +SdGetTargetBusWidth ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + UINT8 BusWidth; + UINT8 PreferredBusWidth; + + PreferredBusWidth = Private->Slot[SlotIndex].OperatingParameters.BusWidth; + + if (BusTiming == SdMmcSdDs || BusTiming == SdMmcSdHs) { + if (PreferredBusWidth != EDKII_SD_MMC_BUS_WIDTH_IGNORE && + (PreferredBusWidth == 1 || PreferredBusWidth == 4)) { + BusWidth = PreferredBusWidth; + } else { + BusWidth = 4; + } + } else { + // + // UHS-I modes support only 4-bit width. + // Switch to 4-bit has been done before calling this function anyway so + // this is purely informational. + // + BusWidth = 4; + } + + return BusWidth; +} + +/** + Get the target clock frequency to be set on the bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] BusTiming Bus timing to be set on the bus + + @return Value of the clock frequency to be set on bus in MHz +**/ +UINT32 +SdGetTargetBusClockFreq ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + UINT32 PreferredClockFreq; + UINT32 MaxClockFreq; + + PreferredClockFreq = Private->Slot[SlotIndex].OperatingParameters.ClockFreq; + + switch (BusTiming) { + case SdMmcUhsSdr104: + MaxClockFreq = 208; + break; + case SdMmcUhsSdr50: + MaxClockFreq = 100; + break; + case SdMmcUhsDdr50: + case SdMmcUhsSdr25: + case SdMmcSdHs: + MaxClockFreq = 50; + break; + case SdMmcUhsSdr12: + case SdMmcSdDs: + default: + MaxClockFreq = 25; + } + + if (PreferredClockFreq != EDKII_SD_MMC_CLOCK_FREQ_IGNORE && PreferredClockFreq < MaxClockFreq) { + return PreferredClockFreq; + } else { + return MaxClockFreq; + } +} + +/** + Get the driver strength to be set on bus. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] CardSupportedDriverStrengths Bitmask indicating which driver strengths are supported on the card + @param[in] BusTiming Bus timing set on the bus + + @return Value of the driver strength to be set on the bus +**/ +EDKII_SD_MMC_DRIVER_STRENGTH +SdGetTargetDriverStrength ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN UINT8 CardSupportedDriverStrengths, + IN SD_MMC_BUS_MODE BusTiming + ) +{ + EDKII_SD_MMC_DRIVER_STRENGTH PreferredDriverStrength; + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; + + if (BusTiming == SdMmcSdDs || BusTiming == SdMmcSdHs) { + DriverStrength.Sd = SdDriverStrengthIgnore; + return DriverStrength; + } + + PreferredDriverStrength = Private->Slot[SlotIndex].OperatingParameters.DriverStrength; + DriverStrength.Sd = SdDriverStrengthTypeB; + + if (PreferredDriverStrength.Sd != EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE && + (CardSupportedDriverStrengths & (BIT0 << PreferredDriverStrength.Sd))) { + + if ((PreferredDriverStrength.Sd == SdDriverStrengthTypeA && + (Private->Capability[SlotIndex].DriverTypeA != 0)) || + (PreferredDriverStrength.Sd == SdDriverStrengthTypeC && + (Private->Capability[SlotIndex].DriverTypeC != 0)) || + (PreferredDriverStrength.Sd == SdDriverStrengthTypeD && + (Private->Capability[SlotIndex].DriverTypeD != 0))) { + DriverStrength.Sd = PreferredDriverStrength.Sd; + } + } + + return DriverStrength; +} + +/** + Get the target settings for the bus mode. + + @param[in] Private Pointer to controller private data + @param[in] SlotIndex Index of the slot in the controller + @param[in] SwitchQueryResp Pointer to switch query response + @param[in] IsInUhsI Flag indicating if link is in UHS-I mode + @param[out] BusMode Target configuration of the bus +**/ +VOID +SdGetTargetBusMode ( + IN SD_MMC_HC_PRIVATE_DATA *Private, + IN UINT8 SlotIndex, + IN UINT8 *SwitchQueryResp, + IN BOOLEAN IsInUhsI, + OUT SD_MMC_BUS_SETTINGS *BusMode + ) +{ + BusMode->BusTiming = SdGetTargetBusTiming (Private, SlotIndex, SwitchQueryResp[13], IsInUhsI); + BusMode->BusWidth = SdGetTargetBusWidth (Private, SlotIndex, BusMode->BusTiming); + BusMode->ClockFreq = SdGetTargetBusClockFreq (Private, SlotIndex, BusMode->BusTiming); + BusMode->DriverStrength = SdGetTargetDriverStrength (Private, SlotIndex, SwitchQueryResp[9], BusMode->BusTiming); +} + /** Switch the high speed timing according to request. @@ -775,13 +1074,10 @@ SdCardSetBusMode ( { EFI_STATUS Status; SD_MMC_HC_SLOT_CAP *Capability; - UINT32 ClockFreq; - UINT8 BusWidth; - UINT8 AccessMode; UINT8 HostCtrl1; UINT8 SwitchResp[64]; - SD_MMC_BUS_MODE Timing; SD_MMC_HC_PRIVATE_DATA *Private; + SD_MMC_BUS_SETTINGS BusMode; Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); @@ -792,61 +1088,51 @@ SdCardSetBusMode ( return Status; } - BusWidth = 4; - - Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, BusWidth); - if (EFI_ERROR (Status)) { - return Status; + if (S18A) { + // + // For UHS-I speed modes 4-bit data bus is requiered so we + // switch here irrespective of platform preference. + // + Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, 4); + if (EFI_ERROR (Status)) { + return Status; + } } + // // Get the supported bus speed from SWITCH cmd return data group #1. // - Status = SdCardSwitch (PassThru, Slot, 0xF, 0xF, 0xF, 0xF, FALSE, SwitchResp); + Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore, 0xF, FALSE, SwitchResp); if (EFI_ERROR (Status)) { return Status; } - // - // Calculate supported bus speed/bus width/clock frequency by host and device capability. - // - ClockFreq = 0; - if (S18A && (Capability->Sdr104 != 0) && ((SwitchResp[13] & BIT3) != 0)) { - ClockFreq = 208; - AccessMode = 3; - Timing = SdMmcUhsSdr104; - } else if (S18A && (Capability->Sdr50 != 0) && ((SwitchResp[13] & BIT2) != 0)) { - ClockFreq = 100; - AccessMode = 2; - Timing = SdMmcUhsSdr50; - } else if (S18A && (Capability->Ddr50 != 0) && ((SwitchResp[13] & BIT4) != 0)) { - ClockFreq = 50; - AccessMode = 4; - Timing = SdMmcUhsDdr50; - } else if ((SwitchResp[13] & BIT1) != 0) { - ClockFreq = 50; - AccessMode = 1; - Timing = SdMmcUhsSdr25; - } else { - ClockFreq = 25; - AccessMode = 0; - Timing = SdMmcUhsSdr12; + + SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode); + + DEBUG ((DEBUG_INFO, "SdCardSetBusMode: Target bus mode: bus timing = %d, bus width = %d, clock freq[MHz] = %d, driver strength = %d\n", + BusMode.BusTiming, BusMode.BusWidth, BusMode.ClockFreq, BusMode.DriverStrength.Sd)); + + if (!S18A) { + Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, BusMode.BusWidth); + if (EFI_ERROR (Status)) { + return Status; + } } - Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, SwitchResp); + Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF, BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp); if (EFI_ERROR (Status)) { return Status; } - if ((SwitchResp[16] & 0xF) != AccessMode) { - DEBUG ((DEBUG_ERROR, "SdCardSetBusMode: Switch to AccessMode %d ClockFreq %d BusWidth %d fails! The Switch response is 0x%1x\n", AccessMode, ClockFreq, BusWidth, SwitchResp[16] & 0xF)); - return EFI_DEVICE_ERROR; + Status = SdMmcSetDriverStrength (Private->PciIo, Slot, BusMode.DriverStrength.Sd); + if (EFI_ERROR (Status)) { + return Status; } - DEBUG ((DEBUG_INFO, "SdCardSetBusMode: Switch to AccessMode %d ClockFreq %d BusWidth %d\n", AccessMode, ClockFreq, BusWidth)); - // - // Set to Hight Speed timing + // Set to High Speed timing // - if (AccessMode == 1) { + if (BusMode.BusTiming == SdMmcSdHs) { HostCtrl1 = BIT2; Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1); if (EFI_ERROR (Status)) { @@ -854,12 +1140,12 @@ SdCardSetBusMode ( } } - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing); + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, BusMode.BusTiming); if (EFI_ERROR (Status)) { return Status; } - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]); + Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000, Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]); if (EFI_ERROR (Status)) { return Status; } @@ -869,7 +1155,7 @@ SdCardSetBusMode ( Private->ControllerHandle, Slot, EdkiiSdMmcSwitchClockFreqPost, - &Timing + &BusMode.BusTiming ); if (EFI_ERROR (Status)) { DEBUG (( @@ -882,7 +1168,7 @@ SdCardSetBusMode ( } } - if ((AccessMode == 3) || ((AccessMode == 2) && (Capability->TuningSDR50 != 0))) { + if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming == SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) { Status = SdCardTuningClock (PciIo, PassThru, Slot); if (EFI_ERROR (Status)) { return Status; diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c index 4881ee44cc..373f1bed45 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c @@ -28,6 +28,11 @@ EFI_DRIVER_BINDING_PROTOCOL gSdMmcPciHcDriverBinding = { NULL }; +#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, \ + {EDKII_SD_MMC_BUS_WIDTH_IGNORE,\ + EDKII_SD_MMC_CLOCK_FREQ_IGNORE,\ + {EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE}}} + // // Template for SD/MMC host controller private data. // @@ -50,8 +55,12 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate = { // Queue INITIALIZE_LIST_HEAD_VARIABLE (gSdMmcPciHcTemplate.Queue), { // Slot - {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0}, - {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0} + SLOT_INIT_TEMPLATE, + SLOT_INIT_TEMPLATE, + SLOT_INIT_TEMPLATE, + SLOT_INIT_TEMPLATE, + SLOT_INIT_TEMPLATE, + SLOT_INIT_TEMPLATE }, { // Capability {0}, @@ -328,6 +337,7 @@ SdMmcPciHcEnumerateDevice ( return; } + /** Tests to see if this driver supports a given controller. If a child device is provided, it further tests to see if this driver supports creating a handle for the specified child device. @@ -619,7 +629,6 @@ SdMmcPciHcDriverBindingStart ( Support64BitDma = TRUE; for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) { Private->Slot[Slot].Enable = TRUE; - // // Get SD/MMC Pci Host Controller Version // @@ -635,19 +644,34 @@ SdMmcPciHcDriverBindingStart ( Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq; - if (mOverride != NULL && mOverride->Capability != NULL) { - Status = mOverride->Capability ( - Controller, - Slot, - &Private->Capability[Slot], - &Private->BaseClkFreq[Slot] - ); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n", - __FUNCTION__, Status)); - continue; + if (mOverride != NULL) { + if (mOverride->Capability != NULL) { + Status = mOverride->Capability ( + Controller, + Slot, + &Private->Capability[Slot], + &Private->BaseClkFreq[Slot] + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n", + __FUNCTION__, Status)); + continue; + } + } + + if (mOverride->NotifyPhase != NULL) { + Status = mOverride->NotifyPhase ( + Controller, + Slot, + EdkiiSdMmcGetOperatingParam, + (VOID*)&Private->Slot[Slot].OperatingParameters + ); + if (EFI_ERROR (Status)) { + DEBUG ((DEBUG_WARN, "%a: Failed to get operating parameters, using defaults\n", __FUNCTION__)); + } } } + DumpCapabilityReg (Slot, &Private->Capability[Slot]); DEBUG (( DEBUG_INFO, diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h index 77bbf83b76..c29e48767e 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h @@ -78,11 +78,12 @@ typedef enum { } EFI_SD_MMC_SLOT_TYPE; typedef struct { - BOOLEAN Enable; - EFI_SD_MMC_SLOT_TYPE SlotType; - BOOLEAN MediaPresent; - BOOLEAN Initialized; - SD_MMC_CARD_TYPE CardType; + BOOLEAN Enable; + EFI_SD_MMC_SLOT_TYPE SlotType; + BOOLEAN MediaPresent; + BOOLEAN Initialized; + SD_MMC_CARD_TYPE CardType; + EDKII_SD_MMC_OPERATING_PARAMETERS OperatingParameters; } SD_MMC_HC_SLOT; typedef struct { @@ -120,6 +121,13 @@ typedef struct { UINT32 BaseClkFreq[SD_MMC_HC_MAX_SLOT]; } SD_MMC_HC_PRIVATE_DATA; +typedef struct { + SD_MMC_BUS_MODE BusTiming; + UINT8 BusWidth; + UINT32 ClockFreq; + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; +} SD_MMC_BUS_SETTINGS; + #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T') // diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c index 5d1f977e55..b9d04e0f17 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c @@ -1339,6 +1339,40 @@ SdMmcHcUhsSignaling ( return EFI_SUCCESS; } +/** + Set driver strength in host controller. + + @param[in] PciIo The PCI IO protocol instance. + @param[in] SlotIndex The slot index of the card. + @param[in] DriverStrength DriverStrength to set in the controller. + + @retval EFI_SUCCESS Driver strength programmed successfully. + @retval Others Failed to set driver strength. +**/ +EFI_STATUS +SdMmcSetDriverStrength ( + IN EFI_PCI_IO_PROTOCOL *PciIo, + IN UINT8 SlotIndex, + IN SD_DRIVER_STRENGTH_TYPE DriverStrength + ) +{ + EFI_STATUS Status; + UINT16 HostCtrl2; + + if (DriverStrength == SdDriverStrengthIgnore) { + return EFI_SUCCESS; + } + + HostCtrl2 = (UINT16)~SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK; + Status = SdMmcHcAndMmio (PciIo, SlotIndex, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2); + if (EFI_ERROR (Status)) { + return Status; + } + + HostCtrl2 = (DriverStrength << 4) & SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK; + return SdMmcHcOrMmio (PciIo, SlotIndex, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2); +} + /** Turn on/off LED. diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h index 0b0d415256..088c70451c 100644 --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h @@ -72,6 +72,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define SD_MMC_HC_CTRL_MMC_HS200 0x0003 #define SD_MMC_HC_CTRL_MMC_HS400 0x0005 +#define SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK 0x0030 + // // The transfer modes supported by SD Host Controller // @@ -617,4 +619,21 @@ SdMmcHcUhsSignaling ( IN SD_MMC_BUS_MODE Timing ); +/** + Set driver strength in host controller. + + @param[in] PciIo The PCI IO protocol instance. + @param[in] SlotIndex The slot index of the card. + @param[in] DriverStrength DriverStrength to set in the controller. + + @retval EFI_SUCCESS Driver strength programmed successfully. + @retval Others Failed to set driver strength. +**/ +EFI_STATUS +SdMmcSetDriverStrength ( + IN EFI_PCI_IO_PROTOCOL *PciIo, + IN UINT8 SlotIndex, + IN SD_DRIVER_STRENGTH_TYPE DriverStrength + ); + #endif -- 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. ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-26 13:10 ` [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz @ 2019-06-27 2:49 ` Wu, Hao A 0 siblings, 0 replies; 19+ messages in thread From: Wu, Hao A @ 2019-06-27 2:49 UTC (permalink / raw) To: Albecki, Mateusz, devel@edk2.groups.io One typo within SdCardSwitch(): if (Mode) { if ((((AccessMode & 0xF) != 0xF) && ((SwitchResp[16] & 0xF) != AccessMode) || ... (((PowerLimit & 0xF) != 0xF) && (((SwitchResp[15] >> 4) & 0xF) != PowerLimit)))) { return EFI_DEVICE_ERROR; } } should be: if (Mode) { if ((((AccessMode & 0xF) != 0xF) && ((SwitchResp[16] & 0xF) != AccessMode)) || ... (((PowerLimit & 0xF) != 0xF) && (((SwitchResp[15] >> 4) & 0xF) != PowerLimit))) { return EFI_DEVICE_ERROR; } } Other than this, this patch looks good to me, Reviewed-by: Hao A Wu <hao.a.wu@intel.com> I will handle this when I push it. 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 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision > 3 of SdMmcOverrideProtocol > > From: "Albecki, Mateusz" <mateusz.albecki@intel.com> > > https://bugzilla.tianocore.org/show_bug.cgi?id=1882 > > Implement support for GetOperatingParamters notify phase > in SdMmcHcDxe driver. GetOperatingParameters notify phase > is signaled before we start card detection and initialization. > Code has been updated for both eMMC and SD card controllers to > take into consideration those new parameters. Initialization process > has been divided into 2 steps. In the first step we bring the link > up to the point where we can get card identification data(Extended > CSD in eMMC case and SWITCH command response in SD card case). This > data is later used along with controller capabilities and operating > parameters passed in GetOperatingParameters phase to choose prefered > bus settings in GetTargetBusSettings function. Those settings are later > on to start bus training to high speeds. If user passes incompatible > setting with selected bus timing driver will assume it's standard behavior > with respect to that setting. For instance if HS400 has been selected as a > target bus timing due to card and controller support bus width setting of > 4 and 1 bit won't be respected and 8 bit setting will be choosen instead. > > Cc: Hao A Wu <hao.a.wu@intel.com> > Signed-off-by: Mateusz Albecki <mateusz.albecki@intel.com> > --- > 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 + > 6 files changed, 814 insertions(+), 231 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > index deaf4468c9..3f4a8e5413 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c > @@ -641,13 +641,13 @@ EmmcSwitchBusWidth ( > Refer to EMMC Electrical Standard Spec 5.1 Section 6.6 and SD Host > Controller > Simplified Spec 3.0 Figure 3-3 for details. > > - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > - @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > - @param[in] Slot The slot number of the SD card to send the command > to. > - @param[in] Rca The relative device address to be assigned. > - @param[in] HsTiming The value to be written to HS_TIMING field of > EXT_CSD register. > - @param[in] Timing The bus mode timing indicator. > - @param[in] ClockFreq The max clock frequency to be set, the unit is > MHz. > + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > + @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @param[in] Slot The slot number of the SD card to send the > command to. > + @param[in] Rca The relative device address to be assigned. > + @param[in] DriverStrength Driver strength to set for speed modes that > support it. > + @param[in] BusTiming The bus mode timing indicator. > + @param[in] ClockFreq The max clock frequency to be set, the unit is > MHz. > > @retval EFI_SUCCESS The operation is done correctly. > @retval Others The operation fails. > @@ -659,8 +659,8 @@ EmmcSwitchBusTiming ( > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > IN UINT16 Rca, > - IN UINT8 HsTiming, > - IN SD_MMC_BUS_MODE Timing, > + IN EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength, > + IN SD_MMC_BUS_MODE BusTiming, > IN UINT32 ClockFreq > ) > { > @@ -678,12 +678,29 @@ EmmcSwitchBusTiming ( > // > Access = 0x03; > Index = OFFSET_OF (EMMC_EXT_CSD, HsTiming); > - Value = HsTiming; > CmdSet = 0; > + switch (BusTiming) { > + case SdMmcMmcHs400: > + Value = (UINT8)((DriverStrength.Emmc << 4) | 3); > + break; > + case SdMmcMmcHs200: > + Value = (UINT8)((DriverStrength.Emmc << 4) | 2); > + break; > + case SdMmcMmcHsSdr: > + case SdMmcMmcHsDdr: > + Value = 1; > + break; > + case SdMmcMmcLegacy: > + Value = 0; > + break; > + default: > + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Unsupported > BusTiming(%d\n)", BusTiming)); > + return EFI_INVALID_PARAMETER; > + } > > Status = EmmcSwitch (PassThru, Slot, Access, Index, Value, CmdSet); > if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to hstiming %d > fails with %r\n", HsTiming, Status)); > + DEBUG ((DEBUG_ERROR, "EmmcSwitchBusTiming: Switch to bus > timing %d fails with %r\n", BusTiming, Status)); > return Status; > } > > @@ -713,7 +730,7 @@ EmmcSwitchBusTiming ( > Private->ControllerHandle, > Slot, > EdkiiSdMmcSwitchClockFreqPost, > - &Timing > + &BusTiming > ); > if (EFI_ERROR (Status)) { > DEBUG (( > @@ -739,10 +756,7 @@ EmmcSwitchBusTiming ( > @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > @param[in] Slot The slot number of the SD card to send the command > to. > @param[in] Rca The relative device address to be assigned. > - @param[in] ClockFreq The max clock frequency to be set. > - @param[in] IsDdr If TRUE, use dual data rate data simpling method. > Otherwise > - use single data rate data simpling method. > - @param[in] BusWidth The bus width to be set, it could be 4 or 8. > + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure > containing bus settings. > > @retval EFI_SUCCESS The operation is done correctly. > @retval Others The operation fails. > @@ -754,25 +768,34 @@ EmmcSwitchToHighSpeed ( > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > IN UINT16 Rca, > - IN UINT32 ClockFreq, > - IN BOOLEAN IsDdr, > - IN UINT8 BusWidth > + IN SD_MMC_BUS_SETTINGS *BusMode > ) > { > EFI_STATUS Status; > - UINT8 HsTiming; > UINT8 HostCtrl1; > - SD_MMC_BUS_MODE Timing; > SD_MMC_HC_PRIVATE_DATA *Private; > + BOOLEAN IsDdr; > > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth); > + if ((BusMode->BusTiming != SdMmcMmcHsSdr && BusMode- > >BusTiming != SdMmcMmcHsDdr) || > + BusMode->ClockFreq > 52) { > + return EFI_INVALID_PARAMETER; > + } > + > + if (BusMode->BusTiming == SdMmcMmcHsDdr) { > + IsDdr = TRUE; > + } else { > + IsDdr = FALSE; > + } > + > + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusMode- > >BusWidth); > if (EFI_ERROR (Status)) { > return Status; > } > + > // > - // Set to Hight Speed timing > + // Set to High Speed timing > // > HostCtrl1 = BIT2; > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof > (HostCtrl1), &HostCtrl1); > @@ -780,37 +803,25 @@ EmmcSwitchToHighSpeed ( > return Status; > } > > - if (IsDdr) { > - Timing = SdMmcMmcHsDdr; > - } else if (ClockFreq == 52) { > - Timing = SdMmcMmcHsSdr; > - } else { > - Timing = SdMmcMmcLegacy; > - } > - > - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > Timing); > + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > BusMode->BusTiming); > if (EFI_ERROR (Status)) { > return Status; > } > > - HsTiming = 1; > - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > - > - return Status; > + return EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode- > >DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); > } > > /** > - Switch to the HS200 timing according to request. > + Switch to the HS200 timing. This function assumes that eMMC bus is still in > legacy mode. > > Refer to EMMC Electrical Standard Spec 5.1 Section 6.6.8 and SD Host > Controller > Simplified Spec 3.0 Figure 2-29 for details. > > - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > - @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > - @param[in] Slot The slot number of the SD card to send the command > to. > - @param[in] Rca The relative device address to be assigned. > - @param[in] ClockFreq The max clock frequency to be set. > - @param[in] BusWidth The bus width to be set, it could be 4 or 8. > + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > + @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @param[in] Slot The slot number of the SD card to send the > command to. > + @param[in] Rca The relative device address to be assigned. > + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure > containing bus settings. > > @retval EFI_SUCCESS The operation is done correctly. > @retval Others The operation fails. > @@ -822,30 +833,25 @@ EmmcSwitchToHS200 ( > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > IN UINT16 Rca, > - IN UINT32 ClockFreq, > - IN UINT8 BusWidth > + IN SD_MMC_BUS_SETTINGS *BusMode > ) > { > EFI_STATUS Status; > - UINT8 HsTiming; > UINT16 ClockCtrl; > - SD_MMC_BUS_MODE Timing; > SD_MMC_HC_PRIVATE_DATA *Private; > > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > - if ((BusWidth != 4) && (BusWidth != 8)) { > + if (BusMode->BusTiming != SdMmcMmcHs200 || > + (BusMode->BusWidth != 4 && BusMode->BusWidth != 8)) { > return EFI_INVALID_PARAMETER; > } > > - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, FALSE, BusWidth); > + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, FALSE, > BusMode->BusWidth); > if (EFI_ERROR (Status)) { > return Status; > } > // > - // Set to HS200/SDR104 timing > - // > - // > // Stop bus clock at first > // > Status = SdMmcHcStopClock (PciIo, Slot); > @@ -853,9 +859,7 @@ EmmcSwitchToHS200 ( > return Status; > } > > - Timing = SdMmcMmcHs200; > - > - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > Timing); > + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > BusMode->BusTiming); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -881,28 +885,27 @@ EmmcSwitchToHS200 ( > ClockCtrl = BIT2; > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof > (ClockCtrl), &ClockCtrl); > > - HsTiming = 2; > - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode- > >DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); > if (EFI_ERROR (Status)) { > return Status; > } > > - Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth); > + Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusMode- > >BusWidth); > > return Status; > } > > /** > - Switch to the HS400 timing according to request. > + Switch to the HS400 timing. This function assumes that eMMC bus is still in > legacy mode. > > Refer to EMMC Electrical Standard Spec 5.1 Section 6.6.8 and SD Host > Controller > Simplified Spec 3.0 Figure 2-29 for details. > > - @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > - @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > - @param[in] Slot The slot number of the SD card to send the command > to. > - @param[in] Rca The relative device address to be assigned. > - @param[in] ClockFreq The max clock frequency to be set. > + @param[in] PciIo A pointer to the EFI_PCI_IO_PROTOCOL instance. > + @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @param[in] Slot The slot number of the SD card to send the > command to. > + @param[in] Rca The relative device address to be assigned. > + @param[in] BusMode Pointer to SD_MMC_BUS_SETTINGS structure > containing bus settings. > > @retval EFI_SUCCESS The operation is done correctly. > @retval Others The operation fails. > @@ -914,47 +917,314 @@ EmmcSwitchToHS400 ( > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > IN UINT16 Rca, > - IN UINT32 ClockFreq > + IN SD_MMC_BUS_SETTINGS *BusMode > ) > { > EFI_STATUS Status; > - UINT8 HsTiming; > - SD_MMC_BUS_MODE Timing; > SD_MMC_HC_PRIVATE_DATA *Private; > + SD_MMC_BUS_SETTINGS Hs200BusMode; > + UINT32 HsFreq; > + > + if (BusMode->BusTiming != SdMmcMmcHs400 || > + BusMode->BusWidth != 8) { > + return EFI_INVALID_PARAMETER; > + } > > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > + Hs200BusMode.BusTiming = SdMmcMmcHs200; > + Hs200BusMode.BusWidth = BusMode->BusWidth; > + Hs200BusMode.ClockFreq = BusMode->ClockFreq; > + Hs200BusMode.DriverStrength = BusMode->DriverStrength; > > - Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8); > + Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, &Hs200BusMode); > if (EFI_ERROR (Status)) { > return Status; > } > + > // > - // Set to Hight Speed timing and set the clock frequency to a value less than > 52MHz. > + // Set to High Speed timing and set the clock frequency to a value less than > or equal to 52MHz. > + // This step is necessary to be able to switch Bus into 8 bit DDR mode which > is unsupported in HS200. > // > - HsTiming = 1; > - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, > SdMmcMmcHsSdr, 52); > + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > SdMmcMmcHsSdr); > if (EFI_ERROR (Status)) { > return Status; > } > - // > - // HS400 mode must use 8 data lines. > - // > - Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, TRUE, 8); > + > + HsFreq = BusMode->ClockFreq < 52 ? BusMode->ClockFreq : 52; > + Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode- > >DriverStrength, SdMmcMmcHsSdr, HsFreq); > if (EFI_ERROR (Status)) { > return Status; > } > > - Timing = SdMmcMmcHs400; > + Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, TRUE, BusMode- > >BusWidth); > + if (EFI_ERROR (Status)) { > + return Status; > + } > > - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > Timing); > + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > BusMode->BusTiming); > if (EFI_ERROR (Status)) { > return Status; > } > > - HsTiming = 3; > - Status = EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, HsTiming, Timing, > ClockFreq); > + return EmmcSwitchBusTiming (PciIo, PassThru, Slot, Rca, BusMode- > >DriverStrength, BusMode->BusTiming, BusMode->ClockFreq); > +} > > - return Status; > +/** > + Check if passed BusTiming is supported in both controller and card. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to the card's extended CSD > + @param[in] BusTiming Bus timing to check > + > + @retval TRUE Both card and controller support given BusTiming > + @retval FALSE Card or controller doesn't support given BusTiming > +**/ > +BOOLEAN > +EmmcIsBusTimingSupported ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + BOOLEAN Supported; > + SD_MMC_HC_SLOT_CAP *Capabilities; > + > + Capabilities = &Private->Capability[SlotIndex]; > + > + Supported = FALSE; > + switch (BusTiming) { > + case SdMmcMmcHs400: > + if ((((ExtCsd->DeviceType & (BIT6 | BIT7)) != 0) && (Capabilities- > >Hs400 != 0)) && Capabilities->BusWidth8 != 0) { > + Supported = TRUE; > + } > + break; > + case SdMmcMmcHs200: > + if ((((ExtCsd->DeviceType & (BIT4 | BIT5)) != 0) && (Capabilities- > >Sdr104 != 0))) { > + Supported = TRUE; > + } > + break; > + case SdMmcMmcHsDdr: > + if ((((ExtCsd->DeviceType & (BIT2 | BIT3)) != 0) && (Capabilities- > >Ddr50 != 0))) { > + Supported = TRUE; > + } > + break; > + case SdMmcMmcHsSdr: > + if ((((ExtCsd->DeviceType & BIT1) != 0) && (Capabilities->HighSpeed != > 0))) { > + Supported = TRUE; > + } > + break; > + case SdMmcMmcLegacy: > + if ((ExtCsd->DeviceType & BIT0) != 0) { > + Supported = TRUE; > + } > + break; > + default: > + ASSERT (FALSE); > + } > + > + return Supported; > +} > + > +/** > + Get the target bus timing to set on the link. This function > + will try to select highest bus timing supported by card, controller > + and the driver. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to the card's extended CSD > + > + @return Bus timing value that should be set on link > +**/ > +SD_MMC_BUS_MODE > +EmmcGetTargetBusTiming ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd > + ) > +{ > + SD_MMC_BUS_MODE BusTiming; > + > + // > + // We start with highest bus timing that this driver currently supports and > + // return as soon as we find supported timing. > + // > + BusTiming = SdMmcMmcHs400; > + while (BusTiming > SdMmcMmcLegacy) { > + if (EmmcIsBusTimingSupported (Private, SlotIndex, ExtCsd, BusTiming)) { > + break; > + } > + BusTiming--; > + } > + > + return BusTiming; > +} > + > +/** > + Check if the passed bus width is supported by controller and card. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] BusTiming Bus timing set on the link > + @param[in] BusWidth Bus width to check > + > + @retval TRUE Passed bus width is supported in current bus configuration > + @retval FALSE Passed bus width is not supported in current bus > configuration > +**/ > +BOOLEAN > +EmmcIsBusWidthSupported ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN SD_MMC_BUS_MODE BusTiming, > + IN UINT16 BusWidth > + ) > +{ > + if (BusWidth == 8 && Private->Capability[SlotIndex].BusWidth8 != 0) { > + return TRUE; > + } else if (BusWidth == 4 && BusTiming != SdMmcMmcHs400) { > + return TRUE; > + } else if (BusWidth == 1 && (BusTiming == SdMmcMmcHsSdr || BusTiming > == SdMmcMmcLegacy)) { > + return TRUE; > + } > + > + return FALSE; > +} > + > +/** > + Get the target bus width to be set on the bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to card's extended CSD > + @param[in] BusTiming Bus timing set on the bus > + > + @return Bus width to be set on the bus > +**/ > +UINT8 > +EmmcGetTargetBusWidth ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + UINT8 BusWidth; > + UINT8 PreferredBusWidth; > + > + PreferredBusWidth = Private- > >Slot[SlotIndex].OperatingParameters.BusWidth; > + > + if (PreferredBusWidth != EDKII_SD_MMC_BUS_WIDTH_IGNORE && > + EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, > PreferredBusWidth)) { > + BusWidth = PreferredBusWidth; > + } else if (EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, 8)) { > + BusWidth = 8; > + } else if (EmmcIsBusWidthSupported (Private, SlotIndex, BusTiming, 4)) { > + BusWidth = 4; > + } else { > + BusWidth = 1; > + } > + > + return BusWidth; > +} > + > +/** > + Get the target clock frequency to be set on the bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to card's extended CSD > + @param[in] BusTiming Bus timing to be set on the bus > + > + @return Value of the clock frequency to be set on bus in MHz > +**/ > +UINT32 > +EmmcGetTargetClockFreq ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + UINT32 PreferredClockFreq; > + UINT32 MaxClockFreq; > + > + PreferredClockFreq = Private- > >Slot[SlotIndex].OperatingParameters.ClockFreq; > + > + switch (BusTiming) { > + case SdMmcMmcHs400: > + case SdMmcMmcHs200: > + MaxClockFreq = 200; > + break; > + case SdMmcMmcHsSdr: > + case SdMmcMmcHsDdr: > + MaxClockFreq = 52; > + break; > + default: > + MaxClockFreq = 26; > + break; > + } > + > + if (PreferredClockFreq != EDKII_SD_MMC_CLOCK_FREQ_IGNORE && > PreferredClockFreq < MaxClockFreq) { > + return PreferredClockFreq; > + } else { > + return MaxClockFreq; > + } > +} > + > +/** > + Get the driver strength to be set on bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to card's extended CSD > + @param[in] BusTiming Bus timing set on the bus > + > + @return Value of the driver strength to be set on the bus > +**/ > +EDKII_SD_MMC_DRIVER_STRENGTH > +EmmcGetTargetDriverStrength ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + EDKII_SD_MMC_DRIVER_STRENGTH PreferredDriverStrength; > + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; > + > + PreferredDriverStrength = Private- > >Slot[SlotIndex].OperatingParameters.DriverStrength; > + DriverStrength.Emmc = EmmcDriverStrengthType0; > + > + if (PreferredDriverStrength.Emmc != > EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE && > + (ExtCsd->DriverStrength & (BIT0 << PreferredDriverStrength.Emmc))) { > + DriverStrength.Emmc = PreferredDriverStrength.Emmc; > + } > + > + return DriverStrength; > +} > + > +/** > + Get the target settings for the bus mode. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] ExtCsd Pointer to card's extended CSD > + @param[out] BusMode Target configuration of the bus > +**/ > +VOID > +EmmcGetTargetBusMode ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN EMMC_EXT_CSD *ExtCsd, > + OUT SD_MMC_BUS_SETTINGS *BusMode > + ) > +{ > + BusMode->BusTiming = EmmcGetTargetBusTiming (Private, SlotIndex, > ExtCsd); > + BusMode->BusWidth = EmmcGetTargetBusWidth (Private, SlotIndex, > ExtCsd, BusMode->BusTiming); > + BusMode->ClockFreq = EmmcGetTargetClockFreq (Private, SlotIndex, > ExtCsd, BusMode->BusTiming); > + BusMode->DriverStrength = EmmcGetTargetDriverStrength (Private, > SlotIndex, ExtCsd, BusMode->BusTiming); > } > > /** > @@ -983,10 +1253,7 @@ EmmcSetBusMode ( > EFI_STATUS Status; > EMMC_CSD Csd; > EMMC_EXT_CSD ExtCsd; > - UINT8 HsTiming; > - BOOLEAN IsDdr; > - UINT32 ClockFreq; > - UINT8 BusWidth; > + SD_MMC_BUS_SETTINGS BusMode; > SD_MMC_HC_PRIVATE_DATA *Private; > > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > @@ -1004,85 +1271,30 @@ EmmcSetBusMode ( > } > > ASSERT (Private->BaseClkFreq[Slot] != 0); > + > // > - // Check if the Host Controller support 8bits bus width. > - // > - if (Private->Capability[Slot].BusWidth8 != 0) { > - BusWidth = 8; > - } else { > - BusWidth = 4; > - } > - // > - // Get Deivce_Type from EXT_CSD register. > + // Get Device_Type from EXT_CSD register. > // > Status = EmmcGetExtCsd (PassThru, Slot, &ExtCsd); > if (EFI_ERROR (Status)) { > DEBUG ((DEBUG_ERROR, "EmmcSetBusMode: GetExtCsd fails with %r\n", > Status)); > return Status; > } > - // > - // Calculate supported bus speed/bus width/clock frequency. > - // > - HsTiming = 0; > - IsDdr = FALSE; > - ClockFreq = 0; > - if (((ExtCsd.DeviceType & (BIT4 | BIT5)) != 0) && (Private- > >Capability[Slot].Sdr104 != 0)) { > - HsTiming = 2; > - IsDdr = FALSE; > - ClockFreq = 200; > - } else if (((ExtCsd.DeviceType & (BIT2 | BIT3)) != 0) && (Private- > >Capability[Slot].Ddr50 != 0)) { > - HsTiming = 1; > - IsDdr = TRUE; > - ClockFreq = 52; > - } else if (((ExtCsd.DeviceType & BIT1) != 0) && (Private- > >Capability[Slot].HighSpeed != 0)) { > - HsTiming = 1; > - IsDdr = FALSE; > - ClockFreq = 52; > - } else if (((ExtCsd.DeviceType & BIT0) != 0) && (Private- > >Capability[Slot].HighSpeed != 0)) { > - HsTiming = 1; > - IsDdr = FALSE; > - ClockFreq = 26; > - } > - // > - // Check if both of the device and the host controller support HS400 DDR > mode. > - // > - if (((ExtCsd.DeviceType & (BIT6 | BIT7)) != 0) && (Private- > >Capability[Slot].Hs400 != 0)) { > - // > - // The host controller supports 8bits bus. > - // > - ASSERT (BusWidth == 8); > - HsTiming = 3; > - IsDdr = TRUE; > - ClockFreq = 200; > - } > > - if ((ClockFreq == 0) || (HsTiming == 0)) { > - // > - // Continue using default setting. > - // > - return EFI_SUCCESS; > - } > + EmmcGetTargetBusMode (Private, Slot, &ExtCsd, &BusMode); > > - DEBUG ((DEBUG_INFO, "EmmcSetBusMode: HsTiming %d ClockFreq %d > BusWidth %d Ddr %a\n", HsTiming, ClockFreq, BusWidth, IsDdr ? > "TRUE":"FALSE")); > + DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Target bus mode: timing > = %d, width = %d, clock freq = %d, driver strength = %d\n", > + BusMode.BusTiming, BusMode.BusWidth, BusMode.ClockFreq, > BusMode.DriverStrength.Emmc)); > > - if (HsTiming == 3) { > - // > - // Execute HS400 timing switch procedure > - // > - Status = EmmcSwitchToHS400 (PciIo, PassThru, Slot, Rca, ClockFreq); > - } else if (HsTiming == 2) { > - // > - // Execute HS200 timing switch procedure > - // > - Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, > BusWidth); > + if (BusMode.BusTiming == SdMmcMmcHs400) { > + Status = EmmcSwitchToHS400 (PciIo, PassThru, Slot, Rca, &BusMode); > + } else if (BusMode.BusTiming == SdMmcMmcHs200) { > + Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, &BusMode); > } else { > - // > - // Execute High Speed timing switch procedure > - // > - Status = EmmcSwitchToHighSpeed (PciIo, PassThru, Slot, Rca, ClockFreq, > IsDdr, BusWidth); > + Status = EmmcSwitchToHighSpeed (PciIo, PassThru, Slot, Rca, &BusMode); > } > > - DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Switch to %a %r\n", (HsTiming > == 3) ? "HS400" : ((HsTiming == 2) ? "HS200" : "HighSpeed"), Status)); > + DEBUG ((DEBUG_INFO, "EmmcSetBusMode: Switch to %a %r\n", > (BusMode.BusTiming == SdMmcMmcHs400) ? "HS400" : > ((BusMode.BusTiming == SdMmcMmcHs200) ? "HS200" : "HighSpeed"), > Status)); > > return Status; > } > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > index 17936a5492..88ece5256c 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > @@ -313,10 +313,6 @@ SdCardSetRca ( > return Status; > } > > - > - > - > - > /** > Send command SELECT_DESELECT_CARD to the SD device to > select/deselect it. > > @@ -472,14 +468,14 @@ SdCardSetBusWidth ( > > Refer to SD Physical Layer Simplified Spec 4.1 Section 4.7 for details. > > - @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > - @param[in] Slot The slot number of the SD card to send the command > to. > - @param[in] AccessMode The value for access mode group. > - @param[in] CommandSystem The value for command set group. > - @param[in] DriveStrength The value for drive length group. > - @param[in] PowerLimit The value for power limit group. > - @param[in] Mode Switch or check function. > - @param[out] SwitchResp The return switch function status. > + @param[in] PassThru A pointer to the > EFI_SD_MMC_PASS_THRU_PROTOCOL instance. > + @param[in] Slot The slot number of the SD card to send the > command to. > + @param[in] BusTiming Target bus timing based on which access group > value will be set. > + @param[in] CommandSystem The value for command set group. > + @param[in] DriverStrength The value for driver strength group. > + @param[in] PowerLimit The value for power limit group. > + @param[in] Mode Switch or check function. > + @param[out] SwitchResp The return switch function status. > > @retval EFI_SUCCESS The operation is done correctly. > @retval Others The operation fails. > @@ -489,9 +485,9 @@ EFI_STATUS > SdCardSwitch ( > IN EFI_SD_MMC_PASS_THRU_PROTOCOL *PassThru, > IN UINT8 Slot, > - IN UINT8 AccessMode, > + IN SD_MMC_BUS_MODE BusTiming, > IN UINT8 CommandSystem, > - IN UINT8 DriveStrength, > + IN SD_DRIVER_STRENGTH_TYPE DriverStrength, > IN UINT8 PowerLimit, > IN BOOLEAN Mode, > OUT UINT8 *SwitchResp > @@ -502,6 +498,7 @@ SdCardSwitch ( > EFI_SD_MMC_PASS_THRU_COMMAND_PACKET Packet; > EFI_STATUS Status; > UINT32 ModeValue; > + UINT8 AccessMode; > > ZeroMem (&SdMmcCmdBlk, sizeof (SdMmcCmdBlk)); > ZeroMem (&SdMmcStatusBlk, sizeof (SdMmcStatusBlk)); > @@ -516,14 +513,49 @@ SdCardSwitch ( > SdMmcCmdBlk.ResponseType = SdMmcResponseTypeR1; > > ModeValue = Mode ? BIT31 : 0; > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit > & 0xF) << 4) | \ > - ((DriveStrength & 0xF) << 8) | ((DriveStrength & 0xF) << 12) > | \ > + > + switch (BusTiming) { > + case SdMmcUhsDdr50: > + AccessMode = 0x4; > + break; > + case SdMmcUhsSdr104: > + AccessMode = 0x3; > + break; > + case SdMmcUhsSdr50: > + AccessMode = 0x2; > + break; > + case SdMmcUhsSdr25: > + case SdMmcSdHs: > + AccessMode = 0x1; > + break; > + case SdMmcUhsSdr12: > + case SdMmcSdDs: > + AccessMode = 0; > + break; > + default: > + AccessMode = 0xF; > + } > + > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | > ((CommandSystem & 0xF) << 4) | \ > + ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | > \ > ModeValue; > > Packet.InDataBuffer = SwitchResp; > Packet.InTransferLength = 64; > > Status = SdMmcPassThruPassThru (PassThru, Slot, &Packet, NULL); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + if (Mode) { > + if ((((AccessMode & 0xF) != 0xF) && ((SwitchResp[16] & 0xF) != > AccessMode) || > + (((CommandSystem & 0xF) != 0xF) && (((SwitchResp[16] >> 4) & 0xF) != > CommandSystem)) || > + (((DriverStrength & 0xF) != 0xF) && ((SwitchResp[15] & 0xF) != > DriverStrength)) || > + (((PowerLimit & 0xF) != 0xF) && (((SwitchResp[15] >> 4) & 0xF) != > PowerLimit)))) { > + return EFI_DEVICE_ERROR; > + } > + } > > return Status; > } > @@ -748,6 +780,273 @@ SdCardSwitchBusWidth ( > return Status; > } > > +/** > + Check if passed BusTiming is supported in both controller and card. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] CardSupportedBusTimings Bitmask indicating which bus > timings are supported by card > + @param[in] IsInUhsI Flag indicating if link is in UHS-I > + > + @retval TRUE Both card and controller support given BusTiming > + @retval FALSE Card or controller doesn't support given BusTiming > +**/ > +BOOLEAN > +SdIsBusTimingSupported ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN UINT8 CardSupportedBusTimings, > + IN BOOLEAN IsInUhsI, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + SD_MMC_HC_SLOT_CAP *Capability; > + > + Capability = &Private->Capability[SlotIndex]; > + > + if (IsInUhsI) { > + switch (BusTiming) { > + case SdMmcUhsSdr104: > + if ((Capability->Sdr104 != 0) && ((CardSupportedBusTimings & BIT3) != > 0)) { > + return TRUE; > + } > + break; > + case SdMmcUhsDdr50: > + if ((Capability->Ddr50 != 0) && ((CardSupportedBusTimings & BIT4) != 0)) > { > + return TRUE; > + } > + break; > + case SdMmcUhsSdr50: > + if ((Capability->Sdr50 != 0) && ((CardSupportedBusTimings & BIT2) != 0)) > { > + return TRUE; > + } > + break; > + case SdMmcUhsSdr25: > + if ((CardSupportedBusTimings & BIT1) != 0) { > + return TRUE; > + } > + break; > + case SdMmcUhsSdr12: > + if ((CardSupportedBusTimings & BIT0) != 0) { > + return TRUE; > + } > + break; > + default: > + break; > + } > + } else { > + switch (BusTiming) { > + case SdMmcSdHs: > + if ((Capability->HighSpeed != 0) && (CardSupportedBusTimings & > BIT1) != 0) { > + return TRUE; > + } > + break; > + case SdMmcSdDs: > + if ((CardSupportedBusTimings & BIT0) != 0) { > + return TRUE; > + } > + break; > + default: > + break; > + } > + } > + > + return FALSE; > +} > + > +/** > + Get the target bus timing to set on the link. This function > + will try to select highest bus timing supported by card, controller > + and the driver. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] CardSupportedBusTimings Bitmask indicating which bus > timings are supported by card > + @param[in] IsInUhsI Flag indicating if link is in UHS-I > + > + @return Bus timing value that should be set on link > +**/ > +SD_MMC_BUS_MODE > +SdGetTargetBusTiming ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN UINT8 CardSupportedBusTimings, > + IN BOOLEAN IsInUhsI > + ) > +{ > + SD_MMC_BUS_MODE BusTiming; > + > + if (IsInUhsI) { > + BusTiming = SdMmcUhsSdr104; > + } else { > + BusTiming = SdMmcSdHs; > + } > + > + while (BusTiming > SdMmcSdDs) { > + if (SdIsBusTimingSupported (Private, SlotIndex, > CardSupportedBusTimings, IsInUhsI, BusTiming)) { > + break; > + } > + BusTiming--; > + } > + > + return BusTiming; > +} > + > +/** > + Get the target bus width to be set on the bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] BusTiming Bus timing set on the bus > + > + @return Bus width to be set on the bus > +**/ > +UINT8 > +SdGetTargetBusWidth ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + UINT8 BusWidth; > + UINT8 PreferredBusWidth; > + > + PreferredBusWidth = Private- > >Slot[SlotIndex].OperatingParameters.BusWidth; > + > + if (BusTiming == SdMmcSdDs || BusTiming == SdMmcSdHs) { > + if (PreferredBusWidth != EDKII_SD_MMC_BUS_WIDTH_IGNORE && > + (PreferredBusWidth == 1 || PreferredBusWidth == 4)) { > + BusWidth = PreferredBusWidth; > + } else { > + BusWidth = 4; > + } > + } else { > + // > + // UHS-I modes support only 4-bit width. > + // Switch to 4-bit has been done before calling this function anyway so > + // this is purely informational. > + // > + BusWidth = 4; > + } > + > + return BusWidth; > +} > + > +/** > + Get the target clock frequency to be set on the bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] BusTiming Bus timing to be set on the bus > + > + @return Value of the clock frequency to be set on bus in MHz > +**/ > +UINT32 > +SdGetTargetBusClockFreq ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + UINT32 PreferredClockFreq; > + UINT32 MaxClockFreq; > + > + PreferredClockFreq = Private- > >Slot[SlotIndex].OperatingParameters.ClockFreq; > + > + switch (BusTiming) { > + case SdMmcUhsSdr104: > + MaxClockFreq = 208; > + break; > + case SdMmcUhsSdr50: > + MaxClockFreq = 100; > + break; > + case SdMmcUhsDdr50: > + case SdMmcUhsSdr25: > + case SdMmcSdHs: > + MaxClockFreq = 50; > + break; > + case SdMmcUhsSdr12: > + case SdMmcSdDs: > + default: > + MaxClockFreq = 25; > + } > + > + if (PreferredClockFreq != EDKII_SD_MMC_CLOCK_FREQ_IGNORE && > PreferredClockFreq < MaxClockFreq) { > + return PreferredClockFreq; > + } else { > + return MaxClockFreq; > + } > +} > + > +/** > + Get the driver strength to be set on bus. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] CardSupportedDriverStrengths Bitmask indicating which > driver strengths are supported on the card > + @param[in] BusTiming Bus timing set on the bus > + > + @return Value of the driver strength to be set on the bus > +**/ > +EDKII_SD_MMC_DRIVER_STRENGTH > +SdGetTargetDriverStrength ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN UINT8 CardSupportedDriverStrengths, > + IN SD_MMC_BUS_MODE BusTiming > + ) > +{ > + EDKII_SD_MMC_DRIVER_STRENGTH PreferredDriverStrength; > + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; > + > + if (BusTiming == SdMmcSdDs || BusTiming == SdMmcSdHs) { > + DriverStrength.Sd = SdDriverStrengthIgnore; > + return DriverStrength; > + } > + > + PreferredDriverStrength = Private- > >Slot[SlotIndex].OperatingParameters.DriverStrength; > + DriverStrength.Sd = SdDriverStrengthTypeB; > + > + if (PreferredDriverStrength.Sd != > EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE && > + (CardSupportedDriverStrengths & (BIT0 << PreferredDriverStrength.Sd))) > { > + > + if ((PreferredDriverStrength.Sd == SdDriverStrengthTypeA && > + (Private->Capability[SlotIndex].DriverTypeA != 0)) || > + (PreferredDriverStrength.Sd == SdDriverStrengthTypeC && > + (Private->Capability[SlotIndex].DriverTypeC != 0)) || > + (PreferredDriverStrength.Sd == SdDriverStrengthTypeD && > + (Private->Capability[SlotIndex].DriverTypeD != 0))) { > + DriverStrength.Sd = PreferredDriverStrength.Sd; > + } > + } > + > + return DriverStrength; > +} > + > +/** > + Get the target settings for the bus mode. > + > + @param[in] Private Pointer to controller private data > + @param[in] SlotIndex Index of the slot in the controller > + @param[in] SwitchQueryResp Pointer to switch query response > + @param[in] IsInUhsI Flag indicating if link is in UHS-I mode > + @param[out] BusMode Target configuration of the bus > +**/ > +VOID > +SdGetTargetBusMode ( > + IN SD_MMC_HC_PRIVATE_DATA *Private, > + IN UINT8 SlotIndex, > + IN UINT8 *SwitchQueryResp, > + IN BOOLEAN IsInUhsI, > + OUT SD_MMC_BUS_SETTINGS *BusMode > + ) > +{ > + BusMode->BusTiming = SdGetTargetBusTiming (Private, SlotIndex, > SwitchQueryResp[13], IsInUhsI); > + BusMode->BusWidth = SdGetTargetBusWidth (Private, SlotIndex, > BusMode->BusTiming); > + BusMode->ClockFreq = SdGetTargetBusClockFreq (Private, SlotIndex, > BusMode->BusTiming); > + BusMode->DriverStrength = SdGetTargetDriverStrength (Private, > SlotIndex, SwitchQueryResp[9], BusMode->BusTiming); > +} > + > /** > Switch the high speed timing according to request. > > @@ -775,13 +1074,10 @@ SdCardSetBusMode ( > { > EFI_STATUS Status; > SD_MMC_HC_SLOT_CAP *Capability; > - UINT32 ClockFreq; > - UINT8 BusWidth; > - UINT8 AccessMode; > UINT8 HostCtrl1; > UINT8 SwitchResp[64]; > - SD_MMC_BUS_MODE Timing; > SD_MMC_HC_PRIVATE_DATA *Private; > + SD_MMC_BUS_SETTINGS BusMode; > > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru); > > @@ -792,61 +1088,51 @@ SdCardSetBusMode ( > return Status; > } > > - BusWidth = 4; > - > - Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, BusWidth); > - if (EFI_ERROR (Status)) { > - return Status; > + if (S18A) { > + // > + // For UHS-I speed modes 4-bit data bus is requiered so we > + // switch here irrespective of platform preference. > + // > + Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, 4); > + if (EFI_ERROR (Status)) { > + return Status; > + } > } > + > // > // Get the supported bus speed from SWITCH cmd return data group #1. > // > - Status = SdCardSwitch (PassThru, Slot, 0xF, 0xF, 0xF, 0xF, FALSE, > SwitchResp); > + Status = SdCardSwitch (PassThru, Slot, 0xFF, 0xF, SdDriverStrengthIgnore, > 0xF, FALSE, SwitchResp); > if (EFI_ERROR (Status)) { > return Status; > } > - // > - // Calculate supported bus speed/bus width/clock frequency by host and > device capability. > - // > - ClockFreq = 0; > - if (S18A && (Capability->Sdr104 != 0) && ((SwitchResp[13] & BIT3) != 0)) { > - ClockFreq = 208; > - AccessMode = 3; > - Timing = SdMmcUhsSdr104; > - } else if (S18A && (Capability->Sdr50 != 0) && ((SwitchResp[13] & BIT2) != 0)) > { > - ClockFreq = 100; > - AccessMode = 2; > - Timing = SdMmcUhsSdr50; > - } else if (S18A && (Capability->Ddr50 != 0) && ((SwitchResp[13] & BIT4) != > 0)) { > - ClockFreq = 50; > - AccessMode = 4; > - Timing = SdMmcUhsDdr50; > - } else if ((SwitchResp[13] & BIT1) != 0) { > - ClockFreq = 50; > - AccessMode = 1; > - Timing = SdMmcUhsSdr25; > - } else { > - ClockFreq = 25; > - AccessMode = 0; > - Timing = SdMmcUhsSdr12; > + > + SdGetTargetBusMode (Private, Slot, SwitchResp, S18A, &BusMode); > + > + DEBUG ((DEBUG_INFO, "SdCardSetBusMode: Target bus mode: bus timing > = %d, bus width = %d, clock freq[MHz] = %d, driver strength = %d\n", > + BusMode.BusTiming, BusMode.BusWidth, BusMode.ClockFreq, > BusMode.DriverStrength.Sd)); > + > + if (!S18A) { > + Status = SdCardSwitchBusWidth (PciIo, PassThru, Slot, Rca, > BusMode.BusWidth); > + if (EFI_ERROR (Status)) { > + return Status; > + } > } > > - Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, > SwitchResp); > + Status = SdCardSwitch (PassThru, Slot, BusMode.BusTiming, 0xF, > BusMode.DriverStrength.Sd, 0xF, TRUE, SwitchResp); > if (EFI_ERROR (Status)) { > return Status; > } > > - if ((SwitchResp[16] & 0xF) != AccessMode) { > - DEBUG ((DEBUG_ERROR, "SdCardSetBusMode: Switch to AccessMode %d > ClockFreq %d BusWidth %d fails! The Switch response is 0x%1x\n", > AccessMode, ClockFreq, BusWidth, SwitchResp[16] & 0xF)); > - return EFI_DEVICE_ERROR; > + Status = SdMmcSetDriverStrength (Private->PciIo, Slot, > BusMode.DriverStrength.Sd); > + if (EFI_ERROR (Status)) { > + return Status; > } > > - DEBUG ((DEBUG_INFO, "SdCardSetBusMode: Switch to AccessMode %d > ClockFreq %d BusWidth %d\n", AccessMode, ClockFreq, BusWidth)); > - > // > - // Set to Hight Speed timing > + // Set to High Speed timing > // > - if (AccessMode == 1) { > + if (BusMode.BusTiming == SdMmcSdHs) { > HostCtrl1 = BIT2; > Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof > (HostCtrl1), &HostCtrl1); > if (EFI_ERROR (Status)) { > @@ -854,12 +1140,12 @@ SdCardSetBusMode ( > } > } > > - Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > Timing); > + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, > BusMode.BusTiming); > if (EFI_ERROR (Status)) { > return Status; > } > > - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private- > >BaseClkFreq[Slot], Private->ControllerVersion[Slot]); > + Status = SdMmcHcClockSupply (PciIo, Slot, BusMode.ClockFreq * 1000, > Private->BaseClkFreq[Slot], Private->ControllerVersion[Slot]); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -869,7 +1155,7 @@ SdCardSetBusMode ( > Private->ControllerHandle, > Slot, > EdkiiSdMmcSwitchClockFreqPost, > - &Timing > + &BusMode.BusTiming > ); > if (EFI_ERROR (Status)) { > DEBUG (( > @@ -882,7 +1168,7 @@ SdCardSetBusMode ( > } > } > > - if ((AccessMode == 3) || ((AccessMode == 2) && (Capability- > >TuningSDR50 != 0))) { > + if ((BusMode.BusTiming == SdMmcUhsSdr104) || ((BusMode.BusTiming > == SdMmcUhsSdr50) && (Capability->TuningSDR50 != 0))) { > Status = SdCardTuningClock (PciIo, PassThru, Slot); > if (EFI_ERROR (Status)) { > return Status; > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > index 4881ee44cc..373f1bed45 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c > @@ -28,6 +28,11 @@ EFI_DRIVER_BINDING_PROTOCOL > gSdMmcPciHcDriverBinding = { > NULL > }; > > +#define SLOT_INIT_TEMPLATE {0, UnknownSlot, 0, 0, 0, \ > + {EDKII_SD_MMC_BUS_WIDTH_IGNORE,\ > + EDKII_SD_MMC_CLOCK_FREQ_IGNORE,\ > + {EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE}}} > + > // > // Template for SD/MMC host controller private data. > // > @@ -50,8 +55,12 @@ SD_MMC_HC_PRIVATE_DATA gSdMmcPciHcTemplate > = { > // Queue > INITIALIZE_LIST_HEAD_VARIABLE (gSdMmcPciHcTemplate.Queue), > { // Slot > - {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, > 0}, > - {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, 0}, {0, UnknownSlot, 0, 0, > 0} > + SLOT_INIT_TEMPLATE, > + SLOT_INIT_TEMPLATE, > + SLOT_INIT_TEMPLATE, > + SLOT_INIT_TEMPLATE, > + SLOT_INIT_TEMPLATE, > + SLOT_INIT_TEMPLATE > }, > { // Capability > {0}, > @@ -328,6 +337,7 @@ SdMmcPciHcEnumerateDevice ( > > return; > } > + > /** > Tests to see if this driver supports a given controller. If a child device is > provided, > it further tests to see if this driver supports creating a handle for the > specified child device. > @@ -619,7 +629,6 @@ SdMmcPciHcDriverBindingStart ( > Support64BitDma = TRUE; > for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) { > Private->Slot[Slot].Enable = TRUE; > - > // > // Get SD/MMC Pci Host Controller Version > // > @@ -635,19 +644,34 @@ SdMmcPciHcDriverBindingStart ( > > Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq; > > - if (mOverride != NULL && mOverride->Capability != NULL) { > - Status = mOverride->Capability ( > - Controller, > - Slot, > - &Private->Capability[Slot], > - &Private->BaseClkFreq[Slot] > - ); > - if (EFI_ERROR (Status)) { > - DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n", > - __FUNCTION__, Status)); > - continue; > + if (mOverride != NULL) { > + if (mOverride->Capability != NULL) { > + Status = mOverride->Capability ( > + Controller, > + Slot, > + &Private->Capability[Slot], > + &Private->BaseClkFreq[Slot] > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n", > + __FUNCTION__, Status)); > + continue; > + } > + } > + > + if (mOverride->NotifyPhase != NULL) { > + Status = mOverride->NotifyPhase ( > + Controller, > + Slot, > + EdkiiSdMmcGetOperatingParam, > + (VOID*)&Private->Slot[Slot].OperatingParameters > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_WARN, "%a: Failed to get operating parameters, > using defaults\n", __FUNCTION__)); > + } > } > } > + > DumpCapabilityReg (Slot, &Private->Capability[Slot]); > DEBUG (( > DEBUG_INFO, > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > index 77bbf83b76..c29e48767e 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h > @@ -78,11 +78,12 @@ typedef enum { > } EFI_SD_MMC_SLOT_TYPE; > > typedef struct { > - BOOLEAN Enable; > - EFI_SD_MMC_SLOT_TYPE SlotType; > - BOOLEAN MediaPresent; > - BOOLEAN Initialized; > - SD_MMC_CARD_TYPE CardType; > + BOOLEAN Enable; > + EFI_SD_MMC_SLOT_TYPE SlotType; > + BOOLEAN MediaPresent; > + BOOLEAN Initialized; > + SD_MMC_CARD_TYPE CardType; > + EDKII_SD_MMC_OPERATING_PARAMETERS OperatingParameters; > } SD_MMC_HC_SLOT; > > typedef struct { > @@ -120,6 +121,13 @@ typedef struct { > UINT32 BaseClkFreq[SD_MMC_HC_MAX_SLOT]; > } SD_MMC_HC_PRIVATE_DATA; > > +typedef struct { > + SD_MMC_BUS_MODE BusTiming; > + UINT8 BusWidth; > + UINT32 ClockFreq; > + EDKII_SD_MMC_DRIVER_STRENGTH DriverStrength; > +} SD_MMC_BUS_SETTINGS; > + > #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T') > > // > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > index 5d1f977e55..b9d04e0f17 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c > @@ -1339,6 +1339,40 @@ SdMmcHcUhsSignaling ( > return EFI_SUCCESS; > } > > +/** > + Set driver strength in host controller. > + > + @param[in] PciIo The PCI IO protocol instance. > + @param[in] SlotIndex The slot index of the card. > + @param[in] DriverStrength DriverStrength to set in the controller. > + > + @retval EFI_SUCCESS Driver strength programmed successfully. > + @retval Others Failed to set driver strength. > +**/ > +EFI_STATUS > +SdMmcSetDriverStrength ( > + IN EFI_PCI_IO_PROTOCOL *PciIo, > + IN UINT8 SlotIndex, > + IN SD_DRIVER_STRENGTH_TYPE DriverStrength > + ) > +{ > + EFI_STATUS Status; > + UINT16 HostCtrl2; > + > + if (DriverStrength == SdDriverStrengthIgnore) { > + return EFI_SUCCESS; > + } > + > + HostCtrl2 = (UINT16)~SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK; > + Status = SdMmcHcAndMmio (PciIo, SlotIndex, SD_MMC_HC_HOST_CTRL2, > sizeof (HostCtrl2), &HostCtrl2); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + HostCtrl2 = (DriverStrength << 4) & > SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK; > + return SdMmcHcOrMmio (PciIo, SlotIndex, SD_MMC_HC_HOST_CTRL2, > sizeof (HostCtrl2), &HostCtrl2); > +} > + > /** > Turn on/off LED. > > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > index 0b0d415256..088c70451c 100644 > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h > @@ -72,6 +72,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #define SD_MMC_HC_CTRL_MMC_HS200 0x0003 > #define SD_MMC_HC_CTRL_MMC_HS400 0x0005 > > +#define SD_MMC_HC_CTRL_DRIVER_STRENGTH_MASK 0x0030 > + > // > // The transfer modes supported by SD Host Controller > // > @@ -617,4 +619,21 @@ SdMmcHcUhsSignaling ( > IN SD_MMC_BUS_MODE Timing > ); > > +/** > + Set driver strength in host controller. > + > + @param[in] PciIo The PCI IO protocol instance. > + @param[in] SlotIndex The slot index of the card. > + @param[in] DriverStrength DriverStrength to set in the controller. > + > + @retval EFI_SUCCESS Driver strength programmed successfully. > + @retval Others Failed to set driver strength. > +**/ > +EFI_STATUS > +SdMmcSetDriverStrength ( > + IN EFI_PCI_IO_PROTOCOL *PciIo, > + IN UINT8 SlotIndex, > + IN SD_DRIVER_STRENGTH_TYPE DriverStrength > + ); > + > #endif > -- > 2.14.1.windows.1 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-26 13:10 [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz 2019-06-26 13:10 ` [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz 2019-06-26 13:10 ` [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz @ 2019-06-27 2:49 ` Wu, Hao A 2019-06-27 6:25 ` [edk2-devel] " Marcin Wojtas 3 siblings, 0 replies; 19+ messages in thread From: Wu, Hao A @ 2019-06-27 2:49 UTC (permalink / raw) To: devel@edk2.groups.io, Ard Biesheuvel, Marcin Wojtas; +Cc: Albecki, Mateusz 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 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-26 13:10 [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz ` (2 preceding siblings ...) 2019-06-27 2:49 ` [PATCH v4 0/2] " Wu, Hao A @ 2019-06-27 6:25 ` Marcin Wojtas 2019-06-27 6:29 ` Wu, Hao A 3 siblings, 1 reply; 19+ messages in thread From: Marcin Wojtas @ 2019-06-27 6:25 UTC (permalink / raw) To: Albecki, Mateusz; +Cc: edk2-devel-groups-io, Hao A Wu Hi Mateusz, Can you please push those patches somewhere (github?) so that I can easily do a quick check for regression? Thanks, Marcin śr., 26 cze 2019 o 15:10 Albecki, Mateusz <mateusz.albecki@intel.com> napisał(a): > > 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 > > -------------------------------------------------------------------- > > 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. > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-27 6:25 ` [edk2-devel] " Marcin Wojtas @ 2019-06-27 6:29 ` Wu, Hao A 2019-06-27 8:10 ` Ard Biesheuvel 0 siblings, 1 reply; 19+ messages in thread From: Wu, Hao A @ 2019-06-27 6:29 UTC (permalink / raw) To: Marcin Wojtas, Albecki, Mateusz; +Cc: edk2-devel-groups-io > -----Original Message----- > From: Marcin Wojtas [mailto:mw@semihalf.com] > Sent: Thursday, June 27, 2019 2:25 PM > To: Albecki, Mateusz > Cc: edk2-devel-groups-io; Wu, Hao A > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > Hi Mateusz, > > Can you please push those patches somewhere (github?) so that I can > easily do a quick check for regression? > > Thanks, > Marcin Hello Marcin, I have just pushed the series at: https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 Please help to check. Best Regards, Hao Wu > > śr., 26 cze 2019 o 15:10 Albecki, Mateusz <mateusz.albecki@intel.com> > napisał(a): > > > > 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 > > > > -------------------------------------------------------------------- > > > > 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. > > > > > > > > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-27 6:29 ` Wu, Hao A @ 2019-06-27 8:10 ` Ard Biesheuvel 2019-06-27 13:38 ` sumit.garg 0 siblings, 1 reply; 19+ messages in thread From: Ard Biesheuvel @ 2019-06-27 8:10 UTC (permalink / raw) To: edk2-devel-groups-io, Wu, Hao A, Sumit Garg Cc: Marcin Wojtas, Albecki, Mateusz (+ Sumit) On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > -----Original Message----- > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > Sent: Thursday, June 27, 2019 2:25 PM > > To: Albecki, Mateusz > > Cc: edk2-devel-groups-io; Wu, Hao A > > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > > Implement revision 3 of SdMmcOverrideProtocol > > > > Hi Mateusz, > > > > Can you please push those patches somewhere (github?) so that I can > > easily do a quick check for regression? > > > > Thanks, > > Marcin > > > Hello Marcin, > > I have just pushed the series at: > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > Please help to check. > I have cc'ed my colleague Sumit, who has kindly agreed to regression test this branch on Socionext SynQuacer, which also uses the SD/MMC override infrastructure. Sumit, please reply here with your results. And thanks again! ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-27 8:10 ` Ard Biesheuvel @ 2019-06-27 13:38 ` sumit.garg 2019-06-28 0:57 ` Wu, Hao A 0 siblings, 1 reply; 19+ messages in thread From: sumit.garg @ 2019-06-27 13:38 UTC (permalink / raw) To: Ard Biesheuvel Cc: edk2-devel-groups-io, Wu, Hao A, Marcin Wojtas, Albecki, Mateusz On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > (+ Sumit) > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > > > -----Original Message----- > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > Sent: Thursday, June 27, 2019 2:25 PM > > > To: Albecki, Mateusz > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > Hi Mateusz, > > > > > > Can you please push those patches somewhere (github?) so that I can > > > easily do a quick check for regression? > > > > > > Thanks, > > > Marcin > > > > > > Hello Marcin, > > > > I have just pushed the series at: > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > Please help to check. > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > test this branch on Socionext SynQuacer, which also uses the SD/MMC > override infrastructure. > > Sumit, please reply here with your results. And thanks again! I did picked this patch-set and applied on top of edk2 master branch. It works well on SynQuacer with eMMC device enumerated properly and all three eMMC partitions are visible: BLK4: Alias(s): VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,000030520000000000)/eMMC(0x 0)/Ctrl(0x0) BLK5: Alias(s): VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,000030520000000000)/eMMC(0x 0)/Ctrl(0x1) BLK6: Alias(s): VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,000030520000000000)/eMMC(0x 0)/Ctrl(0x2) Shell> devices <snip> E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,0000305200000000 00)/eMMC(0x0)/Ctrl(0x0) EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,0000305200000000 00)/eMMC(0x0)/Ctrl(0x1) EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0-ECA0CC8D514A,0000305200000000 00)/eMMC(0x0)/Ctrl(0x2) So you can add following: Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> -Sumit ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-27 13:38 ` sumit.garg @ 2019-06-28 0:57 ` Wu, Hao A 2019-06-28 6:32 ` Marcin Wojtas 0 siblings, 1 reply; 19+ messages in thread From: Wu, Hao A @ 2019-06-28 0:57 UTC (permalink / raw) To: Sumit Garg, Ard Biesheuvel Cc: edk2-devel-groups-io, Marcin Wojtas, Albecki, Mateusz > -----Original Message----- > From: Sumit Garg [mailto:sumit.garg@linaro.org] > Sent: Thursday, June 27, 2019 9:39 PM > To: Ard Biesheuvel > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> > wrote: > > > > (+ Sumit) > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > > > > > -----Original Message----- > > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > To: Albecki, Mateusz > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > MdeModulePkg/SdMmcHcDxe: > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > Hi Mateusz, > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > easily do a quick check for regression? > > > > > > > > Thanks, > > > > Marcin > > > > > > > > > Hello Marcin, > > > > > > I have just pushed the series at: > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > Please help to check. > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > override infrastructure. > > > > Sumit, please reply here with your results. And thanks again! > > I did picked this patch-set and applied on top of edk2 master branch. > It works well on SynQuacer with eMMC device enumerated properly and > all three eMMC partitions are visible: > > BLK4: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x0) > BLK5: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x1) > BLK6: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x2) > > Shell> devices > <snip> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x0) > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x1) > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x2) > > So you can add following: > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> Thanks a lot for the testing. Best Regards, Hao Wu > > -Sumit ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 0:57 ` Wu, Hao A @ 2019-06-28 6:32 ` Marcin Wojtas 2019-06-28 6:33 ` Marcin Wojtas 2019-06-28 7:23 ` Wu, Hao A 0 siblings, 2 replies; 19+ messages in thread From: Marcin Wojtas @ 2019-06-28 6:32 UTC (permalink / raw) To: Wu, Hao A, Albecki, Mateusz Cc: Sumit Garg, Ard Biesheuvel, edk2-devel-groups-io [-- Attachment #1: Type: text/plain, Size: 4904 bytes --] Hi, I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: Board 1 (out of tree): SD - OK, MMC - OK Board 2: (Armada80x0McBin): SD - OK, MMC - OK Board 3: (Armada70x0Db): SD - problems, MMC - OK In the latter case I get stall and booting takes around 3 minutes. Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. SdCardIdentification: Found a SD device at slot [0] SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 However right after Csd register dump the booting stalls until printing following and continuing: FatOpenDevice: read of part_lba failed Time out This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -536,8 +536,8 @@ SdCardSwitch ( AccessMode = 0xF; } - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? Best regards, Marcin pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > > -----Original Message----- > > From: Sumit Garg [mailto:sumit.garg@linaro.org] > > Sent: Thursday, June 27, 2019 9:39 PM > > To: Ard Biesheuvel > > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > > Implement revision 3 of SdMmcOverrideProtocol > > > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> > > wrote: > > > > > > (+ Sumit) > > > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > > To: Albecki, Mateusz > > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > > MdeModulePkg/SdMmcHcDxe: > > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > > > Hi Mateusz, > > > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > > easily do a quick check for regression? > > > > > > > > > > Thanks, > > > > > Marcin > > > > > > > > > > > > Hello Marcin, > > > > > > > > I have just pushed the series at: > > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > > > Please help to check. > > > > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > > override infrastructure. > > > > > > Sumit, please reply here with your results. And thanks again! > > > > I did picked this patch-set and applied on top of edk2 master branch. > > It works well on SynQuacer with eMMC device enumerated properly and > > all three eMMC partitions are visible: > > > > BLK4: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x0) > > BLK5: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x1) > > BLK6: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x2) > > > > Shell> devices > > <snip> > > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x0) > > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x1) > > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x2) > > > > So you can add following: > > > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> > > > Thanks a lot for the testing. > > Best Regards, > Hao Wu > > > > > > -Sumit > > > > [-- Attachment #2: Type: text/html, Size: 6991 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 6:32 ` Marcin Wojtas @ 2019-06-28 6:33 ` Marcin Wojtas 2019-06-28 7:23 ` Wu, Hao A 1 sibling, 0 replies; 19+ messages in thread From: Marcin Wojtas @ 2019-06-28 6:33 UTC (permalink / raw) To: Wu, Hao A, Albecki, Mateusz Cc: Sumit Garg, Ard Biesheuvel, edk2-devel-groups-io pt., 28 cze 2019 o 08:32 Marcin Wojtas <mw@semihalf.com> napisał(a): > > Hi, > > I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: > Board 1 (out of tree): SD - OK, MMC - OK > Board 2: (Armada80x0McBin): SD - OK, MMC - OK > Board 3: (Armada70x0Db): SD - problems, MMC - OK > > In the latter case I get stall and booting takes around 3 minutes. > Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. > > I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. > SdCardIdentification: Found a SD device at slot [0] > SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 > > However right after Csd register dump the booting stalls until printing following and continuing: > FatOpenDevice: read of part_lba failed Time out > > This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: vanilla edk2 of course :) > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > @@ -536,8 +536,8 @@ SdCardSwitch ( > AccessMode = 0xF; > } > > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ > - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M > + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M > ModeValue; > > Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? > > Best regards, > Marcin > > > pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com> napisał(a): >> >> > -----Original Message----- >> > From: Sumit Garg [mailto:sumit.garg@linaro.org] >> > Sent: Thursday, June 27, 2019 9:39 PM >> > To: Ard Biesheuvel >> > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz >> > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: >> > Implement revision 3 of SdMmcOverrideProtocol >> > >> > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> >> > wrote: >> > > >> > > (+ Sumit) >> > > >> > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: >> > > > >> > > > > -----Original Message----- >> > > > > From: Marcin Wojtas [mailto:mw@semihalf.com] >> > > > > Sent: Thursday, June 27, 2019 2:25 PM >> > > > > To: Albecki, Mateusz >> > > > > Cc: edk2-devel-groups-io; Wu, Hao A >> > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] >> > MdeModulePkg/SdMmcHcDxe: >> > > > > Implement revision 3 of SdMmcOverrideProtocol >> > > > > >> > > > > Hi Mateusz, >> > > > > >> > > > > Can you please push those patches somewhere (github?) so that I can >> > > > > easily do a quick check for regression? >> > > > > >> > > > > Thanks, >> > > > > Marcin >> > > > >> > > > >> > > > Hello Marcin, >> > > > >> > > > I have just pushed the series at: >> > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 >> > > > >> > > > Please help to check. >> > > > >> > > >> > > I have cc'ed my colleague Sumit, who has kindly agreed to regression >> > > test this branch on Socionext SynQuacer, which also uses the SD/MMC >> > > override infrastructure. >> > > >> > > Sumit, please reply here with your results. And thanks again! >> > >> > I did picked this patch-set and applied on top of edk2 master branch. >> > It works well on SynQuacer with eMMC device enumerated properly and >> > all three eMMC partitions are visible: >> > >> > BLK4: Alias(s): >> > VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,000030520000000000)/eMMC(0x >> > 0)/Ctrl(0x0) >> > BLK5: Alias(s): >> > VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,000030520000000000)/eMMC(0x >> > 0)/Ctrl(0x1) >> > BLK6: Alias(s): >> > VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,000030520000000000)/eMMC(0x >> > 0)/Ctrl(0x2) >> > >> > Shell> devices >> > <snip> >> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,0000305200000000 >> > 00)/eMMC(0x0)/Ctrl(0x0) >> > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,0000305200000000 >> > 00)/eMMC(0x0)/Ctrl(0x1) >> > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- >> > ECA0CC8D514A,0000305200000000 >> > 00)/eMMC(0x0)/Ctrl(0x2) >> > >> > So you can add following: >> > >> > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> >> >> >> Thanks a lot for the testing. >> >> Best Regards, >> Hao Wu >> >> >> > >> > -Sumit >> >> >> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 6:32 ` Marcin Wojtas 2019-06-28 6:33 ` Marcin Wojtas @ 2019-06-28 7:23 ` Wu, Hao A 2019-06-28 7:41 ` Marcin Wojtas 1 sibling, 1 reply; 19+ messages in thread From: Wu, Hao A @ 2019-06-28 7:23 UTC (permalink / raw) To: Marcin Wojtas, Albecki, Mateusz Cc: Sumit Garg, Ard Biesheuvel, edk2-devel-groups-io [-- Attachment #1: Type: text/plain, Size: 5847 bytes --] Hello Marcin, Do you mean by only reverting as below: - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; All your devices work fine? Since the origin code is clearly not correct (DriveStrength used 2 times, the offset of PowerLimit is not 4, should be 12 according to SD spec). Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com] Sent: Friday, June 28, 2019 2:33 PM To: Wu, Hao A; Albecki, Mateusz Cc: Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi, I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: Board 1 (out of tree): SD - OK, MMC - OK Board 2: (Armada80x0McBin): SD - OK, MMC - OK Board 3: (Armada70x0Db): SD - problems, MMC - OK In the latter case I get stall and booting takes around 3 minutes. Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. SdCardIdentification: Found a SD device at slot [0] SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 However right after Csd register dump the booting stalls until printing following and continuing: FatOpenDevice: read of part_lba failed Time out This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -536,8 +536,8 @@ SdCardSwitch ( AccessMode = 0xF; } - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? Best regards, Marcin pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): > -----Original Message----- > From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>] > Sent: Thursday, June 27, 2019 9:39 PM > To: Ard Biesheuvel > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> > wrote: > > > > (+ Sumit) > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: > > > > > > > -----Original Message----- > > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > To: Albecki, Mateusz > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > MdeModulePkg/SdMmcHcDxe: > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > Hi Mateusz, > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > easily do a quick check for regression? > > > > > > > > Thanks, > > > > Marcin > > > > > > > > > Hello Marcin, > > > > > > I have just pushed the series at: > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > Please help to check. > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > override infrastructure. > > > > Sumit, please reply here with your results. And thanks again! > > I did picked this patch-set and applied on top of edk2 master branch. > It works well on SynQuacer with eMMC device enumerated properly and > all three eMMC partitions are visible: > > BLK4: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x0) > BLK5: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x1) > BLK6: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x2) > > Shell> devices > <snip> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x0) > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x1) > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x2) > > So you can add following: > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>> Thanks a lot for the testing. Best Regards, Hao Wu > > -Sumit [-- Attachment #2: Type: text/html, Size: 35719 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 7:23 ` Wu, Hao A @ 2019-06-28 7:41 ` Marcin Wojtas 2019-06-28 8:12 ` Albecki, Mateusz 2019-06-28 8:14 ` Wu, Hao A 0 siblings, 2 replies; 19+ messages in thread From: Marcin Wojtas @ 2019-06-28 7:41 UTC (permalink / raw) To: Wu, Hao A Cc: Albecki, Mateusz, Sumit Garg, Ard Biesheuvel, edk2-devel-groups-io [-- Attachment #1: Type: text/plain, Size: 6700 bytes --] Hi Hao, pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > Hello Marcin, > > > > Do you mean by only reverting as below: > > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & > 0xF) << 4) | \ > > - ((DriverStrength & 0xF) << 8) | (( > PowerLimit & 0xF) << 12) | \ > > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) > << 4) | \^M > > + ((DriverStrength & 0xF) << 8) | > ((DriverStrength & 0xF) << 12) | \^M > > ModeValue; > > > > All your devices work fine? > > > > Since the origin code is clearly not correct (DriveStrength used 2 times, > > the offset of PowerLimit is not 4, should be 12 according to SD spec). > Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for the next time I think it may be worth to split some improvements out of such big patches. I won't be able to debug my board until second week of July (at best), so in order not to block you please go ahead with merging (the most important board (MacchiatoBin) seems not suffer any regression). Best regards, Marcin > > > Best Regards, > > Hao Wu > > > > *From:* Marcin Wojtas [mailto:mw@semihalf.com] > *Sent:* Friday, June 28, 2019 2:33 PM > *To:* Wu, Hao A; Albecki, Mateusz > *Cc:* Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io > *Subject:* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > > > Hi, > > > > I was almost happily sending you email with 'tested-by' information, but I > checked 3 boards: > > Board 1 (out of tree): SD - OK, MMC - OK > > Board 2: (Armada80x0McBin): SD - OK, MMC - OK > > Board 3: (Armada70x0Db): SD - problems, MMC - OK > > > > In the latter case I get stall and booting takes around 3 minutes. > > Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of > SdMmcOverrideProtocol" patch it works as before. > > > > I enabled debugs, and in theory everything seems fine, the DriverStrength > is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. > > SdCardIdentification: Found a SD device at slot [0] > > SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock > freq[MHz] = 50, driver strength = 255 > > > > However right after Csd register dump the booting stalls until printing > following and continuing: > > FatOpenDevice: read of part_lba failed Time out > > > > This is absent from the prints I dumped from vanilla kernel. Despite I > currently really have no time to additional debug, I checked and with > following diff, everything works as before: > > > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > @@ -536,8 +536,8 @@ SdCardSwitch ( > > AccessMode = 0xF; > > } > > > > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & > 0xF) << 4) | \ > > - ((DriverStrength & 0xF) << 8) | (( > PowerLimit & 0xF) << 12) | \ > > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) > << 4) | \^M > > + ((DriverStrength & 0xF) << 8) | > ((DriverStrength & 0xF) << 12) | \^M > > ModeValue; > > > > Above is restoring SdMmcCmdBlk.CommandArgument to the state from before > the patch in question. Now the question - why the layout of the command > changed? CommandSystem was unused before, and PowerLimit changed its > position. Is this change really related to the rest of the patch? What is > the justification? > > > > Best regards, > > Marcin > > > > > > pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > > > -----Original Message----- > > From: Sumit Garg [mailto:sumit.garg@linaro.org] > > Sent: Thursday, June 27, 2019 9:39 PM > > To: Ard Biesheuvel > > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > > Implement revision 3 of SdMmcOverrideProtocol > > > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> > > wrote: > > > > > > (+ Sumit) > > > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > > To: Albecki, Mateusz > > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > > MdeModulePkg/SdMmcHcDxe: > > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > > > Hi Mateusz, > > > > > > > > > > Can you please push those patches somewhere (github?) so that I > can > > > > > easily do a quick check for regression? > > > > > > > > > > Thanks, > > > > > Marcin > > > > > > > > > > > > Hello Marcin, > > > > > > > > I have just pushed the series at: > > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > > > Please help to check. > > > > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > > override infrastructure. > > > > > > Sumit, please reply here with your results. And thanks again! > > > > I did picked this patch-set and applied on top of edk2 master branch. > > It works well on SynQuacer with eMMC device enumerated properly and > > all three eMMC partitions are visible: > > > > BLK4: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x0) > > BLK5: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x1) > > BLK6: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x2) > > > > Shell> devices > > <snip> > > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x0) > > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x1) > > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x2) > > > > So you can add following: > > > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> > > > Thanks a lot for the testing. > > Best Regards, > Hao Wu > > > > > > -Sumit > > > > [-- Attachment #2: Type: text/html, Size: 16454 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 7:41 ` Marcin Wojtas @ 2019-06-28 8:12 ` Albecki, Mateusz 2019-06-28 8:57 ` Marcin Wojtas 2019-06-28 8:14 ` Wu, Hao A 1 sibling, 1 reply; 19+ messages in thread From: Albecki, Mateusz @ 2019-06-28 8:12 UTC (permalink / raw) To: devel@edk2.groups.io, mw@semihalf.com, Wu, Hao A Cc: Sumit Garg, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 8215 bytes --] Hi, Do you use override driver with this SD controller(if yes and it is open source could you provide the link)? There is one change introduced in this patch that might require changes in the override driver. We have added enumeration for SdMmcSdDs and SdMmcSdHs modes which were so far indicated to override drivers as SdMmcUhsSdr12 and SdMmcUhsSdr25 respectively so if there were any actions that were necessary for high speed to work and those were done only for SdMmcUhsSdr25 then that might be the reason for regression. Thanks, Mateusz From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Marcin Wojtas Sent: Friday, June 28, 2019 9:42 AM To: Wu, Hao A <hao.a.wu@intel.com> Cc: Albecki, Mateusz <mateusz.albecki@intel.com>; Sumit Garg <sumit.garg@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel-groups-io <devel@edk2.groups.io> Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi Hao, pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): Hello Marcin, Do you mean by only reverting as below: - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; All your devices work fine? Since the origin code is clearly not correct (DriveStrength used 2 times, the offset of PowerLimit is not 4, should be 12 according to SD spec). Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for the next time I think it may be worth to split some improvements out of such big patches. I won't be able to debug my board until second week of July (at best), so in order not to block you please go ahead with merging (the most important board (MacchiatoBin) seems not suffer any regression). Best regards, Marcin Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] Sent: Friday, June 28, 2019 2:33 PM To: Wu, Hao A; Albecki, Mateusz Cc: Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi, I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: Board 1 (out of tree): SD - OK, MMC - OK Board 2: (Armada80x0McBin): SD - OK, MMC - OK Board 3: (Armada70x0Db): SD - problems, MMC - OK In the latter case I get stall and booting takes around 3 minutes. Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. SdCardIdentification: Found a SD device at slot [0] SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 However right after Csd register dump the booting stalls until printing following and continuing: FatOpenDevice: read of part_lba failed Time out This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -536,8 +536,8 @@ SdCardSwitch ( AccessMode = 0xF; } - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? Best regards, Marcin pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): > -----Original Message----- > From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>] > Sent: Thursday, June 27, 2019 9:39 PM > To: Ard Biesheuvel > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> > wrote: > > > > (+ Sumit) > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: > > > > > > > -----Original Message----- > > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > To: Albecki, Mateusz > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > MdeModulePkg/SdMmcHcDxe: > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > Hi Mateusz, > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > easily do a quick check for regression? > > > > > > > > Thanks, > > > > Marcin > > > > > > > > > Hello Marcin, > > > > > > I have just pushed the series at: > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > Please help to check. > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > override infrastructure. > > > > Sumit, please reply here with your results. And thanks again! > > I did picked this patch-set and applied on top of edk2 master branch. > It works well on SynQuacer with eMMC device enumerated properly and > all three eMMC partitions are visible: > > BLK4: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x0) > BLK5: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x1) > BLK6: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x2) > > Shell> devices > <snip> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x0) > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x1) > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x2) > > So you can add following: > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>> Thanks a lot for the testing. Best Regards, Hao Wu > > -Sumit -------------------------------------------------------------------- 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. [-- Attachment #2: Type: text/html, Size: 29647 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 8:12 ` Albecki, Mateusz @ 2019-06-28 8:57 ` Marcin Wojtas 2019-07-01 1:26 ` Wu, Hao A 0 siblings, 1 reply; 19+ messages in thread From: Marcin Wojtas @ 2019-06-28 8:57 UTC (permalink / raw) To: Albecki, Mateusz Cc: devel@edk2.groups.io, Wu, Hao A, Sumit Garg, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 9360 bytes --] Hi Mateusz, pt., 28 cze 2019 o 10:12 Albecki, Mateusz <mateusz.albecki@intel.com> napisał(a): > Hi, > > > > Do you use override driver with this SD controller(if yes and it is open > source could you provide the link)? > [MW] Of course it's open source https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Marvell/Drivers/SdMmc/XenonDxe The platform is Armada70x0Db.dsc from the same edk2-platforms repo. > There is one change introduced in this patch that might require changes in > the override driver. We have added enumeration for SdMmcSdDs and SdMmcSdHs > modes which were so far indicated to override drivers as SdMmcUhsSdr12 and > SdMmcUhsSdr25 respectively so if there were any actions that were necessary > for high speed to work and those were done only for SdMmcUhsSdr25 then that > might be the reason for regression. > > > [MW] Now, that was the reason. This interface required special handling for high speed. This patch fixed it: https://pastebin.com/rdRe9wAh I will submit it after your patchset is merged. Best regards, Marcin > Thanks, > > Mateusz > > > > *From:* devel@edk2.groups.io [mailto:devel@edk2.groups.io] *On Behalf Of *Marcin > Wojtas > *Sent:* Friday, June 28, 2019 9:42 AM > *To:* Wu, Hao A <hao.a.wu@intel.com> > *Cc:* Albecki, Mateusz <mateusz.albecki@intel.com>; Sumit Garg < > sumit.garg@linaro.org>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; > edk2-devel-groups-io <devel@edk2.groups.io> > *Subject:* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > > > Hi Hao, > > > > pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > > Hello Marcin, > > > > Do you mean by only reverting as below: > > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & > 0xF) << 4) | \ > > - ((DriverStrength & 0xF) << 8) | (( > PowerLimit & 0xF) << 12) | \ > > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) > << 4) | \^M > > + ((DriverStrength & 0xF) << 8) | > ((DriverStrength & 0xF) << 12) | \^M > > ModeValue; > > > > All your devices work fine? > > > > Since the origin code is clearly not correct (DriveStrength used 2 times, > > the offset of PowerLimit is not 4, should be 12 according to SD spec). > > > > Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for > the next time I think it may be worth to split some improvements out of > such big patches. > > > > I won't be able to debug my board until second week of July (at best), so > in order not to block you please go ahead with merging (the most important > board (MacchiatoBin) seems not suffer any regression). > > > > Best regards, > > Marcin > > > > > > Best Regards, > > Hao Wu > > > > *From:* Marcin Wojtas [mailto:mw@semihalf.com] > *Sent:* Friday, June 28, 2019 2:33 PM > *To:* Wu, Hao A; Albecki, Mateusz > *Cc:* Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io > *Subject:* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > > > Hi, > > > > I was almost happily sending you email with 'tested-by' information, but I > checked 3 boards: > > Board 1 (out of tree): SD - OK, MMC - OK > > Board 2: (Armada80x0McBin): SD - OK, MMC - OK > > Board 3: (Armada70x0Db): SD - problems, MMC - OK > > > > In the latter case I get stall and booting takes around 3 minutes. > > Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of > SdMmcOverrideProtocol" patch it works as before. > > > > I enabled debugs, and in theory everything seems fine, the DriverStrength > is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. > > SdCardIdentification: Found a SD device at slot [0] > > SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock > freq[MHz] = 50, driver strength = 255 > > > > However right after Csd register dump the booting stalls until printing > following and continuing: > > FatOpenDevice: read of part_lba failed Time out > > > > This is absent from the prints I dumped from vanilla kernel. Despite I > currently really have no time to additional debug, I checked and with > following diff, everything works as before: > > > > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c > > @@ -536,8 +536,8 @@ SdCardSwitch ( > > AccessMode = 0xF; > > } > > > > - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & > 0xF) << 4) | \ > > - ((DriverStrength & 0xF) << 8) | (( > PowerLimit & 0xF) << 12) | \ > > + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) > << 4) | \^M > > + ((DriverStrength & 0xF) << 8) | > ((DriverStrength & 0xF) << 12) | \^M > > ModeValue; > > > > Above is restoring SdMmcCmdBlk.CommandArgument to the state from before > the patch in question. Now the question - why the layout of the command > changed? CommandSystem was unused before, and PowerLimit changed its > position. Is this change really related to the rest of the patch? What is > the justification? > > > > Best regards, > > Marcin > > > > > > pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com> napisał(a): > > > -----Original Message----- > > From: Sumit Garg [mailto:sumit.garg@linaro.org] > > Sent: Thursday, June 27, 2019 9:39 PM > > To: Ard Biesheuvel > > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > > Implement revision 3 of SdMmcOverrideProtocol > > > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org> > > wrote: > > > > > > (+ Sumit) > > > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com> wrote: > > > > > > > > > -----Original Message----- > > > > > From: Marcin Wojtas [mailto:mw@semihalf.com] > > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > > To: Albecki, Mateusz > > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > > MdeModulePkg/SdMmcHcDxe: > > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > > > Hi Mateusz, > > > > > > > > > > Can you please push those patches somewhere (github?) so that I > can > > > > > easily do a quick check for regression? > > > > > > > > > > Thanks, > > > > > Marcin > > > > > > > > > > > > Hello Marcin, > > > > > > > > I have just pushed the series at: > > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > > > Please help to check. > > > > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > > override infrastructure. > > > > > > Sumit, please reply here with your results. And thanks again! > > > > I did picked this patch-set and applied on top of edk2 master branch. > > It works well on SynQuacer with eMMC device enumerated properly and > > all three eMMC partitions are visible: > > > > BLK4: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x0) > > BLK5: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x1) > > BLK6: Alias(s): > > VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,000030520000000000)/eMMC(0x > > 0)/Ctrl(0x2) > > > > Shell> devices > > <snip> > > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x0) > > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x1) > > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > > ECA0CC8D514A,0000305200000000 > > 00)/eMMC(0x0)/Ctrl(0x2) > > > > So you can add following: > > > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org> > > > Thanks a lot for the testing. > > Best Regards, > Hao Wu > > > > > > -Sumit > > > > --------------------------------------------------------------------- > > *Intel Technology Poland sp. z o.o.*ul. Słowackiego 173 | 80-298 > Gdańsk | Sąd Rejonowy Gdańsk Północ | VII Wydział > Gospodarczy Krajowego Rejestru Sądowego - KRS 101882 | NIP > 957-07-52-316 | Kapitał zakładowy 200.000 PLN. > > Ta wiadomość wraz z załącznikami jest przeznaczona dla > określonego adresata i może zawierać informacje poufne. W razie > przypadkowego otrzymania tej wiadomości, prosimy o powiadomienie > nadawcy oraz trwałe jej usunięcie; jakiekolwiek przeglądanie > 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. > > [-- Attachment #2: Type: text/html, Size: 27186 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 8:57 ` Marcin Wojtas @ 2019-07-01 1:26 ` Wu, Hao A 0 siblings, 0 replies; 19+ messages in thread From: Wu, Hao A @ 2019-07-01 1:26 UTC (permalink / raw) To: Marcin Wojtas, Albecki, Mateusz Cc: devel@edk2.groups.io, Sumit Garg, Ard Biesheuvel [-- Attachment #1: Type: text/plain, Size: 9556 bytes --] Hello Marcin, Thanks for the verification. The series has been pushed via commits a37e18f6fc..adec1f5deb. Please help to raise if there is any help needed. Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com] Sent: Friday, June 28, 2019 4:58 PM To: Albecki, Mateusz Cc: devel@edk2.groups.io; Wu, Hao A; Sumit Garg; Ard Biesheuvel Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi Mateusz, pt., 28 cze 2019 o 10:12 Albecki, Mateusz <mateusz.albecki@intel.com<mailto:mateusz.albecki@intel.com>> napisał(a): Hi, Do you use override driver with this SD controller(if yes and it is open source could you provide the link)? [MW] Of course it's open source https://github.com/tianocore/edk2-platforms/tree/master/Silicon/Marvell/Drivers/SdMmc/XenonDxe The platform is Armada70x0Db.dsc from the same edk2-platforms repo. There is one change introduced in this patch that might require changes in the override driver. We have added enumeration for SdMmcSdDs and SdMmcSdHs modes which were so far indicated to override drivers as SdMmcUhsSdr12 and SdMmcUhsSdr25 respectively so if there were any actions that were necessary for high speed to work and those were done only for SdMmcUhsSdr25 then that might be the reason for regression. [MW] Now, that was the reason. This interface required special handling for high speed. This patch fixed it: https://pastebin.com/rdRe9wAh I will submit it after your patchset is merged. Best regards, Marcin Thanks, Mateusz From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io<mailto:devel@edk2.groups.io>] On Behalf Of Marcin Wojtas Sent: Friday, June 28, 2019 9:42 AM To: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> Cc: Albecki, Mateusz <mateusz.albecki@intel.com<mailto:mateusz.albecki@intel.com>>; Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>>; Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>>; edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi Hao, pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): Hello Marcin, Do you mean by only reverting as below: - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; All your devices work fine? Since the origin code is clearly not correct (DriveStrength used 2 times, the offset of PowerLimit is not 4, should be 12 according to SD spec). Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for the next time I think it may be worth to split some improvements out of such big patches. I won't be able to debug my board until second week of July (at best), so in order not to block you please go ahead with merging (the most important board (MacchiatoBin) seems not suffer any regression). Best regards, Marcin Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] Sent: Friday, June 28, 2019 2:33 PM To: Wu, Hao A; Albecki, Mateusz Cc: Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi, I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: Board 1 (out of tree): SD - OK, MMC - OK Board 2: (Armada80x0McBin): SD - OK, MMC - OK Board 3: (Armada70x0Db): SD - problems, MMC - OK In the latter case I get stall and booting takes around 3 minutes. Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. SdCardIdentification: Found a SD device at slot [0] SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 However right after Csd register dump the booting stalls until printing following and continuing: FatOpenDevice: read of part_lba failed Time out This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -536,8 +536,8 @@ SdCardSwitch ( AccessMode = 0xF; } - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? Best regards, Marcin pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): > -----Original Message----- > From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>] > Sent: Thursday, June 27, 2019 9:39 PM > To: Ard Biesheuvel > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> > wrote: > > > > (+ Sumit) > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: > > > > > > > -----Original Message----- > > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > To: Albecki, Mateusz > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > MdeModulePkg/SdMmcHcDxe: > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > Hi Mateusz, > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > easily do a quick check for regression? > > > > > > > > Thanks, > > > > Marcin > > > > > > > > > Hello Marcin, > > > > > > I have just pushed the series at: > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > Please help to check. > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > override infrastructure. > > > > Sumit, please reply here with your results. And thanks again! > > I did picked this patch-set and applied on top of edk2 master branch. > It works well on SynQuacer with eMMC device enumerated properly and > all three eMMC partitions are visible: > > BLK4: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x0) > BLK5: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x1) > BLK6: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x2) > > Shell> devices > <snip> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x0) > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x1) > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x2) > > So you can add following: > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>> Thanks a lot for the testing. Best Regards, Hao Wu > > -Sumit --------------------------------------------------------------------- Intel Technology Poland sp. z o.o. ul. Słowackiego 173 | 80-298 Gdańsk | Sąd Rejonowy Gdańsk Północ | VII Wydział Gospodarczy Krajowego Rejestru Sądowego - KRS 101882 | NIP 957-07-52-316 | Kapitał zakładowy 200.000 PLN. Ta wiadomość wraz z załącznikami jest przeznaczona dla określonego adresata i może zawierać informacje poufne. W razie przypadkowego otrzymania tej wiadomości, prosimy o powiadomienie nadawcy oraz trwałe jej usunięcie; jakiekolwiek przeglądanie 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. [-- Attachment #2: Type: text/html, Size: 60217 bytes --] ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol 2019-06-28 7:41 ` Marcin Wojtas 2019-06-28 8:12 ` Albecki, Mateusz @ 2019-06-28 8:14 ` Wu, Hao A 1 sibling, 0 replies; 19+ messages in thread From: Wu, Hao A @ 2019-06-28 8:14 UTC (permalink / raw) To: Marcin Wojtas Cc: Albecki, Mateusz, Sumit Garg, Ard Biesheuvel, edk2-devel-groups-io [-- Attachment #1.1: Type: text/plain, Size: 7327 bytes --] Hello Marcin, I just realized that the series added 2 new bus modes (SdMmcSdDs and SdMmcSdHs) for 3.3V signaling SD card: Judging from your log: SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 which “bus timing = 1” means SdMmcSdHs (3.3V High Speed mode) is selected by the SdMmcPciHcDxe driver. So I have attached a patch to handle the 2 added bus modes in Silicon/Marvell/Drivers/SdMmc/XenonDxe/. Or you can get the patch at https://github.com/hwu25/edk2-platforms/tree/Marvell_XenonDxe Please help to see if this solve your issue, thanks in advance. Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com] Sent: Friday, June 28, 2019 3:42 PM To: Wu, Hao A Cc: Albecki, Mateusz; Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi Hao, pt., 28 cze 2019 o 09:23 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): Hello Marcin, Do you mean by only reverting as below: - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; All your devices work fine? Since the origin code is clearly not correct (DriveStrength used 2 times, the offset of PowerLimit is not 4, should be 12 according to SD spec). Ok, just rechecked. It doesn't help for my 1 problematic case. Anyway for the next time I think it may be worth to split some improvements out of such big patches. I won't be able to debug my board until second week of July (at best), so in order not to block you please go ahead with merging (the most important board (MacchiatoBin) seems not suffer any regression). Best regards, Marcin Best Regards, Hao Wu From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] Sent: Friday, June 28, 2019 2:33 PM To: Wu, Hao A; Albecki, Mateusz Cc: Sumit Garg; Ard Biesheuvel; edk2-devel-groups-io Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Hi, I was almost happily sending you email with 'tested-by' information, but I checked 3 boards: Board 1 (out of tree): SD - OK, MMC - OK Board 2: (Armada80x0McBin): SD - OK, MMC - OK Board 3: (Armada70x0Db): SD - problems, MMC - OK In the latter case I get stall and booting takes around 3 minutes. Without "MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol" patch it works as before. I enabled debugs, and in theory everything seems fine, the DriverStrength is set to EDKII_SD_MMC_DRIVER_STRENGTH_IGNORE. SdCardIdentification: Found a SD device at slot [0] SdCardSetBusMode: Target bus mode: bus timing = 1, bus width = 4, clock freq[MHz] = 50, driver strength = 255 However right after Csd register dump the booting stalls until printing following and continuing: FatOpenDevice: read of part_lba failed Time out This is absent from the prints I dumped from vanilla kernel. Despite I currently really have no time to additional debug, I checked and with following diff, everything works as before: --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c @@ -536,8 +536,8 @@ SdCardSwitch ( AccessMode = 0xF; } - SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((CommandSystem & 0xF) << 4) | \ - ((DriverStrength & 0xF) << 8) | ((PowerLimit & 0xF) << 12) | \ + SdMmcCmdBlk.CommandArgument = (AccessMode & 0xF) | ((PowerLimit & 0xF) << 4) | \^M + ((DriverStrength & 0xF) << 8) | ((DriverStrength & 0xF) << 12) | \^M ModeValue; Above is restoring SdMmcCmdBlk.CommandArgument to the state from before the patch in question. Now the question - why the layout of the command changed? CommandSystem was unused before, and PowerLimit changed its position. Is this change really related to the rest of the patch? What is the justification? Best regards, Marcin pt., 28 cze 2019 o 02:57 Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> napisał(a): > -----Original Message----- > From: Sumit Garg [mailto:sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>] > Sent: Thursday, June 27, 2019 9:39 PM > To: Ard Biesheuvel > Cc: edk2-devel-groups-io; Wu, Hao A; Marcin Wojtas; Albecki, Mateusz > Subject: Re: [edk2-devel] [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: > Implement revision 3 of SdMmcOverrideProtocol > > On Thu, 27 Jun 2019 at 13:40, Ard Biesheuvel <ard.biesheuvel@linaro.org<mailto:ard.biesheuvel@linaro.org>> > wrote: > > > > (+ Sumit) > > > > On Thu, 27 Jun 2019 at 08:29, Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>> wrote: > > > > > > > -----Original Message----- > > > > From: Marcin Wojtas [mailto:mw@semihalf.com<mailto:mw@semihalf.com>] > > > > Sent: Thursday, June 27, 2019 2:25 PM > > > > To: Albecki, Mateusz > > > > Cc: edk2-devel-groups-io; Wu, Hao A > > > > Subject: Re: [edk2-devel] [PATCH v4 0/2] > MdeModulePkg/SdMmcHcDxe: > > > > Implement revision 3 of SdMmcOverrideProtocol > > > > > > > > Hi Mateusz, > > > > > > > > Can you please push those patches somewhere (github?) so that I can > > > > easily do a quick check for regression? > > > > > > > > Thanks, > > > > Marcin > > > > > > > > > Hello Marcin, > > > > > > I have just pushed the series at: > > > https://github.com/hwu25/edk2/tree/sdmmc_override_extend_v4 > > > > > > Please help to check. > > > > > > > I have cc'ed my colleague Sumit, who has kindly agreed to regression > > test this branch on Socionext SynQuacer, which also uses the SD/MMC > > override infrastructure. > > > > Sumit, please reply here with your results. And thanks again! > > I did picked this patch-set and applied on top of edk2 master branch. > It works well on SynQuacer with eMMC device enumerated properly and > all three eMMC partitions are visible: > > BLK4: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x0) > BLK5: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x1) > BLK6: Alias(s): > VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,000030520000000000)/eMMC(0x > 0)/Ctrl(0x2) > > Shell> devices > <snip> > E9 D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x0) > EA D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x1) > EB D - - 1 1 0 VenHw(0D51905B-B77E-452A-A2C0- > ECA0CC8D514A,0000305200000000 > 00)/eMMC(0x0)/Ctrl(0x2) > > So you can add following: > > Regression-tested-by: Sumit Garg <sumit.garg@linaro.org<mailto:sumit.garg@linaro.org>> Thanks a lot for the testing. Best Regards, Hao Wu > > -Sumit [-- Attachment #1.2: Type: text/html, Size: 46442 bytes --] [-- Attachment #2: 0001-Marvell-XenonDxe-Add-handles-for-SD-3.3V-bus-modes.patch --] [-- Type: application/octet-stream, Size: 1803 bytes --] From 94a64de93172b8096fe6555194705abede4e03ff Mon Sep 17 00:00:00 2001 From: Hao A Wu <hao.a.wu@intel.com> Date: Fri, 28 Jun 2019 16:06:04 +0800 Subject: [PATCH] Marvell/XenonDxe: Add handles for SD 3.3V bus modes --- Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c index 0b4949db28..1893d4bb09 100755 --- a/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c +++ b/Silicon/Marvell/Drivers/SdMmc/XenonDxe/XenonSdhci.c @@ -387,6 +387,8 @@ XenonPhySlowMode ( if (((Timing == SdMmcUhsSdr50) || (Timing == SdMmcUhsSdr25) || (Timing == SdMmcUhsSdr12) || + (Timing == SdMmcSdHs) || + (Timing == SdMmcSdDs) || (Timing == SdMmcMmcHsDdr) || (Timing == SdMmcMmcHsSdr) || (Timing == SdMmcMmcLegacy)) && SlowMode) { @@ -423,7 +425,7 @@ XenonSetPhy ( Var &= ~(EMMC5_1_FC_CMD_PD | EMMC5_1_FC_DQ_PD); XenonHcRwMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_PAD_CONTROL1, FALSE, SDHC_REG_SIZE_4B, &Var); - if (Timing == SdMmcUhsSdr12) { + if (Timing == SdMmcUhsSdr12 || Timing == SdMmcSdDs) { if (SlowMode) { XenonHcRwMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, TRUE, SDHC_REG_SIZE_4B, &Var); Var |= QSN_PHASE_SLOW_MODE_BIT; @@ -442,7 +444,8 @@ XenonSetPhy ( (Timing == SdMmcUhsDdr50) || (Timing == SdMmcUhsSdr50) || (Timing == SdMmcUhsSdr104) || - (Timing == SdMmcUhsSdr25)) { + (Timing == SdMmcUhsSdr25) || + (Timing == SdMmcSdHs)) { Var = ~OUTPUT_QSN_PHASE_SELECT; XenonHcAndMmio (PciIo, SD_BAR_INDEX, EMMC_PHY_TIMING_ADJUST, SDHC_REG_SIZE_4B, &Var); } -- 2.12.0.windows.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-07-01 1:26 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-26 13:10 [PATCH v4 0/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz 2019-06-26 13:10 ` [PATCH v4 1/2] MdeModulePkg/SdMmcOverride: Add GetOperatingParam notify phase Albecki, Mateusz 2019-06-27 2:49 ` [edk2-devel] " Wu, Hao A 2019-06-26 13:10 ` [PATCH v4 2/2] MdeModulePkg/SdMmcHcDxe: Implement revision 3 of SdMmcOverrideProtocol Albecki, Mateusz 2019-06-27 2:49 ` Wu, Hao A 2019-06-27 2:49 ` [PATCH v4 0/2] " Wu, Hao A 2019-06-27 6:25 ` [edk2-devel] " Marcin Wojtas 2019-06-27 6:29 ` Wu, Hao A 2019-06-27 8:10 ` Ard Biesheuvel 2019-06-27 13:38 ` sumit.garg 2019-06-28 0:57 ` Wu, Hao A 2019-06-28 6:32 ` Marcin Wojtas 2019-06-28 6:33 ` Marcin Wojtas 2019-06-28 7:23 ` Wu, Hao A 2019-06-28 7:41 ` Marcin Wojtas 2019-06-28 8:12 ` Albecki, Mateusz 2019-06-28 8:57 ` Marcin Wojtas 2019-07-01 1:26 ` Wu, Hao A 2019-06-28 8:14 ` Wu, Hao A
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox