| Message ID | 20241206102327.8737-2-biju.das.jz@bp.renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add RZ/G3E pinctrl support | expand |
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
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
Hi Biju, On Thu, Dec 12, 2024 at 6:15 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > -----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 > > > > 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> > > > --- 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. True. We do have (smaller) gaps already, as not all ports have 8 GPIOs. > > 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. At the lowest level, 2 and 3 are the same solution. I think using the numbers from the hardware manual (which match the hardware registers indices) is the safest solution. And the RZG3E_{PORT_PINMUX,GPIO}() macros below improve the user experience, by retaining the actual alpha-numerical names. BTW, have you checked the non-documented registers in the gaps, i.e. do their values look like they are backed by hardware blocks? I wouldn't be surprised if they do exist, and are reserved for use by the CM33, NPU, or some other non-disclosed processing core. Or perhaps there is a non-public variant in a package with more pins? BTW, the sentence about IRQ0 pinmuxing on page 312 refers to P90, which does not exist. In fact none of the referred pins can be muxed to IRQ0 on RZ/G3E. > > > + > > > /* > > > * 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. Gr{oetje,eeting}s, Geert
Hi Geert, Thanks for the feedback. > -----Original Message----- > From: Geert Uytterhoeven <geert@linux-m68k.org> > Sent: 13 December 2024 09:17 > Subject: Re: [PATCH v2 1/4] dt-bindings: pinctrl: renesas: Document RZ/G3E SoC > > Hi Biju, > > On Thu, Dec 12, 2024 at 6:15 PM Biju Das <biju.das.jz@bp.renesas.com> wrote: > > > -----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 > > > > > > 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> > > > > > --- 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. > > True. We do have (smaller) gaps already, as not all ports have 8 GPIOs. Ok. > > > > 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. > > At the lowest level, 2 and 3 are the same solution. > I think using the numbers from the hardware manual (which match the hardware registers indices) is the > safest solution. > And the RZG3E_{PORT_PINMUX,GPIO}() macros below improve the user experience, by retaining the actual > alpha-numerical names. > > BTW, have you checked the non-documented registers in the gaps, i.e. > do their values look like they are backed by hardware blocks? For RZ/G3E: As per Table 4.2-6 Corresponding Pins and Offset Addresses of PFC_P_mn x= pins from 0..7 0x20-0x28 --> P0x - P8x 0x2A-0x2F --> Pax - PFx 0x3A-0x31 --> PGx - PHx Gap(Possibly PIx) 0x33-0x36 --> PJx - PMx Gap (Possibly PNx, POx, PPx, PQx, PRx) 0x3C --> PSx For RZ/V2H: 0x20-0x2B --> P0x-PBx Register offset = 0x20 + Alpha-numeric (0..9, A..Z} OK, as you suggested, will use the numbers from hardware manual and RZG3E_{PORT_PINMUX,GPIO}() macros. > I wouldn't be surprised if they do exist, and are reserved for use by the CM33, NPU, or some other > non-disclosed processing core. > Or perhaps there is a non-public variant in a package with more pins? Pins are fixed as per the excel sheet. > > BTW, the sentence about IRQ0 pinmuxing on page 312 refers to P90, which does not exist. In fact none > of the referred pins can be muxed to IRQ0 on RZ/G3E. I guess it is cut + paste error from RZ/V2H. Thanks for pointing out, will create a ticket with REL for fixing this issue. Cheers, Biju
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 */
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(-)