lkml.org 
[lkml]   [2021]   [Nov]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: [PATCH v2 5/8] gpiolib: shrink further
On Tue, Nov 9, 2021 at 11:24 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:

> > @@ -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?

> > 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)

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.c
b/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

\
 
 \ /
  Last update: 2021-11-09 12:18    [W:0.060 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site