From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by mx.groups.io with SMTP id smtpd.web12.19503.1594733140171166082 for ; Tue, 14 Jul 2020 06:25:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=IhEvPDaz; spf=pass (domain: nuviainc.com, ip: 209.85.128.68, mailfrom: leif@nuviainc.com) Received: by mail-wm1-f68.google.com with SMTP id o2so5936542wmh.2 for ; Tue, 14 Jul 2020 06:25:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=EHhpHtvrM99y1U9N5xJDz79F8CZHpFBrC69DFuAHkfI=; b=IhEvPDazYomVr7q6ndN7SFadQaobIFyE3e1FCWZAOinXmJiPfdI/vhbMrc4xhrHoVL pkRweMS1CEtqVb/oGhsMzTBlQlV0D4DS3Yz1H1w1IONzSDqBXQJa2yG7cZPjLq0g8i7S Py2Db9N5K6944N0Sm8YMq2VJeTPQb0TYiaZJA4aUm7nYaAiXri3VlQyUjQ5Srs/OiLy3 0BsS7Oy4xMnBfXZHelItrNTBY+82qgQC9TUQRzgfk/jXcAZLmO9NPXZ6IHxyCMUFDVI8 MNh4M/3zHKYRl0yOle+mdNKe+adxqpahKKqUOOo6lKouAqesnvUBfUar480iyCeTPPUg KDdw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=EHhpHtvrM99y1U9N5xJDz79F8CZHpFBrC69DFuAHkfI=; b=FV9nb93pMKk/ABtCawC2bdheTzoOtP6HrqK00+nMYrAwWJbDDr1Wq5Z6TfWeM3San7 EI7s5Nw2LnC/dZrWWPZQQTdgO+ZnUs57d7/J9mVZ0MtZgtb1aWwX7K9CAOf9su6ML3I5 piFwge+LqxBaF7+wfLlCQMeczYF8gUwl9j24hu4xBXCcR5FYSQMkK/D5BmPnqNkl9rL5 3DnRI/t1aXAzvxAGHTPwhx3Mw704+YgqIS9D93RYvc+UjeKVc9l7we7BtoP9+CLzBi9O sSs2m8ShghcKasSZYMM0Kv/xyr0Sjvxrh1VHPUK1ggtviGIbAdpYM55ZVTc1azgPwy1t m4vQ== X-Gm-Message-State: AOAM533qGzUDJx2HwFymE5vck/GqxeTQtkBOB//FYEm0zHnng+wj6h1/ kP/LWYekI6jYs+JCPVf++P6jCA== X-Google-Smtp-Source: ABdhPJyMXguWvAjh8Sf0ruSPFNiOpTTDY4bh1mYdXlWY7N25ZrLujjc1BPSH3Dmr4reN7lJ6IH3HGQ== X-Received: by 2002:a1c:48d:: with SMTP id 135mr4686567wme.102.1594733137874; Tue, 14 Jul 2020 06:25:37 -0700 (PDT) Return-Path: Received: from vanye ([2001:470:1f09:12f0:b26e:bfff:fea9:f1b8]) by smtp.gmail.com with ESMTPSA id z8sm4556378wmg.39.2020.07.14.06.25.36 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Jul 2020 06:25:36 -0700 (PDT) Date: Tue, 14 Jul 2020 14:25:34 +0100 From: "Leif Lindholm" To: Pete Batard Cc: devel@edk2.groups.io, ard.biesheuvel@arm.com, awarkentin@vmware.com, Andrei Warkentin Subject: Re: [edk2-platforms][PATCH v3 1/1] Platform/RaspberryPi/Drivers: Add SD/(e)MMC card detection Message-ID: <20200714132534.GW12303@vanye> References: <20200713111620.3596-1-pete@akeo.ie> <20200713111620.3596-2-pete@akeo.ie> MIME-Version: 1.0 In-Reply-To: <20200713111620.3596-2-pete@akeo.ie> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 > Signed-off-by: Pete Batard 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 >