mbox series

[v3,00/12] gpiolib cleanups

Message ID 20230207142952.51844-1-andriy.shevchenko@linux.intel.com
Headers show
Series gpiolib cleanups | expand

Message

Andy Shevchenko Feb. 7, 2023, 2:29 p.m. UTC
These are some older patches Arnd did last year, rebased to
linux-next-20230207. On top there are Andy's patches regarding
similar topic.

The main goal is to remove some of the legacy bits of the gpiolib
interfaces, where the corner cases are easily avoided or replaced
with gpio descriptor based interfaces.

Changes in v3:
- reworked touchscreen patch in accordance with Dmitry's comments
- rebased on the latest Linux Next
- added on top Andy's series

Changes in v2:
- dropped patch 8 after Andy's identical patch was merged
- rebase on latest gpio tree
- leave unused gpio_cansleep() in place for now
- address feedback from Andy Shevchenko

Andy Shevchenko (5):
  gpio: aggregator: Add missing header(s)
  gpiolib: Drop unused forward declaration from driver.h
  gpiolib: Deduplicate forward declarations in consumer.h
  gpiolib: Group forward declarations in consumer.h
  gpiolib: Clean up headers

Arnd Bergmann (7):
  gpiolib: remove empty asm/gpio.h files
  gpiolib: coldfire: remove custom asm/gpio.h
  gpiolib: remove asm-generic/gpio.h
  gpiolib: remove gpio_set_debounce
  gpiolib: remove legacy gpio_export
  gpiolib: split linux/gpio/driver.h out of linux/gpio.h
  gpiolib: split of_mm_gpio_chip out of linux/of_gpio.h

 Documentation/admin-guide/gpio/sysfs.rst      |   2 +-
 Documentation/driver-api/gpio/legacy.rst      |  23 ---
 .../zh_CN/driver-api/gpio/legacy.rst          |  20 ---
 Documentation/translations/zh_TW/gpio.txt     |  19 ---
 MAINTAINERS                                   |   1 -
 arch/arm/Kconfig                              |   1 -
 arch/arm/include/asm/gpio.h                   |  21 ---
 arch/arm/mach-omap1/irq.c                     |   1 +
 arch/arm/mach-omap2/pdata-quirks.c            |   9 +-
 arch/arm/mach-orion5x/board-rd88f5182.c       |   1 +
 arch/arm/mach-s3c/s3c64xx.c                   |   1 +
 arch/arm/mach-sa1100/assabet.c                |   1 +
 arch/arm/plat-orion/gpio.c                    |   1 +
 arch/m68k/Kconfig.cpu                         |   1 -
 arch/m68k/include/asm/gpio.h                  |  95 -----------
 arch/m68k/include/asm/mcfgpio.h               |   2 +-
 arch/powerpc/platforms/44x/Kconfig            |   1 +
 arch/powerpc/platforms/4xx/gpio.c             |   2 +-
 arch/powerpc/platforms/8xx/Kconfig            |   1 +
 arch/powerpc/platforms/8xx/cpm1.c             |   2 +-
 arch/powerpc/platforms/Kconfig                |   2 +
 arch/powerpc/sysdev/cpm_common.c              |   2 +-
 arch/sh/Kconfig                               |   1 -
 arch/sh/boards/board-magicpanelr2.c           |   1 +
 arch/sh/boards/mach-ap325rxa/setup.c          |   7 +-
 arch/sh/include/asm/gpio.h                    |  45 ------
 drivers/gpio/Kconfig                          |  19 ++-
 drivers/gpio/TODO                             |  15 +-
 drivers/gpio/gpio-aggregator.c                |   9 +-
 drivers/gpio/gpio-altera.c                    |   2 +-
 drivers/gpio/gpio-davinci.c                   |   2 -
 drivers/gpio/gpio-mm-lantiq.c                 |   2 +-
 drivers/gpio/gpio-mpc5200.c                   |   2 +-
 drivers/gpio/gpiolib-acpi.c                   |  10 +-
 drivers/gpio/gpiolib-acpi.h                   |   1 -
 drivers/gpio/gpiolib-of.c                     |   9 +-
 drivers/gpio/gpiolib-of.h                     |   1 -
 drivers/gpio/gpiolib-swnode.c                 |   5 +-
 drivers/gpio/gpiolib-sysfs.c                  |  25 ++-
 drivers/gpio/gpiolib.c                        |   9 +-
 drivers/input/touchscreen/ads7846.c           |  24 +--
 drivers/media/pci/sta2x11/sta2x11_vip.c       |  10 +-
 drivers/net/ieee802154/ca8210.c               |   3 +-
 .../broadcom/brcm80211/brcmsmac/led.c         |   1 +
 drivers/pinctrl/core.c                        |   1 -
 drivers/soc/fsl/qe/gpio.c                     |   2 +-
 include/asm-generic/gpio.h                    | 147 ------------------
 include/linux/gpio.h                          | 100 +++++++-----
 include/linux/gpio/consumer.h                 |  24 +--
 include/linux/gpio/driver.h                   |  31 +++-
 .../legacy-of-mm-gpiochip.h}                  |  33 +---
 include/linux/mfd/ucb1x00.h                   |   1 +
 include/linux/of_gpio.h                       |  21 ---
 53 files changed, 223 insertions(+), 549 deletions(-)
 delete mode 100644 arch/arm/include/asm/gpio.h
 delete mode 100644 arch/m68k/include/asm/gpio.h
 delete mode 100644 arch/sh/include/asm/gpio.h
 delete mode 100644 include/asm-generic/gpio.h
 copy include/linux/{of_gpio.h => gpio/legacy-of-mm-gpiochip.h} (50%)

