[RFC PATCH 0/3] pinctrl: starfive: jh7110: support force inputs

[RFC PATCH 0/3] pinctrl: starfive: jh7110: support force inputs

[Icenowy Zheng]

The input signals inside the JH7110 SoC (to be routed by the pin
controller) could be routed to GPIOs and internal fixed low/high levels.
As the total GPIO count of JH7110 is not very high, it's sometime
feasible to omit some hardwiring outside the SoC and do them in the pin
controller. One such example is the USB overcurrent_n signal, which
defaults to low at SoC reset, needs to be high for the USB controller to
correctly work (the _n means low indicates overcurrent situation) and
gets omitted on the Pine64 Star64 board.

Add the support for hardwiring GPI signals inside the JH7110 pin
controllers, via two new DT properties. The patchset is tagged RFC
because I personally feel the DT binding a little dirty and not
scalable, but the currently specially-encoded pinmux property makes me
feeling dirty too, and especially dirty if adding virtual pins to mean
internal low/high (and these virtual pins should be sharable among
multiple signals, which seems to be not so desirable for pinconfs).

Icenowy Zheng (3):
  dt-bindings: pinctrl: jh7110-sys: add force inputs
  pinctrl: starfive: jh7110: support forcing inputs to low/high
  riscv: dts: starfive: jh7110-pine64-star64: force no USB overcurrent

 .../pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 10 +++++
 .../dts/starfive/jh7110-pine64-star64.dts     |  2 +
 .../starfive/pinctrl-starfive-jh7110.c        | 43 +++++++++++++++++++
 3 files changed, 55 insertions(+)

-- 
2.49.0

[RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Icenowy Zheng]

The internal inputs routed by the pin controller could be routed not to
any GPIOs but forced low/high in the pin controller, which is a useful
feature for some input signals (e.g. overcurrent_n pin of the USB
controller, which needs to be tied to high if not externally routed).

Add two properties to support forcing inputs to low/high.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 .../bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml  | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
index 222b9e240f8af..d793290afdd74 100644
--- a/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/starfive,jh7110-sys-pinctrl.yaml
@@ -46,6 +46,16 @@ properties:
   '#gpio-cells':
     const: 2
 
+  starfive,force-low-inputs:
+    description:
+      The list of input signals forced to be low inside the SoC itself.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  starfive,force-high-inputs:
+    description:
+      The list of input signals forced to be high inside the SoC itself.
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
 patternProperties:
   '-[0-9]+$':
     type: object
-- 
2.49.0

Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Linus Walleij]

Hi Icenowy,

thanks for your patch!

On Tue, Apr 22, 2025 at 6:23 PM Icenowy Zheng <uwu@icenowy.me> wrote:

> +  starfive,force-low-inputs:
> +    description:
> +      The list of input signals forced to be low inside the SoC itself.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

I don't see why you need this hack.

Use the existing per-pin output-low property (see
Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml).

> +  starfive,force-high-inputs:
> +    description:
> +      The list of input signals forced to be high inside the SoC itself.
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

Use the existing output-high property.

Now I *know* these two properties are per-pin. That is more talkative
but way easier for users to read.

Then use pincontrol hogs to make sure these configs are set up
at probe.

Yours,
Linus Walleij

Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Icenowy Zheng]

于 2025年4月23日 GMT+08:00 17:09:42，Linus Walleij <linus.walleij@linaro.org> 写道：
>Hi Icenowy,
>
>thanks for your patch!
>
>On Tue, Apr 22, 2025 at 6:23 PM Icenowy Zheng <uwu@icenowy.me> wrote:
>
>> +  starfive,force-low-inputs:
>> +    description:
>> +      The list of input signals forced to be low inside the SoC itself.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>
>I don't see why you need this hack.

Unfortunately these properties are not for pins, but internal signals that isn't
bound to external pins.

