HID: separate quirks for report descriptor fixup

Lately there have been quite a lot of bug reports against broken devices
which require us to fix their report descriptor in the runtime, before it
is passed to the HID parser. Those devices have eaten quite an amount of
our quirks space, which isn't particularly necessary - the quirks are not
needed after the report descriptor is parsed, and they just consume bits.

Therefore this patch separates the quirks for report descriptor fixup, and
moves their handling into separate code. The quirks are then forgotten as
soon as the report descriptor has been parsed.

Module parameter 'rdesc_quirks' is introduced to be able to modify these
quirks in runtime in a similar way to 'quirks' parameter for ordinary HID
quirks.

Signed-off-by: Jiri Kosina <jkosina@suse.cz>

drivers/hid/usbhid/hid-core.c

@@ -60,6 +60,12 @@ MODULE_PARM_DESC(quirks, "Add/modify USB HID quirks by specifying "
 		" quirks=vendorID:productID:quirks"
 		" where vendorID, productID, and quirks are all in"
 		" 0x-prefixed hex");
+static char *rdesc_quirks_param[MAX_USBHID_BOOT_QUIRKS] = { [ 0 ... (MAX_USBHID_BOOT_QUIRKS - 1) ] = NULL };
+module_param_array_named(rdesc_quirks, rdesc_quirks_param, charp, NULL, 0444);
+MODULE_PARM_DESC(rdesc_quirks, "Add/modify report descriptor quirks by specifying "
+		" rdesc_quirks=vendorID:productID:rdesc_quirks"
+		" where vendorID, productID, and rdesc_quirks are all in"
+		" 0x-prefixed hex");
 /*
  * Input submission and I/O error handler.
  */
@@ -632,20 +638,6 @@ static void hid_free_buffers(struct usb_device *dev, struct hid_device *hid)
 	usb_buffer_free(dev, usbhid->bufsize, usbhid->ctrlbuf, usbhid->ctrlbuf_dma);
 }
 
-/*
- * Cherry Cymotion keyboard have an invalid HID report descriptor,
- * that needs fixing before we can parse it.
- */
-
-static void hid_fixup_cymotion_descriptor(char *rdesc, int rsize)
-{
-	if (rsize >= 17 && rdesc[11] == 0x3c && rdesc[12] == 0x02) {
-		info("Fixing up Cherry Cymotion report descriptor");
-		rdesc[11] = rdesc[16] = 0xff;
-		rdesc[12] = rdesc[17] = 0x03;
-	}
-}
-
 /*
  * Sending HID_REQ_GET_REPORT changes the operation mode of the ps3 controller
  * to "operational".  Without this, the ps3 controller will not report any
@@ -672,61 +664,6 @@ static void hid_fixup_sony_ps3_controller(struct usb_device *dev, int ifnum)
 	kfree(buf);
 }
 
-/*
- * Certain Logitech keyboards send in report #3 keys which are far
- * above the logical maximum described in descriptor. This extends
- * the original value of 0x28c of logical maximum to 0x104d
- */
-static void hid_fixup_logitech_descriptor(unsigned char *rdesc, int rsize)
-{
-	if (rsize >= 90 && rdesc[83] == 0x26
-			&& rdesc[84] == 0x8c
-			&& rdesc[85] == 0x02) {
-		info("Fixing up Logitech keyboard report descriptor");
-		rdesc[84] = rdesc[89] = 0x4d;
-		rdesc[85] = rdesc[90] = 0x10;
-	}
-}
-
-/* Petalynx Maxter Remote has maximum for consumer page set too low */
-static void hid_fixup_petalynx_descriptor(unsigned char *rdesc, int rsize)
-{
-	if (rsize >= 60 && rdesc[39] == 0x2a
-			&& rdesc[40] == 0xf5
-			&& rdesc[41] == 0x00
-			&& rdesc[59] == 0x26
-			&& rdesc[60] == 0xf9
-			&& rdesc[61] == 0x00) {
-		info("Fixing up Petalynx Maxter Remote report descriptor");
-		rdesc[60] = 0xfa;
-		rdesc[40] = 0xfa;
-	}
-}
-
-/*
- * Some USB barcode readers from cypress have usage min and usage max in
- * the wrong order
- */
-static void hid_fixup_cypress_descriptor(unsigned char *rdesc, int rsize)
-{
-	short fixed = 0;
-	int i;
-
-	for (i = 0; i < rsize - 4; i++) {
-		if (rdesc[i] == 0x29 && rdesc [i+2] == 0x19) {
-			unsigned char tmp;
-
-			rdesc[i] = 0x19; rdesc[i+2] = 0x29;
-			tmp = rdesc[i+3];
-			rdesc[i+3] = rdesc[i+1];
-			rdesc[i+1] = tmp;
-		}
-	}
-
-	if (fixed)
-		info("Fixing up Cypress report descriptor");
-}
-
 static struct hid_device *usb_hid_configure(struct usb_interface *intf)
 {
 	struct usb_host_interface *interface = intf->cur_altsetting;
@@ -787,17 +724,9 @@ static struct hid_device *usb_hid_configure(struct usb_interface *intf)
 		return NULL;
 	}
 
