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

Hi Claudiu,

On Fri, 10 Jul 2026 at 13:36, Claudiu Beznea
<[email protected]> wrote:
> 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]>

Thanks for your patch!

> --- 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 */

This repeats the existing pattern of having both #defines...

> +
>  #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) */

> @@ -441,6 +455,7 @@ struct rzg2l_pinctrl {
>  };
>
>  static const u16 available_ps[] = { 1800, 2500, 3300 };
> +static const u16 available_i3c_ps[] = { 1200, 1800 };

... and arrays for the possible values.

>
>  static u64 rzg2l_pinctrl_get_variable_pin_cfg(struct rzg2l_pinctrl *pctrl,
>                                               u64 pincfg,

>
>  static int rzg2l_pwr_reg_val_to_ps(u8 val, u32 caps)
>  {
> +       if (caps & PIN_CFG_IO_VMC_I3C) {
> +               switch (val) {
> +               case PVDD_I3C_1200:
> +                       return 1200;
> +               case PVDD_I3C_1800:
> +                       return 1800;
> +               }

Look-up uses the defines...

> +
> +               return -EINVAL;
> +       }
> +
>         switch (val) {
>         case PVDD_1800:
>                 return 1800;
> @@ -1121,6 +1152,19 @@ static int rzg2l_pwr_reg_val_to_ps(u8 val, u32 caps)
>
>  static int rzg2l_ps_to_pwr_reg_val(u8 *val, u32 ps, u32 caps)
>  {
> +       if (caps & PIN_CFG_IO_VMC_I3C) {
> +               switch (ps) {
> +               case 1200:
> +                       *val = PVDD_I3C_1200;
> +                       return 0;
> +               case 1800:
> +                       *val = PVDD_I3C_1800;
> +                       return 0;
> +               }

... just like reverse look-up...

> +
> +               return -EINVAL;
> +       }
> +
>         switch (ps) {
>         case 1800:
>                 *val = PVDD_1800;
> @@ -1194,12 +1238,21 @@ static int rzg2l_set_power_source(struct rzg2l_pinctrl *pctrl, u32 pin, u32 caps
>         return 0;
>  }
>
> -static bool rzg2l_ps_is_supported(u16 ps)
> +static bool rzg2l_ps_is_supported(u16 ps, u32 caps)
>  {
> -       unsigned int i;
> +       unsigned int i, len;
> +       const u16 *array;
>
> -       for (i = 0; i < ARRAY_SIZE(available_ps); i++) {
> -               if (available_ps[i] == ps)
> +       if (caps & PIN_CFG_IO_VMC_I3C) {
> +               array = available_i3c_ps;
> +               len = ARRAY_SIZE(available_i3c_ps);
> +       } else {
> +               array = available_ps;
> +               len = ARRAY_SIZE(available_ps);
> +       }
> +
> +       for (i = 0; i < len; i++) {
> +               if (array[i] == ps)
>                         return true;
>         }

... while validity checks use the arrays.

I guess one could drop the defines, and always use the arrays?

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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