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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

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.
What if a future variant (or a future documentation update) exposes
the ports in between?

> +
>  /*
>   * 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.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds