| Message ID | 20230127101149.3475929-5-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib cleanups | expand |
On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gpio_set_debounce() only has a single user, which is trivially > converted to gpiod_set_debounce(). LGTM, thanks! Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > Documentation/driver-api/gpio/legacy.rst | 2 -- > .../zh_CN/driver-api/gpio/legacy.rst | 1 - > Documentation/translations/zh_TW/gpio.txt | 1 - > drivers/input/touchscreen/ads7846.c | 25 ++++++++++--------- > include/linux/gpio.h | 10 -------- > 5 files changed, 13 insertions(+), 26 deletions(-) > > diff --git a/Documentation/driver-api/gpio/legacy.rst b/Documentation/driver-api/gpio/legacy.rst > index a0559d93efd1..e0306e78e34b 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() > - > > > Claiming and Releasing GPIOs > diff --git a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > index 74fa473bb504..dee2a0517c1c 100644 > --- a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > +++ b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > @@ -219,7 +219,6 @@ GPIO 值的命令需要等待其信息排到队首才发送命令，再获得其 > ## gpio_free_array() > > gpio_free() > - gpio_set_debounce() > > > > diff --git a/Documentation/translations/zh_TW/gpio.txt b/Documentation/translations/zh_TW/gpio.txt > index 1b986bbb0909..dc608358d90a 100644 > --- a/Documentation/translations/zh_TW/gpio.txt > +++ b/Documentation/translations/zh_TW/gpio.txt > @@ -226,7 +226,6 @@ GPIO 值的命令需要等待其信息排到隊首才發送命令，再獲得其 > ## gpio_free_array() > > gpio_free() > - gpio_set_debounce() > > > > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c > index 4c3dd01902d0..da3c55d9cb98 100644 > --- a/drivers/input/touchscreen/ads7846.c > +++ b/drivers/input/touchscreen/ads7846.c > @@ -25,8 +25,8 @@ > #include <linux/slab.h> > #include <linux/pm.h> > #include <linux/of.h> > -#include <linux/of_gpio.h> > #include <linux/of_device.h> > +#include <linux/gpio/consumer.h> > #include <linux/gpio.h> > #include <linux/spi/spi.h> > #include <linux/spi/ads7846.h> > @@ -139,7 +139,7 @@ struct ads7846 { > int (*filter)(void *data, int data_idx, int *val); > void *filter_data; > int (*get_pendown_state)(void); > - int gpio_pendown; > + struct gpio_desc *gpio_pendown; > > void (*wait_for_sync)(void); > }; > @@ -222,7 +222,7 @@ static int get_pendown_state(struct ads7846 *ts) > if (ts->get_pendown_state) > return ts->get_pendown_state(); > > - return !gpio_get_value(ts->gpio_pendown); > + return !gpiod_get_value(ts->gpio_pendown); > } > > static void ads7846_report_pen_up(struct ads7846 *ts) > @@ -1005,7 +1005,6 @@ static int ads7846_setup_pendown(struct spi_device *spi, > if (pdata->get_pendown_state) { > ts->get_pendown_state = pdata->get_pendown_state; > } else if (gpio_is_valid(pdata->gpio_pendown)) { > - > err = devm_gpio_request_one(&spi->dev, pdata->gpio_pendown, > GPIOF_IN, "ads7846_pendown"); > if (err) { > @@ -1015,15 +1014,17 @@ static int ads7846_setup_pendown(struct spi_device *spi, > return err; > } > > - ts->gpio_pendown = pdata->gpio_pendown; > - > - if (pdata->gpio_pendown_debounce) > - gpio_set_debounce(pdata->gpio_pendown, > - pdata->gpio_pendown_debounce); > + ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown); > } else { > - dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); > - return -EINVAL; > + ts->gpio_pendown = gpiod_get(&spi->dev, "pendown-gpio", GPIOD_IN); > + if (IS_ERR(ts->gpio_pendown)) { > + dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); > + return PTR_ERR(ts->gpio_pendown); > + } > } > + if (pdata->gpio_pendown_debounce) > + gpiod_set_debounce(ts->gpio_pendown, > + pdata->gpio_pendown_debounce); > > return 0; > } > @@ -1192,7 +1193,7 @@ static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev) > pdata->wakeup = of_property_read_bool(node, "wakeup-source") || > of_property_read_bool(node, "linux,wakeup"); > > - pdata->gpio_pendown = of_get_named_gpio(dev->of_node, "pendown-gpio", 0); > + pdata->gpio_pendown = -1; > > return pdata; > } > diff --git a/include/linux/gpio.h b/include/linux/gpio.h > index 6719a82eeec5..220e8656f2ab 100644 > --- a/include/linux/gpio.h > +++ b/include/linux/gpio.h > @@ -100,11 +100,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)); > @@ -215,11 +210,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 */ > -- > 2.39.0 >
On Fri, Jan 27, 2023 at 11:12 AM Arnd Bergmann <arnd@kernel.org> wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gpio_set_debounce() only has a single user, which is trivially > converted to gpiod_set_debounce(). > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > gpio_set_debounce() only has a single user, which is trivially > converted to gpiod_set_debounce(). > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > Documentation/driver-api/gpio/legacy.rst | 2 -- > .../zh_CN/driver-api/gpio/legacy.rst | 1 - > Documentation/translations/zh_TW/gpio.txt | 1 - > drivers/input/touchscreen/ads7846.c | 25 ++++++++++--------- > include/linux/gpio.h | 10 -------- > 5 files changed, 13 insertions(+), 26 deletions(-) > > diff --git a/Documentation/driver-api/gpio/legacy.rst b/Documentation/driver-api/gpio/legacy.rst > index a0559d93efd1..e0306e78e34b 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() > - > > > Claiming and Releasing GPIOs > diff --git a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > index 74fa473bb504..dee2a0517c1c 100644 > --- a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > +++ b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst > @@ -219,7 +219,6 @@ GPIO 值的命令需要等待其信息排到队首才发送命令，再获得其 > ## gpio_free_array() > > gpio_free() > - gpio_set_debounce() > > > > diff --git a/Documentation/translations/zh_TW/gpio.txt b/Documentation/translations/zh_TW/gpio.txt > index 1b986bbb0909..dc608358d90a 100644 > --- a/Documentation/translations/zh_TW/gpio.txt > +++ b/Documentation/translations/zh_TW/gpio.txt > @@ -226,7 +226,6 @@ GPIO 值的命令需要等待其信息排到隊首才發送命令，再獲得其 > ## gpio_free_array() > > gpio_free() > - gpio_set_debounce() > > > > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c > index 4c3dd01902d0..da3c55d9cb98 100644 > --- a/drivers/input/touchscreen/ads7846.c > +++ b/drivers/input/touchscreen/ads7846.c > @@ -25,8 +25,8 @@ > #include <linux/slab.h> > #include <linux/pm.h> > #include <linux/of.h> > -#include <linux/of_gpio.h> > #include <linux/of_device.h> > +#include <linux/gpio/consumer.h> > #include <linux/gpio.h> > #include <linux/spi/spi.h> > #include <linux/spi/ads7846.h> > @@ -139,7 +139,7 @@ struct ads7846 { > int (*filter)(void *data, int data_idx, int *val); > void *filter_data; > int (*get_pendown_state)(void); > - int gpio_pendown; > + struct gpio_desc *gpio_pendown; > > void (*wait_for_sync)(void); > }; > @@ -222,7 +222,7 @@ static int get_pendown_state(struct ads7846 *ts) > if (ts->get_pendown_state) > return ts->get_pendown_state(); > > - return !gpio_get_value(ts->gpio_pendown); > + return !gpiod_get_value(ts->gpio_pendown); No, we can not blindly do that without checking annotations on GPIOs. > + ts->gpio_pendown = gpiod_get(&spi->dev, "pendown-gpio", GPIOD_IN); Wrong name, you will be looking for "pendown-gpio-gpios". Sorry, but I have to NAK this in the current form. Thanks.
On Tue, Jan 31, 2023 at 09:44:31PM -0800, Dmitry Torokhov wrote: > On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: ... > > - return !gpio_get_value(ts->gpio_pendown); > > + return !gpiod_get_value(ts->gpio_pendown); > > No, we can not blindly do that without checking annotations on GPIOs. But this is easy to fix, i.e. use raw API, no? > > + ts->gpio_pendown = gpiod_get(&spi->dev, "pendown-gpio", GPIOD_IN); > > Wrong name, you will be looking for "pendown-gpio-gpios". > > Sorry, but I have to NAK this in the current form. Oh, indeed. On top of that the conversion missing the label to be set.
On Wed, Feb 01, 2023 at 08:32:06PM +0200, Andy Shevchenko wrote: > On Tue, Jan 31, 2023 at 09:44:31PM -0800, Dmitry Torokhov wrote: > > On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: > > ... > > > > - return !gpio_get_value(ts->gpio_pendown); > > > + return !gpiod_get_value(ts->gpio_pendown); > > > > No, we can not blindly do that without checking annotations on GPIOs. > > But this is easy to fix, i.e. use raw API, no? I'd rather not (I hope I can make this driver respect declared polarity at some point), so for debounce we could do: gpiod_set_debounce(gpio_to_gpiod(), ...); in ads7846 for now, and get rid of gpio_set_debounce() as a publc API. Thanks.
On Wed, Feb 01, 2023 at 11:07:35AM -0800, Dmitry Torokhov wrote: > On Wed, Feb 01, 2023 at 08:32:06PM +0200, Andy Shevchenko wrote: > > On Tue, Jan 31, 2023 at 09:44:31PM -0800, Dmitry Torokhov wrote: > > > On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: ... > > > > - return !gpio_get_value(ts->gpio_pendown); > > > > + return !gpiod_get_value(ts->gpio_pendown); > > > > > > No, we can not blindly do that without checking annotations on GPIOs. > > > > But this is easy to fix, i.e. use raw API, no? > > I'd rather not (I hope I can make this driver respect declared polarity > at some point), so for debounce we could do: > > gpiod_set_debounce(gpio_to_gpiod(), ...); > > in ads7846 for now, and get rid of gpio_set_debounce() as a publc API. This will work and we can keep it for a while (gpio_to_desc(), I believe you meant this one, is part of the new API to keep this bridge for the cases like this). Arnd, are you going to send a v3? It would be really nice to have less collisions next cycle if your series is applied.
On Tue, Feb 7, 2023, at 12:25, Andy Shevchenko wrote: > On Wed, Feb 01, 2023 at 11:07:35AM -0800, Dmitry Torokhov wrote: >> On Wed, Feb 01, 2023 at 08:32:06PM +0200, Andy Shevchenko wrote: >> > On Tue, Jan 31, 2023 at 09:44:31PM -0800, Dmitry Torokhov wrote: >> > > On Fri, Jan 27, 2023 at 11:11:46AM +0100, Arnd Bergmann wrote: > > ... > >> > > > - return !gpio_get_value(ts->gpio_pendown); >> > > > + return !gpiod_get_value(ts->gpio_pendown); >> > > >> > > No, we can not blindly do that without checking annotations on GPIOs. >> > >> > But this is easy to fix, i.e. use raw API, no? >> >> I'd rather not (I hope I can make this driver respect declared polarity >> at some point), so for debounce we could do: >> >> gpiod_set_debounce(gpio_to_gpiod(), ...); >> >> in ads7846 for now, and get rid of gpio_set_debounce() as a publc API. > > This will work and we can keep it for a while (gpio_to_desc(), I believe you > meant this one, is part of the new API to keep this bridge for the cases like > this). > > Arnd, are you going to send a v3? It would be really nice to have less > collisions next cycle if your series is applied. I was planning to, but I see you beat me to it already, sorry for dropping the ball here and thanks for picking it up! Arnd
diff --git a/Documentation/driver-api/gpio/legacy.rst b/Documentation/driver-api/gpio/legacy.rst index a0559d93efd1..e0306e78e34b 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() - Claiming and Releasing GPIOs diff --git a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst index 74fa473bb504..dee2a0517c1c 100644 --- a/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst +++ b/Documentation/translations/zh_CN/driver-api/gpio/legacy.rst @@ -219,7 +219,6 @@ GPIO 值的命令需要等待其信息排到队首才发送命令，再获得其 ## gpio_free_array() gpio_free() - gpio_set_debounce() diff --git a/Documentation/translations/zh_TW/gpio.txt b/Documentation/translations/zh_TW/gpio.txt index 1b986bbb0909..dc608358d90a 100644 --- a/Documentation/translations/zh_TW/gpio.txt +++ b/Documentation/translations/zh_TW/gpio.txt @@ -226,7 +226,6 @@ GPIO 值的命令需要等待其信息排到隊首才發送命令，再獲得其 ## gpio_free_array() gpio_free() - gpio_set_debounce() diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index 4c3dd01902d0..da3c55d9cb98 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -25,8 +25,8 @@ #include <linux/slab.h> #include <linux/pm.h> #include <linux/of.h> -#include <linux/of_gpio.h> #include <linux/of_device.h> +#include <linux/gpio/consumer.h> #include <linux/gpio.h> #include <linux/spi/spi.h> #include <linux/spi/ads7846.h> @@ -139,7 +139,7 @@ struct ads7846 { int (*filter)(void *data, int data_idx, int *val); void *filter_data; int (*get_pendown_state)(void); - int gpio_pendown; + struct gpio_desc *gpio_pendown; void (*wait_for_sync)(void); }; @@ -222,7 +222,7 @@ static int get_pendown_state(struct ads7846 *ts) if (ts->get_pendown_state) return ts->get_pendown_state(); - return !gpio_get_value(ts->gpio_pendown); + return !gpiod_get_value(ts->gpio_pendown); } static void ads7846_report_pen_up(struct ads7846 *ts) @@ -1005,7 +1005,6 @@ static int ads7846_setup_pendown(struct spi_device *spi, if (pdata->get_pendown_state) { ts->get_pendown_state = pdata->get_pendown_state; } else if (gpio_is_valid(pdata->gpio_pendown)) { - err = devm_gpio_request_one(&spi->dev, pdata->gpio_pendown, GPIOF_IN, "ads7846_pendown"); if (err) { @@ -1015,15 +1014,17 @@ static int ads7846_setup_pendown(struct spi_device *spi, return err; } - ts->gpio_pendown = pdata->gpio_pendown; - - if (pdata->gpio_pendown_debounce) - gpio_set_debounce(pdata->gpio_pendown, - pdata->gpio_pendown_debounce); + ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown); } else { - dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); - return -EINVAL; + ts->gpio_pendown = gpiod_get(&spi->dev, "pendown-gpio", GPIOD_IN); + if (IS_ERR(ts->gpio_pendown)) { + dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); + return PTR_ERR(ts->gpio_pendown); + } } + if (pdata->gpio_pendown_debounce) + gpiod_set_debounce(ts->gpio_pendown, + pdata->gpio_pendown_debounce); return 0; } @@ -1192,7 +1193,7 @@ static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev) pdata->wakeup = of_property_read_bool(node, "wakeup-source") || of_property_read_bool(node, "linux,wakeup"); - pdata->gpio_pendown = of_get_named_gpio(dev->of_node, "pendown-gpio", 0); + pdata->gpio_pendown = -1; return pdata; } diff --git a/include/linux/gpio.h b/include/linux/gpio.h index 6719a82eeec5..220e8656f2ab 100644 --- a/include/linux/gpio.h +++ b/include/linux/gpio.h @@ -100,11 +100,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)); @@ -215,11 +210,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 */