[PATCH v2 1/3] HID: asus: fix and generalize ambiguous preprocessor macros

[PATCH v2 1/3] HID: asus: fix and generalize ambiguous preprocessor macros

[Matjaz Hegedic]

Before commits a1cbda7a65a7a ("HID: asus: drop dependency
on I2C_HID") and 64a403c6555fd ("HID: asus: support Republic
Of Gamers special keys") hid-asus only pertained to a single
I2C keyboard model found in ASUS X205TA, F205TA, & X200HA. The
aforementioned commits expanded this support to other ASUS
laptop keyboard models.

In order to clarify that existing keyboard and touchpad quirks
only apply to the I2C devices, and not ASUS keyboards in
general, I2C HID IDs and their corresponding quirk sets have
been renamed. In addition, the latter commit introduced
special key handling, which also applies to the I2C keyboard,
not just Republic of Gamers series. Therefore, the
rog_map_key_clear() macro is renamed to asus_map_key_clear()
for the sake of generality.

Signed-off-by: Matjaz Hegedic <matjaz.hegedic@gmail.com>
---
 drivers/hid/hid-asus.c | 38 +++++++++++++++++++-------------------
 drivers/hid/hid-core.c |  4 ++--
 drivers/hid/hid-ids.h  |  4 ++--
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 5bae6ff..20fe97f 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -64,9 +64,9 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_SKIP_INPUT_MAPPING	BIT(2)
 #define QUIRK_IS_MULTITOUCH		BIT(3)
 
-#define KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
+#define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
 						 QUIRK_NO_INIT_REPORTS)
-#define TOUCHPAD_QUIRKS			(QUIRK_NO_INIT_REPORTS | \
+#define I2C_TOUCHPAD_QUIRKS			(QUIRK_NO_INIT_REPORTS | \
 						 QUIRK_SKIP_INPUT_MAPPING | \
 						 QUIRK_IS_MULTITOUCH)
 
@@ -199,7 +199,7 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
 	return 0;
 }
 
-#define rog_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, \
+#define asus_map_key_clear(c)	hid_map_usage_clear(hi, usage, bit, \
 						    max, EV_KEY, (c))
 static int asus_input_mapping(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
@@ -215,31 +215,31 @@ static int asus_input_mapping(struct hid_device *hdev,
 		return -1;
 	}
 
-	/* ASUS Republic of Gamers laptop keyboard hotkeys */
+	/* ASUS-specific keyboard hotkeys */
 	if ((usage->hid & HID_USAGE_PAGE) == 0xff310000) {
 		set_bit(EV_REP, hi->input->evbit);
 		switch (usage->hid & HID_USAGE) {
-		case 0x10: rog_map_key_clear(KEY_BRIGHTNESSDOWN);	break;
-		case 0x20: rog_map_key_clear(KEY_BRIGHTNESSUP);		break;
-		case 0x35: rog_map_key_clear(KEY_DISPLAY_OFF);		break;
-		case 0x6c: rog_map_key_clear(KEY_SLEEP);		break;
-		case 0x82: rog_map_key_clear(KEY_CAMERA);		break;
-		case 0x88: rog_map_key_clear(KEY_WLAN);			break;
-		case 0xb5: rog_map_key_clear(KEY_CALC);			break;
-		case 0xc4: rog_map_key_clear(KEY_KBDILLUMUP);		break;
-		case 0xc5: rog_map_key_clear(KEY_KBDILLUMDOWN);		break;
+		case 0x10: asus_map_key_clear(KEY_BRIGHTNESSDOWN);	break;
+		case 0x20: asus_map_key_clear(KEY_BRIGHTNESSUP);		break;
+		case 0x35: asus_map_key_clear(KEY_DISPLAY_OFF);		break;
+		case 0x6c: asus_map_key_clear(KEY_SLEEP);		break;
+		case 0x82: asus_map_key_clear(KEY_CAMERA);		break;
+		case 0x88: asus_map_key_clear(KEY_WLAN);			break;
+		case 0xb5: asus_map_key_clear(KEY_CALC);			break;
+		case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP);		break;
+		case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN);		break;
 
 		/* ASUS touchpad toggle */
-		case 0x6b: rog_map_key_clear(KEY_F21);			break;
+		case 0x6b: asus_map_key_clear(KEY_F21);			break;
 
 		/* ROG key */
-		case 0x38: rog_map_key_clear(KEY_PROG1);		break;
+		case 0x38: asus_map_key_clear(KEY_PROG1);		break;
 
 		/* Fn+C ASUS Splendid */
-		case 0xba: rog_map_key_clear(KEY_PROG2);		break;
+		case 0xba: asus_map_key_clear(KEY_PROG2);		break;
 
 		/* Fn+Space Power4Gear Hybrid */
-		case 0x5c: rog_map_key_clear(KEY_PROG3);		break;
+		case 0x5c: asus_map_key_clear(KEY_PROG3);		break;
 
 		default:
 			return 0;
@@ -354,9 +354,9 @@ static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 
 static const struct hid_device_id asus_devices[] = {
 	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
-		 USB_DEVICE_ID_ASUSTEK_NOTEBOOK_KEYBOARD), KEYBOARD_QUIRKS},
+		USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD), I2C_KEYBOARD_QUIRKS},
 	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK,