>
>Use the existing per-pin output-low property (see
>Documentation/devicetree/bindings/pinctrl/pincfg-node.yaml).
>
>> +  starfive,force-high-inputs:
>> +    description:
>> +      The list of input signals forced to be high inside the SoC itself.
>> +    $ref: /schemas/types.yaml#/definitions/uint32-array
>
>Use the existing output-high property.
>
>Now I *know* these two properties are per-pin. That is more talkative
>but way easier for users to read.
>
>Then use pincontrol hogs to make sure these configs are set up
>at probe.
>
>Yours,
>Linus Walleij

Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Linus Walleij]

On Wed, Apr 23, 2025 at 11:41 AM Icenowy Zheng <uwu@icenowy.me> wrote:
> 于 2025年4月23日 GMT+08:00 17:09:42，Linus Walleij <linus.walleij@linaro.org> 写道：
> >Hi Icenowy,
> >
> >thanks for your patch!
> >
> >On Tue, Apr 22, 2025 at 6:23 PM Icenowy Zheng <uwu@icenowy.me> wrote:
> >
> >> +  starfive,force-low-inputs:
> >> +    description:
> >> +      The list of input signals forced to be low inside the SoC itself.
> >> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> >I don't see why you need this hack.
>
> Unfortunately these properties are not for pins, but internal signals that isn't
> bound to external pins.

We don't really care if pins are external or not, we are an operating system
not a philosophy department ;)

You calculate the offset and shift like this and write into a base+offset:

+                       offset = 4 * (pin / 4);
+                       shift  = 8 * (pin % 4);
+
+                       val = readl_relaxed(sfp->base +
+                                           info->gpi_reg_base + offset);

Compare to jh7110_pin_dbg_show():

               unsigned int offset = 4 * (pin / 4);
                unsigned int shift  = 8 * (pin % 4);
                u32 dout = readl_relaxed(sfp->base +
info->dout_reg_base + offset);
                u32 doen = readl_relaxed(sfp->base +
info->doen_reg_base + offset);

So clearly the entities that you affect are in the same numberspace,
and that is all we care about. They are not enumerated in any way
orthogonal to any other pins AFAICT.

Both pin control and GPIO handle chip-internal lines that are not
routed outside sometimes, that's fine. Just deal with them as any other
"pins".

Yours,
Linus Walleij

Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Icenowy Zheng]

在 2025-04-23星期三的 12:55 +0200，Linus Walleij写道：
> On Wed, Apr 23, 2025 at 11:41 AM Icenowy Zheng <uwu@icenowy.me>
> wrote:
> > 于 2025年4月23日 GMT+08:00 17:09:42，Linus Walleij
> > <linus.walleij@linaro.org> 写道：
> > > Hi Icenowy,
> > > 
> > > thanks for your patch!
> > > 
> > > On Tue, Apr 22, 2025 at 6:23 PM Icenowy Zheng <uwu@icenowy.me>
> > > wrote:
> > > 
> > > > +  starfive,force-low-inputs:
> > > > +    description:
> > > > +      The list of input signals forced to be low inside the
> > > > SoC itself.
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > > 
> > > I don't see why you need this hack.
> > 
> > Unfortunately these properties are not for pins, but internal
> > signals that isn't
> > bound to external pins.
> 
> We don't really care if pins are external or not, we are an operating
> system
> not a philosophy department ;)
> 
> You calculate the offset and shift like this and write into a
> base+offset:
> 
> +                       offset = 4 * (pin / 4);
> +                       shift  = 8 * (pin % 4);
> +
> +                       val = readl_relaxed(sfp->base +
> +                                           info->gpi_reg_base +
> offset);
> 
> Compare to jh7110_pin_dbg_show():
> 
>                unsigned int offset = 4 * (pin / 4);
>                 unsigned int shift  = 8 * (pin % 4);
>                 u32 dout = readl_relaxed(sfp->base +
> info->dout_reg_base + offset);
>                 u32 doen = readl_relaxed(sfp->base +
> info->doen_reg_base + offset);
> 
> So clearly the entities that you affect are in the same numberspace,
> and that is all we care about. They are not enumerated in any way
> orthogonal to any other pins AFAICT.