-	if ((quirks & HID_QUIRK_CYMOTION))
-		hid_fixup_cymotion_descriptor(rdesc, rsize);
-
-	if (quirks & HID_QUIRK_LOGITECH_DESCRIPTOR)
-		hid_fixup_logitech_descriptor(rdesc, rsize);
-
-	if (quirks & HID_QUIRK_SWAPPED_MIN_MAX)
-		hid_fixup_cypress_descriptor(rdesc, rsize);
-
-	if (quirks & HID_QUIRK_PETALYNX_DESCRIPTOR)
-		hid_fixup_petalynx_descriptor(rdesc, rsize);
+	usbhid_fixup_report_descriptor(le16_to_cpu(dev->descriptor.idVendor),
+			le16_to_cpu(dev->descriptor.idProduct), rdesc,
+			rsize, rdesc_quirks_param);
 
 #ifdef CONFIG_HID_DEBUG
 	printk(KERN_DEBUG __FILE__ ": report descriptor (size %u, read %d) = ", rsize, n);

drivers/hid/usbhid/hid-quirks.c

@@ -299,8 +299,6 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD, HID_QUIRK_BADPAD },
 	{ USB_VENDOR_ID_TOPMAX, USB_DEVICE_ID_TOPMAX_COBRAPAD, HID_QUIRK_BADPAD },
 	
-	{ USB_VENDOR_ID_CHERRY, USB_DEVICE_ID_CHERRY_CYMOTION, HID_QUIRK_CYMOTION },
-
 	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_DINOVO_EDGE, HID_QUIRK_DUPLICATE_USAGES },
 
 	{ USB_VENDOR_ID_BELKIN, USB_DEVICE_ID_FLIP_KVM, HID_QUIRK_HIDDEV },
@@ -424,17 +422,11 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_FLAIR, HID_QUIRK_IGNORE },
 	{ USB_VENDOR_ID_ACECAD, USB_DEVICE_ID_ACECAD_302, HID_QUIRK_IGNORE },
 
-	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER, HID_QUIRK_LOGITECH_DESCRIPTOR },
-	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER, HID_QUIRK_LOGITECH_DESCRIPTOR },
-	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2, HID_QUIRK_LOGITECH_DESCRIPTOR },
-
 	{ USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MIGHTYMOUSE, HID_QUIRK_MIGHTYMOUSE | HID_QUIRK_INVERT_HWHEEL },
 
 	{ USB_VENDOR_ID_PANTHERLORD, USB_DEVICE_ID_PANTHERLORD_TWIN_USB_JOYSTICK, HID_QUIRK_MULTI_INPUT | HID_QUIRK_SKIP_OUTPUT_REPORTS },
 	{ USB_VENDOR_ID_PLAYDOTCOM, USB_DEVICE_ID_PLAYDOTCOM_EMS_USBII, HID_QUIRK_MULTI_INPUT },
 
