Re: [PATCH 26/37] pinctrl: renesas: rzg2l: move ds and oi to SoC specific configuration

[Geert Uytterhoeven]

Hi Claudiu,

On Tue, Sep 12, 2023 at 6:53 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Move drive strength and output impedance values to SoC specific
> configuration data structure (struct rzg2l_hwcfg). This allows extending
> the drive strength support for RZ/G3S. Along with this the DS values
> were converted to uA for simple integration with RZ/G3S support.
>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> @@ -133,13 +133,27 @@ struct rzg2l_register_offsets {
>         u16 sd_ch;
>  };
>
> +/**
> + * enum rzg2l_iolh_index - starting indexes in IOLH specific arrays

indices

> + * @RZG2L_IOLH_IDX_3V3: starting index for 3V3 power source
> + * @RZG2L_IOLH_IDX_MAX: maximum index
> + */
> +enum rzg2l_iolh_index {
> +       RZG2L_IOLH_IDX_3V3 = 0,
> +       RZG2L_IOLH_IDX_MAX = 4,
> +};
> +
>  /**
>   * struct rzg2l_hwcfg - hardware configuration data structure
>   * @regs: hardware specific register offsets
> + * @iolh_groupa_ua: IOLH group A micro amps specific values

uA (or µA ;-)

> + * @iolh_groupb_oi: IOLH group B output impedance specific values
>   * @func_base: base number for port function (see register PFC)
>   */
>  struct rzg2l_hwcfg {
>         const struct rzg2l_register_offsets regs;
> +       u16 iolh_groupa_ua[RZG2L_IOLH_IDX_MAX];
> +       u16 iolh_groupb_oi[RZG2L_IOLH_IDX_MAX];
>         u8 func_base;
>  };
>

> @@ -708,11 +719,11 @@ static int rzg2l_pinctrl_pinconf_set(struct pinctrl_dev *pctldev,
>                         if (!(cfg & PIN_CFG_IOLH_A))
>                                 return -EINVAL;
>
> -                       for (index = 0; index < ARRAY_SIZE(iolh_groupa_mA); index++) {
> -                               if (arg == iolh_groupa_mA[index])
> +                       for (index = RZG2L_IOLH_IDX_3V3; index < RZG2L_IOLH_IDX_3V3 + 4; index++) {

I'm not so fond of the hardcoded "+ 4", here and below.
Please add and use a #define.

> +                               if (arg == (hwcfg->iolh_groupa_ua[index] / 1000))
>                                         break;
>                         }
> -                       if (index >= ARRAY_SIZE(iolh_groupa_mA))
> +                       if (index == (RZG2L_IOLH_IDX_3V3 + 4))
>                                 return -EINVAL;
>
>                         rzg2l_rmw_pin_config(pctrl, IOLH(off), bit, IOLH_MASK, index);


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