Re: [PATCH v6 3/5] pinctrl: renesas: rzg2l: Unify the power source handling

[Geert Uytterhoeven]

Hi Claudiu,

On Thu, 3 Sept 2026 at 13:34, Claudiu Beznea
<claudiu.beznea+renesas@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The previous code handled power sources using a mixture of power
> source specific definitions and lookups in the available_ps[] array.
> Unify the power source handling by introducing
> struct rzg2l_pinctrl_ps_desc, whose purpose is to describe a power
> source through its power source value, associated register value,
> associated capabilities (e.g. Ethernet), and associated IOLH index.
>
> Introduce two new functions, rzg2l_ps_to_desc() and
> rzg2l_pwr_reg_val_to_desc(), using the
> RZG2L_PINCTRL_PS_DESC_MEMBER_TO_DESC_FUNC() macro, as their
> implementations are similar.
>
> These functions retrieve a power source descriptor based on either a
> power source value or a power source register value. Other functions
> that need to perform power source specific operations can call them to
> retrieve the corresponding power source descriptor.
>
> The register values for the soft power sources were kept to zero since
> they are not used across the driver's code.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v6:
> - described all the available power sources in the available_ps[]
>   array and dropped to wildcard approach to avoid letting the user
>   selecting unavailable power source for pins
> - simplified RZG2L_PINCTRL_PS_DESC_MEMBER_TO_DESC_FUNC()
> - for these two points ^ didn't collect the Wolfram's Tb tag
> - dropped rzg2l_ps_is_supported()
> - used conditional operator in rzg2l_ps_to_iolh_idx()
> - adjusted the patch description

Thanks for the update!

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

> +static const struct rzg2l_pinctrl_ps_desc available_ps[] = {
> +       /* Ethernet I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_IO_VMC_ETH0 | PIN_CFG_IO_VMC_ETH1,
> +                             RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(2500, 2, PIN_CFG_IO_VMC_ETH0 | PIN_CFG_IO_VMC_ETH1,
> +                             RZG2L_IOLH_IDX_2V5),
> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_IO_VMC_ETH0 | PIN_CFG_IO_VMC_ETH1,
> +                             RZG2L_IOLH_IDX_3V3),
> +
> +       /* SD I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_IO_VMC_SD0 | PIN_CFG_IO_VMC_SD1 |
> +                             PIN_CFG_IO_VMC_SD2, RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(2500, 2, PIN_CFG_IO_VMC_SD0 | PIN_CFG_IO_VMC_SD1 |
> +                             PIN_CFG_IO_VMC_SD2, RZG2L_IOLH_IDX_2V5),

Before (see below), only Ethernet could select the 2.5V domain, so
shouldn't this entry be dropped?

> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_IO_VMC_SD0 | PIN_CFG_IO_VMC_SD1 |
> +                             PIN_CFG_IO_VMC_SD2, RZG2L_IOLH_IDX_3V3),
> +
> +       /* QSPI I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_IO_VMC_QSPI, RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(2500, 2, PIN_CFG_IO_VMC_QSPI, RZG2L_IOLH_IDX_2V5),

Same here?

> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_IO_VMC_QSPI, RZG2L_IOLH_IDX_3V3),
> +
> +       /* AWO I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_PVDD1833_OTH_AWO_POC,
> +                             RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_PVDD1833_OTH_AWO_POC,
> +                             RZG2L_IOLH_IDX_3V3),
> +
> +       /* ISO I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_PVDD1833_OTH_ISO_POC,
> +                             RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_PVDD1833_OTH_ISO_POC,
> +                             RZG2L_IOLH_IDX_3V3),
> +
> +       /* WDTOVF I/O voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 1, PIN_CFG_WDTOVF_N_POC, RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_WDTOVF_N_POC, RZG2L_IOLH_IDX_3V3),
> +
> +       /* Software voltage domains */
> +       RZG2L_PINCTRL_PS_DESC(1800, 0, PIN_CFG_SOFT_PS, RZG2L_IOLH_IDX_1V8),
> +       RZG2L_PINCTRL_PS_DESC(2500, 0, PIN_CFG_SOFT_PS, RZG2L_IOLH_IDX_2V5),
> +       RZG2L_PINCTRL_PS_DESC(3300, 0, PIN_CFG_SOFT_PS, RZG2L_IOLH_IDX_3V3),

I never really understood PIN_CFG_SOFT_PS...

> +};

> -static int rzg2l_ps_to_pwr_reg_val(u32 ps, u32 caps)
> +static int rzg2l_ps_to_pwr_reg_val(u16 ps, u32 caps)
>  {
> -       switch (ps) {
> -       case 1800:
> -               return PVDD_1800;
> -       case 2500:
> -               if (!(caps & (PIN_CFG_IO_VMC_ETH0 | PIN_CFG_IO_VMC_ETH1)))
> -                       return -EINVAL;
> -               return PVDD_2500;
> -       case 3300:
> -               return PVDD_3300;
> -       }
> +       const struct rzg2l_pinctrl_ps_desc *desc;
>
> -       return -EINVAL;
> +       desc = rzg2l_ps_to_desc(ps, caps);
> +       if (!desc)
> +               return -EINVAL;
> +
> +       return desc->pwr_reg_val;
>  }
>
>  static int rzg2l_get_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps)

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