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

[Yulin Lu]

> > 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.

Hi, Conor. Thanks for your reply.

For pins 14 to 18, when they are not multiplexed as JTAG or SPI,
they can be used as GPIO. The corresponding -pins combinations
for JTAG, SPI, and GPIO are all implemented in eic7700-pinctrl.dtsi.

Take pin 14 as an example. In eic7700-pinctrl.dtsi, it is defined:

dual_spi2_pins: dual-spi2-pins {
    pins = "spi2_cs0_n", "jtag0_tck", "jtag0_tms", "jtag0_tdi";
    function = "spi";
};

quad_spi2_pins: quad-spi2-pins {
    pins = "spi2_cs0_n", "spi2_cs1_n", "jtag0_tck", "jtag0_tms",
           "jtag0_tdi", "jtag0_tdo", "gpio5";
    function = "spi";
};

gpio1_pins: gpio1-pins {
    pins = "jtag0_tck";
    function = "gpio";
};

jtag0_pins: jtag0-pins {
    pins = "jtag0_tck", "jtag0_tms", "jtag0_tdi", "jtag0_tdo";
    function = "jtag";
};

Which specific combination is called, and how they are combined,
depends on the board-level system design and is referenced in
the device nodes of the board-level DTS. For example:

pinctrl-0 = <&gpio1_pins>, <&jtag2_pins>;

The pin multiplexing configuration depends on the board-level system design.
Using combinations of '-pins' is sufficient to satisfy all requirements,
and there is no need to rely on '-grp' for this purpose.

Regards,
Yulin Lu