RE: [PATCH v3 4/7] pinctrl: renesas: rzg2l: Add support for RZ/G3E SoC

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

Hi Geert Uytterhoeven,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 16 December 2024 15:25
> Subject: Re: [PATCH v3 4/7] pinctrl: renesas: rzg2l: Add support for RZ/G3E SoC
> 
> Hi Biju,
> 
> On Fri, Dec 13, 2024 at 6:39 PM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Add pinctrl driver support for RZ/G3E SoC.
> >
> > Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > ---
> > v2->v3:
> >  * Replaced RZG3E_* macro with generic PORT_* macro.
> >  * Added ports based on hardware indices.
> >  * Replaced macros WDTUDF_CA->WDTUDFCA and WDTUDF_CM->WDTUDFCM.
> >  * Replaced macro QSD0_*->SD0*.
> 
> Thanks for the update!
> 
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -1962,6 +2000,73 @@ static const u64 r9a08g045_gpio_configs[] = {
> >         RZG2L_GPIO_PORT_PACK(6, 0x2a, RZG3S_MPXED_PIN_FUNCS(A)),                        /* P18 */
> >  };
> >
> > +static const char * const rzg3e_gpio_names[] = {
> > +       "P00", "P01", "P02", "P03", "P04", "P05", "P06", "P07",
> > +       "P10", "P11", "P12", "P13", "P14", "P15", "P16", "P17",
> > +       "P20", "P21", "P22", "P23", "P24", "P25", "P26", "P27",
> > +       "P30", "P31", "P32", "P33", "P34", "P35", "P36", "P37",
> > +       "P40", "P41", "P42", "P43", "P44", "P45", "P46", "P47",
> > +       "P50", "P51", "P52", "P53", "P54", "P55", "P56", "P57",
> > +       "P60", "P61", "P62", "P63", "P64", "P65", "P66", "P67",
> > +       "P70", "P71", "P72", "P73", "P74", "P75", "P76", "P77",
> > +       "P80", "P81", "P82", "P83", "P84", "P85", "P86", "P87",
> > +       "P90", "P91", "P92", "P93", "P94", "P95", "P96", "P97",
> > +       "PA0", "PA1", "PA2", "PA3", "PA4", "PA5", "PA6", "PA7",
> > +       "PB0", "PB1", "PB2", "PB3", "PB4", "PB5", "PB6", "PB7",
> > +       "PC0", "PC1", "PC2", "PC3", "PC4", "PC5", "PC6", "PC7",
> > +       "PD0", "PD1", "PD2", "PD3", "PD4", "PD5", "PD6", "PD7",
> > +       "PE0", "PE1", "PE2", "PE3", "PE4", "PE5", "PE6", "PE7",
> > +       "PF0", "PF1", "PF2", "PF3", "PF4", "PF5", "PF6", "PF7",
> > +       "PG0", "PG1", "PG2", "PG3", "PG4", "PG5", "PG6", "PG7",
> > +       "PH0", "PH1", "PH2", "PH3", "PH4", "PH5", "PH6", "PH7",
> > +       "PI0", "PI1", "PI2", "PI3", "PI4", "PI5", "PI6", "PI7",
> > +       "PJ0", "PJ1", "PJ2", "PJ3", "PJ4", "PJ5", "PJ6", "PJ7",
> > +       "PK0", "PK1", "PK2", "PK3", "PK4", "PK5", "PK6", "PK7",
> > +       "PL0", "PL1", "PL2", "PL3", "PL4", "PL5", "PL6", "PL7",
> > +       "PM0", "PM1", "PM2", "PM3", "PM4", "PM5", "PM6", "PM7",
> > +       "PN0", "PN1", "PN2", "PN3", "PN4", "PN5", "PN6", "PN7",
> > +       "PO0", "PO1", "PO2", "PO3", "PO4", "PO5", "PO6", "PO7",
> > +       "PP0", "PP1", "PP2", "PP3", "PP4", "PP5", "PP6", "PP7",
> > +       "PQ0", "PQ1", "PQ2", "PQ3", "PQ4", "PQ5", "PQ6", "PQ7",
> > +       "PR0", "PR1", "PR2", "PR3", "PR4", "PR5", "PR6", "PR7",
> > +       "PS0", "PS1", "PS2", "PS3", "PS4", "PS5", "PS6", "PS7",
> 
> Can the non-existing P9x, PIx, and PNx-PRx be left NULL?

OK.

> 
> > +};
> > +
> > +static const u64 r9a09g047_gpio_configs[] = {
> > +       RZG2L_GPIO_PORT_PACK(8, 0x20, RZV2H_MPXED_PIN_FUNCS),   /* P0 */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x21, RZV2H_MPXED_PIN_FUNCS |
> > +                                     PIN_CFG_ELC),             /* P1 */
> > +       RZG2L_GPIO_PORT_PACK(2, 0x22, RZG2L_MPXED_COMMON_PIN_FUNCS(RZV2H) |
> > +                                     PIN_CFG_NOD),             /* P2 */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x23, RZV2H_MPXED_PIN_FUNCS),   /* P3 */
> > +       RZG2L_GPIO_PORT_PACK(6, 0x24, RZV2H_MPXED_PIN_FUNCS),   /* P4 */
> > +       RZG2L_GPIO_PORT_PACK(7, 0x25, RZV2H_MPXED_PIN_FUNCS),   /* P5 */
> > +       RZG2L_GPIO_PORT_PACK(7, 0x26, RZV2H_MPXED_PIN_FUNCS),   /* P6 */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x27, RZV2H_MPXED_PIN_FUNCS |
> > +                                     PIN_CFG_ELC),             /* P7 */
> > +       RZG2L_GPIO_PORT_PACK(6, 0x28, RZV2H_MPXED_PIN_FUNCS),   /* P8 */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x29, RZV2H_MPXED_PIN_FUNCS),   /* P9 */
> 
> P9 does not exist, so I think it should be left empty (NULL?).

