linux-kernel - Re: [PATCH v2 2/9] pinctrl: renesas: rzg2l: Clean up and refactor OEN read/write functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdXe8aaweQJ2=V7ksKTqcJCnqewKhSrrO4h7X924Vbk-_Q@mail.gmail.com>
Date: Mon, 17 Jun 2024 14:02:17 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Paul Barker <paul.barker.ct@...renesas.com>
Cc: Magnus Damm <magnus.damm@...il.com>, Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Linus Walleij <linus.walleij@...aro.org>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>, linux-renesas-soc@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-gpio@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/9] pinctrl: renesas: rzg2l: Clean up and refactor OEN
 read/write functions

Hi Paul,

On Tue, Jun 11, 2024 at 1:33 PM Paul Barker
<paul.barker.ct@...renesas.com> wrote:
> The variable naming in the various OEN functions has been confusing. We
> were passing the _pin & bit variables from rzg2l_pinctrl_pinconf_get()
> and rzg2l_pinctrl_pinconf_set() as the offset & pin argument to the
> read_oen() and write_oen() functions. This doesn't make sense, the first
> of these isn't actually an offset and the second is not needed for
> RZ/V2H but leads to confusion with the bit variable used within these
> functions.
>
> To tidy this up, instead pass the _pin variable directly to the
> read_oen() and write_oen() functions with consistent naming. Then
> rzg3s_read_oen() and rzg3s_write_oen() can use macros to get the port
> and pin numbers it needs.
>
> Also, merge rzg3s_oen_is_supported() into rzg3s_pin_to_oen_bit() to give
> a single translation function which returns an error if the pin doesn't
> support OEN. While we're here, remove an unnecessary branch and clarify
> the variable naming.
>
> Signed-off-by: Paul Barker <paul.barker.ct@...renesas.com>
> ---
> Changes v1->v2:
>   * Merged patches 1 & 2 from the previous series, updated to be
>     compatible with recent patches adding RZ/V2H support.

Thanks for your patch!

> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c

> @@ -994,53 +994,43 @@ static bool rzg2l_ds_is_supported(struct rzg2l_pinctrl *pctrl, u32 caps,
>         return false;
>  }
>
> -static bool rzg3s_oen_is_supported(u32 caps, u8 pin, u8 max_pin)
> +static int rzg3s_pin_to_oen_bit(const struct rzg2l_hwcfg *hwcfg, u32 caps, u32 port, u8 pin)
>  {
> -       if (!(caps & PIN_CFG_OEN))
> -               return false;
> +       u8 bit = pin * 2;
>
> -       if (pin > max_pin)
> -               return false;
> +       if (!(caps & PIN_CFG_OEN) || pin > hwcfg->oen_max_pin)
> +               return -EINVAL;
>
> -       return true;
> +       if (port == hwcfg->oen_max_port)
> +               bit += 1;
> +
> +       return bit;
>  }
>
> -static u8 rzg3s_pin_to_oen_bit(u32 offset, u8 pin, u8 max_port)
> +static u32 rzg3s_read_oen(struct rzg2l_pinctrl *pctrl, u32 caps, unsigned int _pin)
>  {
> -       if (pin)
> -               pin *= 2;
> +       u32 port = RZG2L_PIN_ID_TO_PORT(_pin);
> +       u8 pin = RZG2L_PIN_ID_TO_PIN(_pin);

It's OK to use RZG2L_PIN_ID_TO_PIN() unconditionally, as RZ/G3S does
not have any dedicated pins with the OEN capability, right?

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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