Re: [PATCH v2 1/3] dt-bindings: pinctrl: starfive,jh7110: add PAD_INTERNAL_* virtual pins

[Icenowy Zheng]

在 2025-04-24星期四的 01:15 -0700，E Shattow写道：
> On 4/23/25 23:20, Icenowy Zheng wrote:
> > The JH7110 SoC could support internal GPI signals to be routed to
> > not
> > external GPIO but internal low/high levels.
> > 
> > Add two macros, PAD_INTERNAL_LOW and PAD_INTERNAL_HIGH, as two
> > virtual
> > "pads" to represent internal GPI sources with fixed low/high
> > levels.
> > 
> > Signed-off-by: Icenowy Zheng <uwu@xxxxxxxxxx>
> > ---
> >  include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> > b/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> > index 3865f01396395..3cca874b2bef7 100644
> > --- a/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> > +++ b/include/dt-bindings/pinctrl/starfive,jh7110-pinctrl.h
> > @@ -126,6 +126,10 @@
> >  #define        PAD_GMAC0_TXEN          18
> >  #define        PAD_GMAC0_TXC           19
> >  
> > +/* virtual pins for forcing GPI */
> > +#define PAD_INTERNAL_LOW       254
> > +#define PAD_INTERNAL_HIGH      255
> > +
> >  #define GPOUT_LOW              0
> >  #define GPOUT_HIGH             1
> >  
> 
> Asking about the choice of 255 and 254 values for virtual high/low
> pins,
> here. There's not much result when grep Linux source for 'virtual
> pin'
> to compare with. Are these the best values for this approach?

These two values are picked because the following reasons:

- The pin field has 8 bits (see the comments of jh7110_pinmux_din() in
pinctrl-starfive-jh7110.c)
- We are already using values 0 and 1 for GPIO0/GPIO1

If we're designing from scratch, it's possible to have another practice
by using 0 and 1 for internal low/high and 2 for gpio0 so on.

> 
> What happens when devicetree has in it to route PAD_INTERNAL_LOW to
> PAD_INTERNAL_HIGH and other unlikely combinations?  Or a devicetree
> blob
> with this computed value is paired to Linux kernel that does not have
> the code to handle these virtual pins, for compatibility concern?

I think it's not supported for newer DTs to be compatible with old
kernels, but I analyzed the code, a read-out-of-bound could happen in
jh7110_set_function() in pinctrl-starfive-jh7110-sys.c . Well this is
unfortunate, but we can do few things to fix old kernels -- we can fix
the problem in newer kernels.

And even picking other values cannot make things better...

> 
> Do we know yet if JH8100 will share some of this design?

We don't know yet whether JH8100 can exist.

> 
> -E