-			 USB_DEVICE_ID_ASUSTEK_TOUCHPAD), TOUCHPAD_QUIRKS },
+		USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD), I2C_TOUCHPAD_QUIRKS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
 		USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index bbc3c3b..6298675 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1852,8 +1852,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGIC_KEYBOARD_ANSI) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_FOUNTAIN_TP_ONLY) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_GEYSER1_TP_ONLY) },
-	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_NOTEBOOK_KEYBOARD) },
-	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_TOUCHPAD) },
+	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD) },
+	{ HID_I2C_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_AUREAL, USB_DEVICE_ID_AUREAL_W01RN) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ea5b968..e267a95 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -173,8 +173,8 @@
 #define USB_VENDOR_ID_ASUSTEK		0x0b05
 #define USB_DEVICE_ID_ASUSTEK_LCM	0x1726
 #define USB_DEVICE_ID_ASUSTEK_LCM2	0x175b
-#define USB_DEVICE_ID_ASUSTEK_NOTEBOOK_KEYBOARD	0x8585
-#define USB_DEVICE_ID_ASUSTEK_TOUCHPAD	0x0101
+#define USB_DEVICE_ID_ASUSTEK_I2C_KEYBOARD	0x8585
+#define USB_DEVICE_ID_ASUSTEK_I2C_TOUCHPAD	0x0101
 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD1 0x1854
 #define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD2 0x1837
 
-- 
2.7.4

[PATCH v2 2/3] HID: asus: ignore declared dummy usages

[Matjaz Hegedic]

Keyboards handled by hid-asus declare special key functions
using a vendor-specific page, however, alongside legitimate
key functions, dummy usages with seemingly arbitrary values
are also declared and can lead to keyboards being detected
as pointer devices by some software (such as X.org).

In addition, for the I2C keyboard volume controls are
separately declared in a Consumer Usage page, with the same
dummy usage problem.

The fix in 1989dada7ce0 ("HID: input: ignore System Control
application usages if not System Controls") does not mitigate
the problem described above, therefore dummy usages need to
be ignored in the driver itself.

This fix properly ignores dummy usages and introduces a quirk
for custom handling of the Consumer Usages on the I2C keyboard.

Signed-off-by: Matjaz Hegedic <matjaz.hegedic@gmail.com>
---
 drivers/hid/hid-asus.c | 23 +++++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 20fe97f..e0de54b 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -63,9 +63,11 @@ MODULE_DESCRIPTION("Asus HID Keyboard and TouchPad");
 #define QUIRK_NO_INIT_REPORTS		BIT(1)
 #define QUIRK_SKIP_INPUT_MAPPING	BIT(2)
 #define QUIRK_IS_MULTITOUCH		BIT(3)
+#define QUIRK_NO_CONSUMER_USAGES	BIT(4)
 
 #define I2C_KEYBOARD_QUIRKS			(QUIRK_FIX_NOTEBOOK_REPORT | \
-						 QUIRK_NO_INIT_REPORTS)
+						 QUIRK_NO_INIT_REPORTS | \
+						 QUIRK_NO_CONSUMER_USAGES)
 #define I2C_TOUCHPAD_QUIRKS			(QUIRK_NO_INIT_REPORTS | \
 						 QUIRK_SKIP_INPUT_MAPPING | \
 						 QUIRK_IS_MULTITOUCH)
