diff mbox series

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

Message ID 20241206102327.8737-2-biju.das.jz@bp.renesas.com
State New
Headers show
Series Add RZ/G3E pinctrl support | expand

Commit Message

Biju Das Dec. 6, 2024, 10:23 a.m. UTC
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@bp.renesas.com>
---
v1->v2:
 * Fixed the warnings reported by bot.
---
 .../pinctrl/renesas,rzg2l-pinctrl.yaml        |  7 ++++--
 include/dt-bindings/pinctrl/rzg2l-pinctrl.h   | 25 +++++++++++++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven Dec. 12, 2024, 4:26 p.m. UTC | #1
Hi Biju,

On Fri, Dec 6, 2024 at 11:23 AM Biju Das <biju.das.jz@bp.renesas.com> 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@bp.renesas.com>
> ---
> 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@linux-m68k.org

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
Biju Das Dec. 12, 2024, 5:14 p.m. UTC | #2
Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@linux-m68k.org>
> 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@bp.renesas.com> 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@bp.renesas.com>
> > ---
> > 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
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
index a1805b6e3f63..768bb3c2b456 100644
--- a/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/renesas,rzg2l-pinctrl.yaml
@@ -26,6 +26,7 @@  properties:
               - renesas,r9a07g043-pinctrl # RZ/G2UL{Type-1,Type-2} and RZ/Five
               - renesas,r9a07g044-pinctrl # RZ/G2{L,LC}
               - renesas,r9a08g045-pinctrl # RZ/G3S
+              - renesas,r9a09g047-pinctrl # RZ/G3E
               - renesas,r9a09g057-pinctrl # RZ/V2H(P)
 
       - items:
@@ -125,7 +126,7 @@  additionalProperties:
         drive-push-pull: true
         renesas,output-impedance:
           description:
-            Output impedance for pins on the RZ/V2H(P) SoC. The value provided by this
+            Output impedance for pins on the RZ/{G3E,V2H(P)} SoC. The value provided by this
             property corresponds to register bit values that can be set in the PFC_IOLH_mn
             register, which adjusts the drive strength value and is pin-dependent.
           $ref: /schemas/types.yaml#/definitions/uint32
@@ -142,7 +143,9 @@  allOf:
       properties:
         compatible:
           contains:
-            const: renesas,r9a09g057-pinctrl
+            enum:
+              - renesas,r9a09g047-pinctrl
+              - renesas,r9a09g057-pinctrl
     then:
       properties:
         resets:
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
+
 /*
  * 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 */