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.warken...@gmail.com> > > > Signed-off-by: Pete Batard <p...@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 <l...@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/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-