Re: [PATCH v4 4/5] pinctrl: renesas: rzg2l: Add RZ/G3S support for selecting the I3C power source
From: Geert Uytterhoeven
Date: Thu Aug 13 2026 - 11:56:09 EST
Hi Claudiu,
On Fri, 10 Jul 2026 at 13:36, Claudiu Beznea
<claudiu.beznea+renesas@xxxxxxxxx> wrote:
> 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>
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 <geert+renesas@xxxxxxxxx>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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