* [PATCH v4 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
2018-11-09 23:01 [PATCH v4 0/4] SdMmcOverride extension Marcin Wojtas
@ 2018-11-09 23:01 ` Marcin Wojtas
2018-11-09 23:01 ` [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-09 23:01 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] 12+ messages in thread* [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
2018-11-09 23:01 [PATCH v4 0/4] SdMmcOverride extension Marcin Wojtas
2018-11-09 23:01 ` [PATCH v4 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase Marcin Wojtas
@ 2018-11-09 23:01 ` Marcin Wojtas
2018-11-12 10:22 ` Ard Biesheuvel
2018-11-09 23:01 ` [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
` (2 subsequent siblings)
4 siblings, 1 reply; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-09 23:01 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 | 34 ++++++++
MdeModulePkg/Include/Protocol/SdMmcOverride.h | 17 ++++
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 86 ++++++++-----------
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 15 ++--
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 89 ++++++++++++++++++++
5 files changed, 181 insertions(+), 60 deletions(-)
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index 7e3f588..b47b0df 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,23 @@ SdMmcHcInitTimeoutCtrl (
IN UINT8 Slot
);
+/**
+ Set SD Host Controller control 2 registry according to selected speed.
+
+ @param[in] ControllerHandle The handle of the controller.
+ @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_HANDLE ControllerHandle,
+ 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..db4e8a1 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,25 +761,15 @@ 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 (Private->ControllerHandle, PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -814,10 +807,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 +833,14 @@ 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);
- 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);
+
+ Timing = SdMmcMmcHs200;
+
+ Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
+
//
// Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
//
@@ -910,9 +898,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,19 +924,10 @@ 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);
- 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);
+
+ Timing = SdMmcMmcHs400;
+
+ Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 6ee9ed7..55b3564 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,13 +859,7 @@ SdCardSetBusMode (
}
}
- HostCtrl2 = (UINT8)~0x7;
- Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- HostCtrl2 = AccessMode;
- Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
+ Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 923c55b..c3eec8b 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1138,6 +1138,95 @@ SdMmcHcInitHost (
}
/**
+ Set SD Host Controler control 2 registry according to selected speed.
+
+ @param[in] ControllerHandle The handle of the controller.
+ @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_HANDLE ControllerHandle,
+ 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);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+ Status = mOverride->NotifyPhase (
+ 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;
+ }
+ }
+
+ return EFI_SUCCESS;
+}
+
+/**
Turn on/off LED.
@param[in] PciIo The PCI IO protocol instance.
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
2018-11-09 23:01 ` [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
@ 2018-11-12 10:22 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-11-12 10:22 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 Sat, 10 Nov 2018 at 00:02, 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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 34 ++++++++
> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 17 ++++
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 86 ++++++++-----------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 15 ++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 89 ++++++++++++++++++++
> 5 files changed, 181 insertions(+), 60 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
> index 7e3f588..b47b0df 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,23 @@ SdMmcHcInitTimeoutCtrl (
> IN UINT8 Slot
> );
>
> +/**
> + Set SD Host Controller control 2 registry according to selected speed.
> +
> + @param[in] ControllerHandle The handle of the controller.
> + @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_HANDLE ControllerHandle,
> + 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..db4e8a1 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,25 +761,15 @@ 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 (Private->ControllerHandle, PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -814,10 +807,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 +833,14 @@ 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);
> - 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);
> +
> + Timing = SdMmcMmcHs200;
> +
> + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> +
> //
> // Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
> //
> @@ -910,9 +898,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,19 +924,10 @@ 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);
> - 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);
> +
> + Timing = SdMmcMmcHs400;
> +
> + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 6ee9ed7..55b3564 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,13 +859,7 @@ SdCardSetBusMode (
> }
> }
>
> - HostCtrl2 = (UINT8)~0x7;
> - Status = SdMmcHcAndMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> - if (EFI_ERROR (Status)) {
> - return Status;
> - }
> - HostCtrl2 = AccessMode;
> - Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
> + Status = SdMmcHcUhsSignaling (Private->ControllerHandle, PciIo, Slot, Timing);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 923c55b..c3eec8b 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -1138,6 +1138,95 @@ SdMmcHcInitHost (
> }
>
> /**
> + Set SD Host Controler control 2 registry according to selected speed.
> +
> + @param[in] ControllerHandle The handle of the controller.
> + @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_HANDLE ControllerHandle,
> + 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);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
> + Status = mOverride->NotifyPhase (
> + 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;
> + }
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> +/**
> Turn on/off LED.
>
> @param[in] PciIo The PCI IO protocol instance.
> --
> 2.7.4
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
2018-11-09 23:01 [PATCH v4 0/4] SdMmcOverride extension Marcin Wojtas
2018-11-09 23:01 ` [PATCH v4 1/4] MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase Marcin Wojtas
2018-11-09 23:01 ` [PATCH v4 2/4] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol Marcin Wojtas
@ 2018-11-09 23:01 ` Marcin Wojtas
2018-11-12 10:23 ` Ard Biesheuvel
2018-11-09 23:01 ` [PATCH v4 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency Marcin Wojtas
2018-11-13 7:38 ` [PATCH v4 0/4] SdMmcOverride extension Wu, Hao A
4 siblings, 1 reply; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-09 23:01 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 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 | 31 +++++++++++++++++---
MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++++++++
3 files changed, 46 insertions(+), 4 deletions(-)
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 db4e8a1..b75a9bb 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -651,6 +651,7 @@ EmmcSwitchBusWidth (
@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.
@retval EFI_SUCCESS The operation is done correctly.
@@ -664,6 +665,7 @@ EmmcSwitchClockFreq (
IN UINT8 Slot,
IN UINT16 Rca,
IN UINT8 HsTiming,
+ IN SD_MMC_BUS_MODE Timing,
IN UINT32 ClockFreq
)
{
@@ -706,6 +708,27 @@ EmmcSwitchClockFreq (
// Convert the clock freq unit from MHz to KHz.
//
Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
+ 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;
}
@@ -775,7 +798,7 @@ EmmcSwitchToHighSpeed (
}
HsTiming = 1;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+ Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
return Status;
}
@@ -863,7 +886,7 @@ EmmcSwitchToHS200 (
Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
HsTiming = 2;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+ Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -913,7 +936,7 @@ EmmcSwitchToHS400 (
// Set to Hight Speed timing and set the clock frequency to a value less than 52MHz.
//
HsTiming = 1;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 52);
+ Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -933,7 +956,7 @@ EmmcSwitchToHS400 (
}
HsTiming = 3;
- Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+ Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 55b3564..32fd416 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -869,6 +869,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] 12+ messages in thread* Re: [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
2018-11-09 23:01 ` [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
@ 2018-11-12 10:23 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2018-11-12 10:23 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 Sat, 10 Nov 2018 at 00:02, Marcin Wojtas <mw@semihalf.com> wrote:
>
> From: Tomasz Michalec <tm@semihalf.com>
>
> Some SD Host Controlers need to do additional opperations
operations
> after clock
> frequency switch.
>
> This patch add new callback type to NotifyPhase of the SdMmcOverride
> protocol. It is called after SdMmcHcClockSupply.
>
> 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 | 1 +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 31 +++++++++++++++++---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 18 ++++++++++++
> 3 files changed, 46 insertions(+), 4 deletions(-)
>
> 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 db4e8a1..b75a9bb 100755
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
> @@ -651,6 +651,7 @@ EmmcSwitchBusWidth (
> @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.
>
> @retval EFI_SUCCESS The operation is done correctly.
> @@ -664,6 +665,7 @@ EmmcSwitchClockFreq (
> IN UINT8 Slot,
> IN UINT16 Rca,
> IN UINT8 HsTiming,
> + IN SD_MMC_BUS_MODE Timing,
> IN UINT32 ClockFreq
> )
> {
> @@ -706,6 +708,27 @@ EmmcSwitchClockFreq (
> // Convert the clock freq unit from MHz to KHz.
> //
> Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->Capability[Slot]);
> + 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;
> }
> @@ -775,7 +798,7 @@ EmmcSwitchToHighSpeed (
> }
>
> HsTiming = 1;
> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> + Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
>
> return Status;
> }
> @@ -863,7 +886,7 @@ EmmcSwitchToHS200 (
> Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
>
> HsTiming = 2;
> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> + Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -913,7 +936,7 @@ EmmcSwitchToHS400 (
> // Set to Hight Speed timing and set the clock frequency to a value less than 52MHz.
> //
> HsTiming = 1;
> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, 52);
> + Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, SdMmcMmcHsSdr, 52);
> if (EFI_ERROR (Status)) {
> return Status;
> }
> @@ -933,7 +956,7 @@ EmmcSwitchToHS400 (
> }
>
> HsTiming = 3;
> - Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
> + Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, Timing, ClockFreq);
>
> return Status;
> }
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> index 55b3564..32fd416 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
> @@ -869,6 +869,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] 12+ messages in thread
* [PATCH v4 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
2018-11-09 23:01 [PATCH v4 0/4] SdMmcOverride extension Marcin Wojtas
` (2 preceding siblings ...)
2018-11-09 23:01 ` [PATCH v4 3/4] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride Marcin Wojtas
@ 2018-11-09 23:01 ` Marcin Wojtas
2018-11-13 7:38 ` [PATCH v4 0/4] SdMmcOverride extension Wu, Hao A
4 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-09 23:01 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>
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 b47b0df..dd45cbd 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 b75a9bb..2d3fb68 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -707,7 +707,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]);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1007,7 +1007,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 32fd416..68485c8 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -864,7 +864,7 @@ SdCardSetBusMode (
return Status;
}
- Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
+ Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1064,7 +1064,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 c3eec8b..ddf6dcf 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] 12+ messages in thread* Re: [PATCH v4 0/4] SdMmcOverride extension
2018-11-09 23:01 [PATCH v4 0/4] SdMmcOverride extension Marcin Wojtas
` (3 preceding siblings ...)
2018-11-09 23:01 ` [PATCH v4 4/4] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency Marcin Wojtas
@ 2018-11-13 7:38 ` Wu, Hao A
2018-11-13 8:25 ` Marcin Wojtas
4 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2018-11-13 7:38 UTC (permalink / raw)
To: Marcin Wojtas, edk2-devel@lists.01.org
Cc: leif.lindholm@linaro.org, Kinney, Michael D, Gao, Liming,
ard.biesheuvel@linaro.org, nadavh@marvell.com, jsd@semihalf.com,
tm@semihalf.com, jaz@semihalf.com
Hi Marcin,
The code changes look good to me.
Could you please grant me some time for some additional tests for these
patches?
I will inform you with the results sometime next week. Thanks in advance.
Best Regards,
Hao Wu
> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@semihalf.com]
> Sent: Saturday, November 10, 2018 7:01 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> Subject: [PATCH v4 0/4] SdMmcOverride extension
>
> Hi,
>
> Although I could've waited for Hao's remarks, I think it may
> be better if he takes a look at much cleaner code, which
> addresses v3 review comments.
> The newest version of the patchset cleans-up significantly
> patches 2&3 by removing code duplication and other minor
> improvements.
>
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/sdmmc-override-upstream-r20181109
>
> Please note that extending SdMmcOverride protocol was impacting
> so far the only user of it (Synquacer controller). In paralel
> edk2-platforms patchset, a patch can be found:
> ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> which adjust to the new API.
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/xenon-upstream-r20181109
>
> I'm looking forward to the comments and remarks.
>
> Best regards,
> Marcin
>
> Changelog:
> v3->v4
> * 2/4:
> - avoid duplication by calling SdMmcOverride callback in
> SdMmcHcUhsSignaling
>
> * 3/4:
> - avoid duplication by calling SdMmcOverride callback in
> EmmcSwitchClockFreq
>
> * 4/4:
> - add Ard's RB
>
> v2->v3
> * 1/4:
> - rename new parameter to PhaseData
> - add Ard's RB
>
> * 2/4:
> - s/Controler/Controller/
> - remove all references to MMC_SDR_50 mode
> - rename and reorder MMC bus modes
> - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
> and move it to protocol header in order to drop including private one
> - fix if condition in EmmcSwitchToHighSpeed
> - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
> callback, so that protocol producer can optionally modify only quirky
> timing mode values.
>
> *4/4
> - bump protocol version to 2
> - remove redundant assert from SdMmcPciHcDriverBindingStart
> (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
> - update comment in SdMmcHcInitClockFreq
> - restore original DumpCapabilityReg and append
>
> v1 -> v2
> * Rebase onto newest master
> * 1/4 [new patch] - preparation for extending NotifyPhase
> * 2/4 - UhsSignaling as a part of NotifyPhase
> * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> * 4/4 - Allow updating BaseClkFreq via Capability instead of the
> independent callback.
>
>
> Marcin Wojtas (2):
> MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
> MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
>
> Tomasz Michalec (2):
> MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride
> protocol
> MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> SdMmcOverride
>
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 42 ++++++-
> MdeModulePkg/Include/Protocol/SdMmcOverride.h | 29 ++++-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 121 ++++++++++-
> --------
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 35 ++++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 +-
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 124
> +++++++++++++++++---
> 7 files changed, 280 insertions(+), 90 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] SdMmcOverride extension
2018-11-13 7:38 ` [PATCH v4 0/4] SdMmcOverride extension Wu, Hao A
@ 2018-11-13 8:25 ` Marcin Wojtas
2018-11-13 8:29 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-13 8:25 UTC (permalink / raw)
To: hao.a.wu
Cc: edk2-devel-01, Leif Lindholm, Kinney, Michael D, Gao, Liming,
Ard Biesheuvel, nadavh, jsd@semihalf.com, Tomasz Michalec,
Grzegorz Jaszczyk
Hi Hao,
wt., 13 lis 2018 o 08:38 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
>
> Hi Marcin,
>
> The code changes look good to me.
>
> Could you please grant me some time for some additional tests for these
> patches?
Sure.
> I will inform you with the results sometime next week. Thanks in advance.
>
Ard gave his RB to 2/4 and 3/4. Moreover he pointed a typo in 3/4
commit message - should I repost, or could those be included when
applying the patches (unless you don't request any code change, of
course)?
Best regards,
Marcin
> Best Regards,
> Hao Wu
>
>
> > -----Original Message-----
> > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > Sent: Saturday, November 10, 2018 7:01 AM
> > To: edk2-devel@lists.01.org
> > Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> > ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> > jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> > Subject: [PATCH v4 0/4] SdMmcOverride extension
> >
> > Hi,
> >
> > Although I could've waited for Hao's remarks, I think it may
> > be better if he takes a look at much cleaner code, which
> > addresses v3 review comments.
> > The newest version of the patchset cleans-up significantly
> > patches 2&3 by removing code duplication and other minor
> > improvements.
> >
> > Patches are available in the github:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > platform/commits/sdmmc-override-upstream-r20181109
> >
> > Please note that extending SdMmcOverride protocol was impacting
> > so far the only user of it (Synquacer controller). In paralel
> > edk2-platforms patchset, a patch can be found:
> > ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> > which adjust to the new API.
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > platform/commits/xenon-upstream-r20181109
> >
> > I'm looking forward to the comments and remarks.
> >
> > Best regards,
> > Marcin
> >
> > Changelog:
> > v3->v4
> > * 2/4:
> > - avoid duplication by calling SdMmcOverride callback in
> > SdMmcHcUhsSignaling
> >
> > * 3/4:
> > - avoid duplication by calling SdMmcOverride callback in
> > EmmcSwitchClockFreq
> >
> > * 4/4:
> > - add Ard's RB
> >
> > v2->v3
> > * 1/4:
> > - rename new parameter to PhaseData
> > - add Ard's RB
> >
> > * 2/4:
> > - s/Controler/Controller/
> > - remove all references to MMC_SDR_50 mode
> > - rename and reorder MMC bus modes
> > - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
> > and move it to protocol header in order to drop including private one
> > - fix if condition in EmmcSwitchToHighSpeed
> > - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
> > callback, so that protocol producer can optionally modify only quirky
> > timing mode values.
> >
> > *4/4
> > - bump protocol version to 2
> > - remove redundant assert from SdMmcPciHcDriverBindingStart
> > (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
> > - update comment in SdMmcHcInitClockFreq
> > - restore original DumpCapabilityReg and append
> >
> > v1 -> v2
> > * Rebase onto newest master
> > * 1/4 [new patch] - preparation for extending NotifyPhase
> > * 2/4 - UhsSignaling as a part of NotifyPhase
> > * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> > * 4/4 - Allow updating BaseClkFreq via Capability instead of the
> > independent callback.
> >
> >
> > Marcin Wojtas (2):
> > MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in NotifyPhase
> > MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
> >
> > Tomasz Michalec (2):
> > MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride
> > protocol
> > MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> > SdMmcOverride
> >
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 42 ++++++-
> > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 29 ++++-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 121 ++++++++++-
> > --------
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 35 ++++--
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 +-
> > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 124
> > +++++++++++++++++---
> > 7 files changed, 280 insertions(+), 90 deletions(-)
> >
> > --
> > 2.7.4
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] SdMmcOverride extension
2018-11-13 8:25 ` Marcin Wojtas
@ 2018-11-13 8:29 ` Wu, Hao A
2018-11-20 5:58 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2018-11-13 8:29 UTC (permalink / raw)
To: Marcin Wojtas
Cc: edk2-devel-01, Leif Lindholm, Kinney, Michael D, Gao, Liming,
Ard Biesheuvel, nadavh@marvell.com, jsd@semihalf.com,
Tomasz Michalec, Grzegorz Jaszczyk
> -----Original Message-----
> From: Marcin Wojtas [mailto:mw@semihalf.com]
> Sent: Tuesday, November 13, 2018 4:25 PM
> To: Wu, Hao A
> Cc: edk2-devel-01; Leif Lindholm; Kinney, Michael D; Gao, Liming; Ard
> Biesheuvel; nadavh@marvell.com; jsd@semihalf.com; Tomasz Michalec;
> Grzegorz Jaszczyk
> Subject: Re: [PATCH v4 0/4] SdMmcOverride extension
>
> Hi Hao,
>
> wt., 13 lis 2018 o 08:38 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
> >
> > Hi Marcin,
> >
> > The code changes look good to me.
> >
> > Could you please grant me some time for some additional tests for these
> > patches?
>
> Sure.
>
> > I will inform you with the results sometime next week. Thanks in advance.
> >
>
> Ard gave his RB to 2/4 and 3/4. Moreover he pointed a typo in 3/4
> commit message - should I repost, or could those be included when
> applying the patches (unless you don't request any code change, of
> course)?
If no other issues, I will help to address the typo issue in patch 3/4.
A little concern on the commit author information if I modify the commit, but
I will let you know if a repost from you is needed.
Best Regards,
Hao Wu
>
> Best regards,
> Marcin
>
> > Best Regards,
> > Hao Wu
> >
> >
> > > -----Original Message-----
> > > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > > Sent: Saturday, November 10, 2018 7:01 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> > > ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> > > jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> > > Subject: [PATCH v4 0/4] SdMmcOverride extension
> > >
> > > Hi,
> > >
> > > Although I could've waited for Hao's remarks, I think it may
> > > be better if he takes a look at much cleaner code, which
> > > addresses v3 review comments.
> > > The newest version of the patchset cleans-up significantly
> > > patches 2&3 by removing code duplication and other minor
> > > improvements.
> > >
> > > Patches are available in the github:
> > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > platform/commits/sdmmc-override-upstream-r20181109
> > >
> > > Please note that extending SdMmcOverride protocol was impacting
> > > so far the only user of it (Synquacer controller). In paralel
> > > edk2-platforms patchset, a patch can be found:
> > > ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> > > which adjust to the new API.
> > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > platform/commits/xenon-upstream-r20181109
> > >
> > > I'm looking forward to the comments and remarks.
> > >
> > > Best regards,
> > > Marcin
> > >
> > > Changelog:
> > > v3->v4
> > > * 2/4:
> > > - avoid duplication by calling SdMmcOverride callback in
> > > SdMmcHcUhsSignaling
> > >
> > > * 3/4:
> > > - avoid duplication by calling SdMmcOverride callback in
> > > EmmcSwitchClockFreq
> > >
> > > * 4/4:
> > > - add Ard's RB
> > >
> > > v2->v3
> > > * 1/4:
> > > - rename new parameter to PhaseData
> > > - add Ard's RB
> > >
> > > * 2/4:
> > > - s/Controler/Controller/
> > > - remove all references to MMC_SDR_50 mode
> > > - rename and reorder MMC bus modes
> > > - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
> > > and move it to protocol header in order to drop including private one
> > > - fix if condition in EmmcSwitchToHighSpeed
> > > - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
> > > callback, so that protocol producer can optionally modify only quirky
> > > timing mode values.
> > >
> > > *4/4
> > > - bump protocol version to 2
> > > - remove redundant assert from SdMmcPciHcDriverBindingStart
> > > (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
> > > - update comment in SdMmcHcInitClockFreq
> > > - restore original DumpCapabilityReg and append
> > >
> > > v1 -> v2
> > > * Rebase onto newest master
> > > * 1/4 [new patch] - preparation for extending NotifyPhase
> > > * 2/4 - UhsSignaling as a part of NotifyPhase
> > > * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> > > * 4/4 - Allow updating BaseClkFreq via Capability instead of the
> > > independent callback.
> > >
> > >
> > > Marcin Wojtas (2):
> > > MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in
> NotifyPhase
> > > MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
> > >
> > > Tomasz Michalec (2):
> > > MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride
> > > protocol
> > > MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> > > SdMmcOverride
> > >
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 42 ++++++-
> > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 29 ++++-
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 121
> ++++++++++-
> > > --------
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 35 ++++--
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 +-
> > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 124
> > > +++++++++++++++++---
> > > 7 files changed, 280 insertions(+), 90 deletions(-)
> > >
> > > --
> > > 2.7.4
> >
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] SdMmcOverride extension
2018-11-13 8:29 ` Wu, Hao A
@ 2018-11-20 5:58 ` Wu, Hao A
2018-11-20 7:22 ` Marcin Wojtas
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2018-11-20 5:58 UTC (permalink / raw)
To: Wu, Hao A, Marcin Wojtas
Cc: Tomasz Michalec, Grzegorz Jaszczyk, edk2-devel-01, Gao, Liming,
nadavh@marvell.com, Kinney, Michael D
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Wu, Hao A
> Sent: Tuesday, November 13, 2018 4:29 PM
> To: Marcin Wojtas
> Cc: Tomasz Michalec; Grzegorz Jaszczyk; edk2-devel-01; Gao, Liming;
> nadavh@marvell.com; Kinney, Michael D
> Subject: Re: [edk2] [PATCH v4 0/4] SdMmcOverride extension
>
> > -----Original Message-----
> > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > Sent: Tuesday, November 13, 2018 4:25 PM
> > To: Wu, Hao A
> > Cc: edk2-devel-01; Leif Lindholm; Kinney, Michael D; Gao, Liming; Ard
> > Biesheuvel; nadavh@marvell.com; jsd@semihalf.com; Tomasz Michalec;
> > Grzegorz Jaszczyk
> > Subject: Re: [PATCH v4 0/4] SdMmcOverride extension
> >
> > Hi Hao,
> >
> > wt., 13 lis 2018 o 08:38 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
> > >
> > > Hi Marcin,
> > >
> > > The code changes look good to me.
> > >
> > > Could you please grant me some time for some additional tests for these
> > > patches?
> >
> > Sure.
> >
> > > I will inform you with the results sometime next week. Thanks in advance.
> > >
> >
> > Ard gave his RB to 2/4 and 3/4. Moreover he pointed a typo in 3/4
> > commit message - should I repost, or could those be included when
> > applying the patches (unless you don't request any code change, of
> > course)?
>
> If no other issues, I will help to address the typo issue in patch 3/4.
>
> A little concern on the commit author information if I modify the commit, but
> I will let you know if a repost from you is needed.
>
> Best Regards,
> Hao Wu
For the series:
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
Pushed at 49c9953425..7f3b0bad4b
Best Regards,
Hao Wu
>
> >
> > Best regards,
> > Marcin
> >
> > > Best Regards,
> > > Hao Wu
> > >
> > >
> > > > -----Original Message-----
> > > > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > > > Sent: Saturday, November 10, 2018 7:01 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> > > > ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> > > > jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> > > > Subject: [PATCH v4 0/4] SdMmcOverride extension
> > > >
> > > > Hi,
> > > >
> > > > Although I could've waited for Hao's remarks, I think it may
> > > > be better if he takes a look at much cleaner code, which
> > > > addresses v3 review comments.
> > > > The newest version of the patchset cleans-up significantly
> > > > patches 2&3 by removing code duplication and other minor
> > > > improvements.
> > > >
> > > > Patches are available in the github:
> > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > platform/commits/sdmmc-override-upstream-r20181109
> > > >
> > > > Please note that extending SdMmcOverride protocol was impacting
> > > > so far the only user of it (Synquacer controller). In paralel
> > > > edk2-platforms patchset, a patch can be found:
> > > > ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> > > > which adjust to the new API.
> > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > platform/commits/xenon-upstream-r20181109
> > > >
> > > > I'm looking forward to the comments and remarks.
> > > >
> > > > Best regards,
> > > > Marcin
> > > >
> > > > Changelog:
> > > > v3->v4
> > > > * 2/4:
> > > > - avoid duplication by calling SdMmcOverride callback in
> > > > SdMmcHcUhsSignaling
> > > >
> > > > * 3/4:
> > > > - avoid duplication by calling SdMmcOverride callback in
> > > > EmmcSwitchClockFreq
> > > >
> > > > * 4/4:
> > > > - add Ard's RB
> > > >
> > > > v2->v3
> > > > * 1/4:
> > > > - rename new parameter to PhaseData
> > > > - add Ard's RB
> > > >
> > > > * 2/4:
> > > > - s/Controler/Controller/
> > > > - remove all references to MMC_SDR_50 mode
> > > > - rename and reorder MMC bus modes
> > > > - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
> > > > and move it to protocol header in order to drop including private one
> > > > - fix if condition in EmmcSwitchToHighSpeed
> > > > - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
> > > > callback, so that protocol producer can optionally modify only quirky
> > > > timing mode values.
> > > >
> > > > *4/4
> > > > - bump protocol version to 2
> > > > - remove redundant assert from SdMmcPciHcDriverBindingStart
> > > > (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
> > > > - update comment in SdMmcHcInitClockFreq
> > > > - restore original DumpCapabilityReg and append
> > > >
> > > > v1 -> v2
> > > > * Rebase onto newest master
> > > > * 1/4 [new patch] - preparation for extending NotifyPhase
> > > > * 2/4 - UhsSignaling as a part of NotifyPhase
> > > > * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> > > > * 4/4 - Allow updating BaseClkFreq via Capability instead of the
> > > > independent callback.
> > > >
> > > >
> > > > Marcin Wojtas (2):
> > > > MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in
> > NotifyPhase
> > > > MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock
> frequency
> > > >
> > > > Tomasz Michalec (2):
> > > > MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to
> SdMmcOverride
> > > > protocol
> > > > MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> > > > SdMmcOverride
> > > >
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 42
> ++++++-
> > > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 29 ++++-
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 121
> > ++++++++++-
> > > > --------
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 35 ++++--
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 +-
> > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 124
> > > > +++++++++++++++++---
> > > > 7 files changed, 280 insertions(+), 90 deletions(-)
> > > >
> > > > --
> > > > 2.7.4
> > >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH v4 0/4] SdMmcOverride extension
2018-11-20 5:58 ` Wu, Hao A
@ 2018-11-20 7:22 ` Marcin Wojtas
0 siblings, 0 replies; 12+ messages in thread
From: Marcin Wojtas @ 2018-11-20 7:22 UTC (permalink / raw)
To: hao.a.wu
Cc: Tomasz Michalec, Grzegorz Jaszczyk, edk2-devel-01, Gao, Liming,
nadavh, Kinney, Michael D
wt., 20 lis 2018 o 06:59 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Wu, Hao A
> > Sent: Tuesday, November 13, 2018 4:29 PM
> > To: Marcin Wojtas
> > Cc: Tomasz Michalec; Grzegorz Jaszczyk; edk2-devel-01; Gao, Liming;
> > nadavh@marvell.com; Kinney, Michael D
> > Subject: Re: [edk2] [PATCH v4 0/4] SdMmcOverride extension
> >
> > > -----Original Message-----
> > > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > > Sent: Tuesday, November 13, 2018 4:25 PM
> > > To: Wu, Hao A
> > > Cc: edk2-devel-01; Leif Lindholm; Kinney, Michael D; Gao, Liming; Ard
> > > Biesheuvel; nadavh@marvell.com; jsd@semihalf.com; Tomasz Michalec;
> > > Grzegorz Jaszczyk
> > > Subject: Re: [PATCH v4 0/4] SdMmcOverride extension
> > >
> > > Hi Hao,
> > >
> > > wt., 13 lis 2018 o 08:38 Wu, Hao A <hao.a.wu@intel.com> napisał(a):
> > > >
> > > > Hi Marcin,
> > > >
> > > > The code changes look good to me.
> > > >
> > > > Could you please grant me some time for some additional tests for these
> > > > patches?
> > >
> > > Sure.
> > >
> > > > I will inform you with the results sometime next week. Thanks in advance.
> > > >
> > >
> > > Ard gave his RB to 2/4 and 3/4. Moreover he pointed a typo in 3/4
> > > commit message - should I repost, or could those be included when
> > > applying the patches (unless you don't request any code change, of
> > > course)?
> >
> > If no other issues, I will help to address the typo issue in patch 3/4.
> >
> > A little concern on the commit author information if I modify the commit, but
> > I will let you know if a repost from you is needed.
> >
> > Best Regards,
> > Hao Wu
>
> For the series:
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>
> Pushed at 49c9953425..7f3b0bad4b
>
Thanks a lot!
Marcin
>
> Best Regards,
> Hao Wu
>
> >
> > >
> > > Best regards,
> > > Marcin
> > >
> > > > Best Regards,
> > > > Hao Wu
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Marcin Wojtas [mailto:mw@semihalf.com]
> > > > > Sent: Saturday, November 10, 2018 7:01 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: leif.lindholm@linaro.org; Wu, Hao A; Kinney, Michael D; Gao, Liming;
> > > > > ard.biesheuvel@linaro.org; nadavh@marvell.com; mw@semihalf.com;
> > > > > jsd@semihalf.com; tm@semihalf.com; jaz@semihalf.com
> > > > > Subject: [PATCH v4 0/4] SdMmcOverride extension
> > > > >
> > > > > Hi,
> > > > >
> > > > > Although I could've waited for Hao's remarks, I think it may
> > > > > be better if he takes a look at much cleaner code, which
> > > > > addresses v3 review comments.
> > > > > The newest version of the patchset cleans-up significantly
> > > > > patches 2&3 by removing code duplication and other minor
> > > > > improvements.
> > > > >
> > > > > Patches are available in the github:
> > > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > > platform/commits/sdmmc-override-upstream-r20181109
> > > > >
> > > > > Please note that extending SdMmcOverride protocol was impacting
> > > > > so far the only user of it (Synquacer controller). In paralel
> > > > > edk2-platforms patchset, a patch can be found:
> > > > > ("Silicon/SynQuacer/PlatformDxe: adjust to updated SdMmcOverride")
> > > > > which adjust to the new API.
> > > > > https://github.com/MarvellEmbeddedProcessors/edk2-open-
> > > > > platform/commits/xenon-upstream-r20181109
> > > > >
> > > > > I'm looking forward to the comments and remarks.
> > > > >
> > > > > Best regards,
> > > > > Marcin
> > > > >
> > > > > Changelog:
> > > > > v3->v4
> > > > > * 2/4:
> > > > > - avoid duplication by calling SdMmcOverride callback in
> > > > > SdMmcHcUhsSignaling
> > > > >
> > > > > * 3/4:
> > > > > - avoid duplication by calling SdMmcOverride callback in
> > > > > EmmcSwitchClockFreq
> > > > >
> > > > > * 4/4:
> > > > > - add Ard's RB
> > > > >
> > > > > v2->v3
> > > > > * 1/4:
> > > > > - rename new parameter to PhaseData
> > > > > - add Ard's RB
> > > > >
> > > > > * 2/4:
> > > > > - s/Controler/Controller/
> > > > > - remove all references to MMC_SDR_50 mode
> > > > > - rename and reorder MMC bus modes
> > > > > - rename enum: s/SD_MMC_UHS_TIMING/SD_MMC_BUS_MODE/
> > > > > and move it to protocol header in order to drop including private one
> > > > > - fix if condition in EmmcSwitchToHighSpeed
> > > > > - call SdMmcHcUhsSignaling unconditionally before SdMmcOverride
> > > > > callback, so that protocol producer can optionally modify only quirky
> > > > > timing mode values.
> > > > >
> > > > > *4/4
> > > > > - bump protocol version to 2
> > > > > - remove redundant assert from SdMmcPciHcDriverBindingStart
> > > > > (BaseClkFreq is already checked in SdMmcHcInitClockFreq)
> > > > > - update comment in SdMmcHcInitClockFreq
> > > > > - restore original DumpCapabilityReg and append
> > > > >
> > > > > v1 -> v2
> > > > > * Rebase onto newest master
> > > > > * 1/4 [new patch] - preparation for extending NotifyPhase
> > > > > * 2/4 - UhsSignaling as a part of NotifyPhase
> > > > > * 3/4 - SwitchClockFreqPost as a part of NotifyPhase
> > > > > * 4/4 - Allow updating BaseClkFreq via Capability instead of the
> > > > > independent callback.
> > > > >
> > > > >
> > > > > Marcin Wojtas (2):
> > > > > MdeModulePkg/SdMmcPciHcDxe: Add an optional parameter in
> > > NotifyPhase
> > > > > MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock
> > frequency
> > > > >
> > > > > Tomasz Michalec (2):
> > > > > MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to
> > SdMmcOverride
> > > > > protocol
> > > > > MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to
> > > > > SdMmcOverride
> > > > >
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h | 6 +
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h | 42
> > ++++++-
> > > > > MdeModulePkg/Include/Protocol/SdMmcOverride.h | 29 ++++-
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 121
> > > ++++++++++-
> > > > > --------
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c | 35 ++++--
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 13 +-
> > > > > MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 124
> > > > > +++++++++++++++++---
> > > > > 7 files changed, 280 insertions(+), 90 deletions(-)
> > > > >
> > > > > --
> > > > > 2.7.4
> > > >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 12+ messages in thread