They just share the field width, they're not in the same numberspace.

The design of the JH7110 pin mux control is quite simple and stupid:

- First per-GPIO map configuration to map the GPIO's DOEn pin to
internal tri-stating signals.
- Then per-GPIO map configuration to map the GPIO's DOUT pin to
internal output signals.
- Then per-input-signal configuration (note that it's no longer per-
GPIO) map configuration to map the signal to a GPIO's DIN (or fixed
low/high).

For examples, here's some field names of a few registers (according to
the TRM):
0x0 [5:0]: sys_iomux_gpo0_doen_cfg
0x40 [6:0]: sys_iomux_gpo0_dout_cfg
0x80 [6:0]: sys_iomux_gpi_u0_WAVE511_i_­uart_rxsin_cfg

P.S. I just now found that the vendor's 6.12 BSP (which uses the
mainlined pinctrl driver) has pinmux configuration that maps the same
GPIO pin (GPIO38) to two input signals (both BCLK signals of two I2S
controllers, one RX and one TX) [1]. This makes me think use two
virtual pad numbers (one for internal low level, another for internal
high) for these internal connections viable again -- In this case I can
write `GPIOMUX(PAD_INTERNAL_HIGH, GPOUT_LOW, GPOEN_DISABLE,
GPI_SYS_USB_OVERCURRENT)` .

[1]
https://github.com/starfive-tech/linux/blob/JH7110_VisionFive2_6.12.y_devel/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-2.dtsi#L298

> 
> Both pin control and GPIO handle chip-internal lines that are not
> routed outside sometimes, that's fine. Just deal with them as any
> other
> "pins".
> 
> Yours,
> Linus Walleij

Re: [RFC PATCH 1/3] dt-bindings: pinctrl: jh7110-sys: add force inputs

[Linus Walleij]

On Wed, Apr 23, 2025 at 4:22 PM Icenowy Zheng <uwu@icenowy.me> wrote:

> > So clearly the entities that you affect are in the same numberspace,
> > and that is all we care about. They are not enumerated in any way
> > orthogonal to any other pins AFAICT.
>
> They just share the field width, they're not in the same numberspace.

OK I trust you on this, I just had to put a bit of pressure so we try
to stay with standard bindings.

> The design of the JH7110 pin mux control is quite simple and stupid:
>
> - First per-GPIO map configuration to map the GPIO's DOEn pin to
> internal tri-stating signals.
> - Then per-GPIO map configuration to map the GPIO's DOUT pin to
> internal output signals.
> - Then per-input-signal configuration (note that it's no longer per-
> GPIO) map configuration to map the signal to a GPIO's DIN (or fixed
> low/high).

I get it, I think.

So if I understand correctly this set-up is necessary to use any one
pin as a GPIO pin?

In that case, consider that this must probably be deeply integrated
with the GPIO subsystem rather than the pin control subsystem.

For example GPIO usually has this:

gpio-ranges = <&pinctrl1 0 20 10>, <&pinctrl2 10 50 20>;

Indicating which pins are actually routed as GPIO and implicitly
contains the information you need as to which pins are
affected.

There is also

gpio-reserved-ranges = <0 4>, <12 2>;

that can be used to say certain number ranges in the GPIO
controller can *not* be used for GPIO.

This type of inferred information should ideally be used to
infer the configuration rather than hardcoded properties.

And if you have pin control as a back-end to GPIO, the
callbacks in struct pinmux_ops:

        int (*gpio_request_enable) (struct pinctrl_dev *pctldev,
                                    struct pinctrl_gpio_range *range,
                                    unsigned int offset);
        void (*gpio_disable_free) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned int offset);
        int (*gpio_set_direction) (struct pinctrl_dev *pctldev,
                                   struct pinctrl_gpio_range *range,
                                   unsigned int offset,
                                   bool input);

