| Message ID | 20200504173745.1246505-3-daniel@zonque.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/3] input: touch: ads7846: add short-hand for spi->dev in probe() function | expand |
Hi Daniel, thanks for picking my suggestion and converting the driver :) Unfortunately it is not that easy since we need to care about the existing users. You will get the legacy users if you grep for ads7846_platform_data. Those platforms needs to converted to. Again, pls align the commit subject prefix with the existing patches. On 20-05-04 19:37, Daniel Mack wrote: > Use gpiod_* function to access the pendown GPIO line. Maybe this is better: Input: ads7846 - Convert to GPIO descriptors This converts the ads7846 touchscreen driver to use GPIO descriptors and switches all platform data boards over to passing descriptors instead of global GPIO numbers. We use the "pendown-gpio" name as found in the device tree bindings for this touchscreen driver. > > Signed-off-by: Daniel Mack <daniel@zonque.org> > --- > drivers/input/touchscreen/ads7846.c | 53 ++++++++++++++++------------- > 1 file changed, 30 insertions(+), 23 deletions(-) > > diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c > index 7f4ead542a73..b3e17ee4e499 100644 > --- a/drivers/input/touchscreen/ads7846.c > +++ b/drivers/input/touchscreen/ads7846.c > @@ -27,7 +27,7 @@ > #include <linux/of.h> > #include <linux/of_gpio.h> > #include <linux/of_device.h> > -#include <linux/gpio.h> > +#include <linux/gpio/consumer.h> > #include <linux/spi/spi.h> > #include <linux/spi/ads7846.h> > #include <linux/regulator/consumer.h> > @@ -137,7 +137,7 @@ struct ads7846 { > void *filter_data; > void (*filter_cleanup)(void *data); > int (*get_pendown_state)(void); > - int gpio_pendown; > + struct gpio_desc *gpio_pendown; > > void (*wait_for_sync)(void); > }; > @@ -598,7 +598,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); The gpio_get_value() function return the raw value and the ! before inverts the logic. Thanks to gpiod we don't need to care about this inverting logic anymore. > } > > static void null_wait_for_sync(void) > @@ -919,6 +919,7 @@ static int ads7846_setup_pendown(struct spi_device *spi, > struct ads7846 *ts, > const struct ads7846_platform_data *pdata) > { > + struct device *dev = &spi->dev; > int err; > > /* > @@ -929,27 +930,33 @@ 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) { > - dev_err(&spi->dev, > - "failed to request/setup pendown GPIO%d: %d\n", > - pdata->gpio_pendown, err); > - return err; > - } > + return 0; > + } > > - ts->gpio_pendown = pdata->gpio_pendown; > + ts->gpio_pendown = devm_gpiod_get(dev, "pendown", GPIOD_IN); > + if (IS_ERR(ts->gpio_pendown)) { > + err = PTR_ERR(ts->gpio_pendown); > > - if (pdata->gpio_pendown_debounce) > - gpio_set_debounce(pdata->gpio_pendown, > - pdata->gpio_pendown_debounce); > - } else { > - dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); > - return -EINVAL; > + if (gpio_is_valid(pdata->gpio_pendown)) { > + err = devm_gpio_request_one(dev, pdata->gpio_pendown, > + GPIOF_IN, > + "ads7846_pendown"); > + if (err < 0) > + return err; > + > + ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown); > + if (!ts->gpio_pendown) > + return -EINVAL; > + } > + > + if (err < 0) > + return err; > } > > + if (pdata->gpio_pendown_debounce) > + gpiod_set_debounce(ts->gpio_pendown, > + pdata->gpio_pendown_debounce); > + > return 0; > } > > @@ -1236,8 +1243,6 @@ 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); > - > return pdata; > } It's a bit hard to follow the changes within ads7846_setup_pendown(). To make it easier and IMHO cleaner to everyone we should convert the ads7846_platform_data to gpio_desc too and request the gpio within the ads7846_probe_dt() function. Pls take a look on this patchset [1] from Linus. There you will get all informations who platform data based machines can be changed usign gpiod_lookup_table's. [1] https://patches.linaro.org/patch/185481/ Thanks for working on this =) Regards, Marco > #else > @@ -1340,8 +1345,10 @@ static int ads7846_probe(struct spi_device *spi) > } > > err = ads7846_setup_pendown(spi, ts, pdata); > - if (err) > + if (err) { > + dev_err(dev, "Unable to request pendown GPIO: %d", err); > goto err_cleanup_filter; > + } > > if (pdata->penirq_recheck_delay_usecs) > ts->penirq_recheck_delay_usecs = > -- > 2.26.2
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index 7f4ead542a73..b3e17ee4e499 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -27,7 +27,7 @@ #include <linux/of.h> #include <linux/of_gpio.h> #include <linux/of_device.h> -#include <linux/gpio.h> +#include <linux/gpio/consumer.h> #include <linux/spi/spi.h> #include <linux/spi/ads7846.h> #include <linux/regulator/consumer.h> @@ -137,7 +137,7 @@ struct ads7846 { void *filter_data; void (*filter_cleanup)(void *data); int (*get_pendown_state)(void); - int gpio_pendown; + struct gpio_desc *gpio_pendown; void (*wait_for_sync)(void); }; @@ -598,7 +598,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 null_wait_for_sync(void) @@ -919,6 +919,7 @@ static int ads7846_setup_pendown(struct spi_device *spi, struct ads7846 *ts, const struct ads7846_platform_data *pdata) { + struct device *dev = &spi->dev; int err; /* @@ -929,27 +930,33 @@ 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) { - dev_err(&spi->dev, - "failed to request/setup pendown GPIO%d: %d\n", - pdata->gpio_pendown, err); - return err; - } + return 0; + } - ts->gpio_pendown = pdata->gpio_pendown; + ts->gpio_pendown = devm_gpiod_get(dev, "pendown", GPIOD_IN); + if (IS_ERR(ts->gpio_pendown)) { + err = PTR_ERR(ts->gpio_pendown); - if (pdata->gpio_pendown_debounce) - gpio_set_debounce(pdata->gpio_pendown, - pdata->gpio_pendown_debounce); - } else { - dev_err(&spi->dev, "no get_pendown_state nor gpio_pendown?\n"); - return -EINVAL; + if (gpio_is_valid(pdata->gpio_pendown)) { + err = devm_gpio_request_one(dev, pdata->gpio_pendown, + GPIOF_IN, + "ads7846_pendown"); + if (err < 0) + return err; + + ts->gpio_pendown = gpio_to_desc(pdata->gpio_pendown); + if (!ts->gpio_pendown) + return -EINVAL; + } + + if (err < 0) + return err; } + if (pdata->gpio_pendown_debounce) + gpiod_set_debounce(ts->gpio_pendown, + pdata->gpio_pendown_debounce); + return 0; } @@ -1236,8 +1243,6 @@ 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); - return pdata; } #else @@ -1340,8 +1345,10 @@ static int ads7846_probe(struct spi_device *spi) } err = ads7846_setup_pendown(spi, ts, pdata); - if (err) + if (err) { + dev_err(dev, "Unable to request pendown GPIO: %d", err); goto err_cleanup_filter; + } if (pdata->penirq_recheck_delay_usecs) ts->penirq_recheck_delay_usecs =
Use gpiod_* function to access the pendown GPIO line. Signed-off-by: Daniel Mack <daniel@zonque.org> --- drivers/input/touchscreen/ads7846.c | 53 ++++++++++++++++------------- 1 file changed, 30 insertions(+), 23 deletions(-)