Re: [PATCH v3 6/8] pinctrl: renesas: rzg2l: Add support for RZ/G3L SoC

[Geert Uytterhoeven]

Hi Biju,

On Tue, 17 Mar 2026 at 11:16, Biju <biju.das.au@xxxxxxxxx> wrote:
> From: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
>
> Add pinctrl driver support for RZ/G3L SoC.
>
> Signed-off-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c

> @@ -2479,6 +2614,37 @@ static struct rzg2l_dedicated_configs rzg3e_dedicated_pins[] = {
>          (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
>  };
>
> +static const struct rzg2l_dedicated_configs rzg3l_dedicated_pins[] = {
> +       { "WDTOVF_N", RZG2L_SINGLE_PIN_PACK(0x5, 0,
> +         (PIN_CFG_IOLH_A | PIN_CFG_WDTOVF_N_POC)) },
> +       { "SCIF_RXD", RZG2L_SINGLE_PIN_PACK(0x6, 0,
> +         (PIN_CFG_IOLH_A | PIN_CFG_PUPD | PIN_CFG_PVDD1833_OTH_AWO_POC)) },
> +       { "SCIF_TXD", RZG2L_SINGLE_PIN_PACK(0x6, 1,
> +         (PIN_CFG_IOLH_A | PIN_CFG_PUPD | PIN_CFG_PVDD1833_OTH_AWO_POC)) },

The main documentation indeed calls these pins "SCIF_RXD" and
"SCIF_TXD", like on RZ/V2H and RZ/G3E.
However, unlike the latter SoCs, RZ/G3L has multiple SCIF interfaces.
As the pin function spreadsheet does call them "SCIF0_RXD" resp.
"SCIF0_TXD", and users will probably use that spreadsheet to find the
right pin control configuration, I think it makes sense to use the
names that include the zero index.

> +       { "SD0_CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0,  PIN_CFG_IOLH_B) },

One space too many before PIN_CFG_IOLH_B.

> +       { "SD0_CMD", RZG2L_SINGLE_PIN_PACK(0x9, 1,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_RST#", RZG2L_SINGLE_PIN_PACK(0x9, 2, PIN_CFG_IOLH_B) },
> +       { "SD0_DS", RZG2L_SINGLE_PIN_PACK(0x9, 5,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA0", RZG2L_SINGLE_PIN_PACK(0x0a, 0,
> +         (PIN_CFG_IOLH_B |  PIN_CFG_IEN | PIN_CFG_PUPD)) },

One space too many before PIN_CFG_IEN.

> +       { "SD0_DATA1", RZG2L_SINGLE_PIN_PACK(0x0a, 1,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA2", RZG2L_SINGLE_PIN_PACK(0x0a, 2,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA3", RZG2L_SINGLE_PIN_PACK(0x0a, 3,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA4", RZG2L_SINGLE_PIN_PACK(0x0a, 4,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA5", RZG2L_SINGLE_PIN_PACK(0x0a, 5,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA6", RZG2L_SINGLE_PIN_PACK(0x0a, 6,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> +       { "SD0_DATA7", RZG2L_SINGLE_PIN_PACK(0x0a, 7,
> +         (PIN_CFG_IOLH_B | PIN_CFG_IEN | PIN_CFG_PUPD)) },

The SD data pins are called "SD0_D[0-7]" in the main docs, but
"SD0_DAT[0-7]" in the spreadsheet. So please pick one of these ;-)

> +};
> +
>  static int rzg2l_gpio_get_gpioint(unsigned int virq, struct rzg2l_pinctrl *pctrl)
>  {
>         const struct pinctrl_pin_desc *pin_desc = &pctrl->desc.pins[virq];

> @@ -3263,6 +3432,8 @@ static int rzg2l_pinctrl_suspend_noirq(struct device *dev)
>
>         cache->qspi = readb(pctrl->base + QSPI);
>         cache->oen = readb(pctrl->base + pctrl->data->hwcfg->regs.oen);
> +       if (regs->other_poc)
> +               cache->other_poc = readb(pctrl->base + regs->other_poc);
>
>         if (!atomic_read(&pctrl->wakeup_path))
>                 clk_disable_unprepare(pctrl->clk);
> @@ -3288,6 +3459,8 @@ static int rzg2l_pinctrl_resume_noirq(struct device *dev)
>         }
>
>         writeb(cache->qspi, pctrl->base + QSPI);

RZ/G3L does not have the QSPI register.
However, this write is harmless, as it has the SD_CH1_POC at this offset,
which is thus saved/restored twice.

> +       if (regs->other_poc)
> +               writeb(cache->other_poc, pctrl->base + regs->other_poc);
>
>         raw_spin_lock_irqsave(&pctrl->lock, flags);
>         rzg2l_oen_write_with_pwpr(pctrl, cache->oen);

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