-	{ USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE, HID_QUIRK_PETALYNX_DESCRIPTOR | HID_QUIRK_NOGET },
-
 	{ USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_PS3_CONTROLLER, HID_QUIRK_SONY_PS3_CONTROLLER },
 
 	{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_UC100KM, HID_QUIRK_NOGET },
@@ -443,6 +435,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVM, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_ATEN, USB_DEVICE_ID_ATEN_4PORTKVMC, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_WHEEL, HID_QUIRK_NOGET },
+	{ USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_SUN, USB_DEVICE_ID_RARITAN_KVM_DONGLE, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_TURBOX, USB_DEVICE_ID_TURBOX_KEYBOARD, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_WISEGROUP, USB_DEVICE_ID_DUAL_USB_JOYPAD, HID_QUIRK_NOGET | HID_QUIRK_MULTI_INPUT },
@@ -466,8 +459,26 @@ static const struct hid_blacklist {
 
 	{ USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_W7658, HID_QUIRK_RESET_LEDS },
 
-	{ USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_1, HID_QUIRK_SWAPPED_MIN_MAX },
-	{ USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_2, HID_QUIRK_SWAPPED_MIN_MAX },
+	{ 0, 0 }
+};
+
+/* Quirks for devices which require report descriptor fixup go here */
+static const struct hid_rdesc_blacklist {
+	__u16 idVendor;
+	__u16 idProduct;
+	__u32 quirks;
+} hid_rdesc_blacklist[] = {
+
+	{ USB_VENDOR_ID_CHERRY, USB_DEVICE_ID_CHERRY_CYMOTION, HID_QUIRK_RDESC_CYMOTION },
+
+	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_MX3000_RECEIVER, HID_QUIRK_RDESC_LOGITECH },
+	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER, HID_QUIRK_RDESC_LOGITECH },
+	{ USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_S510_RECEIVER_2, HID_QUIRK_RDESC_LOGITECH },
+
+	{ USB_VENDOR_ID_PETALYNX, USB_DEVICE_ID_PETALYNX_MAXTER_REMOTE, HID_QUIRK_RDESC_PETALYNX },
+
+	{ USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_1, HID_QUIRK_RDESC_SWAPPED_MIN_MAX },
+	{ USB_VENDOR_ID_CYPRESS, USB_DEVICE_ID_CYPRESS_BARCODE_2, HID_QUIRK_RDESC_SWAPPED_MIN_MAX },
 
 	{ 0, 0 }
 };
@@ -576,7 +587,6 @@ int usbhid_modify_dquirk(const u16 idVendor, const u16 idProduct,
 	return 0;
 }
 
