Re: [PATCH v2 2/4] arm64: dts: renesas: add support for solidrun rzg2l som and hb-iiot evb

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] USB VBUS pinmux nodes are defined but never referenced, breaking USB host power.
- [High] Edge-triggered interrupt for TCAL6416 GPIO expander leads to lost interrupts.
- [High] Inverse polarity on `rfkill-m2-b-gnss` disable GPIO will disable the GPS when it should be enabled.
- [High] Omitted `GPIO_OPEN_DRAIN` flag on I2C mux reset line forces dangerous push-pull drive.
- [Medium] Missing SDIO interrupt capabilities on WiFi MMC interface degrades performance.
- [Medium] Pinctrl strict locking prevents documented manual RS485 toggling.
- [Medium] Contradictory regulator polarity flags in `vmmc` create ambiguous regulator state.
- [Medium] Unimplemented internal pull-ups on GPIO hogs lead to floating open-drain lines.
- [Low] Improper append operator `+=` used for multi-blob DTB definitions in Makefile.
--

commit 264a692a0b295dedac1ff5214c5f24eb1c644647
Author: Josua Mayer <josua@solid-run.com>

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

This commit adds support for the SolidRun RZ/G2L SoM on Hummingboard IIoT,
including definitions for Ethernet, eMMC, DDR, and WiFi/Bluetooth.
Descriptions for eMMC, microSD, and RS485 are provided as overlays due to
their dependency on configurable mux states.

[1] https://www.solid-run.com/embedded-industrial-iot/renesas-rz-family/rz-g2l-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/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index ca45d2857ea7f..37cfefe66a308 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -169,6 +169,19 @@ dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-smarc-cru-csi-ov5645.dtbo
>  r9a07g044c2-smarc-cru-csi-ov5645-dtbs := r9a07g044c2-smarc.dtb r9a07g044c2-smarc-cru-csi-ov5645.dtbo
>  dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044c2-smarc-cru-csi-ov5645.dtb
>  
> +dtb-$(CONFIG_ARCH_R9A07G044) += r9a07g044l2-hummingboard-iiot.dtb
> +dtb-$(CONFIG_ARCH_R9A07G044) += rzg2l-sr-som-emmc.dtbo
> +r9a07g044l2-hummingboard-iiot-emmc-dtbs += r9a07g044l2-hummingboard-iiot.dtb rzg2l-sr-som-emmc.dtbo

Is it safer to use the exact assignment operator := instead of the append
operator += for composite device tree blobs? Standard Kbuild convention
dictates using := for <target>-dtbs to ensure immediate evaluation and avoid
any risk of build pollution from inherited environment variables.

[ ... ]
> diff --git a/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-common.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-common.dtsi
> new file mode 100644
> index 0000000000000..ff2c42bcffe2f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-common.dtsi
[ ... ]
> +	vmmc: regulator-mmc {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vmmc";
> +		regulator-max-microvolt = <3300000>;
> +		regulator-min-microvolt = <3300000>;
> +		startup-delay-us = <250>;
> +		vin-supply = <&v_3_3>;
> +		gpio = <&pinctrl RZG2L_GPIO(4, 1) GPIO_ACTIVE_LOW>;
> +		enable-active-high;
> +	};

Mixing enable-active-high and GPIO_ACTIVE_LOW creates an ambiguous
configuration. The fixed regulator driver interprets enable-active-high as a
physical active-high requirement, but modern gpiolib automatically handles
the GPIO_ACTIVE_LOW inversion at the descriptor level.

Could this conflicting double-inversion cause the regulator to be driven to
the wrong physical state and fail to power the eMMC module?

[ ... ]
> +	rfkill-m2-b-gnss {
> +		compatible = "rfkill-gpio";
> +		/* rfkill-gpio inverts internally */
> +		shutdown-gpios = <&tca6416_u20 10 GPIO_ACTIVE_HIGH>;
> +		label = "m2-b gnss";
> +		radio-type = "gps";
> +	};

The rfkill-gpio driver asserts shutdown_gpios (drives a logical 1) when the
radio is turned on. With GPIO_ACTIVE_HIGH, a logical 1 maps to a physical
HIGH signal. 

