[PATCH v4 0/2] quirks handling for SDHCI controllers

[PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

Many SDHCI implementations exist that are almost spec complicant, and
could be driven by the generic SD/MMC host controller driver except
for some minimal necessary init time tweaks.

Adding such tweaks to the generic driver is undesirable. On the other
hand, forking the driver for every platform that has such a SDHCI
controller is problematic when it comes to upstreaming and ongoing
maintenance (which is arguably the point of upstreaming in the first
place).

So these patches propose a workaround that is minimally invasive on the
EDK2 side, but gives platforms a lot of leeway when it comes to applying
SDHCI quirks.

Changes since v3:
- remove PassThru argument from protocol members: it is unclear whether the
  protocol is available when the override protocol is invoked, and my
  example use case does not need it
- replace incorrect HandleProtocol with LocateProtocol, given that the override
  protocol is now a singleton instance
- merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
  required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA* argument
  and so the prototypes no longer belong in SdMmcPciHci.h and have been moved
  to SdMmcPciHcDxe.h
- use VOID* type for capability not UINT64* since we don't know its alignment

Changes since v2:
- use a singleton instance of the SD/MMC protocol rather than one per
  controller; this is needed to support 'reconnect -r', as pointed out
  by Ray
- use EDKII prefixes for all types defined by the protocol
- replace 'hook' with 'notify', and tweak some other identifiers
- add missing function comment headers for factored out functions

Changes since RFC/v1:
- add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
- use UINT64* not VOID* to pass capability structure (which is always 64 bits
  in size)

Ard Biesheuvel (2):
  MdeModulePkg: introduce SD/MMC override protocol
  MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden

 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95 +++++++++++++++++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
 MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                        |  3 +
 7 files changed, 257 insertions(+), 46 deletions(-)
 create mode 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h

-- 
2.11.0

[PATCH v4 1/2] MdeModulePkg: introduce SD/MMC override protocol

[Ard Biesheuvel]

Many ARM based SoCs have integrated SDHCI controllers, and often,
these implementations deviate in subtle ways from the pertinent
specifications. On the one hand, these deviations are quite easy
to work around, but on the other hand, having a collection of SoC
specific workarounds in the generic driver stack is undesirable.

So let's introduce an optional SD/MMC override protocol that we
can invoke at the appropriate moments in the device initialization.
That way, the workaround itself remains platform specific, but we
can still use the generic driver stack on such platforms.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Include/Protocol/SdMmcOverride.h | 97 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                 |  3 +
 2 files changed, 100 insertions(+)

diff --git a/MdeModulePkg/Include/Protocol/SdMmcOverride.h b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
new file mode 100644
index 000000000000..0d070023f9d3
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/SdMmcOverride.h
@@ -0,0 +1,97 @@
+/** @file
+  Protocol to describe overrides required to support non-standard SDHCI
+  implementations
+
+  Copyright (c) 2017, Linaro, Ltd. All rights reserved.<BR>
+
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#ifndef __SD_MMC_OVERRIDE_H__
+#define __SD_MMC_OVERRIDE_H__
+
+#include <Protocol/SdMmcPassThru.h>
+
+#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
+
+typedef struct _EDKII_SD_MMC_OVERRIDE EDKII_SD_MMC_OVERRIDE;
+
+typedef enum {
+  EdkiiSdMmcResetPre,
+  EdkiiSdMmcResetPost,
+  EdkiiSdMmcInitHostPre,
+  EdkiiSdMmcInitHostPost,
+} EDKII_SD_MMC_PHASE_TYPE;
+
+/**
+
+  Override function for SDHCI capability bits
+
+  @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.
+
+  @retval EFI_SUCCESS           The override function completed successfully.
+  @retval EFI_NOT_FOUND         The specified controller or slot does not exist.
+  @retval EFI_INVALID_PARAMETER SdMmcHcSlotCapability is NULL
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EDKII_SD_MMC_CAPABILITY) (
+  IN      EFI_HANDLE                      ControllerHandle,
+  IN      UINT8                           Slot,
+  IN  OUT VOID                            *SdMmcHcSlotCapability
+  );
+
+/**
+
+  Override function for SDHCI controller operations
+
+  @param[in]      ControllerHandle      The EFI_HANDLE of the controller.
+  @param[in]      Slot                  The 0 based slot index.
+  @param[in]      PhaseType             The type of operation and whether the
+                                        hook is invoked right before (pre) or
+                                        right after (post)
+
+  @retval EFI_SUCCESS           The override function completed successfully.
+  @retval EFI_NOT_FOUND         The specified controller or slot does not exist.
+  @retval EFI_INVALID_PARAMETER PhaseType is invalid
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI * EDKII_SD_MMC_NOTIFY_PHASE) (
+  IN      EFI_HANDLE                      ControllerHandle,
+  IN      UINT8                           Slot,
+  IN      EDKII_SD_MMC_PHASE_TYPE         PhaseType
+  );
+
+struct _EDKII_SD_MMC_OVERRIDE {
+  //
+  // Protocol version of this implementation
+  //
+  UINTN                         Version;
+  //
+  // Callback to override SD/MMC host controller capability bits
+  //
+  EDKII_SD_MMC_CAPABILITY       Capability;
+  //
+  // Callback to invoke SD/MMC override hooks
+  //
+  EDKII_SD_MMC_NOTIFY_PHASE     NotifyPhase;
+};
+
+extern EFI_GUID gEdkiiSdMmcOverrideProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 856d67aceb21..64ceea029f94 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -562,6 +562,9 @@ [Protocols]
   ## Include/Protocol/SmmMemoryAttribute.h
   gEdkiiSmmMemoryAttributeProtocolGuid = { 0x69b792ea, 0x39ce, 0x402d, { 0xa2, 0xa6, 0xf7, 0x21, 0xde, 0x35, 0x1d, 0xfe } }
 
+  ## Include/Protocol/SdMmcOverride.h
+  gEdkiiSdMmcOverrideProtocolGuid = { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23, 0x23 } }
+
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
-- 
2.11.0

[PATCH v4 2/2] MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden

[Ard Biesheuvel]

Invoke the newly introduced SD/MMC override protocol to override
the capabilities register after reading it from the device registers,
and to call the pre/post host init and reset hooks at the appropriate
times.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 +++++++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95 ++++++++++++++++++--
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 --------
 5 files changed, 157 insertions(+), 46 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index 0be8828abfcc..f923930bbae9 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -17,6 +17,8 @@
 
 #include "SdMmcPciHcDxe.h"
 
+EDKII_SD_MMC_OVERRIDE           *mOverride;
+
 //
 // Driver Global Variables
 //
@@ -281,14 +283,14 @@ SdMmcPciHcEnumerateDevice (
         //
         // Reset the specified slot of the SD/MMC Pci Host Controller
         //
-        Status = SdMmcHcReset (Private->PciIo, Slot);
+        Status = SdMmcHcReset (Private, Slot);
         if (EFI_ERROR (Status)) {
           continue;
         }
         //
         // Reinitialize slot and restart identification process for the new attached device
         //
-        Status = SdMmcHcInitHost (Private->PciIo, Slot, Private->Capability[Slot]);
+        Status = SdMmcHcInitHost (Private, Slot);
         if (EFI_ERROR (Status)) {
           continue;
         }
@@ -601,6 +603,20 @@ SdMmcPciHcDriverBindingStart (
     goto Done;
   }
 
+  //
+  // Attempt to locate the singleton instance of the SD/MMC override protocol,
+  // which implements platform specific workarounds for non-standard SDHCI
+  // implementations.
+  //
+  if (mOverride == NULL) {
+    Status = gBS->LocateProtocol (&gEdkiiSdMmcOverrideProtocolGuid, NULL,
+                    (VOID **)&mOverride);
+    if (!EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_INFO, "%a: found SD/MMC override protocol\n",
+        __FUNCTION__));
+    }
+  }
+
   Support64BitDma = TRUE;
   for (Slot = FirstBar; Slot < (FirstBar + SlotNum); Slot++) {
     Private->Slot[Slot].Enable = TRUE;
@@ -609,6 +625,17 @@ SdMmcPciHcDriverBindingStart (
     if (EFI_ERROR (Status)) {
       continue;
     }
+    if (mOverride != NULL && mOverride->Capability != NULL) {
+      Status = mOverride->Capability (
+                            Controller,
+                            Slot,
+                            &Private->Capability[Slot]);
+      if (EFI_ERROR (Status)) {
+        DEBUG ((DEBUG_WARN, "%a: Failed to override capability - %r\n",
+          __FUNCTION__, Status));
+        continue;
+      }
+    }
     DumpCapabilityReg (Slot, &Private->Capability[Slot]);
 
     Support64BitDma &= Private->Capability[Slot].SysBus64;
@@ -627,7 +654,7 @@ SdMmcPciHcDriverBindingStart (
     //
     // Reset the specified slot of the SD/MMC Pci Host Controller
     //
-    Status = SdMmcHcReset (PciIo, Slot);
+    Status = SdMmcHcReset (Private, Slot);
     if (EFI_ERROR (Status)) {
       continue;
     }
@@ -642,7 +669,7 @@ SdMmcPciHcDriverBindingStart (
       continue;
     }
 
-    Status = SdMmcHcInitHost (PciIo, Slot, Private->Capability[Slot]);
+    Status = SdMmcHcInitHost (Private, Slot);
     if (EFI_ERROR (Status)) {
       continue;
     }
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 6a2a27969936..c683600f2ef2 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -35,6 +35,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 #include <Protocol/DriverBinding.h>
 #include <Protocol/ComponentName.h>
 #include <Protocol/ComponentName2.h>
+#include <Protocol/SdMmcOverride.h>
 #include <Protocol/SdMmcPassThru.h>
 
 #include "SdMmcPciHci.h"
@@ -43,6 +44,8 @@ extern EFI_COMPONENT_NAME_PROTOCOL  gSdMmcPciHcComponentName;
 extern EFI_COMPONENT_NAME2_PROTOCOL gSdMmcPciHcComponentName2;
 extern EFI_DRIVER_BINDING_PROTOCOL  gSdMmcPciHcDriverBinding;
 
+extern EDKII_SD_MMC_OVERRIDE        *mOverride;
+
 #define SD_MMC_HC_PRIVATE_SIGNATURE  SIGNATURE_32 ('s', 'd', 't', 'f')
 
 #define SD_MMC_HC_PRIVATE_FROM_THIS(a) \
@@ -782,4 +785,37 @@ SdCardIdentification (
   IN UINT8                              Slot
   );
 
+/**
+  Software reset the specified SD/MMC host controller.
+
+  @param[in] Private        A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+  @param[in] Slot           The slot number of the SD card to send the command to.
+
+  @retval EFI_SUCCESS       The software reset executes successfully.
+  @retval Others            The software reset fails.
+
+**/
+EFI_STATUS
+SdMmcHcReset (
+  IN SD_MMC_HC_PRIVATE_DATA *Private,
+  IN UINT8                  Slot
+  );
+
+/**
+  Initial SD/MMC host controller with lowest clock frequency, max power and max timeout value
+  at initialization.
+
+  @param[in] Private        A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
+  @param[in] Slot           The slot number of the SD card to send the command to.
+
+  @retval EFI_SUCCESS       The host controller is initialized successfully.
+  @retval Others            The host controller isn't initialized successfully.
+
+**/
+EFI_STATUS
+SdMmcHcInitHost (
+  IN SD_MMC_HC_PRIVATE_DATA *Private,
+  IN UINT8                  Slot
+  );
+
 #endif
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
index e26e6a098c17..154ce45d8223 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf
@@ -48,6 +48,7 @@ [Sources]
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
   DevicePathLib
@@ -61,6 +62,7 @@ [LibraryClasses]
   DebugLib
 
 [Protocols]
+  gEdkiiSdMmcOverrideProtocolGuid               ## SOMETIMES_CONSUMES
   gEfiDevicePathProtocolGuid                    ## TO_START
   gEfiPciIoProtocolGuid                         ## TO_START
   gEfiSdMmcPassThruProtocolGuid                 ## BY_START
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index aa75aa8d2434..385a50b12079 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -419,7 +419,7 @@ SdMmcHcWaitMmioSet (
 /**
   Software reset the specified SD/MMC host controller and enable all interrupts.
 
-  @param[in] PciIo          The PCI IO protocol instance.
+  @param[in] Private        A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
 
   @retval EFI_SUCCESS       The software reset executes successfully.
@@ -428,13 +428,32 @@ SdMmcHcWaitMmioSet (
 **/
 EFI_STATUS
 SdMmcHcReset (
-  IN EFI_PCI_IO_PROTOCOL    *PciIo,
+  IN SD_MMC_HC_PRIVATE_DATA *Private,
   IN UINT8                  Slot
   )
 {
   EFI_STATUS                Status;
   UINT8                     SwReset;
+  EFI_PCI_IO_PROTOCOL       *PciIo;
+
+  //
+  // Notify the SD/MMC override protocol that we are about to reset
+  // the SD/MMC host controller.
+  //
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+    Status = mOverride->NotifyPhase (
+                          Private->ControllerHandle,
+                          Slot,
+                          EdkiiSdMmcResetPre);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN,
+        "%a: SD/MMC pre reset notifier callback failed - %r\n",
+        __FUNCTION__, Status));
+      return Status;
+    }
+  }
 
