[PATCH] udev: generalize the check in touchscreen rule

[PATCH] udev: generalize the check in touchscreen rule

[Andrea Adami]

* The actual rule only works for devices reporting ts pressure
* and ignores the other touchscreens (happens e.g. on ipaq h1940).
* a0,1,*18   a = absolute axes (ABS_X 0x00 ABS_Y 0x01 ABS_PRESSURE 0x18)
* (see include/linux/input.h)
*
* Guidelines for touchscreen seem suggesting only ABS_X and ABS_Y are mandatory
* (http://kernel.org/doc/Documentation/input/event-codes.txt)
*
* Bump PR

Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
---
 meta/recipes-core/udev/udev/local.rules |    3 +--
 meta/recipes-core/udev/udev_164.bb      |    2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/meta/recipes-core/udev/udev/local.rules b/meta/recipes-core/udev/udev/local.rules
index 625e49a..49e8d28 100644
--- a/meta/recipes-core/udev/udev/local.rules
+++ b/meta/recipes-core/udev/udev/local.rules
@@ -31,5 +31,4 @@ KERNEL=="rtc0", SYMLINK+="rtc"
 ACTION=="add", DEVPATH=="/devices/*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe $env{MODALIAS}"
 
 # Create a symlink to any touchscreen input device
-SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*18,*", SYMLINK+="input/touchscreen0"
-
+SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*", SYMLINK+="input/touchscreen0"
diff --git a/meta/recipes-core/udev/udev_164.bb b/meta/recipes-core/udev/udev_164.bb
index c5813ec..0462ff2 100644
--- a/meta/recipes-core/udev/udev_164.bb
+++ b/meta/recipes-core/udev/udev_164.bb
@@ -1,6 +1,6 @@
 include udev.inc
 
-PR = "r13"
+PR = "r14"
 
 SRC_URI += "file://udev-166-v4l1-1.patch"
 
-- 
1.7.3.4

Re: [PATCH] udev: generalize the check in touchscreen rule

[Andrea Adami]

On Mon, Apr 2, 2012 at 11:35 PM, Andrea Adami <andrea.adami@gmail.com> wrote:
> * The actual rule only works for devices reporting ts pressure
> * and ignores the other touchscreens (happens e.g. on ipaq h1940).
> * a0,1,*18   a = absolute axes (ABS_X 0x00 ABS_Y 0x01 ABS_PRESSURE 0x18)
> * (see include/linux/input.h)
> *
> * Guidelines for touchscreen seem suggesting only ABS_X and ABS_Y are mandatory
> * (http://kernel.org/doc/Documentation/input/event-codes.txt)
> *
> * Bump PR
>
> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> ---
>  meta/recipes-core/udev/udev/local.rules |    3 +--
>  meta/recipes-core/udev/udev_164.bb      |    2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/udev/udev/local.rules b/meta/recipes-core/udev/udev/local.rules
> index 625e49a..49e8d28 100644
> --- a/meta/recipes-core/udev/udev/local.rules
> +++ b/meta/recipes-core/udev/udev/local.rules
> @@ -31,5 +31,4 @@ KERNEL=="rtc0", SYMLINK+="rtc"
>  ACTION=="add", DEVPATH=="/devices/*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe $env{MODALIAS}"
>
>  # Create a symlink to any touchscreen input device
> -SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*18,*", SYMLINK+="input/touchscreen0"
> -
> +SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*", SYMLINK+="input/touchscreen0"
> diff --git a/meta/recipes-core/udev/udev_164.bb b/meta/recipes-core/udev/udev_164.bb
> index c5813ec..0462ff2 100644
> --- a/meta/recipes-core/udev/udev_164.bb
> +++ b/meta/recipes-core/udev/udev_164.bb
> @@ -1,6 +1,6 @@
>  include udev.inc
>
> -PR = "r13"
> +PR = "r14"
>
>  SRC_URI += "file://udev-166-v4l1-1.patch"
>
> --
> 1.7.3.4
>

Bump

I'm not linux-input specialist but this fix looks rather harmless.

Regards

Andrea

Re: [PATCH] udev: generalize the check in touchscreen rule

[Richard Purdie]

On Wed, 2012-04-11 at 10:59 +0200, Andrea Adami wrote:
> On Mon, Apr 2, 2012 at 11:35 PM, Andrea Adami <andrea.adami@gmail.com> wrote:
> > * The actual rule only works for devices reporting ts pressure
> > * and ignores the other touchscreens (happens e.g. on ipaq h1940).
> > * a0,1,*18   a = absolute axes (ABS_X 0x00 ABS_Y 0x01 ABS_PRESSURE 0x18)
> > * (see include/linux/input.h)
> > *
> > * Guidelines for touchscreen seem suggesting only ABS_X and ABS_Y are mandatory
> > * (http://kernel.org/doc/Documentation/input/event-codes.txt)
> > *
> > * Bump PR
> >
> > Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> > ---
> >  meta/recipes-core/udev/udev/local.rules |    3 +--
> >  meta/recipes-core/udev/udev_164.bb      |    2 +-
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/meta/recipes-core/udev/udev/local.rules b/meta/recipes-core/udev/udev/local.rules
> > index 625e49a..49e8d28 100644
> > --- a/meta/recipes-core/udev/udev/local.rules
> > +++ b/meta/recipes-core/udev/udev/local.rules
> > @@ -31,5 +31,4 @@ KERNEL=="rtc0", SYMLINK+="rtc"
> >  ACTION=="add", DEVPATH=="/devices/*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe $env{MODALIAS}"
> >
> >  # Create a symlink to any touchscreen input device
> > -SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*18,*", SYMLINK+="input/touchscreen0"
> > -
> > +SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*", SYMLINK+="input/touchscreen0"
> > diff --git a/meta/recipes-core/udev/udev_164.bb b/meta/recipes-core/udev/udev_164.bb
> > index c5813ec..0462ff2 100644
> > --- a/meta/recipes-core/udev/udev_164.bb
> > +++ b/meta/recipes-core/udev/udev_164.bb
> > @@ -1,6 +1,6 @@
> >  include udev.inc
> >
> > -PR = "r13"
> > +PR = "r14"
> >
> >  SRC_URI += "file://udev-166-v4l1-1.patch"
> >
> > --
> > 1.7.3.4
> >
> 
> Bump
> 
> I'm not linux-input specialist but this fix looks rather harmless.

I'm not 100% convinced about that. There needs to be some mechanism to
signal to the system when there is pressure applied to the screen and
when pressure was released. I think there are two ways you can do this,
BTN_TOUCH and ABS_PRESSURE. Regardless of what the input events codes
say, I'm really more interested in what tslib accepts.

This code has been like this for a long time and I'm surprised that if
there were issues it wasn't patched long before now. I'm therefore left
wondering if the touchscreen driver you're using is doing the right
things.

Cheers,

Richard

Re: [PATCH] udev: generalize the check in touchscreen rule

[Koen Kooi]

Op 11 apr. 2012, om 12:16 heeft Richard Purdie het volgende geschreven:

> On Wed, 2012-04-11 at 10:59 +0200, Andrea Adami wrote:
>> On Mon, Apr 2, 2012 at 11:35 PM, Andrea Adami <andrea.adami@gmail.com> wrote:
>>> * The actual rule only works for devices reporting ts pressure
>>> * and ignores the other touchscreens (happens e.g. on ipaq h1940).
>>> * a0,1,*18   a = absolute axes (ABS_X 0x00 ABS_Y 0x01 ABS_PRESSURE 0x18)
>>> * (see include/linux/input.h)
>>> *
>>> * Guidelines for touchscreen seem suggesting only ABS_X and ABS_Y are mandatory
>>> * (http://kernel.org/doc/Documentation/input/event-codes.txt)
>>> *
>>> * Bump PR
>>> 
>>> Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
>>> ---
>>> meta/recipes-core/udev/udev/local.rules |    3 +--
>>> meta/recipes-core/udev/udev_164.bb      |    2 +-
>>> 2 files changed, 2 insertions(+), 3 deletions(-)
>>> 
>>> diff --git a/meta/recipes-core/udev/udev/local.rules b/meta/recipes-core/udev/udev/local.rules
>>> index 625e49a..49e8d28 100644
>>> --- a/meta/recipes-core/udev/udev/local.rules
>>> +++ b/meta/recipes-core/udev/udev/local.rules
>>> @@ -31,5 +31,4 @@ KERNEL=="rtc0", SYMLINK+="rtc"
>>> ACTION=="add", DEVPATH=="/devices/*", ENV{MODALIAS}=="?*", RUN+="/sbin/modprobe $env{MODALIAS}"
>>> 
>>> # Create a symlink to any touchscreen input device
>>> -SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*18,*", SYMLINK+="input/touchscreen0"
>>> -
>>> +SUBSYSTEM=="input", KERNEL=="event[0-9]*", ATTRS{modalias}=="input:*-e0*,3,*a0,1,*", SYMLINK+="input/touchscreen0"
>>> diff --git a/meta/recipes-core/udev/udev_164.bb b/meta/recipes-core/udev/udev_164.bb
>>> index c5813ec..0462ff2 100644
>>> --- a/meta/recipes-core/udev/udev_164.bb
>>> +++ b/meta/recipes-core/udev/udev_164.bb
>>> @@ -1,6 +1,6 @@
>>> include udev.inc
>>> 
>>> -PR = "r13"
>>> +PR = "r14"
>>> 
>>> SRC_URI += "file://udev-166-v4l1-1.patch"
>>> 
>>> --
>>> 1.7.3.4
>>> 
>> 
>> Bump
>> 
>> I'm not linux-input specialist but this fix looks rather harmless.
> 
> I'm not 100% convinced about that. There needs to be some mechanism to
> signal to the system when there is pressure applied to the screen and
> when pressure was released. I think there are two ways you can do this,
> BTN_TOUCH and ABS_PRESSURE. Regardless of what the input events codes
> say, I'm really more interested in what tslib accepts.
> 
> This code has been like this for a long time and I'm surprised that if
> there were issues it wasn't patched long before now. I'm therefore left
> wondering if the touchscreen driver you're using is doing the right
> things.

FWIW, we had a similar patch to udev in the TI overlay, but the kernel team fixed the ts driver in the kernel a few weeks later.  Qt and X11 can use evdev natively nowadays, so tslib usage is on its way out.

regards,

Koen

Re: [PATCH] udev: generalize the check in touchscreen rule

[Andrea Adami]

As far as I understand that, the rule was originally written for corgi
touchscreen and did indeed work for ads7846 ts.

Then something went wrong and this patch appeared: "udev 124: support
devices with ads7846 touchscreen and wrong modalias"

http://lists.linuxtogo.org/pipermail/openembedded-commits/2009-January/022948.html

Finally, bluelightning reported that on ipaq h1940 the rule did not
match and added a .bbappend in meta-handheld.

I analyzed the output of the touchscreen I have (Zaurus poodle, corgi,
spitz) and the generic rule still matched fine, because sysfs reports
18 (pressure).

Now, looking at the s3c2410_ts.c driver on that ipaq, it seems to me
it only reports:

125                         input_report_abs(ts.input, ABS_X, ts.xp);
126                         input_report_abs(ts.input, ABS_Y, ts.yp);
127
128                         input_report_key(ts.input, BTN_TOUCH, 1);
129                         input_sync(ts.input);


but not pressure.

In fact, ABS_X and ABS_Y are reported, there are no relative axes,
there is no pressure.
That should be enough to classify it as touchscreen imho.

Regards

Andrea

Re: [PATCH] udev: generalize the check in touchscreen rule

[Marko Katić]

[-- Attachment #1: Type: text/plain, Size: 3365 bytes --]

Current tslib indeed does use ABS_PRESSURE to signal applied pressure.
BTN_TOUCH is only used to indicate pen up events. This happens in the
input_raw module.
Tslib could be modified to check whether the underlying input device
supports ABS_PRESSURE. if not, it shoud use BTN_TOUCH to signal applied
pressure.


On Wed, Apr 11, 2012 at 12:16 PM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Wed, 2012-04-11 at 10:59 +0200, Andrea Adami wrote:
> > On Mon, Apr 2, 2012 at 11:35 PM, Andrea Adami <andrea.adami@gmail.com>
> wrote:
> > > * The actual rule only works for devices reporting ts pressure
> > > * and ignores the other touchscreens (happens e.g. on ipaq h1940).
> > > * a0,1,*18   a = absolute axes (ABS_X 0x00 ABS_Y 0x01 ABS_PRESSURE
> 0x18)
> > > * (see include/linux/input.h)
> > > *
> > > * Guidelines for touchscreen seem suggesting only ABS_X and ABS_Y are
> mandatory
> > > * (http://kernel.org/doc/Documentation/input/event-codes.txt)
> > > *
> > > * Bump PR
> > >
> > > Signed-off-by: Andrea Adami <andrea.adami@gmail.com>
> > > ---
> > >  meta/recipes-core/udev/udev/local.rules |    3 +--
> > >  meta/recipes-core/udev/udev_164.bb      |    2 +-
> > >  2 files changed, 2 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/meta/recipes-core/udev/udev/local.rules
> b/meta/recipes-core/udev/udev/local.rules
> > > index 625e49a..49e8d28 100644
> > > --- a/meta/recipes-core/udev/udev/local.rules
> > > +++ b/meta/recipes-core/udev/udev/local.rules
> > > @@ -31,5 +31,4 @@ KERNEL=="rtc0", SYMLINK+="rtc"
> > >  ACTION=="add", DEVPATH=="/devices/*", ENV{MODALIAS}=="?*",
> RUN+="/sbin/modprobe $env{MODALIAS}"
> > >
> > >  # Create a symlink to any touchscreen input device
> > > -SUBSYSTEM=="input", KERNEL=="event[0-9]*",
> ATTRS{modalias}=="input:*-e0*,3,*a0,1,*18,*", SYMLINK+="input/touchscreen0"
> > > -
> > > +SUBSYSTEM=="input", KERNEL=="event[0-9]*",
> ATTRS{modalias}=="input:*-e0*,3,*a0,1,*", SYMLINK+="input/touchscreen0"
> > > diff --git a/meta/recipes-core/udev/udev_164.bbb/meta/recipes-core/udev/
> udev_164.bb
> > > index c5813ec..0462ff2 100644
> > > --- a/meta/recipes-core/udev/udev_164.bb
> > > +++ b/meta/recipes-core/udev/udev_164.bb
> > > @@ -1,6 +1,6 @@
> > >  include udev.inc
> > >
> > > -PR = "r13"
> > > +PR = "r14"
> > >
> > >  SRC_URI += "file://udev-166-v4l1-1.patch"
> > >
> > > --
> > > 1.7.3.4
> > >
> >
> > Bump
> >
> > I'm not linux-input specialist but this fix looks rather harmless.
>
> I'm not 100% convinced about that. There needs to be some mechanism to
> signal to the system when there is pressure applied to the screen and
> when pressure was released. I think there are two ways you can do this,
> BTN_TOUCH and ABS_PRESSURE. Regardless of what the input events codes
> say, I'm really more interested in what tslib accepts.
>
> This code has been like this for a long time and I'm surprised that if
> there were issues it wasn't patched long before now. I'm therefore left
> wondering if the touchscreen driver you're using is doing the right
> things.
>
> Cheers,
>
> Richard
>
>
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
>

[-- Attachment #2: Type: text/html, Size: 4912 bytes --]

Re: [PATCH] udev: generalize the check in touchscreen rule

[Paul Eggleton]

On Wednesday 11 April 2012 13:57:29 Andrea Adami wrote:
> Finally, bluelightning reported that on ipaq h1940 the rule did not
> match and added a .bbappend in meta-handheld.

FYI this is because pressure support was removed from the driver. I don't know 
if there's ever hope of it coming back.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] udev: generalize the check in touchscreen rule

[Andrea Adami]

On Wed, Apr 11, 2012 at 2:55 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> On Wednesday 11 April 2012 13:57:29 Andrea Adami wrote:
>> Finally, bluelightning reported that on ipaq h1940 the rule did not
>> match and added a .bbappend in meta-handheld.
>
> FYI this is because pressure support was removed from the driver. I don't know
> if there's ever hope of it coming back.

Well, there was a NACKed patch:

https://lkml.org/lkml/2011/6/23/817

Mark Brown:
"It seems much more sane to fix this in tslib, the kernel is not actually
reporting pressure meaningfully here and there's already BTN_TOUCH to
report if the pen is down."

Regards

Andrea

>
> Cheers,
> Paul
>
> --
>
> Paul Eggleton
> Intel Open Source Technology Centre
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Re: [PATCH] udev: generalize the check in touchscreen rule

[Paul Eggleton]

On Wednesday 11 April 2012 15:09:10 Andrea Adami wrote:
> On Wed, Apr 11, 2012 at 2:55 PM, Paul Eggleton
> <paul.eggleton@linux.intel.com> wrote:
> > On Wednesday 11 April 2012 13:57:29 Andrea Adami wrote:
> >> Finally, bluelightning reported that on ipaq h1940 the rule did not
> >> match and added a .bbappend in meta-handheld.
> > 
> > FYI this is because pressure support was removed from the driver. I don't
> > know if there's ever hope of it coming back.
> 
> Well, there was a NACKed patch:
> 
> https://lkml.org/lkml/2011/6/23/817
> 
> Mark Brown:
> "It seems much more sane to fix this in tslib, the kernel is not actually
> reporting pressure meaningfully here and there's already BTN_TOUCH to
> report if the pen is down."

Interesting. FWIW, I can report that with a patch to the rule such as this one 
at least on h1940 the touchscreen works fine, and there was no need to 
additionally fix anything in tslib.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] udev: generalize the check in touchscreen rule

[Paul Eggleton]

On Wednesday 11 April 2012 14:15:57 Paul Eggleton wrote:
> FWIW, I can report that with a patch to the rule such as this
> one 

Just to clarify, I meant Andrea's patch to the udev rule, not the NACKed 
kernel patch.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH] udev: generalize the check in touchscreen rule

[Andrea Adami]

On Wed, Apr 11, 2012 at 3:18 PM, Paul Eggleton
<paul.eggleton@linux.intel.com> wrote:
> On Wednesday 11 April 2012 14:15:57 Paul Eggleton wrote:
>> FWIW, I can report that with a patch to the rule such as this
>> one
>
> Just to clarify, I meant Andrea's patch to the udev rule, not the NACKed
> kernel patch.
>
> Cheers,
> Paul
>
In fact the patch had been already NACKed :

https://lkml.org/lkml/2010/11/9/270

Dmitry Torokhov
"No, if device does not provide true pressure readings it should not send
ABS_PRESSURE events. Please fix your userspace."

I think this can explain what's happening.

For the specific case, instead of changing the rule, we could just add
a new rule for that s3c2410 based on the ATTRS{modalias}.

Regards

Andrea