[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMuHMdWKUq=Tf9zTx2LDr_gkbsxBPnQeK=buDSsQ__-QUHX5Gw@mail.gmail.com>
Date: Thu, 4 Jul 2024 19:28:24 +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 v3 2/9] pinctrl: renesas: rzg2l: Clean up and refactor OEN
read/write functions
Hi Paul,
On Tue, Jun 25, 2024 at 10:03 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
> oen_read() and oen_write() 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
> oen_read() and oen_write() functions with consistent naming. Then
> rzg3s_read_oen() and rzg3s_write_oen() can use macros to get the port
> and pin numbers it needs.
>
> Instead of passing the pin capabilities into oen_read() and oen_write(),
> we can check that the pin supports OEN before calling these functions.
>
> 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. Lastly, check that we are not dealing with a
> dedicated pin before calling RZG2L_PIN_ID_TO_{PORT,PIN}().
>
> Signed-off-by: Paul Barker <paul.barker.ct@...renesas.com>
> Acked-by: Linus Walleij <linus.walleij@...aro.org>
> ---
> Changes v2->v3:
> * Picked up Linus W's Acked-by tag.
> * Skipped Geert's Reviewed-by tag as I've made a few changes here and
> want another review.
> * Check pin caps before calling oen_{read,write}() so we can drop the
> caps argument to those functions.
> * Simplify arguments to rzg3s_pin_to_oen_bit() and decode
> pin_data/port/pin inside this function.
> * Check for dedicated pin before calling RZG2L_PIN_ID_TO_{PORT,PIN}().
Thanks for the update!
> --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> -static u32 rzg3s_oen_read(struct rzg2l_pinctrl *pctrl, u32 caps, u32 offset, u8 pin)
> +static int rzg3s_pin_to_oen_bit(struct rzg2l_pinctrl *pctrl, unsigned int _pin)
> {
> + u64 *pin_data = pctrl->desc.pins[_pin].drv_data;
> u8 max_port = pctrl->data->hwcfg->oen_max_port;
> u8 max_pin = pctrl->data->hwcfg->oen_max_pin;
As max_port and max_pin are used only once...
> - u8 bit;
> + u8 port, pin, bit;
>
> - if (!rzg3s_oen_is_supported(caps, pin, max_pin))
> - return 0;
> + if (*pin_data & RZG2L_SINGLE_PIN)
> + return -EINVAL;
>
> - bit = rzg3s_pin_to_oen_bit(offset, pin, max_port);
> + port = RZG2L_PIN_ID_TO_PORT(_pin);
> + pin = RZG2L_PIN_ID_TO_PIN(_pin);
> + if (pin > max_pin)
... I'd rather use their origins here...
> + return -EINVAL;
> +
> + bit = pin * 2;
> + if (port == max_port)
... and here.
I can do that while applying.
> + bit += 1;
> +
> + return bit;
> +}
Reviewed-by: Geert Uytterhoeven <geert+renesas@...der.be>
I'll check with Linus whether I can still queue this in renesas-pinctrl
for v6.11. As the first three patches of this series are a hard
dependency for the DTS parts, it would be good to get them in sooner
rather than later.
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
Powered by blists - more mailing lists