+  PciIo   = Private->PciIo;
   SwReset = 0xFF;
   Status  = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_SW_RST, FALSE, sizeof (SwReset), &SwReset);
 
@@ -456,10 +475,32 @@ SdMmcHcReset (
     DEBUG ((DEBUG_INFO, "SdMmcHcReset: reset done with %r\n", Status));
     return Status;
   }
+
   //
   // Enable all interrupt after reset all.
   //
   Status = SdMmcHcEnableInterrupt (PciIo, Slot);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "SdMmcHcReset: SdMmcHcEnableInterrupt done with %r\n",
+      Status));
+    return Status;
+  }
+
+  //
+  // Notify the SD/MMC override protocol that we have just reset
+  // the SD/MMC host controller.
+  //
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+    Status = mOverride->NotifyPhase (
+                          Private->ControllerHandle,
+                          Slot,
+                          EdkiiSdMmcResetPost);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN,
+        "%a: SD/MMC post reset notifier callback failed - %r\n",
+        __FUNCTION__, Status));
+    }
+  }
 
   return Status;
 }
@@ -1021,7 +1062,7 @@ SdMmcHcInitTimeoutCtrl (
   Initial SD/MMC host controller with lowest clock frequency, max power and max timeout value
   at initialization.
 
-  @param[in] PciIo          The PCI IO protocol instance.
+  @param[in] Private        A pointer to the SD_MMC_HC_PRIVATE_DATA instance.
   @param[in] Slot           The slot number of the SD card to send the command to.
   @param[in] Capability     The capability of the slot.
 
@@ -1031,12 +1072,33 @@ SdMmcHcInitTimeoutCtrl (
 **/
 EFI_STATUS
 SdMmcHcInitHost (
-  IN EFI_PCI_IO_PROTOCOL    *PciIo,
-  IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
+  IN SD_MMC_HC_PRIVATE_DATA *Private,
+  IN UINT8                  Slot
   )
 {
-  EFI_STATUS       Status;
+  EFI_STATUS                Status;
+  EFI_PCI_IO_PROTOCOL       *PciIo;
+  SD_MMC_HC_SLOT_CAP        Capability;
+
+  //
+  // Notify the SD/MMC override protocol that we are about to initialize
+  // the SD/MMC host controller.
+  //
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+    Status = mOverride->NotifyPhase (
+                          Private->ControllerHandle,
+                          Slot,
+                          EdkiiSdMmcInitHostPre);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN,
+        "%a: SD/MMC pre init notifier callback failed - %r\n",
+        __FUNCTION__, Status));
+      return Status;
+    }
+  }
+
+  PciIo = Private->PciIo;
+  Capability = Private->Capability[Slot];
 
   Status = SdMmcHcInitClockFreq (PciIo, Slot, Capability);
   if (EFI_ERROR (Status)) {
@@ -1049,6 +1111,25 @@ SdMmcHcInitHost (
   }
 
   Status = SdMmcHcInitTimeoutCtrl (PciIo, Slot);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  //
+  // Notify the SD/MMC override protocol that we are have just initialized
+  // the SD/MMC host controller.
+  //
+  if (mOverride != NULL && mOverride->NotifyPhase != NULL) {
+    Status = mOverride->NotifyPhase (
+                          Private->ControllerHandle,
+                          Slot,
+                          EdkiiSdMmcInitHostPost);
+    if (EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_WARN,
+        "%a: SD/MMC post init notifier callback failed - %r\n",
+        __FUNCTION__, Status));
+    }
+  }
   return Status;
 }
 
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index fb627586022c..e389d52184f4 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -298,22 +298,6 @@ SdMmcHcWaitMmioSet (
   );
 
 /**
-  Software reset the specified SD/MMC host controller.
-
-  @param[in] PciIo          The PCI IO protocol instance.
-  @param[in] Slot           The slot number of the SD card to send the command to.
-
-  @retval EFI_SUCCESS       The software reset executes successfully.
-  @retval Others            The software reset fails.
-
-**/
-EFI_STATUS
-SdMmcHcReset (
-  IN EFI_PCI_IO_PROTOCOL    *PciIo,
-  IN UINT8                  Slot
-  );
-
-/**
   Set all interrupt status bits in Normal and Error Interrupt Status Enable
   register.
 
@@ -524,23 +508,4 @@ SdMmcHcInitTimeoutCtrl (
   IN UINT8                  Slot
   );
 
-/**
-  Initial SD/MMC host controller with lowest clock frequency, max power and max timeout value
-  at initialization.
-
-  @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.
-
-  @retval EFI_SUCCESS       The host controller is initialized successfully.
-  @retval Others            The host controller isn't initialized successfully.
-
-**/
-EFI_STATUS
-SdMmcHcInitHost (
-  IN EFI_PCI_IO_PROTOCOL    *PciIo,
-  IN UINT8                  Slot,
-  IN SD_MMC_HC_SLOT_CAP     Capability
-  );
-
 #endif
