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
