[PATCH] mmc: sd: Handle UHS-I voltage signaling without power cycle

[Peng Fan]

Revisit this patch, since it break one board [1].

[1] https://lore.kernel.org/all/52ec8007-ce50-4f12-b796-4b8c2aa1822e@ti.com/

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 <tanmay.kathpalia at altera.com>
>---
> 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

>+	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.

> #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.

>+			/*
>+			 * 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.

Thanks
Peng