-- 
2.11.0

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 7 December 2017 at 22:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Many SDHCI implementations exist that are almost spec complicant, and
> could be driven by the generic SD/MMC host controller driver except
> for some minimal necessary init time tweaks.
>
> Adding such tweaks to the generic driver is undesirable. On the other
> hand, forking the driver for every platform that has such a SDHCI
> controller is problematic when it comes to upstreaming and ongoing
> maintenance (which is arguably the point of upstreaming in the first
> place).
>
> So these patches propose a workaround that is minimally invasive on the
> EDK2 side, but gives platforms a lot of leeway when it comes to applying
> SDHCI quirks.
>
> Changes since v3:
> - remove PassThru argument from protocol members: it is unclear whether the
>   protocol is available when the override protocol is invoked, and my
>   example use case does not need it
> - replace incorrect HandleProtocol with LocateProtocol, given that the override
>   protocol is now a singleton instance
> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA* argument
>   and so the prototypes no longer belong in SdMmcPciHci.h and have been moved
>   to SdMmcPciHcDxe.h
> - use VOID* type for capability not UINT64* since we don't know its alignment
>
> Changes since v2:
> - use a singleton instance of the SD/MMC protocol rather than one per
>   controller; this is needed to support 'reconnect -r', as pointed out
>   by Ray
> - use EDKII prefixes for all types defined by the protocol
> - replace 'hook' with 'notify', and tweak some other identifiers
> - add missing function comment headers for factored out functions
>
> Changes since RFC/v1:
> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>   in size)
>
> Ard Biesheuvel (2):
>   MdeModulePkg: introduce SD/MMC override protocol
>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>

