[PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Marcin Wojtas]

Hi,

This patchset extends SdMmcOverride protocol with new callbacks:
* UhsSignaling - allow writing custom values to HostControl2 register
* SwitchClockFreqPost - perform additional opperations after clock switch
* BaseClockFreq - allow overriding base clock frequency
Also a couple of fixes for MMC, card detection and reset are submitted.
More details can be found in the commit messages.

Patches are available in the github:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20180902

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: Modify initialization of SdMmcOverride")
which immunizes for above and future extensions of the protocol:
https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20180902

I'm looking forward to the comments and remarks.

Best regards,
Marcin

Marcin Wojtas (3):
  MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
  MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
  MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot

Tomasz Michalec (4):
  MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
  MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
  MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
  MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +++++-
 MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  83 ++++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 246 +++++++++++++-------
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  55 ++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++++++--
 7 files changed, 467 insertions(+), 129 deletions(-)

-- 
2.7.4

[PATCH 1/7] MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation

[Marcin Wojtas]

When switching to any of high speed modes (HS, HS200, HS400)
there is need to set HS_ENABLE bit in Host Control 1 register
which allow Host Controller to output CMD and DAT lines on
both edges of clock. In Linux it is done after switching bus
width in sdhci_set_ios().

Also according to JESD84-B50-1 chapter 6.6.4 "HS200 timing mode
selection" (documentation about eMMC4.5 standard) there is
no need to disable clock when switching to HS200.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h |  5 ++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c  | 30 +++-----------------
 2 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index e389d52..e3fadb5 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -63,6 +63,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define SD_MMC_HC_CTRL_VER            0xFE
 
 //
