Re: [PATCH] Handle spurious backslash key repeats on some keyboards

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

On Sun, Sep 14, 2014 at 6:45 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote:
> Hi
>
> On Sun, Aug 10, 2014 at 11:56 AM, Fredrik Hallenberg
> <megahallon@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>>  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;
>> +       }
>> +
>
> Ok, I have looked through some more reports and at a bunch of AT
> keyboards and how the HID standard maps them. The backslash/pipe key
> is the only key affected by this. There _might_ be some similar
> overlap with Korean 106 and Japanese 109 layouts, but as there haven't
> been any reports, we probably don't care right now.
>
> And indeed, I'm kinda reluctant to add an hwdb entry for the reported
> keyboards as I couldn't find anyone with non-105 keyboards to compare
> VID/PID. Furthermore, similar issues will probably show up again in
> the future. However, I still dislike the quick hack in this patch. I
> especially dislike matching on KEY_BACKSLASH while we really want to
> match on 0x31 and 0x32. If users use setkeycode() to change a mapping,
> this should not trigger our quirk. We could easily change the patch to
> match on usage, but I think there's a better way:
>
> hid-core.c: hid_input_field()
> This is the main entry point for data that is matched on HID fields.
> We already have a RollOver quirk in there, which is extremely bad
> documented and I have no idea what it does.. However, this function
> saves the reported values so we can retrieve them on a following
> report. Why don't we skip events that have the same value reported as
> before? Obviously, we should do this only for absolute data, not
> relative. So I think something like this should work (sorry for
> line-wraps):
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 12b6e67..000a768 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1220,7 +1220,10 @@
>         for (n = 0; n < count; n++) {
>
>                 if (HID_MAIN_ITEM_VARIABLE & field->flags) {
> -                       hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);
> +                       /* ignore absolute values if they didn't change */
> +                       if (HID_MAIN_ITEM_RELATIVE & field->flags ||
> +                           value[n] != field->value[n])
> +                               hid_process_event(hid, field,
> &field->usage[n], value[n], interrupt);

This should probably be:
  (HID_MAIN_ITEM_RELATIVE | HID_MAIN_ITEM_BUFFERED_BYTE)
..though I wonder whether buffered-byte makes much sense without "relative"..
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux