[PATCH v2 0/2] HID: glorious: fix Glorious Model I HID report and rename vendor ID

[PATCH v2 0/2] HID: glorious: fix Glorious Model I HID report and rename vendor ID

[Brett Raye]

The Glorious Model I mouse presents a faulty HID report descriptor which
prevents some of its reprogrammable buttons from working. This patch
series performs a fixup on the faulty descriptor, and renames a vendor ID
for consistency and clarity.

Changes in v2:
- Split vendor ID rename into its own patch.
- Link to v1: https://lore.kernel.org/linux-input/20231103011038.27462-1-braye@fastmail.com/

Brett Raye (2):
  HID: glorious: fix Glorious Model I HID report
  HID: glorious: rename USB_VENDOR_ID_GLORIOUS

 drivers/hid/hid-glorious.c | 16 ++++++++++++++--
 drivers/hid/hid-ids.h      | 11 +++++++----
 2 files changed, 21 insertions(+), 6 deletions(-)


base-commit: bab19d1b21547046b0a38dde948086f6cbcaefaa
-- 
2.40.1

[PATCH v2 1/2] HID: glorious: fix Glorious Model I HID report

[Brett Raye]

The Glorious Model I mouse has a buggy HID report descriptor for its
keyboard endpoint (used for programmable buttons). For report ID 2, there
is a mismatch between Logical Minimum and Usage Minimum in the array that
reports keycodes.

The offending portion of the descriptor: (from hid-decode)

0x95, 0x05,                    //  Report Count (5)                   30
0x75, 0x08,                    //  Report Size (8)                    32
0x15, 0x00,                    //  Logical Minimum (0)                34
0x25, 0x65,                    //  Logical Maximum (101)              36
0x05, 0x07,                    //  Usage Page (Keyboard)              38
0x19, 0x01,                    //  Usage Minimum (1)                  40
0x29, 0x65,                    //  Usage Maximum (101)                42
0x81, 0x00,                    //  Input (Data,Arr,Abs)               44

This bug shifts all programmed keycodes up by 1. Importantly, this causes
"empty" array indexes of 0x00 to be interpreted as 0x01, ErrorRollOver.
The presence of ErrorRollOver causes the system to ignore all keypresses
from the endpoint and breaks the ability to use the programmable buttons.

Setting byte 41 to 0x00 fixes this, and causes keycodes to be interpreted
correctly.

Signed-off-by: Brett Raye <braye@fastmail.com>
---
 drivers/hid/hid-glorious.c | 12 ++++++++++++
 drivers/hid/hid-ids.h      |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/drivers/hid/hid-glorious.c b/drivers/hid/hid-glorious.c
index 558eb08c19ef..e44a39a17151 100644
--- a/drivers/hid/hid-glorious.c
+++ b/drivers/hid/hid-glorious.c
@@ -21,6 +21,10 @@ MODULE_DESCRIPTION("HID driver for Glorious PC Gaming Race mice");
  * Glorious Model O and O- specify the const flag in the consumer input
  * report descriptor, which leads to inputs being ignored. Fix this
  * by patching the descriptor.
+ *
+ * Glorious Model I incorrectly specifes the Usage Minimum for its
+ * keyboard HID report, causing keycodes to be misinterpreted.
+ * Fix this by setting Usage Minimum to 0 in that report.
  */
 static __u8 *glorious_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		unsigned int *rsize)
@@ -32,6 +36,10 @@ static __u8 *glorious_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 		rdesc[85] = rdesc[113] = rdesc[141] = \
 			HID_MAIN_ITEM_VARIABLE | HID_MAIN_ITEM_RELATIVE;
 	}
+	if (*rsize == 156 && rdesc[41] == 1) {
+		hid_info(hdev, "patching Glorious Model I keyboard report descriptor\n");
+		rdesc[41] = 0;
+	}
 	return rdesc;
 }
 
@@ -44,6 +52,8 @@ static void glorious_update_name(struct hid_device *hdev)
 		model = "Model O"; break;
 	case USB_DEVICE_ID_GLORIOUS_MODEL_D:
 		model = "Model D"; break;
+	case USB_DEVICE_ID_GLORIOUS_MODEL_I:
+		model = "Model I"; break;
 	}
 
 	snprintf(hdev->name, sizeof(hdev->name), "%s %s", "Glorious", model);