-
 /**
  * usbhid_remove_all_dquirks: remove all runtime HID quirks from memory
  *
@@ -709,3 +719,126 @@ u32 usbhid_lookup_quirk(const u16 idVendor, const u16 idProduct)
 	return quirks;
 }
 
+/*
+ * Cherry Cymotion keyboard have an invalid HID report descriptor,
+ * that needs fixing before we can parse it.
+ */
+static void usbhid_fixup_cymotion_descriptor(char *rdesc, int rsize)
+{
+	if (rsize >= 17 && rdesc[11] == 0x3c && rdesc[12] == 0x02) {
+		printk(KERN_INFO "Fixing up Cherry Cymotion report descriptor\n");
+		rdesc[11] = rdesc[16] = 0xff;
+		rdesc[12] = rdesc[17] = 0x03;
+	}
+}
+
+
+/*
+ * Certain Logitech keyboards send in report #3 keys which are far
+ * above the logical maximum described in descriptor. This extends
+ * the original value of 0x28c of logical maximum to 0x104d
+ */
+static void usbhid_fixup_logitech_descriptor(unsigned char *rdesc, int rsize)
+{
+	if (rsize >= 90 && rdesc[83] == 0x26
+			&& rdesc[84] == 0x8c
+			&& rdesc[85] == 0x02) {
+		printk(KERN_INFO "Fixing up Logitech keyboard report descriptor\n");
+		rdesc[84] = rdesc[89] = 0x4d;
+		rdesc[85] = rdesc[90] = 0x10;
+	}
+}
+
+/* Petalynx Maxter Remote has maximum for consumer page set too low */
+static void usbhid_fixup_petalynx_descriptor(unsigned char *rdesc, int rsize)
+{
+	if (rsize >= 60 && rdesc[39] == 0x2a
+			&& rdesc[40] == 0xf5
+			&& rdesc[41] == 0x00
+			&& rdesc[59] == 0x26
+			&& rdesc[60] == 0xf9
+			&& rdesc[61] == 0x00) {
+		printk(KERN_INFO "Fixing up Petalynx Maxter Remote report descriptor\n");
+		rdesc[60] = 0xfa;
+		rdesc[40] = 0xfa;
+	}
+}
+
+/*
+ * Some USB barcode readers from cypress have usage min and usage max in
+ * the wrong order
+ */
+static void usbhid_fixup_cypress_descriptor(unsigned char *rdesc, int rsize)
+{
+	short fixed = 0;
+	int i;
+
+	for (i = 0; i < rsize - 4; i++) {
+		if (rdesc[i] == 0x29 && rdesc [i+2] == 0x19) {
+			unsigned char tmp;
+
+			rdesc[i] = 0x19; rdesc[i+2] = 0x29;
+			tmp = rdesc[i+3];
+			rdesc[i+3] = rdesc[i+1];
+			rdesc[i+1] = tmp;
+		}
+	}
+
+	if (fixed)
+		printk(KERN_INFO "Fixing up Cypress report descriptor\n");
+}
+
+
+static void __usbhid_fixup_report_descriptor(__u32 quirks, char *rdesc, unsigned rsize)
+{
+	if ((quirks & HID_QUIRK_RDESC_CYMOTION))
+		usbhid_fixup_cymotion_descriptor(rdesc, rsize);
+
+	if (quirks & HID_QUIRK_RDESC_LOGITECH)
+		usbhid_fixup_logitech_descriptor(rdesc, rsize);
+
+	if (quirks & HID_QUIRK_RDESC_SWAPPED_MIN_MAX)
+		usbhid_fixup_cypress_descriptor(rdesc, rsize);
+
+	if (quirks & HID_QUIRK_RDESC_PETALYNX)
+		usbhid_fixup_petalynx_descriptor(rdesc, rsize);
+}
+
+/**
+ * usbhid_fixup_report_descriptor: check if report descriptor needs fixup
+ *
+ * Description:
+ *	Walks the hid_rdesc_blacklist[] array and checks whether the device
+ *	is known to have broken report descriptor that needs to be fixed up
+ *	prior to entering the HID parser
+ *
+ * Returns: nothing
+ */
+void usbhid_fixup_report_descriptor(const u16 idVendor, const u16 idProduct,
+				    char *rdesc, unsigned rsize, char **quirks_param)
+{
+	int n, m;
+	u16 paramVendor, paramProduct;
+	u32 quirks;
+
+	/* static rdesc quirk entries */
+	for (n = 0; hid_rdesc_blacklist[n].idVendor; n++)
+		if (hid_rdesc_blacklist[n].idVendor == idVendor &&
+				hid_rdesc_blacklist[n].idProduct == idProduct)
+			__usbhid_fixup_report_descriptor(hid_rdesc_blacklist[n].quirks,
+					rdesc, rsize);
+
+	/* runtime rdesc quirk entries handling */
+	for (n = 0; quirks_param[n] && n < MAX_USBHID_BOOT_QUIRKS; n++) {
+		m = sscanf(quirks_param[n], "0x%hx:0x%hx:0x%x",
+				&paramVendor, &paramProduct, &quirks);
+
+		if (m != 3)
+			printk(KERN_WARNING
+				"Could not parse HID quirk module param %s\n",
+				quirks_param[n]);
+		else if (paramVendor == idVendor && paramProduct == idProduct)
+			__usbhid_fixup_report_descriptor(quirks, rdesc, rsize);
+	}
+
+}

include/linux/hid.h

@@ -263,21 +263,26 @@ struct hid_item {
 #define HID_QUIRK_2WHEEL_MOUSE_HACK_5		0x00000100
 #define HID_QUIRK_2WHEEL_MOUSE_HACK_ON		0x00000200
 #define HID_QUIRK_MIGHTYMOUSE			0x00000400
-#define HID_QUIRK_CYMOTION			0x00000800
-#define HID_QUIRK_POWERBOOK_HAS_FN		0x00001000
-#define HID_QUIRK_POWERBOOK_FN_ON		0x00002000
-#define HID_QUIRK_INVERT_HWHEEL			0x00004000
-#define HID_QUIRK_POWERBOOK_ISO_KEYBOARD        0x00008000
-#define HID_QUIRK_BAD_RELATIVE_KEYS		0x00010000
-#define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00020000
-#define HID_QUIRK_IGNORE_MOUSE			0x00040000
-#define HID_QUIRK_SONY_PS3_CONTROLLER		0x00080000
-#define HID_QUIRK_LOGITECH_DESCRIPTOR		0x00100000
-#define HID_QUIRK_DUPLICATE_USAGES		0x00200000
-#define HID_QUIRK_RESET_LEDS			0x00400000
-#define HID_QUIRK_SWAPPED_MIN_MAX		0x00800000
-#define HID_QUIRK_HIDINPUT			0x01000000
-#define HID_QUIRK_PETALYNX_DESCRIPTOR		0x02000000
+#define HID_QUIRK_POWERBOOK_HAS_FN		0x00000800
+#define HID_QUIRK_POWERBOOK_FN_ON		0x00001000
+#define HID_QUIRK_INVERT_HWHEEL			0x00002000
+#define HID_QUIRK_POWERBOOK_ISO_KEYBOARD        0x00004000
+#define HID_QUIRK_BAD_RELATIVE_KEYS		0x00008000
+#define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
+#define HID_QUIRK_IGNORE_MOUSE			0x00020000
+#define HID_QUIRK_SONY_PS3_CONTROLLER		0x00040000
+#define HID_QUIRK_DUPLICATE_USAGES		0x00080000
+#define HID_QUIRK_RESET_LEDS			0x00100000
+#define HID_QUIRK_HIDINPUT			0x00200000
+
+/*
+ * Separate quirks for runtime report descriptor fixup
+ */
+
+#define HID_QUIRK_RDESC_CYMOTION		0x00000001
+#define HID_QUIRK_RDESC_LOGITECH		0x00000002
+#define HID_QUIRK_RDESC_SWAPPED_MIN_MAX		0x00000004
+#define HID_QUIRK_RDESC_PETALYNX		0x00000008
 
 /*
  * This is the global environment of the parser. This information is
@@ -508,6 +513,7 @@ u32 usbhid_lookup_quirk(const u16 idVendor, const u16 idProduct);
 int usbhid_modify_dquirk(const u16 idVendor, const u16 idProduct, const u32 quirks);
 int usbhid_quirks_init(char **quirks_param);
 void usbhid_quirks_exit(void);
+void usbhid_fixup_report_descriptor(const u16, const u16, char *, unsigned, char **);
 
 #ifdef CONFIG_HID_FF
 int hid_ff_init(struct hid_device *hid);