OK.

> 
> > +       RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x2a),                 /* PA */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x2b, RZV2H_MPXED_PIN_FUNCS),   /* PB */
> > +       RZG2L_GPIO_PORT_PACK(3, 0x2c, RZV2H_MPXED_PIN_FUNCS),   /* PC */
> > +       RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x2d),                 /* PD */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x2e, RZV2H_MPXED_PIN_FUNCS),   /* PE */
> > +       RZG2L_GPIO_PORT_PACK(3, 0x2f, RZV2H_MPXED_PIN_FUNCS),   /* PF */
> > +       RZG2L_GPIO_PORT_PACK_VARIABLE(8, 0x30),                 /* PG */
> > +       RZG2L_GPIO_PORT_PACK_VARIABLE(6, 0x31),                 /* PH */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x32, RZV2H_MPXED_PIN_FUNCS),   /* PI */
> 
> Likewise for PI...
> 
> > +       RZG2L_GPIO_PORT_PACK_VARIABLE(5, 0x33),                 /* PJ */
> > +       RZG2L_GPIO_PORT_PACK(4, 0x34, RZV2H_MPXED_PIN_FUNCS),   /* PK */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x35, RZV2H_MPXED_PIN_FUNCS),   /* PL */
> > +       RZG2L_GPIO_PORT_PACK(8, 0x36, RZV2H_MPXED_PIN_FUNCS),   /* PM */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x37, RZV2H_MPXED_PIN_FUNCS),   /* PN */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x38, RZV2H_MPXED_PIN_FUNCS),   /* PO */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x39, RZV2H_MPXED_PIN_FUNCS),   /* PP */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x3a, RZV2H_MPXED_PIN_FUNCS),   /* PQ */
> > +       RZG2L_GPIO_PORT_PACK(0, 0x3b, RZV2H_MPXED_PIN_FUNCS),   /* PR */
> 
> ... and PN-PR.

Agreed.

Cheers,
Biju