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

[Claudiu Beznea <claudiu.beznea+renesas@tuxon.dev>]

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@tuxon.dev> wrote:
>> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>>
>> 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@bp.renesas.com>
>> ---
>>
>> 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