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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] Board-specific GPIO pin configurations are defined but never referenced via `pinctrl-0`, rendering them inert.
- [Medium] Peripheral nodes do not reference their newly added pinctrl configurations.

Pre-existing issues:
- [High] The SoC device tree lacks `gpio-ranges` properties linking the DW APB GPIO controllers to the newly introduced pinctrl controller, bypassing GPIO muxing logic.
--

commit 92abe836b11b6b811731ac872e0c01dccbccefd0
Author: Yulin Lu <luyulin@eswincomputing.com>

riscv: dts: eswin: eic7700: add pinctrl support

This commit adds pinctrl nodes and related pin configurations for the EIC7700
SoC. It also provides the specific pin states for the HiFive Premier P550
board.

> 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 1266762dfcd8c..743dedbdb081a 100644
> --- a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> +++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
[ ... ]
> @@ -25,6 +35,105 @@ &clock_24m {
>  	clock-output-names = "clock_24m";
>  };
>  
> +&gpio0_pins {
> +	bias-disable;
> +	input-enable;
> +};
> +
> +&gpio5_pins {
> +	bias-disable;
> +	input-enable;
> +};

[Severity: Medium]
Will these board-specific GPIO pin configurations ever be applied?

The board DTS overrides properties like bias-pull-up for various GPIO nodes 
(&gpio0_pins, &gpio14_pins, etc.), but these pin configurations are never 
referenced by any pinctrl-0 property in a consumer device or as a pinctrl hog.

Without being referenced via pinctrl-names = "default"; and pinctrl-0 = <...>;
within the &pinctrl node, the pinctrl subsystem will parse them as templates
but never commit the hardware configurations.

This could leave the hardware lacking required electrical configurations.

[ ... ]
> +&pinctrl {
> +	vrgmii-supply = <&vcc_1v8>;
> +};
> +
>  &uart0 {
>  	status = "okay";
>  };

[Severity: Medium]
Should this peripheral node reference its newly added pinctrl configurations?

The eic7700-pinctrl.dtsi file defines extensive pinmux configurations, such as
uart0_pins and i2c0_pins, and the board DTS enables these peripherals here. 

However, the board DTS fails to assign these pinmux configurations to the 
peripherals using pinctrl-names = "default"; and pinctrl-0 = <&uart0_pins>;.

As a result, peripheral pin multiplexing is not applied by the kernel, relying 
entirely on the bootloader's initial state.

> diff --git a/arch/riscv/boot/dts/eswin/eic7700.dtsi b/arch/riscv/boot/dts/eswin/eic7700.dtsi
> index af9e40d8739ad..a0fb82f4adca9 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]
This isn't a bug introduced by this patch, but does the SoC device tree lack
gpio-ranges properties linking the DW APB GPIO controllers to the newly
introduced pinctrl controller?

The eic7700.dtsi defines GPIO controllers using the snps,dw-apb-gpio driver:

			gpioA: gpio-port@0 {
				compatible = "snps,dw-apb-gpio-port";

This driver unconditionally sets .request = gpiochip_generic_request. 

When a GPIO is requested, gpiochip_generic_request checks if the
gc->gpiodev->pin_ranges list is empty. 

Because eic7700.dtsi fails to define gpio-ranges mapping the gpio-port subnodes
to the pinctrl controller, the pin_ranges list is empty. As a result,
gpiochip_generic_request() returns 0 early and never calls
pinctrl_gpio_request(). 

This bypasses the pinctrl driver's hardware muxing logic entirely, meaning
GPIO hardware muxing will not occur when a GPIO is requested, leaving the pins
in their default or peripheral state.

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