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

[Biju Das]

Hi Claudiu,

> -----Original Message-----
> From: claudiu beznea <claudiu.beznea@xxxxxxxxx>
> 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 <claudiu.beznea@xxxxxxxxx>
> >> 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 <claudiu.beznea+renesas@xxxxxxxxx>
> >>>> 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 <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>>
> >>>> 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 <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> >>>> Tested-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> >>>> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> >>>> ---
> >>>>
> >>>> 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.

> -                       u8 i3c_set_poc;
> -               };
>          };
> };
> 
> // ...
> 
> const struct rzg2l_hwcfg rzg3l_hwcfg = {
>          // ...
> 
>          .masks = {
>                  .other_poc_pvdd1833_oth_awo_poc = BIT(0),
>                  .other_poc_pvdd1833_oth_iso_poc = BIT(1),
>                  .other_poc_wdtovf_n_poc = BIT(2),
> +               .i3c_set_poc = BIT(x),
			.i3c_set_poc = BIT(2),

Cheers,
Biju

>          },
> 
>          // ...
> };
> 
> The rest of configuration and initialization code remains the same.
> 
> Thank you,
> Claudiu