OK, so I did send my v4 but I couldn't find it in my mail folders :-)

Comments anyone?

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Wu, Hao A]

Hi Ard,

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
> Biesheuvel
> Sent: Tuesday, December 12, 2017 3:00 PM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Tian, Feng; Ard Biesheuvel; Wu, Hao A; Leif Lindholm; Kinney,
> Michael D; Zeng, Star
> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> On 7 December 2017 at 22:43, Ard Biesheuvel <ard.biesheuvel@linaro.org>
> wrote:
> > Many SDHCI implementations exist that are almost spec complicant, and
> > could be driven by the generic SD/MMC host controller driver except
> > for some minimal necessary init time tweaks.
> >
> > Adding such tweaks to the generic driver is undesirable. On the other
> > hand, forking the driver for every platform that has such a SDHCI
> > controller is problematic when it comes to upstreaming and ongoing
> > maintenance (which is arguably the point of upstreaming in the first
> > place).
> >
> > So these patches propose a workaround that is minimally invasive on the
> > EDK2 side, but gives platforms a lot of leeway when it comes to applying
> > SDHCI quirks.
> >
> > Changes since v3:
> > - remove PassThru argument from protocol members: it is unclear whether the
> >   protocol is available when the override protocol is invoked, and my
> >   example use case does not need it
> > - replace incorrect HandleProtocol with LocateProtocol, given that the
> override
> >   protocol is now a singleton instance
> > - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
> >   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
> argument
> >   and so the prototypes no longer belong in SdMmcPciHci.h and have been
> moved
> >   to SdMmcPciHcDxe.h
> > - use VOID* type for capability not UINT64* since we don't know its
> alignment
> >
> > Changes since v2:
> > - use a singleton instance of the SD/MMC protocol rather than one per
> >   controller; this is needed to support 'reconnect -r', as pointed out
> >   by Ray
> > - use EDKII prefixes for all types defined by the protocol
> > - replace 'hook' with 'notify', and tweak some other identifiers
> > - add missing function comment headers for factored out functions
> >
> > Changes since RFC/v1:
> > - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
> > - use UINT64* not VOID* to pass capability structure (which is always 64 bits
> >   in size)
> >
> > Ard Biesheuvel (2):
> >   MdeModulePkg: introduce SD/MMC override protocol
> >   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
> >
> 
> OK, so I did send my v4 but I couldn't find it in my mail folders :-)
> 
> Comments anyone?

I still need some time to evaluate whether the current proposed override
protocol can be utilized in our using scenario.

Will let you know the feedbacks as soon as I finish the evaluation.

Really sorry for the delay.


Best Regards,
Hao Wu

> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 12 December 2017 at 10:56, Wu, Hao A <hao.a.wu@intel.com> wrote:
> Hi Ard,
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>> Biesheuvel
>> Sent: Tuesday, December 12, 2017 3:00 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu; Tian, Feng; Ard Biesheuvel; Wu, Hao A; Leif Lindholm; Kinney,
>> Michael D; Zeng, Star
>> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
>>
>> On 7 December 2017 at 22:43, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>> > Many SDHCI implementations exist that are almost spec complicant, and
>> > could be driven by the generic SD/MMC host controller driver except
>> > for some minimal necessary init time tweaks.
>> >
>> > Adding such tweaks to the generic driver is undesirable. On the other
>> > hand, forking the driver for every platform that has such a SDHCI
>> > controller is problematic when it comes to upstreaming and ongoing
>> > maintenance (which is arguably the point of upstreaming in the first
>> > place).
>> >
>> > So these patches propose a workaround that is minimally invasive on the
>> > EDK2 side, but gives platforms a lot of leeway when it comes to applying
>> > SDHCI quirks.
>> >
>> > Changes since v3:
>> > - remove PassThru argument from protocol members: it is unclear whether the
>> >   protocol is available when the override protocol is invoked, and my
>> >   example use case does not need it
>> > - replace incorrect HandleProtocol with LocateProtocol, given that the
>> override
>> >   protocol is now a singleton instance
>> > - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>> >   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
>> argument
>> >   and so the prototypes no longer belong in SdMmcPciHci.h and have been
>> moved
>> >   to SdMmcPciHcDxe.h
>> > - use VOID* type for capability not UINT64* since we don't know its
>> alignment
>> >
>> > Changes since v2:
>> > - use a singleton instance of the SD/MMC protocol rather than one per
>> >   controller; this is needed to support 'reconnect -r', as pointed out
>> >   by Ray
>> > - use EDKII prefixes for all types defined by the protocol
>> > - replace 'hook' with 'notify', and tweak some other identifiers
>> > - add missing function comment headers for factored out functions
>> >
>> > Changes since RFC/v1:
>> > - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>> > - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>> >   in size)
>> >
>> > Ard Biesheuvel (2):
>> >   MdeModulePkg: introduce SD/MMC override protocol
>> >   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>> >
>>
>> OK, so I did send my v4 but I couldn't find it in my mail folders :-)
>>
>> Comments anyone?
>
> I still need some time to evaluate whether the current proposed override
> protocol can be utilized in our using scenario.
>
> Will let you know the feedbacks as soon as I finish the evaluation.
>
> Really sorry for the delay.
>

