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

[Icenowy Zheng <uwu@icenowy.me>]

在 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