Re: [PATCH v2] arm64: dts: rockchip: enable SD UHS-I on Firefly-RK3399

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] arm64: dts: rockchip: sdmmc UHS-I fails at boot and mismatches IO voltage
- [High] arm64: dts: rockchip: vcc_sdio drives 3.0V onto 1.8V SD IO lines and IO domain
- [High] arm64: dts: rockchip: sdmmc enables UHS-I without vmmc-supply on Firefly-RK3399
- [Medium] arm64: dts: rockchip: vcc_sdio 3.0V cap on rk3399-firefly logs KERN_ERR
--

commit 8066f5f531470e89215d716c8eed028fd1c7fe76
Author: Yaozhong Li <yaozhonguwl@gmail.com>

arm64: dts: rockchip: enable SD UHS-I on Firefly-RK3399

This commit updates the power supplies for sdio0 and sdmmc on the
Firefly-RK3399 board, switching sdio0 vmmc-supply to vcc3v3_sys and
assigning vcc_sdio as vqmmc-supply on sdmmc with sd-uhs-sdr104 enabled.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
> index 0568dfa140b32..fe1f8d75d94d0 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-firefly.dts

[ ... ]

> @@ -804,6 +804,8 @@ &sdmmc {
>  	max-frequency = <150000000>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&sdmmc_clk &sdmmc_cmd &sdmmc_bus4>;
> +	sd-uhs-sdr104;
> +	vqmmc-supply = <&vcc_sdio>;

[Severity: High]
When a UHS-I SD card is present at boot, can enabling sd-uhs-sdr104 and
vqmmc-supply = <&vcc_sdio> on &sdmmc leave the bus in a 3.0V host versus
1.8V card signaling mismatch when fe320000.mmc probes before &io_domains?

In arch/arm64/boot/dts/rockchip/rk3399-firefly.dts, both &io_domains
(sdmmc-supply = <&vcc_sdio>) and &sdmmc (vqmmc-supply = <&vcc_sdio>) are
consumers of vcc_sdio without a supplier-consumer device link ordering
&io_domains before &sdmmc. If &sdmmc runs mmc_rescan() before
rockchip_iodomain_probe() registers its notifier via
regulator_register_notifier(), mmc_sd_get_cid() -> mmc_set_uhs_voltage()
sends CMD11 to switch the card to 1.8V signaling and lowers vcc_sdio to
1.8V while GRF_IO_VSEL for SDMMC0_VDD is still configured for 3.3V logic.

When SDR104 tuning in mmc_sd_init_uhs_card() ->
dw_mci_rk3288_execute_tuning() then fails across all phases, &sdmmc has no
vmmc-supply to cut VDD power to the card during mmc_power_off() or
mmc_power_cycle(). In drivers/mmc/core/sd.c:mmc_sd_init_card():

	if (!v18_fixup_failed && !mmc_host_is_spi(host) && mmc_host_can_uhs(host) &&
	    mmc_sd_card_using_v18(card) &&
	    host->ios.signal_voltage != MMC_SIGNAL_VOLTAGE_180) {
		if (mmc_host_set_uhs_voltage(host) ||
		    mmc_sd_init_uhs_card(card)) {
			v18_fixup_failed = true;
			mmc_power_cycle(host, ocr);
			if (!oldcard)
				mmc_remove_card(card);
			goto retry;
		}
		goto cont;
	}

mmc_power_cycle() -> mmc_power_up() -> mmc_set_initial_signal_voltage()
raises vcc_sdio back to 3.0V without cutting card VDD, and the retry pass
falls through to mmc_sd_switch_hs(), operating at 50 MHz High-Speed with
vcc_sdio and SDMMC0_VDD at 3.0V while the SD card remains latched in 1.8V
signaling mode from CMD11.

[Severity: High]
As reported by Sashiko on v1 [1], when a UHS-I SD card is initialized in
1.8V mode, doesn't system suspend drive 3.0V from vcc_sdio onto the 1.8V
configured SDMMC0_VDD IO domain and 1.8V-latched SD card IO lines?

In arch/arm64/boot/dts/rockchip/rk3399-firefly.dts, vcc_sdio configures its
sleep state to 3.0V:

			vcc_sdio: LDO_REG4 {
				regulator-name = "vcc_sdio";
				regulator-always-on;
				regulator-boot-on;
				regulator-min-microvolt = <1800000>;
				regulator-max-microvolt = <3000000>;
				regulator-state-mem {
					regulator-on-in-suspend;
					regulator-suspend-microvolt = <3000000>;
				};
			};

Once mmc_sd_get_cid() -> mmc_set_uhs_voltage() switches the card and
vcc_sdio to 1.8V, rockchip_iodomain_notify() sets the SDMMC0_VDD bit in
GRF_IO_VSEL to 1.8V mode. During PM_SUSPEND_MEM, _mmc_sd_suspend() ->
mmc_power_off() -> dw_mci_set_ios() does not restore vqmmc to 3.0V and
cannot cut card VDD without a vmmc-supply. When PMIC_SLEEP is asserted, the
RK808 PMIC switches LDO_REG4 to 3.0V via RK808_LDO4_SLP_VSEL_REG without
notifying rockchip-iodomain, applying 3.0V to the 1.8V-configured SoC IO
domain and the powered 1.8V-mode SD card.

[Severity: High]
As reported by Sashiko on v1 [2], because &sdmmc has no vmmc-supply in
arch/arm64/boot/dts/rockchip/rk3399-firefly.dts, how can the kernel
power-cycle the SD card VDD rail after switching the card to 1.8V signaling
via CMD11?

In drivers/mmc/host/dw_mmc.c:dw_mci_set_ios():

	case MMC_POWER_OFF:
		/* Turn clock off before power goes down */
		dw_mci_setup_bus(slot, false);

		if (!IS_ERR(mmc->supply.vmmc))
			mmc_regulator_set_ocr(mmc, mmc->supply.vmmc, 0);

		mmc_regulator_disable_vqmmc(mmc);

Without vmmc-supply, MMC_POWER_OFF cannot cut VDD to reset the card from
1.8V back to 3.3V signaling mode. Across a warm reboot after UHS-I
negotiation (_mmc_sd_suspend() -> mmc_power_off()), the card remains stuck
in 1.8V signaling mode when the BootROM or bootloader tries to reinitialize
the slot at 3.0V/3.3V.

[Severity: Medium]
Does setting vqmmc-supply = <&vcc_sdio> on &sdmmc without a 3.0V
vmmc-supply cause regulator_check_voltage() to log an error on every 3.3V
signal voltage setup?

In arch/arm64/boot/dts/rockchip/rk3399-firefly.dts, vcc_sdio sets
regulator-max-microvolt = <3000000>. Because &sdmmc has no vmmc-supply,
dw_mci_init_host() defaults mmc->ocr_avail to MMC_VDD_32_33 | MMC_VDD_33_34
(3.3V-3.4V).

On every mmc_power_up(), mmc_set_initial_signal_voltage() calls
dw_mci_switch_voltage() -> mmc_regulator_set_vqmmc() with
MMC_SIGNAL_VOLTAGE_330, which invokes regulator_set_voltage_triplet() for
the range 3000000-3600000 uV with a target of 3300000 uV. The first pass in
regulator_set_voltage_triplet() calls regulator_set_voltage(vcc_sdio,
3300000, 3600000), hitting drivers/regulator/core.c:regulator_check_voltage():

	if (*max_uV > rdev->constraints->max_uV)
		*max_uV = rdev->constraints->max_uV;
	if (*min_uV < rdev->constraints->min_uV)
		*min_uV = rdev->constraints->min_uV;

	if (*min_uV > *max_uV) {
		rdev_err(rdev, "unsupportable voltage range: %d-%duV\n",
			 *min_uV, *max_uV);
		return -EINVAL;
	}

This logs an "unsupportable voltage range: 3300000-3000000uV" error before
falling back to 3.0V. Should &sdmmc either provide a 3.0V vmmc-supply or
adjust the vcc_sdio constraints to match?

[1] https://lore.kernel.org/r/20260922055011.14972-1-yaozhonguwl@gmail.com
[2] https://lore.kernel.org/r/20260922055011.14972-1-yaozhonguwl@gmail.com

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105102556.1365-1-yaozhonguwl@gmail.com?part=1