* [PATCH v3 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
2018-11-08 1:57 [PATCH v3 0/4] SdMmcOverride extension Marcin Wojtas
@ 2018-11-08 1:57 ` Marcin Wojtas
2018-11-08 1:57 ` [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 1:57 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, hao.a.wu, michael.d.kinney, liming.gao,
ard.biesheuvel, nadavh, mw, jsd, tm, jaz
In order to ensure bigger flexibility in the NotifyPhase
routine of the SdMmcOverride protocol, enable using an
optional phase-specific data. This will allow to exchange
more information between the protocol producer driver
and SdMmcPciHcDxe in the newly added callbacks.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
MdeModulePkg/Include/Protocol/SdMmcOverride.h | 4 +++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 12 ++++++++----
2 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 0766252..8a7669e 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -63,6 +63,7 @@ EFI_STATUS
@param[in] PhaseType The type of operation and whether the
hook is invoked right before (pre) or
right after (post)
+ @param[in,out] PhaseData The pointer to a phase-specific data.
@retval EFI_SUCCESS The override function completed successfully.
@retval EFI_NOT_FOUND The specified controller or slot does not exist.
@@ -74,7 +75,8 @@ EFI_STATUS
(EFIAPI * EDKII_SD_MMC_NOTIFY_PHASE) (
IN EFI_HANDLE ControllerHandle,
IN UINT8 Slot,
- IN EDKII_SD_MMC_PHASE_TYPE PhaseType
+ IN EDKII_SD_MMC_PHASE_TYPE PhaseType,
+ IN OUT VOID *PhaseData
);
struct _EDKII_SD_MMC_OVERRIDE {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index bedc968..923c55b 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -444,7 +444,8 @@ SdMmcHcReset (
Status = mOverride->NotifyPhase (
Private->ControllerHandle,
Slot,
- EdkiiSdMmcResetPre);
+ EdkiiSdMmcResetPre,
+ NULL);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN,
"%a: SD/MMC pre reset notifier callback failed - %r\n",
@@ -494,7 +495,8 @@ SdMmcHcReset (
Status = mOverride->NotifyPhase (
Private->ControllerHandle,
Slot,
- EdkiiSdMmcResetPost);
+ EdkiiSdMmcResetPost,
+ NULL);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN,
"%a: SD/MMC post reset notifier callback failed - %r\n",
@@ -1088,7 +1090,8 @@ SdMmcHcInitHost (
Status = mOverride->NotifyPhase (
Private->ControllerHandle,
Slot,
- EdkiiSdMmcInitHostPre);
+ EdkiiSdMmcInitHostPre,
+ NULL);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN,
"%a: SD/MMC pre init notifier callback failed - %r\n",
@@ -1123,7 +1126,8 @@ SdMmcHcInitHost (
Status = mOverride->NotifyPhase (
Private->ControllerHandle,
Slot,
- EdkiiSdMmcInitHostPost);
+ EdkiiSdMmcInitHostPost,
+ NULL);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN,
"%a: SD/MMC post init notifier callback failed - %r\n",
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
2018-11-08 1:57 [PATCH v3 0/4] SdMmcOverride extension Marcin Wojtas
2018-11-08 1:57 ` [PATCH v3 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase Marcin Wojtas
@ 2018-11-08 1:57 ` Marcin Wojtas
2018-11-08 11:06 ` Ard Biesheuvel
2018-11-08 1:57 ` [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
2018-11-08 1:57 ` [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency Marcin Wojtas
3 siblings, 1 reply; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 1:57 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, hao.a.wu, michael.d.kinney, liming.gao,
ard.biesheuvel, nadavh, mw, jsd, tm, jaz
From: Tomasz Michalec <tm@semihalf.com>
Some SD Host Controllers use different values in Host Control 2 Register
to select UHS Mode. This patch adds a new UhsSignaling type routine to
the NotifyPhase of the SdMmcOverride protocol.
UHS signaling configuration is moved to a common, default routine
(SdMmcHcUhsSignaling). After it is executed, the protocol producer
can override the values if needed..
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 32 +++++
MdeModulePkg/Include/Protocol/SdMmcOverride.h | 17 +++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 136 +++++++++++++-------
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 31 ++++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 66 ++++++++++
5 files changed, 225 insertions(+), 57 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 7e3f588..1a11d51 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -63,6 +63,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#define SD_MMC_HC_CTRL_VER 0xFE
//
+// SD Host Controller bits to HOST_CTRL2 register
+//
+#define SD_MMC_HC_CTRL_UHS_MASK 0x0007
+#define SD_MMC_HC_CTRL_UHS_SDR12 0x0000
+#define SD_MMC_HC_CTRL_UHS_SDR25 0x0001
+#define SD_MMC_HC_CTRL_UHS_SDR50 0x0002
+#define SD_MMC_HC_CTRL_UHS_SDR104 0x0003
+#define SD_MMC_HC_CTRL_UHS_DDR50 0x0004
+#define SD_MMC_HC_CTRL_MMC_LEGACY 0x0000
+#define SD_MMC_HC_CTRL_MMC_HS_SDR 0x0001
+#define SD_MMC_HC_CTRL_MMC_HS_DDR 0x0004
+#define SD_MMC_HC_CTRL_MMC_HS200 0x0003
+#define SD_MMC_HC_CTRL_MMC_HS400 0x0005
+
+//
// The transfer modes supported by SD Host Controller
// Simplified Spec 3.0 Table 1-2
//
@@ -518,4 +533,21 @@ SdMmcHcInitTimeoutCtrl (
IN UINT8 Slot
);
+/**
+ Set SD Host Controller control 2 registry according to selected speed.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number of the SD card to send the command to.
+ @param[in] Timing The timing to select.
+
+ @retval EFI_SUCCESS The timing is set successfully.
+ @retval Others The timing isn't set successfully.
+**/
+EFI_STATUS
+SdMmcHcUhsSignaling (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8 Slot,
+ IN SD_MMC_BUS_MODE Timing
+ );
+
#endif
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 8a7669e..f948bef 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -26,11 +26,28 @@
typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
+//
+// Bus timing modes
+//
+typedef enum {
+ SdMmcUhsSdr12,
+ SdMmcUhsSdr25,
+ SdMmcUhsSdr50,
+ SdMmcUhsSdr104,
+ SdMmcUhsDdr50,
+ SdMmcMmcLegacy,
+ SdMmcMmcHsSdr,
+ SdMmcMmcHsDdr,
+ SdMmcMmcHs200,
+ SdMmcMmcHs400,
+} SD_MMC_BUS_MODE;
+
typedef enum {
EdkiiSdMmcResetPre,
EdkiiSdMmcResetPost,
EdkiiSdMmcInitHostPre,
EdkiiSdMmcInitHostPost,
+ EdkiiSdMmcUhsSignaling,
} EDKII_SD_MMC_PHASE_TYPE;
/**
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index c5fd214..473df8d 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed (
IN UINT8 BusWidth
)
{
- EFI_STATUS Status;
- UINT8 HsTiming;
- UINT8 HostCtrl1;
- UINT8 HostCtrl2;
+ EFI_STATUS Status;
+ UINT8 HsTiming;
+ UINT8 HostCtrl1;
+ SD_MMC_BUS_MODE Timing;
+ SD_MMC_HC_PRIVATE_DATA *Private;
+
+ Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
if (EFI_ERROR (Status)) {
@@ -758,29 +761,37 @@ EmmcSwitchToHighSpeed (
return Status;
}
- //
- // Clean UHS Mode Select field of Host Control 2 reigster before update
- //
- HostCtrl2 = (UINT8)~0x7;
- Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- //
- // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/50
- //
if (IsDdr) {
- HostCtrl2 = BIT2;
+ Timing = SdMmcMmcHsDdr;
} else if (ClockFreq == 52) {
- HostCtrl2 = BIT0;
+ Timing = SdMmcMmcHsSdr;
} else {
- HostCtrl2 = 0;
+ Timing = SdMmcMmcLegacy;
}
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+
+ Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcUhsSignaling,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
HsTiming = 1;
Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
@@ -814,10 +825,13 @@ EmmcSwitchToHS200 (
IN UINT8 BusWidth
)
{
- EFI_STATUS Status;
- UINT8 HsTiming;
- UINT8 HostCtrl2;
- UINT16 ClockCtrl;
+ 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)) {
return EFI_INVALID_PARAMETER;
@@ -837,22 +851,32 @@ EmmcSwitchToHS200 (
if (EFI_ERROR (Status)) {
return Status;
}
- //
- // Clean UHS Mode Select field of Host Control 2 reigster before update
- //
- HostCtrl2 = (UINT8)~0x7;
- Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+
+ Timing = SdMmcMmcHs200;
+
+ Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
- //
- // Set UHS Mode Select field of Host Control 2 reigster to SDR104
- //
- HostCtrl2 = BIT0 | BIT1;
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
- if (EFI_ERROR (Status)) {
- return Status;
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcUhsSignaling,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
}
+
//
// Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
//
@@ -910,9 +934,12 @@ EmmcSwitchToHS400 (
IN UINT32 ClockFreq
)
{
- EFI_STATUS Status;
- UINT8 HsTiming;
- UINT8 HostCtrl2;
+ EFI_STATUS Status;
+ UINT8 HsTiming;
+ SD_MMC_BUS_MODE Timing;
+ SD_MMC_HC_PRIVATE_DATA *Private;
+
+ Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8);
if (EFI_ERROR (Status)) {
@@ -933,21 +960,30 @@ EmmcSwitchToHS400 (
if (EFI_ERROR (Status)) {
return Status;
}
- //
- // Clean UHS Mode Select field of Host Control 2 reigster before update
- //
- HostCtrl2 = (UINT8)~0x7;
- Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+
+ Timing = SdMmcMmcHs400;
+
+ Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
- //
- // Set UHS Mode Select field of Host Control 2 reigster to HS400
- //
- HostCtrl2 = BIT0 | BIT2;
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
- if (EFI_ERROR (Status)) {
- return Status;
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcUhsSignaling,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
}
HsTiming = 3;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 6ee9ed7..850ad26 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -784,8 +784,8 @@ SdCardSetBusMode (
UINT8 BusWidth;
UINT8 AccessMode;
UINT8 HostCtrl1;
- UINT8 HostCtrl2;
UINT8 SwitchResp[64];
+ SD_MMC_BUS_MODE Timing;
SD_MMC_HC_PRIVATE_DATA *Private;
Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
@@ -817,18 +817,23 @@ SdCardSetBusMode (
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;
}
Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, SwitchResp);
@@ -854,15 +859,27 @@ SdCardSetBusMode (
}
}
- HostCtrl2 = (UINT8)~0x7;
- Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+ Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
- HostCtrl2 = AccessMode;
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
- if (EFI_ERROR (Status)) {
- return Status;
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcUhsSignaling,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
}
Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 923c55b..85aa625 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1138,6 +1138,72 @@ SdMmcHcInitHost (
}
/**
+ Set SD Host Controler control 2 registry according to selected speed.
+
+ @param[in] PciIo The PCI IO protocol instance.
+ @param[in] Slot The slot number of the SD card to send the command to.
+ @param[in] Timing The timing to select.
+
+ @retval EFI_SUCCESS The timing is set successfully.
+ @retval Others The timing isn't set successfully.
+**/
+EFI_STATUS
+SdMmcHcUhsSignaling (
+ IN EFI_PCI_IO_PROTOCOL *PciIo,
+ IN UINT8 Slot,
+ IN SD_MMC_BUS_MODE Timing
+ )
+{
+ EFI_STATUS Status;
+ UINT8 HostCtrl2;
+
+ HostCtrl2 = (UINT8)~SD_MMC_HC_CTRL_UHS_MASK;
+ Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ switch (Timing) {
+ case SdMmcUhsSdr12:
+ HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR12;
+ break;
+ case SdMmcUhsSdr25:
+ HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR25;
+ break;
+ case SdMmcUhsSdr50:
+ HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR50;
+ break;
+ case SdMmcUhsSdr104:
+ HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR104;
+ break;
+ case SdMmcUhsDdr50:
+ HostCtrl2 = SD_MMC_HC_CTRL_UHS_DDR50;
+ break;
+ case SdMmcMmcLegacy:
+ HostCtrl2 = SD_MMC_HC_CTRL_MMC_LEGACY;
+ break;
+ case SdMmcMmcHsSdr:
+ HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_SDR;
+ break;
+ case SdMmcMmcHsDdr:
+ HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_DDR;
+ break;
+ case SdMmcMmcHs200:
+ HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS200;
+ break;
+ case SdMmcMmcHs400:
+ HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS400;
+ break;
+ default:
+ HostCtrl2 = 0;
+ break;
+ }
+ Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+
+ return Status;
+}
+
+/**
Turn on/off LED.
@param[in] PciIo The PCI IO protocol instance.
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
2018-11-08 1:57 ` [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
@ 2018-11-08 11:06 ` Ard Biesheuvel
2018-11-08 11:15 ` Marcin Wojtas
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-08 11:06 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel@lists.01.org, Leif Lindholm, Wu, Hao A,
Kinney, Michael D, Gao, Liming, Nadav Haklai,
Jan Dąbroś, Tomasz Michalec, Grzegorz Jaszczyk
On 8 November 2018 at 02:57, Marcin Wojtas <mw@semihalf.com> wrote:
> From: Tomasz Michalec <tm@semihalf.com>
>
> Some SD Host Controllers use different values in Host Control 2 Register
> to select UHS Mode. This patch adds a new UhsSignaling type routine to
> the NotifyPhase of the SdMmcOverride protocol.
>
> UHS signaling configuration is moved to a common, default routine
> (SdMmcHcUhsSignaling). After it is executed, the protocol producer
> can override the values if needed..
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 32 +++++
> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 17 +++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 136 +++++++++++++-------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 31 ++++-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 66 ++++++++++
> 5 files changed, 225 insertions(+), 57 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 7e3f588..1a11d51 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -63,6 +63,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> #define SD_MMC_HC_CTRL_VER 0xFE
>
> //
> +// SD Host Controller bits to HOST_CTRL2 register
> +//
> +#define SD_MMC_HC_CTRL_UHS_MASK 0x0007
> +#define SD_MMC_HC_CTRL_UHS_SDR12 0x0000
> +#define SD_MMC_HC_CTRL_UHS_SDR25 0x0001
> +#define SD_MMC_HC_CTRL_UHS_SDR50 0x0002
> +#define SD_MMC_HC_CTRL_UHS_SDR104 0x0003
> +#define SD_MMC_HC_CTRL_UHS_DDR50 0x0004
> +#define SD_MMC_HC_CTRL_MMC_LEGACY 0x0000
> +#define SD_MMC_HC_CTRL_MMC_HS_SDR 0x0001
> +#define SD_MMC_HC_CTRL_MMC_HS_DDR 0x0004
> +#define SD_MMC_HC_CTRL_MMC_HS200 0x0003
> +#define SD_MMC_HC_CTRL_MMC_HS400 0x0005
> +
> +//
> // The transfer modes supported by SD Host Controller
> // Simplified Spec 3.0 Table 1-2
> //
> @@ -518,4 +533,21 @@ SdMmcHcInitTimeoutCtrl (
> IN UINT8 Slot
> );
>
> +/**
> + Set SD Host Controller control 2 registry according to selected speed.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] Slot The slot number of the SD card to send the command to.
> + @param[in] Timing The timing to select.
> +
> + @retval EFI_SUCCESS The timing is set successfully.
> + @retval Others The timing isn't set successfully.
> +**/
> +EFI_STATUS
> +SdMmcHcUhsSignaling (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8 Slot,
> + IN SD_MMC_BUS_MODE Timing
> + );
> +
> #endif
> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> index 8a7669e..f948bef 100644
> --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> @@ -26,11 +26,28 @@
>
> typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
>
> +//
> +// Bus timing modes
> +//
> +typedef enum {
> + SdMmcUhsSdr12,
> + SdMmcUhsSdr25,
> + SdMmcUhsSdr50,
> + SdMmcUhsSdr104,
> + SdMmcUhsDdr50,
> + SdMmcMmcLegacy,
> + SdMmcMmcHsSdr,
> + SdMmcMmcHsDdr,
> + SdMmcMmcHs200,
> + SdMmcMmcHs400,
> +} SD_MMC_BUS_MODE;
> +
> typedef enum {
> EdkiiSdMmcResetPre,
> EdkiiSdMmcResetPost,
> EdkiiSdMmcInitHostPre,
> EdkiiSdMmcInitHostPost,
> + EdkiiSdMmcUhsSignaling,
> } EDKII_SD_MMC_PHASE_TYPE;
>
> /**
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index c5fd214..473df8d 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed (
> IN UINT8 BusWidth
> )
> {
> - EFI_STATUS Status;
> - UINT8 HsTiming;
> - UINT8 HostCtrl1;
> - UINT8 HostCtrl2;
> + EFI_STATUS Status;
> + UINT8 HsTiming;
> + UINT8 HostCtrl1;
> + SD_MMC_BUS_MODE Timing;
> + SD_MMC_HC_PRIVATE_DATA *Private;
> +
> + Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
>
> Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
> if (EFI_ERROR (Status)) {
> @@ -758,29 +761,37 @@ EmmcSwitchToHighSpeed (
> return Status;
> }
>
> - //
> - // Clean UHS Mode Select field of Host Control 2 reigster before update
> - //
> - HostCtrl2 = (UINT8)~0x7;
> - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> - //
> - // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/50
> - //
> if (IsDdr) {
> - HostCtrl2 = BIT2;
> + Timing = SdMmcMmcHsDdr;
> } else if (ClockFreq == 52) {
> - HostCtrl2 = BIT0;
> + Timing = SdMmcMmcHsSdr;
> } else {
> - HostCtrl2 = 0;
> + Timing = SdMmcMmcLegacy;
> }
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> +
> + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcUhsSignaling,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
> +
Can we move the override handling into SdMmcHcUhsSignaling()? That
way, we no longer have to duplicate it 4 times.
> HsTiming = 1;
> Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
>
> @@ -814,10 +825,13 @@ EmmcSwitchToHS200 (
> IN UINT8 BusWidth
> )
> {
> - EFI_STATUS Status;
> - UINT8 HsTiming;
> - UINT8 HostCtrl2;
> - UINT16 ClockCtrl;
> + 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)) {
> return EFI_INVALID_PARAMETER;
> @@ -837,22 +851,32 @@ EmmcSwitchToHS200 (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - //
> - // Clean UHS Mode Select field of Host Control 2 reigster before update
> - //
> - HostCtrl2 = (UINT8)~0x7;
> - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> +
> + Timing = SdMmcMmcHs200;
> +
> + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - //
> - // Set UHS Mode Select field of Host Control 2 reigster to SDR104
> - //
> - HostCtrl2 = BIT0 | BIT1;
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> - if (EFI_ERROR (Status)) {
> - return Status;
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcUhsSignaling,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> }
> +
> //
> // Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
> //
> @@ -910,9 +934,12 @@ EmmcSwitchToHS400 (
> IN UINT32 ClockFreq
> )
> {
> - EFI_STATUS Status;
> - UINT8 HsTiming;
> - UINT8 HostCtrl2;
> + EFI_STATUS Status;
> + UINT8 HsTiming;
> + SD_MMC_BUS_MODE Timing;
> + SD_MMC_HC_PRIVATE_DATA *Private;
> +
> + Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
>
> Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8);
> if (EFI_ERROR (Status)) {
> @@ -933,21 +960,30 @@ EmmcSwitchToHS400 (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - //
> - // Clean UHS Mode Select field of Host Control 2 reigster before update
> - //
> - HostCtrl2 = (UINT8)~0x7;
> - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> +
> + Timing = SdMmcMmcHs400;
> +
> + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - //
> - // Set UHS Mode Select field of Host Control 2 reigster to HS400
> - //
> - HostCtrl2 = BIT0 | BIT2;
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> - if (EFI_ERROR (Status)) {
> - return Status;
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcUhsSignaling,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> }
>
> HsTiming = 3;
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 6ee9ed7..850ad26 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -784,8 +784,8 @@ SdCardSetBusMode (
> UINT8 BusWidth;
> UINT8 AccessMode;
> UINT8 HostCtrl1;
> - UINT8 HostCtrl2;
> UINT8 SwitchResp[64];
> + SD_MMC_BUS_MODE Timing;
> SD_MMC_HC_PRIVATE_DATA *Private;
>
> Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> @@ -817,18 +817,23 @@ SdCardSetBusMode (
> 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;
> }
>
> Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, SwitchResp);
> @@ -854,15 +859,27 @@ SdCardSetBusMode (
> }
> }
>
> - HostCtrl2 = (UINT8)~0x7;
> - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - HostCtrl2 = AccessMode;
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> - if (EFI_ERROR (Status)) {
> - return Status;
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcUhsSignaling,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> }
>
> Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 923c55b..85aa625 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -1138,6 +1138,72 @@ SdMmcHcInitHost (
> }
>
> /**
> + Set SD Host Controler control 2 registry according to selected speed.
> +
> + @param[in] PciIo The PCI IO protocol instance.
> + @param[in] Slot The slot number of the SD card to send the command to.
> + @param[in] Timing The timing to select.
> +
> + @retval EFI_SUCCESS The timing is set successfully.
> + @retval Others The timing isn't set successfully.
> +**/
> +EFI_STATUS
> +SdMmcHcUhsSignaling (
> + IN EFI_PCI_IO_PROTOCOL *PciIo,
> + IN UINT8 Slot,
> + IN SD_MMC_BUS_MODE Timing
> + )
> +{
> + EFI_STATUS Status;
> + UINT8 HostCtrl2;
> +
> + HostCtrl2 = (UINT8)~SD_MMC_HC_CTRL_UHS_MASK;
> + Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + switch (Timing) {
> + case SdMmcUhsSdr12:
> + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR12;
> + break;
> + case SdMmcUhsSdr25:
> + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR25;
> + break;
> + case SdMmcUhsSdr50:
> + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR50;
> + break;
> + case SdMmcUhsSdr104:
> + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR104;
> + break;
> + case SdMmcUhsDdr50:
> + HostCtrl2 = SD_MMC_HC_CTRL_UHS_DDR50;
> + break;
> + case SdMmcMmcLegacy:
> + HostCtrl2 = SD_MMC_HC_CTRL_MMC_LEGACY;
> + break;
> + case SdMmcMmcHsSdr:
> + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_SDR;
> + break;
> + case SdMmcMmcHsDdr:
> + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_DDR;
> + break;
> + case SdMmcMmcHs200:
> + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS200;
> + break;
> + case SdMmcMmcHs400:
> + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS400;
> + break;
> + default:
> + HostCtrl2 = 0;
> + break;
> + }
> + Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> +
> + return Status;
> +}
> +
> +/**
> Turn on/off LED.
>
> @param[in] PciIo The PCI IO protocol instance.
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
2018-11-08 11:06 ` Ard Biesheuvel
@ 2018-11-08 11:15 ` Marcin Wojtas
0 siblings, 0 replies; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 11:15 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-01, Leif Lindholm, hao.a.wu, Kinney, Michael D,
Gao, Liming, nadavh, jsd@semihalf.com, Tomasz Michalec,
Grzegorz Jaszczyk
Hi Ard,
I'm glad you're back :)
czw., 8 lis 2018 o 12:06 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On 8 November 2018 at 02:57, Marcin Wojtas <mw@semihalf.com> wrote:
> > From: Tomasz Michalec <tm@semihalf.com>
> >
> > Some SD Host Controllers use different values in Host Control 2 Register
> > to select UHS Mode. This patch adds a new UhsSignaling type routine to
> > the NotifyPhase of the SdMmcOverride protocol.
> >
> > UHS signaling configuration is moved to a common, default routine
> > (SdMmcHcUhsSignaling). After it is executed, the protocol producer
> > can override the values if needed..
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 32 +++++
> > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 17 +++
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 136 +++++++++++++-------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 31 ++++-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 66 ++++++++++
> > 5 files changed, 225 insertions(+), 57 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > index 7e3f588..1a11d51 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> > @@ -63,6 +63,21 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> > #define SD_MMC_HC_CTRL_VER 0xFE
> >
> > //
> > +// SD Host Controller bits to HOST_CTRL2 register
> > +//
> > +#define SD_MMC_HC_CTRL_UHS_MASK 0x0007
> > +#define SD_MMC_HC_CTRL_UHS_SDR12 0x0000
> > +#define SD_MMC_HC_CTRL_UHS_SDR25 0x0001
> > +#define SD_MMC_HC_CTRL_UHS_SDR50 0x0002
> > +#define SD_MMC_HC_CTRL_UHS_SDR104 0x0003
> > +#define SD_MMC_HC_CTRL_UHS_DDR50 0x0004
> > +#define SD_MMC_HC_CTRL_MMC_LEGACY 0x0000
> > +#define SD_MMC_HC_CTRL_MMC_HS_SDR 0x0001
> > +#define SD_MMC_HC_CTRL_MMC_HS_DDR 0x0004
> > +#define SD_MMC_HC_CTRL_MMC_HS200 0x0003
> > +#define SD_MMC_HC_CTRL_MMC_HS400 0x0005
> > +
> > +//
> > // The transfer modes supported by SD Host Controller
> > // Simplified Spec 3.0 Table 1-2
> > //
> > @@ -518,4 +533,21 @@ SdMmcHcInitTimeoutCtrl (
> > IN UINT8 Slot
> > );
> >
> > +/**
> > + Set SD Host Controller control 2 registry according to selected speed.
> > +
> > + @param[in] PciIo The PCI IO protocol instance.
> > + @param[in] Slot The slot number of the SD card to send the command to.
> > + @param[in] Timing The timing to select.
> > +
> > + @retval EFI_SUCCESS The timing is set successfully.
> > + @retval Others The timing isn't set successfully.
> > +**/
> > +EFI_STATUS
> > +SdMmcHcUhsSignaling (
> > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE Timing
> > + );
> > +
> > #endif
> > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > index 8a7669e..f948bef 100644
> > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > @@ -26,11 +26,28 @@
> >
> > typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
> >
> > +//
> > +// Bus timing modes
> > +//
> > +typedef enum {
> > + SdMmcUhsSdr12,
> > + SdMmcUhsSdr25,
> > + SdMmcUhsSdr50,
> > + SdMmcUhsSdr104,
> > + SdMmcUhsDdr50,
> > + SdMmcMmcLegacy,
> > + SdMmcMmcHsSdr,
> > + SdMmcMmcHsDdr,
> > + SdMmcMmcHs200,
> > + SdMmcMmcHs400,
> > +} SD_MMC_BUS_MODE;
> > +
> > typedef enum {
> > EdkiiSdMmcResetPre,
> > EdkiiSdMmcResetPost,
> > EdkiiSdMmcInitHostPre,
> > EdkiiSdMmcInitHostPost,
> > + EdkiiSdMmcUhsSignaling,
> > } EDKII_SD_MMC_PHASE_TYPE;
> >
> > /**
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index c5fd214..473df8d 100755
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -740,10 +740,13 @@ EmmcSwitchToHighSpeed (
> > IN UINT8 BusWidth
> > )
> > {
> > - EFI_STATUS Status;
> > - UINT8 HsTiming;
> > - UINT8 HostCtrl1;
> > - UINT8 HostCtrl2;
> > + EFI_STATUS Status;
> > + UINT8 HsTiming;
> > + UINT8 HostCtrl1;
> > + SD_MMC_BUS_MODE Timing;
> > + SD_MMC_HC_PRIVATE_DATA *Private;
> > +
> > + Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> >
> > Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
> > if (EFI_ERROR (Status)) {
> > @@ -758,29 +761,37 @@ EmmcSwitchToHighSpeed (
> > return Status;
> > }
> >
> > - //
> > - // Clean UHS Mode Select field of Host Control 2 reigster before update
> > - //
> > - HostCtrl2 = (UINT8)~0x7;
> > - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > - if (EFI_ERROR (Status)) {
> > - return Status;
> > - }
> > - //
> > - // Set UHS Mode Select field of Host Control 2 reigster to SDR12/25/50
> > - //
> > if (IsDdr) {
> > - HostCtrl2 = BIT2;
> > + Timing = SdMmcMmcHsDdr;
> > } else if (ClockFreq == 52) {
> > - HostCtrl2 = BIT0;
> > + Timing = SdMmcMmcHsSdr;
> > } else {
> > - HostCtrl2 = 0;
> > + Timing = SdMmcMmcLegacy;
> > }
> > - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > +
> > + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> >
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcUhsSignaling,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> > +
>
> Can we move the override handling into SdMmcHcUhsSignaling()? That
> way, we no longer have to duplicate it 4 times.
Sure! I should've come up on this on my own...
Thanks,
Marcin
>
> > HsTiming = 1;
> > Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> >
> > @@ -814,10 +825,13 @@ EmmcSwitchToHS200 (
> > IN UINT8 BusWidth
> > )
> > {
> > - EFI_STATUS Status;
> > - UINT8 HsTiming;
> > - UINT8 HostCtrl2;
> > - UINT16 ClockCtrl;
> > + 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)) {
> > return EFI_INVALID_PARAMETER;
> > @@ -837,22 +851,32 @@ EmmcSwitchToHS200 (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - //
> > - // Clean UHS Mode Select field of Host Control 2 reigster before update
> > - //
> > - HostCtrl2 = (UINT8)~0x7;
> > - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > +
> > + Timing = SdMmcMmcHs200;
> > +
> > + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - //
> > - // Set UHS Mode Select field of Host Control 2 reigster to SDR104
> > - //
> > - HostCtrl2 = BIT0 | BIT1;
> > - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > - if (EFI_ERROR (Status)) {
> > - return Status;
> > +
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcUhsSignaling,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > }
> > +
> > //
> > // Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
> > //
> > @@ -910,9 +934,12 @@ EmmcSwitchToHS400 (
> > IN UINT32 ClockFreq
> > )
> > {
> > - EFI_STATUS Status;
> > - UINT8 HsTiming;
> > - UINT8 HostCtrl2;
> > + EFI_STATUS Status;
> > + UINT8 HsTiming;
> > + SD_MMC_BUS_MODE Timing;
> > + SD_MMC_HC_PRIVATE_DATA *Private;
> > +
> > + Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> >
> > Status = EmmcSwitchToHS200 (PciIo, PassThru, Slot, Rca, ClockFreq, 8);
> > if (EFI_ERROR (Status)) {
> > @@ -933,21 +960,30 @@ EmmcSwitchToHS400 (
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - //
> > - // Clean UHS Mode Select field of Host Control 2 reigster before update
> > - //
> > - HostCtrl2 = (UINT8)~0x7;
> > - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > +
> > + Timing = SdMmcMmcHs400;
> > +
> > + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - //
> > - // Set UHS Mode Select field of Host Control 2 reigster to HS400
> > - //
> > - HostCtrl2 = BIT0 | BIT2;
> > - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > - if (EFI_ERROR (Status)) {
> > - return Status;
> > +
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcUhsSignaling,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > }
> >
> > HsTiming = 3;
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 6ee9ed7..850ad26 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -784,8 +784,8 @@ SdCardSetBusMode (
> > UINT8 BusWidth;
> > UINT8 AccessMode;
> > UINT8 HostCtrl1;
> > - UINT8 HostCtrl2;
> > UINT8 SwitchResp[64];
> > + SD_MMC_BUS_MODE Timing;
> > SD_MMC_HC_PRIVATE_DATA *Private;
> >
> > Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
> > @@ -817,18 +817,23 @@ SdCardSetBusMode (
> > 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;
> > }
> >
> > Status = SdCardSwitch (PassThru, Slot, AccessMode, 0xF, 0xF, 0xF, TRUE, SwitchResp);
> > @@ -854,15 +859,27 @@ SdCardSetBusMode (
> > }
> > }
> >
> > - HostCtrl2 = (UINT8)~0x7;
> > - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > + Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
> > if (EFI_ERROR (Status)) {
> > return Status;
> > }
> > - HostCtrl2 = AccessMode;
> > - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > - if (EFI_ERROR (Status)) {
> > - return Status;
> > +
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcUhsSignaling,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > }
> >
> > Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > index 923c55b..85aa625 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> > @@ -1138,6 +1138,72 @@ SdMmcHcInitHost (
> > }
> >
> > /**
> > + Set SD Host Controler control 2 registry according to selected speed.
> > +
> > + @param[in] PciIo The PCI IO protocol instance.
> > + @param[in] Slot The slot number of the SD card to send the command to.
> > + @param[in] Timing The timing to select.
> > +
> > + @retval EFI_SUCCESS The timing is set successfully.
> > + @retval Others The timing isn't set successfully.
> > +**/
> > +EFI_STATUS
> > +SdMmcHcUhsSignaling (
> > + IN EFI_PCI_IO_PROTOCOL *PciIo,
> > + IN UINT8 Slot,
> > + IN SD_MMC_BUS_MODE Timing
> > + )
> > +{
> > + EFI_STATUS Status;
> > + UINT8 HostCtrl2;
> > +
> > + HostCtrl2 = (UINT8)~SD_MMC_HC_CTRL_UHS_MASK;
> > + Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + switch (Timing) {
> > + case SdMmcUhsSdr12:
> > + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR12;
> > + break;
> > + case SdMmcUhsSdr25:
> > + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR25;
> > + break;
> > + case SdMmcUhsSdr50:
> > + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR50;
> > + break;
> > + case SdMmcUhsSdr104:
> > + HostCtrl2 = SD_MMC_HC_CTRL_UHS_SDR104;
> > + break;
> > + case SdMmcUhsDdr50:
> > + HostCtrl2 = SD_MMC_HC_CTRL_UHS_DDR50;
> > + break;
> > + case SdMmcMmcLegacy:
> > + HostCtrl2 = SD_MMC_HC_CTRL_MMC_LEGACY;
> > + break;
> > + case SdMmcMmcHsSdr:
> > + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_SDR;
> > + break;
> > + case SdMmcMmcHsDdr:
> > + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS_DDR;
> > + break;
> > + case SdMmcMmcHs200:
> > + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS200;
> > + break;
> > + case SdMmcMmcHs400:
> > + HostCtrl2 = SD_MMC_HC_CTRL_MMC_HS400;
> > + break;
> > + default:
> > + HostCtrl2 = 0;
> > + break;
> > + }
> > + Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> > +
> > + return Status;
> > +}
> > +
> > +/**
> > Turn on/off LED.
> >
> > @param[in] PciIo The PCI IO protocol instance.
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
2018-11-08 1:57 [PATCH v3 0/4] SdMmcOverride extension Marcin Wojtas
2018-11-08 1:57 ` [PATCH v3 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase Marcin Wojtas
2018-11-08 1:57 ` [PATCH v3 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
@ 2018-11-08 1:57 ` Marcin Wojtas
2018-11-08 11:09 ` Ard Biesheuvel
2018-11-08 1:57 ` [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency Marcin Wojtas
3 siblings, 1 reply; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 1:57 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, hao.a.wu, michael.d.kinney, liming.gao,
ard.biesheuvel, nadavh, mw, jsd, tm, jaz
From: Tomasz Michalec <tm@semihalf.com>
Some SD Host Controlers need to do additional opperations after clock
frequency switch.
This patch add new callback type to NotifyPhase of the SdMmcOverride
protocol. It is called after EmmcSwitchClockFreq and SdMmcHcClockSupply.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
MdeModulePkg/Include/Protocol/SdMmcOverride.h | 1 +
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 60 ++++++++++++++++++++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++
3 files changed, 79 insertions(+)
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index f948bef..6160b5b 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -48,6 +48,7 @@ typedef enum {
EdkiiSdMmcInitHostPre,
EdkiiSdMmcInitHostPost,
EdkiiSdMmcUhsSignaling,
+ EdkiiSdMmcSwitchClockFreqPost,
} EDKII_SD_MMC_PHASE_TYPE;
/**
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 473df8d..6fc6871 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -794,6 +794,27 @@ EmmcSwitchToHighSpeed (
HsTiming = 1;
Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcSwitchClockFreqPost,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
return Status;
}
@@ -904,6 +925,24 @@ EmmcSwitchToHS200 (
return Status;
}
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcSwitchClockFreqPost,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth);
return Status;
@@ -988,6 +1027,27 @@ EmmcSwitchToHS400 (
HsTiming = 3;
Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcSwitchClockFreqPost,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 850ad26..5408bbc 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -887,6 +887,24 @@ SdCardSetBusMode (
return Status;
}
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ Private->ControllerHandle,
+ Slot,
+ EdkiiSdMmcSwitchClockFreqPost,
+ &Timing
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+ }
+
if ((AccessMode == 3) || ((AccessMode == 2) && (Capability->TuningSDR50 != 0))) {
Status = SdCardTuningClock (PciIo, PassThru, Slot);
if (EFI_ERROR (Status)) {
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
2018-11-08 1:57 ` [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
@ 2018-11-08 11:09 ` Ard Biesheuvel
2018-11-08 11:18 ` Marcin Wojtas
0 siblings, 1 reply; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-08 11:09 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel@lists.01.org, Leif Lindholm, Wu, Hao A,
Kinney, Michael D, Gao, Liming, Nadav Haklai,
Jan Dąbroś, Tomasz Michalec, Grzegorz Jaszczyk
On 8 November 2018 at 02:57, Marcin Wojtas <mw@semihalf.com> wrote:
> From: Tomasz Michalec <tm@semihalf.com>
>
> Some SD Host Controlers need to do additional opperations after clock
> frequency switch.
>
> This patch add new callback type to NotifyPhase of the SdMmcOverride
> protocol. It is called after EmmcSwitchClockFreq and SdMmcHcClockSupply.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> ---
> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 1 +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 60 ++++++++++++++++++++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++
> 3 files changed, 79 insertions(+)
>
> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> index f948bef..6160b5b 100644
> --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> @@ -48,6 +48,7 @@ typedef enum {
> EdkiiSdMmcInitHostPre,
> EdkiiSdMmcInitHostPost,
> EdkiiSdMmcUhsSignaling,
> + EdkiiSdMmcSwitchClockFreqPost,
> } EDKII_SD_MMC_PHASE_TYPE;
>
> /**
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index 473df8d..6fc6871 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -794,6 +794,27 @@ EmmcSwitchToHighSpeed (
>
> HsTiming = 1;
> Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcSwitchClockFreqPost,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
Is there a way we could move this into EmmcSwitchClockFreq() rather
than duplicate it?
>
> return Status;
> }
> @@ -904,6 +925,24 @@ EmmcSwitchToHS200 (
> return Status;
> }
>
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcSwitchClockFreqPost,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
> +
> Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth);
>
> return Status;
> @@ -988,6 +1027,27 @@ EmmcSwitchToHS400 (
>
> HsTiming = 3;
> Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcSwitchClockFreqPost,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
>
> return Status;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 850ad26..5408bbc 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -887,6 +887,24 @@ SdCardSetBusMode (
> return Status;
> }
>
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + Private->ControllerHandle,
> + Slot,
> + EdkiiSdMmcSwitchClockFreqPost,
> + &Timing
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> + }
> +
> if ((AccessMode == 3) || ((AccessMode == 2) && (Capability->TuningSDR50 != 0))) {
> Status = SdCardTuningClock (PciIo, PassThru, Slot);
> if (EFI_ERROR (Status)) {
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
2018-11-08 11:09 ` Ard Biesheuvel
@ 2018-11-08 11:18 ` Marcin Wojtas
0 siblings, 0 replies; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 11:18 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel-01, Leif Lindholm, hao.a.wu, Kinney, Michael D,
Gao, Liming, nadavh, jsd@semihalf.com, Tomasz Michalec,
Grzegorz Jaszczyk
czw., 8 lis 2018 o 12:09 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On 8 November 2018 at 02:57, Marcin Wojtas <mw@semihalf.com> wrote:
> > From: Tomasz Michalec <tm@semihalf.com>
> >
> > Some SD Host Controlers need to do additional opperations after clock
> > frequency switch.
> >
> > This patch add new callback type to NotifyPhase of the SdMmcOverride
> > protocol. It is called after EmmcSwitchClockFreq and SdMmcHcClockSupply.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas <mw@semihalf.com>
> > ---
> > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 1 +
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 60 ++++++++++++++++++++
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++
> > 3 files changed, 79 insertions(+)
> >
> > diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > index f948bef..6160b5b 100644
> > --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> > @@ -48,6 +48,7 @@ typedef enum {
> > EdkiiSdMmcInitHostPre,
> > EdkiiSdMmcInitHostPost,
> > EdkiiSdMmcUhsSignaling,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > } EDKII_SD_MMC_PHASE_TYPE;
> >
> > /**
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > index 473df8d..6fc6871 100755
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> > @@ -794,6 +794,27 @@ EmmcSwitchToHighSpeed (
> >
> > HsTiming = 1;
> > Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
>
> Is there a way we could move this into EmmcSwitchClockFreq() rather
> than duplicate it?
>
Yes, it will only require modifying EmmcSwitchClockFreq argument list,
but this is no cost.
> >
> > return Status;
> > }
> > @@ -904,6 +925,24 @@ EmmcSwitchToHS200 (
> > return Status;
> > }
> >
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> > +
> > Status = EmmcTuningClkForHs200 (PciIo, PassThru, Slot, BusWidth);
> >
> > return Status;
> > @@ -988,6 +1027,27 @@ EmmcSwitchToHS400 (
> >
> > HsTiming = 3;
> > Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > +
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> >
> > return Status;
> > }
> > diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > index 850ad26..5408bbc 100644
> > --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> > @@ -887,6 +887,24 @@ SdCardSetBusMode (
> > return Status;
> > }
> >
> > + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> > + Status = mOverride->NotifyPhase (
> > + Private->ControllerHandle,
> > + Slot,
> > + EdkiiSdMmcSwitchClockFreqPost,
> > + &Timing
> > + );
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((
> > + DEBUG_ERROR,
> > + "%a: SD/MMC switch clock freq post notifier callback failed - %r\n",
> > + __FUNCTION__,
> > + Status
> > + ));
> > + return Status;
> > + }
> > + }
> > +
> > if ((AccessMode == 3) || ((AccessMode == 2) && (Capability->TuningSDR50 != 0))) {
> > Status = SdCardTuningClock (PciIo, PassThru, Slot);
> > if (EFI_ERROR (Status)) {
> > --
> > 2.7.4
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
2018-11-08 1:57 [PATCH v3 0/4] SdMmcOverride extension Marcin Wojtas
` (2 preceding siblings ...)
2018-11-08 1:57 ` [PATCH v3 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
@ 2018-11-08 1:57 ` Marcin Wojtas
2018-11-08 11:10 ` Ard Biesheuvel
3 siblings, 1 reply; 10+ messages in thread
From: Marcin Wojtas @ 2018-11-08 1:57 UTC (permalink / raw)
To: edk2-devel
Cc: leif.lindholm, hao.a.wu, michael.d.kinney, liming.gao,
ard.biesheuvel, nadavh, mw, jsd, tm, jaz
Some SdMmc host controllers are run by clocks with different
frequency than it is reflected in Capabilities Register 1.
It is allowed by SDHCI specification ver. 4.2 - if BaseClkFreq
field value of the Capability Register 1 is zero, the clock
frequency must be obtained via another method.
Because the bitfield is only 8 bits wide, a maximum value
that could be obtained from hardware is 255MHz.
In case the actual frequency exceeds 255MHz, the 8-bit BaseClkFreq
member of SD_MMC_HC_SLOT_CAP structure occurs to be not sufficient
to be used for setting the clock speed in SdMmcHcClockSupply
function.
This patch adds new UINT32 array ('BaseClkFreq[]') to
SD_MMC_HC_PRIVATE_DATA structure for specifying
the input clock speed for each slot of the host controller.
All routines that are used for clock configuration are
updated accordingly.
This patch also adds new IN OUT BaseClockFreq field
in the Capability callback of the SdMmcOverride,
protocol which allows to update BaseClkFreq value.
The patch reuses original commit from edk2-platforms:
20f6f144d3a8 ("Marvell/Drivers: XenonDxe: Allow overriding base clock
frequency")
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +++++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 8 +++----
MdeModulePkg/Include/Protocol/SdMmcOverride.h | 7 ++++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 4 ++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 ++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 ++++++++++-
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 23 ++++++++++----------
7 files changed, 43 insertions(+), 22 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index c683600..8c1a589 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -118,6 +118,12 @@ typedef struct {
UINT64 MaxCurrent[SD_MMC_HC_MAX_SLOT];
UINT32 ControllerVersion;
+
+ //
+ // Some controllers may require to override base clock frequency
+ // value stored in Capabilities Register 1.
+ //
+ UINT32 BaseClkFreq[SD_MMC_HC_MAX_SLOT];
} SD_MMC_HC_PRIVATE_DATA;
#define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T')
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 1a11d51..8eefc31 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -423,7 +423,7 @@ SdMmcHcStopClock (
@param[in] PciIo The PCI IO protocol instance.
@param[in] Slot The slot number of the SD card to send the command to.
@param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
- @param[in] Capability The capability of the slot.
+ @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
@retval EFI_SUCCESS The clock is supplied successfully.
@retval Others The clock isn't supplied successfully.
@@ -434,7 +434,7 @@ SdMmcHcClockSupply (
IN EFI_PCI_IO_PROTOCOL *PciIo,
IN UINT8 Slot,
IN UINT64 ClockFreq,
- IN SD_MMC_HC_SLOT_CAP Capability
+ IN UINT32 BaseClkFreq
);
/**
@@ -482,7 +482,7 @@ SdMmcHcSetBusWidth (
@param[in] PciIo The PCI IO protocol instance.
@param[in] Slot The slot number of the SD card to send the command to.
- @param[in] Capability The capability of the slot.
+ @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
@retval EFI_SUCCESS The clock is supplied successfully.
@retval Others The clock isn't supplied successfully.
@@ -492,7 +492,7 @@ EFI_STATUS
SdMmcHcInitClockFreq (
IN EFI_PCI_IO_PROTOCOL *PciIo,
IN UINT8 Slot,
- IN SD_MMC_HC_SLOT_CAP Capability
+ IN UINT32 BaseClkFreq
);
/**
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 6160b5b..0aaf258 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -22,7 +22,7 @@
#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 0x1
+#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION 0x2
typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
@@ -58,6 +58,8 @@ typedef enum {
@param[in] ControllerHandle The EFI_HANDLE of the controller.
@param[in] Slot The 0 based slot index.
@param[in,out] SdMmcHcSlotCapability The SDHCI capability structure.
+ @param[in,out] BaseClkFreq The base clock frequency value that
+ optionally can be updated.
@retval EFI_SUCCESS The override function completed successfully.
@retval EFI_NOT_FOUND The specified controller or slot does not exist.
@@ -69,7 +71,8 @@ EFI_STATUS
(EFIAPI * EDKII_SD_MMC_CAPABILITY) (
IN EFI_HANDLE ControllerHandle,
IN UINT8 Slot,
- IN OUT VOID *SdMmcHcSlotCapability
+ IN OUT VOID *SdMmcHcSlotCapability,
+ IN OUT UINT32 *BaseClkFreq
);
/**
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 6fc6871..0393fd4 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -705,7 +705,7 @@ EmmcSwitchClockFreq (
//
// Convert the clock freq unit from MHz to KHz.
//
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
return Status;
}
@@ -1098,7 +1098,7 @@ EmmcSetBusMode (
return Status;
}
- ASSERT (Private->Capability[Slot].BaseClkFreq != 0);
+ ASSERT (Private->BaseClkFreq[Slot] != 0);
//
// Check if the Host Controller support 8bits bus width.
//
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 5408bbc..a9d0d3a 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -882,7 +882,7 @@ SdCardSetBusMode (
}
}
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1082,7 +1082,7 @@ SdCardIdentification (
goto Error;
}
- SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]);
+ SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
gBS->Stall (1000);
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index bf9869d..a87f8de 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -625,11 +625,16 @@ SdMmcPciHcDriverBindingStart (
if (EFI_ERROR (Status)) {
continue;
}
+
+ Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq;
+
if (mOverride != NULL && mOverride->Capability != NULL) {
Status = mOverride->Capability (
Controller,
Slot,
- &Private->Capability[Slot]);
+ &Private->Capability[Slot],
+ &Private->BaseClkFreq[Slot]
+ );
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
__FUNCTION__, Status));
@@ -637,6 +642,12 @@ SdMmcPciHcDriverBindingStart (
}
}
DumpCapabilityReg (Slot, &Private->Capability[Slot]);
+ DEBUG ((
+ DEBUG_INFO,
+ "Slot[%d] Base Clock Frequency: %dMHz\n",
+ Slot,
+ Private->BaseClkFreq[Slot]
+ ));
Support64BitDma &= Private->Capability[Slot].SysBus64;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 85aa625..040a959 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -721,7 +721,7 @@ SdMmcHcStopClock (
@param[in] PciIo The PCI IO protocol instance.
@param[in] Slot The slot number of the SD card to send the command to.
@param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
- @param[in] Capability The capability of the slot.
+ @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
@retval EFI_SUCCESS The clock is supplied successfully.
@retval Others The clock isn't supplied successfully.
@@ -732,11 +732,10 @@ SdMmcHcClockSupply (
IN EFI_PCI_IO_PROTOCOL *PciIo,
IN UINT8 Slot,
IN UINT64 ClockFreq,
- IN SD_MMC_HC_SLOT_CAP Capability
+ IN UINT32 BaseClkFreq
)
{
EFI_STATUS Status;
- UINT32 BaseClkFreq;
UINT32 SettingFreq;
UINT32 Divisor;
UINT32 Remainder;
@@ -746,9 +745,8 @@ SdMmcHcClockSupply (
//
// Calculate a divisor for SD clock frequency
//
- ASSERT (Capability.BaseClkFreq != 0);
+ ASSERT (BaseClkFreq != 0);
- BaseClkFreq = Capability.BaseClkFreq;
if (ClockFreq == 0) {
return EFI_INVALID_PARAMETER;
}
@@ -940,7 +938,7 @@ SdMmcHcSetBusWidth (
@param[in] PciIo The PCI IO protocol instance.
@param[in] Slot The slot number of the SD card to send the command to.
- @param[in] Capability The capability of the slot.
+ @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
@retval EFI_SUCCESS The clock is supplied successfully.
@retval Others The clock isn't supplied successfully.
@@ -950,16 +948,19 @@ EFI_STATUS
SdMmcHcInitClockFreq (
IN EFI_PCI_IO_PROTOCOL *PciIo,
IN UINT8 Slot,
- IN SD_MMC_HC_SLOT_CAP Capability
+ IN UINT32 BaseClkFreq
)
{
EFI_STATUS Status;
UINT32 InitFreq;
//
- // Calculate a divisor for SD clock frequency
+ // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
+ // the Capability Register 1 can be zero, which means a need for obtaining
+ // the clock frequency via another method. Fail in case it is not updated
+ // by SW at this point.
//
- if (Capability.BaseClkFreq == 0) {
+ if (BaseClkFreq == 0) {
//
// Don't support get Base Clock Frequency information via another method
//
@@ -969,7 +970,7 @@ SdMmcHcInitClockFreq (
// Supply 400KHz clock frequency at initialization phase.
//
InitFreq = 400;
- Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, Capability);
+ Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq);
return Status;
}
@@ -1103,7 +1104,7 @@ SdMmcHcInitHost (
PciIo = Private->PciIo;
Capability = Private->Capability[Slot];
- Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability);
+ Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
2018-11-08 1:57 ` [PATCH v3 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency Marcin Wojtas
@ 2018-11-08 11:10 ` Ard Biesheuvel
0 siblings, 0 replies; 10+ messages in thread
From: Ard Biesheuvel @ 2018-11-08 11:10 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel@lists.01.org, Leif Lindholm, Wu, Hao A,
Kinney, Michael D, Gao, Liming, Nadav Haklai,
Jan Dąbroś, Tomasz Michalec, Grzegorz Jaszczyk
On 8 November 2018 at 02:57, Marcin Wojtas <mw@semihalf.com> wrote:
> Some SdMmc host controllers are run by clocks with different
> frequency than it is reflected in Capabilities Register 1.
> It is allowed by SDHCI specification ver. 4.2 - if BaseClkFreq
> field value of the Capability Register 1 is zero, the clock
> frequency must be obtained via another method.
>
> Because the bitfield is only 8 bits wide, a maximum value
> that could be obtained from hardware is 255MHz.
> In case the actual frequency exceeds 255MHz, the 8-bit BaseClkFreq
> member of SD_MMC_HC_SLOT_CAP structure occurs to be not sufficient
> to be used for setting the clock speed in SdMmcHcClockSupply
> function.
>
> This patch adds new UINT32 array ('BaseClkFreq[]') to
> SD_MMC_HC_PRIVATE_DATA structure for specifying
> the input clock speed for each slot of the host controller.
> All routines that are used for clock configuration are
> updated accordingly.
>
> This patch also adds new IN OUT BaseClockFreq field
> in the Capability callback of the SdMmcOverride,
> protocol which allows to update BaseClkFreq value.
>
> The patch reuses original commit from edk2-platforms:
> 20f6f144d3a8 ("Marvell/Drivers: XenonDxe: Allow overriding base clock
> frequency")
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas <mw@semihalf.com>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +++++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 8 +++----
> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 7 ++++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 4 ++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 4 ++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 ++++++++++-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 23 ++++++++++----------
> 7 files changed, 43 insertions(+), 22 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> index c683600..8c1a589 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
> @@ -118,6 +118,12 @@ typedef struct {
> UINT64 MaxCurrent[SD_MMC_HC_MAX_SLOT];
>
> UINT32 ControllerVersion;
> +
> + //
> + // Some controllers may require to override base clock frequency
> + // value stored in Capabilities Register 1.
> + //
> + UINT32 BaseClkFreq[SD_MMC_HC_MAX_SLOT];
> } SD_MMC_HC_PRIVATE_DATA;
>
> #define SD_MMC_HC_TRB_SIG SIGNATURE_32 ('T', 'R', 'B', 'T')
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 1a11d51..8eefc31 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> @@ -423,7 +423,7 @@ SdMmcHcStopClock (
> @param[in] PciIo The PCI IO protocol instance.
> @param[in] Slot The slot number of the SD card to send the command to.
> @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> - @param[in] Capability The capability of the slot.
> + @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
>
> @retval EFI_SUCCESS The clock is supplied successfully.
> @retval Others The clock isn't supplied successfully.
> @@ -434,7 +434,7 @@ SdMmcHcClockSupply (
> IN EFI_PCI_IO_PROTOCOL *PciIo,
> IN UINT8 Slot,
> IN UINT64 ClockFreq,
> - IN SD_MMC_HC_SLOT_CAP Capability
> + IN UINT32 BaseClkFreq
> );
>
> /**
> @@ -482,7 +482,7 @@ SdMmcHcSetBusWidth (
>
> @param[in] PciIo The PCI IO protocol instance.
> @param[in] Slot The slot number of the SD card to send the command to.
> - @param[in] Capability The capability of the slot.
> + @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
>
> @retval EFI_SUCCESS The clock is supplied successfully.
> @retval Others The clock isn't supplied successfully.
> @@ -492,7 +492,7 @@ EFI_STATUS
> SdMmcHcInitClockFreq (
> IN EFI_PCI_IO_PROTOCOL *PciIo,
> IN UINT8 Slot,
> - IN SD_MMC_HC_SLOT_CAP Capability
> + IN UINT32 BaseClkFreq
> );
>
> /**
> diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> index 6160b5b..0aaf258 100644
> --- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> +++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
> @@ -22,7 +22,7 @@
> #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 0x1
> +#define EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION 0x2
>
> typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
>
> @@ -58,6 +58,8 @@ typedef enum {
> @param[in] ControllerHandle The EFI_HANDLE of the controller.
> @param[in] Slot The 0 based slot index.
> @param[in,out] SdMmcHcSlotCapability The SDHCI capability structure.
> + @param[in,out] BaseClkFreq The base clock frequency value that
> + optionally can be updated.
>
> @retval EFI_SUCCESS The override function completed successfully.
> @retval EFI_NOT_FOUND The specified controller or slot does not exist.
> @@ -69,7 +71,8 @@ EFI_STATUS
> (EFIAPI * EDKII_SD_MMC_CAPABILITY) (
> IN EFI_HANDLE ControllerHandle,
> IN UINT8 Slot,
> - IN OUT VOID *SdMmcHcSlotCapability
> + IN OUT VOID *SdMmcHcSlotCapability,
> + IN OUT UINT32 *BaseClkFreq
> );
>
> /**
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> index 6fc6871..0393fd4 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -705,7 +705,7 @@ EmmcSwitchClockFreq (
> //
> // Convert the clock freq unit from MHz to KHz.
> //
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
>
> return Status;
> }
> @@ -1098,7 +1098,7 @@ EmmcSetBusMode (
> return Status;
> }
>
> - ASSERT (Private->Capability[Slot].BaseClkFreq != 0);
> + ASSERT (Private->BaseClkFreq[Slot] != 0);
> //
> // Check if the Host Controller support 8bits bus width.
> //
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 5408bbc..a9d0d3a 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -882,7 +882,7 @@ SdCardSetBusMode (
> }
> }
>
> - Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
> + Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -1082,7 +1082,7 @@ SdCardIdentification (
> goto Error;
> }
>
> - SdMmcHcInitClockFreq (PciIo, Slot, Private->Capability[Slot]);
> + SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
>
> gBS->Stall (1000);
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index bf9869d..a87f8de 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -625,11 +625,16 @@ SdMmcPciHcDriverBindingStart (
> if (EFI_ERROR (Status)) {
> continue;
> }
> +
> + Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq;
> +
> if (mOverride != NULL && mOverride->Capability != NULL) {
> Status = mOverride->Capability (
> Controller,
> Slot,
> - &Private->Capability[Slot]);
> + &Private->Capability[Slot],
> + &Private->BaseClkFreq[Slot]
> + );
> if (EFI_ERROR (Status)) {
> DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
> __FUNCTION__, Status));
> @@ -637,6 +642,12 @@ SdMmcPciHcDriverBindingStart (
> }
> }
> DumpCapabilityReg (Slot, &Private->Capability[Slot]);
> + DEBUG ((
> + DEBUG_INFO,
> + "Slot[%d] Base Clock Frequency: %dMHz\n",
> + Slot,
> + Private->BaseClkFreq[Slot]
> + ));
>
> Support64BitDma &= Private->Capability[Slot].SysBus64;
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 85aa625..040a959 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -721,7 +721,7 @@ SdMmcHcStopClock (
> @param[in] PciIo The PCI IO protocol instance.
> @param[in] Slot The slot number of the SD card to send the command to.
> @param[in] ClockFreq The max clock frequency to be set. The unit is KHz.
> - @param[in] Capability The capability of the slot.
> + @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
>
> @retval EFI_SUCCESS The clock is supplied successfully.
> @retval Others The clock isn't supplied successfully.
> @@ -732,11 +732,10 @@ SdMmcHcClockSupply (
> IN EFI_PCI_IO_PROTOCOL *PciIo,
> IN UINT8 Slot,
> IN UINT64 ClockFreq,
> - IN SD_MMC_HC_SLOT_CAP Capability
> + IN UINT32 BaseClkFreq
> )
> {
> EFI_STATUS Status;
> - UINT32 BaseClkFreq;
> UINT32 SettingFreq;
> UINT32 Divisor;
> UINT32 Remainder;
> @@ -746,9 +745,8 @@ SdMmcHcClockSupply (
> //
> // Calculate a divisor for SD clock frequency
> //
> - ASSERT (Capability.BaseClkFreq != 0);
> + ASSERT (BaseClkFreq != 0);
>
> - BaseClkFreq = Capability.BaseClkFreq;
> if (ClockFreq == 0) {
> return EFI_INVALID_PARAMETER;
> }
> @@ -940,7 +938,7 @@ SdMmcHcSetBusWidth (
>
> @param[in] PciIo The PCI IO protocol instance.
> @param[in] Slot The slot number of the SD card to send the command to.
> - @param[in] Capability The capability of the slot.
> + @param[in] BaseClkFreq The base clock frequency of host controller in MHz.
>
> @retval EFI_SUCCESS The clock is supplied successfully.
> @retval Others The clock isn't supplied successfully.
> @@ -950,16 +948,19 @@ EFI_STATUS
> SdMmcHcInitClockFreq (
> IN EFI_PCI_IO_PROTOCOL *PciIo,
> IN UINT8 Slot,
> - IN SD_MMC_HC_SLOT_CAP Capability
> + IN UINT32 BaseClkFreq
> )
> {
> EFI_STATUS Status;
> UINT32 InitFreq;
>
> //
> - // Calculate a divisor for SD clock frequency
> + // According to SDHCI specification ver. 4.2, BaseClkFreq field value of
> + // the Capability Register 1 can be zero, which means a need for obtaining
> + // the clock frequency via another method. Fail in case it is not updated
> + // by SW at this point.
> //
> - if (Capability.BaseClkFreq == 0) {
> + if (BaseClkFreq == 0) {
> //
> // Don't support get Base Clock Frequency information via another method
> //
> @@ -969,7 +970,7 @@ SdMmcHcInitClockFreq (
> // Supply 400KHz clock frequency at initialization phase.
> //
> InitFreq = 400;
> - Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, Capability);
> + Status = SdMmcHcClockSupply (PciIo, Slot, InitFreq, BaseClkFreq);
> return Status;
> }
>
> @@ -1103,7 +1104,7 @@ SdMmcHcInitHost (
> PciIo = Private->PciIo;
> Capability = Private->Capability[Slot];
>
> - Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability);
> + Status = SdMmcHcInitClockFreq (PciIo, Slot, Private->BaseClkFreq[Slot]);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 10+ messages in thread