@@ -242,11 +244,28 @@ static int asus_input_mapping(struct hid_device *hdev,
 		case 0x5c: asus_map_key_clear(KEY_PROG3);		break;
 
 		default:
-			return 0;
+			/* ASUS lazily declares 256 usages, ignore the rest,
+			 * as some make the keyboard appear as a pointer device. */
+			return -1;
 		}
 		return 1;
 	}
 
+	if (drvdata->quirks & QUIRK_NO_CONSUMER_USAGES &&
+		(usage->hid & HID_USAGE_PAGE) == HID_UP_CONSUMER) {
+		switch (usage->hid & HID_USAGE) {
+		case 0xe2: /* Mute */
+		case 0xe9: /* Volume up */
+		case 0xea: /* Volume down */
+			return 0;
+		default:
+			/* Ignore dummy Consumer usages which make the
+			 * keyboard incorrectly appear as a pointer device.
+			 */
+			return -1;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4

[PATCH v2 3/3] HID: asus: change mapping from KEY_WLAN to KEY_RFKILL

[Matjaz Hegedic]

The input mapping code incorrectly maps the Airplane Mode button to
KEY_WLAN, which stands for WiFi toggle, but doesn't affect Bluetooth
(and other active radios) which is expected behavior for Airplane
Mode.

The fix replaces KEY_WLAN with the more appropriate KEY_RFKILL.

The declared usage code 0x88 corresponds to Airplane Mode button on
all keyboards handled by hid-asus (I2C netbook keyboards and USB
RoG series keyboards), so the fix doesn't introduce any
inconsistencies across different models.

Signed-off-by: Matjaz Hegedic <matjaz.hegedic@gmail.com>
---
 drivers/hid/hid-asus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index e0de54b..bacba97 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -226,7 +226,7 @@ static int asus_input_mapping(struct hid_device *hdev,
 		case 0x35: asus_map_key_clear(KEY_DISPLAY_OFF);		break;
 		case 0x6c: asus_map_key_clear(KEY_SLEEP);		break;
 		case 0x82: asus_map_key_clear(KEY_CAMERA);		break;
-		case 0x88: asus_map_key_clear(KEY_WLAN);			break;
+		case 0x88: asus_map_key_clear(KEY_RFKILL);			break;
 		case 0xb5: asus_map_key_clear(KEY_CALC);			break;
 		case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP);		break;
 		case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN);		break;
-- 
2.7.4

Re: [PATCH v2 3/3] HID: asus: change mapping from KEY_WLAN to KEY_RFKILL

[Benjamin Tissoires]

On Mar 09 2017 or thereabouts, Matjaz Hegedic wrote:
> The input mapping code incorrectly maps the Airplane Mode button to
> KEY_WLAN, which stands for WiFi toggle, but doesn't affect Bluetooth
> (and other active radios) which is expected behavior for Airplane
> Mode.
> 
> The fix replaces KEY_WLAN with the more appropriate KEY_RFKILL.
> 
> The declared usage code 0x88 corresponds to Airplane Mode button on
> all keyboards handled by hid-asus (I2C netbook keyboards and USB
> RoG series keyboards), so the fix doesn't introduce any
> inconsistencies across different models.
> 
> Signed-off-by: Matjaz Hegedic <matjaz.hegedic@gmail.com>
> ---

The series is:
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

>  drivers/hid/hid-asus.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index e0de54b..bacba97 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -226,7 +226,7 @@ static int asus_input_mapping(struct hid_device *hdev,
>  		case 0x35: asus_map_key_clear(KEY_DISPLAY_OFF);		break;
>  		case 0x6c: asus_map_key_clear(KEY_SLEEP);		break;
>  		case 0x82: asus_map_key_clear(KEY_CAMERA);		break;
> -		case 0x88: asus_map_key_clear(KEY_WLAN);			break;
> +		case 0x88: asus_map_key_clear(KEY_RFKILL);			break;
>  		case 0xb5: asus_map_key_clear(KEY_CALC);			break;
>  		case 0xc4: asus_map_key_clear(KEY_KBDILLUMUP);		break;
>  		case 0xc5: asus_map_key_clear(KEY_KBDILLUMDOWN);		break;
> -- 
> 2.7.4
> 

Re: [PATCH v2 1/3] HID: asus: fix and generalize ambiguous preprocessor macros

[Jiri Kosina]

Hi,

I've applied the series to for-4.12/asus.

Thanks,

-- 
Jiri Kosina
SUSE Labs