Re: [PATCH v5 5/5] arm64: dts: renesas: add support for solidrun rzg2lc som and hb-iiot evb

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] SD0_DATA4 through SD0_DATA7 are incorrectly included in the sd0_ctrl_dat03 pinctrl node, defeating the intended overvoltage protection for the 1.8V eMMC.
- [Medium] The usb0_vbus_pins configuration is defined but never referenced, leaving the USB0 VBUS enable pin unconfigured.
- [Medium] The fixed regulator v_1_2 is named for 1.2V but configured for 1.8V, creating a mismatch for the Cypress HX3 USB hub.
--

--- Patch [5]: [PATCH v5 5/5] arm64: dts: renesas: add support for solidrun rzg2lc som and hb-iiot evb ---
commit 99c1d6c6c90388a93caa65e01f5c690e6a6d45a2
Author: Josua Mayer <[email protected]>

arm64: dts: renesas: add support for solidrun rzg2lc som and hb-iiot evb

Add support for the SolidRun RZ/G2LC SoM [1] on Hummingboard IIoT [2].
The RZ/G2LC SoM was designed to be pin compatible to G2L SoM, with
slightly reduced feature set.

[1] https://www.solid-run.com/embedded-industrial-iot/renesas-rz-family/rz-g2lc-som/
[2] https://www.solid-run.com/embedded-industrial-iot/renesas-rz-family/hummingboard-rz-series-sbcs/hummingboard-rz-g2l-iot-sbc/

> diff --git a/arch/arm64/boot/dts/renesas/r9a07g044c2-hummingboard-iiot.dts b/arch/arm64/boot/dts/renesas/r9a07g044c2-hummingboard-iiot.dts
> new file mode 100644
> index 0000000000000..9cc21ae32ed4d
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r9a07g044c2-hummingboard-iiot.dts
> @@ -0,0 +1,20 @@
[ ... ]
> +#include "r9a07g044c2.dtsi"
> +#include "rzg2lc-sr-som.dtsi"
> +#include "rzg2l-hummingboard-iiot-common.dtsi"

[Severity: Medium]
Is the v_1_2 regulator configured incorrectly in rzg2l-hummingboard-iiot-common.dtsi?

The common file defines a regulator named "1v2" with 1.8V limits:

    v_1_2: regulator-1-2 {
        compatible = "regulator-fixed";
        regulator-name = "1v2";
        regulator-max-microvolt = <1800000>;
        regulator-min-microvolt = <1800000>;
    };

This regulator is consumed by the Cypress HX3 USB hub which requires 1.2V
on its vdd-supply. This creates a mismatch that either misrepresents the board
wiring or provides an incorrect constraint to the regulator framework.

> diff --git a/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi
> new file mode 100644
> index 0000000000000..d98d544a0fdc9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/rzg2lc-sr-som.dtsi
> @@ -0,0 +1,414 @@
[ ... ]
> +	sdhi0_4bit_pins: sdhi0-4bit {
> +		sd0_ctrl_dat03 {
> +			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3",
> +			       "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7",
> +			       "SD0_CLK", "SD0_CMD";
> +			power-source = <3300>;
> +		};
> +
> +		/*
> +		 * Pins 4-7 are hard-wired to eMMC with 1.8V IO voltage,
> +		 * with all pins sharing a single voltage domain.
> +		 *
> +		 * Configure as input without pull to avoid overvoltage,
> +		 * and accidental use e.g. as gpio (default pin-function).
> +		 */
> +		sd0_dat47 {
> +			bias-disable;
> +			input-enable;
> +			pins = "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7";
> +			power-source = <3300>;
> +		};
> +	};

[Severity: High]
Does this configuration violate the 1.8V eMMC maximum ratings?

SD0_DATA4 through SD0_DATA7 are included in the sd0_ctrl_dat03 group along with
SD0_DATA0..3 and power-source = <3300>.

When the pinctrl driver processes these subnodes sequentially, it will
configure pins 4-7 without the protective bias-disable and input-enable
properties before proceeding to sd0_dat47. This creates a window during boot
where the SoC's internal pull-ups or output drivers may be active on the eMMC
pins at 3.3V.

[ ... ]
> +	sdhi0_4bit_uhs_pins: sdhi0-4bit-uhs {
> +		sd0_ctrl_dat03 {
> +			pins = "SD0_DATA0", "SD0_DATA1", "SD0_DATA2", "SD0_DATA3",
> +			       "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7",
> +			       "SD0_CLK", "SD0_CMD";
> +			power-source = <1800>;
> +		};
> +
> +		/*
> +		 * Pins 4-7 are hard-wired to eMMC with 1.8V IO voltage,
> +		 * with all pins sharing a single voltage domain.
> +		 *
> +		 * Configure as input without pull to avoid accidental use,
> +		 * e.g. as gpio (default pin-function).
> +		 */
> +		sd0_dat47 {
> +			bias-disable;
> +			input-enable;
> +			pins = "SD0_DATA4", "SD0_DATA5", "SD0_DATA6", "SD0_DATA7";
> +			power-source = <1800>;
> +		};
> +	};

[Severity: High]
Does this node repeat the same configuration conflict?

SD0_DATA4 through SD0_DATA7 are also incorrectly included in the sd0_ctrl_dat03
pinctrl group here, which configures them before the intended sd0_dat47 node.

[ ... ]
> +	usb0_vbus_pins: usb0-vbus {
> +		pinmux = <RZG2L_PORT_PINMUX(4, 0, 1)>; /* USB0_VBUSEN */
> +	};
> +};

[ ... ]

> +&usb2_phy0 {
> +	vbus-supply = <&usb0_vbus_otg>;
> +	status = "okay";
> +};

[Severity: Medium]
Is usb0_vbus_pins unused dead code?

The USB PHY node relies on VBUS but fails to apply the corresponding pin
multiplexing via pinctrl-0 = <&usb0_vbus_pins>. This leaves the VBUS enable
pin unconfigured and prevents the SoC from powering connected USB devices.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5