On Wed, Jun 18, 2014 at 8:13 AM, Jiri Kosina [off-list ref] wrote:

On Mon, 19 May 2014, Benjamin Tissoires wrote:

quoted

Hi Jiri,

Please have a look at: https://bugzilla.kernel.org/show_bug.cgi?id=76461

To me, this device sends garbage and should not send the modifiers
both in a Var input and in an Array input.

I can fix it either by amending the report descriptor of the device
(one byte to change) or by changing hid-input.c for it to send the
events in the array processing no matter what was the previous state
(see below).

I don't know which one to pick because this seems to be a corner case,
so fixing hid-input might not be the place, but I am afraid of
removing part of the reports coming from the device (what if the
modifiers are actually used for something in the report).

Any thoughts on it?

For the record, fixing hid-input would just remove the test in hid_input_field:

                if (value[n] >= min && value[n] <= max
-                       && field->usage[value[n] - min].hid
-                       && search(field->value, value[n], count))
+                       && field->usage[value[n] - min].hid)
                                hid_process_event(hid, field,
&field->usage[value[n] - min], 1, interrupt);
        }

(Sorry, gmail, whitespace damaged and everything)

So if modifying the report descriptor is not the option (which is what I
understood from Bryan's mail), I have to say I don't really like this
change in hid_input_field(), as it's really rather "non obvious".

How about just creating a device specific input event quirk for this?

--
Jiri Kosina
SUSE Labs

A device-specific quirk would be fine.

But what's wrong with simply removing the "search" test?  It's a no-op
for non-quirky devices (which don't send redundant keycodes) and a bug
for quirky ones (which should take the union of redundant keycodes
instead of dropping them).

Removing the test produces simpler code and handles the quirky devices
correctly without the need to check for them explicitly.