2013-04-23 09:06:06

by Artem Bityutskiy

[permalink] [raw]
Subject: eGalax touchscreen regression

Hi Dmitry,

in the below commit

commit ae495e844a77344fdaedbb2ad97d925d096e9f0d
Author: Hui Wang <[email protected]>
Date: Thu Oct 25 00:38:01 2012 -0700

Input: egalax_ts - get gpio from devicetree

The irq_to_gpio() is old, most platforms use GENERIC_GPIO framework
and don't support this API anymore.

The i.MX6q sabrelite platform equips an egalax touchscreen controller,
and this platform already transfered to GENERIC_GPIO framework, to
support this driver, we use a more generic way to get gpio.

Add a return value checking for waking up the controller in the probe
function, this guarantee only a workable device can pass init.

[[email protected]: Make driver depend on CONFIG_OF as it is
now required.]

Acked-by Zhang Jiejing <[email protected]>
Reviewed-by: Shawn Guo <[email protected]>
Signed-off-by: Hui Wang <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>

the eGalax driver now requires OF. For us this is a regression because
we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740

--
Best Regards,
Artem Bityutskiy


2013-04-23 11:09:23

by Artem Bityutskiy

[permalink] [raw]
Subject: [PATCH] input: egalax_ts: remove bogus OF dependency

There are platforms using this driver which do not have OF. However, commit
ae495e844a77344fdaedbb2ad97d925d096e9f0d added a Kconfig dependency on OF and
broke OF-less setups.

Signed-off-by: Artem Bityutskiy <[email protected]>
Cc: [email protected] [v3.7+]
---
drivers/input/touchscreen/Kconfig | 2 +-
drivers/input/touchscreen/egalax_ts.c | 2 ++
2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 515cfe7..d53b0d6 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -227,7 +227,7 @@ config TOUCHSCREEN_EETI

config TOUCHSCREEN_EGALAX
tristate "EETI eGalax multi-touch panel support"
- depends on I2C && OF
+ depends on I2C
help
Say Y here to enable support for I2C connected EETI
eGalax multi-touch panels.
diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
index 17c9097..b984cfc 100644
--- a/drivers/input/touchscreen/egalax_ts.c
+++ b/drivers/input/touchscreen/egalax_ts.c
@@ -287,10 +287,12 @@ static int egalax_ts_resume(struct device *dev)

static SIMPLE_DEV_PM_OPS(egalax_ts_pm_ops, egalax_ts_suspend, egalax_ts_resume);

+#ifdef CONFIG_OF
static struct of_device_id egalax_ts_dt_ids[] = {
{ .compatible = "eeti,egalax_ts" },
{ /* sentinel */ }
};
+#endif

