RE: [PATCH v2 1/4] dt-bindings: pinctrl: renesas: Document RZ/G3E SoC

[Biju Das <biju.das.jz@xxxxxxxxxxxxxx>]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 12 December 2024 16:27
> Subject: Re: [PATCH v2 1/4] dt-bindings: pinctrl: renesas: Document RZ/G3E SoC
> 
> Hi Biju,
> 
> On Fri, Dec 6, 2024 at 11:23 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Add documentation for the pin controller found on the Renesas RZ/G3E
> > (R9A09G047) SoC. The RZ/G3E PFC is similar to the RZ/V2H SoC but has
> > more pins(P00-PS3). The port number is alpha-numeric compared to the
> > number on the other SoCs. So add macros for alpha-numeric to number conversion.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v1->v2:
> >  * Fixed the warnings reported by bot.
> 
> Thanks for the update!
> 
> > ---
> > a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
> > +++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.
> > +++ yaml
> 
> The changes to the bindings LGTM.
> 
> > diff --git a/include/dt-bindings/pinctrl/rzg2l-pinctrl.h
> > b/include/dt-bindings/pinctrl/rzg2l-pinctrl.h
> > index c78ed5e5efb7..1b1b1114a84c 100644
> > --- a/include/dt-bindings/pinctrl/rzg2l-pinctrl.h
> > +++ b/include/dt-bindings/pinctrl/rzg2l-pinctrl.h
> > @@ -11,13 +11,38 @@
> >
> >  #define RZG2L_PINS_PER_PORT    8
> >
> > +#define RZG3E_P0               0
> > +#define RZG3E_P1               1
> > +#define RZG3E_P2               2
> > +#define RZG3E_P3               3
> > +#define RZG3E_P4               4
> > +#define RZG3E_P5               5
> > +#define RZG3E_P6               6
> > +#define RZG3E_P7               7
> > +#define RZG3E_P8               8
> > +#define RZG3E_PA               9
> > +#define RZG3E_PB               10
> > +#define RZG3E_PC               11
> > +#define RZG3E_PD               12
> > +#define RZG3E_PE               13
> > +#define RZG3E_PF               14
> > +#define RZG3E_PG               15
> > +#define RZG3E_PH               16
> > +#define RZG3E_PJ               17
> > +#define RZG3E_PK               18
> > +#define RZG3E_PL               19
> > +#define RZG3E_PM               20
> > +#define RZG3E_PS               21
> 
> This maps the discontiguous alpha-numerical port name range to a contiguous numerical range.
> As there are corresponding holes in the register layout, I am not sure such a mapping is a good idea.

If I make contiguous alpha-numerical port name range to a contiguous numerical range.
GPIO ranges increases from 172->232. that is the reason for making exactly ports defined
in hardware manual to contiguous numerical range.

> What if a future variant (or a future documentation update) exposes the ports in between?

If a future variant or to accommodate RZ/V2H, contiguous alpha-numerical port name range
to a contiguous numerical range will be better, if we plan to support ports as alpha
numeric as mentioned in the hardware manual.

Other option is just using numbers.

Please let me know your preference

1) discontinuous alpha-numerical port name range to a contiguous numerical range.
2) contiguous alpha-numerical port name range to a contiguous numerical range.
3) Just use numbers like the one used in RZ/V2H
Or
4)Any other smart way of handling this.


> 
> > +
> >  /*
> >   * Create the pin index from its bank and position numbers and store in
> >   * the upper 16 bits the alternate function identifier
> >   */
> >  #define RZG2L_PORT_PINMUX(b, p, f)     ((b) * RZG2L_PINS_PER_PORT + (p) | ((f) << 16))
> > +#define RZG3E_PORT_PINMUX(b, p, f)     RZG2L_PORT_PINMUX(RZG3E_P##b, p, f)
> >
> >  /* Convert a port and pin label to its global pin index */  #define
> > RZG2L_GPIO(port, pin)  ((port) * RZG2L_PINS_PER_PORT + (pin))
> > +#define RZG3E_GPIO(port, pin)  RZG2L_GPIO(RZG3E_P##port, pin)
> >
> >  #endif /* __DT_BINDINGS_RZG2L_PINCTRL_H */
> 
> Note that I do like the clever scheme to handle alpha-numerical port names. Perhaps this should be
> implemented for RZ/V2H, too?
> RZG2L_GPIO(10, 2) and RZG2L_GPIO(10, 3) in r9a09g057h44-rzv2h-evk.dts do refer to PA2 and PA3.

I agree, if we are taking alpha-numeric ports route, then we need to fix RZ/V2H as well.

Cheers,
Biju