Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
MIME-Version: 1.0
References: <20230912045157.177966-1-claudiu.beznea.uj@bp.renesas.com> <20230912045157.177966-27-claudiu.beznea.uj@bp.renesas.com>
In-Reply-To: <20230912045157.177966-27-claudiu.beznea.uj@bp.renesas.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu, 21 Sep 2023 14:54:49 +0200
Message-ID: <CAMuHMdU_P8QM1qrwTJQcDCaKhMDCDnO1G_u=Nx=kuzy9MGuT8Q@mail.gmail.com>
Subject: Re: [PATCH 26/37] pinctrl: renesas: rzg2l: move ds and oi to SoC
 specific configuration
To:     Claudiu <claudiu.beznea@tuxon.dev>
Cc:     mturquette@baylibre.com, sboyd@kernel.org, robh+dt@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, conor+dt@kernel.org,
        ulf.hansson@linaro.org, linus.walleij@linaro.org,
        gregkh@linuxfoundation.org, jirislaby@kernel.org,
        magnus.damm@gmail.com, catalin.marinas@arm.com, will@kernel.org,
        prabhakar.mahadev-lad.rj@bp.renesas.com,
        biju.das.jz@bp.renesas.com, quic_bjorande@quicinc.com,
        arnd@arndb.de, konrad.dybcio@linaro.org, neil.armstrong@linaro.org,
        nfraprado@collabora.com, rafal@milecki.pl,
        wsa+renesas@sang-engineering.com,
        linux-renesas-soc@vger.kernel.org, linux-clk@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-mmc@vger.kernel.org, linux-gpio@vger.kernel.org,
        linux-serial@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

Hi Claudiu,

On Tue, Sep 12, 2023 at 6:53 AM Claudiu <claudiu.beznea@tuxon.dev> wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
>
> 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@bp.renesas.com>

Thanks for your patch!

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> --- 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@linux-m68k.org

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