[edk2-devel] [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection

Leif Lindholm leif at nuviainc.com
Tue Jul 14 14:44:00 UTC 2020


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 at gmail.com>
> > > Signed-off-by: Pete Batard <pete at 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 at 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
> > > 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#62543): https://edk2.groups.io/g/devel/message/62543
Mute This Topic: https://groups.io/mt/75474585/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-




More information about the edk2-devel-archive mailing list