Re: [PATCH v2 6/8] gpiolib: remove legacy gpio_export

[Arnd Bergmann]

On Tue, Nov 9, 2021 at 11:30 AM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 09, 2021 at 11:02:05AM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@xxxxxxxx>
> >
> > There are only a handful of users of gpio_export() and
> > related functions.
> >
> > As these are just wrappers around the modern gpiod_export()
> > helper, remove the wrappers and open-code the gpio_to_desc
> > in all callers to shrink the legacy API.
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
> What I wish to see meanwhile is a section in the TODO list somewhere to clean
> up those modules that have gpio.h. Linus, do we have one in the kernel or is
> it your personal TODO?
>
> In case we have one in the kernel, please add there modules you modified in
> a way that they still need further attention.

I think the TODO list is in Linus' head, but it would include all the files
that use one of the interfaces in linux/gpio.h. I found about 350 of them,
so there is little point in listing them one at a time. IIRC Linus is going
through those one subsystem at a time.

It might help to make it harder to get new users if we add some pattern
matching to scripts/checkpatch.pl, and/or something for scripts/coccinelle/.
I don't think it's possible to convert a gpio_request() user to gpio_get()
in a scripted way because you usually have to change the platform side
at the same time as the driver side.

I also found that we have a ton of users of linux/of_gpio.h, which is
somewhere inbetween the linux/gpio.h interface and the
linux/gpio/consumer.h version.

> > @@ -259,17 +259,19 @@ static int evm_sw_setup(struct i2c_client *client, int gpio,
> >       char label[10];
> >
> >       for (i = 0; i < 4; ++i) {
> > +             struct gpio_desc *desc = gpio_to_desc(gpio + i);
> > +
> >               snprintf(label, 10, "user_sw%d", i);
> > -             status = gpio_request(gpio, label);
> > +             status = gpio_request(gpio + i, label);
>
> Shouldn't be gpiod_get() or so at the end?

Yes, but that would be a more invasive change that I think should be done
separately.

        Arnd