No problem at all. Just keep me informed.

Thanks,
Ard.

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 12 December 2017 at 10:56, Wu, Hao A <hao.a.wu@intel.com> wrote:
> Hi Ard,
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard
>> Biesheuvel
>> Sent: Tuesday, December 12, 2017 3:00 PM
>> To: edk2-devel@lists.01.org
>> Cc: Ni, Ruiyu; Tian, Feng; Ard Biesheuvel; Wu, Hao A; Leif Lindholm; Kinney,
>> Michael D; Zeng, Star
>> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
>>
>> On 7 December 2017 at 22:43, Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> wrote:
>> > Many SDHCI implementations exist that are almost spec complicant, and
>> > could be driven by the generic SD/MMC host controller driver except
>> > for some minimal necessary init time tweaks.
>> >
>> > Adding such tweaks to the generic driver is undesirable. On the other
>> > hand, forking the driver for every platform that has such a SDHCI
>> > controller is problematic when it comes to upstreaming and ongoing
>> > maintenance (which is arguably the point of upstreaming in the first
>> > place).
>> >
>> > So these patches propose a workaround that is minimally invasive on the
>> > EDK2 side, but gives platforms a lot of leeway when it comes to applying
>> > SDHCI quirks.
>> >
>> > Changes since v3:
>> > - remove PassThru argument from protocol members: it is unclear whether the
>> >   protocol is available when the override protocol is invoked, and my
>> >   example use case does not need it
>> > - replace incorrect HandleProtocol with LocateProtocol, given that the
>> override
>> >   protocol is now a singleton instance
>> > - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>> >   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
>> argument
>> >   and so the prototypes no longer belong in SdMmcPciHci.h and have been
>> moved
>> >   to SdMmcPciHcDxe.h
>> > - use VOID* type for capability not UINT64* since we don't know its
>> alignment
>> >
>> > Changes since v2:
>> > - use a singleton instance of the SD/MMC protocol rather than one per
>> >   controller; this is needed to support 'reconnect -r', as pointed out
>> >   by Ray
>> > - use EDKII prefixes for all types defined by the protocol
>> > - replace 'hook' with 'notify', and tweak some other identifiers
>> > - add missing function comment headers for factored out functions
>> >
>> > Changes since RFC/v1:
>> > - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>> > - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>> >   in size)
>> >
>> > Ard Biesheuvel (2):
>> >   MdeModulePkg: introduce SD/MMC override protocol
>> >   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>> >
>>
>> OK, so I did send my v4 but I couldn't find it in my mail folders :-)
>>
>> Comments anyone?
>
> I still need some time to evaluate whether the current proposed override
> protocol can be utilized in our using scenario.
>
> Will let you know the feedbacks as soon as I finish the evaluation.
>
> Really sorry for the delay.
>

Any news on this?

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Wu, Hao A]

One minor comment, please help to remove the line (around line 1067):
@param[in] Capability     The capability of the slot.

within function description comment for SdMmcHcInitHost() in file:
MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c

Other than that, the series is good to me:
Reviewed-by: Hao Wu <hao.a.wu@intel.com>

Really sorry for the delay.

Best Regards,
Hao Wu


> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Friday, December 08, 2017 6:43 AM
> To: edk2-devel@lists.01.org
> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian, Feng; Ni,
> Ruiyu; Wu, Hao A; Ard Biesheuvel
> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> Many SDHCI implementations exist that are almost spec complicant, and
> could be driven by the generic SD/MMC host controller driver except
> for some minimal necessary init time tweaks.
> 
> Adding such tweaks to the generic driver is undesirable. On the other
> hand, forking the driver for every platform that has such a SDHCI
> controller is problematic when it comes to upstreaming and ongoing
> maintenance (which is arguably the point of upstreaming in the first
> place).
> 
> So these patches propose a workaround that is minimally invasive on the
> EDK2 side, but gives platforms a lot of leeway when it comes to applying
> SDHCI quirks.
> 
> Changes since v3:
> - remove PassThru argument from protocol members: it is unclear whether the
>   protocol is available when the override protocol is invoked, and my
>   example use case does not need it
> - replace incorrect HandleProtocol with LocateProtocol, given that the override
>   protocol is now a singleton instance
> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
> argument
>   and so the prototypes no longer belong in SdMmcPciHci.h and have been
> moved
>   to SdMmcPciHcDxe.h
> - use VOID* type for capability not UINT64* since we don't know its alignment
> 
> Changes since v2:
> - use a singleton instance of the SD/MMC protocol rather than one per
>   controller; this is needed to support 'reconnect -r', as pointed out
>   by Ray
> - use EDKII prefixes for all types defined by the protocol
> - replace 'hook' with 'notify', and tweak some other identifiers
> - add missing function comment headers for factored out functions
> 
> Changes since RFC/v1:
> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>   in size)
> 
> Ard Biesheuvel (2):
>   MdeModulePkg: introduce SD/MMC override protocol
>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
> 
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
> +++++++++++++++++--
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
> ++++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
>  7 files changed, 257 insertions(+), 46 deletions(-)
>  create mode 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
> 
> --
> 2.11.0

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 29 January 2018 at 05:13, Wu, Hao A <hao.a.wu@intel.com> wrote:
> One minor comment, please help to remove the line (around line 1067):
> @param[in] Capability     The capability of the slot.
>
> within function description comment for SdMmcHcInitHost() in file:
> MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c
>
> Other than that, the series is good to me:
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>

Thank you very much!

> Really sorry for the delay.
>

No worries. Star, Ray, any more comments from your side?