which you could implement in jh7110_pinmux_ops to get
a tighter connection between you pinmux and GPIO controller
portions.

I have a strong feeling that the missing piece is using these
callbacks along with the gpio-ranges to connect the GPIO and
pin mux systems together so that you can set this stuff up
in the above callbacks instead.

That would save you the weird DT properties that will be a real
pain to keep in sync with the actual use.

Yours,
Linus Walleij

[RFC PATCH 2/3] pinctrl: starfive: jh7110: support forcing inputs to low/high

[Icenowy Zheng]

The internal input signals inside JH7110 SoC (output of the pin
controller) could be routed to fixed low/high level instead of external
GPIOs.

Support this feature in the pinctrl driver, which is quite useful for
some peripherals (e.g. USB) because of the low GPIO count of JH7110.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 .../starfive/pinctrl-starfive-jh7110.c        | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
index 1d0d6c224c104..5652f9bb658a2 100644
--- a/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
+++ b/drivers/pinctrl/starfive/pinctrl-starfive-jh7110.c
@@ -837,6 +837,41 @@ static void jh7110_disable_clock(void *data)
 	clk_disable_unprepare(data);
 }
 
+static int jh7110_force_input_pins(struct jh7110_pinctrl *sfp,
+				   const char *property, u32 forced_input)
+{
+	int i, nforce;
+	int ret;
+	u32 pin, val;
+	unsigned int offset, shift;
+	struct device *dev = sfp->dev;
+	const struct jh7110_pinctrl_soc_info *info = sfp->info;
+
+	nforce = of_property_count_u32_elems(dev->of_node, property);
+
+	if (nforce > 0) {
+		for (i = 0; i < nforce; i++) {
+			ret = of_property_read_u32_index(dev->of_node, property,
+							 i, &pin);
+			if (ret)
+				return ret;
+
+			offset = 4 * (pin / 4);
+			shift  = 8 * (pin % 4);
+
+			val = readl_relaxed(sfp->base +
+					    info->gpi_reg_base + offset);
+			val &= info->gpi_mask << shift;
+			val |= (forced_input & info->gpi_mask) << shift;
+
+			writel_relaxed(val, sfp->base +
+					    info->gpi_reg_base + offset);
+		}
+	}
+
+	return 0;
+}
+
 int jh7110_pinctrl_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -964,6 +999,14 @@ int jh7110_pinctrl_probe(struct platform_device *pdev)
 
 	dev_info(dev, "StarFive GPIO chip registered %d GPIOs\n", sfp->gc.ngpio);
 
+	ret = jh7110_force_input_pins(sfp, "starfive,force-low-inputs", 0);
+	if (ret)
+		return ret;
+
+	ret = jh7110_force_input_pins(sfp, "starfive,force-high-inputs", 1);
+	if (ret)
+		return ret;
+
 	return pinctrl_enable(sfp->pctl);
 }
 EXPORT_SYMBOL_GPL(jh7110_pinctrl_probe);
-- 
2.49.0

[RFC PATCH 3/3] riscv: dts: starfive: jh7110-pine64-star64: force no USB overcurrent

[Icenowy Zheng]

The Star64 board has no GPIOs to indicate USB overcurrent, however the
USB controller would stop to work if the overcurrent_n signal it gets is
low (which means overcurrent situations happening because of the _n).

Use the pin controller to force the overcurrent_n signal to be high in
order to ensure stable behavior of the USB controller.

Signed-off-by: Icenowy Zheng <uwu@icenowy.me>
---
 arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
index 31e825be2065a..dab0e0054e648 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-pine64-star64.dts
@@ -81,6 +81,8 @@ &spi0 {
 };
 
 &sysgpio {
+	starfive,force-high-inputs = <GPI_SYS_USB_OVERCURRENT>;
+
 	usb0_pins: usb0-0 {
 		vbus-pins {
 			pinmux = <GPIOMUX(25,  GPOUT_SYS_USB_DRIVE_VBUS,
-- 
2.49.0