Re: [PATCH v3 4/8] pinctrl: renesas: rzg2l: Add support for selecting power source for {WDT,AWO,ISO}

[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>
>
> The RZ/G3L SoC has support for setting power source that are not
> controlled by the following voltage control registers:
>   - SD_CH{0,1,2}_POC, XSPI_POC, ETH{0,1}_POC, I3C_SET.POC
>
> Add support for selecting voltages using OTHER_POC register for
> setting I/O domain voltage for WDT, ISO and AWO.
>
> 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
> @@ -63,10 +63,18 @@
>  #define PIN_CFG_SMT                    BIT(16) /* Schmitt-trigger input control */
>  #define PIN_CFG_ELC                    BIT(17)
>  #define PIN_CFG_IOLH_RZV2H             BIT(18)
> +#define PIN_CFG_PVDD1833_OTH_AWO_POC   BIT(19) /* known on RZ/G3L only */
> +#define PIN_CFG_PVDD1833_OTH_ISO_POC   BIT(20) /* known on RZ/G3L only */
> +#define PIN_CFG_WDTOVF_N_POC           BIT(21) /* known on RZ/G3L only */
>
>  #define RZG2L_SINGLE_PIN               BIT_ULL(63)     /* Dedicated pin */
>  #define RZG2L_VARIABLE_CFG             BIT_ULL(62)     /* Variable cfg for port pins */
>
> +#define PIN_CFG_OTHER_POC_MASK \
> +                                       (PIN_CFG_PVDD1833_OTH_AWO_POC | \
> +                                        PIN_CFG_PVDD1833_OTH_ISO_POC | \
> +                                        PIN_CFG_WDTOVF_N_POC)
> +
>  #define RZG2L_MPXED_COMMON_PIN_FUNCS(group) \
>                                         (PIN_CFG_IOLH_##group | \
>                                          PIN_CFG_PUPD | \
> @@ -146,6 +154,7 @@
>  #define SD_CH(off, ch)         ((off) + (ch) * 4)
>  #define ETH_POC(off, ch)       ((off) + (ch) * 4)
>  #define QSPI                   (0x3008)
> +#define OTHER_POC              (0x3028) /* known on RZ/G3L only */
>
>  #define PVDD_2500              2       /* I/O domain voltage 2.5V */
>  #define PVDD_1800              1       /* I/O domain voltage <= 1.8V */
> @@ -906,6 +915,12 @@ static int rzg2l_caps_to_pwr_reg(const struct rzg2l_register_offsets *regs, u32
>                 return ETH_POC(regs->eth_poc, 1);
>         if (caps & PIN_CFG_IO_VMC_QSPI)
>                 return QSPI;
> +       if (caps & PIN_CFG_PVDD1833_OTH_AWO_POC)
> +               return OTHER_POC;
> +       if (caps & PIN_CFG_PVDD1833_OTH_ISO_POC)
> +               return OTHER_POC;
> +       if (caps & PIN_CFG_WDTOVF_N_POC)
> +               return OTHER_POC;

"if (caps & PIN_CFG_OTHER_POC_MASK)" would cover all cases at once.

>
>         return -EINVAL;
>  }
> @@ -925,6 +940,13 @@ static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps
>                 return pwr_reg;
>
>         val = readb(pctrl->base + pwr_reg);
> +       if (pwr_reg == OTHER_POC) {
> +               u32 poc = FIELD_GET(PIN_CFG_OTHER_POC_MASK, caps);
> +               u8 offs = ffs(poc) - 1;

Yesterday I thought "Why not store the bit number itself?", but after
a night of sleep, I had my aha-moment...
While clever, it is rather fragile to rely on the three PIN_CFG_*
values to match the order of the bits in the OTHER_POC registers.
So please check which PIN_CFG_* flag is set instead.

Perhaps rzg2l_caps_to_pwr_reg() should return both a register offset and
a bitmask, to be used with field_prep() and field_get()?

> +
> +               val = (val >> offs) & 0x1;
> +       }
> +
>         switch (val) {
>         case PVDD_1800:
>                 return 1800;
> @@ -943,6 +965,7 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps
>         const struct rzg2l_hwcfg *hwcfg = pctrl->data->hwcfg;
>         const struct rzg2l_register_offsets *regs = &hwcfg->regs;
>         int pwr_reg;
> +       u8 poc_val;
>         u8 val;

u8 poc_val, val;

>
>         if (caps & PIN_CFG_SOFT_PS) {

> @@ -970,6 +993,17 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps
>         if (pwr_reg < 0)
>                 return pwr_reg;
>
> +       if (pwr_reg == OTHER_POC) {
> +               u32 poc = FIELD_GET(PIN_CFG_OTHER_POC_MASK, caps);
> +               u8 offs = ffs(poc) - 1;
> +
> +               val = readb(pctrl->base + pwr_reg);

if (poc_val)
    val += BIT(offs);
else
    val &= ~BIT(offs);

> +               val &= ~BIT(offs);
> +               val |= (poc_val << offs);
> +       } else {
> +               val = poc_val;
> +       }
> +
>         writeb(val, pctrl->base + pwr_reg);
>         pctrl->settings[pin].power_source = ps;
>

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