Re: [PATCH v2 5/8] gpiolib: shrink further

[Arnd Bergmann <arnd@xxxxxxxxxx>]

On Tue, Nov 9, 2021 at 11:17 PM Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
>
> On Tue, Nov 9, 2021 at 12:18 PM Arnd Bergmann <arnd@xxxxxxxxxx> wrote:
>
> > Ideally we should only use linux/gpio/consumer.h, which is required for
> > gpiod_set_debounce(). of_gpio.h is still needed for of_get_named_gpio()
> > and should be taken out once we change this to gpiod_get(), while
> > linux/gpio.h is still needed for gpio_is_valid()/gpio_get_value() and should
> > be removed when those are changed to the gpiod_ versions.
> >
> > We could do an intermediate patch that converts one half of the
> > interface, something like
>
> When I convert stuff I try to go all the way when I can. It can
> be a bit daring if no one is there to test changes.
>
> The patch looks good though apart from:
>
> > -               ts->gpio_pendown = pdata->gpio_pendown;
> > +               ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown);
>
> I usually even go into the defined platform data and try to convert
> the boardfile to use a descriptor table so this is never needed.
> (But, more work.)

Yes, I noticed. I had done some conversions for pxa this way, I should
look in my tree if I should resend those.

My hope would be that by making the steps smaller, it's easier to find
people that are willing and able to help out. From looking at it so far,
I would partition the problem something like:

a) Remove the (now) trivial wrappers around gpiod_*() functions
by using open-coded gpio_to_desc() calls everywhere. This doesn't
improve the code, but it can be trivially scripted and it should help
by making it less practical to put new users in.

b) one driver/subsystem at a time, replace all calls to
{devm_,}gpio_{free,request{,_one}} with a new
struct gpio_desc *gpiod_get_legacy(struct device *dev, int gpio, enum
gpiod_flags flags);
This takes the conversion only half-way, but is much more manageable
for a random contributor or reviewer, and it undoes the ugly bits
added in step a), making it a clear improvement.

c) convert the boardfile/platform_data/of_get_named_gpio side along with
the corresponding s/gpiod_get_legacy/gpiod_get/, which is now a fairly simple
change on the driver side, while the platform side can be reviewed by
the platform
maintainers.

> Examples:
> git log -p --author=Walleij arch/arm/mach-pxa/
>
> > -       pdata->gpio_pendown = of_get_named_gpio(dev->of_node,
> > "pendown-gpio", 0);
> > +       ts->gpio_pendown = gpiod_get(dev, "pendown-gpio", GPIOD_IN);
>
> Needs to be just gpiod_get(dev, "pendown", GPIOD_IN); the new
> API tries the "-gpio[s]" suffixes when going into the device tree.

Ok, got it.

        Arnd