@@ -70,6 +80,8 @@ static const struct hid_device_id glorious_devices[] = {
 		USB_DEVICE_ID_GLORIOUS_MODEL_O) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_GLORIOUS,
 		USB_DEVICE_ID_GLORIOUS_MODEL_D) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_LAVIEW,
+		USB_DEVICE_ID_GLORIOUS_MODEL_I) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, glorious_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e4d2dfd5d253..c30bca343982 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -744,6 +744,9 @@
 #define USB_VENDOR_ID_LABTEC		0x1020
 #define USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD	0x0006
 
+#define USB_VENDOR_ID_LAVIEW		0x22D4
+#define USB_DEVICE_ID_GLORIOUS_MODEL_I	0x1503
+
 #define USB_VENDOR_ID_LCPOWER		0x1241
 #define USB_DEVICE_ID_LCPOWER_LC1000	0xf767
 
-- 
2.40.1

[PATCH v2 2/2] HID: glorious: rename USB_VENDOR_ID_GLORIOUS

[Brett Raye]

USB_VENDOR_ID_GLORIOUS is changed to USB_VENDOR_ID_SINOWEALTH. Glorious
seems to be white-labeling controller boards or mice from many vendors.
There isn't a single canonical vendor ID for Glorious products.

Signed-off-by: Brett Raye <braye@fastmail.com>
---
 drivers/hid/hid-glorious.c | 4 ++--
 drivers/hid/hid-ids.h      | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-glorious.c b/drivers/hid/hid-glorious.c
index e44a39a17151..281b3a7187ce 100644
--- a/drivers/hid/hid-glorious.c
+++ b/drivers/hid/hid-glorious.c
@@ -76,9 +76,9 @@ static int glorious_probe(struct hid_device *hdev,
 }
 
 static const struct hid_device_id glorious_devices[] = {
-	{ HID_USB_DEVICE(USB_VENDOR_ID_GLORIOUS,
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
 		USB_DEVICE_ID_GLORIOUS_MODEL_O) },
-	{ HID_USB_DEVICE(USB_VENDOR_ID_GLORIOUS,
+	{ HID_USB_DEVICE(USB_VENDOR_ID_SINOWEALTH,
 		USB_DEVICE_ID_GLORIOUS_MODEL_D) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_LAVIEW,
 		USB_DEVICE_ID_GLORIOUS_MODEL_I) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c30bca343982..9ed9ec03ad1b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -510,10 +510,6 @@
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_010A 0x010a
 #define USB_DEVICE_ID_GENERAL_TOUCH_WIN8_PIT_E100 0xe100
 
-#define USB_VENDOR_ID_GLORIOUS  0x258a
-#define USB_DEVICE_ID_GLORIOUS_MODEL_D 0x0033
-#define USB_DEVICE_ID_GLORIOUS_MODEL_O 0x0036
-
 #define I2C_VENDOR_ID_GOODIX		0x27c6
 #define I2C_DEVICE_ID_GOODIX_01F0	0x01f0
 
@@ -1162,6 +1158,10 @@
 #define USB_VENDOR_ID_SIGMATEL		0x066F
 #define USB_DEVICE_ID_SIGMATEL_STMP3780	0x3780
 
+#define USB_VENDOR_ID_SINOWEALTH  0x258a
+#define USB_DEVICE_ID_GLORIOUS_MODEL_D 0x0033
+#define USB_DEVICE_ID_GLORIOUS_MODEL_O 0x0036
+
 #define USB_VENDOR_ID_SIS_TOUCH		0x0457
 #define USB_DEVICE_ID_SIS9200_TOUCH	0x9200
 #define USB_DEVICE_ID_SIS817_TOUCH	0x0817
-- 
2.40.1

Re: [PATCH v2 0/2] HID: glorious: fix Glorious Model I HID report and rename vendor ID

[Rahul Rameshbabu]

On Mon, 06 Nov, 2023 15:55:55 -0800 "Brett Raye" <braye@fastmail.com> wrote:
> The Glorious Model I mouse presents a faulty HID report descriptor which
> prevents some of its reprogrammable buttons from working. This patch
> series performs a fixup on the faulty descriptor, and renames a vendor ID
> for consistency and clarity.

Thanks for the patches.

Reviewed-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>