Re: Re: Re: Re: [PATCH 3/7] riscv: dts: eswin: eic7700: add pinctrl support
From: Yulin Lu
Date: Mon Jun 29 2026 - 05:42:23 EST
> > > > 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.
>
> What has -grp got to do with this?
> All I am asking for is for you to do
> gpio_jtag_pins: gpio-jtag-pins {
> pins = "jtag0_tck", "jtag0_tms", "jtag0_tdi", "jtag0_tdo";
> function = "gpio";
> };
>
> and wherever else you can make similar simplifications.
Hi Conor,
Apologies for the confusing reply earlier - the discussion about -grp
was off-topic and I should have addressed your actual question directly.
You are correct that from a mux perspective, these 4 pins will always
be switched as a unit. There is no scenario where jtag0_tck is muxed
to GPIO while jtag0_tms remains as JTAG. The mux function argument
is valid.
However, the individual per-pin groups serve a different purpose: they
act as anchors for board-level DTS overrides of per-pin electrical
configuration. The eic7700-pinctrl.dtsi is a shared library for all
boards built on the EIC7700 SoC. Different boards may repurpose these
GPIO pins for unrelated functions, each requiring its own electrical
configuration. For example:
&gpio1_pins { /* jtag0_tck repurposed as GPIO input */
bias-disable;
input-enable;
};
&gpio2_pins { /* jtag0_tms repurposed as GPIO output */
bias-disable;
input-disable;
};
If we merge them into a single node, a board DTS can only apply one
configuration to all 4 pins simultaneously:
&gpio_jtag_pins {
bias-disable;
input-enable; /* forced onto all 4 pins - no per-pin control */
};
This removes the ability for board DTS files to independently configure
the electrical properties of each pin when they are used as GPIO for
different purposes.
We will send a v2 that also fixes two bugs introduced in this patch:
rgmii0_pins and rgmii1_pins incorrectly specify function = "hdmi"
instead of "rgmii", and i2s0_pins, i2s1_pins, i2s2_pins specify
function = "hdmi" instead of "i2s". These would cause the mux
requests to fail at runtime.
Regards,
Yulin Lu