linux-kernel - Re: [PATCH] gpiolib: allow line names from device props to override driver names

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: <CAHp75Ve06-3KU60-tXFR-KMw4iSX9rOSNF3Oq7vm+ST8Qc8ujA@mail.gmail.com>
Date:   Thu, 9 Dec 2021 14:12:25 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Peter Rosin <peda@...ntia.se>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        Alexander Dahl <ada@...rsis.com>,
        Ludovic Desroches <ludovic.desroches@...rochip.com>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: Re: [PATCH] gpiolib: allow line names from device props to override
 driver names

On Thu, Dec 9, 2021 at 1:32 PM Peter Rosin <peda@...ntia.se> wrote:
>
> Some gpio providers set names for gpio lines that match the names of

gpio -> GPIO here and everywhere else in the patch where it is applicable.

> the pins on the SoC, or variations on that theme. These names are
> generally generic, such as pioC12 in the at91 case. These generic names

"generally generic" (reminds me
https://en.wiktionary.org/wiki/%D0%BC%D0%B0%D1%81%D0%BB%D0%BE_%D0%BC%D0%B0%D1%81%D0%BB%D1%8F%D0%BD%D0%BE%D0%B5)

Perhaps

"These names generally speaking are generic, ..." ?

> block the possibility to name gpio lines with in device properties

within ?

> (i.e. gpio-line-names).
>
> Allow overriding a generic name given by the gpio driver if there is
> a name given to the gpio line using device properties, but leave the
> generic name alone if no better name is available.
>
> However, there is a risk. If user space is depending on the above
> mentioned fixed gpio names, AND there are device properties that
> previously did not reach the surface, the name change might cause
> regressions. But hopefully this stays below the radar...

After addressing the mentioned grammar nit-picks, FWIW,
Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
Thanks!

> Signed-off-by: Peter Rosin <peda@...ntia.se>
> ---
>  drivers/gpio/gpiolib.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
>
> Instead of doing this only for pinctrl-at91.c as in my recent patch [1], do
> it for everyone.
>
> Cheers,
> Peter
>
> [1] https://lore.kernel.org/lkml/4d17866a-d9a4-a3d7-189a-781d18dbea00@axentia.se/
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index abfbf546d159..00a2a689c202 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -422,8 +422,15 @@ static int devprop_gpiochip_set_names(struct gpio_chip *chip)
>         if (count > chip->ngpio)
>                 count = chip->ngpio;
>
> -       for (i = 0; i < count; i++)
> -               gdev->descs[i].name = names[chip->offset + i];
> +       for (i = 0; i < count; i++) {
> +               /*
> +                * Allow overriding "fixed" names provided by the gpio
> +                * provider, the "fixed" names are generally generic and less
> +                * informative than the names given in device properties.
> +                */
> +               if (names[chip->offset + i] && names[chip->offset + i][0])
> +                       gdev->descs[i].name = names[chip->offset + i];
> +       }
>
>         kfree(names);
>
> @@ -708,10 +715,12 @@ int gpiochip_add_data_with_key(struct gpio_chip *gc, void *data,
>         INIT_LIST_HEAD(&gdev->pin_ranges);
>  #endif
>
> -       if (gc->names)
> +       if (gc->names) {
>                 ret = gpiochip_set_desc_names(gc);
> -       else
> -               ret = devprop_gpiochip_set_names(gc);
> +               if (ret)
> +                       goto err_remove_from_list;
> +       }
> +       ret = devprop_gpiochip_set_names(gc);
>         if (ret)
>                 goto err_remove_from_list;
>
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko