Re: [PATCH 3/7] riscv: dts: eswin: eic7700: add pinctrl support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Dead Code / Missing Pinctrl Hogs
- [High] Invalid Pin Mux Function / Copy-Paste Error
- [High] Missing GPIO to Pinctrl Mapping
- [Low] Device Tree Standard Compliance
--

commit 7180be18ad292ee17c6b942dc71e252ce0fdbeef
Author: Yulin Lu <luyulin@eswincomputing.com>

riscv: dts: eswin: eic7700: add pinctrl support

Add pinctrl node and related pin configuration for EIC7700 SoC.

> diff --git a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> index 1fb92f0e7c550..e7bb96e149585 100644
> --- a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> +++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> @@ -18,6 +19,15 @@ aliases {
>  	chosen {
>  		stdout-path = "serial0:115200n8";
>  	};
> +
> +	vcc_1v8: vcc1v8 {
> +		 compatible = "regulator-fixed";

[Severity: Low]
Should this node be named using the generic regulator-* pattern (such as
regulator-1v8) to avoid dtbs_check warnings for deprecated names?

There is also an extra leading space on the properties within this node
that deviates from standard spacing.

> +		 regulator-name = "vcc1v8";
> +		 regulator-always-on;
> +		 regulator-boot-on;
> +		 regulator-min-microvolt = <1800000>;
> +		 regulator-max-microvolt = <1800000>;
> +	 };
>  };

[ ... ]

> @@ -25,6 +35,105 @@ &xtal {
>  	clock-output-names = "xtal24m";
>  };
>  
> +&gpio0_pins {
> +	bias-disable;
> +	input-enable;
> +};

[Severity: High]
Will these pin configuration overrides be silently ignored by the pinctrl
core during boot?

Since these pin definitions are not referenced by a pinctrl-0 property in
an active device node, and are not configured as pinctrl hogs in the provider,
the intended hardware states for these pins will not be programmed.

[ ... ]

> diff --git a/arch/riscv/boot/dts/eswin/eic7700-pinctrl.dtsi b/arch/riscv/boot/dts/eswin/eic7700-pinctrl.dtsi
> new file mode 100644
> index 0000000000000..7293df146aa7b
> --- /dev/null
> +++ b/arch/riscv/boot/dts/eswin/eic7700-pinctrl.dtsi
> @@ -0,0 +1,888 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)

[ ... ]

> +		hdmi_pins: hdmi-pins {
> +			pins = "hdmi_scl", "hdmi_sda", "hdmi_cec";
> +			function = "hdmi";
> +		};
> +
> +		rgmii0_pins: rgmii0-pins {
> +			pins = "rgmii0_clk_125", "rgmii0_txen", "rgmii0_txclk", "rgmii0_txd0",
> +			       "rgmii0_txd1", "rgmii0_txd2", "rgmii0_txd3", "rgmii0_rxclk",
> +			       "rgmii0_rxdv", "rgmii0_rxd0", "rgmii0_rxd1", "rgmii0_rxd2",
> +			       "rgmii0_rxd3", "rgmii0_mdc", "rgmii0_mdio", "rgmii0_intb";
> +			function = "hdmi";
> +		};

[Severity: High]
Is the hdmi function correct for this network pin group?

It appears that rgmii0, rgmii1, and the i2s pin groups that follow all use
the hdmi function. This will prevent the pin controller from routing these
pins properly, likely leading to driver probe failures when peripherals
attempt to request them.

[ ... ]

> diff --git a/arch/riscv/boot/dts/eswin/eic7700.dtsi b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> index a7ebb11159580..8798c50f7584d 100644
> --- a/arch/riscv/boot/dts/eswin/eic7700.dtsi
> +++ b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> @@ -349,6 +349,11 @@ gpioD: gpio-port@3 {
>  			};
>  		};
>  
> +		pinctrl: pinctrl@51600080 {
> +			compatible = "eswin,eic7700-pinctrl";
> +			reg = <0x0 0x51600080 0x0 0x1fff80>;
> +		};
> +

[Severity: High]
Should the existing gpio controller nodes (gpioA, gpioB, gpioC, and gpioD)
be updated with a gpio-ranges property mapping to this new pinctrl provider?

Without gpio-ranges, the GPIO subsystem cannot translate GPIO offsets into
pinctrl pins. The pinctrl core's .gpio_request_enable callback will not be
invoked when a software component requests a GPIO dynamically, preventing
the multiplexer from automatically switching the pin to GPIO mode.

>  		clk: clock-controller@51828000 {
>  			compatible = "eswin,eic7700-clock";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615122016.1110206-1-pinkesh.vaghela@einfochips.com?part=3