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

[Conor Dooley]

On Fri, Jun 26, 2026 at 02:01:42PM +0800, Yulin Lu wrote:
> Hi, Conor. Thanks for your review.
> 
> > On Mon, Jun 15, 2026 at 05:50:12PM +0530, Pinkesh Vaghela wrote:
> > > From: Yulin Lu <luyulin@xxxxxxxxxxxxxxxxxx>
> > > 
> > > Add pinctrl node and related pin configuration for EIC7700 SoC
> > > 
> > > Co-developed-by: Pritesh Patel <pritesh.patel@xxxxxxxxxxxxxx>
> > > Signed-off-by: Pritesh Patel <pritesh.patel@xxxxxxxxxxxxxx>
> > > Signed-off-by: Yulin Lu <luyulin@xxxxxxxxxxxxxxxxxx>
> > > Signed-off-by: Pinkesh Vaghela <pinkesh.vaghela@xxxxxxxxxxxxxx>
> > > ---
> > >  .../dts/eswin/eic7700-hifive-premier-p550.dts | 109 +++
> > >  .../riscv/boot/dts/eswin/eic7700-pinctrl.dtsi | 888 ++++++++++++++++++
> > >  arch/riscv/boot/dts/eswin/eic7700.dtsi        |   5 +
> > >  3 files changed, 1002 insertions(+)
> > >  create mode 100644 arch/riscv/boot/dts/eswin/eic7700-pinctrl.dtsi
> > > 
> > > 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 1fb92f0e7c55..e7bb96e14958 100644
> > > --- a/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> > > +++ b/arch/riscv/boot/dts/eswin/eic7700-hifive-premier-p550.dts
> > > @@ -6,6 +6,7 @@
> > >  /dts-v1/;
> > >  
> > >  #include "eic7700.dtsi"
> > > +#include "eic7700-pinctrl.dtsi"
> > >  
> 
> ...
> 
> > > +&gpio79_pins {
> > > +	bias-disable;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio80_pins {
> > > +	bias-pull-up;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio82_pins {
> > > +	bias-pull-up;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio84_pins {
> > > +	bias-disable;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio85_pins {
> > > +	bias-pull-up;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio94_pins {
> > > +	bias-disable;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio106_pins {
> > > +	bias-disable;
> > > +	input-disable;
> > > +};
> > > +
> > > +&gpio111_pins {
> > > +	bias-disable;
> > > +	input-disable;
> > > +};
> > > +
> > > +&pinctrl {
> > > +	vrgmii-supply = <&vcc_1v8>;
> > > +};
> > > +
> > >  &uart0 {
> > >  	status = "okay";
> > >  };
> > > 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 000000000000..7293df146aa7
> > > --- /dev/null
> > > +++ b/arch/riscv/boot/dts/eswin/eic7700-pinctrl.dtsi
> > > @@ -0,0 +1,888 @@
> > > +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> > > +/*
> > > + * Copyright (c) 2025 Beijing ESWIN Computing Technology Co., Ltd.
> > > + *
> > > + * ESWIN's EIC7700 SoC pin-mux and pin-config options are listed as
> > > + * device tree nodes in this file.
> > > + *
> > > + * Authors: Yulin Lu <luyulin@xxxxxxxxxxxxxxxxxx>
> > > + */
> > > +
> > 
> > I don't really understand the groups here. I think you should make more
> > effort to put more pins in each group.
> > 
> > > +		gpio1_pins: gpio1-pins {
> > > +			pins = "jtag0_tck";
> > > +			function = "gpio";
> > > +		};
> > > +
> > > +		gpio2_pins: gpio2-pins {
> > > +			pins = "jtag0_tms";
> > > +			function = "gpio";
> > > +		};
> > > +
> > > +		gpio3_pins: gpio3-pins {
> > > +			pins = "jtag0_tdi";
> > > +			function = "gpio";
> > > +		};
> > > +
> > > +		gpio4_pins: gpio4-pins {
> > > +			pins = "jtag0_tdo";
> > > +			function = "gpio";
> > > +		};
> > 
> > Like these 4 for example, why not group these?
> 
> The 'group' is used to correspond to the '-grp' tag in the YAML file and
> has no practical significance.
> Different board designs have different requirements for pin multiplexing.
> Therefore, eic7700-pinctrl.dtsi only provides pins for the board-level DTS.
> Pins are combined and used in the board-level DTS via pinctrl-0 property.

These 4 pins in the driver are represented as:
	EIC7700_PIN(14,  "jtag0_tck",		[0] = F_JTAG, [1] = F_SPI, [2] = F_GPIO),
	EIC7700_PIN(15,  "jtag0_tms",		[0] = F_JTAG, [1] = F_SPI, [2] = F_GPIO),
	EIC7700_PIN(16,  "jtag0_tdi",		[0] = F_JTAG, [1] = F_SPI, [2] = F_GPIO),
	EIC7700_PIN(17,  "jtag0_tdo",		[0] = F_JTAG, [1] = F_SPI, [2] = F_GPIO),

	EIC7700_PIN(18,  "gpio5",		[0] = F_GPIO, [1] = F_SPI),

There is no reason to ever partially use these as GPIO. Either they will
be all jtag, all spi or all gpio. pin 18 on the other than makes sense to have
in a dedicated group.

Attachment: signature.asc
Description: PGP signature