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: 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@xxxxxxxxxxxxxx> wrote:
> > > -----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
> > >
> > > 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>
> 
> > > > --- 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