[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YYpMcKlcZ3JWqp5M@smile.fi.intel.com>
Date: Tue, 9 Nov 2021 12:24:48 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Arnd Bergmann <arnd@...nel.org>
Cc: linux-gpio@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
Linus Walleij <linus.walleij@...aro.org>,
Bartosz Golaszewski <brgl@...ev.pl>,
Geert Uytterhoeven <geert+renesas@...der.be>,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 5/8] gpiolib: shrink further
On Tue, Nov 09, 2021 at 11:02:04AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@...db.de>
>
> gpio_set_debounce() only has a single user, which is trivially
> converted to gpiod_set_debounce(), while gpio_cansleep() and
> devm_gpio_free() have no users at all.
>
> Remove them all to shrink the old gpio interface.
A couple of nit-picks below.
In either case,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> .../driver-api/driver-model/devres.rst | 1 -
> Documentation/driver-api/gpio/legacy.rst | 2 --
> drivers/gpio/gpiolib-devres.c | 25 ----------------
> drivers/input/touchscreen/ads7846.c | 3 +-
> include/linux/gpio.h | 29 -------------------
> 5 files changed, 2 insertions(+), 58 deletions(-)
>
> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
> index 148e19381b79..52821478decd 100644
> --- a/Documentation/driver-api/driver-model/devres.rst
> +++ b/Documentation/driver-api/driver-model/devres.rst
> @@ -277,7 +277,6 @@ GPIO
> devm_gpiochip_add_data()
> devm_gpio_request()
> devm_gpio_request_one()
> - devm_gpio_free()
>
> I2C
> devm_i2c_new_dummy_device()
> diff --git a/Documentation/driver-api/gpio/legacy.rst b/Documentation/driver-api/gpio/legacy.rst
> index 06c05e2d62c1..eae185f771d7 100644
> --- a/Documentation/driver-api/gpio/legacy.rst
> +++ b/Documentation/driver-api/gpio/legacy.rst
> @@ -238,8 +238,6 @@ setup or driver probe/teardown code, so this is an easy constraint.)::
> ## gpio_free_array()
>
> gpio_free()
> - gpio_set_debounce()
> -
>
One more blank line removal?
>
> Claiming and Releasing GPIOs
> diff --git a/drivers/gpio/gpiolib-devres.c b/drivers/gpio/gpiolib-devres.c
> index 79da85d17b71..55465ead492f 100644
> --- a/drivers/gpio/gpiolib-devres.c
> +++ b/drivers/gpio/gpiolib-devres.c
> @@ -385,13 +385,6 @@ static void devm_gpio_release(struct device *dev, void *res)
> gpio_free(*gpio);
> }
>
> -static int devm_gpio_match(struct device *dev, void *res, void *data)
> -{
> - unsigned *this = res, *gpio = data;
> -
> - return *this == *gpio;
> -}
> -
> /**
> * devm_gpio_request - request a GPIO for a managed device
> * @dev: device to request the GPIO for
> @@ -459,24 +452,6 @@ int devm_gpio_request_one(struct device *dev, unsigned gpio,
> }
> EXPORT_SYMBOL_GPL(devm_gpio_request_one);
>
> -/**
> - * devm_gpio_free - free a GPIO
> - * @dev: device to free GPIO for
> - * @gpio: GPIO to free
> - *
> - * Except for the extra @dev argument, this function takes the
> - * same arguments and performs the same function as gpio_free().
> - * This function instead of gpio_free() should be used to manually
> - * free GPIOs allocated with devm_gpio_request().
> - */
> -void devm_gpio_free(struct device *dev, unsigned int gpio)
> -{
> -
> - WARN_ON(devres_release(dev, devm_gpio_release, devm_gpio_match,
> - &gpio));
> -}
> -EXPORT_SYMBOL_GPL(devm_gpio_free);
> -
> static void devm_gpio_chip_release(void *data)
> {
> struct gpio_chip *gc = data;
> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> index a25a77dd9a32..d0664e3b89bb 100644
> --- a/drivers/input/touchscreen/ads7846.c
> +++ b/drivers/input/touchscreen/ads7846.c
> @@ -27,6 +27,7 @@
> #include <linux/of.h>
> #include <linux/of_gpio.h>
(1)
> #include <linux/of_device.h>
> +#include <linux/gpio/consumer.h>
(2)
> #include <linux/gpio.h>
(3)
Seems too many...
Are you planning to clean up this driver to get rid of (1) and (3) altogether?
(I understand that for current purposes above is a good quick cleanup)
> #include <linux/spi/spi.h>
> #include <linux/spi/ads7846.h>
> @@ -1018,7 +1019,7 @@ 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,
> + gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown),
> pdata->gpio_pendown_debounce);
> } else {
> dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
> diff --git a/include/linux/gpio.h b/include/linux/gpio.h
> index c19256f67e02..64cc8f09eba8 100644
> --- a/include/linux/gpio.h
> +++ b/include/linux/gpio.h
> @@ -117,11 +117,6 @@ static inline int gpio_direction_output(unsigned gpio, int value)
> return gpiod_direction_output_raw(gpio_to_desc(gpio), value);
> }
>
> -static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
> -{
> - return gpiod_set_debounce(gpio_to_desc(gpio), debounce);
> -}
> -
> static inline int gpio_get_value_cansleep(unsigned gpio)
> {
> return gpiod_get_raw_value_cansleep(gpio_to_desc(gpio));
> @@ -140,11 +135,6 @@ static inline void gpio_set_value(unsigned gpio, int value)
> return gpiod_set_raw_value(gpio_to_desc(gpio), value);
> }
>
> -static inline int gpio_cansleep(unsigned gpio)
> -{
> - return gpiod_cansleep(gpio_to_desc(gpio));
> -}
> -
> static inline int gpio_to_irq(unsigned gpio)
> {
> return gpiod_to_irq(gpio_to_desc(gpio));
> @@ -181,8 +171,6 @@ struct device;
> int devm_gpio_request(struct device *dev, unsigned gpio, const char *label);
> int devm_gpio_request_one(struct device *dev, unsigned gpio,
> unsigned long flags, const char *label);
> -void devm_gpio_free(struct device *dev, unsigned int gpio);
> -
> #else /* ! CONFIG_GPIOLIB */
>
> #include <linux/kernel.h>
> @@ -239,11 +227,6 @@ static inline int gpio_direction_output(unsigned gpio, int value)
> return -ENOSYS;
> }
>
> -static inline int gpio_set_debounce(unsigned gpio, unsigned debounce)
> -{
> - return -ENOSYS;
> -}
> -
> static inline int gpio_get_value(unsigned gpio)
> {
> /* GPIO can never have been requested or set as {in,out}put */
> @@ -257,13 +240,6 @@ static inline void gpio_set_value(unsigned gpio, int value)
> WARN_ON(1);
> }
>
> -static inline int gpio_cansleep(unsigned gpio)
> -{
> - /* GPIO can never have been requested or set as {in,out}put */
> - WARN_ON(1);
> - return 0;
> -}
> -
> static inline int gpio_get_value_cansleep(unsigned gpio)
> {
> /* GPIO can never have been requested or set as {in,out}put */
> @@ -319,11 +295,6 @@ static inline int devm_gpio_request_one(struct device *dev, unsigned gpio,
> return -EINVAL;
> }
>
> -static inline void devm_gpio_free(struct device *dev, unsigned int gpio)
> -{
> - WARN_ON(1);
> -}
> -
> #endif /* ! CONFIG_GPIOLIB */
>
> #endif /* __LINUX_GPIO_H */
> --
> 2.29.2
>
--
With Best Regards,
Andy Shevchenko
Powered by blists - more mailing lists