Re: [PATCH 7/9] riscv: dts: ultrarisc: add Rongda M0 board device tree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] Unused hardware blocks (PCIe controllers and UART3) are left enabled without proper configuration.
- [Low] Board-specific power management features (`gpio-poweroff` and `gpio-restart`) are explicitly disabled.
- [Low] Unused PCIe link pin configurations conflict with actively used I2C2 pins.
--

commit 024b81fdfc4200deef00a8435cf80efa58aed943
Author: Jia Wang <wangjia@ultrarisc.com>

riscv: dts: ultrarisc: add Rongda M0 board device tree

This commit adds the device tree for the Rongda M0 mATX motherboard based
on the UltraRISC DP1000 SoC. It configures pin controls, aliases, and
active peripherals like I2C, SPI, UART, and Ethernet.

> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi
> new file mode 100644
> index 0000000000000..101b416b10792
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0-pinctrl.dtsi
> @@ -0,0 +1,85 @@
[ ... ]
> +	i2c2_pins: i2c2-pins {
> +		pins = "PC0", "PC1";
> +		function = "func0";
> +		bias-pull-up;
> +		drive-strength = <33>;
> +	};
[ ... ]
> +	pciex4a_link_pins: pciex4a-link-pins {
> +		pins = "PC0";
> +		function = "func1";
> +		bias-pull-down;
> +		drive-strength = <33>;
> +	};
> +
> +	pciex4b_link_pins: pciex4b-link-pins {
> +		pins = "PC1";
> +		function = "func1";
> +		bias-pull-down;
> +		drive-strength = <33>;
> +	};

Are these pciex4a_link_pins and pciex4b_link_pins configurations needed?
They appear to use pins PC0 and PC1, which are also mapped to i2c2_pins and
actively assigned to i2c2 below.

Since these PCIe link pin configurations are unused in the board device tree
and conflict with the active I2C pins, could they be removed?

> diff --git a/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts
> new file mode 100644
> index 0000000000000..6f72d60ad55e4
> --- /dev/null
> +++ b/arch/riscv/boot/dts/ultrarisc/dp1000-rongda-m0.dts
> @@ -0,0 +1,111 @@
[ ... ]
> +	aliases {
> +		serial0 = &uart0;
> +		serial1 = &uart1;
> +		serial2 = &uart2;
> +		serial3 = &uart3;
> +	};

Is uart3 intended to be active on this board? It is aliased here but does
not appear to be configured with pins or explicitly disabled.

If unused hardware blocks like uart3 or the PCIe controllers are left enabled
in the generic SoC device tree without being explicitly disabled in the board
device tree, could probing them waste resources or expose dangling interfaces?

Should unused IP blocks be explicitly marked with status = "disabled" in
this board-level device tree?

> +
> +	chosen {
> +		stdout-path = "serial0:115200n8";
> +	};
> +
> +	gpio-poweroff {
> +		compatible = "gpio-poweroff";
> +		gpios = <&gpio_b 0 GPIO_ACTIVE_HIGH>;
> +		active-delay-ms = <100>;
> +
> +		status = "disabled";
> +	};
> +
> +	gpio-restart {
> +		compatible = "gpio-restart";
> +		gpios = <&gpio_b 1 GPIO_ACTIVE_HIGH>;
> +		active-delay = <100>;
> +
> +		status = "disabled";
> +	};

Should these gpio-poweroff and gpio-restart nodes be included if they are
explicitly disabled?

If the board is physically wired to support these features, could they be
enabled by removing the status lines? If not, does it make sense to omit
these nodes from the board file entirely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-ultrarisc-pinctrl-v1-0-bf559589ea8a@ultrarisc.com?part=7