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

[Claudiu Beznea]

Hi, Geert,

On 9/3/26 18:32, Geert Uytterhoeven wrote:
> 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?

That's a mistake. I'll drop it from here and from QSPI section.

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

That was to match b/w a voltage and an IOLH value according to the table from 
45.3.7 Driving Ability Control Register (IOLH_m) in the RZ/G3S manual (rev.1.30)

Thank you,
Claudiu