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

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

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?

> +};
> +
> +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?).

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

> +       RZG2L_GPIO_PORT_PACK(4, 0x3c, RZV2H_MPXED_PIN_FUNCS),   /* PS */
> +};

The rest LGTM.

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