static struct i2c_driver egalax_ts_driver = {
.driver = {
--
1.7.7

2013-04-23 15:38:01

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH] input: egalax_ts: remove bogus OF dependency

On Tue, Apr 23, 2013 at 02:09:17PM +0300, Artem Bityutskiy wrote:
> There are platforms using this driver which do not have OF. However, commit
> ae495e844a77344fdaedbb2ad97d925d096e9f0d added a Kconfig dependency on OF and
> broke OF-less setups.
>
> Signed-off-by: Artem Bityutskiy <[email protected]>
> Cc: [email protected] [v3.7+]
> ---
> drivers/input/touchscreen/Kconfig | 2 +-
> drivers/input/touchscreen/egalax_ts.c | 2 ++
> 2 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 515cfe7..d53b0d6 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -227,7 +227,7 @@ config TOUCHSCREEN_EETI
>
> config TOUCHSCREEN_EGALAX
> tristate "EETI eGalax multi-touch panel support"
> - depends on I2C && OF
> + depends on I2C

No, because egalax_wake_up_device() needs OF data and with !CONFIG_OF
you'll end up using stubs, the wakeup will fail and the touchscreen will
fail probe.

It wasn't tested, was it?

Thanks.

> help
> Say Y here to enable support for I2C connected EETI
> eGalax multi-touch panels.
> diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
> index 17c9097..b984cfc 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -287,10 +287,12 @@ static int egalax_ts_resume(struct device *dev)
>
> static SIMPLE_DEV_PM_OPS(egalax_ts_pm_ops, egalax_ts_suspend, egalax_ts_resume);
>
> +#ifdef CONFIG_OF
> static struct of_device_id egalax_ts_dt_ids[] = {
> { .compatible = "eeti,egalax_ts" },
> { /* sentinel */ }
> };
> +#endif
>
> static struct i2c_driver egalax_ts_driver = {
> .driver = {
> --
> 1.7.7
>

--
Dmitry

2013-04-23 15:40:14

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

Hi Artem,

On Tue, Apr 23, 2013 at 12:08:08PM +0300, Artem Bityutskiy wrote:
> Hi Dmitry,
>
> in the below commit
>
> commit ae495e844a77344fdaedbb2ad97d925d096e9f0d
> Author: Hui Wang <[email protected]>
> Date: Thu Oct 25 00:38:01 2012 -0700
>
> Input: egalax_ts - get gpio from devicetree
>
> The irq_to_gpio() is old, most platforms use GENERIC_GPIO framework
> and don't support this API anymore.
>
> The i.MX6q sabrelite platform equips an egalax touchscreen controller,
> and this platform already transfered to GENERIC_GPIO framework, to
> support this driver, we use a more generic way to get gpio.
>
> Add a return value checking for waking up the controller in the probe
> function, this guarantee only a workable device can pass init.
>
> [[email protected]: Make driver depend on CONFIG_OF as it is
> now required.]
>
> Acked-by Zhang Jiejing <[email protected]>
> Reviewed-by: Shawn Guo <[email protected]>
> Signed-off-by: Hui Wang <[email protected]>
> Signed-off-by: Dmitry Torokhov <[email protected]>
>
> the eGalax driver now requires OF. For us this is a regression because
> we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740

I see. In this case we need to come up with a platform data to pass
wakeup gpio in case platform does not support OF. irq_to_gpio() is not
supported on many platforms, causing compilation errors.

Is the platform that you are trying to use the touchscreen upstream?

Thanks.

--
Dmitry

2013-04-24 05:56:32

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: [PATCH] input: egalax_ts: remove bogus OF dependency

On Tue, 2013-04-23 at 08:37 -0700, Dmitry Torokhov wrote:
> On Tue, Apr 23, 2013 at 02:09:17PM +0300, Artem Bityutskiy wrote:
> > There are platforms using this driver which do not have OF. However, commit
> > ae495e844a77344fdaedbb2ad97d925d096e9f0d added a Kconfig dependency on OF and
> > broke OF-less setups.
> >
> > Signed-off-by: Artem Bityutskiy <[email protected]>
> > Cc: [email protected] [v3.7+]
> > ---
> > drivers/input/touchscreen/Kconfig | 2 +-
> > drivers/input/touchscreen/egalax_ts.c | 2 ++
> > 2 files changed, 3 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> > index 515cfe7..d53b0d6 100644
> > --- a/drivers/input/touchscreen/Kconfig
> > +++ b/drivers/input/touchscreen/Kconfig
> > @@ -227,7 +227,7 @@ config TOUCHSCREEN_EETI
> >
> > config TOUCHSCREEN_EGALAX
> > tristate "EETI eGalax multi-touch panel support"
> > - depends on I2C && OF
> > + depends on I2C
>
> No, because egalax_wake_up_device() needs OF data and with !CONFIG_OF
> you'll end up using stubs, the wakeup will fail and the touchscreen will
> fail probe.
>
> It wasn't tested, was it?

Not tested. But Ulf (the reporter of the issue) confirms it does not
solve the issue.

--
Best Regards,
Artem Bityutskiy

2013-04-24 06:04:45

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > the eGalax driver now requires OF. For us this is a regression because
> > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
>
> I see. In this case we need to come up with a platform data to pass
> wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> supported on many platforms, causing compilation errors.
>
> Is the platform that you are trying to use the touchscreen upstream?

Unfortunately I do not really know about it, but once I learn, I'll come
back. Thanks!

--
Best Regards,
Artem Bityutskiy

2013-04-24 09:01:11

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > the eGalax driver now requires OF. For us this is a regression because
> > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
>
> I see. In this case we need to come up with a platform data to pass
> wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> supported on many platforms, causing compilation errors.
>
> Is the platform that you are trying to use the touchscreen upstream?

OK, so this is about just a monitor with a touchscreen (Giantec high-res
capacitive 11.6” LCD monitor). The touchscreen is connected to a
commodity hardware via USB.

So yes, the platform _is_ upstream :-) Namely, we use just a SandyBridge
PC.

--
Best Regards,
Artem Bityutskiy

2013-04-26 14:59:12

by Arnd Bergmann

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Wednesday 24 April 2013 12:03:13 Artem Bityutskiy wrote:
> On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > > the eGalax driver now requires OF. For us this is a regression because
> > > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
> >
> > I see. In this case we need to come up with a platform data to pass
> > wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> > supported on many platforms, causing compilation errors.
> >
> > Is the platform that you are trying to use the touchscreen upstream?
>
> OK, so this is about just a monitor with a touchscreen (Giantec high-res
> capacitive 11.6” LCD monitor). The touchscreen is connected to a
> commodity hardware via USB.
>
> So yes, the platform _is_ upstream Namely, we use just a SandyBridge
> PC.

So you use a usb-to-i2c interface?

You have to figure out how the wakeup line is connected to the eGalax
device and provide an alternative implementation for egalax_wake_up_device
that can be used in your case then.

Arnd

2013-04-26 15:06:52

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Fri, 2013-04-26 at 16:59 +0200, Arnd Bergmann wrote:
> On Wednesday 24 April 2013 12:03:13 Artem Bityutskiy wrote:
> > On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > > > the eGalax driver now requires OF. For us this is a regression because
> > > > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
> > >
> > > I see. In this case we need to come up with a platform data to pass
> > > wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> > > supported on many platforms, causing compilation errors.
> > >
> > > Is the platform that you are trying to use the touchscreen upstream?
> >
> > OK, so this is about just a monitor with a touchscreen (Giantec high-res
> > capacitive 11.6” LCD monitor). The touchscreen is connected to a
> > commodity hardware via USB.
> >
> > So yes, the platform _is_ upstream Namely, we use just a SandyBridge
> > PC.
>
> So you use a usb-to-i2c interface?

I do not think so. The problem is that I do not have the HW, so I was
making guesses, and it looks like this driver has nothing to do with the
regression we've got. Sorry for the noise. We'll investigate this more
carefully internally.

--
Best Regards,
Artem Bityutskiy

2013-04-26 15:39:17

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Fri, Apr 26, 2013 at 06:09:05PM +0300, Artem Bityutskiy wrote:
> On Fri, 2013-04-26 at 16:59 +0200, Arnd Bergmann wrote:
> > On Wednesday 24 April 2013 12:03:13 Artem Bityutskiy wrote:
> > > On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > > > > the eGalax driver now requires OF. For us this is a regression because
> > > > > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
> > > >
> > > > I see. In this case we need to come up with a platform data to pass
> > > > wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> > > > supported on many platforms, causing compilation errors.
> > > >
> > > > Is the platform that you are trying to use the touchscreen upstream?
> > >
> > > OK, so this is about just a monitor with a touchscreen (Giantec high-res
> > > capacitive 11.6” LCD monitor). The touchscreen is connected to a
> > > commodity hardware via USB.
> > >
> > > So yes, the platform _is_ upstream Namely, we use just a SandyBridge
> > > PC.
> >
> > So you use a usb-to-i2c interface?
>
> I do not think so. The problem is that I do not have the HW, so I was
> making guesses, and it looks like this driver has nothing to do with the
> regression we've got. Sorry for the noise. We'll investigate this more
> carefully internally.

OK, so assuming you simply using USB version of eGalax device the only
recent change (3.6-rc7) was:

commit 037a833ed05a86d01ea27a2c32043b86c549be1b
Author: Forest Bond <[email protected]>
Date: Tue Sep 4 20:27:37 2012 -0700

Input: usbtouchscreen - initialize eGalax devices

Certain eGalax devices expose an interface with class HID and protocol
None. Some work with usbhid and some work with usbtouchscreen, but
there is no easy way to differentiate. Sending an eGalax diagnostic
packet seems to kick them all into using the right protocol for
usbtouchscreen, so we can continue to bind them all there (as opposed to
handing some off to usbhid).

This fixes a regression for devices that were claimed by (and worked
with) usbhid prior to commit 139ebe8dc80dd74cb2ac9f5603d18fbf5cff049f
("Input: usbtouchscreen - fix eGalax HID ignoring"), which made
usbtouchscreen claim them instead. With this patch they will still be
claimed by usbtouchscreen, but they will actually report events
usbtouchscreen can understand. Note that these devices will be limited
to the usbtouchscreen feature set so e.g. dual touch features are not
supported.

I have the distinct pleasure of needing to support devices of both types
and have tested accordingly.

Signed-off-by: Forest Bond <[email protected]>
Signed-off-by: Dmitry Torokhov <[email protected]>

Thanks.

--
Dmitry

2013-04-26 16:01:17

by Artem Bityutskiy

[permalink] [raw]
Subject: Re: eGalax touchscreen regression

On Fri, 2013-04-26 at 08:39 -0700, Dmitry Torokhov wrote:
> On Fri, Apr 26, 2013 at 06:09:05PM +0300, Artem Bityutskiy wrote:
> > On Fri, 2013-04-26 at 16:59 +0200, Arnd Bergmann wrote:
> > > On Wednesday 24 April 2013 12:03:13 Artem Bityutskiy wrote:
> > > > On Tue, 2013-04-23 at 08:40 -0700, Dmitry Torokhov wrote:
> > > > > > the eGalax driver now requires OF. For us this is a regression because
> > > > > > we do not have OF: https://bugs.tizen.org/jira/browse/TIVI-740
> > > > >
> > > > > I see. In this case we need to come up with a platform data to pass
> > > > > wakeup gpio in case platform does not support OF. irq_to_gpio() is not
> > > > > supported on many platforms, causing compilation errors.
> > > > >
> > > > > Is the platform that you are trying to use the touchscreen upstream?
> > > >
> > > > OK, so this is about just a monitor with a touchscreen (Giantec high-res
> > > > capacitive 11.6” LCD monitor). The touchscreen is connected to a
> > > > commodity hardware via USB.
> > > >
> > > > So yes, the platform _is_ upstream Namely, we use just a SandyBridge
> > > > PC.
> > >
> > > So you use a usb-to-i2c interface?
> >
> > I do not think so. The problem is that I do not have the HW, so I was
> > making guesses, and it looks like this driver has nothing to do with the
> > regression we've got. Sorry for the noise. We'll investigate this more
> > carefully internally.
>
> OK, so assuming you simply using USB version of eGalax device the only
> recent change (3.6-rc7) was:
>
> commit 037a833ed05a86d01ea27a2c32043b86c549be1b
> Author: Forest Bond <[email protected]>
> Date: Tue Sep 4 20:27:37 2012 -0700

Thanks for the input Dmitry, appreciated. We'll be looking more
carefully at this next week.

--
Best Regards,
Artem Bityutskiy