Thanks for completing the patch, it works fine with my Corsair K70. On Mon, Dec 29, 2014 at 3:21 PM, David Herrmann <dh.herrmann@xxxxxxxxx> wrote: > On an PC-101/103/104 keyboard (American layout) the 'Enter' key and its > neighbours look like this: > > +---+ +---+ +-------+ > | 1 | | 2 | | 5 | > +---+ +---+ +-------+ > +---+ +-----------+ > | 3 | | 4 | > +---+ +-----------+ > > On a PC-102/105 keyboard (European layout) it looks like this: > > +---+ +---+ +-------+ > | 1 | | 2 | | | > +---+ +---+ +-+ 4 | > +---+ +---+ | | > | 3 | | 5 | | | > +---+ +---+ +-----+ > > (Note that the number of keys is the same, but key '5' is moved down and > the shape of key '4' is changed. Keys '1' to '3' are exactly the same.) > > The keys 1-4 report the same scan-code in HID in both layouts, even though > the keysym they produce is usually different depending on the XKB-keymap > used by user-space. > However, key '5' (US 'backslash'/'pipe') reports 0x31 for the upper layout > and 0x32 for the lower layout, as defined by the HID spec. This is highly > confusing as the linux-input API uses a single keycode for both. > > So far, this was never a problem as there never has been a keyboard with > both of those keys present at the same time. It would have to look > something like this: > > +---+ +---+ +-------+ > | 1 | | 2 | | x31 | > +---+ +---+ +-------+ > +---+ +---+ +-----+ > | 3 | |x32| | 4 | > +---+ +---+ +-----+ > > HID can represent such a keyboard, but the linux-input API cannot. > Furthermore, any user-space mapping would be confused by this and, > luckily, no-one ever produced such hardware. > > Now, the HID input layer fixed this mess by mapping both 0x31 and 0x32 to > the same keycode (KEY_BACKSLASH==0x2b). As only one of both physical keys > is present on a hardware, this works just fine. > > Lets introduce hardware-vendors into this: > ------------------------------------------ > > Unfortunately, it seems way to expensive to produce a different device for > American and European layouts. Therefore, hardware-vendors put both keys, > (0x31 and 0x32) on the same keyboard, but only one of them is hooked up > to the physical button, the other one is 'dead'. > This means, they can use the same hardware, with a different button-layout > and automatically produce the correct HID events for American *and* > European layouts. This is unproblematic for normal keyboards, as the > 'dead' key will never report any KEY-DOWN events. But RollOver keyboards > send the whole matrix on each key-event, allowing n-key roll-over mode. > This means, we get a 0x31 and 0x32 event on each key-press. One of them > will always be 0, the other reports the real state. As we map both to the > same keycode, we will get spurious key-events, even though the real > key-state never changed. > > The easiest way would be to blacklist 'dead' keys and never handle those. > We could simply read the 'country' tag of USB devices and blacklist either > key according to the layout. But... hardware vendors... want the same > device for all countries and thus many of them set 'country' to 0 for all > devices. Meh.. > > So we have to deal with this properly. As we cannot know which of the keys > is 'dead', we either need a heuristic and track those keys, or we simply > make use of our value-tracking for HID fields. We simply ignore HID events > for absolute data if the data didn't change. As HID tracks events on the > HID level, we haven't done the keycode translation, yet. Therefore, the > 'dead' key is tracked independently of the real key, therefore, any events > on it will be ignored. > > This patch simply discards any HID events for absolute data if it didn't > change compared to the last report. We need to ignore relative and > buffered-byte reports for obvious reasons. But those cannot be affected by > this bug, so we're fine. > > Preferably, we'd do this filtering on the HID-core level. But this might > break a lot of custom drivers, if they do not follow the HID specs. > Therefore, we do this late in hid-input just before we inject it into the > input layer (which does the exact same filtering, but on the keycode > level). > > If this turns out to break some devices, we might have to limit filtering > to EV_KEY events. But lets try to do the Right Thing first, and properly > filter any absolute data that didn't change. > > This patch is tagged for 'stable' as it fixes a lot of n-key RollOver > hardware. We might wanna wait with backporting for a while, before we know > it doesn't break anything else, though. > > Cc: <stable@xxxxxxxxxxxxxxx> > Reported-by: Adam Goode <adam@xxxxxxxxxxxxx> > Reported-by: Fredrik Hallenberg <megahallon@xxxxxxxxx> > Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxx> > --- > Hi > > Fredrik, Adam, can you give this a try? It should fix your issues. > > Thanks > David > > drivers/hid/hid-input.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 725f22c..8976895 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -1101,6 +1101,22 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct > return; > } > > + /* > + * Ignore reports for absolute data if the data didn't change. This is > + * not only an optimization but also fixes 'dead' key reports. Some > + * RollOver implementations for localized keys (like BACKSLASH/PIPE; HID > + * 0x31 and 0x32) report multiple keys, even though a localized keyboard > + * can only have one of them physically available. The 'dead' keys > + * report constant 0. As all map to the same keycode, they'd confuse > + * the input layer. If we filter the 'dead' keys on the HID level, we > + * skip the keycode translation and only forward real events. > + */ > + if (!(field->flags & (HID_MAIN_ITEM_RELATIVE | > + HID_MAIN_ITEM_BUFFERED_BYTE)) && > + usage->usage_index < field->maxusage && > + value == field->value[usage->usage_index]) > + 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); > -- > 2.2.1 > -- 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