>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Friday, December 08, 2017 6:43 AM
>> To: edk2-devel@lists.01.org
>> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian, Feng; Ni,
>> Ruiyu; Wu, Hao A; Ard Biesheuvel
>> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
>>
>> Many SDHCI implementations exist that are almost spec complicant, and
>> could be driven by the generic SD/MMC host controller driver except
>> for some minimal necessary init time tweaks.
>>
>> Adding such tweaks to the generic driver is undesirable. On the other
>> hand, forking the driver for every platform that has such a SDHCI
>> controller is problematic when it comes to upstreaming and ongoing
>> maintenance (which is arguably the point of upstreaming in the first
>> place).
>>
>> So these patches propose a workaround that is minimally invasive on the
>> EDK2 side, but gives platforms a lot of leeway when it comes to applying
>> SDHCI quirks.
>>
>> Changes since v3:
>> - remove PassThru argument from protocol members: it is unclear whether the
>>   protocol is available when the override protocol is invoked, and my
>>   example use case does not need it
>> - replace incorrect HandleProtocol with LocateProtocol, given that the override
>>   protocol is now a singleton instance
>> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
>> argument
>>   and so the prototypes no longer belong in SdMmcPciHci.h and have been
>> moved
>>   to SdMmcPciHcDxe.h
>> - use VOID* type for capability not UINT64* since we don't know its alignment
>>
>> Changes since v2:
>> - use a singleton instance of the SD/MMC protocol rather than one per
>>   controller; this is needed to support 'reconnect -r', as pointed out
>>   by Ray
>> - use EDKII prefixes for all types defined by the protocol
>> - replace 'hook' with 'notify', and tweak some other identifiers
>> - add missing function comment headers for factored out functions
>>
>> Changes since RFC/v1:
>> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>>   in size)
>>
>> Ard Biesheuvel (2):
>>   MdeModulePkg: introduce SD/MMC override protocol
>>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>>
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
>> +++++++++++++++++--
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
>>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
>> ++++++++++++++++++++
>>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
>>  7 files changed, 257 insertions(+), 46 deletions(-)
>>  create mode 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
>>
>> --
>> 2.11.0
>

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

Thanks Hao's investigation and Ard's contribution.


Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Monday, January 29, 2018 4:26 PM
To: Wu, Hao A <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>
Subject: Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

On 29 January 2018 at 05:13, Wu, Hao A <hao.a.wu@intel.com> wrote:
> One minor comment, please help to remove the line (around line 1067):
> @param[in] Capability     The capability of the slot.
>
> within function description comment for SdMmcHcInitHost() in file:
> MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c
>
> Other than that, the series is good to me:
> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>

Thank you very much!

> Really sorry for the delay.
>

No worries. Star, Ray, any more comments from your side?


>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>> Sent: Friday, December 08, 2017 6:43 AM
>> To: edk2-devel@lists.01.org
>> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian, 
>> Feng; Ni, Ruiyu; Wu, Hao A; Ard Biesheuvel
>> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
>>
>> Many SDHCI implementations exist that are almost spec complicant, and 
>> could be driven by the generic SD/MMC host controller driver except 
>> for some minimal necessary init time tweaks.
>>
>> Adding such tweaks to the generic driver is undesirable. On the other 
>> hand, forking the driver for every platform that has such a SDHCI 
>> controller is problematic when it comes to upstreaming and ongoing 
>> maintenance (which is arguably the point of upstreaming in the first 
>> place).
>>
>> So these patches propose a workaround that is minimally invasive on 
>> the
>> EDK2 side, but gives platforms a lot of leeway when it comes to 
>> applying SDHCI quirks.
>>
>> Changes since v3:
>> - remove PassThru argument from protocol members: it is unclear whether the
>>   protocol is available when the override protocol is invoked, and my
>>   example use case does not need it
>> - replace incorrect HandleProtocol with LocateProtocol, given that the override
>>   protocol is now a singleton instance
>> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA* 
>> argument
>>   and so the prototypes no longer belong in SdMmcPciHci.h and have 
>> been moved
>>   to SdMmcPciHcDxe.h
>> - use VOID* type for capability not UINT64* since we don't know its 
>> alignment
>>
>> Changes since v2:
>> - use a singleton instance of the SD/MMC protocol rather than one per
>>   controller; this is needed to support 'reconnect -r', as pointed out
>>   by Ray
>> - use EDKII prefixes for all types defined by the protocol
>> - replace 'hook' with 'notify', and tweak some other identifiers
>> - add missing function comment headers for factored out functions
>>
>> Changes since RFC/v1:
>> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>>   in size)
>>
>> Ard Biesheuvel (2):
>>   MdeModulePkg: introduce SD/MMC override protocol
>>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>>
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
>> +++++++++++++++++--
>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
>>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
>> ++++++++++++++++++++
>>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
>>  7 files changed, 257 insertions(+), 46 deletions(-)  create mode 
>> 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
>>
>> --
>> 2.11.0
>

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 30 January 2018 at 01:24, Zeng, Star <star.zeng@intel.com> wrote:
> Reviewed-by: Star Zeng <star.zeng@intel.com>
>
> Thanks Hao's investigation and Ard's contribution.
>

Thanks all

Pushed as 864701886fc3..b23fc39cd3c3

