From patchwork Mon Dec 29 14:21:26 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Herrmann X-Patchwork-Id: 5549481 X-Patchwork-Delegate: jikos@jikos.cz Return-Path: X-Original-To: patchwork-linux-input@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id AA3549F2B9 for ; Mon, 29 Dec 2014 14:22:09 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7738020120 for ; Mon, 29 Dec 2014 14:22:08 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 5127A20109 for ; Mon, 29 Dec 2014 14:22:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751570AbaL2OWG (ORCPT ); Mon, 29 Dec 2014 09:22:06 -0500 Received: from mail-wi0-f170.google.com ([209.85.212.170]:59804 "EHLO mail-wi0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751206AbaL2OWF (ORCPT ); Mon, 29 Dec 2014 09:22:05 -0500 Received: by mail-wi0-f170.google.com with SMTP id bs8so23808380wib.5; Mon, 29 Dec 2014 06:22:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=E4fJGyKnqtGtqZkyRm4N8T8Lwm1bNgYv+w6dN9fE+ps=; b=NWfUsSAOYB2aGAaHHCyDUeDUsdQc8v6O8d/obuOvnnOzqsYJwvyhZhPv5eKfphGG2M S1zF4T/joh35NXUaUvkyVoIasyiuaq5q32v+dSBsxExWhOkEo1+8gPD8vTQ6ctMjaQOb O666pQ6jMPHcD/dOpCFg7d1i7aAlIQ4zHdiU060z+ob+oeJwcoVOe99k8UhK/QGyisMw E3a2kaCu0tjuSTwurlV5SrylufkcCvheIzwxZc57Q+T7p7FHoTx5mJpAFMEK8CWb+fBx Itg9WrM34fGcCdf9Tr3wx1XFyscUfpaOVqceO1bsweGkwN681OwqPAw4eJ9cZ1rIap8r oDJQ== X-Received: by 10.180.108.143 with SMTP id hk15mr98400139wib.6.1419862922716; Mon, 29 Dec 2014 06:22:02 -0800 (PST) Received: from david-t2.localdomain (stgt-5f702edf.pool.mediaWays.net. [95.112.46.223]) by mx.google.com with ESMTPSA id ej10sm43033193wib.1.2014.12.29.06.22.00 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 29 Dec 2014 06:22:01 -0800 (PST) From: David Herrmann To: linux-input@vger.kernel.org Cc: David Herrmann , Adam Goode , Fredrik Hallenberg , Benjamin Tissoires , Jiri Kosina , Dmitry Torokhov , Subject: [PATCH] HID: input: fix confusion on conflicting mappings Date: Mon, 29 Dec 2014 15:21:26 +0100 Message-Id: <1419862886-1319-1-git-send-email-dh.herrmann@gmail.com> X-Mailer: git-send-email 2.2.1 Sender: linux-input-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-input@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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: Reported-by: Adam Goode Reported-by: Fredrik Hallenberg Signed-off-by: David Herrmann --- 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);