Since the pin is an active-low enable (GPS_EN#), driving it physically HIGH
will disable the GPS. Does this cause the kernel to disable the GPS radio
when it attempts to enable it? Should the polarity be GPIO_ACTIVE_LOW?

[ ... ]
> +	tca6416_u21: gpio@21 {
> +		compatible = "ti,tcal6416";
> +		reg = <0x21>;
> +		#interrupt-cells = <2>;
> +		interrupt-controller;
> +		#gpio-cells = <2>;
> +		gpio-controller;
> +		gpio-line-names = "TCA_SPI_TPM/EXT", "TCA_TPM_RST#",
> +				  "TCA_I2C_RST", "TCA_RS232_SHTD#",
> +				  "TCA_LCD_I2C_RST", "TCA_DIG_OUT1",
> +				  "TCA_bDIG_IN1", "TCA_SENS_INT",
> +				  "TCA_ALERT#", "TCA_TPM_PIRQ#",
> +				  "TCA_RTC_INT", "TCA_M.2M_WAKW_ON_LAN",
> +				  "TCA_M.2M_CLKREQ#", "TCA_LVDS_INT#",
> +				  "", "TCA_POE_AT";
> +		interrupts-extended = <&pinctrl RZG2L_GPIO(4, 0) IRQ_TYPE_EDGE_FALLING>;

The hardware INT pin for the TCAL6416 GPIO expander is an active-low
level-sensitive output that remains low as long as unread state changes
exist. 

If this is configured as IRQ_TYPE_EDGE_FALLING, could the kernel miss
subsequent interrupts if a second GPIO state change occurs before the first
is cleared, since the line will remain low without generating a new falling
edge? Should this be IRQ_TYPE_LEVEL_LOW instead?

> +
> +		lcd-i2c-reset-hog {
> +			gpios = <4 (GPIO_ACTIVE_LOW|GPIO_PULL_UP|GPIO_OPEN_DRAIN)>;

The ti,tcal6416 chip is driven by gpio-pca953x, which does not implement the
.set_config callback for this specific IC because it lacks the PCA_PCAL
hardware flag. 

Will gpiolib silently ignore the GPIO_PULL_UP flag? For open-drain lines
like the lcd reset and the TPM interrupt below, could this leave the lines
floating and lead to spurious interrupts or unstable hardware states if
there are no external board-level pull-ups?

[ ... ]
> +	i2c-mux@70 {
> +		compatible = "nxp,pca9546";
> +		reg = <0x70>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		/*
> +		 * This reset is open drain,
> +		 * but reset core does not support GPIO_OPEN_DRAIN flag.
> +		 */
> +		reset-gpios = <&tca6416_u21 2 GPIO_ACTIVE_LOW>;

The comment mentions that the reset core does not support the
GPIO_OPEN_DRAIN flag, but the i2c-mux-pca954x driver requests the GPIO using
devm_gpiod_get_optional(), which fully supports device tree flags.

By omitting the flag, does the GPIO controller get forced to drive the pin
in push-pull mode? Could actively driving a hard-wired open-drain line high
cause short circuits or component damage?

[ ... ]
> diff --git a/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-rs485-a.dtso b/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-rs485-a.dtso
> new file mode 100644
> index 0000000000000..886f4d4ecd67f
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-hummingboard-iiot-rs485-a.dtso
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/*
> + * Copyright 2025 Josua Mayer <josua@solid-run.com>
> + *
> + * Overlay for enabling HummingBoard IIoT on-board RS485 Port A on connector J5004.
> + *
> + * Because Renesas uart driver does not support rs485,
> + * users must manually toggle P41_1 between RX & TX.

The overlay below adds <&scif1_rts_pins> to the pinctrl-0 property of the
UART controllers. Since the Renesas RZ/G2L pinctrl driver operates in strict
mode, it will exclusively claim these RTS pins.

Will this prevent userspace from subsequently requesting these pins as GPIOs
for the documented manual toggling, causing requests to be rejected with
-EBUSY?

> + */
> +
> +/dts-v1/;
> +/plugin/;
> +
> +&scif1 {
> +	pinctrl-0 = <&scif1_pins>, <&scif1_rts_pins>;
> +};
[ ... ]
> diff --git a/arch/arm64/boot/dts/renesas/rzg2l-sr-som.dtsi b/arch/arm64/boot/dts/renesas/rzg2l-sr-som.dtsi
> new file mode 100644
> index 0000000000000..1d52530dcf43c
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/rzg2l-sr-som.dtsi
[ ... ]
> +&sdhi1 {
> +	/* Murata 1MW max rate is 50MHz */
> +	max-frequency = <50000000>;
> +	bus-width = <4>;
> +	mmc-pwrseq = <&sdhi1_pwrseq>;
> +	non-removable;
> +	no-1-8-v;
> +	no-sd;
> +	pinctrl-0 = <&sdhi1_pins>;
> +	pinctrl-names = "default";
> +	vmmc-supply = <&reg_pmic_buck4>;
> +	/*
> +	 * Host controller IO voltage is provided from reg_pmic_ldo2,
> +	 * WiFi module IO voltage from reg_pmic_buck4.
> +	 * Neither is configurable at run-time so either can be set here.
> +	 */
> +	vqmmc-supply = <&reg_pmic_ldo2>;
> +	status = "okay";
> +};

The sdhi1 node is designated for an on-board WiFi module but omits the
cap-sdio-irq property and lacks a child node defining an out-of-band
interrupt. 

Will the MMC core fall back to SDIO polling without this capability
declared, and could this degrade I/O performance and inflate CPU utilization?

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

The usb0_vbus_pins and usb1_vbus_pins nodes are defined in the pinctrl
section earlier in this file to configure the USB VBUS enable pins, but
neither &usb2_phy0 nor &usb2_phy1 appears to reference them via a pinctrl-0
property.

Without being multiplexed, will the USB host controllers be able to drive
VBUS power to connected devices?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-rzg2-sr-boards-v2-0-82aebbd27891@solid-run.com?part=2