diff mbox

Handle spurious backslash key repeats on some keyboards

Message ID 1407664566-5303-1-git-send-email-megahallon@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Fredrik Hallenberg Aug. 10, 2014, 9:56 a.m. UTC
Here is my attempt on a fix for bug 70181, please review it. It is
tested on my nordic Corsair K70, if this solution is deemed acceptable
I can ask some people with other problematic keyboards to test it.

Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
---
 drivers/hid/hid-input.c | 14 ++++++++++++++
 include/linux/hid.h     |  1 +
 2 files changed, 15 insertions(+)

Comments

Fredrik Hallenberg Sept. 9, 2014, 9:40 a.m. UTC | #1
Hi, time for a bump on this one.

On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Here is my attempt on a fix for bug 70181, please review it. It is
> tested on my nordic Corsair K70, if this solution is deemed acceptable
> I can ask some people with other problematic keyboards to test it.
>
> Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
> ---
>  drivers/hid/hid-input.c | 14 ++++++++++++++
>  include/linux/hid.h     |  1 +
>  2 files changed, 15 insertions(+)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 2619f7f..56429c0 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>                 return;
>         }
>
> +       /*
> +        * Some keyboards will report both HID keys 0x31 (\ and |) and
> +        * 0x32 (Non-US # and ~) which are both mapped to
> +        * KEY_BACKSLASH. This will cause spurious events causing
> +        * repeated backslash key presses. Handle this by tracking the
> +        * active HID code and ignoring the other one.
> +        */
> +       if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
> +               if (value)
> +                       field->hidinput->backslash_usage = usage->hid;
> +               else if (field->hidinput->backslash_usage != usage->hid)
> +                       return;
> +       }
> +
>         /* report the usage code as scancode if the key status has changed */
>         if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
>                 input_event(input, EV_MSC, MSC_SCAN, usage->hid);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index f53c4a9..1c59f8f 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -448,6 +448,7 @@ struct hid_input {
>         struct list_head list;
>         struct hid_report *report;
>         struct input_dev *input;
> +       unsigned backslash_usage;
>  };
>
>  enum hid_type {
> --
> 2.1.0.rc1
>
--
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
Jiri Kosina Sept. 9, 2014, 9:42 a.m. UTC | #2
On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:

> Hi, time for a bump on this one.

Do you have any idea how common this pattern is? How many keyboards have 
you encountered doing this?

I hate to be polluting generic code with such strange quirks ... we've 
spent quite some time factoring those out to specific drivers.
So knowing how commong this is could be helpful in deciding how to handle 
this.

Thanks.
Fredrik Hallenberg Sept. 9, 2014, 10:37 a.m. UTC | #3
I have seen reports for Gigabyte Osmium, Corsair Vengeance K-series
and QPAD MK-series. There are probably more. The reason I chose to not
make it a quirk specific to some specific hardware is that it feels
more like a general problem caused by the fact the there are two HID
key codes mapped to a single Linux key code.

On Tue, Sep 9, 2014 at 11:42 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:
>
>> Hi, time for a bump on this one.
>
> Do you have any idea how common this pattern is? How many keyboards have
> you encountered doing this?
>
> I hate to be polluting generic code with such strange quirks ... we've
> spent quite some time factoring those out to specific drivers.
> So knowing how commong this is could be helpful in deciding how to handle
> this.
>
> Thanks.
>
> --
> Jiri Kosina
> SUSE Labs
--
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
Jiri Kosina Sept. 10, 2014, 8:26 a.m. UTC | #4
On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:

> Hi, time for a bump on this one.
> 
> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
> > Here is my attempt on a fix for bug 70181, please review it. It is
> > tested on my nordic Corsair K70, if this solution is deemed acceptable
> > I can ask some people with other problematic keyboards to test it.
> >
> > Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
> > ---
> >  drivers/hid/hid-input.c | 14 ++++++++++++++
> >  include/linux/hid.h     |  1 +
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> > index 2619f7f..56429c0 100644
> > --- a/drivers/hid/hid-input.c
> > +++ b/drivers/hid/hid-input.c
> > @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
> >                 return;
> >         }
> >
> > +       /*
> > +        * Some keyboards will report both HID keys 0x31 (\ and |) and
> > +        * 0x32 (Non-US # and ~) which are both mapped to
> > +        * KEY_BACKSLASH. This will cause spurious events causing

Hmm, makes me wonder if this mapping in hid_keyboard[] is even correct. 
The footnote in HUT 1.12 specifies very different mappings for different 
national keyboards.
David Herrmann Sept. 10, 2014, 10:09 a.m. UTC | #5
Hi

(CC Dmitry for AT details)

On Wed, Sep 10, 2014 at 10:26 AM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:
>
>> Hi, time for a bump on this one.
>>
>> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
>> <megahallon@gmail.com> wrote:
>> > Here is my attempt on a fix for bug 70181, please review it. It is
>> > tested on my nordic Corsair K70, if this solution is deemed acceptable
>> > I can ask some people with other problematic keyboards to test it.
>> >
>> > Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
>> > ---
>> >  drivers/hid/hid-input.c | 14 ++++++++++++++
>> >  include/linux/hid.h     |  1 +
>> >  2 files changed, 15 insertions(+)
>> >
>> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>> > index 2619f7f..56429c0 100644
>> > --- a/drivers/hid/hid-input.c
>> > +++ b/drivers/hid/hid-input.c
>> > @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>> >                 return;
>> >         }
>> >
>> > +       /*
>> > +        * Some keyboards will report both HID keys 0x31 (\ and |) and
>> > +        * 0x32 (Non-US # and ~) which are both mapped to
>> > +        * KEY_BACKSLASH. This will cause spurious events causing
>
> Hmm, makes me wonder if this mapping in hid_keyboard[] is even correct.
> The footnote in HUT 1.12 specifies very different mappings for different
> national keyboards.

This is the key that's moved on European layouts due to the different
shape of the 'Enter' key. HID defines two different usage types, key
0x31 and 0x32. It doesn't make sense for a keyboard to enable both, so
this really sounds like a broken hid description to me.

We currently map both those keys to KEY_BACKSLASH, which means
regardless whether it's the European or standard layout, the same
keycodes are generated. So mapping 0x31 and 0x32 to the same key
sounds fine (or at least consistent).

@Fredrik: can you send us your HID descriptor from the broken deivce?
(cat /sys/kernel/debug/hid/<device>/desc).

I wonder why you get key-repeat events, though. I mean, the keyboard
has to send both 0x31 and 0x32 events for you to get those repeat
events. But it really doesn't make sense to me for a keyboard to
report both keys, and furthermore your report sounded like you only
pressed a single key, right?

Thanks
David
--
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
Fredrik Hallenberg Sept. 10, 2014, 11:13 a.m. UTC | #6
The keyboards in question have n-key rollover, this seems to be
implemented by sending every single key id every time the keyboard is
polled, this includes several keys that does not exist physically for
example 0x31. This may be a bit excessive but not broken. If I press
a-key and '-key in rapid succession, I get the following events
(omitting all other keys):

VALUE 1 CODE 30 HID 0x4
VALUE 0 CODE 43 HID 0x31
VALUE 0 CODE 43 HID 0x32
Output: a

VALUE 1 CODE 30 HID 0x4
VALUE 0 CODE 43 HID 0x31  - 43 is up
VALUE 1 CODE 43 HID 0x32  - 43 is down
Output: '

VALUE 0 CODE 30 HID 0x4
VALUE 0 CODE 43 HID 0x31  - 43 is up
VALUE 1 CODE 43 HID 0x32  - 43 is down
Output: '

VALUE 0 CODE 30 HID 0x4
VALUE 0 CODE 43 HID 0x31  - 43 is up
VALUE 1 CODE 43 HID 0x32  - 43 is down
Output: '

So even though the keyboard is behaving a bit weird the problem is in
the kernel as it will interpret 0x31 and 0x32 as the same key.

I don't have access to the keyboard at the moment, I will send the
descriptor later today.


On Wed, Sep 10, 2014 at 12:09 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> (CC Dmitry for AT details)
>
> On Wed, Sep 10, 2014 at 10:26 AM, Jiri Kosina <jkosina@suse.cz> wrote:
>> On Tue, 9 Sep 2014, Fredrik Hallenberg wrote:
>>
>>> Hi, time for a bump on this one.
>>>
>>> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
>>> <megahallon@gmail.com> wrote:
>>> > Here is my attempt on a fix for bug 70181, please review it. It is
>>> > tested on my nordic Corsair K70, if this solution is deemed acceptable
>>> > I can ask some people with other problematic keyboards to test it.
>>> >
>>> > Signed-off-by: Fredrik Hallenberg <megahallon@gmail.com>
>>> > ---
>>> >  drivers/hid/hid-input.c | 14 ++++++++++++++
>>> >  include/linux/hid.h     |  1 +
>>> >  2 files changed, 15 insertions(+)
>>> >
>>> > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
>>> > index 2619f7f..56429c0 100644
>>> > --- a/drivers/hid/hid-input.c
>>> > +++ b/drivers/hid/hid-input.c
>>> > @@ -1085,6 +1085,20 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
>>> >                 return;
>>> >         }
>>> >
>>> > +       /*
>>> > +        * Some keyboards will report both HID keys 0x31 (\ and |) and
>>> > +        * 0x32 (Non-US # and ~) which are both mapped to
>>> > +        * KEY_BACKSLASH. This will cause spurious events causing
>>
>> Hmm, makes me wonder if this mapping in hid_keyboard[] is even correct.
>> The footnote in HUT 1.12 specifies very different mappings for different
>> national keyboards.
>
> This is the key that's moved on European layouts due to the different
> shape of the 'Enter' key. HID defines two different usage types, key
> 0x31 and 0x32. It doesn't make sense for a keyboard to enable both, so
> this really sounds like a broken hid description to me.
>
> We currently map both those keys to KEY_BACKSLASH, which means
> regardless whether it's the European or standard layout, the same
> keycodes are generated. So mapping 0x31 and 0x32 to the same key
> sounds fine (or at least consistent).
>
> @Fredrik: can you send us your HID descriptor from the broken deivce?
> (cat /sys/kernel/debug/hid/<device>/desc).
>
> I wonder why you get key-repeat events, though. I mean, the keyboard
> has to send both 0x31 and 0x32 events for you to get those repeat
> events. But it really doesn't make sense to me for a keyboard to
> report both keys, and furthermore your report sounded like you only
> pressed a single key, right?
>
> Thanks
> David
--
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
David Herrmann Sept. 10, 2014, 2:22 p.m. UTC | #7
Hi

On Wed, Sep 10, 2014 at 1:13 PM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> The keyboards in question have n-key rollover, this seems to be
> implemented by sending every single key id every time the keyboard is
> polled, this includes several keys that does not exist physically for
> example 0x31. This may be a bit excessive but not broken. If I press
> a-key and '-key in rapid succession, I get the following events
> (omitting all other keys):
>
> VALUE 1 CODE 30 HID 0x4
> VALUE 0 CODE 43 HID 0x31
> VALUE 0 CODE 43 HID 0x32
> Output: a
>
> VALUE 1 CODE 30 HID 0x4
> VALUE 0 CODE 43 HID 0x31  - 43 is up
> VALUE 1 CODE 43 HID 0x32  - 43 is down
> Output: '
>
> VALUE 0 CODE 30 HID 0x4
> VALUE 0 CODE 43 HID 0x31  - 43 is up
> VALUE 1 CODE 43 HID 0x32  - 43 is down
> Output: '
>
> VALUE 0 CODE 30 HID 0x4
> VALUE 0 CODE 43 HID 0x31  - 43 is up
> VALUE 1 CODE 43 HID 0x32  - 43 is down
> Output: '
>
> So even though the keyboard is behaving a bit weird the problem is in
> the kernel as it will interpret 0x31 and 0x32 as the same key.

Thanks for the information! Please include that in follow-up
commit-messages so others can see it as well. I don't think a report
descriptor is needed, anymore.

This indeed explains the problem. We only track keys on the keycode
level, not scancode level. Therefore, you get weird key-up or
key-repeat events depending on the scan-order.

The nicest fix, obviously, is to blacklist keys that are not
physically present on the keyboard. But I assume the keyboard reports
either key depending on the model (standard vs. European). Given that
this key is indeed special as we only have a single keycode for it, we
probably need some quirk like yours. I'd like to hear Dmitry's
comments on this, maybe he has had similar problems with non-HID
keyboards.

Thanks
David
--
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
Dmitry Torokhov Sept. 10, 2014, 10:51 p.m. UTC | #8
On Wed, Sep 10, 2014 at 04:22:13PM +0200, David Herrmann wrote:
> Hi
> 
> On Wed, Sep 10, 2014 at 1:13 PM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
> > The keyboards in question have n-key rollover, this seems to be
> > implemented by sending every single key id every time the keyboard is
> > polled, this includes several keys that does not exist physically for
> > example 0x31. This may be a bit excessive but not broken. If I press
> > a-key and '-key in rapid succession, I get the following events
> > (omitting all other keys):
> >
> > VALUE 1 CODE 30 HID 0x4
> > VALUE 0 CODE 43 HID 0x31
> > VALUE 0 CODE 43 HID 0x32
> > Output: a
> >
> > VALUE 1 CODE 30 HID 0x4
> > VALUE 0 CODE 43 HID 0x31  - 43 is up
> > VALUE 1 CODE 43 HID 0x32  - 43 is down
> > Output: '
> >
> > VALUE 0 CODE 30 HID 0x4
> > VALUE 0 CODE 43 HID 0x31  - 43 is up
> > VALUE 1 CODE 43 HID 0x32  - 43 is down
> > Output: '
> >
> > VALUE 0 CODE 30 HID 0x4
> > VALUE 0 CODE 43 HID 0x31  - 43 is up
> > VALUE 1 CODE 43 HID 0x32  - 43 is down
> > Output: '
> >
> > So even though the keyboard is behaving a bit weird the problem is in
> > the kernel as it will interpret 0x31 and 0x32 as the same key.
> 
> Thanks for the information! Please include that in follow-up
> commit-messages so others can see it as well. I don't think a report
> descriptor is needed, anymore.
> 
> This indeed explains the problem. We only track keys on the keycode
> level, not scancode level. Therefore, you get weird key-up or
> key-repeat events depending on the scan-order.
> 
> The nicest fix, obviously, is to blacklist keys that are not
> physically present on the keyboard. But I assume the keyboard reports

Hmm, somebody could still load keymap with duplicate keycodes though...

>
> either key depending on the model (standard vs. European). Given that
> this key is indeed special as we only have a single keycode for it, we
> probably need some quirk like yours. I'd like to hear Dmitry's
> comments on this, maybe he has had similar problems with non-HID
> keyboards.

Hmm, I do not think we have a good story for duplicate keycodes for
drivers that send entire state as opposed to just changed bits.

Thanks.
Fredrik Hallenberg Sept. 11, 2014, 8:50 a.m. UTC | #9
>> The nicest fix, obviously, is to blacklist keys that are not
>> physically present on the keyboard. But I assume the keyboard reports
>
> Hmm, somebody could still load keymap with duplicate keycodes though...
>

The HID mapping is hardcoded, so only key code 43 will have this
problem regardless of user keymaps.

Regarding blacklisting it would be good but I don't think there is any
way to know in advance which key (0x31 or 0x32) the keyboard actually
uses. If anybody has ideas on how this could be done let me know.

Jiri, do you still think this should be a hardware specific quirk?
--
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
David Herrmann Sept. 11, 2014, 12:41 p.m. UTC | #10
Hi

On Thu, Sep 11, 2014 at 10:50 AM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
>>> The nicest fix, obviously, is to blacklist keys that are not
>>> physically present on the keyboard. But I assume the keyboard reports
>>
>> Hmm, somebody could still load keymap with duplicate keycodes though...
>>
>
> The HID mapping is hardcoded, so only key code 43 will have this
> problem regardless of user keymaps.

I think Dmitry is talking about setkeycode() which can change the
keycode a scancode is mapped to. This is indeed a problem, but I
thought we implicitly tell people to not map two scancodes to the same
keycode.. we simply don't support it (as we only have 1bit state per
keycode, and 0bit per scancode).

But this reminds me: we can use udev hwdb and setkeycode() to fix your
keyboard. Simply add this to /lib/udev/hwdb.d/60-keyboard.hwdb:
    keyboard:name:*Corsair*:dmi:bvn*:bvr*:bd*:svn*:pn*:pvr*
        KEYBOARD_KEY_32=0

this will run setkeycode(0x32, KEY_RESERVED) and thus disable the key
0x32 entirely.
Adding properly crafted match-rules to udev should solve this mess, I
guess. Imho, this is the cleanest solution, but depends on udev, of
course.

Thanks
David
--
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
Fredrik Hallenberg Sept. 11, 2014, 8:35 p.m. UTC | #11
Thanks I had no idea about the hwdb but I have tried it now, it works if I use

KEYBOARD_KEY_70031=reserved

on my nordic keyboard, but a US keyboard user would have to use
scancode 70032 instead. If you chose the wrong one the backslash key
will be disabled.

I don't think this is good solution as long as there is no way to
detect which version is used with the hardware matcher string.

However I suppose udev should make it possible to do some kind of
script that does something similar to my original patch, that is,
detects which of the two keys in actually used and disables the other
one.

On Thu, Sep 11, 2014 at 2:41 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Sep 11, 2014 at 10:50 AM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>>>> The nicest fix, obviously, is to blacklist keys that are not
>>>> physically present on the keyboard. But I assume the keyboard reports
>>>
>>> Hmm, somebody could still load keymap with duplicate keycodes though...
>>>
>>
>> The HID mapping is hardcoded, so only key code 43 will have this
>> problem regardless of user keymaps.
>
> I think Dmitry is talking about setkeycode() which can change the
> keycode a scancode is mapped to. This is indeed a problem, but I
> thought we implicitly tell people to not map two scancodes to the same
> keycode.. we simply don't support it (as we only have 1bit state per
> keycode, and 0bit per scancode).
>
> But this reminds me: we can use udev hwdb and setkeycode() to fix your
> keyboard. Simply add this to /lib/udev/hwdb.d/60-keyboard.hwdb:
>     keyboard:name:*Corsair*:dmi:bvn*:bvr*:bd*:svn*:pn*:pvr*
>         KEYBOARD_KEY_32=0
>
> this will run setkeycode(0x32, KEY_RESERVED) and thus disable the key
> 0x32 entirely.
> Adding properly crafted match-rules to udev should solve this mess, I
> guess. Imho, this is the cleanest solution, but depends on udev, of
> course.
>
> Thanks
> David
--
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
David Herrmann Sept. 12, 2014, 10:29 a.m. UTC | #12
Hi

On Thu, Sep 11, 2014 at 10:35 PM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Thanks I had no idea about the hwdb but I have tried it now, it works if I use
>
> KEYBOARD_KEY_70031=reserved

Thanks for confirming!

> on my nordic keyboard, but a US keyboard user would have to use
> scancode 70032 instead. If you chose the wrong one the backslash key
> will be disabled.
>
> I don't think this is good solution as long as there is no way to
> detect which version is used with the hardware matcher string.

Well, you can match on "country" tags. Currently, the device
identifiers contain vendor+product+version+country information. I
assume the "country" field is different between your keyboard and a
keyboard that swaps the key reports.

> However I suppose udev should make it possible to do some kind of
> script that does something similar to my original patch, that is,
> detects which of the two keys in actually used and disables the other
> one.

If you post your mod-alias line of your device (or better: all
information you can gather about the device), I can push it into hwdb.
It is up to Dmitry and Jiri to decide whether it makes sense to deal
with it in the kernel.

Thanks
David
--
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
Fredrik Hallenberg Sept. 12, 2014, 11:34 a.m. UTC | #13
Is the country tag derived from the bCountryCode seen in the HID
device descriptor seen when doing "lsusb -v"?
If so it probably wont work as it says "0 (Not supported)" on my
keyboard and a few others I have tested.

On Fri, Sep 12, 2014 at 12:29 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Thu, Sep 11, 2014 at 10:35 PM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Thanks I had no idea about the hwdb but I have tried it now, it works if I use
>>
>> KEYBOARD_KEY_70031=reserved
>
> Thanks for confirming!
>
>> on my nordic keyboard, but a US keyboard user would have to use
>> scancode 70032 instead. If you chose the wrong one the backslash key
>> will be disabled.
>>
>> I don't think this is good solution as long as there is no way to
>> detect which version is used with the hardware matcher string.
>
> Well, you can match on "country" tags. Currently, the device
> identifiers contain vendor+product+version+country information. I
> assume the "country" field is different between your keyboard and a
> keyboard that swaps the key reports.
>
>> However I suppose udev should make it possible to do some kind of
>> script that does something similar to my original patch, that is,
>> detects which of the two keys in actually used and disables the other
>> one.
>
> If you post your mod-alias line of your device (or better: all
> information you can gather about the device), I can push it into hwdb.
> It is up to Dmitry and Jiri to decide whether it makes sense to deal
> with it in the kernel.
>
> Thanks
> David
--
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
David Herrmann Sept. 12, 2014, 11:47 a.m. UTC | #14
Hi

On Fri, Sep 12, 2014 at 1:34 PM, Fredrik Hallenberg
<megahallon@gmail.com> wrote:
> Is the country tag derived from the bCountryCode seen in the HID
> device descriptor seen when doing "lsusb -v"?

Yes, it is.

> If so it probably wont work as it says "0 (Not supported)" on my
> keyboard and a few others I have tested.

"0" might also be used as default, so it's not surprising that your
US-style keyboard uses "0".

Furthermore, the non-US style keyboards might even use different
product IDs or version numbers. So it'd be nice if you could include a
full set of device information here.

Thanks
David
--
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
Fredrik Hallenberg Sept. 12, 2014, 5:36 p.m. UTC | #15
Here is the output from lsusb, let me know if you need more information.

Note this is the nordic version so if they are using the country code
at all it should have some other value than 0. As I mentioned I
checked a few other nordic/swedish keyboards and all of them use code
0.

Anyway as you say there could be some other value that could be used,
I will try to find someone with a US-version of this keyboard and
compare the values.

On Fri, Sep 12, 2014 at 1:47 PM, David Herrmann <dh.herrmann@gmail.com> wrote:
> Hi
>
> On Fri, Sep 12, 2014 at 1:34 PM, Fredrik Hallenberg
> <megahallon@gmail.com> wrote:
>> Is the country tag derived from the bCountryCode seen in the HID
>> device descriptor seen when doing "lsusb -v"?
>
> Yes, it is.
>
>> If so it probably wont work as it says "0 (Not supported)" on my
>> keyboard and a few others I have tested.
>
> "0" might also be used as default, so it's not surprising that your
> US-style keyboard uses "0".
>
> Furthermore, the non-US style keyboards might even use different
> product IDs or version numbers. So it'd be nice if you could include a
> full set of device information here.
>
> Thanks
> David
Bus 001 Device 003: ID 1b1c:1b09 Corsair 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x1b1c Corsair
  idProduct          0x1b09 
  bcdDevice            1.09
  iManufacturer           1 Corsair
  iProduct                2 Corsair K70R Gaming Keyboard
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           84
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      1 Keyboard
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      65
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      25
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0004  1x 4 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      37
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x000f  1x 15 bytes
        bInterval               1
Device Status:     0x0000
  (Bus Powered)
r h Dec. 19, 2014, 4:28 p.m. UTC | #16
Hello,

I experience the same issue with my new keyboard:
*Corsair Raptor K30 (German version)*

The issue can be fixed/worked around with this rule in hwdb:

---------------
#keyboard:name:*Corsair*:dmi:bvn*:bvr*:bd*:svn*:pn*:pvr*
keyboard:usb:v1B1Cp1B0A*
 KEYBOARD_KEY_70031=reserved
---------------

So you think the US version won't work with this anymore? I guess they 
will send both scancodes, too.

This is the output of lsusb:
---------------
Bus 004 Device 008: ID 1b1c:1b0a Corsair 
Device Descriptor:
  bLength                18
  bDescriptorType         1
  bcdUSB               2.00
  bDeviceClass            0 (Defined at Interface level)
  bDeviceSubClass         0 
  bDeviceProtocol         0 
  bMaxPacketSize0        64
  idVendor           0x1b1c Corsair
  idProduct          0x1b0a 
  bcdDevice            1.03
  iManufacturer           1 Corsair
  iProduct                2 Corsair K30A Gaming Keyboard
  iSerial                 0 
  bNumConfigurations      1
  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength           84
    bNumInterfaces          3
    bConfigurationValue     1
    iConfiguration          0 
    bmAttributes         0xa0
      (Bus Powered)
      Remote Wakeup
    MaxPower              500mA
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      1 Boot Interface Subclass
      bInterfaceProtocol      1 Keyboard
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      65
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x81  EP 1 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0008  1x 8 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        1
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      25
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x82  EP 2 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x0004  1x 4 bytes
        bInterval               1
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      bNumEndpoints           1
      bInterfaceClass         3 Human Interface Device
      bInterfaceSubClass      0 No Subclass
      bInterfaceProtocol      0 None
      iInterface              0 
        HID Device Descriptor:
          bLength                 9
          bDescriptorType        33
          bcdHID               1.11
          bCountryCode            0 Not supported
          bNumDescriptors         1
          bDescriptorType        34 Report
          wDescriptorLength      37
         Report Descriptors: 
           ** UNAVAILABLE **
      Endpoint Descriptor:
        bLength                 7
        bDescriptorType         5
        bEndpointAddress     0x83  EP 3 IN
        bmAttributes            3
          Transfer Type            Interrupt
          Synch Type               None
          Usage Type               Data
        wMaxPacketSize     0x000f  1x 15 bytes
        bInterval               1
Device Status:     0x0000
  (Bus Powered)
---------------

Did you already find someone with a US keyboard? Shall I write in the 
corsair forums or to the support to find out whether US keyboards have 
the same product IDs? If this is not the case, we could safely use vendor/
product IDs for the workaround.

I'd really like to submit working hwdb lines to the udev database to save 
other users from having these troubles.

Also, do you know why

Greetings,
RH

--
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
Fredrik Hallenberg Dec. 20, 2014, 7:56 p.m. UTC | #17
> Did you already find someone with a US keyboard?

Yes as mentioned in thread it does not seem possible to write a
hwdb-rule that works for both US and non-US keyboard. Also it is a
general problem that affects other keyboards than Corsair, again see
previous mails in this thread.

David, it would be great if you could get your patch ready so this one
could be closed. If you don't have the time to do it, perhaps my
original patch could be considered again. I fully aggree that it is a
hack but it does solve the problem with little risk of bad side
effects. It can always be removed later we you have a proper fix.

Regards,

Fredrik


On Fri, Dec 19, 2014 at 5:28 PM, RH <r@hirner.at> wrote:
> Hello,
>
> I experience the same issue with my new keyboard:
> *Corsair Raptor K30 (German version)*
>
> The issue can be fixed/worked around with this rule in hwdb:
>
> ---------------
> #keyboard:name:*Corsair*:dmi:bvn*:bvr*:bd*:svn*:pn*:pvr*
> keyboard:usb:v1B1Cp1B0A*
>  KEYBOARD_KEY_70031=reserved
> ---------------
>
> So you think the US version won't work with this anymore? I guess they
> will send both scancodes, too.
>
> This is the output of lsusb:
> ---------------
> Bus 004 Device 008: ID 1b1c:1b0a Corsair
> Device Descriptor:
>   bLength                18
>   bDescriptorType         1
>   bcdUSB               2.00
>   bDeviceClass            0 (Defined at Interface level)
>   bDeviceSubClass         0
>   bDeviceProtocol         0
>   bMaxPacketSize0        64
>   idVendor           0x1b1c Corsair
>   idProduct          0x1b0a
>   bcdDevice            1.03
>   iManufacturer           1 Corsair
>   iProduct                2 Corsair K30A Gaming Keyboard
>   iSerial                 0
>   bNumConfigurations      1
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength           84
>     bNumInterfaces          3
>     bConfigurationValue     1
>     iConfiguration          0
>     bmAttributes         0xa0
>       (Bus Powered)
>       Remote Wakeup
>     MaxPower              500mA
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       bNumEndpoints           1
>       bInterfaceClass         3 Human Interface Device
>       bInterfaceSubClass      1 Boot Interface Subclass
>       bInterfaceProtocol      1 Keyboard
>       iInterface              0
>         HID Device Descriptor:
>           bLength                 9
>           bDescriptorType        33
>           bcdHID               1.11
>           bCountryCode            0 Not supported
>           bNumDescriptors         1
>           bDescriptorType        34 Report
>           wDescriptorLength      65
>          Report Descriptors:
>            ** UNAVAILABLE **
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x81  EP 1 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0008  1x 8 bytes
>         bInterval               1
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        1
>       bAlternateSetting       0
>       bNumEndpoints           1
>       bInterfaceClass         3 Human Interface Device
>       bInterfaceSubClass      0 No Subclass
>       bInterfaceProtocol      0 None
>       iInterface              0
>         HID Device Descriptor:
>           bLength                 9
>           bDescriptorType        33
>           bcdHID               1.11
>           bCountryCode            0 Not supported
>           bNumDescriptors         1
>           bDescriptorType        34 Report
>           wDescriptorLength      25
>          Report Descriptors:
>            ** UNAVAILABLE **
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x82  EP 2 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x0004  1x 4 bytes
>         bInterval               1
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        2
>       bAlternateSetting       0
>       bNumEndpoints           1
>       bInterfaceClass         3 Human Interface Device
>       bInterfaceSubClass      0 No Subclass
>       bInterfaceProtocol      0 None
>       iInterface              0
>         HID Device Descriptor:
>           bLength                 9
>           bDescriptorType        33
>           bcdHID               1.11
>           bCountryCode            0 Not supported
>           bNumDescriptors         1
>           bDescriptorType        34 Report
>           wDescriptorLength      37
>          Report Descriptors:
>            ** UNAVAILABLE **
>       Endpoint Descriptor:
>         bLength                 7
>         bDescriptorType         5
>         bEndpointAddress     0x83  EP 3 IN
>         bmAttributes            3
>           Transfer Type            Interrupt
>           Synch Type               None
>           Usage Type               Data
>         wMaxPacketSize     0x000f  1x 15 bytes
>         bInterval               1
> Device Status:     0x0000
>   (Bus Powered)
> ---------------
>
> Did you already find someone with a US keyboard? Shall I write in the
> corsair forums or to the support to find out whether US keyboards have
> the same product IDs? If this is not the case, we could safely use vendor/
> product IDs for the workaround.
>
> I'd really like to submit working hwdb lines to the udev database to save
> other users from having these troubles.
>
> Also, do you know why
>
> Greetings,
> RH
>
> --
> 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
--
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
diff mbox

Patch

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 2619f7f..56429c0 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1085,6 +1085,20 @@  void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
 		return;
 	}
 
+	/*
+	 * Some keyboards will report both HID keys 0x31 (\ and |) and
+	 * 0x32 (Non-US # and ~) which are both mapped to
+	 * KEY_BACKSLASH. This will cause spurious events causing
+	 * repeated backslash key presses. Handle this by tracking the
+	 * active HID code and ignoring the other one.
+	 */
+	if (usage->type == EV_KEY && usage->code == KEY_BACKSLASH) {
+		if (value)
+			field->hidinput->backslash_usage = usage->hid;
+		else if (field->hidinput->backslash_usage != usage->hid)
+			return;
+	}
+
 	/* report the usage code as scancode if the key status has changed */
 	if (usage->type == EV_KEY && !!test_bit(usage->code, input->key) != value)
 		input_event(input, EV_MSC, MSC_SCAN, usage->hid);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index f53c4a9..1c59f8f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -448,6 +448,7 @@  struct hid_input {
 	struct list_head list;
 	struct hid_report *report;
 	struct input_dev *input;
+	unsigned backslash_usage;
 };
 
 enum hid_type {