Re: [GIT] HID for 3.12 merge window

Re: [GIT] HID for 3.12 merge window

[Markus Trippelsdorf]

On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
> 
> David Herrmann (12):
...
>       HID: wiimote: add support for Guitar-Hero drums

 commit 61e00655e9cb82e034eb72b95a51072e718d14a7
 Author: David Herrmann <dh.herrmann@gmail.com>
 Date:   Mon Aug 26 19:14:46 2013 +0200

     Input: introduce BTN/ABS bits for drums and guitars

The commit above breaks my Logitech mouse. The mouse cursor just sits in
the middle of the screen and doesn't react to movements. dmesg is
normal, but Xorg.0.log says:

[     5.717] (II) config/udev: Adding input device Logitech Unifying Device. Wireless PID:4026 (/dev/input/event0)
[     5.717] (**) Logitech Unifying Device. Wireless PID:4026: Applying InputClass "evdev pointer catchall"
[     5.717] (**) Logitech Unifying Device. Wireless PID:4026: Applying InputClass "evdev keyboard catchall"
[     5.717] (II) Using input driver 'evdev' for 'Logitech Unifying Device. Wireless PID:4026'
[     5.717] (**) Logitech Unifying Device. Wireless PID:4026: always reports core events
[     5.717] (**) evdev: Logitech Unifying Device. Wireless PID:4026: Device: "/dev/input/event0"
[     5.717] (EE) evdev: Logitech Unifying Device. Wireless PID:4026: ioctl EVIOCGABSi(32) failed: Invalid argument
[     5.763] (EE) PreInit returned 8 for "Logitech Unifying Device. Wireless PID:4026"
[     5.763] (II) UnloadModule: "evdev"
[     5.763] (II) config/udev: Adding input device Logitech Unifying Device. Wireless PID:4026 (/dev/input/mouse0)
[     5.763] (II) No input driver specified, ignoring this device.
[     5.763] (II) This device may have been added with another device file.

I've double-checked the bisection by reverting the commit on top of
current tree and this fixes the issue...

>From dmesg:
[    1.551437] XFS (sda): Mounting Filesystem
[    1.555823] tsc: Refined TSC clocksource calibration: 3210.826 MHz
[    1.729233] usb 4-2: new full-speed USB device number 2 using ohci-pci
[    1.857691] XFS (sda): Ending clean mount
[    1.915581] logitech-djreceiver 0003:046D:C52B.0003: hiddev0,hidraw0: USB HID v1.11 Device [Logitech USB Receiver] on usb-0000:00:12.1-2/input2
[    1.920263] input: Logitech Unifying Device. Wireless PID:4026 as /devices/pci0000:00/0000:00:12.1/usb4/4-2/4-2:1.2/0003:046D:C52B.0003/input/input0
[    1.920479] logitech-djdevice 0003:046D:C52B.0004: input,hidraw1: USB HID v1.11 Keyboard [Logitech Unifying Device. Wireless PID:4026] on usb-0000:00:12.1-2:1
[    2.042641] usb 3-1: new low-speed USB device number 2 using ohci-pci
[    2.204791] input: HID 046a:0011 as /devices/pci0000:00/0000:00:12.0/usb3/3-1/3-1:1.0/input/input1
[    2.204975] hid-generic 0003:046A:0011.0005: input,hidraw2: USB HID v1.10 Keyboard [HID 046a:0011] on usb-0000:00:12.0-1/input0
[    2.341424] udevd[98]: starting eudev version 1.0
[    2.556172] Switched to clocksource tsc
[    2.791794] ATL1E 0000:02:00.0 eth0: NIC Link is Up <1000 Mbps Full Duplex>
[    3.945433] Adding 3071996k swap on /var/cache/swapfile.img.  Priority:-1 extents:1 across:3071996k 

-- 
Markus

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

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

Hi

On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
>>
>> David Herrmann (12):
> ...
>>       HID: wiimote: add support for Guitar-Hero drums
>
>  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>  Author: David Herrmann <dh.herrmann@gmail.com>
>  Date:   Mon Aug 26 19:14:46 2013 +0200
>
>      Input: introduce BTN/ABS bits for drums and guitars
>
> The commit above breaks my Logitech mouse. The mouse cursor just sits in
> the middle of the screen and doesn't react to movements. dmesg is
> normal, but Xorg.0.log says:

Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
(used as mask). That wasn't really obvious to me. Attached is a patch
which should fix that. Could you apply it on top of linus/master and
give it a try?

@Dmitry: The IOC_NR part of the definition of EVIOCSABS() is now
bigger than 1-byte. I need to check how that affects the 'E' part. Any
idea what to do here?

Thanks
David

Patch is also attached as I doubt that inlining it works in that
stupid web-client:

>From 653fe4d46ad368cdbf9b56a559a8468bd6f5cb3c Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 6 Sep 2013 23:46:08 +0200
Subject: [PATCH] Input: evdev: don't assume ABS_MAX to be a power-of-2 minus 1

ABS_MAX is no longer a full mask. Hence, don't use it directly to get any
parameter for ioctls. Furthermore, the parameter-region and
ioctl-definition overlap, so even bumping ABS_MAX to 0x7f wouldn't help.

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/evdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d2b34fb..82e0073 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -939,12 +939,13 @@ static long evdev_do_ioctl(struct file *file,
unsigned int cmd,
  _IOC_NR(cmd) & EV_MAX, size,
  p, compat_mode);

- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
+ if (_IOC_NR(cmd) >= _IOC_NR(EVIOCGABS(0)) &&
+    _IOC_NR(cmd) <= _IOC_NR(EVIOCGABS(ABS_MAX))) {

  if (!dev->absinfo)
  return -EINVAL;

- t = _IOC_NR(cmd) & ABS_MAX;
+ t = _IOC_NR(cmd) - _IOC_NR(EVIOCGABS(0));
  abs = dev->absinfo[t];

  if (copy_to_user(p, &abs, min_t(size_t,
@@ -957,12 +958,13 @@ static long evdev_do_ioctl(struct file *file,
unsigned int cmd,

  if (_IOC_DIR(cmd) == _IOC_WRITE) {

- if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
+ if (_IOC_NR(cmd) >= _IOC_NR(EVIOCSABS(0)) &&
+    _IOC_NR(cmd) <= _IOC_NR(EVIOCSABS(ABS_MAX))) {

  if (!dev->absinfo)
  return -EINVAL;

- t = _IOC_NR(cmd) & ABS_MAX;
+ t = _IOC_NR(cmd) - _IOC_NR(EVIOCSABS(0));

  if (copy_from_user(&abs, p, min_t(size_t,
  size, sizeof(struct input_absinfo))))
-- 
1.8.4

[-- Attachment #2: 0001-Input-evdev-don-t-assume-ABS_MAX-to-be-a-power-of-2-.patch --]
[-- Type: application/octet-stream, Size: 1820 bytes --]

From 653fe4d46ad368cdbf9b56a559a8468bd6f5cb3c Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 6 Sep 2013 23:46:08 +0200
Subject: [PATCH] Input: evdev: don't assume ABS_MAX to be a power-of-2 minus 1

ABS_MAX is no longer a full mask. Hence, don't use it directly to get any
parameter for ioctls. Furthermore, the parameter-region and
ioctl-definition overlap, so even bumping ABS_MAX to 0x7f wouldn't help.

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 drivers/input/evdev.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/input/evdev.c b/drivers/input/evdev.c
index d2b34fb..82e0073 100644
--- a/drivers/input/evdev.c
+++ b/drivers/input/evdev.c
@@ -939,12 +939,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 						_IOC_NR(cmd) & EV_MAX, size,
 						p, compat_mode);
 
-		if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCGABS(0))) {
+		if (_IOC_NR(cmd) >= _IOC_NR(EVIOCGABS(0)) &&
+		    _IOC_NR(cmd) <= _IOC_NR(EVIOCGABS(ABS_MAX))) {
 
 			if (!dev->absinfo)
 				return -EINVAL;
 
-			t = _IOC_NR(cmd) & ABS_MAX;
+			t = _IOC_NR(cmd) - _IOC_NR(EVIOCGABS(0));
 			abs = dev->absinfo[t];
 
 			if (copy_to_user(p, &abs, min_t(size_t,
@@ -957,12 +958,13 @@ static long evdev_do_ioctl(struct file *file, unsigned int cmd,
 
 	if (_IOC_DIR(cmd) == _IOC_WRITE) {
 
-		if ((_IOC_NR(cmd) & ~ABS_MAX) == _IOC_NR(EVIOCSABS(0))) {
+		if (_IOC_NR(cmd) >= _IOC_NR(EVIOCSABS(0)) &&
+		    _IOC_NR(cmd) <= _IOC_NR(EVIOCSABS(ABS_MAX))) {
 
 			if (!dev->absinfo)
 				return -EINVAL;
 
-			t = _IOC_NR(cmd) & ABS_MAX;
+			t = _IOC_NR(cmd) - _IOC_NR(EVIOCSABS(0));
 
 			if (copy_from_user(&abs, p, min_t(size_t,
 					size, sizeof(struct input_absinfo))))
-- 
1.8.4

Re: [GIT] HID for 3.12 merge window

[Markus Trippelsdorf]

On 2013.09.06 at 23:50 +0200, David Herrmann wrote:
> Hi
> 
> On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
> >>
> >> David Herrmann (12):
> > ...
> >>       HID: wiimote: add support for Guitar-Hero drums
> >
> >  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
> >  Author: David Herrmann <dh.herrmann@gmail.com>
> >  Date:   Mon Aug 26 19:14:46 2013 +0200
> >
> >      Input: introduce BTN/ABS bits for drums and guitars
> >
> > The commit above breaks my Logitech mouse. The mouse cursor just sits in
> > the middle of the screen and doesn't react to movements. dmesg is
> > normal, but Xorg.0.log says:
> 
> Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
> (used as mask). That wasn't really obvious to me. Attached is a patch
> which should fix that. Could you apply it on top of linus/master and
> give it a try?

Your patch fixes the issue. Thanks.

-- 
Markus

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

Hi

On Fri, Sep 6, 2013 at 11:59 PM, Markus Trippelsdorf
<markus@trippelsdorf.de> wrote:
> On 2013.09.06 at 23:50 +0200, David Herrmann wrote:
>> Hi
>>
>> On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
>> <markus@trippelsdorf.de> wrote:
>> > On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
>> >>
>> >> David Herrmann (12):
>> > ...
>> >>       HID: wiimote: add support for Guitar-Hero drums
>> >
>> >  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>> >  Author: David Herrmann <dh.herrmann@gmail.com>
>> >  Date:   Mon Aug 26 19:14:46 2013 +0200
>> >
>> >      Input: introduce BTN/ABS bits for drums and guitars
>> >
>> > The commit above breaks my Logitech mouse. The mouse cursor just sits in
>> > the middle of the screen and doesn't react to movements. dmesg is
>> > normal, but Xorg.0.log says:
>>
>> Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
>> (used as mask). That wasn't really obvious to me. Attached is a patch
>> which should fix that. Could you apply it on top of linus/master and
>> give it a try?
>
> Your patch fixes the issue. Thanks.

Thanks a lot for reporting+testing!

I am still not sure how to solve the EVIOCSABS thingy. Problem is,
it's defined as:
  #define EVIOCSABS(_abs) ...0xc0 + (_abs)...
But if (_abs > 0x3f) this will be bigger than 0xff. Unfortunately, the
upper part of the ioctl is defined as 'E' which is 0x45 in hex and
thus sets the LSB. That means we cannot extend the _IOC_TYPE field to
the upper region (which would cause endian-issues, anyway). I guess
we're screwed here and need to revert that...

Dmitry, any comment on this? Or am I missing something?

Regards
David

Re: [GIT] HID for 3.12 merge window

[Dmitry Torokhov]

On Sat, Sep 07, 2013 at 12:51:27AM +0200, David Herrmann wrote:
> Hi
> 
> On Fri, Sep 6, 2013 at 11:59 PM, Markus Trippelsdorf
> <markus@trippelsdorf.de> wrote:
> > On 2013.09.06 at 23:50 +0200, David Herrmann wrote:
> >> Hi
> >>
> >> On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
> >> <markus@trippelsdorf.de> wrote:
> >> > On 2013.09.06 at 14:00 +0200, Jiri Kosina wrote:
> >> >>
> >> >> David Herrmann (12):
> >> > ...
> >> >>       HID: wiimote: add support for Guitar-Hero drums
> >> >
> >> >  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
> >> >  Author: David Herrmann <dh.herrmann@gmail.com>
> >> >  Date:   Mon Aug 26 19:14:46 2013 +0200
> >> >
> >> >      Input: introduce BTN/ABS bits for drums and guitars
> >> >
> >> > The commit above breaks my Logitech mouse. The mouse cursor just sits in
> >> > the middle of the screen and doesn't react to movements. dmesg is
> >> > normal, but Xorg.0.log says:
> >>
> >> Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
> >> (used as mask). That wasn't really obvious to me. Attached is a patch
> >> which should fix that. Could you apply it on top of linus/master and
> >> give it a try?
> >
> > Your patch fixes the issue. Thanks.
> 
> Thanks a lot for reporting+testing!
> 
> I am still not sure how to solve the EVIOCSABS thingy. Problem is,
> it's defined as:
>   #define EVIOCSABS(_abs) ...0xc0 + (_abs)...
> But if (_abs > 0x3f) this will be bigger than 0xff. Unfortunately, the
> upper part of the ioctl is defined as 'E' which is 0x45 in hex and
> thus sets the LSB. That means we cannot extend the _IOC_TYPE field to
> the upper region (which would cause endian-issues, anyway). I guess
> we're screwed here and need to revert that...
> 
> Dmitry, any comment on this? Or am I missing something?

We have gaps below ABS_MT constants, I think for 3.12 you could move
your whammy there and revert ABS_MAX change, but we need to plan for
expanding it in the future.

Thanks.

-- 
Dmitry

Re: [GIT] HID for 3.12 merge window

[Linus Torvalds]

On Fri, Sep 6, 2013 at 2:50 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
>>
>>  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>>  Author: David Herrmann <dh.herrmann@gmail.com>
>>  Date:   Mon Aug 26 19:14:46 2013 +0200
>>
>>      Input: introduce BTN/ABS bits for drums and guitars
>>
>> The commit above breaks my Logitech mouse. The mouse cursor just sits in
>> the middle of the screen and doesn't react to movements. dmesg is
>> normal, but Xorg.0.log says:
>
> Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus 1
> (used as mask). That wasn't really obvious to me. Attached is a patch
> which should fix that. Could you apply it on top of linus/master and
> give it a try?

Gah. I just wasted too much time bisecting down my logitech wireless
keyboard not working to within a few commits of this, and decided to
just try your patch.

And yes, it makes my keyboard work.

Dmitry, should I just apply the patch, or should we revert and use
other bits? Please, this needs to be resolved, I stopped merging when
I noticed this problem..

          Linus

Re: [GIT] HID for 3.12 merge window

[Dmitry Torokhov]

Linus Torvalds <torvalds@linux-foundation.org> wrote:
>On Fri, Sep 6, 2013 at 2:50 PM, David Herrmann <dh.herrmann@gmail.com>
>wrote:
>> Hi
>>
>> On Fri, Sep 6, 2013 at 10:20 PM, Markus Trippelsdorf
>>>
>>>  commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>>>  Author: David Herrmann <dh.herrmann@gmail.com>
>>>  Date:   Mon Aug 26 19:14:46 2013 +0200
>>>
>>>      Input: introduce BTN/ABS bits for drums and guitars
>>>
>>> The commit above breaks my Logitech mouse. The mouse cursor just
>sits in
>>> the middle of the screen and doesn't react to movements. dmesg is
>>> normal, but Xorg.0.log says:
>>
>> Ok, the issue is the kernel assumes ABS_MAX to be a power-of-2 minus
>1
>> (used as mask). That wasn't really obvious to me. Attached is a patch
>> which should fix that. Could you apply it on top of linus/master and
>> give it a try?
>
>Gah. I just wasted too much time bisecting down my logitech wireless
>keyboard not working to within a few commits of this, and decided to
>just try your patch.
>
>And yes, it makes my keyboard work.
>
>Dmitry, should I just apply the patch, or should we revert and use
>other bits? Please, this needs to be resolved, I stopped merging when
>I noticed this problem..

The patch still had problems so I'd revert it and wii bits and try again later.


Thanks.

-- 
Dmitry

Re: [GIT] HID for 3.12 merge window

[Linus Torvalds]

On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> The patch still had problems so I'd revert it and wii bits and try again later.

Ok. Mind giving me a list of commits, so that I don't have to do a
trial-and-error thing? I know the primary commit that causes problems,
but there are commits that seem to depend on it..

             Linus

Re: [GIT] HID for 3.12 merge window

[Dmitry Torokhov]

On Fri, Sep 06, 2013 at 06:00:29PM -0700, Linus Torvalds wrote:
> On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > The patch still had problems so I'd revert it and wii bits and try again later.
> 
> Ok. Mind giving me a list of commits, so that I don't have to do a
> trial-and-error thing? I know the primary commit that causes problems,
> but there are commits that seem to depend on it..


Sorry for the delay. I believe you need to revert:

73f8645 HID: wiimote: add support for Guitar-Hero drums
61e0065 Input: introduce BTN/ABS bits for drums and guitars

Hmm... there also was "HID: wiimote: add support for Guitar-Hero
guitars" but I do not see it...

Thanks.

-- 
Dmitry

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

Hi

On Sat, Sep 7, 2013 at 5:22 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Sep 06, 2013 at 06:00:29PM -0700, Linus Torvalds wrote:
>> On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>> >
>> > The patch still had problems so I'd revert it and wii bits and try again later.
>>
>> Ok. Mind giving me a list of commits, so that I don't have to do a
>> trial-and-error thing? I know the primary commit that causes problems,
>> but there are commits that seem to depend on it..
>
>
> Sorry for the delay. I believe you need to revert:
>
> 73f8645 HID: wiimote: add support for Guitar-Hero drums
> 61e0065 Input: introduce BTN/ABS bits for drums and guitars
>
> Hmm... there also was "HID: wiimote: add support for Guitar-Hero
> guitars" but I do not see it...

The commits to revert are:

commit 61e00655e9cb82e034eb72b95a51072e718d14a7
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Mon Aug 26 19:14:46 2013 +0200

    Input: introduce BTN/ABS bits for drums and guitars

commit 73f8645db1913ab2475ec3c1cee8d5f748963aa7
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Mon Aug 26 19:14:47 2013 +0200

    HID: wiimote: add support for Guitar-Hero drums

commit 8e22ecb603c88b7397ab2e80b7b0811b8a1318f5
Author: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
Date:   Mon Aug 26 19:14:48 2013 +0200

    HID: wiimote: add support for Guitar-Hero guitars

Last one is not from me, so I guess that's why Dmitry missed it. And
sorry for the delay, stupid UTC+1..
The bug only occurs for multi-touch devices (their ABS_* bits are
>0x1f), that's why I didn't see it happening during my tests..

If you didn't revert it, yet, attached (and inlined) is a patch which
reverts the ABS_MAX change and just moves the new identifiers in
between the others. On top of 3.12-rc1, this should fix all issues. If
you already reverted the patches, I guess we will just push it in
linux-next again and wait for 3.13?

Thanks
David

(Inlined patch below, also attached to mail as gmail hates long lines)

>From 13d549489b4bd2eade719cf2ec8c37724be6e640 Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Sat, 7 Sep 2013 09:17:11 +0200
Subject: [PATCH] Revert/Fix "Input: introduce BTN/ABS bits for drums and
 guitars"

commit 61e00655e9cb82e034eb72b95a51072e718d14a7
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Mon Aug 26 19:14:46 2013 +0200

    Input: introduce BTN/ABS bits for drums and guitars

This introduced several new identifiers for drums/guitar devices. However,
it also increased ABS_MAX. Unfortunately, 0x3f is already the maximum we
can use for ABS_MAX due to the limited EVIOCSABS ioctl.

Revert this commit and move the new identifiers somewhere in between. We
need to figure out some other way to add new identifiers for the future.
To avoid this happening again, I added a small comment.

This also fixes an issue where xf86-input-evdev failed on EVIOCGABS() due
to ABS_MAX not being a full mask.

Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/linux/mod_devicetable.h |  2 +-
 include/uapi/linux/input.h      | 33 ++++++++++++++++++---------------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 329aa30..45e9214 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -277,7 +277,7 @@ struct pcmcia_device_id {
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING 0x71
 #define INPUT_DEVICE_ID_KEY_MAX 0x2ff
 #define INPUT_DEVICE_ID_REL_MAX 0x0f
-#define INPUT_DEVICE_ID_ABS_MAX 0x4f
+#define INPUT_DEVICE_ID_ABS_MAX 0x3f
 #define INPUT_DEVICE_ID_MSC_MAX 0x07
 #define INPUT_DEVICE_ID_LED_MAX 0x0f
 #define INPUT_DEVICE_ID_SND_MAX 0x07
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 76457ee..12c33f8 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -819,8 +819,24 @@ struct input_keymap_entry {

 #define ABS_VOLUME 0x20

+/* Drums and guitars (mostly toys) */
+#define ABS_TOM_FAR_LEFT 0x21
+#define ABS_TOM_LEFT 0x22
+#define ABS_TOM_RIGHT 0x23
+#define ABS_TOM_FAR_RIGHT 0x24
+#define ABS_CYMBAL_FAR_LEFT 0x25
+#define ABS_CYMBAL_LEFT 0x26
+#define ABS_CYMBAL_RIGHT 0x27
+
 #define ABS_MISC 0x28

+/* Drums and guitars continued */
+#define ABS_CYMBAL_FAR_RIGHT 0x29
+#define ABS_BASS 0x2a
+#define ABS_HI_HAT 0x2b
+#define ABS_FRET_BOARD 0x2c /* Guitar fret board, vertical pos */
+#define ABS_WHAMMY_BAR 0x2d /* Guitar whammy bar (or vibrato) */
+
 #define ABS_MT_SLOT 0x2f /* MT slot being modified */
 #define ABS_MT_TOUCH_MAJOR 0x30 /* Major axis of touching ellipse */
 #define ABS_MT_TOUCH_MINOR 0x31 /* Minor axis (omit if circular) */
@@ -837,21 +853,8 @@ struct input_keymap_entry {
 #define ABS_MT_TOOL_X 0x3c /* Center X tool position */
 #define ABS_MT_TOOL_Y 0x3d /* Center Y tool position */

-/* Drums and guitars (mostly toys) */
-#define ABS_TOM_FAR_LEFT 0x40
-#define ABS_TOM_LEFT 0x41
-#define ABS_TOM_RIGHT 0x42
-#define ABS_TOM_FAR_RIGHT 0x43
-#define ABS_CYMBAL_FAR_LEFT 0x44
-#define ABS_CYMBAL_LEFT 0x45
-#define ABS_CYMBAL_RIGHT 0x46
-#define ABS_CYMBAL_FAR_RIGHT 0x47
-#define ABS_BASS 0x48
-#define ABS_HI_HAT 0x49
-#define ABS_FRET_BOARD 0x4a /* Guitar fret board, vertical pos */
-#define ABS_WHAMMY_BAR 0x4b /* Guitar whammy bar (or vibrato) */
-
-#define ABS_MAX 0x4f
+/* EVIOCSABS() does not support ABS_MAX > 0x3f */
+#define ABS_MAX 0x3f
 #define ABS_CNT (ABS_MAX+1)

 /*
-- 
1.8.4

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

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

And this time also attached...

On Sat, Sep 7, 2013 at 9:31 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sat, Sep 7, 2013 at 5:22 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Fri, Sep 06, 2013 at 06:00:29PM -0700, Linus Torvalds wrote:
>>> On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>> >
>>> > The patch still had problems so I'd revert it and wii bits and try again later.
>>>
>>> Ok. Mind giving me a list of commits, so that I don't have to do a
>>> trial-and-error thing? I know the primary commit that causes problems,
>>> but there are commits that seem to depend on it..
>>
>>
>> Sorry for the delay. I believe you need to revert:
>>
>> 73f8645 HID: wiimote: add support for Guitar-Hero drums
>> 61e0065 Input: introduce BTN/ABS bits for drums and guitars
>>
>> Hmm... there also was "HID: wiimote: add support for Guitar-Hero
>> guitars" but I do not see it...
>
> The commits to revert are:
>
> commit 61e00655e9cb82e034eb72b95a51072e718d14a7
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Mon Aug 26 19:14:46 2013 +0200
>
>     Input: introduce BTN/ABS bits for drums and guitars
>
> commit 73f8645db1913ab2475ec3c1cee8d5f748963aa7
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Mon Aug 26 19:14:47 2013 +0200
>
>     HID: wiimote: add support for Guitar-Hero drums
>
> commit 8e22ecb603c88b7397ab2e80b7b0811b8a1318f5
> Author: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
> Date:   Mon Aug 26 19:14:48 2013 +0200
>
>     HID: wiimote: add support for Guitar-Hero guitars
>
> Last one is not from me, so I guess that's why Dmitry missed it. And
> sorry for the delay, stupid UTC+1..
> The bug only occurs for multi-touch devices (their ABS_* bits are
>>0x1f), that's why I didn't see it happening during my tests..
>
> If you didn't revert it, yet, attached (and inlined) is a patch which
> reverts the ABS_MAX change and just moves the new identifiers in
> between the others. On top of 3.12-rc1, this should fix all issues. If
> you already reverted the patches, I guess we will just push it in
> linux-next again and wait for 3.13?
>
> Thanks
> David
[..snap..]

[-- Attachment #2: 0001-Revert-Fix-Input-introduce-BTN-ABS-bits-for-drums-an.patch --]
[-- Type: application/octet-stream, Size: 3540 bytes --]

From 13d549489b4bd2eade719cf2ec8c37724be6e640 Mon Sep 17 00:00:00 2001
From: David Herrmann <dh.herrmann@gmail.com>
Date: Sat, 7 Sep 2013 09:17:11 +0200
Subject: [PATCH] Revert/Fix "Input: introduce BTN/ABS bits for drums and
 guitars"

commit 61e00655e9cb82e034eb72b95a51072e718d14a7
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Mon Aug 26 19:14:46 2013 +0200

    Input: introduce BTN/ABS bits for drums and guitars

This introduced several new identifiers for drums/guitar devices. However,
it also increased ABS_MAX. Unfortunately, 0x3f is already the maximum we
can use for ABS_MAX due to the limited EVIOCSABS ioctl.

Revert this commit and move the new identifiers somewhere in between. We
need to figure out some other way to add new identifiers for the future.
To avoid this happening again, I added a small comment.

This also fixes an issue where xf86-input-evdev failed on EVIOCGABS() due
to ABS_MAX not being a full mask.

Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
---
 include/linux/mod_devicetable.h |  2 +-
 include/uapi/linux/input.h      | 33 ++++++++++++++++++---------------
 2 files changed, 19 insertions(+), 16 deletions(-)

diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 329aa30..45e9214 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -277,7 +277,7 @@ struct pcmcia_device_id {
 #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING	0x71
 #define INPUT_DEVICE_ID_KEY_MAX		0x2ff
 #define INPUT_DEVICE_ID_REL_MAX		0x0f
-#define INPUT_DEVICE_ID_ABS_MAX		0x4f
+#define INPUT_DEVICE_ID_ABS_MAX		0x3f
 #define INPUT_DEVICE_ID_MSC_MAX		0x07
 #define INPUT_DEVICE_ID_LED_MAX		0x0f
 #define INPUT_DEVICE_ID_SND_MAX		0x07
diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index 76457ee..12c33f8 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -819,8 +819,24 @@ struct input_keymap_entry {
 
 #define ABS_VOLUME		0x20
 
+/* Drums and guitars (mostly toys) */
+#define ABS_TOM_FAR_LEFT	0x21
+#define ABS_TOM_LEFT		0x22
+#define ABS_TOM_RIGHT		0x23
+#define ABS_TOM_FAR_RIGHT	0x24
+#define ABS_CYMBAL_FAR_LEFT	0x25
+#define ABS_CYMBAL_LEFT		0x26
+#define ABS_CYMBAL_RIGHT	0x27
+
 #define ABS_MISC		0x28
 
+/* Drums and guitars continued */
+#define ABS_CYMBAL_FAR_RIGHT	0x29
+#define ABS_BASS		0x2a
+#define ABS_HI_HAT		0x2b
+#define ABS_FRET_BOARD		0x2c	/* Guitar fret board, vertical pos */
+#define ABS_WHAMMY_BAR		0x2d	/* Guitar whammy bar (or vibrato) */
+
 #define ABS_MT_SLOT		0x2f	/* MT slot being modified */
 #define ABS_MT_TOUCH_MAJOR	0x30	/* Major axis of touching ellipse */
 #define ABS_MT_TOUCH_MINOR	0x31	/* Minor axis (omit if circular) */
@@ -837,21 +853,8 @@ struct input_keymap_entry {
 #define ABS_MT_TOOL_X		0x3c	/* Center X tool position */
 #define ABS_MT_TOOL_Y		0x3d	/* Center Y tool position */
 
-/* Drums and guitars (mostly toys) */
-#define ABS_TOM_FAR_LEFT	0x40
-#define ABS_TOM_LEFT		0x41
-#define ABS_TOM_RIGHT		0x42
-#define ABS_TOM_FAR_RIGHT	0x43
-#define ABS_CYMBAL_FAR_LEFT	0x44
-#define ABS_CYMBAL_LEFT		0x45
-#define ABS_CYMBAL_RIGHT	0x46
-#define ABS_CYMBAL_FAR_RIGHT	0x47
-#define ABS_BASS		0x48
-#define ABS_HI_HAT		0x49
-#define ABS_FRET_BOARD		0x4a	/* Guitar fret board, vertical pos */
-#define ABS_WHAMMY_BAR		0x4b	/* Guitar whammy bar (or vibrato) */
-
-#define ABS_MAX			0x4f
+/* EVIOCSABS() does not support ABS_MAX > 0x3f */
+#define ABS_MAX			0x3f
 #define ABS_CNT			(ABS_MAX+1)
 
 /*
-- 
1.8.4

Re: [GIT] HID for 3.12 merge window

[Benjamin Tissoires]

On Sat, Sep 7, 2013 at 9:31 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Sat, Sep 7, 2013 at 5:22 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>> On Fri, Sep 06, 2013 at 06:00:29PM -0700, Linus Torvalds wrote:
>>> On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>> >
>>> > The patch still had problems so I'd revert it and wii bits and try again later.
>>>
>>> Ok. Mind giving me a list of commits, so that I don't have to do a
>>> trial-and-error thing? I know the primary commit that causes problems,
>>> but there are commits that seem to depend on it..
>>
>>
>> Sorry for the delay. I believe you need to revert:
>>
>> 73f8645 HID: wiimote: add support for Guitar-Hero drums
>> 61e0065 Input: introduce BTN/ABS bits for drums and guitars
>>
>> Hmm... there also was "HID: wiimote: add support for Guitar-Hero
>> guitars" but I do not see it...
>
> The commits to revert are:
>
> commit 61e00655e9cb82e034eb72b95a51072e718d14a7
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Mon Aug 26 19:14:46 2013 +0200
>
>     Input: introduce BTN/ABS bits for drums and guitars
>
> commit 73f8645db1913ab2475ec3c1cee8d5f748963aa7
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Mon Aug 26 19:14:47 2013 +0200
>
>     HID: wiimote: add support for Guitar-Hero drums
>
> commit 8e22ecb603c88b7397ab2e80b7b0811b8a1318f5
> Author: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
> Date:   Mon Aug 26 19:14:48 2013 +0200
>
>     HID: wiimote: add support for Guitar-Hero guitars
>
> Last one is not from me, so I guess that's why Dmitry missed it. And
> sorry for the delay, stupid UTC+1..
> The bug only occurs for multi-touch devices (their ABS_* bits are
>>0x1f), that's why I didn't see it happening during my tests..
>
> If you didn't revert it, yet, attached (and inlined) is a patch which
> reverts the ABS_MAX change and just moves the new identifiers in
> between the others. On top of 3.12-rc1, this should fix all issues. If
> you already reverted the patches, I guess we will just push it in
> linux-next again and wait for 3.13?
>
> Thanks
> David
>
> (Inlined patch below, also attached to mail as gmail hates long lines)
>
> From 13d549489b4bd2eade719cf2ec8c37724be6e640 Mon Sep 17 00:00:00 2001
> From: David Herrmann <dh.herrmann@gmail.com>
> Date: Sat, 7 Sep 2013 09:17:11 +0200
> Subject: [PATCH] Revert/Fix "Input: introduce BTN/ABS bits for drums and
>  guitars"
>
> commit 61e00655e9cb82e034eb72b95a51072e718d14a7
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Mon Aug 26 19:14:46 2013 +0200
>
>     Input: introduce BTN/ABS bits for drums and guitars
>
> This introduced several new identifiers for drums/guitar devices. However,
> it also increased ABS_MAX. Unfortunately, 0x3f is already the maximum we
> can use for ABS_MAX due to the limited EVIOCSABS ioctl.
>
> Revert this commit and move the new identifiers somewhere in between. We
> need to figure out some other way to add new identifiers for the future.
> To avoid this happening again, I added a small comment.
>
> This also fixes an issue where xf86-input-evdev failed on EVIOCGABS() due
> to ABS_MAX not being a full mask.
>
> Markus Trippelsdorf <markus@trippelsdorf.de>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> ---
>  include/linux/mod_devicetable.h |  2 +-
>  include/uapi/linux/input.h      | 33 ++++++++++++++++++---------------
>  2 files changed, 19 insertions(+), 16 deletions(-)
>
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 329aa30..45e9214 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -277,7 +277,7 @@ struct pcmcia_device_id {
>  #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING 0x71
>  #define INPUT_DEVICE_ID_KEY_MAX 0x2ff
>  #define INPUT_DEVICE_ID_REL_MAX 0x0f
> -#define INPUT_DEVICE_ID_ABS_MAX 0x4f
> +#define INPUT_DEVICE_ID_ABS_MAX 0x3f
>  #define INPUT_DEVICE_ID_MSC_MAX 0x07
>  #define INPUT_DEVICE_ID_LED_MAX 0x0f
>  #define INPUT_DEVICE_ID_SND_MAX 0x07
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index 76457ee..12c33f8 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -819,8 +819,24 @@ struct input_keymap_entry {
>
>  #define ABS_VOLUME 0x20
>
> +/* Drums and guitars (mostly toys) */
> +#define ABS_TOM_FAR_LEFT 0x21
> +#define ABS_TOM_LEFT 0x22
> +#define ABS_TOM_RIGHT 0x23
> +#define ABS_TOM_FAR_RIGHT 0x24
> +#define ABS_CYMBAL_FAR_LEFT 0x25
> +#define ABS_CYMBAL_LEFT 0x26
> +#define ABS_CYMBAL_RIGHT 0x27
> +
>  #define ABS_MISC 0x28
>
> +/* Drums and guitars continued */
> +#define ABS_CYMBAL_FAR_RIGHT 0x29
> +#define ABS_BASS 0x2a
> +#define ABS_HI_HAT 0x2b
> +#define ABS_FRET_BOARD 0x2c /* Guitar fret board, vertical pos */
> +#define ABS_WHAMMY_BAR 0x2d /* Guitar whammy bar (or vibrato) */
> +

I'm not particularly in favor of adding semantic between ABS_MISC and
ABS_MT_SLOT. We already did that once with the ABS_MT_* stuff, and the
problem comes from devices showing several axes, not really mapped
(like joysticks). These axes are all mapped to ABS_MISC, but the input
core function map them to ABS_MISC+N. This way some joysticks show
multitouch axes and are treated as such, whereas the axis are just
regular absolute axes. (not sure I am clear enough... :( )

Currently, nothing is done in X or in Wayland to detect these false
multitouch devices with a large number of absolute axes. But if we
keep a hole between ABS_MISC and ABS_MT_SLOT, we could add an
heuristic like:
"if all the axes between ABS_MISC and ABS_MT_SLOT are available, then
ABS_MT_SLOT does not mean multitouch, but ABS_MISC+6."

Cheers,
Benjamin

>  #define ABS_MT_SLOT 0x2f /* MT slot being modified */
>  #define ABS_MT_TOUCH_MAJOR 0x30 /* Major axis of touching ellipse */
>  #define ABS_MT_TOUCH_MINOR 0x31 /* Minor axis (omit if circular) */
> @@ -837,21 +853,8 @@ struct input_keymap_entry {
>  #define ABS_MT_TOOL_X 0x3c /* Center X tool position */
>  #define ABS_MT_TOOL_Y 0x3d /* Center Y tool position */
>
> -/* Drums and guitars (mostly toys) */
> -#define ABS_TOM_FAR_LEFT 0x40
> -#define ABS_TOM_LEFT 0x41
> -#define ABS_TOM_RIGHT 0x42
> -#define ABS_TOM_FAR_RIGHT 0x43
> -#define ABS_CYMBAL_FAR_LEFT 0x44
> -#define ABS_CYMBAL_LEFT 0x45
> -#define ABS_CYMBAL_RIGHT 0x46
> -#define ABS_CYMBAL_FAR_RIGHT 0x47
> -#define ABS_BASS 0x48
> -#define ABS_HI_HAT 0x49
> -#define ABS_FRET_BOARD 0x4a /* Guitar fret board, vertical pos */
> -#define ABS_WHAMMY_BAR 0x4b /* Guitar whammy bar (or vibrato) */
> -
> -#define ABS_MAX 0x4f
> +/* EVIOCSABS() does not support ABS_MAX > 0x3f */
> +#define ABS_MAX 0x3f
>  #define ABS_CNT (ABS_MAX+1)
>
>  /*
> --
> 1.8.4
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

Hi

On Sat, Sep 7, 2013 at 10:24 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
> On Sat, Sep 7, 2013 at 9:31 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>> Hi
>>
>> On Sat, Sep 7, 2013 at 5:22 AM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>> On Fri, Sep 06, 2013 at 06:00:29PM -0700, Linus Torvalds wrote:
>>>> On Fri, Sep 6, 2013 at 5:58 PM, Dmitry Torokhov
>>>> <dmitry.torokhov@gmail.com> wrote:
>>>> >
>>>> > The patch still had problems so I'd revert it and wii bits and try again later.
>>>>
>>>> Ok. Mind giving me a list of commits, so that I don't have to do a
>>>> trial-and-error thing? I know the primary commit that causes problems,
>>>> but there are commits that seem to depend on it..
>>>
>>>
>>> Sorry for the delay. I believe you need to revert:
>>>
>>> 73f8645 HID: wiimote: add support for Guitar-Hero drums
>>> 61e0065 Input: introduce BTN/ABS bits for drums and guitars
>>>
>>> Hmm... there also was "HID: wiimote: add support for Guitar-Hero
>>> guitars" but I do not see it...
>>
>> The commits to revert are:
>>
>> commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>> Author: David Herrmann <dh.herrmann@gmail.com>
>> Date:   Mon Aug 26 19:14:46 2013 +0200
>>
>>     Input: introduce BTN/ABS bits for drums and guitars
>>
>> commit 73f8645db1913ab2475ec3c1cee8d5f748963aa7
>> Author: David Herrmann <dh.herrmann@gmail.com>
>> Date:   Mon Aug 26 19:14:47 2013 +0200
>>
>>     HID: wiimote: add support for Guitar-Hero drums
>>
>> commit 8e22ecb603c88b7397ab2e80b7b0811b8a1318f5
>> Author: Nicolas Adenis-Lamarre <nicolas.adenis.lamarre@gmail.com>
>> Date:   Mon Aug 26 19:14:48 2013 +0200
>>
>>     HID: wiimote: add support for Guitar-Hero guitars
>>
>> Last one is not from me, so I guess that's why Dmitry missed it. And
>> sorry for the delay, stupid UTC+1..
>> The bug only occurs for multi-touch devices (their ABS_* bits are
>>>0x1f), that's why I didn't see it happening during my tests..
>>
>> If you didn't revert it, yet, attached (and inlined) is a patch which
>> reverts the ABS_MAX change and just moves the new identifiers in
>> between the others. On top of 3.12-rc1, this should fix all issues. If
>> you already reverted the patches, I guess we will just push it in
>> linux-next again and wait for 3.13?
>>
>> Thanks
>> David
>>
>> (Inlined patch below, also attached to mail as gmail hates long lines)
>>
>> From 13d549489b4bd2eade719cf2ec8c37724be6e640 Mon Sep 17 00:00:00 2001
>> From: David Herrmann <dh.herrmann@gmail.com>
>> Date: Sat, 7 Sep 2013 09:17:11 +0200
>> Subject: [PATCH] Revert/Fix "Input: introduce BTN/ABS bits for drums and
>>  guitars"
>>
>> commit 61e00655e9cb82e034eb72b95a51072e718d14a7
>> Author: David Herrmann <dh.herrmann@gmail.com>
>> Date:   Mon Aug 26 19:14:46 2013 +0200
>>
>>     Input: introduce BTN/ABS bits for drums and guitars
>>
>> This introduced several new identifiers for drums/guitar devices. However,
>> it also increased ABS_MAX. Unfortunately, 0x3f is already the maximum we
>> can use for ABS_MAX due to the limited EVIOCSABS ioctl.
>>
>> Revert this commit and move the new identifiers somewhere in between. We
>> need to figure out some other way to add new identifiers for the future.
>> To avoid this happening again, I added a small comment.
>>
>> This also fixes an issue where xf86-input-evdev failed on EVIOCGABS() due
>> to ABS_MAX not being a full mask.
>>
>> Markus Trippelsdorf <markus@trippelsdorf.de>
>> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> ---
>>  include/linux/mod_devicetable.h |  2 +-
>>  include/uapi/linux/input.h      | 33 ++++++++++++++++++---------------
>>  2 files changed, 19 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
>> index 329aa30..45e9214 100644
>> --- a/include/linux/mod_devicetable.h
>> +++ b/include/linux/mod_devicetable.h
>> @@ -277,7 +277,7 @@ struct pcmcia_device_id {
>>  #define INPUT_DEVICE_ID_KEY_MIN_INTERESTING 0x71
>>  #define INPUT_DEVICE_ID_KEY_MAX 0x2ff
>>  #define INPUT_DEVICE_ID_REL_MAX 0x0f
>> -#define INPUT_DEVICE_ID_ABS_MAX 0x4f
>> +#define INPUT_DEVICE_ID_ABS_MAX 0x3f
>>  #define INPUT_DEVICE_ID_MSC_MAX 0x07
>>  #define INPUT_DEVICE_ID_LED_MAX 0x0f
>>  #define INPUT_DEVICE_ID_SND_MAX 0x07
>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>> index 76457ee..12c33f8 100644
>> --- a/include/uapi/linux/input.h
>> +++ b/include/uapi/linux/input.h
>> @@ -819,8 +819,24 @@ struct input_keymap_entry {
>>
>>  #define ABS_VOLUME 0x20
>>
>> +/* Drums and guitars (mostly toys) */
>> +#define ABS_TOM_FAR_LEFT 0x21
>> +#define ABS_TOM_LEFT 0x22
>> +#define ABS_TOM_RIGHT 0x23
>> +#define ABS_TOM_FAR_RIGHT 0x24
>> +#define ABS_CYMBAL_FAR_LEFT 0x25
>> +#define ABS_CYMBAL_LEFT 0x26
>> +#define ABS_CYMBAL_RIGHT 0x27
>> +
>>  #define ABS_MISC 0x28
>>
>> +/* Drums and guitars continued */
>> +#define ABS_CYMBAL_FAR_RIGHT 0x29
>> +#define ABS_BASS 0x2a
>> +#define ABS_HI_HAT 0x2b
>> +#define ABS_FRET_BOARD 0x2c /* Guitar fret board, vertical pos */
>> +#define ABS_WHAMMY_BAR 0x2d /* Guitar whammy bar (or vibrato) */
>> +
>
> I'm not particularly in favor of adding semantic between ABS_MISC and
> ABS_MT_SLOT. We already did that once with the ABS_MT_* stuff, and the
> problem comes from devices showing several axes, not really mapped
> (like joysticks). These axes are all mapped to ABS_MISC, but the input
> core function map them to ABS_MISC+N. This way some joysticks show
> multitouch axes and are treated as such, whereas the axis are just
> regular absolute axes. (not sure I am clear enough... :( )
>
> Currently, nothing is done in X or in Wayland to detect these false
> multitouch devices with a large number of absolute axes. But if we
> keep a hole between ABS_MISC and ABS_MT_SLOT, we could add an
> heuristic like:
> "if all the axes between ABS_MISC and ABS_MT_SLOT are available, then
> ABS_MT_SLOT does not mean multitouch, but ABS_MISC+6."

So Xorg just took control over the unused ABS_* slots? Hmm, then I
have no other idea than reverting the 3 patches.
Sadly, that also means there's no way for us to easily extend the
ABS_* bits. If someone has an idea, let me know. But for now I'll just
put the drums/guitar patches on hold.

Thanks
David

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

Hi

On Sat, Sep 7, 2013 at 11:20 AM, Benjamin Tissoires
<benjamin.tissoires@gmail.com> wrote:
>
>
> On 07/09/13 10:57, David Herrmann wrote:
>> Hi
>>
>> On Sat, Sep 7, 2013 at 10:24 AM, Benjamin Tissoires
>>> I'm not particularly in favor of adding semantic between ABS_MISC and
>>> ABS_MT_SLOT. We already did that once with the ABS_MT_* stuff, and the
>>> problem comes from devices showing several axes, not really mapped
>>> (like joysticks). These axes are all mapped to ABS_MISC, but the input
>>> core function map them to ABS_MISC+N. This way some joysticks show
>>> multitouch axes and are treated as such, whereas the axis are just
>>> regular absolute axes. (not sure I am clear enough... :( )
>>>
>>> Currently, nothing is done in X or in Wayland to detect these false
>>> multitouch devices with a large number of absolute axes. But if we
>>> keep a hole between ABS_MISC and ABS_MT_SLOT, we could add an
>>> heuristic like:
>>> "if all the axes between ABS_MISC and ABS_MT_SLOT are available, then
>>> ABS_MT_SLOT does not mean multitouch, but ABS_MISC+6."
>>
>> So Xorg just took control over the unused ABS_* slots? Hmm, then I
>> have no other idea than reverting the 3 patches.
>> Sadly, that also means there's no way for us to easily extend the
>> ABS_* bits. If someone has an idea, let me know. But for now I'll just
>> put the drums/guitar patches on hold.
>>
>
> Well, I would say the "problem" lies in hid-input.c, line 919:
>>>>>>>>>>>>>>>>>>>
>         while (usage->code <= max && test_and_set_bit(usage->code, bit))
>                 usage->code = find_next_zero_bit(bit, max + 1, usage->code);
>>>>>>>>>>>>>>>>>>>
>
> So if you try to map several times ABS_MISC or any other axis, then you are mapping ABS_MISC+N.
>
> Anyway, the only problematic axis is ABS_MISC, as it is a default fallback. IMO, we can extend the ABS_* definitions after ABS_MT_TOOL_Y, like the original patch you proposed without having to deal with the above problem (though this will report the patch series to v3.13).
> To prevent future mistakes, we should just define ABS_MISC0 (0x29) to ABS_MISC5 (0x2e) to fill the hole.

As mentioned earlier in this thread, ABS_* is limited to 0x3f, so we
have only 2 more free slots after ABS_MT_TOOL_Y.
We will figure something out for 3.13.

I will also prepare a patch for ABS_MISC0-MISC5.

Regards
David

Re: [GIT] HID for 3.12 merge window

[Linus Torvalds]

On Sat, Sep 7, 2013 at 12:31 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>
> The bug only occurs for multi-touch devices (their ABS_* bits are
>>0x1f), that's why I didn't see it happening during my tests..

It definitely affects keyboards too. That's how I noticed it. That is
with the logitech "universal" receiver, though, so maybe that counts
as a potential multi-touch device..

    Linus

Re: [GIT] HID for 3.12 merge window

[Dmitry Torokhov]

David Herrmann <dh.herrmann@gmail.com> wrote:
>Hi
>
>On Sat, Sep 7, 2013 at 11:20 AM, Benjamin Tissoires
><benjamin.tissoires@gmail.com> wrote:
>>
>>
>> On 07/09/13 10:57, David Herrmann wrote:
>>> Hi
>>>
>>> On Sat, Sep 7, 2013 at 10:24 AM, Benjamin Tissoires
>>>> I'm not particularly in favor of adding semantic between ABS_MISC
>and
>>>> ABS_MT_SLOT. We already did that once with the ABS_MT_* stuff, and
>the
>>>> problem comes from devices showing several axes, not really mapped
>>>> (like joysticks). These axes are all mapped to ABS_MISC, but the
>input
>>>> core function map them to ABS_MISC+N. This way some joysticks show
>>>> multitouch axes and are treated as such, whereas the axis are just
>>>> regular absolute axes. (not sure I am clear enough... :( )
>>>>
>>>> Currently, nothing is done in X or in Wayland to detect these false
>>>> multitouch devices with a large number of absolute axes. But if we
>>>> keep a hole between ABS_MISC and ABS_MT_SLOT, we could add an
>>>> heuristic like:
>>>> "if all the axes between ABS_MISC and ABS_MT_SLOT are available,
>then
>>>> ABS_MT_SLOT does not mean multitouch, but ABS_MISC+6."
>>>
>>> So Xorg just took control over the unused ABS_* slots? Hmm, then I
>>> have no other idea than reverting the 3 patches.
>>> Sadly, that also means there's no way for us to easily extend the
>>> ABS_* bits. If someone has an idea, let me know. But for now I'll
>just
>>> put the drums/guitar patches on hold.
>>>
>>
>> Well, I would say the "problem" lies in hid-input.c, line 919:
>>>>>>>>>>>>>>>>>>>>
>>         while (usage->code <= max && test_and_set_bit(usage->code,
>bit))
>>                 usage->code = find_next_zero_bit(bit, max + 1,
>usage->code);
>>>>>>>>>>>>>>>>>>>>
>>
>> So if you try to map several times ABS_MISC or any other axis, then
>you are mapping ABS_MISC+N.
>>
>> Anyway, the only problematic axis is ABS_MISC, as it is a default
>fallback. IMO, we can extend the ABS_* definitions after ABS_MT_TOOL_Y,
>like the original patch you proposed without having to deal with the
>above problem (though this will report the patch series to v3.13).
>> To prevent future mistakes, we should just define ABS_MISC0 (0x29) to
>ABS_MISC5 (0x2e) to fill the hole.
>
>As mentioned earlier in this thread, ABS_* is limited to 0x3f, so we
>have only 2 more free slots after ABS_MT_TOOL_Y.
>We will figure something out for 3.13.
>
>I will also prepare a patch for ABS_MISC0-MISC5.
>

Regardless of whether we settle on using lower bits or bite the bullet and extend number of axis I think it's time for EVIOCGABS2 that would take axis in its argument instead of encoding it in ioctl number - that idea wasn't that good.


Thanks.

-- 
Dmitry

Re: [GIT] HID for 3.12 merge window

[David Herrmann]

Hi Linus

On Sat, Sep 7, 2013 at 6:52 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Sep 7, 2013 at 12:31 AM, David Herrmann <dh.herrmann@gmail.com> wrote:
>>
>> The bug only occurs for multi-touch devices (their ABS_* bits are
>>>0x1f), that's why I didn't see it happening during my tests..
>
> It definitely affects keyboards too. That's how I noticed it. That is
> with the logitech "universal" receiver, though, so maybe that counts
> as a potential multi-touch device..

I have been quite wrong with that statement, yepp. xorg-evdev fetches
the whole device-information. So if a keyboard provides an affected
ABS_* bit, it'll break, too. My ancient hardware didn't use any of the
higher ABS bits, though.
I'm sorry for the troubles caused. ABS_MAX must be at most 0x3f. So
please, go ahead and revert the 3 mentioned patches.

Thanks
David