>
> Star
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, January 29, 2018 4:26 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>
> Subject: Re: [PATCH v4 0/2] quirks handling for SDHCI controllers
>
> On 29 January 2018 at 05:13, Wu, Hao A <hao.a.wu@intel.com> wrote:
>> One minor comment, please help to remove the line (around line 1067):
>> @param[in] Capability     The capability of the slot.
>>
>> within function description comment for SdMmcHcInitHost() in file:
>> MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c
>>
>> Other than that, the series is good to me:
>> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
>>
>
> Thank you very much!
>
>> Really sorry for the delay.
>>
>
> No worries. Star, Ray, any more comments from your side?
>
>
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: Friday, December 08, 2017 6:43 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian,
>>> Feng; Ni, Ruiyu; Wu, Hao A; Ard Biesheuvel
>>> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
>>>
>>> Many SDHCI implementations exist that are almost spec complicant, and
>>> could be driven by the generic SD/MMC host controller driver except
>>> for some minimal necessary init time tweaks.
>>>
>>> Adding such tweaks to the generic driver is undesirable. On the other
>>> hand, forking the driver for every platform that has such a SDHCI
>>> controller is problematic when it comes to upstreaming and ongoing
>>> maintenance (which is arguably the point of upstreaming in the first
>>> place).
>>>
>>> So these patches propose a workaround that is minimally invasive on
>>> the
>>> EDK2 side, but gives platforms a lot of leeway when it comes to
>>> applying SDHCI quirks.
>>>
>>> Changes since v3:
>>> - remove PassThru argument from protocol members: it is unclear whether the
>>>   protocol is available when the override protocol is invoked, and my
>>>   example use case does not need it
>>> - replace incorrect HandleProtocol with LocateProtocol, given that the override
>>>   protocol is now a singleton instance
>>> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
>>>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
>>> argument
>>>   and so the prototypes no longer belong in SdMmcPciHci.h and have
>>> been moved
>>>   to SdMmcPciHcDxe.h
>>> - use VOID* type for capability not UINT64* since we don't know its
>>> alignment
>>>
>>> Changes since v2:
>>> - use a singleton instance of the SD/MMC protocol rather than one per
>>>   controller; this is needed to support 'reconnect -r', as pointed out
>>>   by Ray
>>> - use EDKII prefixes for all types defined by the protocol
>>> - replace 'hook' with 'notify', and tweak some other identifiers
>>> - add missing function comment headers for factored out functions
>>>
>>> Changes since RFC/v1:
>>> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
>>> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
>>>   in size)
>>>
>>> Ard Biesheuvel (2):
>>>   MdeModulePkg: introduce SD/MMC override protocol
>>>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
>>>
>>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
>>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36 ++++++++
>>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
>>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
>>> +++++++++++++++++--
>>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
>>>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
>>> ++++++++++++++++++++
>>>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
>>>  7 files changed, 257 insertions(+), 46 deletions(-)  create mode
>>> 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
>>>
>>> --
>>> 2.11.0
>>

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Meenakshi Aggarwal]

Hi,

I am using Mmc Driver implemented in " EmbeddedPkg/Universal/MmcDxe/" for my SD/MMC controller and my controller is not on PCI bus.

I am a bit confused if i should move to SD implementation available in 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe".

Please suggest.


Thanks,
Meenakshi

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, January 30, 2018 3:22 PM
> To: Zeng, Star <star.zeng@intel.com>
> Cc: Ni, Ruiyu <ruiyu.ni@intel.com>; Tian, Feng <feng.tian@intel.com>; Wu,
> Hao A <hao.a.wu@intel.com>; edk2-devel@lists.01.org;
> leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> On 30 January 2018 at 01:24, Zeng, Star <star.zeng@intel.com> wrote:
> > Reviewed-by: Star Zeng <star.zeng@intel.com>
> >
> > Thanks Hao's investigation and Ard's contribution.
> >
> 
> Thanks all
> 
> Pushed as 864701886fc3..b23fc39cd3c3
> 
> >
> > Star
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Monday, January 29, 2018 4:26 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>
> > Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>
> > Subject: Re: [PATCH v4 0/2] quirks handling for SDHCI controllers
> >
> > On 29 January 2018 at 05:13, Wu, Hao A <hao.a.wu@intel.com> wrote:
> >> One minor comment, please help to remove the line (around line 1067):
> >> @param[in] Capability     The capability of the slot.
> >>
> >> within function description comment for SdMmcHcInitHost() in file:
> >> MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c
> >>
> >> Other than that, the series is good to me:
> >> Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> >>
> >
> > Thank you very much!
> >
> >> Really sorry for the delay.
> >>
> >
> > No worries. Star, Ray, any more comments from your side?
> >
> >
> >>
> >>> -----Original Message-----
> >>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >>> Sent: Friday, December 08, 2017 6:43 AM
> >>> To: edk2-devel@lists.01.org
> >>> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian,
> >>> Feng; Ni, Ruiyu; Wu, Hao A; Ard Biesheuvel
> >>> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
> >>>
> >>> Many SDHCI implementations exist that are almost spec complicant, and
> >>> could be driven by the generic SD/MMC host controller driver except
> >>> for some minimal necessary init time tweaks.
> >>>
> >>> Adding such tweaks to the generic driver is undesirable. On the other
> >>> hand, forking the driver for every platform that has such a SDHCI
> >>> controller is problematic when it comes to upstreaming and ongoing
> >>> maintenance (which is arguably the point of upstreaming in the first
> >>> place).
> >>>
> >>> So these patches propose a workaround that is minimally invasive on
> >>> the
> >>> EDK2 side, but gives platforms a lot of leeway when it comes to
> >>> applying SDHCI quirks.
> >>>
> >>> Changes since v3:
> >>> - remove PassThru argument from protocol members: it is unclear
> whether the
> >>>   protocol is available when the override protocol is invoked, and my
> >>>   example use case does not need it
> >>> - replace incorrect HandleProtocol with LocateProtocol, given that the
> override
> >>>   protocol is now a singleton instance
> >>> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
> >>>   required changing the prototype to take a
> SD_MMC_HC_PRIVATE_DATA*
> >>> argument
> >>>   and so the prototypes no longer belong in SdMmcPciHci.h and have
> >>> been moved
> >>>   to SdMmcPciHcDxe.h
> >>> - use VOID* type for capability not UINT64* since we don't know its
> >>> alignment
> >>>
> >>> Changes since v2:
> >>> - use a singleton instance of the SD/MMC protocol rather than one per
> >>>   controller; this is needed to support 'reconnect -r', as pointed out
> >>>   by Ray
> >>> - use EDKII prefixes for all types defined by the protocol
> >>> - replace 'hook' with 'notify', and tweak some other identifiers
> >>> - add missing function comment headers for factored out functions
> >>>
> >>> Changes since RFC/v1:
> >>> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override
> methods
> >>> - use UINT64* not VOID* to pass capability structure (which is always 64
> bits
> >>>   in size)
> >>>
> >>> Ard Biesheuvel (2):
> >>>   MdeModulePkg: introduce SD/MMC override protocol
> >>>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be
> overridden
> >>>
> >>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35
> ++++++-
> >>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36
> ++++++++
> >>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
> >>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
> >>> +++++++++++++++++--
> >>>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
> >>>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
> >>> ++++++++++++++++++++
> >>>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
> >>>  7 files changed, 257 insertions(+), 46 deletions(-)  create mode
> >>> 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
> >>>
> >>> --
> >>> 2.11.0
> >>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.01.org%2Fmailman%2Flistinfo%2Fedk2-
> devel&data=02%7C01%7Cmeenakshi.aggarwal%40nxp.com%7Ce1c1a5872a7
> 1476d17fc08d567c7280c%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
> 7C636529027474388370&sdata=gkmLosBokCxUJsWSPlvmsUABakEYdbIZTOho
> qFsbbpI%3D&reserved=0

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 6 March 2018 at 05:14, Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> wrote:
> Hi,
>
> I am using Mmc Driver implemented in " EmbeddedPkg/Universal/MmcDxe/" for my SD/MMC controller and my controller is not on PCI bus.
>
> I am a bit confused if i should move to SD implementation available in 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe".
>
> Please suggest.
>

