* Re: [PATCH v2] Input: tsc2007 - use GPIO descriptor
[not found] ` <20190902155144.GK187474@dtor-ws>
@ 2019-10-06 0:02 ` Shawn Guo
0 siblings, 0 replies; only message in thread
From: Shawn Guo @ 2019-10-06 0:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linus Walleij, Sascha Hauer, linux-input, Denis Carikli,
Pengutronix Kernel Team, NXP Linux Team, H . Nikolaus Schaller,
devicetree
On Mon, Sep 02, 2019 at 08:51:44AM -0700, Dmitry Torokhov wrote:
> On Fri, Aug 23, 2019 at 09:10:21AM +0200, Linus Walleij wrote:
> > This switches the TSC2007 to use a GPIO descriptor to read
> > the pendown GPIO line.
> >
> > As this will make the gpiolib start to respect polarity
> > inversion flags on the GPIO lines, drop the inversion when
> > reading the line with gpio_get_value(), fix two offenders
> > in the i.MX device trees, and also emphasize the importance
> > of marking the polarity right in the device tree bindings.
> >
> > Cc: Denis Carikli <denis@eukrea.com>
> > Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> > Cc: NXP Linux Team <linux-imx@nxp.com>
> > Cc: H. Nikolaus Schaller <hns@goldelico.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> > ChangeLog v1->v2:
> > - Drop inversion on the GPIO descriptor value, rely on the
> > gpiolib to handle polarity inversion.
> > - Comb through device trees, identify two offenders, fix
> > them as part of the patch for a clean cut.
> > - Also fix the device tree bindings.
> > ---
> > .../bindings/input/touchscreen/tsc2007.txt | 5 +++--
> > arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi | 3 ++-
> > arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi | 3 ++-
>
> Shawn, Sascha, any objections to these DTS changes?
No objections from me. But in generally, we prefer to have DTS changes
in separate patch going through arm-soc tree.
Shawn
>
> > drivers/input/touchscreen/tsc2007.h | 4 +++-
> > drivers/input/touchscreen/tsc2007_core.c | 14 +++++++-------
> > 5 files changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > index ed00f61b8c08..b08b54d49699 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > @@ -7,7 +7,8 @@ Required properties:
> >
> > Optional properties:
> > - gpios: the interrupt gpio the chip is connected to (trough the penirq pin).
> > - The penirq pin goes to low when the panel is touched.
> > + The penirq pin goes to low when the panel is touched, so make sure to tag
> > + the GPIO line with GPIO_ACTIVE_LOW.
> > (see GPIO binding[1] for more details).
> > - interrupts: (gpio) interrupt to which the chip is connected
> > (see interrupt binding[0]).
> > @@ -31,7 +32,7 @@ Example:
> > reg = <0x49>;
> > interrupt-parent = <&gpio4>;
> > interrupts = <0x0 0x8>;
> > - gpios = <&gpio4 0 0>;
> > + gpios = <&gpio4 0 GPIO_ACTIVE_LOW>;
> > ti,x-plate-ohms = <180>;
> > };
> >
> > diff --git a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> > index 17bd2a97609a..b8ac60622936 100644
> > --- a/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> > +++ b/arch/arm/boot/dts/imx35-eukrea-cpuimx35.dtsi
> > @@ -3,6 +3,7 @@
> > * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
> > */
> >
> > +#include <dt-bindings/gpio/gpio.h>
> > #include "imx35.dtsi"
> >
> > / {
> > @@ -33,7 +34,7 @@
> >
> > tsc2007: tsc2007@48 {
> > compatible = "ti,tsc2007";
> > - gpios = <&gpio3 2 0>;
> > + gpios = <&gpio3 2 GPIO_ACTIVE_LOW>;
> > interrupt-parent = <&gpio3>;
> > interrupts = <0x2 0x8>;
> > pinctrl-names = "default";
> > diff --git a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
> > index c2a929ba8ceb..016d0bc64bdb 100644
> > --- a/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
> > +++ b/arch/arm/boot/dts/imx51-eukrea-cpuimx51.dtsi
> > @@ -3,6 +3,7 @@
> > * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
> > */
> >
> > +#include <dt-bindings/gpio/gpio.h>
> > #include "imx51.dtsi"
> >
> > / {
> > @@ -33,7 +34,7 @@
> >
> > tsc2007: tsc2007@49 {
> > compatible = "ti,tsc2007";
> > - gpios = <&gpio4 0 1>;
> > + gpios = <&gpio4 0 GPIO_ACTIVE_LOW>;
> > interrupt-parent = <&gpio4>;
> > interrupts = <0x0 0x8>;
> > pinctrl-names = "default";
> > diff --git a/drivers/input/touchscreen/tsc2007.h b/drivers/input/touchscreen/tsc2007.h
> > index 91c60bf6dcaf..0306c38b56c7 100644
> > --- a/drivers/input/touchscreen/tsc2007.h
> > +++ b/drivers/input/touchscreen/tsc2007.h
> > @@ -49,6 +49,8 @@
> > #define READ_X (ADC_ON_12BIT | TSC2007_MEASURE_X)
> > #define PWRDOWN (TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
> >
> > +struct gpio_desc;
> > +
> > struct ts_event {
> > u16 x;
> > u16 y;
> > @@ -69,7 +71,7 @@ struct tsc2007 {
> > int fuzzy;
> > int fuzzz;
> >
> > - unsigned int gpio;
> > + struct gpio_desc *gpiod;
> > int irq;
> >
> > wait_queue_head_t wait;
> > diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
> > index 3b80abfc1eca..0eadd6d86fa0 100644
> > --- a/drivers/input/touchscreen/tsc2007_core.c
> > +++ b/drivers/input/touchscreen/tsc2007_core.c
> > @@ -23,7 +23,7 @@
> > #include <linux/interrupt.h>
> > #include <linux/i2c.h>
> > #include <linux/of_device.h>
> > -#include <linux/of_gpio.h>
> > +#include <linux/gpio/consumer.h>
> > #include <linux/platform_data/tsc2007.h>
> > #include "tsc2007.h"
> >
> > @@ -226,7 +226,7 @@ static int tsc2007_get_pendown_state_gpio(struct device *dev)
> > struct i2c_client *client = to_i2c_client(dev);
> > struct tsc2007 *ts = i2c_get_clientdata(client);
> >
> > - return !gpio_get_value(ts->gpio);
> > + return gpiod_get_value(ts->gpiod);
> > }
> >
> > static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
> > @@ -266,13 +266,13 @@ static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
> > return -EINVAL;
> > }
> >
> > - ts->gpio = of_get_gpio(np, 0);
> > - if (gpio_is_valid(ts->gpio))
> > + ts->gpiod = devm_gpiod_get_optional(&client->dev, NULL, GPIOD_IN);
> > + if (IS_ERR(ts->gpiod))
> > + return PTR_ERR(ts->gpiod);
> > + if (ts->gpiod)
> > ts->get_pendown_state = tsc2007_get_pendown_state_gpio;
> > else
> > - dev_warn(&client->dev,
> > - "GPIO not specified in DT (of_get_gpio returned %d)\n",
> > - ts->gpio);
> > + dev_warn(&client->dev, "GPIO not specified in DT\n");
> >
> > return 0;
> > }
> > --
> > 2.21.0
> >
>
> --
> Dmitry
^ permalink raw reply [flat|nested] only message in thread