Re: [PATCH v4 4/5] pinctrl: renesas: rzg2l: Add RZ/G3S support for selecting the I3C power source

On 7/13/26 18:15, Biju Das wrote:
> Hi Claudiu,
> 
>> -----Original Message-----
>> From: claudiu beznea<[email protected]>
>> Sent: 13 July 2026 15:57
>> Subject: Re: [PATCH v4 4/5] pinctrl: renesas: rzg2l: Add RZ/G3S support for selecting the I3C power
>> source
>>
>>
>>
>> On 7/13/26 16:56, Biju Das wrote:
>>> Hi Claudiu,
>>>
>>>> -----Original Message-----
>>>> From: claudiu beznea<[email protected]>
>>>> Sent: 13 July 2026 14:19
>>>> Subject: Re: [PATCH v4 4/5] pinctrl: renesas: rzg2l: Add RZ/G3S
>>>> support for selecting the I3C power source
>>>>
>>>> Hi, Biju,
>>>>
>>>> On 7/12/26 17:55, Biju Das wrote:
>>>>> Hi Claudiu,
>>>>>
>>>>> Thanks for the patch.
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Claudiu Beznea<[email protected]>
>>>>>> Sent: 10 July 2026 12:37
>>>>>> Subject: [PATCH v4 4/5] pinctrl: renesas: rzg2l: Add RZ/G3S support
>>>>>> for selecting the I3C power source
>>>>>>
>>>>>> From: Claudiu Beznea<[email protected]>
>>>>>>
>>>>>> The Renesas RZ/G3S I3C pins can be powered at either 1.8V or 1.2V.
>>>>>> The pin controller provides a register to select between these two options.
>>>>>> Update the Renesas RZ/G2L pin controller driver to allow selecting the I3C power source on RZ/G3S
>> SoC.
>>>>>> Reviewed-by: Wolfram Sang<[email protected]>
>>>>>> Tested-by: Wolfram Sang<[email protected]>
>>>>>> Signed-off-by: Claudiu Beznea<[email protected]>
>>>>>> ---
>>>>>>
>>>>>> Changes in v4:
>>>>>> - none
>>>>>>
>>>>>> Changes in v3:
>>>>>> - collected tags
>>>>>>
>>>>>> Changes in v2:
>>>>>> - none
>>>>>>
>>>>>>     drivers/pinctrl/renesas/pinctrl-rzg2l.c | 73 +++++++++++++++++++++++--
>>>>>>     1 file changed, 68 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>>>>>> b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>>>>>> index b52a85066f63..9a0706fea220 100644
>>>>>> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>>>>>> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
>>>>>> @@ -69,6 +69,7 @@
>>>>>>     #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 PIN_CFG_IO_VMC_I3C		BIT(22)
>>>>>>
>>>>>>     #define RZG2L_SINGLE_PIN		BIT_ULL(63)	/* Dedicated pin */
>>>>>>     #define RZG2L_VARIABLE_CFG		BIT_ULL(62)	/* Variable cfg for port pins */
>>>>>> @@ -186,6 +187,9 @@
>>>>>>     #define PVDD_3300		0	/* I/O domain voltage >= 3.3V */
>>>>>>     #define PVDD_MASK		0x3
>>>>>>
>>>>>> +#define PVDD_I3C_1200		1	/* I3C I/O domain voltage 1.2V */
>>>>>> +#define PVDD_I3C_1800		0	/* I3C I/O domain voltage 1.8V */
>>>>>> +
>>>>>>     #define PWPR_B0WI		BIT(7)	/* Bit Write Disable */
>>>>>>     #define PWPR_PFCWE		BIT(6)	/* PFC Register Write Enable */
>>>>>>     #define PWPR_REGWE_A		BIT(6)	/* PFC and PMC Register Write Enable on RZ/V2H(P) */
>>>>>> @@ -257,6 +261,7 @@ static const struct pin_config_item renesas_rzv2h_conf_items[] = {
>>>>>>      * @oen: OEN register offset
>>>>>>      * @qspi: QSPI register offset
>>>>>>      * @other_poc: OTHER_POC register offset
>>>>>> + * @i3c_set: I3C_SET register offset
>>>>>>      */
>>>>>>     struct rzg2l_register_offsets {
>>>>>>     	u16 pwpr;
>>>>>> @@ -265,6 +270,7 @@ struct rzg2l_register_offsets {
>>>>>>     	u16 oen;
>>>>>>     	u16 qspi;
>>>>>>     	u16 other_poc;
>>>>>> +	u16 i3c_set;
>>>>>
>>>>>>     };
>>>>>>
>>>>>>     /**
>>>>>> @@ -272,6 +278,7 @@ struct rzg2l_register_offsets {
>>>>>>      * @other_poc_pvdd1833_oth_awo_poc: PVDD1833_OTH_AWO_POC mask
>>>>>>      * @other_poc_pvdd1833_oth_iso_poc: PVDD1833_OTH_ISO_POC mask
>>>>>>      * @other_poc_wdtovf_n_poc: WDTOVF_N_POC mask
>>>>>> + * @i3c_set_poc: I3C_SET_POC mask
>>>>>>      */
>>>>>>     struct rzg2l_register_masks {
>>>>>>     	union {
>>>>>> @@ -281,6 +288,11 @@ struct rzg2l_register_masks {
>>>>>>     			u8 other_poc_pvdd1833_oth_iso_poc;
>>>>>>     			u8 other_poc_wdtovf_n_poc;
>>>>>>     		};
>>>>>> +
>>>>>> +		/* RZ/G3S masks */
>>>>>> +		struct {
>>>>>> +			u8 i3c_set_poc;
>>>>> How this POC is different from Ethernet, SDHI and XSPI POC?
>>>> Different bit mask and offset for I3C SET_POC compared with ETH, SDHI, XSPI.
>>> RZ/G3L has i3c_set_poc, which has same bitmask as other_poc_wdtovf_n_poc.
>>> Maybe create register specific masks??
>>>
>>> struct other_poc and struct i3c_set instead of union. So that both
>>> RZ/G3L and RZ/G3S can share the same struct for i3c.
>> When RZ/G3L I3C POC support will be added the i3c_set_poc member of struct rzg2l_register_masks could be
>> moved as common member:
>>
>> Current code base allows for this extension. You can have:
>> struct rzg2l_register_masks {
>> +       /* Common masks. */
>> +       u8 i3c_set_poc;
>>           union {
> OK, This union will go when we add RZ/G3L.
> 
>>                   /* RZ/G3L masks */
>>                   struct {
>>                           u8 other_poc_pvdd1833_oth_awo_poc;
>>                           u8 other_poc_pvdd1833_oth_iso_poc;
>>                           u8 other_poc_wdtovf_n_poc;
>>                   };
>> -               /* RZ/G3S masks */
>> -               struct {
> Looks, this struct not needed now. struct with single member
> has no value at all.

I'll let Geert decide if he wants me to drop it.

Thank you,
Claudiu