+// SD Host Control 1 Register bits description
+//
+#define SD_MMC_HC_HOST_CTRL1_HS_ENABLE  (1 << 2)
+
+//
 // The transfer modes supported by SD Host Controller
 // Simplified Spec 3.0 Table 1-2
 //
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index c5fd214..b3903b4 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -816,8 +816,8 @@ EmmcSwitchToHS200 (
 {
   EFI_STATUS          Status;
   UINT8               HsTiming;
+  UINT8               HostCtrl1;
   UINT8               HostCtrl2;
-  UINT16              ClockCtrl;
 
   if ((BusWidth != 4) && (BusWidth != 8)) {
     return EFI_INVALID_PARAMETER;
@@ -828,12 +828,10 @@ EmmcSwitchToHS200 (
     return Status;
   }
   //
-  // Set to HS200/SDR104 timing
-  //
-  //
-  // Stop bus clock at first
+  // Set to High Speed timing
   //
-  Status = SdMmcHcStopClock (PciIo, Slot);
+  HostCtrl1 = SD_MMC_HC_HOST_CTRL1_HS_ENABLE;
+  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL1, sizeof (HostCtrl1), &HostCtrl1);
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -853,26 +851,6 @@ EmmcSwitchToHS200 (
   if (EFI_ERROR (Status)) {
     return Status;
   }
-  //
-  // Wait Internal Clock Stable in the Clock Control register to be 1 before set SD Clock Enable bit
-  //
-  Status = SdMmcHcWaitMmioSet (
-             PciIo,
-             Slot,
-             SD_MMC_HC_CLOCK_CTRL,
-             sizeof (ClockCtrl),
-             BIT1,
-             BIT1,
-             SD_MMC_HC_GENERIC_TIMEOUT
-             );
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
-  //
-  // Set SD Clock Enable in the Clock Control register to 1
-  //
-  ClockCtrl = BIT2;
-  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_CLOCK_CTRL, sizeof (ClockCtrl), &ClockCtrl);
 
   HsTiming = 2;
   Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
-- 
2.7.4

[PATCH 2/7] MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol

[Marcin Wojtas]

From: Tomasz Michalec <tm@semihalf.com>

Some SD Host Controlers use different values in Host Control 2 Register
to select UHS Mode. This patch adds new UhsSignaling callback to
the SdMmcOverride protocol. UHS signaling configuration is moved
to a common, default routine (SdMmcHcUhsSignaling),
which is called when SdMmcOverride does not cover this functionality.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h |  50 +++++++
 MdeModulePkg/Include/Protocol/SdMmcOverride.h    |  26 ++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c  | 150 ++++++++++++--------
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c    |  36 +++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c |  69 +++++++++
 5 files changed, 263 insertions(+), 68 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index e3fadb5..525828c 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -68,6 +68,39 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #define SD_MMC_HC_HOST_CTRL1_HS_ENABLE  (1 << 2)
 
 //
+// SD Host Controler 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_DDR52      0x0004
+#define SD_MMC_HC_CTRL_MMC_SDR50      0x0002
+#define SD_MMC_HC_CTRL_MMC_SDR25      0x0001
+#define SD_MMC_HC_CTRL_MMC_SDR12      0x0000
+#define SD_MMC_HC_CTRL_HS200          0x0003
+#define SD_MMC_HC_CTRL_HS400          0x0005
+
+//
+// Timing modes for uhs
+//
+typedef enum {
+  SdMmcUhsSdr12,
+  SdMmcUhsSdr25,
+  SdMmcUhsSdr50,
+  SdMmcUhsSdr104,
+  SdMmcUhsDdr50,
+  SdMmcMmcDdr52,
+  SdMmcMmcSdr50,
+  SdMmcMmcSdr25,
+  SdMmcMmcSdr12,
+  SdMmcMmcHs200,
+  SdMmcMmcHs400,
+} SD_MMC_UHS_TIMING;
+
+//
 // The transfer modes supported by SD Host Controller
 // Simplified Spec 3.0 Table 1-2
 //
@@ -513,4 +546,21 @@ SdMmcHcInitTimeoutCtrl (
   IN UINT8                  Slot
   );
 
+/**
+  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_UHS_TIMING      Timing
+  );
+
 #endif
diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index 0766252..a7a57e8 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -17,6 +17,7 @@
 #ifndef __SD_MMC_OVERRIDE_H__
 #define __SD_MMC_OVERRIDE_H__
 
+#include <Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h>
 #include <Protocol/SdMmcPassThru.h>
 
 #define EDKII_SD_MMC_OVERRIDE_PROTOCOL_GUID \
@@ -77,6 +78,27 @@ EFI_STATUS
   IN      EDKII_SD_MMC_PHASE_TYPE         PhaseType
   );
 
+/**
+
+  Override function for uhs signaling
+
+  @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
+  @param[in]      Slot                  The 0 based slot index.
+  @param[in]      Timing                The timing which should be set by
+                                        host controller.
+
+  @retval EFI_SUCCESS           The override function completed successfully.
+  @retval EFI_NOT_FOUND         The specified controller or slot does not exist.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EDKII_SD_MMC_UHS_SIGNALING) (
+  IN      EFI_HANDLE                      ControllerHandle,
+  IN      UINT8                           Slot,
+  IN      SD_MMC_UHS_TIMING               Timing
+  );
+
 struct _EDKII_SD_MMC_OVERRIDE {
   //
   // Protocol version of this implementation
@@ -90,6 +112,10 @@ struct _EDKII_SD_MMC_OVERRIDE {
   // Callback to invoke SD/MMC override hooks
   //
   EDKII_SD_MMC_NOTIFY_PHASE     NotifyPhase;
+  //
+  // Callback to override SD/MMC host controller uhs signaling
+  //
+  EDKII_SD_MMC_UHS_SIGNALING    UhsSignaling;
 };
 
 extern EFI_GUID gEdkiiSdMmcOverrideProtocolGuid;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index b3903b4..d285249 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_UHS_TIMING       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,27 +761,36 @@ 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 = SdMmcMmcDdr52;
   } else if (ClockFreq == 52) {
-    HostCtrl2 = BIT0;
+    Timing = SdMmcMmcSdr50;
+  } else if (ClockFreq == 26) {
+    Timing = SdMmcMmcSdr25;
   } else {
-    HostCtrl2 = 0;
+    Timing = SdMmcMmcSdr12;
   }
-  Status = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_HOST_CTRL2, sizeof (HostCtrl2), &HostCtrl2);
-  if (EFI_ERROR (Status)) {
-    return Status;
+
+  if (mOverride != NULL && mOverride->UhsSignaling != NULL) {
+    Status = mOverride->UhsSignaling (
+                          Private->ControllerHandle,
+                          Slot,
+                          Timing
+                          );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      return Status;
+    }
+  } else {
+    Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
   }
 
   HsTiming = 1;
@@ -814,10 +826,13 @@ EmmcSwitchToHS200 (
   IN UINT8                              BusWidth
   )
 {
-  EFI_STATUS          Status;
-  UINT8               HsTiming;
-  UINT8               HostCtrl1;
-  UINT8               HostCtrl2;
+  EFI_STATUS                 Status;
+  UINT8                      HsTiming;
+  UINT8                      HostCtrl1;
+  SD_MMC_UHS_TIMING          Timing;
+  SD_MMC_HC_PRIVATE_DATA     *Private;
+
+  Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
 
   if ((BusWidth != 4) && (BusWidth != 8)) {
     return EFI_INVALID_PARAMETER;
@@ -835,21 +850,29 @@ 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);
-  if (EFI_ERROR (Status)) {
-    return Status;
+
+  Timing = SdMmcMmcHs200;
+
+  if (mOverride != NULL && mOverride->UhsSignaling != NULL) {
+    Status = mOverride->UhsSignaling (
+                          Private->ControllerHandle,
+                          Slot,
+                          Timing
+                          );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      return Status;
+    }
+  } else {
+    Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
   }
 
   HsTiming = 2;
@@ -888,9 +911,12 @@ EmmcSwitchToHS400 (
   IN UINT32                             ClockFreq
   )
 {
-  EFI_STATUS          Status;
-  UINT8               HsTiming;
-  UINT8               HostCtrl2;
+  EFI_STATUS                 Status;
+  UINT8                      HsTiming;
+  SD_MMC_UHS_TIMING          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)) {
@@ -911,21 +937,29 @@ 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);
-  if (EFI_ERROR (Status)) {
-    return Status;
+
+  Timing = SdMmcMmcHs400;
+
+  if (mOverride != NULL && mOverride->UhsSignaling != NULL) {
+    Status = mOverride->UhsSignaling (
+                          Private->ControllerHandle,
+                          Slot,
+                          Timing
+                          );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      return Status;
+    }
+  } else {
+    Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
+    if (EFI_ERROR (Status)) {
+      return Status;
+    }
   }
 
   HsTiming = 3;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
index 8c93933..f45a367 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_UHS_TIMING            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,26 @@ 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);
-  if (EFI_ERROR (Status)) {
-    return Status;
+  if (mOverride != NULL && mOverride->UhsSignaling != NULL) {
+    Status = mOverride->UhsSignaling (
+                          Private->ControllerHandle,
+                          Slot,
+                          Timing
+                          );
+    if (EFI_ERROR (Status)) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: SD/MMC uhs signaling notifier callback failed - %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      return Status;
+    }
+  } else {
+    Status = SdMmcHcUhsSignaling (PciIo, Slot, Timing);
+    if (EFI_ERROR (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 9672b5b..95627da 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -1133,6 +1133,75 @@ 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_UHS_TIMING      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 SdMmcMmcDdr52:
+      HostCtrl2 = SD_MMC_HC_CTRL_MMC_DDR52;
+      break;
+    case SdMmcMmcSdr50:
+      HostCtrl2 = SD_MMC_HC_CTRL_MMC_SDR50;
+      break;
+    case SdMmcMmcSdr25:
+      HostCtrl2 = SD_MMC_HC_CTRL_MMC_SDR25;
+      break;
+    case SdMmcMmcSdr12:
+      HostCtrl2 = SD_MMC_HC_CTRL_MMC_SDR12;
+      break;
+    case SdMmcMmcHs200:
+      HostCtrl2 = SD_MMC_HC_CTRL_HS200;
+      break;
+    case SdMmcMmcHs400:
+      HostCtrl2 = SD_MMC_HC_CTRL_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

[PATCH 3/7] MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride

[Marcin Wojtas]

From: Tomasz Michalec <tm@semihalf.com>

Some SD Host Controlers need to do additional opperations after clock
frequency switch.

This patch add new callback to SdMmcOverride protocol.
The SwitchClockFreqPost callback 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   | 33 ++++++++++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 57 ++++++++++++++++++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c   | 17 ++++++
 3 files changed, 103 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
index a7a57e8..6ba1c43 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -99,23 +99,48 @@ EFI_STATUS
   IN      SD_MMC_UHS_TIMING               Timing
   );
 
+/**
+
+  Additional operations specific for host controller
+
+  @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
+  @param[in]      Slot                  The 0 based slot index.
+  @param[in]      Timing                The timing which should be set by
+                                        host controller.
+
+  @retval EFI_SUCCESS           The override function completed successfully.
+  @retval EFI_NOT_FOUND         The specified controller or slot does not exist.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EDKII_SD_MMC_POST_CLOCK_FREQ_SWITCH) (
+  IN      EFI_HANDLE                      ControllerHandle,
+  IN      UINT8                           Slot,
+  IN      SD_MMC_UHS_TIMING               Timing
+  );
+
 struct _EDKII_SD_MMC_OVERRIDE {
   //
   // Protocol version of this implementation
   //
-  UINTN                         Version;
+  UINTN                                Version;
   //
   // Callback to override SD/MMC host controller capability bits
   //
-  EDKII_SD_MMC_CAPABILITY       Capability;
+  EDKII_SD_MMC_CAPABILITY              Capability;
   //
   // Callback to invoke SD/MMC override hooks
   //
-  EDKII_SD_MMC_NOTIFY_PHASE     NotifyPhase;
+  EDKII_SD_MMC_NOTIFY_PHASE            NotifyPhase;
   //
   // Callback to override SD/MMC host controller uhs signaling
   //
-  EDKII_SD_MMC_UHS_SIGNALING    UhsSignaling;
+  EDKII_SD_MMC_UHS_SIGNALING           UhsSignaling;
+  //
+  // Callback to add host controller specific operations after SwitchClockFreq
+  //
+  EDKII_SD_MMC_POST_CLOCK_FREQ_SWITCH  SwitchClockFreqPost;
 };
 
 extern EFI_GUID gEdkiiSdMmcOverrideProtocolGuid;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index d285249..78ee3a5 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -795,6 +795,26 @@ EmmcSwitchToHighSpeed (
 
   HsTiming = 1;
   Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (mOverride != NULL && mOverride->SwitchClockFreqPost != NULL) {
+    Status = mOverride->SwitchClockFreqPost (
+                          Private->ControllerHandle,
+                          Slot,
+                          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;
 }
@@ -881,6 +901,23 @@ EmmcSwitchToHS200 (
     return Status;
   }
 
+  if (mOverride != NULL && mOverride->SwitchClockFreqPost != NULL) {
+    Status = mOverride->SwitchClockFreqPost (
+                          Private->ControllerHandle,
+                          Slot,
+                          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;
@@ -964,6 +1001,26 @@ EmmcSwitchToHS400 (
 
   HsTiming = 3;
   Status = EmmcSwitchClockFreq (PciIo, PassThru, Slot, Rca, HsTiming, ClockFreq);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (mOverride != NULL && mOverride->SwitchClockFreqPost != NULL) {
+    Status = mOverride->SwitchClockFreqPost (
+                          Private->ControllerHandle,
+                          Slot,
+                          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 f45a367..777cf08 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -886,6 +886,23 @@ SdCardSetBusMode (
     return Status;
   }
 
+  if (mOverride != NULL && mOverride->SwitchClockFreqPost != NULL) {
+    Status = mOverride->SwitchClockFreqPost (
+                          Private->ControllerHandle,
+                          Slot,
+                          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

[PATCH 4/7] MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency

[Marcin Wojtas]

From: Tomasz Michalec <tm@semihalf.com>

Some SdMmc host controllers are run by clocks with different
frequency than it is reflected in Capabilities Register 1.
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 BaseClockFreq callback to SdMmcOverride
protocol which allows to change new bigger BaseClkFreq
field.

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   | 12 +++++----
 MdeModulePkg/Include/Protocol/SdMmcOverride.h      | 26 +++++++++++++++++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    |  4 +--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  4 +--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 19 +++++++++++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 27 +++++++++++---------
 7 files changed, 76 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 525828c..01ff9f2 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -178,12 +178,14 @@ typedef struct {
 
   @param[in]  Slot            The slot number of the SD card to send the command to.
   @param[in]  Capability      The buffer to store the capability data.
+  @param[in]  BaseClkFreq     The base clock frequency of host controller in MHz.
 
 **/
 VOID
 DumpCapabilityReg (
   IN UINT8                Slot,
-  IN SD_MMC_HC_SLOT_CAP   *Capability
+  IN SD_MMC_HC_SLOT_CAP   *Capability,
+  IN UINT32               BaseClkFreq
   );
 
 /**
@@ -436,7 +438,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.
@@ -447,7 +449,7 @@ SdMmcHcClockSupply (
   IN EFI_PCI_IO_PROTOCOL    *PciIo,
   IN UINT8                  Slot,
   IN UINT64                 ClockFreq,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN UINT32                 BaseClkFreq
   );
 
 /**
@@ -495,7 +497,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.
@@ -505,7 +507,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 6ba1c43..941c04d 100644
--- a/MdeModulePkg/Include/Protocol/SdMmcOverride.h
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -120,6 +120,28 @@ EFI_STATUS
   IN      SD_MMC_UHS_TIMING               Timing
   );
 
+/**
+
+  Callback that allow to override base clock frequency with value
+  higher then 255MHz
+
+  @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
+  @param[in]      Slot                  The 0 based slot index.
+  @param[in,out]  BaseClkFreq           The base clock frequency that can be
+                                        overriden with greater value then 255.
+
+  @retval EFI_SUCCESS           The override function completed successfully.
+  @retval EFI_NOT_FOUND         The specified controller or slot does not exist.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EDKII_SD_MMC_BASE_CLOCK_FREQ) (
+  IN      EFI_HANDLE                      ControllerHandle,
+  IN      UINT8                           Slot,
+  IN  OUT UINT32                          *BaseClkFreq
+  );
+
 struct _EDKII_SD_MMC_OVERRIDE {
   //
   // Protocol version of this implementation
@@ -141,6 +163,10 @@ struct _EDKII_SD_MMC_OVERRIDE {
   // Callback to add host controller specific operations after SwitchClockFreq
   //
   EDKII_SD_MMC_POST_CLOCK_FREQ_SWITCH  SwitchClockFreqPost;
+  //
+  // Callback that allow to override base clock frequency
+  //
+  EDKII_SD_MMC_BASE_CLOCK_FREQ         BaseClockFreq;
 };
 
 extern EFI_GUID gEdkiiSdMmcOverrideProtocolGuid;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index 78ee3a5..e5e0c89 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;
 }
@@ -1071,7 +1071,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 777cf08..23ab5b0 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c
@@ -881,7 +881,7 @@ SdCardSetBusMode (
     }
   }
 
-  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, *Capability);
+  Status = SdMmcHcClockSupply (PciIo, Slot, ClockFreq * 1000, Private->BaseClkFreq[Slot]);
   if (EFI_ERROR (Status)) {
     return Status;
   }
@@ -1079,7 +1079,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 f923930..50c1e74 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -636,7 +636,24 @@ SdMmcPciHcDriverBindingStart (
         continue;
       }
     }
-    DumpCapabilityReg (Slot, &Private->Capability[Slot]);
+    Private->BaseClkFreq[Slot] = Private->Capability[Slot].BaseClkFreq;
+    if (mOverride != NULL && mOverride->BaseClockFreq != NULL) {
+      Status = mOverride->BaseClockFreq (
+                            Controller,
+                            Slot,
+                            &Private->BaseClkFreq[Slot]
+                            );
+      if (EFI_ERROR (Status)) {
+        DEBUG ((
+          DEBUG_ERROR,
+          "%a: Failed to override capability - %r\n",
+          __FUNCTION__,
+          Status
+          ));
+        continue;
+      }
+    }
+    DumpCapabilityReg (Slot, &Private->Capability[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 95627da..e7c51e6 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -22,12 +22,14 @@
 
   @param[in]  Slot            The slot number of the SD card to send the command to.
   @param[in]  Capability      The buffer to store the capability data.
+  @param[in]  BaseClkFreq     The base clock frequency of host controller in MHz.
 
 **/
 VOID
 DumpCapabilityReg (
   IN UINT8                Slot,
-  IN SD_MMC_HC_SLOT_CAP   *Capability
+  IN SD_MMC_HC_SLOT_CAP   *Capability,
+  IN UINT32               BaseClkFreq
   )
 {
   //
@@ -35,7 +37,10 @@ DumpCapabilityReg (
   //
   DEBUG ((DEBUG_INFO, " == Slot [%d] Capability is 0x%x ==\n", Slot, Capability));
   DEBUG ((DEBUG_INFO, "   Timeout Clk Freq  %d%a\n", Capability->TimeoutFreq, (Capability->TimeoutUnit) ? "MHz" : "KHz"));
-  DEBUG ((DEBUG_INFO, "   Base Clk Freq     %dMHz\n", Capability->BaseClkFreq));
+  if (Capability->BaseClkFreq != BaseClkFreq) {
+    DEBUG ((DEBUG_INFO, "   Controller register value overriden:\n"));
+  }
+  DEBUG ((DEBUG_INFO, "   Base Clk Freq     %dMHz\n", BaseClkFreq));
   DEBUG ((DEBUG_INFO, "   Max Blk Len       %dbytes\n", 512 * (1 << Capability->MaxBlkLen)));
   DEBUG ((DEBUG_INFO, "   8-bit Support     %a\n", Capability->BusWidth8 ? "TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   ADMA2 Support     %a\n", Capability->Adma2 ? "TRUE" : "FALSE"));
@@ -719,7 +724,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.
@@ -730,11 +735,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;
@@ -744,9 +748,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;
   }
@@ -937,7 +940,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.
@@ -947,7 +950,7 @@ 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;
@@ -956,7 +959,7 @@ SdMmcHcInitClockFreq (
   //
   // Calculate a divisor for SD clock frequency
   //
-  if (Capability.BaseClkFreq == 0) {
+  if (BaseClkFreq == 0) {
     //
     // Don't support get Base Clock Frequency information via another method
     //
@@ -966,7 +969,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;
 }
 
@@ -1099,7 +1102,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

[PATCH 5/7] MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence

[Marcin Wojtas]

According to JESD84-B50-1 chapter A.6 (documentation about eMMC4.5
standard) step "Changing the data bus width" (A.6.3) should be
execute after step "Switching to high-speed mode" (A.6.2).

This patch fixes the bus-width/clock-setting sequence
in EmmcSwitchToHighSpeed ().

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
index e5e0c89..8615caa 100755
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c
@@ -748,10 +748,6 @@ EmmcSwitchToHighSpeed (
 
   Private = SD_MMC_HC_PRIVATE_FROM_THIS (PassThru);
 
-  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
-  if (EFI_ERROR (Status)) {
-    return Status;
-  }
   //
   // Set to Hight Speed timing
   //
@@ -799,6 +795,11 @@ EmmcSwitchToHighSpeed (
     return Status;
   }
 
+  Status = EmmcSwitchBusWidth (PciIo, PassThru, Slot, Rca, IsDdr, BusWidth);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
   if (mOverride != NULL && mOverride->SwitchClockFreqPost != NULL) {
     Status = mOverride->SwitchClockFreqPost (
                           Private->ControllerHandle,
-- 
2.7.4

[PATCH 6/7] MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits

[Marcin Wojtas]

From: Tomasz Michalec <tm@semihalf.com>

SdMmcHcReset used to set all bits of Software Reset Register to 1
including reserved ones.

Now only first bit is set which means "Software Reset for All".

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index e7c51e6..c9a0acc 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -459,8 +459,8 @@ SdMmcHcReset (
   }
 
   PciIo   = Private->PciIo;
-  SwReset = 0xFF;
-  Status  = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof (SwReset), &SwReset);
+  SwReset = BIT0;
+  Status  = SdMmcHcOrMmio (PciIo, Slot, SD_MMC_HC_SW_RST, sizeof (SwReset), &SwReset);
 
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "SdMmcHcReset: write full 1 fails: %r\n", Status));
@@ -472,7 +472,7 @@ SdMmcHcReset (
              Slot,
              SD_MMC_HC_SW_RST,
              sizeof (SwReset),
-             0xFF,
+             BIT0,
              0x00,
              SD_MMC_HC_GENERIC_TIMEOUT
              );
-- 
2.7.4

[PATCH 7/7] MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot

[Marcin Wojtas]

Some devices can be non removable (such as eMMC) and checking
Present State Register on host controller may falsely return
an information that device is not present. Execute this
check conditionally on the SloType field value.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marcin Wojtas <mw@semihalf.com>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 50c1e74..174efa9 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -678,12 +678,18 @@ SdMmcPciHcDriverBindingStart (
     //
     // Check whether there is a SD/MMC card attached
     //
-    Status = SdMmcHcCardDetect (PciIo, Slot, &MediaPresent);
-    if (EFI_ERROR (Status) && (Status != EFI_MEDIA_CHANGED)) {
-      continue;
-    } else if (!MediaPresent) {
-      DEBUG ((DEBUG_INFO, "SdMmcHcCardDetect: No device attached in Slot[%d]!!!\n", Slot));
-      continue;
+    if (Private->Slot[Slot].SlotType == RemovableSlot) {
+      Status = SdMmcHcCardDetect (PciIo, Slot, &MediaPresent);
+      if (EFI_ERROR (Status) && (Status != EFI_MEDIA_CHANGED)) {
+        continue;
+      } else if (!MediaPresent) {
+        DEBUG ((
+          DEBUG_INFO,
+          "SdMmcHcCardDetect: No device attached in Slot[%d]!!!\n",
+          Slot
+          ));
+        continue;
+      }
     }
 
     Status = SdMmcHcInitHost (Private, Slot);
-- 
2.7.4

Re: [PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Wu, Hao A]

Hi,

I will take a look into this series.
It might take me some time for the review, please help to ping this mail
thread if there is no response from me in 2 weeks. Sorry for the possible
delay.

Also, cc Ard in the list to see if he has any comment on this. As Ard is
the contributor of the SD/MMC override protocol.

Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Marcin Wojtas
> Sent: Monday, September 03, 2018 12:54 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng; nadavh@marvell.com; Gao, Liming; Kinney, Michael D
> Subject: [edk2] [PATCH 0/7] SdMmc fixes and SdMmcOverride extension
> 
> Hi,
> 
> This patchset extends SdMmcOverride protocol with new callbacks:
> * UhsSignaling - allow writing custom values to HostControl2 register
> * SwitchClockFreqPost - perform additional opperations after clock switch
> * BaseClockFreq - allow overriding base clock frequency
> Also a couple of fixes for MMC, card detection and reset are submitted.
> More details can be found in the commit messages.
> 
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/sdmmc-override-upstream-r20180902
> 
> 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: Modify initialization of SdMmcOverride")
> which immunizes for above and future extensions of the protocol:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-
> platform/commits/xenon-upstream-r20180902
> 
> I'm looking forward to the comments and remarks.
> 
> Best regards,
> Marcin
> 
> Marcin Wojtas (3):
>   MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
>   MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
>   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
> 
> Tomasz Michalec (4):
>   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride
> protocol
>   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
>   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
>   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +++++-
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  83 ++++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 246
> +++++++++++++-------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  55 ++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++++++--
>  7 files changed, 467 insertions(+), 129 deletions(-)
> 
> --
> 2.7.4
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Ard Biesheuvel]

On 3 September 2018 at 06:54, Marcin Wojtas <mw@semihalf.com> wrote:
> Hi,
>
> This patchset extends SdMmcOverride protocol with new callbacks:
> * UhsSignaling - allow writing custom values to HostControl2 register
> * SwitchClockFreqPost - perform additional opperations after clock switch
> * BaseClockFreq - allow overriding base clock frequency
> Also a couple of fixes for MMC, card detection and reset are submitted.
> More details can be found in the commit messages.
>
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20180902
>
> 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: Modify initialization of SdMmcOverride")
> which immunizes for above and future extensions of the protocol:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20180902
>
> I'm looking forward to the comments and remarks.
>

Could we split this into a series of fixes/compliance tweaks, and a
set of changes to the SD/MMC override protocol hooks that need to be
added to accommodate Xenon?

I suppose the former can be merged without much hassle, but the latter
may provoke some fierce discussion, I'm afraid.



> Marcin Wojtas (3):
>   MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
>   MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
>   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
>
> Tomasz Michalec (4):
>   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
>   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
>   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
>   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
>
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +++++-
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  83 ++++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 246 +++++++++++++-------
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  55 ++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++++++--
>  7 files changed, 467 insertions(+), 129 deletions(-)
>
> --
> 2.7.4
>

Re: [PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Marcin Wojtas]

Hi Ard,

czw., 6 wrz 2018 o 16:12 Ard Biesheuvel <ard.biesheuvel@linaro.org> napisał(a):
>
> On 3 September 2018 at 06:54, Marcin Wojtas <mw@semihalf.com> wrote:
> > Hi,
> >
> > This patchset extends SdMmcOverride protocol with new callbacks:
> > * UhsSignaling - allow writing custom values to HostControl2 register
> > * SwitchClockFreqPost - perform additional opperations after clock switch
> > * BaseClockFreq - allow overriding base clock frequency
> > Also a couple of fixes for MMC, card detection and reset are submitted.
> > More details can be found in the commit messages.
> >
> > Patches are available in the github:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20180902
> >
> > 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: Modify initialization of SdMmcOverride")
> > which immunizes for above and future extensions of the protocol:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20180902
> >
> > I'm looking forward to the comments and remarks.
> >
>
> Could we split this into a series of fixes/compliance tweaks, and a
> set of changes to the SD/MMC override protocol hooks that need to be
> added to accommodate Xenon?
>
> I suppose the former can be merged without much hassle, but the latter
> may provoke some fierce discussion, I'm afraid.
>

Ok. will split into two separate series. Anyway, I hope the override
protocol won't be that problematic - no functional change for
non-users, and the callbacks resemble the hooks that are provided by
drivers/mmc in Linux.

Marcin

>
>
> > Marcin Wojtas (3):
> >   MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
> >   MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
> >   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
> >
> > Tomasz Michalec (4):
> >   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
> >   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
> >   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
> >   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
> >
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +++++-
> >  MdeModulePkg/Include/Protocol/SdMmcOverride.h      |  83 ++++++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c    | 246 +++++++++++++-------
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c      |  55 ++++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++++++--
> >  7 files changed, 467 insertions(+), 129 deletions(-)
> >
> > --
> > 2.7.4
> >