| Message ID | 20250102181953.1020878-3-aaro.koskinen@iki.fi (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Input/omap1: fix touchscreen functionality on Nokia 770 | expand |
On Thu, Jan 2, 2025 at 7:20 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > The GPIO is set up as an IRQ, so request it as non-exclusive. Otherwise the > probe fails on Nokia 770 with: > > ads7846 spi2.0: failed to request pendown GPIO > ads7846: probe of spi2.0 failed with error -16 > > Also the polarity is wrong. Fix it. > > Fixes: 767d83361aaa ("Input: ads7846 - Convert to use software nodes") > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Yours, Linus Walleij
On Thu, Jan 02, 2025 at 10:32:00PM +0100, Linus Walleij wrote: > On Thu, Jan 2, 2025 at 7:20 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > > > The GPIO is set up as an IRQ, so request it as non-exclusive. Otherwise the > > probe fails on Nokia 770 with: > > > > ads7846 spi2.0: failed to request pendown GPIO > > ads7846: probe of spi2.0 failed with error -16 > > > > Also the polarity is wrong. Fix it. > > > > Fixes: 767d83361aaa ("Input: ads7846 - Convert to use software nodes") > > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Do we need to have this knowledge in the driver or can it be handled in GPIO subsystem or affected board support? Requesting a GPIO with "in" direction when it is also an interrupt source should be pretty common. Thanks.
On Mon, Jan 6, 2025 at 8:02 AM Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > On Thu, Jan 02, 2025 at 10:32:00PM +0100, Linus Walleij wrote: > > On Thu, Jan 2, 2025 at 7:20 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > > > > > The GPIO is set up as an IRQ, so request it as non-exclusive. Otherwise the > > > probe fails on Nokia 770 with: > > > > > > ads7846 spi2.0: failed to request pendown GPIO > > > ads7846: probe of spi2.0 failed with error -16 > > > > > > Also the polarity is wrong. Fix it. > > > > > > Fixes: 767d83361aaa ("Input: ads7846 - Convert to use software nodes") > > > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > Do we need to have this knowledge in the driver or can it be handled > in GPIO subsystem or affected board support? Requesting a GPIO with "in" > direction when it is also an interrupt source should be pretty common. Hm I don't know exactly the question here but I try to answer anyway :) The patch makes the boardfile describe the polarity but the boardfile (or device tree) cannot define directions, consumers must specify this. The main reason is that actual users exist that switch the direction of GPIOs at runtime so this has been designed as a (runtime) consumer duty. As for GPIOD_FLAGS_BIT_NONEXCLUSIVE, this enables the GPIO subsystem to read the GPIO while the irqchip subsystem can also handle the same GPIO line as an interrupt source, so it's not exclusive to either subsystem. Yours, Linus Walleij
On Tue, Jan 14, 2025 at 01:28:32PM +0100, Linus Walleij wrote: > On Mon, Jan 6, 2025 at 8:02 AM Dmitry Torokhov > <dmitry.torokhov@gmail.com> wrote: > > On Thu, Jan 02, 2025 at 10:32:00PM +0100, Linus Walleij wrote: > > > On Thu, Jan 2, 2025 at 7:20 PM Aaro Koskinen <aaro.koskinen@iki.fi> wrote: > > > > > > > The GPIO is set up as an IRQ, so request it as non-exclusive. Otherwise the > > > > probe fails on Nokia 770 with: > > > > > > > > ads7846 spi2.0: failed to request pendown GPIO > > > > ads7846: probe of spi2.0 failed with error -16 > > > > > > > > Also the polarity is wrong. Fix it. > > > > > > > > Fixes: 767d83361aaa ("Input: ads7846 - Convert to use software nodes") > > > > Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> > > > > > > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > > > > Do we need to have this knowledge in the driver or can it be handled > > in GPIO subsystem or affected board support? Requesting a GPIO with "in" > > direction when it is also an interrupt source should be pretty common. > > Hm I don't know exactly the question here but I try to answer > anyway :) > > The patch makes the boardfile describe the polarity but the > boardfile (or device tree) cannot define directions, consumers > must specify this. The main reason is that actual users exist that > switch the direction of GPIOs at runtime so this has been designed > as a (runtime) consumer duty. > > As for GPIOD_FLAGS_BIT_NONEXCLUSIVE, this enables the > GPIO subsystem to read the GPIO while the irqchip subsystem > can also handle the same GPIO line as an interrupt source, so > it's not exclusive to either subsystem. But isn't this something that should work by default, without specifying any additional flags? I understand that using GPIO as an interrupt source and at the same time as an output line is not possible (without reconfiguration "on the fly"), but reading state if an input GPIO line that is also an interrupt should be OK? I am pretty sure there are systems/boards/arches that allow this. This is my objection - we have to add a flag to a driver that is used on multiple systems to tweak behavior of one particular board. Thanks.
diff --git a/arch/arm/mach-omap1/board-nokia770.c b/arch/arm/mach-omap1/board-nokia770.c index 3312ef93355d..9153b1a81577 100644 --- a/arch/arm/mach-omap1/board-nokia770.c +++ b/arch/arm/mach-omap1/board-nokia770.c @@ -287,7 +287,7 @@ static struct gpiod_lookup_table nokia770_irq_gpio_table = { .table = { /* GPIO used by SPI device 1 */ GPIO_LOOKUP("gpio-0-15", 15, "ads7846_irq", - GPIO_ACTIVE_HIGH), + GPIO_ACTIVE_LOW), /* GPIO used for retu IRQ */ GPIO_LOOKUP("gpio-48-63", 15, "retu_irq", GPIO_ACTIVE_HIGH), diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c index 066dc04003fa..54280ecca0a7 100644 --- a/drivers/input/touchscreen/ads7846.c +++ b/drivers/input/touchscreen/ads7846.c @@ -1021,7 +1021,8 @@ static int ads7846_setup_pendown(struct spi_device *spi, if (pdata->get_pendown_state) { ts->get_pendown_state = pdata->get_pendown_state; } else { - ts->gpio_pendown = gpiod_get(&spi->dev, "pendown", GPIOD_IN); + ts->gpio_pendown = gpiod_get(&spi->dev, "pendown", GPIOD_IN | + GPIOD_FLAGS_BIT_NONEXCLUSIVE); if (IS_ERR(ts->gpio_pendown)) { dev_err(&spi->dev, "failed to request pendown GPIO\n"); return PTR_ERR(ts->gpio_pendown);
The GPIO is set up as an IRQ, so request it as non-exclusive. Otherwise the probe fails on Nokia 770 with: ads7846 spi2.0: failed to request pendown GPIO ads7846: probe of spi2.0 failed with error -16 Also the polarity is wrong. Fix it. Fixes: 767d83361aaa ("Input: ads7846 - Convert to use software nodes") Signed-off-by: Aaro Koskinen <aaro.koskinen@iki.fi> --- arch/arm/mach-omap1/board-nokia770.c | 2 +- drivers/input/touchscreen/ads7846.c | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-)