It really depends on whether the IP is SDHCI compatible or not.

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Meenakshi Aggarwal]

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 06, 2018 4:41 PM
> To: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> On 6 March 2018 at 05:14, Meenakshi Aggarwal
> <meenakshi.aggarwal@nxp.com> wrote:
> > Hi,
> >
> > I am using Mmc Driver implemented in "
> EmbeddedPkg/Universal/MmcDxe/" for my SD/MMC controller and my
> controller is not on PCI bus.
> >
> > I am a bit confused if i should move to SD implementation available in
> 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe".
> >
> > Please suggest.
> >
> 
> It really depends on whether the IP is SDHCI compatible or not.

Thanks for reply Ard, its not SDHCI compatible, so i guess no additional advantage in moving to 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe' for my IP.

Re: [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ni, Ruiyu]

Ard,
No more comments from my side.

Thanks/Ray

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Monday, January 29, 2018 4:26 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; Zeng, Star <star.zeng@intel.com>; Ni,
> Ruiyu <ruiyu.ni@intel.com>
> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>; Tian, Feng <feng.tian@intel.com>
> Subject: Re: [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> On 29 January 2018 at 05:13, Wu, Hao A <hao.a.wu@intel.com> wrote:
> > One minor comment, please help to remove the line (around line 1067):
> > @param[in] Capability     The capability of the slot.
> >
> > within function description comment for SdMmcHcInitHost() in file:
> > MdeModulePkg\Bus\Pci\SdMmcPciHcDxe\SdMmcPciHci.c
> >
> > Other than that, the series is good to me:
> > Reviewed-by: Hao Wu <hao.a.wu@intel.com>
> >
> 
> Thank you very much!
> 
> > Really sorry for the delay.
> >
> 
> No worries. Star, Ray, any more comments from your side?
> 
> 
> >
> >> -----Original Message-----
> >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> >> Sent: Friday, December 08, 2017 6:43 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: leif.lindholm@linaro.org; Kinney, Michael D; Zeng, Star; Tian,
> >> Feng; Ni, Ruiyu; Wu, Hao A; Ard Biesheuvel
> >> Subject: [PATCH v4 0/2] quirks handling for SDHCI controllers
> >>
> >> Many SDHCI implementations exist that are almost spec complicant, and
> >> could be driven by the generic SD/MMC host controller driver except
> >> for some minimal necessary init time tweaks.
> >>
> >> Adding such tweaks to the generic driver is undesirable. On the other
> >> hand, forking the driver for every platform that has such a SDHCI
> >> controller is problematic when it comes to upstreaming and ongoing
> >> maintenance (which is arguably the point of upstreaming in the first
> >> place).
> >>
> >> So these patches propose a workaround that is minimally invasive on
> >> the
> >> EDK2 side, but gives platforms a lot of leeway when it comes to
> >> applying SDHCI quirks.
> >>
> >> Changes since v3:
> >> - remove PassThru argument from protocol members: it is unclear whether
> the
> >>   protocol is available when the override protocol is invoked, and my
> >>   example use case does not need it
> >> - replace incorrect HandleProtocol with LocateProtocol, given that the
> override
> >>   protocol is now a singleton instance
> >> - merge notifier calls into SdMmcHcReset() and SdMmcHcInitHost (), this
> >>   required changing the prototype to take a SD_MMC_HC_PRIVATE_DATA*
> >> argument
> >>   and so the prototypes no longer belong in SdMmcPciHci.h and have
> >> been moved
> >>   to SdMmcPciHcDxe.h
> >> - use VOID* type for capability not UINT64* since we don't know its
> >> alignment
> >>
> >> Changes since v2:
> >> - use a singleton instance of the SD/MMC protocol rather than one per
> >>   controller; this is needed to support 'reconnect -r', as pointed out
> >>   by Ray
> >> - use EDKII prefixes for all types defined by the protocol
> >> - replace 'hook' with 'notify', and tweak some other identifiers
> >> - add missing function comment headers for factored out functions
> >>
> >> Changes since RFC/v1:
> >> - add EFI_SD_MMC_PASS_THRU_PROTOCOL* member to override methods
> >> - use UINT64* not VOID* to pass capability structure (which is always 64 bits
> >>   in size)
> >>
> >> Ard Biesheuvel (2):
> >>   MdeModulePkg: introduce SD/MMC override protocol
> >>   MdeModulePkg/SdMmcPciHcDxe: allow HC capabilities to be overridden
> >>
> >>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c   | 35 ++++++-
> >>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h   | 36
> ++++++++
> >>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.inf |  2 +
> >>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c     | 95
> >> +++++++++++++++++--
> >>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h     | 35 -------
> >>  MdeModulePkg/Include/Protocol/SdMmcOverride.h        | 97
> >> ++++++++++++++++++++
> >>  MdeModulePkg/MdeModulePkg.dec                        |  3 +
> >>  7 files changed, 257 insertions(+), 46 deletions(-)  create mode
> >> 100644 MdeModulePkg/Include/Protocol/SdMmcOverride.h
> >>
> >> --
> >> 2.11.0
> >