public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-platforms][PATCH v3 0/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection
@ 2020-07-13 11:16 Pete Batard
  2020-07-13 11:16 ` [edk2-platforms][PATCH v3 1/1] " Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-07-13 11:16 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, awarkentin

This supersedes https://edk2.groups.io/g/devel/message/62339 - 62340

Changes from v2:
- Fix regression for CM3 module due to MMC/eMMC not accepting CMD8

Pete Batard (1):
  Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection

 Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 86 ++++++++++++++++++--
 Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c               | 75 +++++++++++++++--
 Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h               |  6 ++
 3 files changed, 150 insertions(+), 17 deletions(-)

-- 
2.21.0.windows.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection
  2020-07-13 11:16 [edk2-platforms][PATCH v3 0/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection Pete Batard
@ 2020-07-13 11:16 ` Pete Batard
  2020-07-14 13:25   ` Leif Lindholm
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-07-13 11:16 UTC (permalink / raw)
  To: devel; +Cc: ard.biesheuvel, leif, awarkentin, Andrei Warkentin

The Raspberry Pi 3 and Pi 4 platforms (with latest EEPROM) can boot
straight from USB, without the need for an SD card being present.
However, the IsCardPresent () calls from the ArasanMmcHost and SdHost
drivers are currently hardwired to return TRUE, which results in
straight to USB boot failing due to the SD drivers looping on
errors while trying to poke at a non-existent card...

Ideally, we would use the Card Detect signal from the uSD slot, to
report on the presence or absence of a card, but the Raspberry Pi
Foundation did not wire those signals in the Pi 2 and subsequent
models, leaving us with only potentially interfering SD commands
as means to perform card detection.

As a result of this, we are left with no other choice but limit
detection to occurring only once, prior to formal SD card init,
and then return the detected value for subsequent calls. This,
however, is more than good enough for the intended purpose, which
is to allow straight to USB boot. The sequence is a simplified
variant of the identification code in MmcDxe.

Tested on Raspberry Pi 2B, 3B and CM3 (for both SD controllers)
and Pi 4 (for Arasan, as that's the only controller available today)

Addresses pftf/RPi3#13, pftf/RPi3#14, pftf/RPi4#37.

Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
Signed-off-by: Pete Batard <pete@akeo.ie>
---
 Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 86 ++++++++++++++++++--
 Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c               | 75 +++++++++++++++--
 Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h               |  6 ++
 3 files changed, 150 insertions(+), 17 deletions(-)

diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
index 6d706af6f276..d2a8ffddbb66 100644
--- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
@@ -11,7 +11,8 @@
 
 #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
 
-BOOLEAN PreviousIsCardPresent = FALSE;
+BOOLEAN CardIsPresent = FALSE;
+CARD_DETECT_STATE CardDetectState = CardDetectRequired;
 UINT32 LastExecutedCommand = (UINT32) -1;
 
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
@@ -239,14 +240,6 @@ CalculateClockFrequencyDivisor (
   return EFI_SUCCESS;
 }
 
-BOOLEAN
-MMCIsCardPresent (
-  IN EFI_MMC_HOST_PROTOCOL *This
-)
-{
-  return TRUE;
-}
-
 BOOLEAN
 MMCIsReadOnly (
   IN EFI_MMC_HOST_PROTOCOL *This
@@ -418,6 +411,10 @@ MMCNotifyState (
 
   DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
 
+  // Stall all operations except init until card detection has occurred.
+  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
+    return EFI_NOT_READY;
+
   switch (State) {
   case MmcHwInitializationState:
     {
@@ -489,6 +486,77 @@ MMCNotifyState (
   return EFI_SUCCESS;
 }
 
+BOOLEAN
+MMCIsCardPresent (
+  IN EFI_MMC_HOST_PROTOCOL *This
+)
+{
+  EFI_STATUS Status;
+
+  //
+  // If we are already in progress (we may get concurrent calls)
+  // or completed the detection, just return the current value.
+  //
+  if (CardDetectState != CardDetectRequired)
+    return CardIsPresent;
+
+  CardDetectState = CardDetectInProgress;
+  CardIsPresent = FALSE;
+
+  //
+  // The two following commands should succeed even if no card is present.
+  //
+  Status = MMCNotifyState (This, MmcHwInitializationState);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
+    // If we failed init, go back to requiring card detection
+    CardDetectState = CardDetectRequired;
+    return FALSE;
+  }
+
+  Status = MMCSendCommand (This, MMC_CMD0, 0);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: CMD0 Error, Status=%r.\n", Status));
+    goto out;
+  }
+
+  //
+  // CMD8 should tell us if an SD card is present.
+  //
+  Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
+  if (!EFI_ERROR (Status)) {
+     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
+     CardIsPresent = TRUE;
+     goto out;
+  }
+
+  //
+  // MMC/eMMC won't accept CMD8, but we can try CMD1.
+  //
+  Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
+  if (!EFI_ERROR (Status)) {
+     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
+     CardIsPresent = TRUE;
+     goto out;
+  }
+
+  //
+  // SDIO?
+  //
+  Status = MMCSendCommand (This, MMC_CMD5, 0);
+  if (!EFI_ERROR (Status)) {
+     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
+     CardIsPresent = TRUE;
+     goto out;
+  }
+
+  DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
+
+out:
+  CardDetectState = CardDetectCompleted;
+  return CardIsPresent;
+}
+
 EFI_STATUS
 MMCReceiveResponse (
   IN EFI_MMC_HOST_PROTOCOL    *This,
diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
index 2f31c5eb8c46..aac8b34c4bf4 100644
--- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
@@ -64,6 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
                                     "genpulses", "writewait2", "?",
                                     "startpowdown" };
 #endif /* NDEBUG */
+STATIC BOOLEAN CardIsPresent = FALSE;
+STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
 STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
 
 STATIC inline BOOLEAN
@@ -264,14 +266,6 @@ SdHostSetClockFrequency (
   return Status;
 }
 
-STATIC BOOLEAN
-SdIsCardPresent (
-  IN EFI_MMC_HOST_PROTOCOL *This
-  )
-{
-  return TRUE;
-}
-
 STATIC BOOLEAN
 SdIsReadOnly (
   IN EFI_MMC_HOST_PROTOCOL *This
@@ -639,6 +633,10 @@ SdNotifyState (
 {
   DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
 
+  // Stall all operations except init until card detection has occurred.
+  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
+    return EFI_NOT_READY;
+
   switch (State) {
   case MmcHwInitializationState:
     DEBUG ((DEBUG_MMCHOST_SD, "MmcHwInitializationState\n", State));
@@ -718,6 +716,67 @@ SdNotifyState (
   return EFI_SUCCESS;
 }
 
+STATIC BOOLEAN
+SdIsCardPresent (
+  IN EFI_MMC_HOST_PROTOCOL *This
+  )
+{
+  EFI_STATUS Status;
+
+  //
+  // If we are already in progress (we may get concurrent calls)
+  // or completed the detection, just return the current value.
+  //
+  if (CardDetectState != CardDetectRequired)
+    return CardIsPresent;
+
+  CardDetectState = CardDetectInProgress;
+  CardIsPresent = FALSE;
+
+  //
+  // The two following commands should succeed even if no card is present.
+  //
+  Status = SdNotifyState (This, MmcHwInitializationState);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
+    // If we failed init, go back to requiring card detection
+    CardDetectState = CardDetectRequired;
+    return FALSE;
+  }
+
+  Status = SdSendCommand (This, MMC_CMD0, 0);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: CMD0 Error, Status=%r.\n", Status));
+    goto out;
+  }
+
+  //
+  // CMD8 should tell us if an SD card is present.
+  //
+  Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
+  if (!EFI_ERROR (Status)) {
+     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
+     CardIsPresent = TRUE;
+     goto out;
+  }
+
+  //
+  // MMC/eMMC won't accept CMD8, but we can try CMD1.
+  //
+  Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
+  if (!EFI_ERROR (Status)) {
+     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
+     CardIsPresent = TRUE;
+     goto out;
+  }
+
+  DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
+
+out:
+  CardDetectState = CardDetectCompleted;
+  return CardIsPresent;
+}
+
 BOOLEAN
 SdIsMultiBlock (
   IN EFI_MMC_HOST_PROTOCOL *This
diff --git a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
index c558e00bf500..78514a31bc4e 100644
--- a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
+++ b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
@@ -82,6 +82,12 @@ typedef enum _MMC_STATE {
     MmcDisconnectState,
 } MMC_STATE;
 
+typedef enum _CARD_DETECT_STATE {
+    CardDetectRequired = 0,
+    CardDetectInProgress,
+    CardDetectCompleted
+} CARD_DETECT_STATE;
+
 #define EMMCBACKWARD         (0)
 #define EMMCHS26             (1 << 0)      // High-Speed @26MHz at rated device voltages
 #define EMMCHS52             (1 << 1)      // High-Speed @52MHz at rated device voltages
-- 
2.21.0.windows.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection
  2020-07-13 11:16 ` [edk2-platforms][PATCH v3 1/1] " Pete Batard
@ 2020-07-14 13:25   ` Leif Lindholm
  2020-07-14 13:49     ` Pete Batard
  0 siblings, 1 reply; 5+ messages in thread
From: Leif Lindholm @ 2020-07-14 13:25 UTC (permalink / raw)
  To: Pete Batard; +Cc: devel, ard.biesheuvel, awarkentin, Andrei Warkentin

On Mon, Jul 13, 2020 at 12:16:20 +0100, Pete Batard wrote:
> The Raspberry Pi 3 and Pi 4 platforms (with latest EEPROM) can boot
> straight from USB, without the need for an SD card being present.
> However, the IsCardPresent () calls from the ArasanMmcHost and SdHost
> drivers are currently hardwired to return TRUE, which results in
> straight to USB boot failing due to the SD drivers looping on
> errors while trying to poke at a non-existent card...
> 
> Ideally, we would use the Card Detect signal from the uSD slot, to
> report on the presence or absence of a card, but the Raspberry Pi
> Foundation did not wire those signals in the Pi 2 and subsequent
> models, leaving us with only potentially interfering SD commands
> as means to perform card detection.
> 
> As a result of this, we are left with no other choice but limit
> detection to occurring only once, prior to formal SD card init,
> and then return the detected value for subsequent calls. This,
> however, is more than good enough for the intended purpose, which
> is to allow straight to USB boot. The sequence is a simplified
> variant of the identification code in MmcDxe.
> 
> Tested on Raspberry Pi 2B, 3B and CM3 (for both SD controllers)
> and Pi 4 (for Arasan, as that's the only controller available today)
> 
> Addresses pftf/RPi3#13, pftf/RPi3#14, pftf/RPi4#37.
> 
> Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
> Signed-off-by: Pete Batard <pete@akeo.ie>

Some minor style comments below, I'm happy to fix them before pushing
if you're OK with these:

> ---
>  Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 86 ++++++++++++++++++--
>  Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c               | 75 +++++++++++++++--
>  Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h               |  6 ++
>  3 files changed, 150 insertions(+), 17 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> index 6d706af6f276..d2a8ffddbb66 100644
> --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> @@ -11,7 +11,8 @@
>  
>  #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
>  
> -BOOLEAN PreviousIsCardPresent = FALSE;
> +BOOLEAN CardIsPresent = FALSE;
> +CARD_DETECT_STATE CardDetectState = CardDetectRequired;

Global variables, so add 'm' prefix?
Also, add STATIC (which also matches SdHostDxe version)?

>  UINT32 LastExecutedCommand = (UINT32) -1;
>  
>  STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> @@ -239,14 +240,6 @@ CalculateClockFrequencyDivisor (
>    return EFI_SUCCESS;
>  }
>  
> -BOOLEAN
> -MMCIsCardPresent (
> -  IN EFI_MMC_HOST_PROTOCOL *This
> -)
> -{
> -  return TRUE;
> -}
> -
>  BOOLEAN
>  MMCIsReadOnly (
>    IN EFI_MMC_HOST_PROTOCOL *This
> @@ -418,6 +411,10 @@ MMCNotifyState (
>  
>    DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
>  
> +  // Stall all operations except init until card detection has occurred.
> +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> +    return EFI_NOT_READY;
> +

Add {}?

>    switch (State) {
>    case MmcHwInitializationState:
>      {
> @@ -489,6 +486,77 @@ MMCNotifyState (
>    return EFI_SUCCESS;
>  }
>  
> +BOOLEAN
> +MMCIsCardPresent (
> +  IN EFI_MMC_HOST_PROTOCOL *This
> +)
> +{
> +  EFI_STATUS Status;
> +
> +  //
> +  // If we are already in progress (we may get concurrent calls)
> +  // or completed the detection, just return the current value.
> +  //
> +  if (CardDetectState != CardDetectRequired)
> +    return CardIsPresent;

Add {}?

> +
> +  CardDetectState = CardDetectInProgress;
> +  CardIsPresent = FALSE;
> +
> +  //
> +  // The two following commands should succeed even if no card is present.
> +  //
> +  Status = MMCNotifyState (This, MmcHwInitializationState);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> +    // If we failed init, go back to requiring card detection
> +    CardDetectState = CardDetectRequired;
> +    return FALSE;
> +  }
> +
> +  Status = MMCSendCommand (This, MMC_CMD0, 0);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: CMD0 Error, Status=%r.\n", Status));
> +    goto out;
> +  }
> +
> +  //
> +  // CMD8 should tell us if an SD card is present.
> +  //
> +  Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> +  if (!EFI_ERROR (Status)) {
> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
> +     CardIsPresent = TRUE;
> +     goto out;
> +  }
> +
> +  //
> +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
> +  //
> +  Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> +  if (!EFI_ERROR (Status)) {
> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
> +     CardIsPresent = TRUE;
> +     goto out;
> +  }
> +
> +  //
> +  // SDIO?
> +  //
> +  Status = MMCSendCommand (This, MMC_CMD5, 0);
> +  if (!EFI_ERROR (Status)) {
> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
> +     CardIsPresent = TRUE;
> +     goto out;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
> +
> +out:
> +  CardDetectState = CardDetectCompleted;
> +  return CardIsPresent;
> +}
> +
>  EFI_STATUS
>  MMCReceiveResponse (
>    IN EFI_MMC_HOST_PROTOCOL    *This,
> diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> index 2f31c5eb8c46..aac8b34c4bf4 100644
> --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> @@ -64,6 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
>                                      "genpulses", "writewait2", "?",
>                                      "startpowdown" };
>  #endif /* NDEBUG */
> +STATIC BOOLEAN CardIsPresent = FALSE;
> +STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;

Add 'm' prefix?

>  STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
>  
>  STATIC inline BOOLEAN
> @@ -264,14 +266,6 @@ SdHostSetClockFrequency (
>    return Status;
>  }
>  
> -STATIC BOOLEAN
> -SdIsCardPresent (
> -  IN EFI_MMC_HOST_PROTOCOL *This
> -  )
> -{
> -  return TRUE;
> -}
> -
>  STATIC BOOLEAN
>  SdIsReadOnly (
>    IN EFI_MMC_HOST_PROTOCOL *This
> @@ -639,6 +633,10 @@ SdNotifyState (
>  {
>    DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
>  
> +  // Stall all operations except init until card detection has occurred.
> +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> +    return EFI_NOT_READY;
> +

Add {}?

>    switch (State) {
>    case MmcHwInitializationState:
>      DEBUG ((DEBUG_MMCHOST_SD, "MmcHwInitializationState\n", State));
> @@ -718,6 +716,67 @@ SdNotifyState (
>    return EFI_SUCCESS;
>  }
>  
> +STATIC BOOLEAN
> +SdIsCardPresent (
> +  IN EFI_MMC_HOST_PROTOCOL *This
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  //
> +  // If we are already in progress (we may get concurrent calls)
> +  // or completed the detection, just return the current value.
> +  //
> +  if (CardDetectState != CardDetectRequired)
> +    return CardIsPresent;

Add {}?

I.e. in total, fold in:
diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
index d2a8ffddbb66..88e9126e3549 100644
--- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
@@ -11,8 +11,8 @@
 
 #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
 
-BOOLEAN CardIsPresent = FALSE;
-CARD_DETECT_STATE CardDetectState = CardDetectRequired;
+STATIC BOOLEAN mCardIsPresent = FALSE;
+STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
 UINT32 LastExecutedCommand = (UINT32) -1;
 
 STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
@@ -412,8 +412,9 @@ MMCNotifyState (
   DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
 
   // Stall all operations except init until card detection has occurred.
-  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
+  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
     return EFI_NOT_READY;
+  }
 
   switch (State) {
   case MmcHwInitializationState:
@@ -497,11 +498,12 @@ MMCIsCardPresent (
   // If we are already in progress (we may get concurrent calls)
   // or completed the detection, just return the current value.
   //
-  if (CardDetectState != CardDetectRequired)
-    return CardIsPresent;
+  if (mCardDetectState != CardDetectRequired) {
+    return mCardIsPresent;
+  }
 
-  CardDetectState = CardDetectInProgress;
-  CardIsPresent = FALSE;
+  mCardDetectState = CardDetectInProgress;
+  mCardIsPresent = FALSE;
 
   //
   // The two following commands should succeed even if no card is present.
@@ -510,7 +512,7 @@ MMCIsCardPresent (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
     // If we failed init, go back to requiring card detection
-    CardDetectState = CardDetectRequired;
+    mCardDetectState = CardDetectRequired;
     return FALSE;
   }
 
@@ -526,7 +528,7 @@ MMCIsCardPresent (
   Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
   if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
-     CardIsPresent = TRUE;
+     mCardIsPresent = TRUE;
      goto out;
   }
 
@@ -536,7 +538,7 @@ MMCIsCardPresent (
   Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
   if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
-     CardIsPresent = TRUE;
+     mCardIsPresent = TRUE;
      goto out;
   }
 
@@ -546,15 +548,15 @@ MMCIsCardPresent (
   Status = MMCSendCommand (This, MMC_CMD5, 0);
   if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
-     CardIsPresent = TRUE;
+     mCardIsPresent = TRUE;
      goto out;
   }
 
   DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
 
 out:
-  CardDetectState = CardDetectCompleted;
-  return CardIsPresent;
+  mCardDetectState = CardDetectCompleted;
+  return mCardIsPresent;
 }
 
 EFI_STATUS
diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
index aac8b34c4bf4..0fd1ac6e8985 100644
--- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
@@ -64,8 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
                                     "genpulses", "writewait2", "?",
                                     "startpowdown" };
 #endif /* NDEBUG */
-STATIC BOOLEAN CardIsPresent = FALSE;
-STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
+STATIC BOOLEAN mCardIsPresent = FALSE;
+STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
 STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
 
 STATIC inline BOOLEAN
@@ -634,8 +634,9 @@ SdNotifyState (
   DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
 
   // Stall all operations except init until card detection has occurred.
-  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
+  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
     return EFI_NOT_READY;
+  }
 
   switch (State) {
   case MmcHwInitializationState:
@@ -727,11 +728,12 @@ SdIsCardPresent (
   // If we are already in progress (we may get concurrent calls)
   // or completed the detection, just return the current value.
   //
-  if (CardDetectState != CardDetectRequired)
-    return CardIsPresent;
+  if (mCardDetectState != CardDetectRequired) {
+    return mCardIsPresent;
+  }
 
-  CardDetectState = CardDetectInProgress;
-  CardIsPresent = FALSE;
+  mCardDetectState = CardDetectInProgress;
+  mCardIsPresent = FALSE;
 
   //
   // The two following commands should succeed even if no card is present.
@@ -740,7 +742,7 @@ SdIsCardPresent (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
     // If we failed init, go back to requiring card detection
-    CardDetectState = CardDetectRequired;
+    mCardDetectState = CardDetectRequired;
     return FALSE;
   }
 
@@ -756,7 +758,7 @@ SdIsCardPresent (
   Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
   if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
-     CardIsPresent = TRUE;
+     mCardIsPresent = TRUE;
      goto out;
   }
 
@@ -766,15 +768,15 @@ SdIsCardPresent (
   Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
   if (!EFI_ERROR (Status)) {
      DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
-     CardIsPresent = TRUE;
+     mCardIsPresent = TRUE;
      goto out;
   }
 
   DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
 
 out:
-  CardDetectState = CardDetectCompleted;
-  return CardIsPresent;
+  mCardDetectState = CardDetectCompleted;
+  return mCardIsPresent;
 }
 
 BOOLEAN

(Compile tested to ensure I didn't screw up the renamings.)

/
    Leif

> +
> +  CardDetectState = CardDetectInProgress;
> +  CardIsPresent = FALSE;
> +
> +  //
> +  // The two following commands should succeed even if no card is present.
> +  //
> +  Status = SdNotifyState (This, MmcHwInitializationState);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> +    // If we failed init, go back to requiring card detection
> +    CardDetectState = CardDetectRequired;
> +    return FALSE;
> +  }
> +
> +  Status = SdSendCommand (This, MMC_CMD0, 0);
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: CMD0 Error, Status=%r.\n", Status));
> +    goto out;
> +  }
> +
> +  //
> +  // CMD8 should tell us if an SD card is present.
> +  //
> +  Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> +  if (!EFI_ERROR (Status)) {
> +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
> +     CardIsPresent = TRUE;
> +     goto out;
> +  }
> +
> +  //
> +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
> +  //
> +  Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> +  if (!EFI_ERROR (Status)) {
> +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
> +     CardIsPresent = TRUE;
> +     goto out;
> +  }
> +
> +  DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
> +
> +out:
> +  CardDetectState = CardDetectCompleted;
> +  return CardIsPresent;
> +}
> +
>  BOOLEAN
>  SdIsMultiBlock (
>    IN EFI_MMC_HOST_PROTOCOL *This
> diff --git a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> index c558e00bf500..78514a31bc4e 100644
> --- a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> +++ b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> @@ -82,6 +82,12 @@ typedef enum _MMC_STATE {
>      MmcDisconnectState,
>  } MMC_STATE;
>  
> +typedef enum _CARD_DETECT_STATE {
> +    CardDetectRequired = 0,
> +    CardDetectInProgress,
> +    CardDetectCompleted
> +} CARD_DETECT_STATE;
> +
>  #define EMMCBACKWARD         (0)
>  #define EMMCHS26             (1 << 0)      // High-Speed @26MHz at rated device voltages
>  #define EMMCHS52             (1 << 1)      // High-Speed @52MHz at rated device voltages
> -- 
> 2.21.0.windows.1
> 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection
  2020-07-14 13:25   ` Leif Lindholm
@ 2020-07-14 13:49     ` Pete Batard
  2020-07-14 14:44       ` Leif Lindholm
  0 siblings, 1 reply; 5+ messages in thread
From: Pete Batard @ 2020-07-14 13:49 UTC (permalink / raw)
  To: Leif Lindholm; +Cc: devel, ard.biesheuvel, awarkentin, Andrei Warkentin

On 2020.07.14 14:25, Leif Lindholm wrote:
> On Mon, Jul 13, 2020 at 12:16:20 +0100, Pete Batard wrote:
>> The Raspberry Pi 3 and Pi 4 platforms (with latest EEPROM) can boot
>> straight from USB, without the need for an SD card being present.
>> However, the IsCardPresent () calls from the ArasanMmcHost and SdHost
>> drivers are currently hardwired to return TRUE, which results in
>> straight to USB boot failing due to the SD drivers looping on
>> errors while trying to poke at a non-existent card...
>>
>> Ideally, we would use the Card Detect signal from the uSD slot, to
>> report on the presence or absence of a card, but the Raspberry Pi
>> Foundation did not wire those signals in the Pi 2 and subsequent
>> models, leaving us with only potentially interfering SD commands
>> as means to perform card detection.
>>
>> As a result of this, we are left with no other choice but limit
>> detection to occurring only once, prior to formal SD card init,
>> and then return the detected value for subsequent calls. This,
>> however, is more than good enough for the intended purpose, which
>> is to allow straight to USB boot. The sequence is a simplified
>> variant of the identification code in MmcDxe.
>>
>> Tested on Raspberry Pi 2B, 3B and CM3 (for both SD controllers)
>> and Pi 4 (for Arasan, as that's the only controller available today)
>>
>> Addresses pftf/RPi3#13, pftf/RPi3#14, pftf/RPi4#37.
>>
>> Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
>> Signed-off-by: Pete Batard <pete@akeo.ie>
> 
> Some minor style comments below, I'm happy to fix them before pushing
> if you're OK with these:

I agree with the proposed changes. Thanks for volunteering to fix these.

I just tested your diff with a Pi 4, for good measure, and everything 
looks good.

Regards,

/Pete

> 
>> ---
>>   Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 86 ++++++++++++++++++--
>>   Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c               | 75 +++++++++++++++--
>>   Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h               |  6 ++
>>   3 files changed, 150 insertions(+), 17 deletions(-)
>>
>> diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
>> index 6d706af6f276..d2a8ffddbb66 100644
>> --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
>> @@ -11,7 +11,8 @@
>>   
>>   #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
>>   
>> -BOOLEAN PreviousIsCardPresent = FALSE;
>> +BOOLEAN CardIsPresent = FALSE;
>> +CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> 
> Global variables, so add 'm' prefix?
> Also, add STATIC (which also matches SdHostDxe version)?
> 
>>   UINT32 LastExecutedCommand = (UINT32) -1;
>>   
>>   STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
>> @@ -239,14 +240,6 @@ CalculateClockFrequencyDivisor (
>>     return EFI_SUCCESS;
>>   }
>>   
>> -BOOLEAN
>> -MMCIsCardPresent (
>> -  IN EFI_MMC_HOST_PROTOCOL *This
>> -)
>> -{
>> -  return TRUE;
>> -}
>> -
>>   BOOLEAN
>>   MMCIsReadOnly (
>>     IN EFI_MMC_HOST_PROTOCOL *This
>> @@ -418,6 +411,10 @@ MMCNotifyState (
>>   
>>     DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
>>   
>> +  // Stall all operations except init until card detection has occurred.
>> +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
>> +    return EFI_NOT_READY;
>> +
> 
> Add {}?
> 
>>     switch (State) {
>>     case MmcHwInitializationState:
>>       {
>> @@ -489,6 +486,77 @@ MMCNotifyState (
>>     return EFI_SUCCESS;
>>   }
>>   
>> +BOOLEAN
>> +MMCIsCardPresent (
>> +  IN EFI_MMC_HOST_PROTOCOL *This
>> +)
>> +{
>> +  EFI_STATUS Status;
>> +
>> +  //
>> +  // If we are already in progress (we may get concurrent calls)
>> +  // or completed the detection, just return the current value.
>> +  //
>> +  if (CardDetectState != CardDetectRequired)
>> +    return CardIsPresent;
> 
> Add {}?
> 
>> +
>> +  CardDetectState = CardDetectInProgress;
>> +  CardIsPresent = FALSE;
>> +
>> +  //
>> +  // The two following commands should succeed even if no card is present.
>> +  //
>> +  Status = MMCNotifyState (This, MmcHwInitializationState);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
>> +    // If we failed init, go back to requiring card detection
>> +    CardDetectState = CardDetectRequired;
>> +    return FALSE;
>> +  }
>> +
>> +  Status = MMCSendCommand (This, MMC_CMD0, 0);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: CMD0 Error, Status=%r.\n", Status));
>> +    goto out;
>> +  }
>> +
>> +  //
>> +  // CMD8 should tell us if an SD card is present.
>> +  //
>> +  Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
>> +  if (!EFI_ERROR (Status)) {
>> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
>> +     CardIsPresent = TRUE;
>> +     goto out;
>> +  }
>> +
>> +  //
>> +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
>> +  //
>> +  Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
>> +  if (!EFI_ERROR (Status)) {
>> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
>> +     CardIsPresent = TRUE;
>> +     goto out;
>> +  }
>> +
>> +  //
>> +  // SDIO?
>> +  //
>> +  Status = MMCSendCommand (This, MMC_CMD5, 0);
>> +  if (!EFI_ERROR (Status)) {
>> +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
>> +     CardIsPresent = TRUE;
>> +     goto out;
>> +  }
>> +
>> +  DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
>> +
>> +out:
>> +  CardDetectState = CardDetectCompleted;
>> +  return CardIsPresent;
>> +}
>> +
>>   EFI_STATUS
>>   MMCReceiveResponse (
>>     IN EFI_MMC_HOST_PROTOCOL    *This,
>> diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
>> index 2f31c5eb8c46..aac8b34c4bf4 100644
>> --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
>> +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
>> @@ -64,6 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
>>                                       "genpulses", "writewait2", "?",
>>                                       "startpowdown" };
>>   #endif /* NDEBUG */
>> +STATIC BOOLEAN CardIsPresent = FALSE;
>> +STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> 
> Add 'm' prefix?
> 
>>   STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
>>   
>>   STATIC inline BOOLEAN
>> @@ -264,14 +266,6 @@ SdHostSetClockFrequency (
>>     return Status;
>>   }
>>   
>> -STATIC BOOLEAN
>> -SdIsCardPresent (
>> -  IN EFI_MMC_HOST_PROTOCOL *This
>> -  )
>> -{
>> -  return TRUE;
>> -}
>> -
>>   STATIC BOOLEAN
>>   SdIsReadOnly (
>>     IN EFI_MMC_HOST_PROTOCOL *This
>> @@ -639,6 +633,10 @@ SdNotifyState (
>>   {
>>     DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
>>   
>> +  // Stall all operations except init until card detection has occurred.
>> +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
>> +    return EFI_NOT_READY;
>> +
> 
> Add {}?
> 
>>     switch (State) {
>>     case MmcHwInitializationState:
>>       DEBUG ((DEBUG_MMCHOST_SD, "MmcHwInitializationState\n", State));
>> @@ -718,6 +716,67 @@ SdNotifyState (
>>     return EFI_SUCCESS;
>>   }
>>   
>> +STATIC BOOLEAN
>> +SdIsCardPresent (
>> +  IN EFI_MMC_HOST_PROTOCOL *This
>> +  )
>> +{
>> +  EFI_STATUS Status;
>> +
>> +  //
>> +  // If we are already in progress (we may get concurrent calls)
>> +  // or completed the detection, just return the current value.
>> +  //
>> +  if (CardDetectState != CardDetectRequired)
>> +    return CardIsPresent;
> 
> Add {}?
> 
> I.e. in total, fold in:
> diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> index d2a8ffddbb66..88e9126e3549 100644
> --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> @@ -11,8 +11,8 @@
>   
>   #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
>   
> -BOOLEAN CardIsPresent = FALSE;
> -CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> +STATIC BOOLEAN mCardIsPresent = FALSE;
> +STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
>   UINT32 LastExecutedCommand = (UINT32) -1;
>   
>   STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> @@ -412,8 +412,9 @@ MMCNotifyState (
>     DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
>   
>     // Stall all operations except init until card detection has occurred.
> -  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> +  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
>       return EFI_NOT_READY;
> +  }
>   
>     switch (State) {
>     case MmcHwInitializationState:
> @@ -497,11 +498,12 @@ MMCIsCardPresent (
>     // If we are already in progress (we may get concurrent calls)
>     // or completed the detection, just return the current value.
>     //
> -  if (CardDetectState != CardDetectRequired)
> -    return CardIsPresent;
> +  if (mCardDetectState != CardDetectRequired) {
> +    return mCardIsPresent;
> +  }
>   
> -  CardDetectState = CardDetectInProgress;
> -  CardIsPresent = FALSE;
> +  mCardDetectState = CardDetectInProgress;
> +  mCardIsPresent = FALSE;
>   
>     //
>     // The two following commands should succeed even if no card is present.
> @@ -510,7 +512,7 @@ MMCIsCardPresent (
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
>       // If we failed init, go back to requiring card detection
> -    CardDetectState = CardDetectRequired;
> +    mCardDetectState = CardDetectRequired;
>       return FALSE;
>     }
>   
> @@ -526,7 +528,7 @@ MMCIsCardPresent (
>     Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
>     if (!EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
> -     CardIsPresent = TRUE;
> +     mCardIsPresent = TRUE;
>        goto out;
>     }
>   
> @@ -536,7 +538,7 @@ MMCIsCardPresent (
>     Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
>     if (!EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
> -     CardIsPresent = TRUE;
> +     mCardIsPresent = TRUE;
>        goto out;
>     }
>   
> @@ -546,15 +548,15 @@ MMCIsCardPresent (
>     Status = MMCSendCommand (This, MMC_CMD5, 0);
>     if (!EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
> -     CardIsPresent = TRUE;
> +     mCardIsPresent = TRUE;
>        goto out;
>     }
>   
>     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
>   
>   out:
> -  CardDetectState = CardDetectCompleted;
> -  return CardIsPresent;
> +  mCardDetectState = CardDetectCompleted;
> +  return mCardIsPresent;
>   }
>   
>   EFI_STATUS
> diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> index aac8b34c4bf4..0fd1ac6e8985 100644
> --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> @@ -64,8 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
>                                       "genpulses", "writewait2", "?",
>                                       "startpowdown" };
>   #endif /* NDEBUG */
> -STATIC BOOLEAN CardIsPresent = FALSE;
> -STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> +STATIC BOOLEAN mCardIsPresent = FALSE;
> +STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
>   STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
>   
>   STATIC inline BOOLEAN
> @@ -634,8 +634,9 @@ SdNotifyState (
>     DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
>   
>     // Stall all operations except init until card detection has occurred.
> -  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> +  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
>       return EFI_NOT_READY;
> +  }
>   
>     switch (State) {
>     case MmcHwInitializationState:
> @@ -727,11 +728,12 @@ SdIsCardPresent (
>     // If we are already in progress (we may get concurrent calls)
>     // or completed the detection, just return the current value.
>     //
> -  if (CardDetectState != CardDetectRequired)
> -    return CardIsPresent;
> +  if (mCardDetectState != CardDetectRequired) {
> +    return mCardIsPresent;
> +  }
>   
> -  CardDetectState = CardDetectInProgress;
> -  CardIsPresent = FALSE;
> +  mCardDetectState = CardDetectInProgress;
> +  mCardIsPresent = FALSE;
>   
>     //
>     // The two following commands should succeed even if no card is present.
> @@ -740,7 +742,7 @@ SdIsCardPresent (
>     if (EFI_ERROR (Status)) {
>       DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
>       // If we failed init, go back to requiring card detection
> -    CardDetectState = CardDetectRequired;
> +    mCardDetectState = CardDetectRequired;
>       return FALSE;
>     }
>   
> @@ -756,7 +758,7 @@ SdIsCardPresent (
>     Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
>     if (!EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
> -     CardIsPresent = TRUE;
> +     mCardIsPresent = TRUE;
>        goto out;
>     }
>   
> @@ -766,15 +768,15 @@ SdIsCardPresent (
>     Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
>     if (!EFI_ERROR (Status)) {
>        DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
> -     CardIsPresent = TRUE;
> +     mCardIsPresent = TRUE;
>        goto out;
>     }
>   
>     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
>   
>   out:
> -  CardDetectState = CardDetectCompleted;
> -  return CardIsPresent;
> +  mCardDetectState = CardDetectCompleted;
> +  return mCardIsPresent;
>   }
>   
>   BOOLEAN
> 
> (Compile tested to ensure I didn't screw up the renamings.)
> 
> /
>      Leif
> 
>> +
>> +  CardDetectState = CardDetectInProgress;
>> +  CardIsPresent = FALSE;
>> +
>> +  //
>> +  // The two following commands should succeed even if no card is present.
>> +  //
>> +  Status = SdNotifyState (This, MmcHwInitializationState);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
>> +    // If we failed init, go back to requiring card detection
>> +    CardDetectState = CardDetectRequired;
>> +    return FALSE;
>> +  }
>> +
>> +  Status = SdSendCommand (This, MMC_CMD0, 0);
>> +  if (EFI_ERROR (Status)) {
>> +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: CMD0 Error, Status=%r.\n", Status));
>> +    goto out;
>> +  }
>> +
>> +  //
>> +  // CMD8 should tell us if an SD card is present.
>> +  //
>> +  Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
>> +  if (!EFI_ERROR (Status)) {
>> +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
>> +     CardIsPresent = TRUE;
>> +     goto out;
>> +  }
>> +
>> +  //
>> +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
>> +  //
>> +  Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
>> +  if (!EFI_ERROR (Status)) {
>> +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
>> +     CardIsPresent = TRUE;
>> +     goto out;
>> +  }
>> +
>> +  DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
>> +
>> +out:
>> +  CardDetectState = CardDetectCompleted;
>> +  return CardIsPresent;
>> +}
>> +
>>   BOOLEAN
>>   SdIsMultiBlock (
>>     IN EFI_MMC_HOST_PROTOCOL *This
>> diff --git a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
>> index c558e00bf500..78514a31bc4e 100644
>> --- a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
>> +++ b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
>> @@ -82,6 +82,12 @@ typedef enum _MMC_STATE {
>>       MmcDisconnectState,
>>   } MMC_STATE;
>>   
>> +typedef enum _CARD_DETECT_STATE {
>> +    CardDetectRequired = 0,
>> +    CardDetectInProgress,
>> +    CardDetectCompleted
>> +} CARD_DETECT_STATE;
>> +
>>   #define EMMCBACKWARD         (0)
>>   #define EMMCHS26             (1 << 0)      // High-Speed @26MHz at rated device voltages
>>   #define EMMCHS52             (1 << 1)      // High-Speed @52MHz at rated device voltages
>> -- 
>> 2.21.0.windows.1
>>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection
  2020-07-14 13:49     ` Pete Batard
@ 2020-07-14 14:44       ` Leif Lindholm
  0 siblings, 0 replies; 5+ messages in thread
From: Leif Lindholm @ 2020-07-14 14:44 UTC (permalink / raw)
  To: Pete Batard; +Cc: devel, ard.biesheuvel, awarkentin, Andrei Warkentin

On Tue, Jul 14, 2020 at 14:49:21 +0100, Pete Batard wrote:
> On 2020.07.14 14:25, Leif Lindholm wrote:
> > On Mon, Jul 13, 2020 at 12:16:20 +0100, Pete Batard wrote:
> > > The Raspberry Pi 3 and Pi 4 platforms (with latest EEPROM) can boot
> > > straight from USB, without the need for an SD card being present.
> > > However, the IsCardPresent () calls from the ArasanMmcHost and SdHost
> > > drivers are currently hardwired to return TRUE, which results in
> > > straight to USB boot failing due to the SD drivers looping on
> > > errors while trying to poke at a non-existent card...
> > > 
> > > Ideally, we would use the Card Detect signal from the uSD slot, to
> > > report on the presence or absence of a card, but the Raspberry Pi
> > > Foundation did not wire those signals in the Pi 2 and subsequent
> > > models, leaving us with only potentially interfering SD commands
> > > as means to perform card detection.
> > > 
> > > As a result of this, we are left with no other choice but limit
> > > detection to occurring only once, prior to formal SD card init,
> > > and then return the detected value for subsequent calls. This,
> > > however, is more than good enough for the intended purpose, which
> > > is to allow straight to USB boot. The sequence is a simplified
> > > variant of the identification code in MmcDxe.
> > > 
> > > Tested on Raspberry Pi 2B, 3B and CM3 (for both SD controllers)
> > > and Pi 4 (for Arasan, as that's the only controller available today)
> > > 
> > > Addresses pftf/RPi3#13, pftf/RPi3#14, pftf/RPi4#37.
> > > 
> > > Co-authored-by: Andrei Warkentin <andrey.warkentin@gmail.com>
> > > Signed-off-by: Pete Batard <pete@akeo.ie>
> > 
> > Some minor style comments below, I'm happy to fix them before pushing
> > if you're OK with these:
> 
> I agree with the proposed changes. Thanks for volunteering to fix these.
> 
> I just tested your diff with a Pi 4, for good measure, and everything looks
> good.

Thanks! With that
Reviewed-by: Leif Lindholm <leif@nuviainc.com>
Pushed as 225426271bb0.

> Regards,
> 
> /Pete
> 
> > 
> > > ---
> > >   Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 86 ++++++++++++++++++--
> > >   Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c               | 75 +++++++++++++++--
> > >   Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h               |  6 ++
> > >   3 files changed, 150 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > > index 6d706af6f276..d2a8ffddbb66 100644
> > > --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > > +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > > @@ -11,7 +11,8 @@
> > >   #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
> > > -BOOLEAN PreviousIsCardPresent = FALSE;
> > > +BOOLEAN CardIsPresent = FALSE;
> > > +CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> > 
> > Global variables, so add 'm' prefix?
> > Also, add STATIC (which also matches SdHostDxe version)?
> > 
> > >   UINT32 LastExecutedCommand = (UINT32) -1;
> > >   STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> > > @@ -239,14 +240,6 @@ CalculateClockFrequencyDivisor (
> > >     return EFI_SUCCESS;
> > >   }
> > > -BOOLEAN
> > > -MMCIsCardPresent (
> > > -  IN EFI_MMC_HOST_PROTOCOL *This
> > > -)
> > > -{
> > > -  return TRUE;
> > > -}
> > > -
> > >   BOOLEAN
> > >   MMCIsReadOnly (
> > >     IN EFI_MMC_HOST_PROTOCOL *This
> > > @@ -418,6 +411,10 @@ MMCNotifyState (
> > >     DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
> > > +  // Stall all operations except init until card detection has occurred.
> > > +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> > > +    return EFI_NOT_READY;
> > > +
> > 
> > Add {}?
> > 
> > >     switch (State) {
> > >     case MmcHwInitializationState:
> > >       {
> > > @@ -489,6 +486,77 @@ MMCNotifyState (
> > >     return EFI_SUCCESS;
> > >   }
> > > +BOOLEAN
> > > +MMCIsCardPresent (
> > > +  IN EFI_MMC_HOST_PROTOCOL *This
> > > +)
> > > +{
> > > +  EFI_STATUS Status;
> > > +
> > > +  //
> > > +  // If we are already in progress (we may get concurrent calls)
> > > +  // or completed the detection, just return the current value.
> > > +  //
> > > +  if (CardDetectState != CardDetectRequired)
> > > +    return CardIsPresent;
> > 
> > Add {}?
> > 
> > > +
> > > +  CardDetectState = CardDetectInProgress;
> > > +  CardIsPresent = FALSE;
> > > +
> > > +  //
> > > +  // The two following commands should succeed even if no card is present.
> > > +  //
> > > +  Status = MMCNotifyState (This, MmcHwInitializationState);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> > > +    // If we failed init, go back to requiring card detection
> > > +    CardDetectState = CardDetectRequired;
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  Status = MMCSendCommand (This, MMC_CMD0, 0);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: CMD0 Error, Status=%r.\n", Status));
> > > +    goto out;
> > > +  }
> > > +
> > > +  //
> > > +  // CMD8 should tell us if an SD card is present.
> > > +  //
> > > +  Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> > > +  if (!EFI_ERROR (Status)) {
> > > +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
> > > +     CardIsPresent = TRUE;
> > > +     goto out;
> > > +  }
> > > +
> > > +  //
> > > +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
> > > +  //
> > > +  Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> > > +  if (!EFI_ERROR (Status)) {
> > > +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
> > > +     CardIsPresent = TRUE;
> > > +     goto out;
> > > +  }
> > > +
> > > +  //
> > > +  // SDIO?
> > > +  //
> > > +  Status = MMCSendCommand (This, MMC_CMD5, 0);
> > > +  if (!EFI_ERROR (Status)) {
> > > +     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
> > > +     CardIsPresent = TRUE;
> > > +     goto out;
> > > +  }
> > > +
> > > +  DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
> > > +
> > > +out:
> > > +  CardDetectState = CardDetectCompleted;
> > > +  return CardIsPresent;
> > > +}
> > > +
> > >   EFI_STATUS
> > >   MMCReceiveResponse (
> > >     IN EFI_MMC_HOST_PROTOCOL    *This,
> > > diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > > index 2f31c5eb8c46..aac8b34c4bf4 100644
> > > --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > > +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > > @@ -64,6 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
> > >                                       "genpulses", "writewait2", "?",
> > >                                       "startpowdown" };
> > >   #endif /* NDEBUG */
> > > +STATIC BOOLEAN CardIsPresent = FALSE;
> > > +STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> > 
> > Add 'm' prefix?
> > 
> > >   STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
> > >   STATIC inline BOOLEAN
> > > @@ -264,14 +266,6 @@ SdHostSetClockFrequency (
> > >     return Status;
> > >   }
> > > -STATIC BOOLEAN
> > > -SdIsCardPresent (
> > > -  IN EFI_MMC_HOST_PROTOCOL *This
> > > -  )
> > > -{
> > > -  return TRUE;
> > > -}
> > > -
> > >   STATIC BOOLEAN
> > >   SdIsReadOnly (
> > >     IN EFI_MMC_HOST_PROTOCOL *This
> > > @@ -639,6 +633,10 @@ SdNotifyState (
> > >   {
> > >     DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
> > > +  // Stall all operations except init until card detection has occurred.
> > > +  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> > > +    return EFI_NOT_READY;
> > > +
> > 
> > Add {}?
> > 
> > >     switch (State) {
> > >     case MmcHwInitializationState:
> > >       DEBUG ((DEBUG_MMCHOST_SD, "MmcHwInitializationState\n", State));
> > > @@ -718,6 +716,67 @@ SdNotifyState (
> > >     return EFI_SUCCESS;
> > >   }
> > > +STATIC BOOLEAN
> > > +SdIsCardPresent (
> > > +  IN EFI_MMC_HOST_PROTOCOL *This
> > > +  )
> > > +{
> > > +  EFI_STATUS Status;
> > > +
> > > +  //
> > > +  // If we are already in progress (we may get concurrent calls)
> > > +  // or completed the detection, just return the current value.
> > > +  //
> > > +  if (CardDetectState != CardDetectRequired)
> > > +    return CardIsPresent;
> > 
> > Add {}?
> > 
> > I.e. in total, fold in:
> > diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > index d2a8ffddbb66..88e9126e3549 100644
> > --- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > +++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
> > @@ -11,8 +11,8 @@
> >   #define DEBUG_MMCHOST_SD DEBUG_VERBOSE
> > -BOOLEAN CardIsPresent = FALSE;
> > -CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> > +STATIC BOOLEAN mCardIsPresent = FALSE;
> > +STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
> >   UINT32 LastExecutedCommand = (UINT32) -1;
> >   STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
> > @@ -412,8 +412,9 @@ MMCNotifyState (
> >     DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n", State));
> >     // Stall all operations except init until card detection has occurred.
> > -  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> > +  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
> >       return EFI_NOT_READY;
> > +  }
> >     switch (State) {
> >     case MmcHwInitializationState:
> > @@ -497,11 +498,12 @@ MMCIsCardPresent (
> >     // If we are already in progress (we may get concurrent calls)
> >     // or completed the detection, just return the current value.
> >     //
> > -  if (CardDetectState != CardDetectRequired)
> > -    return CardIsPresent;
> > +  if (mCardDetectState != CardDetectRequired) {
> > +    return mCardIsPresent;
> > +  }
> > -  CardDetectState = CardDetectInProgress;
> > -  CardIsPresent = FALSE;
> > +  mCardDetectState = CardDetectInProgress;
> > +  mCardIsPresent = FALSE;
> >     //
> >     // The two following commands should succeed even if no card is present.
> > @@ -510,7 +512,7 @@ MMCIsCardPresent (
> >     if (EFI_ERROR (Status)) {
> >       DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> >       // If we failed init, go back to requiring card detection
> > -    CardDetectState = CardDetectRequired;
> > +    mCardDetectState = CardDetectRequired;
> >       return FALSE;
> >     }
> > @@ -526,7 +528,7 @@ MMCIsCardPresent (
> >     Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> >     if (!EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SD card detected.\n"));
> > -     CardIsPresent = TRUE;
> > +     mCardIsPresent = TRUE;
> >        goto out;
> >     }
> > @@ -536,7 +538,7 @@ MMCIsCardPresent (
> >     Status = MMCSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> >     if (!EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe MMC card detected.\n"));
> > -     CardIsPresent = TRUE;
> > +     mCardIsPresent = TRUE;
> >        goto out;
> >     }
> > @@ -546,15 +548,15 @@ MMCIsCardPresent (
> >     Status = MMCSendCommand (This, MMC_CMD5, 0);
> >     if (!EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Maybe SDIO card detected.\n"));
> > -     CardIsPresent = TRUE;
> > +     mCardIsPresent = TRUE;
> >        goto out;
> >     }
> >     DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Not detected, Status=%r.\n", Status));
> >   out:
> > -  CardDetectState = CardDetectCompleted;
> > -  return CardIsPresent;
> > +  mCardDetectState = CardDetectCompleted;
> > +  return mCardIsPresent;
> >   }
> >   EFI_STATUS
> > diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > index aac8b34c4bf4..0fd1ac6e8985 100644
> > --- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > +++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
> > @@ -64,8 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode", "readdata",
> >                                       "genpulses", "writewait2", "?",
> >                                       "startpowdown" };
> >   #endif /* NDEBUG */
> > -STATIC BOOLEAN CardIsPresent = FALSE;
> > -STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
> > +STATIC BOOLEAN mCardIsPresent = FALSE;
> > +STATIC CARD_DETECT_STATE mCardDetectState = CardDetectRequired;
> >   STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
> >   STATIC inline BOOLEAN
> > @@ -634,8 +634,9 @@ SdNotifyState (
> >     DEBUG ((DEBUG_MMCHOST_SD, "SdHost: SdNotifyState(State: %d) ", State));
> >     // Stall all operations except init until card detection has occurred.
> > -  if (State != MmcHwInitializationState && CardDetectState != CardDetectCompleted)
> > +  if (State != MmcHwInitializationState && mCardDetectState != CardDetectCompleted) {
> >       return EFI_NOT_READY;
> > +  }
> >     switch (State) {
> >     case MmcHwInitializationState:
> > @@ -727,11 +728,12 @@ SdIsCardPresent (
> >     // If we are already in progress (we may get concurrent calls)
> >     // or completed the detection, just return the current value.
> >     //
> > -  if (CardDetectState != CardDetectRequired)
> > -    return CardIsPresent;
> > +  if (mCardDetectState != CardDetectRequired) {
> > +    return mCardIsPresent;
> > +  }
> > -  CardDetectState = CardDetectInProgress;
> > -  CardIsPresent = FALSE;
> > +  mCardDetectState = CardDetectInProgress;
> > +  mCardIsPresent = FALSE;
> >     //
> >     // The two following commands should succeed even if no card is present.
> > @@ -740,7 +742,7 @@ SdIsCardPresent (
> >     if (EFI_ERROR (Status)) {
> >       DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> >       // If we failed init, go back to requiring card detection
> > -    CardDetectState = CardDetectRequired;
> > +    mCardDetectState = CardDetectRequired;
> >       return FALSE;
> >     }
> > @@ -756,7 +758,7 @@ SdIsCardPresent (
> >     Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> >     if (!EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
> > -     CardIsPresent = TRUE;
> > +     mCardIsPresent = TRUE;
> >        goto out;
> >     }
> > @@ -766,15 +768,15 @@ SdIsCardPresent (
> >     Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> >     if (!EFI_ERROR (Status)) {
> >        DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
> > -     CardIsPresent = TRUE;
> > +     mCardIsPresent = TRUE;
> >        goto out;
> >     }
> >     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
> >   out:
> > -  CardDetectState = CardDetectCompleted;
> > -  return CardIsPresent;
> > +  mCardDetectState = CardDetectCompleted;
> > +  return mCardIsPresent;
> >   }
> >   BOOLEAN
> > 
> > (Compile tested to ensure I didn't screw up the renamings.)
> > 
> > /
> >      Leif
> > 
> > > +
> > > +  CardDetectState = CardDetectInProgress;
> > > +  CardIsPresent = FALSE;
> > > +
> > > +  //
> > > +  // The two following commands should succeed even if no card is present.
> > > +  //
> > > +  Status = SdNotifyState (This, MmcHwInitializationState);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: Error MmcHwInitializationState, Status=%r.\n", Status));
> > > +    // If we failed init, go back to requiring card detection
> > > +    CardDetectState = CardDetectRequired;
> > > +    return FALSE;
> > > +  }
> > > +
> > > +  Status = SdSendCommand (This, MMC_CMD0, 0);
> > > +  if (EFI_ERROR (Status)) {
> > > +    DEBUG ((DEBUG_ERROR, "SdIsCardPresent: CMD0 Error, Status=%r.\n", Status));
> > > +    goto out;
> > > +  }
> > > +
> > > +  //
> > > +  // CMD8 should tell us if an SD card is present.
> > > +  //
> > > +  Status = SdSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
> > > +  if (!EFI_ERROR (Status)) {
> > > +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe SD card detected.\n"));
> > > +     CardIsPresent = TRUE;
> > > +     goto out;
> > > +  }
> > > +
> > > +  //
> > > +  // MMC/eMMC won't accept CMD8, but we can try CMD1.
> > > +  //
> > > +  Status = SdSendCommand (This, MMC_CMD1, EMMC_CMD1_CAPACITY_GREATER_THAN_2GB);
> > > +  if (!EFI_ERROR (Status)) {
> > > +     DEBUG ((DEBUG_INFO, "SdIsCardPresent: Maybe MMC card detected.\n"));
> > > +     CardIsPresent = TRUE;
> > > +     goto out;
> > > +  }
> > > +
> > > +  DEBUG ((DEBUG_INFO, "SdIsCardPresent: Not detected, Status=%r.\n", Status));
> > > +
> > > +out:
> > > +  CardDetectState = CardDetectCompleted;
> > > +  return CardIsPresent;
> > > +}
> > > +
> > >   BOOLEAN
> > >   SdIsMultiBlock (
> > >     IN EFI_MMC_HOST_PROTOCOL *This
> > > diff --git a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> > > index c558e00bf500..78514a31bc4e 100644
> > > --- a/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> > > +++ b/Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h
> > > @@ -82,6 +82,12 @@ typedef enum _MMC_STATE {
> > >       MmcDisconnectState,
> > >   } MMC_STATE;
> > > +typedef enum _CARD_DETECT_STATE {
> > > +    CardDetectRequired = 0,
> > > +    CardDetectInProgress,
> > > +    CardDetectCompleted
> > > +} CARD_DETECT_STATE;
> > > +
> > >   #define EMMCBACKWARD         (0)
> > >   #define EMMCHS26             (1 << 0)      // High-Speed @26MHz at rated device voltages
> > >   #define EMMCHS52             (1 << 1)      // High-Speed @52MHz at rated device voltages
> > > -- 
> > > 2.21.0.windows.1
> > > 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-07-14 14:44 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-13 11:16 [edk2-platforms][PATCH v3 0/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection Pete Batard
2020-07-13 11:16 ` [edk2-platforms][PATCH v3 1/1] " Pete Batard
2020-07-14 13:25   ` Leif Lindholm
2020-07-14 13:49     ` Pete Batard
2020-07-14 14:44       ` Leif Lindholm

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox