Hi Peng,
Thank you for reviewing and for the detailed feedback.
On 5/16/2026 3:14 PM, Peng Fan wrote:
Revisit this patch, since it break one board [1].
[1] https://lore.kernel.org/all/[email protected]/
On Tue, Oct 21, 2025 at 01:45:26PM -0700, Tanmay Kathpalia wrote:
Some boards have SD card connectors where the power rail cannot be switched
off by the driver. However there are various circumstances when a card
might be re-initialized, such as after system resume, warm re-boot, or
error handling. However, a UHS card will continue to use 1.8V signaling
unless it is power cycled.
If the card has not been power cycled, it may still be using 1.8V
signaling. According to the SD spec., the Bus Speed Mode (function group 1)
bits 2 to 4 are zero if the card is initialized at 3.3V signal level. Thus
they can be used to determine if the card has already switched to 1.8V
signaling. Detect that situation and try to initialize a UHS-I (1.8V)
transfer mode.
Signed-off-by: Tanmay Kathpalia <[email protected]>
---
drivers/mmc/mmc.c | 55 ++++++++++++++++++++++++++++++++++++++---------
include/mmc.h | 3 +++
2 files changed, 48 insertions(+), 10 deletions(-)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index ec61ed92e86..e1f62a5d0ad 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -643,6 +643,19 @@ static int mmc_switch_voltage(struct mmc *mmc, int
signal_voltage)
return 0;
}
+
+static bool mmc_sd_card_using_v18(struct mmc *mmc)
+{
+ /*
+ * According to the SD spec., the Bus Speed Mode (function group 1) bits
+ * 2 to 4 are zero if the card is initialized at 3.3V signal level. Thus
+ * they can be used to determine if the card has already switched to
+ * 1.8V signaling.
+ */
+ bool volt = mmc->sd3_bus_mode &
+ (SD_MODE_UHS_SDR50 | SD_MODE_UHS_SDR104 | SD_MODE_UHS_DDR50);
This is wrong.
sd3_bus_mode is sd supported bits, not the current running bits.
To detect the sd card running bits, need to use:
(__be32_to_cpu(switch_status[4]) >> 24) & 0xF
You are correct that sd3_bus_mode stores the supported bits - as can be
seen in sd_get_capabilities() where it is filled from the CMD6 status
response:
mmc->sd3_bus_mode = __be32_to_cpu(switch_status[3]) >> 16 & 0x1f;
However, the intent here is not to read the currently active function
code, but to use the supported bits as a proxy for the signaling voltage
level.
According to the SD Physical Layer Simplified Specification v9.0,
Section 4.3.10.4 (Switch Function Command, CMD6): the UHS-I speed modes
SDR50, SDR104, and DDR50 (function group 1 bits 2–4) are only available
when the card is operating at 1.8V signaling. When the card is
initialized at 3.3V, those bits read as zero. Therefore, if any of bits
2-4 are set in the supported field, we can safely infer the card is a
UHS-I card and, when not power cycled, retains 1.8V signaling.
Using switch_status[4] to read the currently active function would
actually NOT work for this scenario. After a warm reboot, the card
receives CMD0 (GO_IDLE_STATE), which resets the card's selected function
back to 0 (SDR12/default) — even though the 1.8V signaling level is
retained. So switch_status[4] would always return 0 in this path,
making it unsuitable as a 1.8V indicator here.
+ return volt;
+}
#endif
static int sd_send_op_cond(struct mmc *mmc, bool uhs_en)
@@ -1369,9 +1382,6 @@ static int sd_get_capabilities(struct mmc *mmc)
ALLOC_CACHE_ALIGN_BUFFER(__be32, switch_status, 16);
struct mmc_data data;
int timeout;
-#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
- u32 sd3_bus_mode;
-#endif
mmc->card_caps = MMC_MODE_1BIT | MMC_CAP(MMC_LEGACY);
@@ -1451,16 +1461,16 @@ static int sd_get_capabilities(struct mmc *mmc)
if (mmc->version < SD_VERSION_3)
return 0;
- sd3_bus_mode = __be32_to_cpu(switch_status[3]) >> 16 & 0x1f;
- if (sd3_bus_mode & SD_MODE_UHS_SDR104)
+ mmc->sd3_bus_mode = __be32_to_cpu(switch_status[3]) >> 16 & 0x1f;
+ if (mmc->sd3_bus_mode & SD_MODE_UHS_SDR104)
mmc->card_caps |= MMC_CAP(UHS_SDR104);
- if (sd3_bus_mode & SD_MODE_UHS_SDR50)
+ if (mmc->sd3_bus_mode & SD_MODE_UHS_SDR50)
mmc->card_caps |= MMC_CAP(UHS_SDR50);
- if (sd3_bus_mode & SD_MODE_UHS_SDR25)
+ if (mmc->sd3_bus_mode & SD_MODE_UHS_SDR25)
mmc->card_caps |= MMC_CAP(UHS_SDR25);
- if (sd3_bus_mode & SD_MODE_UHS_SDR12)
+ if (mmc->sd3_bus_mode & SD_MODE_UHS_SDR12)
mmc->card_caps |= MMC_CAP(UHS_SDR12);
- if (sd3_bus_mode & SD_MODE_UHS_DDR50)
+ if (mmc->sd3_bus_mode & SD_MODE_UHS_DDR50)
mmc->card_caps |= MMC_CAP(UHS_DDR50);
#endif
@@ -1830,7 +1840,11 @@ static int sd_select_mode_and_width(struct mmc *mmc,
uint card_caps)
uint widths[] = {MMC_MODE_4BIT, MMC_MODE_1BIT};
const struct mode_width_tuning *mwt;
#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
- bool uhs_en = (mmc->ocr & OCR_S18R) ? true : false;
+ /*
+ * Enable UHS mode if the card advertises 1.8V support (S18R in OCR)
+ * or is already operating at 1.8V signaling.
+ */
+ bool uhs_en = (mmc->ocr & OCR_S18R) || mmc_sd_card_using_v18(mmc);
In theory,
bool uhs_en = (mmc->ocr & OCR_S18R) ? true : false; is correct.
OCR (including S18R/S18A) only reflects voltage switch negotiation
capability and intent, not the current signaling voltage. So your sd card
should have OCR_S18R returned per my understanding.
I think there is a gap in understanding here. The scenario this patch
targets is warm reboot or system resume - the card was never power cycled,
so it is still operating at 1.8V signaling.
In that situation, when ACMD41 is re-issued, the card will NOT assert
S18A (Switching to 1.8V Accepted) in the OCR response, because the
voltage negotiation already happened in the previous session and the card
has no need to re-negotiate. Per the SD Physical Layer Simplified
Specification v9.0, S18A is set only during the initial 1.8V request
handshake. A card already running at 1.8V will not set S18A again on a
subsequent ACMD41 after warm reset.
This is exactly the corner case: OCR_S18R/S18A will be zero, yet the card
is already at 1.8V. The existing code misses this entirely.
#else
bool uhs_en = false;
#endif
@@ -2701,6 +2715,27 @@ static int mmc_startup(struct mmc *mmc)
err = sd_get_capabilities(mmc);
if (err)
return err;
+
+#if CONFIG_IS_ENABLED(MMC_UHS_SUPPORT)
+ /*
+ * If the card has already switched to 1.8V signaling, then
+ * set the signal voltage to 1.8V.
+ */
+ if (mmc_sd_card_using_v18(mmc)) {
To switch voltage, ACMD41 is required, see mmc_switch_voltage.
Forcing switch to 1.8 here has some risk.
Agreed - ACMD41 is required to initiate a voltage switch on a card that is
currently at 3.3V. However, in this path the card has already completed
the voltage switch in a prior session and is still running at 1.8V. There
is no card-side voltage transition happening; only the host controller
needs to be reconfigured to match the 1.8V signaling level the card
retained. Sending ACMD41 again in this context would be incorrect, as the
card is not in a state to re-negotiate voltage.
+ /*
+ * During a signal voltage level switch, the clock must
be gated
+ * for 5 ms according to the SD spec.
+ */
+ mmc_set_clock(mmc, mmc->clock, MMC_CLK_DISABLE);
+ err = mmc_set_signal_voltage(mmc,
MMC_SIGNAL_VOLTAGE_180);
+ if (err)
+ return err;
+ /* Keep clock gated for at least 10 ms, though spec
only says 5 ms */
+ mdelay(10);
+ mmc_set_clock(mmc, mmc->clock, MMC_CLK_ENABLE);
+ }
+#endif
A proper redesign is required. So I am going to revert this patch.
I would respectfully ask to reconsider before reverting. This patch is
specifically targeted at the warm reboot / system resume / error recovery
scenario where the card is not power cycled. The check in
mmc_sd_card_using_v18() acts as a guard - it only triggers when the
card is already at 1.8V and the normal OCR path did not catch it. On
a cold boot with a fresh 3.3V card, the UHS supported bits will be zero
and this path is never entered, so it cannot regress cold-boot behavior
for any board.
Regarding the reported regression [1], Judith mentioned that he will
debug once he is back. If possible, let us wait for him to conclude
before deciding to revert.
This problem is not unique to U-Boot. Similar discussions and solutions
have been proposed in the Linux community:
https://lore.kernel.org/linux-mmc/[email protected]/
https://lore.kernel.org/linux-mmc/[email protected]/
I am happy to rework this in v2 with a tighter guard condition (already
addressed in [2]) once we have more information from the regression
report.
Please let me know your thoughts.
Thanks,
Tanmay
