RE: [PATCH v2 2/4] pinctrl: renesas: rzg2l: Add support for 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 15:33
> Subject: Re: [PATCH v2 2/4] pinctrl: renesas: rzg2l: Add support for RZ/G3E SoC
> 
> Hi Biju,
> 
> On Fri, Dec 6, 2024 at 11:23 AM Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > Add pinctrl driver support for RZ/G3E 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
> > @@ -2252,6 +2343,43 @@ static struct rzg2l_dedicated_configs rzv2h_dedicated_pins[] = {
> >         { "ET1_RXD3", RZG2L_SINGLE_PIN_PACK(0x14, 7, (PIN_CFG_PUPD))
> > },  };
> >
> > +static struct rzg2l_dedicated_configs rzg3e_dedicated_pins[] = {
> > +       { "WDTUDF_CA", RZG2L_SINGLE_PIN_PACK(0x5, 0,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_PUPD | PIN_CFG_NOD)) },
> > +       { "WDTUDF_CM", RZG2L_SINGLE_PIN_PACK(0x5, 1,
> 
> Except for Table 4.2-13, these are called WDTUDFCA and WDTUDFCM everywhere.

Thanks for pointing out. I have informed manual team about this issue.

> 
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_PUPD | PIN_CFG_NOD)) },
> > +       { "SCIF_RXD", RZG2L_SINGLE_PIN_PACK(0x6, 0,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_PUPD)) },
> > +       { "SCIF_TXD", RZG2L_SINGLE_PIN_PACK(0x6, 1,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_PUPD)) },
> > +       { "QSD0_CLK", RZG2L_SINGLE_PIN_PACK(0x9, 0,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> > +       { "QSD0_CMD", RZG2L_SINGLE_PIN_PACK(0x9, 1,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_RSTN", RZG2L_SINGLE_PIN_PACK(0x9, 2,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> > +       { "QSD0_PWEN", RZG2L_SINGLE_PIN_PACK(0x9, 3,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> > +       { "QSD0_IOVS", RZG2L_SINGLE_PIN_PACK(0x9, 4,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR)) },
> > +       { "QSD0_DAT0", RZG2L_SINGLE_PIN_PACK(0xa, 0,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT1", RZG2L_SINGLE_PIN_PACK(0xa, 1,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT2", RZG2L_SINGLE_PIN_PACK(0xa, 2,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT3", RZG2L_SINGLE_PIN_PACK(0xa, 3,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT4", RZG2L_SINGLE_PIN_PACK(0xa, 4,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT5", RZG2L_SINGLE_PIN_PACK(0xa, 5,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT6", RZG2L_SINGLE_PIN_PACK(0xa, 6,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN | PIN_CFG_PUPD)) },
> > +       { "QSD0_DAT7", RZG2L_SINGLE_PIN_PACK(0xa, 7,
> > +        (PIN_CFG_IOLH_RZV2H | PIN_CFG_SR | PIN_CFG_IEN |
> > + PIN_CFG_PUPD)) },
> 
> SD0_*?

Oops. I will make it SD0* to match with h/w manual.

Cheers,


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