Re: [PATCH 2/3] arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] arm64: dts: renesas: rzg3l-smarc-som: sdhi0 pinctrl fails with
  -EINVAL
- [Low] arm64: dts: renesas: rzg3l-smarc-som uses RZG2L_PORT_PINMUX for
  SD0_CD
--

commit e86cb26292885aecdb37f14368a14d0f62f6aee6
Author: Biju Das <biju.das.jz@bp.renesas.com>

arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0

This commit enables SD card or eMMC support on SDHI0 for the RZ/G3L SMARC
SoM, selected via the SW_SD0_DEV_SEL macro.

> diff --git a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
> index 091a227233cba..f6f4eed9a2ea6 100644
> --- a/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi
> +++ b/arch/arm64/boot/dts/renesas/rzg3l-smarc-som.dtsi

[ ... ]

> @@ -216,7 +234,132 @@ i2c0_pins: i2c0 {
>  		pinmux = <RZG3L_PORT_PINMUX(L, 2, 4)>, /* RIIC0_SCL */
>  			 <RZG3L_PORT_PINMUX(L, 3, 4)>; /* RIIC0_SDA */
>  	};
> +
> +	sd0-pwr-en-hog {
> +		gpio-hog;
> +		gpios = <RZG3L_GPIO(5, 1) GPIO_ACTIVE_HIGH>;
> +		output-high;
> +		line-name = "sd0_pwr_en";
> +	};
> +
> +	sdhi0_emmc_pins: sd0-emmc {
> +		sd0-clk {
> +			pins = "SD0_CLK";
> +			power-source = <1800>;
> +			drive-strength-microamp = <9000>;
> +		};

[Severity: High]
Does configuring power-source on the SD0_* pins cause sdhi0 probe to fail
with -EINVAL?

In drivers/pinctrl/renesas/pinctrl-rzg2l.c, the SD0_* entries in
rzg3l_dedicated_pins do not set PIN_CFG_IO_VMC_SD0 or PIN_CFG_SOFT_PS:

drivers/pinctrl/renesas/pinctrl-rzg2l.c:rzg3l_dedicated_pins[] {
    ...
	{ "SD0_CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0, PIN_CFG_IOLH_B) },
	{ "SD0_CMD", RZG2L_SINGLE_PIN_PACK(0x9, 1,
	  (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
    ...
}

When sdhi0 binds its pinctrl states (sdhi0_emmc_pins, sdhi0_usd_pins, or
sdhi0_usd_uhs_pins), rzg2l_pinctrl_pinconf_set() -> rzg2l_set_power_source()
-> rzg2l_caps_to_pwr_reg() returns -EINVAL, preventing SDHI0 from probing in
both eMMC and uSD modes.  (Dropping power-source from DT while keeping
drive-strength-microamp would instead leave the cached power_source at 0,
causing rzg2l_ps_to_iolh_idx(0) to return RZG2L_IOLH_IDX_MAX and read past
the end of hwcfg->iolh_groupb_ua[] in rzg2l_ds_is_supported().)

[ ... ]

> +	sdhi0_usd_pins: sd0-usd {
> +		sd0-cd {
> +			pinmux = <RZG2L_PORT_PINMUX(5, 0, 8)>; /* SD0_CD */
> +		};

[Severity: Low]
This isn't a bug since RZG3L_P5 expands to 5, but sd0-cd in both
sdhi0_usd_pins and sdhi0_usd_uhs_pins uses RZG2L_PORT_PINMUX(5, 0, 8)
instead of RZG3L_PORT_PINMUX(5, 0, 8) like the rest of this file.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112-sdhi_rzg3l-v1-0-ca30b209615d@bp.renesas.com?part=2