Comments

Vincenzo Palazzo Feb. 7, 2023, 8:53 p.m. UTC | #1
> From: Arnd Bergmann <arnd@arndb.de>
>
> The arm and sh versions of this file are identical to the generic
> versions and can just be removed.
>
> The drivers that actually use the sh3 specific version also include
> cpu/gpio.h directly, with the exception of magicpanelr2, which is
> easily fixed. This leaves coldfire as the only gpio driver
> that needs something custom for gpiolib.
>
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Vincenzo Palazzo <vincenzopalazzodev@gmail.com>
Dmitry Torokhov Feb. 7, 2023, 9:32 p.m. UTC | #2
On Tue, Feb 07, 2023 at 04:29:44PM +0200, Andy Shevchenko wrote:
> @@ -1010,14 +1009,21 @@ static int ads7846_setup_pendown(struct spi_device *spi,
>  		}
>  
>  		ts->gpio_pendown = pdata->gpio_pendown;
> -
> -		if (pdata->gpio_pendown_debounce)
> -			gpio_set_debounce(pdata->gpio_pendown,
> -					  pdata->gpio_pendown_debounce);

Can we please change only this to:

			gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown),
					   pdata->gpio_pendown_debounce);

and not change anything else (i.e. drop the changes below)?

>  	} else {
> -		dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> -		return -EINVAL;
> +		struct gpio_desc *desc;
> +
> +		desc = devm_gpiod_get(&spi->dev, "pendown", GPIOD_IN);
> +		if (IS_ERR(desc)) {
> +			dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> +			return PTR_ERR(desc);
> +		}
> +		gpiod_set_consumer_name(desc, "ads7846_pendown");
> +
> +		ts->gpio_pendown = desc_to_gpio(desc);
>  	}
> +	if (pdata->gpio_pendown_debounce)
> +		gpiod_set_debounce(gpio_to_desc(ts->gpio_pendown),
> +				   pdata->gpio_pendown_debounce);
>  
>  	return 0;

Thanks.
Andy Shevchenko Feb. 7, 2023, 10:56 p.m. UTC | #3
On Tue, Feb 07, 2023 at 01:32:01PM -0800, Dmitry Torokhov wrote:
> On Tue, Feb 07, 2023 at 04:29:44PM +0200, Andy Shevchenko wrote:
> > @@ -1010,14 +1009,21 @@ static int ads7846_setup_pendown(struct spi_device *spi,
> >  		}
> >  
> >  		ts->gpio_pendown = pdata->gpio_pendown;
> > -
> > -		if (pdata->gpio_pendown_debounce)
> > -			gpio_set_debounce(pdata->gpio_pendown,
> > -					  pdata->gpio_pendown_debounce);
> 
> Can we please change only this to:
> 
> 			gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown),
> 					   pdata->gpio_pendown_debounce);
> 
> and not change anything else (i.e. drop the changes below)?

Probably. I can try rollback this.

> >  	} else {
> > -		dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> > -		return -EINVAL;
> > +		struct gpio_desc *desc;
> > +
> > +		desc = devm_gpiod_get(&spi->dev, "pendown", GPIOD_IN);
> > +		if (IS_ERR(desc)) {
> > +			dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> > +			return PTR_ERR(desc);
> > +		}
> > +		gpiod_set_consumer_name(desc, "ads7846_pendown");
> > +
> > +		ts->gpio_pendown = desc_to_gpio(desc);
> >  	}
> > +	if (pdata->gpio_pendown_debounce)
> > +		gpiod_set_debounce(gpio_to_desc(ts->gpio_pendown),
> > +				   pdata->gpio_pendown_debounce);
> >  
> >  	return 0;
Geert Uytterhoeven Feb. 8, 2023, 10:08 a.m. UTC | #4
Hi Andy,

Thanks for your patch!

On Tue, Feb 7, 2023 at 3:29 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> Do not imply that some of the generic headers may be always included.
> Instead, include explicitly what we are direct user of.

That applies only to the addition of #include <linux/slab.h>...
Please also describe the other changes.

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/gpio/gpio-aggregator.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpio/gpio-aggregator.c b/drivers/gpio/gpio-aggregator.c
> index 6d17d262ad91..20a686f12df7 100644
> --- a/drivers/gpio/gpio-aggregator.c
> +++ b/drivers/gpio/gpio-aggregator.c
> @@ -10,19 +10,20 @@
>  #include <linux/bitmap.h>
>  #include <linux/bitops.h>
>  #include <linux/ctype.h>
> -#include <linux/gpio.h>
> -#include <linux/gpio/consumer.h>
> -#include <linux/gpio/driver.h>
> -#include <linux/gpio/machine.h>
>  #include <linux/idr.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/overflow.h>
>  #include <linux/platform_device.h>
> +#include <linux/slab.h>
>  #include <linux/spinlock.h>
>  #include <linux/string.h>
>
> +#include <linux/gpio/consumer.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/gpio/machine.h>
> +
>  #define AGGREGATOR_MAX_GPIOS 512

For the actual changes:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

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