Re: [PATCH v2 5/8] gpiolib: shrink further
From: Arnd Bergmann <arnd@kernel.org>
Date: 2021-11-09 11:18:38
Also in:
linux-gpio, lkml
On Tue, Nov 9, 2021 at 11:24 AM Andy Shevchenko [off-list ref] wrote:
quoted
@@ -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?
I think two blank lines at the end of a section is the normal style for this file. Do you mean I should remove another line, or not remove the third blank line here?
quoted
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>quoted
#include <linux/of_gpio.h>(1)quoted
#include <linux/of_device.h>quoted
+#include <linux/gpio/consumer.h>(2)quoted
#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)
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
diff --git a/drivers/input/touchscreen/ads7846.cb/drivers/input/touchscreen/ads7846.c index d0664e3b89bb..60e6b292ccdf 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c@@ -140,7 +140,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); };
@@ -223,7 +223,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,6 +1005,11 @@ static int ads7846_setup_pendown(struct spi_device *spi, if (pdata->get_pendown_state) { ts->get_pendown_state = pdata->get_pendown_state; + } else if (ts->gpio_pendown) { + if (IS_ERR(ts->gpio_pendown)) { + dev_err(&spi->dev, "missing pendown gpio\n"); + return PTR_ERR(ts->gpio_pendown); + } } else if (gpio_is_valid(pdata->gpio_pendown)) { err = devm_gpio_request_one(&spi->dev, pdata->gpio_pendown,
@@ -1016,10 +1016,10 @@ static int ads7846_setup_pendown(struct spi_device *spi, return err; } - ts->gpio_pendown = pdata->gpio_pendown; + ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown); if (pdata->gpio_pendown_debounce) - gpiod_set_debounce(gpio_to_desc(pdata->gpio_pendown), + gpiod_set_debounce(ts->gpio_pendown, pdata->gpio_pendown_debounce); } else { dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n");
@@ -1128,7 +1128,7 @@ static const struct of_device_id ads7846_dt_ids[] = { }; MODULE_DEVICE_TABLE(of, ads7846_dt_ids); -static const struct ads7846_platform_data *ads7846_probe_dt(struct device *dev) +static const struct ads7846_platform_data *ads7846_probe_dt(struct
ads7846 *ts, struct device *dev)
{
struct ads7846_platform_data *pdata;
struct device_node *node = dev->of_node;@@ -1193,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);
+ ts->gpio_pendown = gpiod_get(dev, "pendown-gpio", GPIOD_IN);
return pdata;
}@@ -1267,7 +1267,7 @@ static int ads7846_probe(struct spi_device *spi) pdata = dev_get_platdata(dev); if (!pdata) { - pdata = ads7846_probe_dt(dev); + pdata = ads7846_probe_dt(ts, dev); if (IS_ERR(pdata)) return PTR_ERR(pdata); }
while leaving the platform side untouched, but I think Linus' plan was to
remove the gpio settings from all platform data and instead use the gpio
lookup in the board files.
Arnd
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel