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

[Biju Das]

Hi Geert,

Thanks for the feedback.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: 28 April 2026 20:04
> Subject: Re: [PATCH v3 6/8] pinctrl: renesas: rzg2l: Add support for RZ/G3L SoC
> 
> 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.

Ok will add zero index.

> 
> > +       { "SD0_CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0,  PIN_CFG_IOLH_B) },
> 
> One space too many before PIN_CFG_IOLH_B.

Will fix.

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

Missed it, Will fix.

> 
> > +       { "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 ;-)

I will stick with spread sheet.

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

QSPI is available only on RZ/{G2L,G2LC,G2UL,Five} SoC. I will create a
separate patch for supporting only for these SoCs.

Cheers,
Biju