From 61595012f28036a58293df5a2ab75f80ca15c327 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Tue, 1 Oct 2024 08:42:36 -0700 Subject: [PATCH 01/40] HID: simplify code in fetch_item() We can easily calculate the size of the item using arithmetic (shifts). This allows to pull duplicated code out of the switch statement, making it cleaner. Signed-off-by: Dmitry Torokhov Link: https://patch.msgid.link/ZvwYbESMZ667QZqY@google.com Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 30de92d0bf0f..0dc3018de2a6 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -754,35 +754,32 @@ static const u8 *fetch_item(const __u8 *start, const __u8 *end, struct hid_item } item->format = HID_ITEM_FORMAT_SHORT; - item->size = b & 3; + item->size = BIT(b & 3) >> 1; /* 0, 1, 2, 3 -> 0, 1, 2, 4 */ + + if (end - start < item->size) + return NULL; switch (item->size) { case 0: - return start; + break; case 1: - if ((end - start) < 1) - return NULL; - item->data.u8 = *start++; - return start; + item->data.u8 = *start; + break; case 2: - if ((end - start) < 2) - return NULL; item->data.u16 = get_unaligned_le16(start); - start = (__u8 *)((__le16 *)start + 1); - return start; + break; - case 3: - item->size++; - if ((end - start) < 4) - return NULL; + case 4: item->data.u32 = get_unaligned_le32(start); - start = (__u8 *)((__le32 *)start + 1); - return start; + break; + + default: + unreachable(); } - return NULL; + return start + item->size; } static void hid_scan_input_usage(struct hid_parser *parser, u32 usage) From ae9b956cb26c0fd5a365629f2d723ab2fb14df79 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Thu, 3 Oct 2024 07:46:50 -0700 Subject: [PATCH 02/40] HID: simplify snto32() snto32() does exactly what sign_extend32() does, but handles potentially malformed data coming from the device. Keep the checks, but then call sign_extend32() to perform the actual conversion. Signed-off-by: Dmitry Torokhov Link: https://patch.msgid.link/20241003144656.3786064-1-dmitry.torokhov@gmail.com Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 0dc3018de2a6..a6fe04e1c726 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1313,9 +1313,7 @@ alloc_err: EXPORT_SYMBOL_GPL(hid_open_report); /* - * Convert a signed n-bit integer to signed 32-bit integer. Common - * cases are done through the compiler, the screwed things has to be - * done by hand. + * Convert a signed n-bit integer to signed 32-bit integer. */ static s32 snto32(__u32 value, unsigned n) @@ -1326,12 +1324,7 @@ static s32 snto32(__u32 value, unsigned n) if (n > 32) n = 32; - switch (n) { - case 8: return ((__s8)value); - case 16: return ((__s16)value); - case 32: return ((__s32)value); - } - return value & (1 << (n - 1)) ? value | (~0U << n) : value; + return sign_extend32(value, n - 1); } s32 hid_snto32(__u32 value, unsigned n) From c653ffc283404a6c1c0e65143a833180c7ff799b Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Thu, 3 Oct 2024 07:46:51 -0700 Subject: [PATCH 03/40] HID: stop exporting hid_snto32() The only user of hid_snto32() is Logitech HID++ driver, which always calls hid_snto32() with valid size (constant, either 12 or 8) and therefore can simply use sign_extend32(). Make the switch and remove hid_snto32(). Move snto32() and s32ton() to avoid introducing forward declaration. Signed-off-by: Dmitry Torokhov Link: https://patch.msgid.link/20241003144656.3786064-2-dmitry.torokhov@gmail.com [bentiss: fix checkpatch warning] Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 63 +++++++++++++++----------------- drivers/hid/hid-logitech-hidpp.c | 6 +-- include/linux/hid.h | 1 - 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index a6fe04e1c726..87b2cdd7f0c4 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -45,6 +45,34 @@ static int hid_ignore_special_drivers = 0; module_param_named(ignore_special_drivers, hid_ignore_special_drivers, int, 0600); MODULE_PARM_DESC(ignore_special_drivers, "Ignore any special drivers and handle all devices by generic driver"); +/* + * Convert a signed n-bit integer to signed 32-bit integer. + */ + +static s32 snto32(__u32 value, unsigned int n) +{ + if (!value || !n) + return 0; + + if (n > 32) + n = 32; + + return sign_extend32(value, n - 1); +} + +/* + * Convert a signed 32-bit integer to a signed n-bit integer. + */ + +static u32 s32ton(__s32 value, unsigned int n) +{ + s32 a = value >> (n - 1); + + if (a && a != -1) + return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1; + return value & ((1 << n) - 1); +} + /* * Register a new report for a device. */ @@ -425,7 +453,7 @@ static int hid_parser_global(struct hid_parser *parser, struct hid_item *item) * both this and the standard encoding. */ raw_value = item_sdata(item); if (!(raw_value & 0xfffffff0)) - parser->global.unit_exponent = hid_snto32(raw_value, 4); + parser->global.unit_exponent = snto32(raw_value, 4); else parser->global.unit_exponent = raw_value; return 0; @@ -1312,39 +1340,6 @@ alloc_err: } EXPORT_SYMBOL_GPL(hid_open_report); -/* - * Convert a signed n-bit integer to signed 32-bit integer. - */ - -static s32 snto32(__u32 value, unsigned n) -{ - if (!value || !n) - return 0; - - if (n > 32) - n = 32; - - return sign_extend32(value, n - 1); -} - -s32 hid_snto32(__u32 value, unsigned n) -{ - return snto32(value, n); -} -EXPORT_SYMBOL_GPL(hid_snto32); - -/* - * Convert a signed 32-bit integer to a signed n-bit integer. - */ - -static u32 s32ton(__s32 value, unsigned n) -{ - s32 a = value >> (n - 1); - if (a && a != -1) - return value < 0 ? 1 << (n - 1) : (1 << (n - 1)) - 1; - return value & ((1 << n) - 1); -} - /* * Extract/implement a data field from/to a little endian report (bit array). * diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 0e33fa0eb8db..0d114268ade8 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -3296,13 +3296,13 @@ static int m560_raw_event(struct hid_device *hdev, u8 *data, int size) 120); } - v = hid_snto32(hid_field_extract(hdev, data+3, 0, 12), 12); + v = sign_extend32(hid_field_extract(hdev, data + 3, 0, 12), 11); input_report_rel(hidpp->input, REL_X, v); - v = hid_snto32(hid_field_extract(hdev, data+3, 12, 12), 12); + v = sign_extend32(hid_field_extract(hdev, data + 3, 12, 12), 11); input_report_rel(hidpp->input, REL_Y, v); - v = hid_snto32(data[6], 8); + v = sign_extend32(data[6], 7); if (v != 0) hidpp_scroll_counter_handle_scroll(hidpp->input, &hidpp->vertical_wheel_counter, v); diff --git a/include/linux/hid.h b/include/linux/hid.h index 121d5b8bc867..f7c3a66647fd 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -974,7 +974,6 @@ const struct hid_device_id *hid_match_device(struct hid_device *hdev, struct hid_driver *hdrv); bool hid_compare_device_paths(struct hid_device *hdev_a, struct hid_device *hdev_b, char separator); -s32 hid_snto32(__u32 value, unsigned n); __u32 hid_field_extract(const struct hid_device *hid, __u8 *report, unsigned offset, unsigned n); From 8b7fd6a15f8c32760c2026a62dcf55219b4da15b Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:05 +0200 Subject: [PATCH 04/40] HID: bpf: move HID-BPF report descriptor fixup earlier Currently, hid_bpf_rdesc_fixup() is called once the match between the HID device and the driver is done. This can be problematic in case the driver selected by the kernel would change the report descriptor after the fact. To give a chance for hid_bpf_rdesc_fixup() to provide hints on to how to select a dedicated driver or not, move the call to that BPF hook earlier in the .probe() process, when we get the first match. However, this means that we might get called more than once (typically once for hid-generic, and once for hid-vendor-specific). So we store the result of HID-BPF fixup in struct hid_device. Basically, this means that ->bpf_rdesc can replace ->dev_rdesc when it was used in the code. In order to not grow struct hid_device, some fields are re-ordered. This was the output of pahole for the first 128 bytes: struct hid_device { __u8 * dev_rdesc; /* 0 8 */ unsigned int dev_rsize; /* 8 4 */ /* XXX 4 bytes hole, try to pack */ __u8 * rdesc; /* 16 8 */ unsigned int rsize; /* 24 4 */ /* XXX 4 bytes hole, try to pack */ struct hid_collection * collection; /* 32 8 */ unsigned int collection_size; /* 40 4 */ unsigned int maxcollection; /* 44 4 */ unsigned int maxapplication; /* 48 4 */ __u16 bus; /* 52 2 */ __u16 group; /* 54 2 */ __u32 vendor; /* 56 4 */ __u32 product; /* 60 4 */ /* --- cacheline 1 boundary (64 bytes) --- */ __u32 version; /* 64 4 */ enum hid_type type; /* 68 4 */ unsigned int country; /* 72 4 */ /* XXX 4 bytes hole, try to pack */ struct hid_report_enum report_enum[3]; /* 80 6216 */ Basically, we got three holes of 4 bytes. We can reorder things a little and makes those 3 holes a continuous 12 bytes hole, which can be replaced by the new pointer and the new unsigned int we need. In terms of code allocation, when not using HID-BPF, we are back to kernel v6.2 in hid_open_report(). These multiple kmemdup() calls will be fixed in a later commit. Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-1-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/bpf/hid_bpf_dispatch.c | 9 ++++++--- drivers/hid/hid-core.c | 30 ++++++++++++++++++++++++++---- include/linux/hid.h | 18 ++++++++++-------- include/linux/hid_bpf.h | 11 +++-------- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/drivers/hid/bpf/hid_bpf_dispatch.c b/drivers/hid/bpf/hid_bpf_dispatch.c index 8420c227e21b..961b7f35aa67 100644 --- a/drivers/hid/bpf/hid_bpf_dispatch.c +++ b/drivers/hid/bpf/hid_bpf_dispatch.c @@ -148,7 +148,7 @@ out: } EXPORT_SYMBOL_GPL(dispatch_hid_bpf_output_report); -u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size) +const u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size) { int ret; struct hid_bpf_ctx_kern ctx_kern = { @@ -183,7 +183,7 @@ u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned ignore_bpf: kfree(ctx_kern.data); - return kmemdup(rdesc, *size, GFP_KERNEL); + return rdesc; } EXPORT_SYMBOL_GPL(call_hid_bpf_rdesc_fixup); @@ -260,8 +260,11 @@ int hid_bpf_allocate_event_data(struct hid_device *hdev) int hid_bpf_reconnect(struct hid_device *hdev) { - if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) + if (!test_and_set_bit(ffs(HID_STAT_REPROBED), &hdev->status)) { + /* trigger call to call_hid_bpf_rdesc_fixup() during the next probe */ + hdev->bpf_rsize = 0; return device_reprobe(&hdev->dev); + } return 0; } diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 30de92d0bf0f..d6bf933623e8 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -698,6 +698,14 @@ static void hid_close_report(struct hid_device *device) device->status &= ~HID_STAT_PARSED; } +static inline void hid_free_bpf_rdesc(struct hid_device *hdev) +{ + /* bpf_rdesc is either equal to dev_rdesc or allocated by call_hid_bpf_rdesc_fixup() */ + if (hdev->bpf_rdesc != hdev->dev_rdesc) + kfree(hdev->bpf_rdesc); + hdev->bpf_rdesc = NULL; +} + /* * Free a device structure, all reports, and all fields. */ @@ -707,6 +715,7 @@ void hiddev_free(struct kref *ref) struct hid_device *hid = container_of(ref, struct hid_device, ref); hid_close_report(hid); + hid_free_bpf_rdesc(hid); kfree(hid->dev_rdesc); kfree(hid); } @@ -1221,13 +1230,12 @@ int hid_open_report(struct hid_device *device) if (WARN_ON(device->status & HID_STAT_PARSED)) return -EBUSY; - start = device->dev_rdesc; + start = device->bpf_rdesc; if (WARN_ON(!start)) return -ENODEV; - size = device->dev_rsize; + size = device->bpf_rsize; - /* call_hid_bpf_rdesc_fixup() ensures we work on a copy of rdesc */ - buf = call_hid_bpf_rdesc_fixup(device, start, &size); + buf = kmemdup(start, size, GFP_KERNEL); if (buf == NULL) return -ENOMEM; @@ -2684,6 +2692,18 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv) const struct hid_device_id *id; int ret; + if (!hdev->bpf_rsize) { + /* in case a bpf program gets detached, we need to free the old one */ + hid_free_bpf_rdesc(hdev); + + /* keep this around so we know we called it once */ + hdev->bpf_rsize = hdev->dev_rsize; + + /* call_hid_bpf_rdesc_fixup will always return a valid pointer */ + hdev->bpf_rdesc = call_hid_bpf_rdesc_fixup(hdev, hdev->dev_rdesc, + &hdev->bpf_rsize); + } + if (!hid_check_device_match(hdev, hdrv, &id)) return -ENODEV; @@ -2940,9 +2960,11 @@ static void hid_remove_device(struct hid_device *hdev) hid_debug_unregister(hdev); hdev->status &= ~HID_STAT_ADDED; } + hid_free_bpf_rdesc(hdev); kfree(hdev->dev_rdesc); hdev->dev_rdesc = NULL; hdev->dev_rsize = 0; + hdev->bpf_rsize = 0; } /** diff --git a/include/linux/hid.h b/include/linux/hid.h index 121d5b8bc867..ff58b5ceb62e 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -599,15 +599,17 @@ enum hid_battery_status { struct hid_driver; struct hid_ll_driver; -struct hid_device { /* device report descriptor */ - const __u8 *dev_rdesc; - unsigned dev_rsize; - const __u8 *rdesc; - unsigned rsize; +struct hid_device { + const __u8 *dev_rdesc; /* device report descriptor */ + const __u8 *bpf_rdesc; /* bpf modified report descriptor, if any */ + const __u8 *rdesc; /* currently used report descriptor */ + unsigned int dev_rsize; + unsigned int bpf_rsize; + unsigned int rsize; + unsigned int collection_size; /* Number of allocated hid_collections */ struct hid_collection *collection; /* List of HID collections */ - unsigned collection_size; /* Number of allocated hid_collections */ - unsigned maxcollection; /* Number of parsed collections */ - unsigned maxapplication; /* Number of applications */ + unsigned int maxcollection; /* Number of parsed collections */ + unsigned int maxapplication; /* Number of applications */ __u16 bus; /* BUS ID */ __u16 group; /* Report group */ __u32 vendor; /* Vendor ID */ diff --git a/include/linux/hid_bpf.h b/include/linux/hid_bpf.h index 6a47223e6460..a6876ab29004 100644 --- a/include/linux/hid_bpf.h +++ b/include/linux/hid_bpf.h @@ -212,7 +212,7 @@ int hid_bpf_connect_device(struct hid_device *hdev); void hid_bpf_disconnect_device(struct hid_device *hdev); void hid_bpf_destroy_device(struct hid_device *hid); int hid_bpf_device_init(struct hid_device *hid); -u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size); +const u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, unsigned int *size); #else /* CONFIG_HID_BPF */ static inline u8 *dispatch_hid_bpf_device_event(struct hid_device *hid, enum hid_report_type type, u8 *data, u32 *size, int interrupt, @@ -228,13 +228,8 @@ static inline int hid_bpf_connect_device(struct hid_device *hdev) { return 0; } static inline void hid_bpf_disconnect_device(struct hid_device *hdev) {} static inline void hid_bpf_destroy_device(struct hid_device *hid) {} static inline int hid_bpf_device_init(struct hid_device *hid) { return 0; } -/* - * This specialized allocator has to be a macro for its allocations to be - * accounted separately (to have a separate alloc_tag). The typecast is - * intentional to enforce typesafety. - */ -#define call_hid_bpf_rdesc_fixup(_hdev, _rdesc, _size) \ - ((u8 *)kmemdup(_rdesc, *(_size), GFP_KERNEL)) +static inline const u8 *call_hid_bpf_rdesc_fixup(struct hid_device *hdev, const u8 *rdesc, + unsigned int *size) { return rdesc; } #endif /* CONFIG_HID_BPF */ From 52cd1906ef6b93d638a78a34765c38c7edadd2ff Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:06 +0200 Subject: [PATCH 05/40] HID: core: save one kmemdup during .probe() Turns out the first kmemdup is only required for the .report_fixup() driver callback. There is no need to do two kmemdup() in a row in case .report_fixup() is not present. Reviewed-by: Peter Hutterer Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-2-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index d6bf933623e8..6053e7cdc0c1 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -1214,7 +1214,7 @@ int hid_open_report(struct hid_device *device) struct hid_item item; unsigned int size; const __u8 *start; - __u8 *buf; + __u8 *buf = NULL; const __u8 *end; const __u8 *next; int ret; @@ -1235,14 +1235,18 @@ int hid_open_report(struct hid_device *device) return -ENODEV; size = device->bpf_rsize; - buf = kmemdup(start, size, GFP_KERNEL); - if (buf == NULL) - return -ENOMEM; + if (device->driver->report_fixup) { + /* + * device->driver->report_fixup() needs to work + * on a copy of our report descriptor so it can + * change it. + */ + buf = kmemdup(start, size, GFP_KERNEL); + if (buf == NULL) + return -ENOMEM; - if (device->driver->report_fixup) start = device->driver->report_fixup(device, buf, &size); - else - start = buf; + } start = kmemdup(start, size, GFP_KERNEL); kfree(buf); From 7316fef4b993c4435f9fe55e7f2590baf25621ec Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:07 +0200 Subject: [PATCH 06/40] HID: core: remove one more kmemdup on .probe() That last kmemdup while opening the report descriptor was required to have a common kfree() on it. Move that kmemdup in the only special case it's required (if there is a .report_fixup()), and add a more elaborated check before freeing hdev->rdesc, to avoid a double free. Reviewed-by: Peter Hutterer Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-3-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 6053e7cdc0c1..b0a22e173502 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -685,7 +685,14 @@ static void hid_close_report(struct hid_device *device) INIT_LIST_HEAD(&report_enum->report_list); } - kfree(device->rdesc); + /* + * If the HID driver had a rdesc_fixup() callback, dev->rdesc + * will be allocated by hid-core and needs to be freed. + * Otherwise, it is either equal to dev_rdesc or bpf_rdesc, in + * which cases it'll be freed later on device removal or destroy. + */ + if (device->rdesc != device->dev_rdesc && device->rdesc != device->bpf_rdesc) + kfree(device->rdesc); device->rdesc = NULL; device->rsize = 0; @@ -1214,7 +1221,6 @@ int hid_open_report(struct hid_device *device) struct hid_item item; unsigned int size; const __u8 *start; - __u8 *buf = NULL; const __u8 *end; const __u8 *next; int ret; @@ -1241,17 +1247,23 @@ int hid_open_report(struct hid_device *device) * on a copy of our report descriptor so it can * change it. */ - buf = kmemdup(start, size, GFP_KERNEL); + __u8 *buf = kmemdup(start, size, GFP_KERNEL); + if (buf == NULL) return -ENOMEM; start = device->driver->report_fixup(device, buf, &size); - } - start = kmemdup(start, size, GFP_KERNEL); - kfree(buf); - if (start == NULL) - return -ENOMEM; + /* + * The second kmemdup is required in case report_fixup() returns + * a static read-only memory, but we have no idea if that memory + * needs to be cleaned up or not at the end. + */ + start = kmemdup(start, size, GFP_KERNEL); + kfree(buf); + if (start == NULL) + return -ENOMEM; + } device->rdesc = start; device->rsize = size; From 6fd47effe92b794c32f08504c2c64d1e40bbb543 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:08 +0200 Subject: [PATCH 07/40] HID: bpf: allow write access to quirks field in struct hid_device This allows to give more control from BPF during report descriptor fixup. We already reset the quirks before calling ->probe(), so now we reset it once before calling hid_bpf_rdesc_fixup(). Reviewed-by: Peter Hutterer Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-4-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/bpf/hid_bpf_struct_ops.c | 1 + drivers/hid/hid-core.c | 11 +++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/hid/bpf/hid_bpf_struct_ops.c b/drivers/hid/bpf/hid_bpf_struct_ops.c index 702c22fae136..0e611a9d79d7 100644 --- a/drivers/hid/bpf/hid_bpf_struct_ops.c +++ b/drivers/hid/bpf/hid_bpf_struct_ops.c @@ -79,6 +79,7 @@ static int hid_bpf_ops_btf_struct_access(struct bpf_verifier_log *log, WRITE_RANGE(hid_device, name, true), WRITE_RANGE(hid_device, uniq, true), WRITE_RANGE(hid_device, phys, true), + WRITE_RANGE(hid_device, quirks, false), }; #undef WRITE_RANGE const struct btf_type *state = NULL; diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index b0a22e173502..8e879937e956 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2709,6 +2709,12 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv) int ret; if (!hdev->bpf_rsize) { + unsigned int quirks; + + /* reset the quirks that has been previously set */ + quirks = hid_lookup_quirk(hdev); + hdev->quirks = quirks; + /* in case a bpf program gets detached, we need to free the old one */ hid_free_bpf_rdesc(hdev); @@ -2718,6 +2724,9 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv) /* call_hid_bpf_rdesc_fixup will always return a valid pointer */ hdev->bpf_rdesc = call_hid_bpf_rdesc_fixup(hdev, hdev->dev_rdesc, &hdev->bpf_rsize); + if (quirks ^ hdev->quirks) + hid_info(hdev, "HID-BPF toggled quirks on the device: %04x", + quirks ^ hdev->quirks); } if (!hid_check_device_match(hdev, hdrv, &id)) @@ -2727,8 +2736,6 @@ static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv) if (!hdev->devres_group_id) return -ENOMEM; - /* reset the quirks that has been previously set */ - hdev->quirks = hid_lookup_quirk(hdev); hdev->driver = hdrv; if (hdrv->probe) { From 0b838d768ccdbdfbcaed5f4b18b4bf63e53a0e0d Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:09 +0200 Subject: [PATCH 08/40] selftests/hid: add dependency on hid_common.h Allows to recompile the C tests when that file changes Reviewed-by: Peter Hutterer Acked-by: Shuah Khan Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-5-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/hid/Makefile b/tools/testing/selftests/hid/Makefile index 72be55ac4bdf..9399fa3f2f9d 100644 --- a/tools/testing/selftests/hid/Makefile +++ b/tools/testing/selftests/hid/Makefile @@ -229,7 +229,7 @@ $(BPF_SKELS): %.skel.h: %.bpf.o $(BPFTOOL) | $(OUTPUT) $(Q)$(BPFTOOL) gen object $(<:.o=.linked1.o) $< $(Q)$(BPFTOOL) gen skeleton $(<:.o=.linked1.o) name $(notdir $(<:.bpf.o=)) > $@ -$(OUTPUT)/%.o: %.c $(BPF_SKELS) +$(OUTPUT)/%.o: %.c $(BPF_SKELS) hid_common.h $(call msg,CC,,$@) $(Q)$(CC) $(CFLAGS) -c $(filter %.c,$^) $(LDLIBS) -o $@ From 4fb41dfde0699796b955eb94e7b8037a67b4b3a5 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:10 +0200 Subject: [PATCH 09/40] selftests/hid: cleanup C tests by adding a common struct uhid_device Allows to have an abstract class uhid_device which handles all of the uhid part without having to mess up with individual fds. struct attach_prog_args is now never used in hid_bpf.c, so drop it as well Acked-by: Shuah Khan Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-6-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/hid_bpf.c | 77 +++++++++--------------- tools/testing/selftests/hid/hid_common.h | 74 +++++++++++++++-------- tools/testing/selftests/hid/hidraw.c | 36 +++-------- 3 files changed, 87 insertions(+), 100 deletions(-) diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c index 86f4d66379f7..31ff92e0debd 100644 --- a/tools/testing/selftests/hid/hid_bpf.c +++ b/tools/testing/selftests/hid/hid_bpf.c @@ -4,13 +4,6 @@ #include "hid_common.h" #include -struct attach_prog_args { - int prog_fd; - unsigned int hid; - int retval; - int insert_head; -}; - struct hid_hw_request_syscall_args { __u8 data[10]; unsigned int hid; @@ -21,11 +14,8 @@ struct hid_hw_request_syscall_args { }; FIXTURE(hid_bpf) { - int dev_id; - int uhid_fd; + struct uhid_device hid; int hidraw_fd; - int hid_id; - pthread_t tid; struct hid *skel; struct bpf_link *hid_links[3]; /* max number of programs loaded in a single test */ }; @@ -54,10 +44,10 @@ static void detach_bpf(FIXTURE_DATA(hid_bpf) * self) FIXTURE_TEARDOWN(hid_bpf) { void *uhid_err; - uhid_destroy(_metadata, self->uhid_fd); + uhid_destroy(_metadata, &self->hid); detach_bpf(self); - pthread_join(self->tid, &uhid_err); + pthread_join(self->hid.tid, &uhid_err); } #define TEARDOWN_LOG(fmt, ...) do { \ TH_LOG(fmt, ##__VA_ARGS__); \ @@ -66,23 +56,10 @@ FIXTURE_TEARDOWN(hid_bpf) { FIXTURE_SETUP(hid_bpf) { - time_t t; int err; - /* initialize random number generator */ - srand((unsigned int)time(&t)); - - self->dev_id = rand() % 1024; - - self->uhid_fd = setup_uhid(_metadata, self->dev_id); - - /* locate the uev, self, variant);ent file of the created device */ - self->hid_id = get_hid_id(self->dev_id); - ASSERT_GT(self->hid_id, 0) - TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id); - - err = uhid_start_listener(_metadata, &self->tid, self->uhid_fd); - ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err); + err = setup_uhid(_metadata, &self->hid); + ASSERT_OK(err); } struct test_program { @@ -129,7 +106,7 @@ static void load_programs(const struct test_program programs[], ops_hid_id = bpf_map__initial_value(map, NULL); ASSERT_OK_PTR(ops_hid_id) TH_LOG("unable to retrieve struct_ops data"); - *ops_hid_id = self->hid_id; + *ops_hid_id = self->hid.hid_id; } /* we disable the auto-attach feature of all maps because we @@ -157,7 +134,7 @@ static void load_programs(const struct test_program programs[], hid__attach(self->skel); - self->hidraw_fd = open_hidraw(self->dev_id); + self->hidraw_fd = open_hidraw(&self->hid); ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw"); } @@ -192,7 +169,7 @@ TEST_F(hid_bpf, raw_event) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* check that hid_first_event() was executed */ ASSERT_EQ(self->skel->data->callback_check, 42) TH_LOG("callback_check1"); @@ -208,7 +185,7 @@ TEST_F(hid_bpf, raw_event) memset(buf, 0, sizeof(buf)); buf[0] = 1; buf[1] = 47; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* check that hid_first_event() was executed */ ASSERT_EQ(self->skel->data->callback_check, 47) TH_LOG("callback_check1"); @@ -239,7 +216,7 @@ TEST_F(hid_bpf, subprog_raw_event) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -252,7 +229,7 @@ TEST_F(hid_bpf, subprog_raw_event) memset(buf, 0, sizeof(buf)); buf[0] = 1; buf[1] = 47; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -303,7 +280,7 @@ TEST_F(hid_bpf, test_attach_detach) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -326,14 +303,14 @@ TEST_F(hid_bpf, test_attach_detach) /* detach the program */ detach_bpf(self); - self->hidraw_fd = open_hidraw(self->dev_id); + self->hidraw_fd = open_hidraw(&self->hid); ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw"); /* inject another event */ memset(buf, 0, sizeof(buf)); buf[0] = 1; buf[1] = 47; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -352,7 +329,7 @@ TEST_F(hid_bpf, test_attach_detach) memset(buf, 0, sizeof(buf)); buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -382,7 +359,7 @@ TEST_F(hid_bpf, test_hid_change_report) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -412,7 +389,7 @@ TEST_F(hid_bpf, test_hid_user_input_report_call) LOAD_BPF; - args.hid = self->hid_id; + args.hid = self->hid.hid_id; args.data[0] = 1; /* report ID */ args.data[1] = 2; /* report ID */ args.data[2] = 42; /* report ID */ @@ -458,7 +435,7 @@ TEST_F(hid_bpf, test_hid_user_output_report_call) LOAD_BPF; - args.hid = self->hid_id; + args.hid = self->hid.hid_id; args.data[0] = 1; /* report ID */ args.data[1] = 2; /* report ID */ args.data[2] = 42; /* report ID */ @@ -506,7 +483,7 @@ TEST_F(hid_bpf, test_hid_user_raw_request_call) LOAD_BPF; - args.hid = self->hid_id; + args.hid = self->hid.hid_id; args.data[0] = 1; /* report ID */ prog_fd = bpf_program__fd(self->skel->progs.hid_user_raw_request); @@ -539,7 +516,7 @@ TEST_F(hid_bpf, test_hid_filter_raw_request_call) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -565,7 +542,7 @@ TEST_F(hid_bpf, test_hid_filter_raw_request_call) /* detach the program */ detach_bpf(self); - self->hidraw_fd = open_hidraw(self->dev_id); + self->hidraw_fd = open_hidraw(&self->hid); ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw"); err = ioctl(self->hidraw_fd, HIDIOCGFEATURE(sizeof(buf)), buf); @@ -641,7 +618,7 @@ TEST_F(hid_bpf, test_hid_filter_output_report_call) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -667,7 +644,7 @@ TEST_F(hid_bpf, test_hid_filter_output_report_call) /* detach the program */ detach_bpf(self); - self->hidraw_fd = open_hidraw(self->dev_id); + self->hidraw_fd = open_hidraw(&self->hid); ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw"); err = write(self->hidraw_fd, buf, 3); @@ -742,7 +719,7 @@ TEST_F(hid_bpf, test_multiply_events_wq) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -780,7 +757,7 @@ TEST_F(hid_bpf, test_multiply_events) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -816,7 +793,7 @@ TEST_F(hid_bpf, test_hid_infinite_loop_input_report_call) buf[1] = 2; buf[2] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -867,7 +844,7 @@ TEST_F(hid_bpf, test_hid_attach_flags) /* inject one event */ buf[0] = 1; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selftests/hid/hid_common.h index f151f151a1ed..a7d836a35bb1 100644 --- a/tools/testing/selftests/hid/hid_common.h +++ b/tools/testing/selftests/hid/hid_common.h @@ -19,6 +19,13 @@ __typeof__(b) _b = (b); \ _a < _b ? _a : _b; }) +struct uhid_device { + int dev_id; /* uniq (random) number to identify the device */ + int uhid_fd; + int hid_id; /* HID device id in the system */ + pthread_t tid; /* thread for reading uhid events */ +}; + static unsigned char rdesc[] = { 0x06, 0x00, 0xff, /* Usage Page (Vendor Defined Page 1) */ 0x09, 0x21, /* Usage (Vendor Usage 0x21) */ @@ -146,14 +153,14 @@ static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb) return uhid_write(_metadata, fd, &ev); } -static void uhid_destroy(struct __test_metadata *_metadata, int fd) +static void uhid_destroy(struct __test_metadata *_metadata, struct uhid_device *hid) { struct uhid_event ev; memset(&ev, 0, sizeof(ev)); ev.type = UHID_DESTROY; - uhid_write(_metadata, fd, &ev); + uhid_write(_metadata, hid->uhid_fd, &ev); } static int uhid_event(struct __test_metadata *_metadata, int fd) @@ -281,7 +288,8 @@ static int uhid_start_listener(struct __test_metadata *_metadata, pthread_t *tid return 0; } -static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf, size_t size) +static int uhid_send_event(struct __test_metadata *_metadata, struct uhid_device *hid, + __u8 *buf, size_t size) { struct uhid_event ev; @@ -294,25 +302,7 @@ static int uhid_send_event(struct __test_metadata *_metadata, int fd, __u8 *buf, memcpy(ev.u.input2.data, buf, size); - return uhid_write(_metadata, fd, &ev); -} - -static int setup_uhid(struct __test_metadata *_metadata, int rand_nb) -{ - int fd; - const char *path = "/dev/uhid"; - int ret; - - fd = open(path, O_RDWR | O_CLOEXEC); - ASSERT_GE(fd, 0) TH_LOG("open uhid-cdev failed; %d", fd); - - ret = uhid_create(_metadata, fd, rand_nb); - ASSERT_EQ(0, ret) { - TH_LOG("create uhid device failed: %d", ret); - close(fd); - } - - return fd; + return uhid_write(_metadata, hid->uhid_fd, &ev); } static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir) @@ -421,12 +411,12 @@ static int get_hidraw(int dev_id) return found; } -static int open_hidraw(int dev_id) +static int open_hidraw(struct uhid_device *hid) { int hidraw_number; char hidraw_path[64] = { 0 }; - hidraw_number = get_hidraw(dev_id); + hidraw_number = get_hidraw(hid->dev_id); if (hidraw_number < 0) return hidraw_number; @@ -434,3 +424,39 @@ static int open_hidraw(int dev_id) sprintf(hidraw_path, "/dev/hidraw%d", hidraw_number); return open(hidraw_path, O_RDWR | O_NONBLOCK); } + +static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid) +{ + const char *path = "/dev/uhid"; + time_t t; + int ret; + + /* initialize random number generator */ + srand((unsigned int)time(&t)); + + hid->dev_id = rand() % 1024; + + hid->uhid_fd = open(path, O_RDWR | O_CLOEXEC); + ASSERT_GE(hid->uhid_fd, 0) TH_LOG("open uhid-cdev failed; %d", hid->uhid_fd); + + ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id); + ASSERT_EQ(0, ret) { + TH_LOG("create uhid device failed: %d", ret); + close(hid->uhid_fd); + return ret; + } + + /* locate the uevent file of the created device */ + hid->hid_id = get_hid_id(hid->dev_id); + ASSERT_GT(hid->hid_id, 0) + TH_LOG("Could not locate uhid device id: %d", hid->hid_id); + + ret = uhid_start_listener(_metadata, &hid->tid, hid->uhid_fd); + ASSERT_EQ(0, ret) { + TH_LOG("could not start udev listener: %d", ret); + close(hid->uhid_fd); + return ret; + } + + return 0; +} diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c index f8b4f7ff292c..5934818b2036 100644 --- a/tools/testing/selftests/hid/hidraw.c +++ b/tools/testing/selftests/hid/hidraw.c @@ -9,11 +9,8 @@ #endif /* HIDIOCREVOKE */ FIXTURE(hidraw) { - int dev_id; - int uhid_fd; + struct uhid_device hid; int hidraw_fd; - int hid_id; - pthread_t tid; }; static void close_hidraw(FIXTURE_DATA(hidraw) * self) { @@ -25,10 +22,10 @@ static void close_hidraw(FIXTURE_DATA(hidraw) * self) FIXTURE_TEARDOWN(hidraw) { void *uhid_err; - uhid_destroy(_metadata, self->uhid_fd); + uhid_destroy(_metadata, &self->hid); close_hidraw(self); - pthread_join(self->tid, &uhid_err); + pthread_join(self->hid.tid, &uhid_err); } #define TEARDOWN_LOG(fmt, ...) do { \ TH_LOG(fmt, ##__VA_ARGS__); \ @@ -37,25 +34,12 @@ FIXTURE_TEARDOWN(hidraw) { FIXTURE_SETUP(hidraw) { - time_t t; int err; - /* initialize random number generator */ - srand((unsigned int)time(&t)); + err = setup_uhid(_metadata, &self->hid); + ASSERT_OK(err); - self->dev_id = rand() % 1024; - - self->uhid_fd = setup_uhid(_metadata, self->dev_id); - - /* locate the uev, self, variant);ent file of the created device */ - self->hid_id = get_hid_id(self->dev_id); - ASSERT_GT(self->hid_id, 0) - TEARDOWN_LOG("Could not locate uhid device id: %d", self->hid_id); - - err = uhid_start_listener(_metadata, &self->tid, self->uhid_fd); - ASSERT_EQ(0, err) TEARDOWN_LOG("could not start udev listener: %d", err); - - self->hidraw_fd = open_hidraw(self->dev_id); + self->hidraw_fd = open_hidraw(&self->hid); ASSERT_GE(self->hidraw_fd, 0) TH_LOG("open_hidraw"); } @@ -79,7 +63,7 @@ TEST_F(hidraw, raw_event) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -101,7 +85,7 @@ TEST_F(hidraw, raw_event_revoked) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -117,7 +101,7 @@ TEST_F(hidraw, raw_event_revoked) /* inject one other event */ buf[0] = 1; buf[1] = 43; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); /* read the data from hidraw */ memset(buf, 0, sizeof(buf)); @@ -161,7 +145,7 @@ TEST_F(hidraw, poll_revoked) /* inject one event */ buf[0] = 1; buf[1] = 42; - uhid_send_event(_metadata, self->uhid_fd, buf, 6); + uhid_send_event(_metadata, &self->hid, buf, 6); while (true) { ready = poll(pfds, 1, 5000); From 72c55473fc8c82b06df79473f04c5231d51580d7 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:11 +0200 Subject: [PATCH 10/40] selftests/hid: allow to parametrize bus/vid/pid/rdesc on the test device This will be useful to introduce variants in tests to test the interactions between HID-BPF and some kernel modules. Reviewed-by: Peter Hutterer Acked-by: Shuah Khan Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-7-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/hid_bpf.c | 2 +- tools/testing/selftests/hid/hid_common.h | 46 +++++++++++++++--------- tools/testing/selftests/hid/hidraw.c | 2 +- 3 files changed, 31 insertions(+), 19 deletions(-) diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c index 31ff92e0debd..1e979fb3542b 100644 --- a/tools/testing/selftests/hid/hid_bpf.c +++ b/tools/testing/selftests/hid/hid_bpf.c @@ -58,7 +58,7 @@ FIXTURE_SETUP(hid_bpf) { int err; - err = setup_uhid(_metadata, &self->hid); + err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc)); ASSERT_OK(err); } diff --git a/tools/testing/selftests/hid/hid_common.h b/tools/testing/selftests/hid/hid_common.h index a7d836a35bb1..f77f69c6657d 100644 --- a/tools/testing/selftests/hid/hid_common.h +++ b/tools/testing/selftests/hid/hid_common.h @@ -23,6 +23,9 @@ struct uhid_device { int dev_id; /* uniq (random) number to identify the device */ int uhid_fd; int hid_id; /* HID device id in the system */ + __u16 bus; + __u32 vid; + __u32 pid; pthread_t tid; /* thread for reading uhid events */ }; @@ -129,7 +132,9 @@ static int uhid_write(struct __test_metadata *_metadata, int fd, const struct uh } } -static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb) +static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb, + __u16 bus, __u32 vid, __u32 pid, __u8 *rdesc, + size_t rdesc_size) { struct uhid_event ev; char buf[25]; @@ -140,10 +145,10 @@ static int uhid_create(struct __test_metadata *_metadata, int fd, int rand_nb) ev.type = UHID_CREATE; strcpy((char *)ev.u.create.name, buf); ev.u.create.rd_data = rdesc; - ev.u.create.rd_size = sizeof(rdesc); - ev.u.create.bus = BUS_USB; - ev.u.create.vendor = 0x0001; - ev.u.create.product = 0x0a37; + ev.u.create.rd_size = rdesc_size; + ev.u.create.bus = bus; + ev.u.create.vendor = vid; + ev.u.create.product = pid; ev.u.create.version = 0; ev.u.create.country = 0; @@ -305,15 +310,17 @@ static int uhid_send_event(struct __test_metadata *_metadata, struct uhid_device return uhid_write(_metadata, hid->uhid_fd, &ev); } -static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *dir) +static bool match_sysfs_device(struct uhid_device *hid, const char *workdir, struct dirent *dir) { - const char *target = "0003:0001:0A37.*"; + char target[20] = ""; char phys[512]; char uevent[1024]; char temp[512]; int fd, nread; bool found = false; + snprintf(target, sizeof(target), "%04X:%04X:%04X.*", hid->bus, hid->vid, hid->pid); + if (fnmatch(target, dir->d_name, 0)) return false; @@ -324,7 +331,7 @@ static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *d if (fd < 0) return false; - sprintf(phys, "PHYS=%d", dev_id); + sprintf(phys, "PHYS=%d", hid->dev_id); nread = read(fd, temp, ARRAY_SIZE(temp)); if (nread > 0 && (strstr(temp, phys)) != NULL) @@ -335,7 +342,7 @@ static bool match_sysfs_device(int dev_id, const char *workdir, struct dirent *d return found; } -static int get_hid_id(int dev_id) +static int get_hid_id(struct uhid_device *hid) { const char *workdir = "/sys/devices/virtual/misc/uhid"; const char *str_id; @@ -350,10 +357,10 @@ static int get_hid_id(int dev_id) d = opendir(workdir); if (d) { while ((dir = readdir(d)) != NULL) { - if (!match_sysfs_device(dev_id, workdir, dir)) + if (!match_sysfs_device(hid, workdir, dir)) continue; - str_id = dir->d_name + sizeof("0003:0001:0A37."); + str_id = dir->d_name + sizeof("0000:0000:0000."); found = (int)strtol(str_id, NULL, 16); break; @@ -367,7 +374,7 @@ static int get_hid_id(int dev_id) return found; } -static int get_hidraw(int dev_id) +static int get_hidraw(struct uhid_device *hid) { const char *workdir = "/sys/devices/virtual/misc/uhid"; char sysfs[1024]; @@ -384,7 +391,7 @@ static int get_hidraw(int dev_id) continue; while ((dir = readdir(d)) != NULL) { - if (!match_sysfs_device(dev_id, workdir, dir)) + if (!match_sysfs_device(hid, workdir, dir)) continue; sprintf(sysfs, "%s/%s/hidraw", workdir, dir->d_name); @@ -416,7 +423,7 @@ static int open_hidraw(struct uhid_device *hid) int hidraw_number; char hidraw_path[64] = { 0 }; - hidraw_number = get_hidraw(hid->dev_id); + hidraw_number = get_hidraw(hid); if (hidraw_number < 0) return hidraw_number; @@ -425,7 +432,8 @@ static int open_hidraw(struct uhid_device *hid) return open(hidraw_path, O_RDWR | O_NONBLOCK); } -static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid) +static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid, + __u16 bus, __u32 vid, __u32 pid, const __u8 *rdesc, size_t rdesc_size) { const char *path = "/dev/uhid"; time_t t; @@ -435,11 +443,15 @@ static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid srand((unsigned int)time(&t)); hid->dev_id = rand() % 1024; + hid->bus = bus; + hid->vid = vid; + hid->pid = pid; hid->uhid_fd = open(path, O_RDWR | O_CLOEXEC); ASSERT_GE(hid->uhid_fd, 0) TH_LOG("open uhid-cdev failed; %d", hid->uhid_fd); - ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id); + ret = uhid_create(_metadata, hid->uhid_fd, hid->dev_id, bus, vid, pid, + (__u8 *)rdesc, rdesc_size); ASSERT_EQ(0, ret) { TH_LOG("create uhid device failed: %d", ret); close(hid->uhid_fd); @@ -447,7 +459,7 @@ static int setup_uhid(struct __test_metadata *_metadata, struct uhid_device *hid } /* locate the uevent file of the created device */ - hid->hid_id = get_hid_id(hid->dev_id); + hid->hid_id = get_hid_id(hid); ASSERT_GT(hid->hid_id, 0) TH_LOG("Could not locate uhid device id: %d", hid->hid_id); diff --git a/tools/testing/selftests/hid/hidraw.c b/tools/testing/selftests/hid/hidraw.c index 5934818b2036..821db37ba4bb 100644 --- a/tools/testing/selftests/hid/hidraw.c +++ b/tools/testing/selftests/hid/hidraw.c @@ -36,7 +36,7 @@ FIXTURE_SETUP(hidraw) { int err; - err = setup_uhid(_metadata, &self->hid); + err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a37, rdesc, sizeof(rdesc)); ASSERT_OK(err); self->hidraw_fd = open_hidraw(&self->hid); From 645c224ac5f6e0013931c342ea707b398d24d410 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:12 +0200 Subject: [PATCH 11/40] HID: add per device quirk to force bind to hid-generic We already have the possibility to force not binding to hid-generic and rely on a dedicated driver, but we couldn't do the other way around. This is useful for BPF programs where we are fixing the report descriptor and the events, but want to avoid a specialized driver to come after BPF which would unwind everything that is done there. Reviewed-by: Peter Hutterer Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-8-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-core.c | 5 +++-- drivers/hid/hid-generic.c | 3 +++ include/linux/hid.h | 2 ++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 8e879937e956..f1c23b21e96a 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -2698,9 +2698,10 @@ static bool hid_check_device_match(struct hid_device *hdev, /* * hid-generic implements .match(), so we must be dealing with a * different HID driver here, and can simply check if - * hid_ignore_special_drivers is set or not. + * hid_ignore_special_drivers or HID_QUIRK_IGNORE_SPECIAL_DRIVER + * are set or not. */ - return !hid_ignore_special_drivers; + return !hid_ignore_special_drivers && !(hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER); } static int __hid_device_probe(struct hid_device *hdev, struct hid_driver *hdrv) diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c index f9db991d3c5a..88882c1bfffe 100644 --- a/drivers/hid/hid-generic.c +++ b/drivers/hid/hid-generic.c @@ -40,6 +40,9 @@ static bool hid_generic_match(struct hid_device *hdev, if (ignore_special_driver) return true; + if (hdev->quirks & HID_QUIRK_IGNORE_SPECIAL_DRIVER) + return true; + if (hdev->quirks & HID_QUIRK_HAVE_SPECIAL_DRIVER) return false; diff --git a/include/linux/hid.h b/include/linux/hid.h index ff58b5ceb62e..63330d623335 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -359,6 +359,7 @@ struct hid_item { * | @HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP: * | @HID_QUIRK_HAVE_SPECIAL_DRIVER: * | @HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE: + * | @HID_QUIRK_IGNORE_SPECIAL_DRIVER * | @HID_QUIRK_FULLSPEED_INTERVAL: * | @HID_QUIRK_NO_INIT_REPORTS: * | @HID_QUIRK_NO_IGNORE: @@ -384,6 +385,7 @@ struct hid_item { #define HID_QUIRK_HAVE_SPECIAL_DRIVER BIT(19) #define HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE BIT(20) #define HID_QUIRK_NOINVERT BIT(21) +#define HID_QUIRK_IGNORE_SPECIAL_DRIVER BIT(22) #define HID_QUIRK_FULLSPEED_INTERVAL BIT(28) #define HID_QUIRK_NO_INIT_REPORTS BIT(29) #define HID_QUIRK_NO_IGNORE BIT(30) From e14e0eaeb040899f7cb363cdfdf8fbee84a45f08 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Tue, 1 Oct 2024 16:30:13 +0200 Subject: [PATCH 12/40] selftests/hid: add test for assigning a given device to hid-generic We use a well known VID/PID on a driver that doesn't need to talk to the device, ensures we created the device against the target driver, then load our program and ensure we have unbound to this driver and use hid-generic instead. Reviewed-by: Peter Hutterer Acked-by: Shuah Khan Link: https://patch.msgid.link/20241001-hid-bpf-hid-generic-v3-9-2ef1019468df@kernel.org Signed-off-by: Benjamin Tissoires --- tools/testing/selftests/hid/hid_bpf.c | 80 ++++++++++++++++++- tools/testing/selftests/hid/progs/hid.c | 12 +++ .../selftests/hid/progs/hid_bpf_helpers.h | 6 +- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/hid/hid_bpf.c b/tools/testing/selftests/hid/hid_bpf.c index 1e979fb3542b..ca58bfa3ca65 100644 --- a/tools/testing/selftests/hid/hid_bpf.c +++ b/tools/testing/selftests/hid/hid_bpf.c @@ -54,11 +54,41 @@ FIXTURE_TEARDOWN(hid_bpf) { hid_bpf_teardown(_metadata, self, variant); \ } while (0) +struct specific_device { + const char test_name[64]; + __u16 bus; + __u32 vid; + __u32 pid; +}; + FIXTURE_SETUP(hid_bpf) { + const struct specific_device *match = NULL; int err; - err = setup_uhid(_metadata, &self->hid, BUS_USB, 0x0001, 0x0a36, rdesc, sizeof(rdesc)); + const struct specific_device devices[] = { + { + .test_name = "test_hid_driver_probe", + .bus = BUS_BLUETOOTH, + .vid = 0x05ac, /* USB_VENDOR_ID_APPLE */ + .pid = 0x022c, /* USB_DEVICE_ID_APPLE_ALU_WIRELESS_ANSI */ + }, { + .test_name = "*", + .bus = BUS_USB, + .vid = 0x0001, + .pid = 0x0a36, + }}; + + for (int i = 0; i < ARRAY_SIZE(devices); i++) { + match = &devices[i]; + if (!strncmp(_metadata->name, devices[i].test_name, sizeof(devices[i].test_name))) + break; + } + + ASSERT_OK_PTR(match); + + err = setup_uhid(_metadata, &self->hid, match->bus, match->vid, match->pid, + rdesc, sizeof(rdesc)); ASSERT_OK(err); } @@ -855,6 +885,54 @@ TEST_F(hid_bpf, test_hid_attach_flags) ASSERT_EQ(buf[3], 3); } +static bool is_using_driver(struct __test_metadata *_metadata, struct uhid_device *hid, + const char *driver) +{ + char driver_line[512]; + char uevent[1024]; + char temp[512]; + int fd, nread; + bool found = false; + + sprintf(uevent, "/sys/bus/hid/devices/%04X:%04X:%04X.%04X/uevent", + hid->bus, hid->vid, hid->pid, hid->hid_id); + + fd = open(uevent, O_RDONLY | O_NONBLOCK); + if (fd < 0) { + TH_LOG("couldn't open '%s': %d, %d", uevent, fd, errno); + return false; + } + + sprintf(driver_line, "DRIVER=%s", driver); + + nread = read(fd, temp, ARRAY_SIZE(temp)); + if (nread > 0 && (strstr(temp, driver_line)) != NULL) + found = true; + + close(fd); + + return found; +} + +/* + * Attach hid_driver_probe to the given uhid device, + * check that the device is now using hid-generic. + */ +TEST_F(hid_bpf, test_hid_driver_probe) +{ + const struct test_program progs[] = { + { + .name = "hid_test_driver_probe", + }, + }; + + ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "apple")); + + LOAD_PROGRAMS(progs); + + ASSERT_TRUE(is_using_driver(_metadata, &self->hid, "hid-generic")); +} + /* * Attach hid_rdesc_fixup to the given uhid device, * retrieve and open the matching hidraw node, diff --git a/tools/testing/selftests/hid/progs/hid.c b/tools/testing/selftests/hid/progs/hid.c index 5ecc845ef792..9b22e9a0e658 100644 --- a/tools/testing/selftests/hid/progs/hid.c +++ b/tools/testing/selftests/hid/progs/hid.c @@ -598,3 +598,15 @@ SEC(".struct_ops.link") struct hid_bpf_ops test_infinite_loop_input_report = { .hid_device_event = (void *)hid_test_infinite_loop_input_report, }; + +SEC("?struct_ops.s/hid_rdesc_fixup") +int BPF_PROG(hid_test_driver_probe, struct hid_bpf_ctx *hid_ctx) +{ + hid_ctx->hid->quirks |= HID_QUIRK_IGNORE_SPECIAL_DRIVER; + return 0; +} + +SEC(".struct_ops.link") +struct hid_bpf_ops test_driver_probe = { + .hid_rdesc_fixup = (void *)hid_test_driver_probe, +}; diff --git a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h index e5db897586bb..1a645684a117 100644 --- a/tools/testing/selftests/hid/progs/hid_bpf_helpers.h +++ b/tools/testing/selftests/hid/progs/hid_bpf_helpers.h @@ -84,10 +84,14 @@ struct hid_bpf_ops { struct hid_device *hdev; }; +#define BIT(n) (1U << n) + #ifndef BPF_F_BEFORE -#define BPF_F_BEFORE (1U << 3) +#define BPF_F_BEFORE BIT(3) #endif +#define HID_QUIRK_IGNORE_SPECIAL_DRIVER BIT(22) + /* following are kfuncs exported by HID for HID-BPF */ extern __u8 *hid_bpf_get_data(struct hid_bpf_ctx *ctx, unsigned int offset, From fb6c0583a1435ace7242d3763ea0dde93522c727 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Mon, 16 Sep 2024 13:55:27 +0200 Subject: [PATCH 13/40] HID: logitech-hidpp: Remove feature_type from hidpp_root_get_feature() Nobody uses that variable after it gets assigned, so this saves us from having to declare it in the first place. Signed-off-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-logitech-hidpp.c | 66 +++++++++----------------------- 1 file changed, 19 insertions(+), 47 deletions(-) diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 0e33fa0eb8db..d29f6598d855 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -928,7 +928,7 @@ static int hidpp_unifying_init(struct hidpp_device *hidpp) #define CMD_ROOT_GET_PROTOCOL_VERSION 0x10 static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature, - u8 *feature_index, u8 *feature_type) + u8 *feature_index) { struct hidpp_report response; int ret; @@ -945,7 +945,6 @@ static int hidpp_root_get_feature(struct hidpp_device *hidpp, u16 feature, return -ENOENT; *feature_index = response.fap.params[0]; - *feature_type = response.fap.params[1]; return ret; } @@ -1012,13 +1011,11 @@ print_version: static int hidpp_get_serial(struct hidpp_device *hidpp, u32 *serial) { struct hidpp_report response; - u8 feature_type; u8 feature_index; int ret; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_DEVICE_INFORMATION, - &feature_index, - &feature_type); + &feature_index); if (ret) return ret; @@ -1125,7 +1122,6 @@ static int hidpp_devicenametype_get_device_name(struct hidpp_device *hidpp, static char *hidpp_get_device_name(struct hidpp_device *hidpp) { - u8 feature_type; u8 feature_index; u8 __name_length; char *name; @@ -1133,7 +1129,7 @@ static char *hidpp_get_device_name(struct hidpp_device *hidpp) int ret; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_GET_DEVICE_NAME_TYPE, - &feature_index, &feature_type); + &feature_index); if (ret) return NULL; @@ -1300,15 +1296,13 @@ static int hidpp20_batterylevel_get_battery_info(struct hidpp_device *hidpp, static int hidpp20_query_battery_info_1000(struct hidpp_device *hidpp) { - u8 feature_type; int ret; int status, capacity, next_capacity, level; if (hidpp->battery.feature_index == 0xff) { ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_BATTERY_LEVEL_STATUS, - &hidpp->battery.feature_index, - &feature_type); + &hidpp->battery.feature_index); if (ret) return ret; } @@ -1489,14 +1483,12 @@ static int hidpp20_map_battery_capacity(struct hid_device *hid_dev, int voltage) static int hidpp20_query_battery_voltage_info(struct hidpp_device *hidpp) { - u8 feature_type; int ret; int status, voltage, level, charge_type; if (hidpp->battery.voltage_feature_index == 0xff) { ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_BATTERY_VOLTAGE, - &hidpp->battery.voltage_feature_index, - &feature_type); + &hidpp->battery.voltage_feature_index); if (ret) return ret; } @@ -1692,7 +1684,6 @@ static int hidpp20_unifiedbattery_get_status(struct hidpp_device *hidpp, static int hidpp20_query_battery_info_1004(struct hidpp_device *hidpp) { - u8 feature_type; int ret; u8 state_of_charge; int status, level; @@ -1700,8 +1691,7 @@ static int hidpp20_query_battery_info_1004(struct hidpp_device *hidpp) if (hidpp->battery.feature_index == 0xff) { ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_UNIFIED_BATTERY, - &hidpp->battery.feature_index, - &feature_type); + &hidpp->battery.feature_index); if (ret) return ret; } @@ -1834,14 +1824,9 @@ static int hidpp_battery_get_property(struct power_supply *psy, static int hidpp_get_wireless_feature_index(struct hidpp_device *hidpp, u8 *feature_index) { - u8 feature_type; - int ret; - - ret = hidpp_root_get_feature(hidpp, - HIDPP_PAGE_WIRELESS_DEVICE_STATUS, - feature_index, &feature_type); - - return ret; + return hidpp_root_get_feature(hidpp, + HIDPP_PAGE_WIRELESS_DEVICE_STATUS, + feature_index); } /* -------------------------------------------------------------------------- */ @@ -1952,14 +1937,11 @@ static bool hidpp20_get_adc_measurement_1f20(struct hidpp_device *hidpp, static int hidpp20_query_adc_measurement_info_1f20(struct hidpp_device *hidpp) { - u8 feature_type; - if (hidpp->battery.adc_measurement_feature_index == 0xff) { int ret; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_ADC_MEASUREMENT, - &hidpp->battery.adc_measurement_feature_index, - &feature_type); + &hidpp->battery.adc_measurement_feature_index); if (ret) return ret; @@ -2014,15 +1996,13 @@ static int hidpp_hrs_set_highres_scrolling_mode(struct hidpp_device *hidpp, bool enabled, u8 *multiplier) { u8 feature_index; - u8 feature_type; int ret; u8 params[1]; struct hidpp_report response; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HI_RESOLUTION_SCROLLING, - &feature_index, - &feature_type); + &feature_index); if (ret) return ret; @@ -2049,12 +2029,11 @@ static int hidpp_hrw_get_wheel_capability(struct hidpp_device *hidpp, u8 *multiplier) { u8 feature_index; - u8 feature_type; int ret; struct hidpp_report response; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, - &feature_index, &feature_type); + &feature_index); if (ret) goto return_default; @@ -2076,13 +2055,12 @@ static int hidpp_hrw_set_wheel_mode(struct hidpp_device *hidpp, bool invert, bool high_resolution, bool use_hidpp) { u8 feature_index; - u8 feature_type; int ret; u8 params[1]; struct hidpp_report response; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, - &feature_index, &feature_type); + &feature_index); if (ret) return ret; @@ -2111,14 +2089,12 @@ static int hidpp_solar_request_battery_event(struct hidpp_device *hidpp) { struct hidpp_report response; u8 params[2] = { 1, 1 }; - u8 feature_type; int ret; if (hidpp->battery.feature_index == 0xff) { ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_SOLAR_KEYBOARD, - &hidpp->battery.solar_feature_index, - &feature_type); + &hidpp->battery.solar_feature_index); if (ret) return ret; } @@ -3098,11 +3074,10 @@ static int wtp_get_config(struct hidpp_device *hidpp) { struct wtp_data *wd = hidpp->private_data; struct hidpp_touchpad_raw_info raw_info = {0}; - u8 feature_type; int ret; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_TOUCHPAD_RAW_XY, - &wd->mt_feature_index, &feature_type); + &wd->mt_feature_index); if (ret) /* means that the device is not powered up */ return ret; @@ -3362,12 +3337,11 @@ static int k400_disable_tap_to_click(struct hidpp_device *hidpp) struct k400_private_data *k400 = hidpp->private_data; struct hidpp_touchpad_fw_items items = {}; int ret; - u8 feature_type; if (!k400->feature_index) { ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_TOUCHPAD_FW_ITEMS, - &k400->feature_index, &feature_type); + &k400->feature_index); if (ret) /* means that the device is not powered up */ return ret; @@ -3439,14 +3413,13 @@ static int g920_get_config(struct hidpp_device *hidpp, struct hidpp_ff_private_data *data) { struct hidpp_report response; - u8 feature_type; int ret; memset(data, 0, sizeof(*data)); /* Find feature and store for later use */ ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_G920_FORCE_FEEDBACK, - &data->feature_index, &feature_type); + &data->feature_index); if (ret) return ret; @@ -3735,17 +3708,16 @@ static int hidpp_initialize_hires_scroll(struct hidpp_device *hidpp) if (hidpp->protocol_major >= 2) { u8 feature_index; - u8 feature_type; ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HIRES_WHEEL, - &feature_index, &feature_type); + &feature_index); if (!ret) { hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL; hid_dbg(hidpp->hid_dev, "Detected HID++ 2.0 hi-res scroll wheel\n"); return 0; } ret = hidpp_root_get_feature(hidpp, HIDPP_PAGE_HI_RESOLUTION_SCROLLING, - &feature_index, &feature_type); + &feature_index); if (!ret) { hidpp->capabilities |= HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL; hid_dbg(hidpp->hid_dev, "Detected HID++ 2.0 hi-res scrolling\n"); From 4005667d3a09d832236c8d0ba598e510b6a20560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= Date: Fri, 20 Sep 2024 17:34:28 +0200 Subject: [PATCH 14/40] HID: i2c-hid-of: Drop explicit initialization of struct i2c_device_id::driver_data to 0 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These drivers don't use the driver_data member of struct i2c_device_id, so don't explicitly initialize this member. This prepares putting driver_data in an anonymous union which requires either no initialization or named designators. But it's also a nice cleanup on its own. While touching the initializer, also remove the comma after the sentinel entry. Signed-off-by: Uwe Kleine-König Reviewed-by: Douglas Anderson Signed-off-by: Jiri Kosina --- drivers/hid/i2c-hid/i2c-hid-of.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/hid/i2c-hid/i2c-hid-of.c b/drivers/hid/i2c-hid/i2c-hid-of.c index 8be4d576da77..57379b77e977 100644 --- a/drivers/hid/i2c-hid/i2c-hid-of.c +++ b/drivers/hid/i2c-hid/i2c-hid-of.c @@ -144,9 +144,9 @@ MODULE_DEVICE_TABLE(of, i2c_hid_of_match); #endif static const struct i2c_device_id i2c_hid_of_id_table[] = { - { "hid", 0 }, - { "hid-over-i2c", 0 }, - { }, + { "hid" }, + { "hid-over-i2c" }, + { } }; MODULE_DEVICE_TABLE(i2c, i2c_hid_of_id_table); From 63cafaf47a834fa15d80238f9d8181d32931be17 Mon Sep 17 00:00:00 2001 From: Erick Archer Date: Sun, 22 Sep 2024 17:22:53 -0700 Subject: [PATCH 15/40] HID: ishtp-hid-client: replace fake-flex arrays with flex-array members One-element arrays as fake flex arrays are deprecated[1] as the kernel has switched to C99 flexible-array members instead. This case, however, has more complexity because it is a flexible array of flexible arrays and this patch needs to be ready to enable the new compiler flag -Wflex-array-member-not-at-end (coming in GCC-14) globally. So, define a new struct type for the single reports: struct report { uint16_t size; struct hostif_msg_hdr msg; } __packed; but without the payload (flex array) in it. And add this payload to the "hostif_msg" structure. This way, in the "report_list" structure we can declare a flex array of single reports which now do not contain another flex array. struct report_list { [...] struct report reports[]; } __packed; Therefore, the "struct hostif_msg" is now made up of a header and a payload. And the "struct report" uses only the "hostif_msg" header. The perfect solution would be for the "report" structure to use the whole "hostif_msg" structure but this is not possible due to nested flexible arrays. Anyway, the end result is equivalent since this patch does attempt to change the behaviour of the code. Now as well, we have more clarity after the cast from the raw bytes to the new structures. Refactor the code accordingly to use the new structures. Also, use "container_of()" whenever we need to retrieve a pointer to the flexible structure, through which we can access the flexible array if needed. Link: https://www.kernel.org/doc/html/next/process/deprecated.html#zero-length-and-one-element-arrays [1] Closes: https://github.com/KSPP/linux/issues/333 Signed-off-by: Erick Archer Link: https://lore.kernel.org/r/AS8PR02MB723760CB93942370E92F00638BF72@AS8PR02MB7237.eurprd02.prod.outlook.com [kees: tweaked commit log and dropped struct_size() uses] Signed-off-by: Kees Cook Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ishtp-hid-client.c | 25 ++++++++++---------- drivers/hid/intel-ish-hid/ishtp-hid.h | 11 +++++---- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/drivers/hid/intel-ish-hid/ishtp-hid-client.c b/drivers/hid/intel-ish-hid/ishtp-hid-client.c index fbd4f8ea1951..cb04cd1d980b 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid-client.c +++ b/drivers/hid/intel-ish-hid/ishtp-hid-client.c @@ -70,10 +70,10 @@ static void process_recv(struct ishtp_cl *hid_ishtp_cl, void *recv_buf, unsigned char *payload; struct device_info *dev_info; int i, j; - size_t payload_len, total_len, cur_pos, raw_len; + size_t payload_len, total_len, cur_pos, raw_len, msg_len; int report_type; struct report_list *reports_list; - char *reports; + struct report *report; size_t report_len; struct ishtp_cl_data *client_data = ishtp_get_client_data(hid_ishtp_cl); int curr_hid_dev = client_data->cur_hid_dev; @@ -280,14 +280,13 @@ do_get_report: case HOSTIF_PUBLISH_INPUT_REPORT_LIST: report_type = HID_INPUT_REPORT; reports_list = (struct report_list *)payload; - reports = (char *)reports_list->reports; + report = reports_list->reports; for (j = 0; j < reports_list->num_of_reports; j++) { - recv_msg = (struct hostif_msg *)(reports + - sizeof(uint16_t)); - report_len = *(uint16_t *)reports; - payload = reports + sizeof(uint16_t) + - sizeof(struct hostif_msg_hdr); + recv_msg = container_of(&report->msg, + struct hostif_msg, hdr); + report_len = report->size; + payload = recv_msg->payload; payload_len = report_len - sizeof(struct hostif_msg_hdr); @@ -304,7 +303,7 @@ do_get_report: 0); } - reports += sizeof(uint16_t) + report_len; + report += sizeof(*report) + payload_len; } break; default: @@ -316,12 +315,12 @@ do_get_report: } - if (!cur_pos && cur_pos + payload_len + - sizeof(struct hostif_msg) < total_len) + msg_len = payload_len + sizeof(struct hostif_msg); + if (!cur_pos && cur_pos + msg_len < total_len) ++client_data->multi_packet_cnt; - cur_pos += payload_len + sizeof(struct hostif_msg); - payload += payload_len + sizeof(struct hostif_msg); + cur_pos += msg_len; + payload += msg_len; } while (cur_pos < total_len); } diff --git a/drivers/hid/intel-ish-hid/ishtp-hid.h b/drivers/hid/intel-ish-hid/ishtp-hid.h index 35dddc5015b3..2bc19e8ba13e 100644 --- a/drivers/hid/intel-ish-hid/ishtp-hid.h +++ b/drivers/hid/intel-ish-hid/ishtp-hid.h @@ -31,6 +31,7 @@ struct hostif_msg_hdr { struct hostif_msg { struct hostif_msg_hdr hdr; + uint8_t payload[]; } __packed; struct hostif_msg_to_sensor { @@ -52,15 +53,17 @@ struct ishtp_version { uint16_t build; } __packed; +struct report { + uint16_t size; + struct hostif_msg_hdr msg; +} __packed; + /* struct for ISHTP aggregated input data */ struct report_list { uint16_t total_size; uint8_t num_of_reports; uint8_t flags; - struct { - uint16_t size_of_report; - uint8_t report[1]; - } __packed reports[1]; + struct report reports[]; } __packed; /* HOSTIF commands */ From aa68d2bd9befe8615852b0ab41b3bd5abeae0979 Mon Sep 17 00:00:00 2001 From: Yan Zhen Date: Tue, 24 Sep 2024 19:50:05 +0800 Subject: [PATCH 16/40] HID: Fix typo in the comment Correctly spelled comments make it easier for the reader to understand the code. Fix typos: 'mninum' -> 'minimum', 'destoyed' -> 'destroyed', 'thridparty' -> 'thirdparty', 'lowcase' -> 'lowercase', 'idenitifiers' -> 'identifiers', 'exeuction' -> 'execution', 'fregments' -> 'fragments', 'devides' -> 'devices'. Signed-off-by: Yan Zhen Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/hid-asus.c | 2 +- drivers/hid/hid-logitech-hidpp.c | 2 +- drivers/hid/hid-picolcd_fb.c | 2 +- drivers/hid/hid-sensor-custom.c | 2 +- drivers/hid/hid-steam.c | 2 +- drivers/hid/intel-ish-hid/ishtp-fw-loader.c | 2 +- drivers/hid/intel-ish-hid/ishtp/client.c | 2 +- drivers/hid/usbhid/hid-core.c | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c index a4b47319ad8e..506c6f377e7d 100644 --- a/drivers/hid/hid-asus.c +++ b/drivers/hid/hid-asus.c @@ -1183,7 +1183,7 @@ static const __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc, if (drvdata->quirks & QUIRK_G752_KEYBOARD && *rsize == 75 && rdesc[61] == 0x15 && rdesc[62] == 0x00) { - /* report is missing usage mninum and maximum */ + /* report is missing usage minimum and maximum */ __u8 *new_rdesc; size_t new_size = *rsize + sizeof(asus_g752_fixed_rdesc); diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 0d114268ade8..a5ae48c6f03b 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -2522,7 +2522,7 @@ static void hidpp_ff_work_handler(struct work_struct *w) /* regular effect destroyed */ data->effect_ids[wd->params[0]-1] = -1; else if (wd->effect_id >= HIDPP_FF_EFFECTID_AUTOCENTER) - /* autocenter spring destoyed */ + /* autocenter spring destroyed */ data->slot_autocenter = 0; break; case HIDPP_FF_SET_GLOBAL_GAINS: diff --git a/drivers/hid/hid-picolcd_fb.c b/drivers/hid/hid-picolcd_fb.c index 83e3409d170c..f8b16a82faef 100644 --- a/drivers/hid/hid-picolcd_fb.c +++ b/drivers/hid/hid-picolcd_fb.c @@ -296,7 +296,7 @@ static void picolcd_fb_destroy(struct fb_info *info) /* make sure no work is deferred */ fb_deferred_io_cleanup(info); - /* No thridparty should ever unregister our framebuffer! */ + /* No thirdparty should ever unregister our framebuffer! */ WARN_ON(fbdata->picolcd != NULL); vfree((u8 *)info->fix.smem_start); diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index 66f0675df24b..617ae240396d 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -946,7 +946,7 @@ hid_sensor_register_platform_device(struct platform_device *pdev, memcpy(real_usage, match->luid, 4); - /* usage id are all lowcase */ + /* usage id are all lowercase */ for (c = real_usage; *c != '\0'; c++) *c = tolower(*c); diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index bf8b633114be..6439913372a8 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -253,7 +253,7 @@ enum ID_CONTROLLER_DECK_STATE = 9 }; -/* String attribute idenitifiers */ +/* String attribute identifiers */ enum { ATTRIB_STR_BOARD_SERIAL, ATTRIB_STR_UNIT_SERIAL, diff --git a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c index e157863a8b25..750bfdd26ddb 100644 --- a/drivers/hid/intel-ish-hid/ishtp-fw-loader.c +++ b/drivers/hid/intel-ish-hid/ishtp-fw-loader.c @@ -793,7 +793,7 @@ static int load_fw_from_host(struct ishtp_cl_data *client_data) if (rv < 0) goto end_err_fw_release; - /* Step 3: Start ISH main firmware exeuction */ + /* Step 3: Start ISH main firmware execution */ rv = ish_fw_start(client_data); if (rv < 0) diff --git a/drivers/hid/intel-ish-hid/ishtp/client.c b/drivers/hid/intel-ish-hid/ishtp/client.c index 8a7f2f6a4f86..e61b01e9902e 100644 --- a/drivers/hid/intel-ish-hid/ishtp/client.c +++ b/drivers/hid/intel-ish-hid/ishtp/client.c @@ -863,7 +863,7 @@ static void ipc_tx_send(void *prm) /* Send ipc fragment */ ishtp_hdr.length = dev->mtu; ishtp_hdr.msg_complete = 0; - /* All fregments submitted to IPC queue with no callback */ + /* All fragments submitted to IPC queue with no callback */ ishtp_write_message(dev, &ishtp_hdr, pmsg); cl->tx_offs += dev->mtu; rem = cl_msg->send_buf.size - cl->tx_offs; diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index cb687ea7325c..956b86737b07 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1100,7 +1100,7 @@ static int usbhid_start(struct hid_device *hid) interval = endpoint->bInterval; - /* Some vendors give fullspeed interval on highspeed devides */ + /* Some vendors give fullspeed interval on highspeed devices */ if (hid->quirks & HID_QUIRK_FULLSPEED_INTERVAL && dev->speed == USB_SPEED_HIGH) { interval = fls(endpoint->bInterval*8); From 6ea2a6fd3872e60a4d500b548ad65ed94e459ddd Mon Sep 17 00:00:00 2001 From: Stuart Hayhurst Date: Wed, 9 Oct 2024 00:30:29 +0100 Subject: [PATCH 17/40] HID: corsair-void: Add Corsair Void headset family driver Introduce a driver for the Corsair Void family of headsets, supporting: - Battery reporting (power_supply) - Sidetone setting support - Physical microphone location reporting - Headset and receiver firmware version reporting - Built-in alert triggering - USB wireless_status Tested with a Void Pro Wireless, Void Elite Wireless and a Void Elite Wired Signed-off-by: Stuart Hayhurst Signed-off-by: Jiri Kosina --- .../ABI/testing/sysfs-driver-hid-corsair-void | 38 + drivers/hid/Kconfig | 3 + drivers/hid/Makefile | 2 +- drivers/hid/hid-corsair-void.c | 829 ++++++++++++++++++ 4 files changed, 871 insertions(+), 1 deletion(-) create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair-void create mode 100644 drivers/hid/hid-corsair-void.c diff --git a/Documentation/ABI/testing/sysfs-driver-hid-corsair-void b/Documentation/ABI/testing/sysfs-driver-hid-corsair-void new file mode 100644 index 000000000000..83fa625c0025 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-driver-hid-corsair-void @@ -0,0 +1,38 @@ +What: /sys/bus/hid/drivers/hid-corsair-void//fw_version_headset +Date: January 2024 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (R) The firmware version of the headset + * Returns -ENODATA if no version was reported + +What: /sys/bus/hid/drivers/hid-corsair-void//fw_version_receiver +Date: January 2024 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (R) The firmware version of the receiver + +What: /sys/bus/hid/drivers/hid-corsair-void//microphone_up +Date: July 2023 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (R) Get the physical position of the microphone + * 1 -> Microphone up + * 0 -> Microphone down + +What: /sys/bus/hid/drivers/hid-corsair-void//send_alert +Date: July 2023 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (W) Play a built-in notification from the headset (0 / 1) + +What: /sys/bus/hid/drivers/hid-corsair-void//set_sidetone +Date: December 2023 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (W) Set the sidetone volume (0 - sidetone_max) + +What: /sys/bus/hid/drivers/hid-corsair-void//sidetone_max +Date: July 2024 +KernelVersion: 6.13 +Contact: Stuart Hayhurst +Description: (R) Report the maximum sidetone volume diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index f8a56d631242..5a90523146f7 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -213,13 +213,16 @@ config HID_CHICONY config HID_CORSAIR tristate "Corsair devices" depends on USB_HID && LEDS_CLASS + select POWER_SUPPLY help Support for Corsair devices that are not fully compliant with the HID standard. + Support for Corsair Void headsets. Supported devices: - Vengeance K90 - Scimitar PRO RGB + - Corsair Void headsets config HID_COUGAR tristate "Cougar devices" diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 496dab54c73a..ea2ed38d7e31 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -38,7 +38,7 @@ obj-$(CONFIG_HID_BIGBEN_FF) += hid-bigbenff.o obj-$(CONFIG_HID_CHERRY) += hid-cherry.o obj-$(CONFIG_HID_CHICONY) += hid-chicony.o obj-$(CONFIG_HID_CMEDIA) += hid-cmedia.o -obj-$(CONFIG_HID_CORSAIR) += hid-corsair.o +obj-$(CONFIG_HID_CORSAIR) += hid-corsair.o hid-corsair-void.o obj-$(CONFIG_HID_COUGAR) += hid-cougar.o obj-$(CONFIG_HID_CP2112) += hid-cp2112.o obj-$(CONFIG_HID_CYPRESS) += hid-cypress.o diff --git a/drivers/hid/hid-corsair-void.c b/drivers/hid/hid-corsair-void.c new file mode 100644 index 000000000000..6ece56b850fc --- /dev/null +++ b/drivers/hid/hid-corsair-void.c @@ -0,0 +1,829 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * HID driver for Corsair Void headsets + * + * Copyright (C) 2023-2024 Stuart Hayhurst + */ + +/* -------------------------------------------------------------------------- */ +/* Receiver report information: (ID 100) */ +/* -------------------------------------------------------------------------- */ +/* + * When queried, the receiver reponds with 5 bytes to describe the battery + * The power button, mute button and moving the mic also trigger this report + * This includes power button + mic + connection + battery status and capacity + * The information below may not be perfect, it's been gathered through guesses + * + * 0: REPORT ID + * 100 for the battery packet + * + * 1: POWER BUTTON + (?) + * Largest bit is 1 when power button pressed + * + * 2: BATTERY CAPACITY + MIC STATUS + * Battery capacity: + * Seems to report ~54 higher than reality when charging + * Capped at 100, charging or not + * Microphone status: + * Largest bit is set to 1 when the mic is physically up + * No bits change when the mic is muted, only when physically moved + * This report is sent every time the mic is moved, no polling required + * + * 3: CONNECTION STATUS + * 16: Wired headset + * 38: Initialising + * 49: Lost connection + * 51: Disconnected, searching + * 52: Disconnected, not searching + * 177: Normal + * + * 4: BATTERY STATUS + * 0: Disconnected + * 1: Normal + * 2: Low + * 3: Critical - sent during shutdown + * 4: Fully charged + * 5: Charging + */ +/* -------------------------------------------------------------------------- */ + +/* -------------------------------------------------------------------------- */ +/* Receiver report information: (ID 102) */ +/* -------------------------------------------------------------------------- */ +/* + * When queried, the recevier responds with 4 bytes to describe the firmware + * The first 2 bytes are for the receiver, the second 2 are the headset + * The headset firmware version will be 0 if no headset is connected + * + * 0: Recevier firmware major version + * Major version of the receiver's firmware + * + * 1: Recevier firmware minor version + * Minor version of the receiver's firmware + * + * 2: Headset firmware major version + * Major version of the headset's firmware + * + * 3: Headset firmware minor version + * Minor version of the headset's firmware + */ +/* -------------------------------------------------------------------------- */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "hid-ids.h" + +#define CORSAIR_VOID_DEVICE(id, type) { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, (id)), \ + .driver_data = (type) } +#define CORSAIR_VOID_WIRELESS_DEVICE(id) CORSAIR_VOID_DEVICE((id), CORSAIR_VOID_WIRELESS) +#define CORSAIR_VOID_WIRED_DEVICE(id) CORSAIR_VOID_DEVICE((id), CORSAIR_VOID_WIRED) + +#define CORSAIR_VOID_STATUS_REQUEST_ID 0xC9 +#define CORSAIR_VOID_NOTIF_REQUEST_ID 0xCA +#define CORSAIR_VOID_SIDETONE_REQUEST_ID 0xFF +#define CORSAIR_VOID_STATUS_REPORT_ID 0x64 +#define CORSAIR_VOID_FIRMWARE_REPORT_ID 0x66 + +#define CORSAIR_VOID_USB_SIDETONE_REQUEST 0x1 +#define CORSAIR_VOID_USB_SIDETONE_REQUEST_TYPE 0x21 +#define CORSAIR_VOID_USB_SIDETONE_VALUE 0x200 +#define CORSAIR_VOID_USB_SIDETONE_INDEX 0xB00 + +#define CORSAIR_VOID_MIC_MASK GENMASK(7, 7) +#define CORSAIR_VOID_CAPACITY_MASK GENMASK(6, 0) + +#define CORSAIR_VOID_WIRELESS_CONNECTED 177 + +#define CORSAIR_VOID_SIDETONE_MAX_WIRELESS 55 +#define CORSAIR_VOID_SIDETONE_MAX_WIRED 4096 + +enum { + CORSAIR_VOID_WIRELESS, + CORSAIR_VOID_WIRED, +}; + +enum { + CORSAIR_VOID_BATTERY_NORMAL = 1, + CORSAIR_VOID_BATTERY_LOW = 2, + CORSAIR_VOID_BATTERY_CRITICAL = 3, + CORSAIR_VOID_BATTERY_CHARGED = 4, + CORSAIR_VOID_BATTERY_CHARGING = 5, +}; + +static enum power_supply_property corsair_void_battery_props[] = { + POWER_SUPPLY_PROP_STATUS, + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_CAPACITY, + POWER_SUPPLY_PROP_CAPACITY_LEVEL, + POWER_SUPPLY_PROP_SCOPE, + POWER_SUPPLY_PROP_MODEL_NAME, + POWER_SUPPLY_PROP_MANUFACTURER, +}; + +struct corsair_void_battery_data { + int status; + bool present; + int capacity; + int capacity_level; +}; + +struct corsair_void_drvdata { + struct hid_device *hid_dev; + struct device *dev; + + char *name; + bool is_wired; + unsigned int sidetone_max; + + struct corsair_void_battery_data battery_data; + bool mic_up; + bool connected; + int fw_receiver_major; + int fw_receiver_minor; + int fw_headset_major; + int fw_headset_minor; + + struct power_supply *battery; + struct power_supply_desc battery_desc; + struct mutex battery_mutex; + + struct delayed_work delayed_status_work; + struct delayed_work delayed_firmware_work; + struct work_struct battery_remove_work; + struct work_struct battery_add_work; +}; + +/* + * Functions to process receiver data +*/ + +static void corsair_void_set_wireless_status(struct corsair_void_drvdata *drvdata) +{ + struct usb_interface *usb_if = to_usb_interface(drvdata->dev->parent); + + if (drvdata->is_wired) + return; + + usb_set_wireless_status(usb_if, drvdata->connected ? + USB_WIRELESS_STATUS_CONNECTED : + USB_WIRELESS_STATUS_DISCONNECTED); +} + +static void corsair_void_set_unknown_batt(struct corsair_void_drvdata *drvdata) +{ + struct corsair_void_battery_data *battery_data = &drvdata->battery_data; + + battery_data->status = POWER_SUPPLY_STATUS_UNKNOWN; + battery_data->present = false; + battery_data->capacity = 0; + battery_data->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_UNKNOWN; +} + +/* Reset data that may change between wireless connections */ +static void corsair_void_set_unknown_wireless_data(struct corsair_void_drvdata *drvdata) +{ + /* Only 0 out headset, receiver is always known if relevant */ + drvdata->fw_headset_major = 0; + drvdata->fw_headset_minor = 0; + + drvdata->connected = false; + drvdata->mic_up = false; + + corsair_void_set_wireless_status(drvdata); +} + +static void corsair_void_process_receiver(struct corsair_void_drvdata *drvdata, + int raw_battery_capacity, + int raw_connection_status, + int raw_battery_status) +{ + struct corsair_void_battery_data *battery_data = &drvdata->battery_data; + struct corsair_void_battery_data orig_battery_data; + + /* Save initial battery data, to compare later */ + orig_battery_data = *battery_data; + + /* Headset not connected, or it's wired */ + if (raw_connection_status != CORSAIR_VOID_WIRELESS_CONNECTED) + goto unknown_battery; + + /* Battery information unavailable */ + if (raw_battery_status == 0) + goto unknown_battery; + + /* Battery must be connected then */ + battery_data->present = true; + battery_data->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_NORMAL; + + /* Set battery status */ + switch (raw_battery_status) { + case CORSAIR_VOID_BATTERY_NORMAL: + case CORSAIR_VOID_BATTERY_LOW: + case CORSAIR_VOID_BATTERY_CRITICAL: + battery_data->status = POWER_SUPPLY_STATUS_DISCHARGING; + if (raw_battery_status == CORSAIR_VOID_BATTERY_LOW) + battery_data->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_LOW; + else if (raw_battery_status == CORSAIR_VOID_BATTERY_CRITICAL) + battery_data->capacity_level = POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL; + + break; + case CORSAIR_VOID_BATTERY_CHARGED: + battery_data->status = POWER_SUPPLY_STATUS_FULL; + break; + case CORSAIR_VOID_BATTERY_CHARGING: + battery_data->status = POWER_SUPPLY_STATUS_CHARGING; + break; + default: + hid_warn(drvdata->hid_dev, "unknown battery status '%d'", + raw_battery_status); + goto unknown_battery; + break; + } + + battery_data->capacity = raw_battery_capacity; + corsair_void_set_wireless_status(drvdata); + + goto success; +unknown_battery: + corsair_void_set_unknown_batt(drvdata); +success: + + /* Inform power supply if battery values changed */ + if (memcmp(&orig_battery_data, battery_data, sizeof(*battery_data))) { + scoped_guard(mutex, &drvdata->battery_mutex) { + if (drvdata->battery) { + power_supply_changed(drvdata->battery); + } + } + } +} + +/* + * Functions to report stored data +*/ + +static int corsair_void_battery_get_property(struct power_supply *psy, + enum power_supply_property prop, + union power_supply_propval *val) +{ + struct corsair_void_drvdata *drvdata = power_supply_get_drvdata(psy); + + switch (prop) { + case POWER_SUPPLY_PROP_SCOPE: + val->intval = POWER_SUPPLY_SCOPE_DEVICE; + break; + case POWER_SUPPLY_PROP_MODEL_NAME: + if (!strncmp(drvdata->hid_dev->name, "Corsair ", 8)) + val->strval = drvdata->hid_dev->name + 8; + else + val->strval = drvdata->hid_dev->name; + break; + case POWER_SUPPLY_PROP_MANUFACTURER: + val->strval = "Corsair"; + break; + case POWER_SUPPLY_PROP_STATUS: + val->intval = drvdata->battery_data.status; + break; + case POWER_SUPPLY_PROP_PRESENT: + val->intval = drvdata->battery_data.present; + break; + case POWER_SUPPLY_PROP_CAPACITY: + val->intval = drvdata->battery_data.capacity; + break; + case POWER_SUPPLY_PROP_CAPACITY_LEVEL: + val->intval = drvdata->battery_data.capacity_level; + break; + default: + return -EINVAL; + } + + return 0; +} + +static ssize_t microphone_up_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + + if (!drvdata->connected) + return -ENODEV; + + return sysfs_emit(buf, "%d\n", drvdata->mic_up); +} + +static ssize_t fw_version_receiver_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata->fw_receiver_major == 0 && drvdata->fw_receiver_minor == 0) + return -ENODATA; + + return sysfs_emit(buf, "%d.%02d\n", drvdata->fw_receiver_major, + drvdata->fw_receiver_minor); +} + + +static ssize_t fw_version_headset_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + + if (drvdata->fw_headset_major == 0 && drvdata->fw_headset_minor == 0) + return -ENODATA; + + return sysfs_emit(buf, "%d.%02d\n", drvdata->fw_headset_major, + drvdata->fw_headset_minor); +} + +static ssize_t sidetone_max_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%d\n", drvdata->sidetone_max); +} + +/* + * Functions to send data to headset +*/ + +static ssize_t send_alert_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + struct hid_device *hid_dev = drvdata->hid_dev; + unsigned char alert_id; + unsigned char *send_buf __free(kfree) = NULL; + int ret; + + if (!drvdata->connected || drvdata->is_wired) + return -ENODEV; + + /* Only accept 0 or 1 for alert ID */ + if (kstrtou8(buf, 10, &alert_id) || alert_id >= 2) + return -EINVAL; + + send_buf = kmalloc(3, GFP_KERNEL); + if (!send_buf) + return -ENOMEM; + + /* Packet format to send alert with ID alert_id */ + send_buf[0] = CORSAIR_VOID_NOTIF_REQUEST_ID; + send_buf[1] = 0x02; + send_buf[2] = alert_id; + + ret = hid_hw_raw_request(hid_dev, CORSAIR_VOID_NOTIF_REQUEST_ID, + send_buf, 3, HID_OUTPUT_REPORT, + HID_REQ_SET_REPORT); + if (ret < 0) + hid_warn(hid_dev, "failed to send alert request (reason: %d)", + ret); + else + ret = count; + + return ret; +} + +static int corsair_void_set_sidetone_wired(struct device *dev, const char *buf, + unsigned int sidetone) +{ + struct usb_interface *usb_if = to_usb_interface(dev->parent); + struct usb_device *usb_dev = interface_to_usbdev(usb_if); + + /* Packet format to set sidetone for wired headsets */ + __le16 sidetone_le = cpu_to_le16(sidetone); + + return usb_control_msg_send(usb_dev, 0, + CORSAIR_VOID_USB_SIDETONE_REQUEST, + CORSAIR_VOID_USB_SIDETONE_REQUEST_TYPE, + CORSAIR_VOID_USB_SIDETONE_VALUE, + CORSAIR_VOID_USB_SIDETONE_INDEX, + &sidetone_le, 2, USB_CTRL_SET_TIMEOUT, + GFP_KERNEL); +} + +static int corsair_void_set_sidetone_wireless(struct device *dev, + const char *buf, + unsigned char sidetone) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + struct hid_device *hid_dev = drvdata->hid_dev; + unsigned char *send_buf __free(kfree) = NULL; + + send_buf = kmalloc(12, GFP_KERNEL); + if (!send_buf) + return -ENOMEM; + + /* Packet format to set sidetone for wireless headsets */ + send_buf[0] = CORSAIR_VOID_SIDETONE_REQUEST_ID; + send_buf[1] = 0x0B; + send_buf[2] = 0x00; + send_buf[3] = 0xFF; + send_buf[4] = 0x04; + send_buf[5] = 0x0E; + send_buf[6] = 0xFF; + send_buf[7] = 0x05; + send_buf[8] = 0x01; + send_buf[9] = 0x04; + send_buf[10] = 0x00; + send_buf[11] = sidetone + 200; + + return hid_hw_raw_request(hid_dev, CORSAIR_VOID_SIDETONE_REQUEST_ID, + send_buf, 12, HID_FEATURE_REPORT, + HID_REQ_SET_REPORT); +} + +static ssize_t set_sidetone_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct corsair_void_drvdata *drvdata = dev_get_drvdata(dev); + struct hid_device *hid_dev = drvdata->hid_dev; + unsigned int sidetone; + int ret; + + if (!drvdata->connected) + return -ENODEV; + + /* sidetone must be between 0 and drvdata->sidetone_max inclusive */ + if (kstrtouint(buf, 10, &sidetone) || sidetone > drvdata->sidetone_max) + return -EINVAL; + + if (drvdata->is_wired) + ret = corsair_void_set_sidetone_wired(dev, buf, sidetone); + else + ret = corsair_void_set_sidetone_wireless(dev, buf, sidetone); + + if (ret < 0) + hid_warn(hid_dev, "failed to send sidetone (reason: %d)", ret); + else + ret = count; + + return ret; +} + +static int corsair_void_request_status(struct hid_device *hid_dev, int id) +{ + unsigned char *send_buf __free(kfree) = NULL; + + send_buf = kmalloc(2, GFP_KERNEL); + if (!send_buf) + return -ENOMEM; + + /* Packet format to request data item (status / firmware) refresh */ + send_buf[0] = CORSAIR_VOID_STATUS_REQUEST_ID; + send_buf[1] = id; + + /* Send request for data refresh */ + return hid_hw_raw_request(hid_dev, CORSAIR_VOID_STATUS_REQUEST_ID, + send_buf, 2, HID_OUTPUT_REPORT, + HID_REQ_SET_REPORT); +} + +/* + * Headset connect / disconnect handlers and work handlers +*/ + +static void corsair_void_status_work_handler(struct work_struct *work) +{ + struct corsair_void_drvdata *drvdata; + struct delayed_work *delayed_work; + int battery_ret; + + delayed_work = container_of(work, struct delayed_work, work); + drvdata = container_of(delayed_work, struct corsair_void_drvdata, + delayed_status_work); + + battery_ret = corsair_void_request_status(drvdata->hid_dev, + CORSAIR_VOID_STATUS_REPORT_ID); + if (battery_ret < 0) { + hid_warn(drvdata->hid_dev, + "failed to request battery (reason: %d)", battery_ret); + } +} + +static void corsair_void_firmware_work_handler(struct work_struct *work) +{ + struct corsair_void_drvdata *drvdata; + struct delayed_work *delayed_work; + int firmware_ret; + + delayed_work = container_of(work, struct delayed_work, work); + drvdata = container_of(delayed_work, struct corsair_void_drvdata, + delayed_firmware_work); + + firmware_ret = corsair_void_request_status(drvdata->hid_dev, + CORSAIR_VOID_FIRMWARE_REPORT_ID); + if (firmware_ret < 0) { + hid_warn(drvdata->hid_dev, + "failed to request firmware (reason: %d)", firmware_ret); + } + +} + +static void corsair_void_battery_remove_work_handler(struct work_struct *work) +{ + struct corsair_void_drvdata *drvdata; + + drvdata = container_of(work, struct corsair_void_drvdata, + battery_remove_work); + scoped_guard(mutex, &drvdata->battery_mutex) { + if (drvdata->battery) { + power_supply_unregister(drvdata->battery); + drvdata->battery = NULL; + } + } +} + +static void corsair_void_battery_add_work_handler(struct work_struct *work) +{ + struct corsair_void_drvdata *drvdata; + struct power_supply_config psy_cfg; + struct power_supply *new_supply; + + drvdata = container_of(work, struct corsair_void_drvdata, + battery_add_work); + guard(mutex)(&drvdata->battery_mutex); + if (drvdata->battery) + return; + + psy_cfg.drv_data = drvdata; + new_supply = power_supply_register(drvdata->dev, + &drvdata->battery_desc, + &psy_cfg); + + if (IS_ERR(new_supply)) { + hid_err(drvdata->hid_dev, + "failed to register battery '%s' (reason: %ld)\n", + drvdata->battery_desc.name, + PTR_ERR(new_supply)); + return; + } + + if (power_supply_powers(new_supply, drvdata->dev)) { + power_supply_unregister(new_supply); + return; + } + + drvdata->battery = new_supply; +} + +static void corsair_void_headset_connected(struct corsair_void_drvdata *drvdata) +{ + schedule_work(&drvdata->battery_add_work); + schedule_delayed_work(&drvdata->delayed_firmware_work, + msecs_to_jiffies(100)); +} + +static void corsair_void_headset_disconnected(struct corsair_void_drvdata *drvdata) +{ + schedule_work(&drvdata->battery_remove_work); + + corsair_void_set_unknown_wireless_data(drvdata); + corsair_void_set_unknown_batt(drvdata); +} + +/* + * Driver setup, probing and HID event handling +*/ + +static DEVICE_ATTR_RO(fw_version_receiver); +static DEVICE_ATTR_RO(fw_version_headset); +static DEVICE_ATTR_RO(microphone_up); +static DEVICE_ATTR_RO(sidetone_max); + +static DEVICE_ATTR_WO(send_alert); +static DEVICE_ATTR_WO(set_sidetone); + +static struct attribute *corsair_void_attrs[] = { + &dev_attr_fw_version_receiver.attr, + &dev_attr_fw_version_headset.attr, + &dev_attr_microphone_up.attr, + &dev_attr_send_alert.attr, + &dev_attr_set_sidetone.attr, + &dev_attr_sidetone_max.attr, + NULL, +}; + +static const struct attribute_group corsair_void_attr_group = { + .attrs = corsair_void_attrs, +}; + +static int corsair_void_probe(struct hid_device *hid_dev, + const struct hid_device_id *hid_id) +{ + int ret; + struct corsair_void_drvdata *drvdata; + char *name; + + if (!hid_is_usb(hid_dev)) + return -EINVAL; + + drvdata = devm_kzalloc(&hid_dev->dev, sizeof(*drvdata), + GFP_KERNEL); + if (!drvdata) + return -ENOMEM; + + hid_set_drvdata(hid_dev, drvdata); + dev_set_drvdata(&hid_dev->dev, drvdata); + + drvdata->dev = &hid_dev->dev; + drvdata->hid_dev = hid_dev; + drvdata->is_wired = hid_id->driver_data == CORSAIR_VOID_WIRED; + + drvdata->sidetone_max = CORSAIR_VOID_SIDETONE_MAX_WIRELESS; + if (drvdata->is_wired) + drvdata->sidetone_max = CORSAIR_VOID_SIDETONE_MAX_WIRED; + + /* Set initial values for no wireless headset attached */ + /* If a headset is attached, it'll be prompted later */ + corsair_void_set_unknown_wireless_data(drvdata); + corsair_void_set_unknown_batt(drvdata); + + /* Receiver version won't be reset after init */ + /* Headset version already set via set_unknown_wireless_data */ + drvdata->fw_receiver_major = 0; + drvdata->fw_receiver_minor = 0; + + ret = hid_parse(hid_dev); + if (ret) { + hid_err(hid_dev, "parse failed (reason: %d)\n", ret); + return ret; + } + + name = devm_kasprintf(drvdata->dev, GFP_KERNEL, + "corsair-void-%d-battery", hid_dev->id); + if (!name) + return -ENOMEM; + + drvdata->battery_desc.name = name; + drvdata->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY; + drvdata->battery_desc.properties = corsair_void_battery_props; + drvdata->battery_desc.num_properties = ARRAY_SIZE(corsair_void_battery_props); + drvdata->battery_desc.get_property = corsair_void_battery_get_property; + + drvdata->battery = NULL; + INIT_WORK(&drvdata->battery_remove_work, + corsair_void_battery_remove_work_handler); + INIT_WORK(&drvdata->battery_add_work, + corsair_void_battery_add_work_handler); + ret = devm_mutex_init(drvdata->dev, &drvdata->battery_mutex); + if (ret) + return ret; + + ret = sysfs_create_group(&hid_dev->dev.kobj, &corsair_void_attr_group); + if (ret) + return ret; + + /* Any failures after here will need to call hid_hw_stop */ + ret = hid_hw_start(hid_dev, HID_CONNECT_DEFAULT); + if (ret) { + hid_err(hid_dev, "hid_hw_start failed (reason: %d)\n", ret); + goto failed_after_sysfs; + } + + /* Refresh battery data, in case wireless headset is already connected */ + INIT_DELAYED_WORK(&drvdata->delayed_status_work, + corsair_void_status_work_handler); + schedule_delayed_work(&drvdata->delayed_status_work, + msecs_to_jiffies(100)); + + /* Refresh firmware versions */ + INIT_DELAYED_WORK(&drvdata->delayed_firmware_work, + corsair_void_firmware_work_handler); + schedule_delayed_work(&drvdata->delayed_firmware_work, + msecs_to_jiffies(100)); + + return 0; + +failed_after_sysfs: + sysfs_remove_group(&hid_dev->dev.kobj, &corsair_void_attr_group); + return ret; +} + +static void corsair_void_remove(struct hid_device *hid_dev) +{ + struct corsair_void_drvdata *drvdata = hid_get_drvdata(hid_dev); + + hid_hw_stop(hid_dev); + cancel_work_sync(&drvdata->battery_remove_work); + cancel_work_sync(&drvdata->battery_add_work); + if (drvdata->battery) + power_supply_unregister(drvdata->battery); + + cancel_delayed_work_sync(&drvdata->delayed_firmware_work); + sysfs_remove_group(&hid_dev->dev.kobj, &corsair_void_attr_group); +} + +static int corsair_void_raw_event(struct hid_device *hid_dev, + struct hid_report *hid_report, + u8 *data, int size) +{ + struct corsair_void_drvdata *drvdata = hid_get_drvdata(hid_dev); + bool was_connected = drvdata->connected; + + /* Description of packets are documented at the top of this file */ + if (hid_report->id == CORSAIR_VOID_STATUS_REPORT_ID) { + drvdata->mic_up = FIELD_GET(CORSAIR_VOID_MIC_MASK, data[2]); + drvdata->connected = (data[3] == CORSAIR_VOID_WIRELESS_CONNECTED) || + drvdata->is_wired; + + corsair_void_process_receiver(drvdata, + FIELD_GET(CORSAIR_VOID_CAPACITY_MASK, data[2]), + data[3], data[4]); + } else if (hid_report->id == CORSAIR_VOID_FIRMWARE_REPORT_ID) { + drvdata->fw_receiver_major = data[1]; + drvdata->fw_receiver_minor = data[2]; + drvdata->fw_headset_major = data[3]; + drvdata->fw_headset_minor = data[4]; + } + + /* Handle wireless headset connect / disconnect */ + if ((was_connected != drvdata->connected) && !drvdata->is_wired) { + if (drvdata->connected) + corsair_void_headset_connected(drvdata); + else + corsair_void_headset_disconnected(drvdata); + } + + return 0; +} + +static const struct hid_device_id corsair_void_devices[] = { + /* Corsair Void Wireless */ + CORSAIR_VOID_WIRELESS_DEVICE(0x0a0c), + CORSAIR_VOID_WIRELESS_DEVICE(0x0a2b), + CORSAIR_VOID_WIRELESS_DEVICE(0x1b23), + CORSAIR_VOID_WIRELESS_DEVICE(0x1b25), + CORSAIR_VOID_WIRELESS_DEVICE(0x1b27), + + /* Corsair Void USB */ + CORSAIR_VOID_WIRED_DEVICE(0x0a0f), + CORSAIR_VOID_WIRED_DEVICE(0x1b1c), + CORSAIR_VOID_WIRED_DEVICE(0x1b29), + CORSAIR_VOID_WIRED_DEVICE(0x1b2a), + + /* Corsair Void Surround */ + CORSAIR_VOID_WIRED_DEVICE(0x0a30), + CORSAIR_VOID_WIRED_DEVICE(0x0a31), + + /* Corsair Void Pro Wireless */ + CORSAIR_VOID_WIRELESS_DEVICE(0x0a14), + CORSAIR_VOID_WIRELESS_DEVICE(0x0a16), + CORSAIR_VOID_WIRELESS_DEVICE(0x0a1a), + + /* Corsair Void Pro USB */ + CORSAIR_VOID_WIRED_DEVICE(0x0a17), + CORSAIR_VOID_WIRED_DEVICE(0x0a1d), + + /* Corsair Void Pro Surround */ + CORSAIR_VOID_WIRED_DEVICE(0x0a18), + CORSAIR_VOID_WIRED_DEVICE(0x0a1e), + CORSAIR_VOID_WIRED_DEVICE(0x0a1f), + + /* Corsair Void Elite Wireless */ + CORSAIR_VOID_WIRELESS_DEVICE(0x0a51), + CORSAIR_VOID_WIRELESS_DEVICE(0x0a55), + CORSAIR_VOID_WIRELESS_DEVICE(0x0a75), + + /* Corsair Void Elite USB */ + CORSAIR_VOID_WIRED_DEVICE(0x0a52), + CORSAIR_VOID_WIRED_DEVICE(0x0a56), + + /* Corsair Void Elite Surround */ + CORSAIR_VOID_WIRED_DEVICE(0x0a53), + CORSAIR_VOID_WIRED_DEVICE(0x0a57), + + {} +}; + +MODULE_DEVICE_TABLE(hid, corsair_void_devices); + +static struct hid_driver corsair_void_driver = { + .name = "hid-corsair-void", + .id_table = corsair_void_devices, + .probe = corsair_void_probe, + .remove = corsair_void_remove, + .raw_event = corsair_void_raw_event, +}; + +module_hid_driver(corsair_void_driver); + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("Stuart Hayhurst "); +MODULE_DESCRIPTION("HID driver for Corsair Void headsets"); From ac0cba683772991b1100e2b26065c188e00a46fe Mon Sep 17 00:00:00 2001 From: Zhang Lixu Date: Wed, 9 Oct 2024 09:10:23 +0800 Subject: [PATCH 18/40] HID: intel-ish-hid: Add firmware version sysfs attributes Introduce sysfs attributes to the intel-ish-ipc driver to expose the base and project firmware versions for ISH devices that load firmware from the host. The build tool embeds these versions into the ISH global manifest within the firmware binary during the firmware build process. The driver, upon loading the firmware, extracts this version information from the manifest and makes it accessible via sysfs. The base version corresponds to the firmware version provided in Intel's Firmware Development Kit (FDK), while the project version reflects the vendor-customized firmware derived from the FDK. These attributes provide userspace tools and applications with the ability to easily query the firmware versions, which is essential for firmware validation and troubleshooting. Example usages: $ cat /sys/devices/pci0000\:00/0000\:00\:12.0/firmware/base_version 5.8.0.7716 $ cat /sys/devices/pci0000\:00/0000\:00\:12.0/firmware/project_version 5.8.0.12472 Signed-off-by: Zhang Lixu Acked-by: Srinivas Pandruvada Signed-off-by: Jiri Kosina --- drivers/hid/intel-ish-hid/ipc/pci-ish.c | 45 +++++++++++++++++++++ drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h | 12 ++++++ drivers/hid/intel-ish-hid/ishtp/loader.c | 35 +++++++++++++++- drivers/hid/intel-ish-hid/ishtp/loader.h | 34 ++++++++++++++++ 4 files changed, 125 insertions(+), 1 deletion(-) diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c index aae0d965b47b..9e2401291a2f 100644 --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c @@ -381,6 +381,50 @@ static int __maybe_unused ish_resume(struct device *device) static SIMPLE_DEV_PM_OPS(ish_pm_ops, ish_suspend, ish_resume); +static ssize_t base_version_show(struct device *cdev, + struct device_attribute *attr, char *buf) +{ + struct ishtp_device *dev = dev_get_drvdata(cdev); + + return sysfs_emit(buf, "%u.%u.%u.%u\n", dev->base_ver.major, + dev->base_ver.minor, dev->base_ver.hotfix, + dev->base_ver.build); +} +static DEVICE_ATTR_RO(base_version); + +static ssize_t project_version_show(struct device *cdev, + struct device_attribute *attr, char *buf) +{ + struct ishtp_device *dev = dev_get_drvdata(cdev); + + return sysfs_emit(buf, "%u.%u.%u.%u\n", dev->prj_ver.major, + dev->prj_ver.minor, dev->prj_ver.hotfix, + dev->prj_ver.build); +} +static DEVICE_ATTR_RO(project_version); + +static struct attribute *ish_firmware_attrs[] = { + &dev_attr_base_version.attr, + &dev_attr_project_version.attr, + NULL +}; + +static umode_t firmware_is_visible(struct kobject *kobj, struct attribute *attr, + int i) +{ + struct ishtp_device *dev = dev_get_drvdata(kobj_to_dev(kobj)); + + return dev->driver_data->fw_generation ? attr->mode : 0; +} + +static const struct attribute_group ish_firmware_group = { + .name = "firmware", + .attrs = ish_firmware_attrs, + .is_visible = firmware_is_visible, +}; + +__ATTRIBUTE_GROUPS(ish_firmware); + static struct pci_driver ish_driver = { .name = KBUILD_MODNAME, .id_table = ish_pci_tbl, @@ -388,6 +432,7 @@ static struct pci_driver ish_driver = { .remove = ish_remove, .shutdown = ish_shutdown, .driver.pm = &ish_pm_ops, + .dev_groups = ish_firmware_groups, }; module_pci_driver(ish_driver); diff --git a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h index cdacce0a4c9d..effbb442c727 100644 --- a/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h +++ b/drivers/hid/intel-ish-hid/ishtp/ishtp-dev.h @@ -140,6 +140,13 @@ struct ishtp_driver_data { char *fw_generation; }; +struct ish_version { + u16 major; + u16 minor; + u16 hotfix; + u16 build; +}; + /** * struct ishtp_device - ISHTP private device struct */ @@ -236,6 +243,11 @@ struct ishtp_device { /* Dump to trace buffers if enabled*/ ishtp_print_log print_log; + /* Base version of Intel's released firmware */ + struct ish_version base_ver; + /* Vendor-customized project version */ + struct ish_version prj_ver; + /* Debug stats */ unsigned int ipc_rx_cnt; unsigned long long ipc_rx_bytes_cnt; diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.c b/drivers/hid/intel-ish-hid/ishtp/loader.c index f76c4437a1f5..f34086b29cf0 100644 --- a/drivers/hid/intel-ish-hid/ishtp/loader.c +++ b/drivers/hid/intel-ish-hid/ishtp/loader.c @@ -308,6 +308,28 @@ static int request_ish_firmware(const struct firmware **firmware_p, return _request_ish_firmware(firmware_p, filename, dev); } +static int copy_manifest(const struct firmware *fw, struct ish_global_manifest *manifest) +{ + u32 offset; + + for (offset = 0; offset + sizeof(*manifest) < fw->size; offset += ISH_MANIFEST_ALIGNMENT) { + memcpy(manifest, fw->data + offset, sizeof(*manifest)); + + if (le32_to_cpu(manifest->sig_fourcc) == ISH_GLOBAL_SIG) + return 0; + } + + return -1; +} + +static void copy_ish_version(struct version_in_manifest *src, struct ish_version *dst) +{ + dst->major = le16_to_cpu(src->major); + dst->minor = le16_to_cpu(src->minor); + dst->hotfix = le16_to_cpu(src->hotfix); + dst->build = le16_to_cpu(src->build); +} + /** * ishtp_loader_work() - Load the ISHTP firmware * @work: The work structure @@ -336,6 +358,7 @@ void ishtp_loader_work(struct work_struct *work) struct loader_xfer_query query = { .header = cpu_to_le32(query_hdr.val32), }; struct loader_start start = { .header = cpu_to_le32(start_hdr.val32), }; union loader_recv_message recv_msg; + struct ish_global_manifest manifest; const struct firmware *ish_fw; void *dma_bufs[FRAGMENT_MAX_NUM] = {}; u32 fragment_size; @@ -372,7 +395,7 @@ void ishtp_loader_work(struct work_struct *work) if (rv) continue; /* try again if failed */ - dev_dbg(dev->devc, "ISH Version %u.%u.%u.%u\n", + dev_dbg(dev->devc, "ISH Bootloader Version %u.%u.%u.%u\n", recv_msg.query_ack.version_major, recv_msg.query_ack.version_minor, recv_msg.query_ack.version_hotfix, @@ -390,6 +413,16 @@ void ishtp_loader_work(struct work_struct *work) continue; /* try again if failed */ dev_info(dev->devc, "firmware loaded. size:%zu\n", ish_fw->size); + if (!copy_manifest(ish_fw, &manifest)) { + copy_ish_version(&manifest.base_ver, &dev->base_ver); + copy_ish_version(&manifest.prj_ver, &dev->prj_ver); + dev_info(dev->devc, "FW base version: %u.%u.%u.%u\n", + dev->base_ver.major, dev->base_ver.minor, + dev->base_ver.hotfix, dev->base_ver.build); + dev_info(dev->devc, "FW project version: %u.%u.%u.%u\n", + dev->prj_ver.major, dev->prj_ver.minor, + dev->prj_ver.hotfix, dev->prj_ver.build); + } break; } while (--retry); diff --git a/drivers/hid/intel-ish-hid/ishtp/loader.h b/drivers/hid/intel-ish-hid/ishtp/loader.h index 308b96085a4d..4dda038b4947 100644 --- a/drivers/hid/intel-ish-hid/ishtp/loader.h +++ b/drivers/hid/intel-ish-hid/ishtp/loader.h @@ -10,6 +10,7 @@ #include #include +#include #include #include "ishtp-dev.h" @@ -228,4 +229,37 @@ struct ish_firmware_variant { */ void ishtp_loader_work(struct work_struct *work); +/* ISH Manifest alignment in binary is 4KB aligned */ +#define ISH_MANIFEST_ALIGNMENT SZ_4K + +/* Signature for ISH global manifest */ +#define ISH_GLOBAL_SIG 0x47485349 /* FourCC 'I', 'S', 'H', 'G' */ + +struct version_in_manifest { + __le16 major; + __le16 minor; + __le16 hotfix; + __le16 build; +}; + +/** + * struct ish_global_manifest - global manifest for ISH + * @sig_fourcc: Signature FourCC, should be 'I', 'S', 'H', 'G'. + * @len: Length of the manifest. + * @header_version: Version of the manifest header. + * @flags: Flags for additional information. + * @base_ver: Base version of Intel's released firmware. + * @reserved: Reserved space for future use. + * @prj_ver: Vendor-customized project version. + */ +struct ish_global_manifest { + __le32 sig_fourcc; + __le32 len; + __le32 header_version; + __le32 flags; + struct version_in_manifest base_ver; + __le32 reserved[13]; + struct version_in_manifest prj_ver; +}; + #endif /* _ISHTP_LOADER_H_ */ From b2b8a75e1d88c551a0b30d44d0be552210219eea Mon Sep 17 00:00:00 2001 From: Nathan Chancellor Date: Tue, 15 Oct 2024 12:23:47 -0700 Subject: [PATCH 19/40] HID: Remove default case statement in fetch_item() A default case statement with a bare unreachable() was recently added to fetch_item(), which by itself introduces undefined behavior. objtool points this out with a few different warnings, depending on configuration and compiler: vmlinux.o: warning: objtool: fetch_item() falls through to next function ... vmlinux.o: warning: objtool: hid_open_report() falls through to next function hid_parser_main() vmlinux.o: warning: objtool: hid_scan_report() falls through to next function hid_allocate_device() vmlinux.o: warning: objtool: hid_open_report+0x21b: can't find jump dest instruction at .text.hid_open_report+0x40f Replacing unreachable() with BUG() is a typical fix to eliminate the undefined behavior and make the default case well defined. However, in this case, all possible values are enumerated in the switch statement, so the default case can never actually happen, as proven with the comment next to the item->size assignment. Just remove the default case altogether, as the return statement would still be valid if the switch statement were ever to be skipped. Fixes: 61595012f280 ("HID: simplify code in fetch_item()") Suggested-by: Dmitry Torokhov Closes: https://lore.kernel.org/20241010222451.GA3571761@thelio-3990X/ Reported-by: Paul E. McKenney Closes: https://lore.kernel.org/fe8c909e-bf02-4466-b3eb-0a4747df32e3@paulmck-laptop/ Tested-by: Paul E. McKenney Signed-off-by: Nathan Chancellor Signed-off-by: Jiri Kosina --- drivers/hid/hid-core.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c index 87b2cdd7f0c4..bdd19bce11eb 100644 --- a/drivers/hid/hid-core.c +++ b/drivers/hid/hid-core.c @@ -802,9 +802,6 @@ static const u8 *fetch_item(const __u8 *start, const __u8 *end, struct hid_item case 4: item->data.u32 = get_unaligned_le32(start); break; - - default: - unreachable(); } return start + item->size; From 7b2daa648eb75ec75a6ebf5fce5629b758ea06df Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Wed, 16 Oct 2024 16:32:40 +0300 Subject: [PATCH 20/40] HID: debug: Remove duplicates from 'keys' Duplicates in 'keys prevents kernel builds with clang, `make W=1` and CONFIG_WERROR=y, for example: drivers/hid/hid-debug.c:3443:18: error: initializer overrides prior initialization of this subobject [-Werror,-Winitializer-overrides] 3443 | [KEY_HANGEUL] = "HanGeul", [KEY_HANGUP_PHONE] = "HangUpPhone", | ^~~~~~~~~ drivers/hid/hid-debug.c:3217:18: note: previous initialization is here 3217 | [KEY_HANGUEL] = "Hangeul", [KEY_HANJA] = "Hanja", | ^~~~~~~~~ Fix this by removing them. The logic of removal is that, remove... 1) if there is a constant that uses another defined constant, OR 2) the one that appears later in the list. Signed-off-by: Andy Shevchenko Signed-off-by: Jiri Kosina --- drivers/hid/hid-debug.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c index d5abfe652fb5..541d682af15a 100644 --- a/drivers/hid/hid-debug.c +++ b/drivers/hid/hid-debug.c @@ -3309,9 +3309,9 @@ static const char *keys[KEY_MAX + 1] = { [KEY_EPG] = "EPG", [KEY_PVR] = "PVR", [KEY_MHP] = "MHP", [KEY_LANGUAGE] = "Language", [KEY_TITLE] = "Title", [KEY_SUBTITLE] = "Subtitle", - [KEY_ANGLE] = "Angle", [KEY_ZOOM] = "Zoom", + [KEY_ANGLE] = "Angle", [KEY_MODE] = "Mode", [KEY_KEYBOARD] = "Keyboard", - [KEY_SCREEN] = "Screen", [KEY_PC] = "PC", + [KEY_PC] = "PC", [KEY_TV] = "TV", [KEY_TV2] = "TV2", [KEY_VCR] = "VCR", [KEY_VCR2] = "VCR2", [KEY_SAT] = "Sat", [KEY_SAT2] = "Sat2", @@ -3409,8 +3409,7 @@ static const char *keys[KEY_MAX + 1] = { [BTN_TRIGGER_HAPPY35] = "TriggerHappy35", [BTN_TRIGGER_HAPPY36] = "TriggerHappy36", [BTN_TRIGGER_HAPPY37] = "TriggerHappy37", [BTN_TRIGGER_HAPPY38] = "TriggerHappy38", [BTN_TRIGGER_HAPPY39] = "TriggerHappy39", [BTN_TRIGGER_HAPPY40] = "TriggerHappy40", - [BTN_DIGI] = "Digi", [BTN_STYLUS3] = "Stylus3", - [BTN_TOOL_QUINTTAP] = "ToolQuintTap", [BTN_WHEEL] = "Wheel", + [BTN_STYLUS3] = "Stylus3", [BTN_TOOL_QUINTTAP] = "ToolQuintTap", [KEY_10CHANNELSDOWN] = "10ChannelsDown", [KEY_10CHANNELSUP] = "10ChannelsUp", [KEY_3D_MODE] = "3DMode", [KEY_ADDRESSBOOK] = "Addressbook", @@ -3440,7 +3439,7 @@ static const char *keys[KEY_MAX + 1] = { [KEY_FN_RIGHT_SHIFT] = "FnRightShift", [KEY_FRAMEBACK] = "FrameBack", [KEY_FRAMEFORWARD] = "FrameForward", [KEY_FULL_SCREEN] = "FullScreen", [KEY_GAMES] = "Games", [KEY_GRAPHICSEDITOR] = "GraphicsEditor", - [KEY_HANGEUL] = "HanGeul", [KEY_HANGUP_PHONE] = "HangUpPhone", + [KEY_HANGUP_PHONE] = "HangUpPhone", [KEY_IMAGES] = "Images", [KEY_KBD_LCD_MENU1] = "KbdLcdMenu1", [KEY_KBD_LCD_MENU2] = "KbdLcdMenu2", [KEY_KBD_LCD_MENU3] = "KbdLcdMenu3", [KEY_KBD_LCD_MENU4] = "KbdLcdMenu4", [KEY_KBD_LCD_MENU5] = "KbdLcdMenu5", From ff39b0bbc2a4cc7f424eda30e52b07a68f82da04 Mon Sep 17 00:00:00 2001 From: Lode Willems Date: Sat, 5 Oct 2024 13:57:03 +0200 Subject: [PATCH 21/40] HID: Add IDs for Kysona Add the vendor ID for Kysona and the product IDs for the M600 mouse (both the dongle and the mouse itself) Signed-off-by: Lode Willems Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 8a991b30e3c6..e47dde3ce3fb 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -734,6 +734,10 @@ #define USB_DEVICE_ID_KYE_MOUSEPEN_I608X_V2 0x501A #define USB_DEVICE_ID_KYE_PENSKETCH_T609A 0x501B +#define USB_VENDOR_ID_KYSONA 0x3554 +#define USB_DEVICE_ID_KYSONA_M600_DONGLE 0xF57C +#define USB_DEVICE_ID_KYSONA_M600_WIRED 0xF57D + #define USB_VENDOR_ID_LABTEC 0x1020 #define USB_DEVICE_ID_LABTEC_WIRELESS_KEYBOARD 0x0006 #define USB_DEVICE_ID_LABTEC_ODDOR_HANDBRAKE 0x8888 From 30c32d05294575c27bdb2fa13c67cd1b601bf400 Mon Sep 17 00:00:00 2001 From: Lode Willems Date: Sat, 5 Oct 2024 13:57:04 +0200 Subject: [PATCH 22/40] HID: Kysona: Add basic battery reporting for Kysona M600 In this initial the battery is only probed once, a following patch will add periodic checking. Signed-off-by: Lode Willems Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 9 ++ drivers/hid/Makefile | 1 + drivers/hid/hid-kysona.c | 208 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 218 insertions(+) create mode 100644 drivers/hid/hid-kysona.c diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index f8a56d631242..d05e851bc87c 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -465,6 +465,15 @@ config HID_KYE - MousePen i608X tablet - EasyPen M610X tablet +config HID_KYSONA + tristate "Kysona devices" + depends on USB_HID + help + Support for Kysona mice. + + Say Y here if you have a Kysona M600 mouse + and want to be able to read its battery capacity. + config HID_UCLOGIC tristate "UC-Logic" depends on USB_HID diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile index 496dab54c73a..51cf887f54bd 100644 --- a/drivers/hid/Makefile +++ b/drivers/hid/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_HID_JABRA) += hid-jabra.o obj-$(CONFIG_HID_KENSINGTON) += hid-kensington.o obj-$(CONFIG_HID_KEYTOUCH) += hid-keytouch.o obj-$(CONFIG_HID_KYE) += hid-kye.o +obj-$(CONFIG_HID_KYSONA) += hid-kysona.o obj-$(CONFIG_HID_LCPOWER) += hid-lcpower.o obj-$(CONFIG_HID_LENOVO) += hid-lenovo.o obj-$(CONFIG_HID_LETSKETCH) += hid-letsketch.o diff --git a/drivers/hid/hid-kysona.c b/drivers/hid/hid-kysona.c new file mode 100644 index 000000000000..91e359c31fa1 --- /dev/null +++ b/drivers/hid/hid-kysona.c @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * USB HID driver for Kysona + * Kysona M600 mice. + * + * Copyright (c) 2024 Lode Willems + */ + +#include +#include +#include + +#include "hid-ids.h" + +#define BATTERY_REPORT_ID 4 + +struct kysona_drvdata { + struct hid_device *hdev; + struct power_supply_desc battery_desc; + struct power_supply *battery; + u8 battery_capacity; + bool battery_charging; + u16 battery_voltage; +}; + +static enum power_supply_property kysona_battery_props[] = { + POWER_SUPPLY_PROP_STATUS, + POWER_SUPPLY_PROP_PRESENT, + POWER_SUPPLY_PROP_CAPACITY, + POWER_SUPPLY_PROP_SCOPE, + POWER_SUPPLY_PROP_MODEL_NAME, + POWER_SUPPLY_PROP_VOLTAGE_NOW +}; + +static int kysona_battery_get_property(struct power_supply *psy, + enum power_supply_property psp, + union power_supply_propval *val) +{ + struct kysona_drvdata *drv_data = power_supply_get_drvdata(psy); + int ret = 0; + + switch (psp) { + case POWER_SUPPLY_PROP_PRESENT: + val->intval = 1; + break; + case POWER_SUPPLY_PROP_STATUS: + // TODO: check if device is online + val->intval = drv_data->battery_charging ? + POWER_SUPPLY_STATUS_CHARGING : + POWER_SUPPLY_STATUS_DISCHARGING; + break; + case POWER_SUPPLY_PROP_SCOPE: + val->intval = POWER_SUPPLY_SCOPE_DEVICE; + break; + case POWER_SUPPLY_PROP_CAPACITY: + val->intval = drv_data->battery_capacity; + break; + case POWER_SUPPLY_PROP_VOLTAGE_NOW: + /* hardware reports voltage in mV. sysfs expects uV */ + val->intval = drv_data->battery_voltage * 1000; + break; + case POWER_SUPPLY_PROP_MODEL_NAME: + val->strval = drv_data->hdev->name; + break; + default: + ret = -EINVAL; + break; + } + return ret; +} + +static const char kysona_battery_request[] = { + 0x08, BATTERY_REPORT_ID, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x49 +}; + +static int kysona_m600_fetch_battery(struct hid_device *hdev) +{ + u8 *write_buf; + int ret; + + /* Request battery information */ + write_buf = kmemdup(kysona_battery_request, sizeof(kysona_battery_request), GFP_KERNEL); + if (!write_buf) + return -ENOMEM; + + ret = hid_hw_raw_request(hdev, kysona_battery_request[0], + write_buf, sizeof(kysona_battery_request), + HID_OUTPUT_REPORT, HID_REQ_SET_REPORT); + if (ret < (int)sizeof(kysona_battery_request)) { + hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret); + ret = -ENODATA; + } + kfree(write_buf); + return ret; +} + +static void kysona_fetch_battery(struct hid_device *hdev) +{ + int ret = kysona_m600_fetch_battery(hdev); + + if (ret < 0) + hid_dbg(hdev, + "Battery query failed (err: %d)\n", ret); +} + +static int kysona_battery_probe(struct hid_device *hdev) +{ + struct kysona_drvdata *drv_data = hid_get_drvdata(hdev); + struct power_supply_config pscfg = { .drv_data = drv_data }; + int ret = 0; + + drv_data->battery_capacity = 100; + + drv_data->battery_desc.properties = kysona_battery_props; + drv_data->battery_desc.num_properties = ARRAY_SIZE(kysona_battery_props); + drv_data->battery_desc.get_property = kysona_battery_get_property; + drv_data->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY; + drv_data->battery_desc.use_for_apm = 0; + drv_data->battery_desc.name = devm_kasprintf(&hdev->dev, GFP_KERNEL, + "kysona-%s-battery", + strlen(hdev->uniq) ? + hdev->uniq : dev_name(&hdev->dev)); + if (!drv_data->battery_desc.name) + return -ENOMEM; + + drv_data->battery = devm_power_supply_register(&hdev->dev, + &drv_data->battery_desc, &pscfg); + if (IS_ERR(drv_data->battery)) { + ret = PTR_ERR(drv_data->battery); + drv_data->battery = NULL; + hid_err(hdev, "Unable to register battery device\n"); + return ret; + } + + power_supply_powers(drv_data->battery, &hdev->dev); + kysona_fetch_battery(hdev); + + return ret; +} + +static int kysona_probe(struct hid_device *hdev, const struct hid_device_id *id) +{ + int ret; + struct kysona_drvdata *drv_data; + struct usb_interface *usbif; + + if (!hid_is_usb(hdev)) + return -EINVAL; + + usbif = to_usb_interface(hdev->dev.parent); + + drv_data = devm_kzalloc(&hdev->dev, sizeof(*drv_data), GFP_KERNEL); + if (!drv_data) + return -ENOMEM; + + hid_set_drvdata(hdev, drv_data); + drv_data->hdev = hdev; + + ret = hid_parse(hdev); + if (ret) + return ret; + + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT); + if (ret) + return ret; + + if (usbif->cur_altsetting->desc.bInterfaceNumber == 1) { + if (kysona_battery_probe(hdev) < 0) + hid_err(hdev, "Kysona hid battery_probe failed: %d\n", ret); + } + + return 0; +} + +static int kysona_raw_event(struct hid_device *hdev, + struct hid_report *report, u8 *data, int size) +{ + struct kysona_drvdata *drv_data = hid_get_drvdata(hdev); + + if (drv_data->battery && size == sizeof(kysona_battery_request) && + data[0] == 8 && data[1] == BATTERY_REPORT_ID) { + drv_data->battery_capacity = data[6]; + drv_data->battery_charging = data[7]; + drv_data->battery_voltage = (data[8] << 8) | data[9]; + } + + return 0; +} + +static const struct hid_device_id kysona_devices[] = { + { HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_DONGLE) }, + { HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_WIRED) }, + { } +}; +MODULE_DEVICE_TABLE(hid, kysona_devices); + +static struct hid_driver kysona_driver = { + .name = "kysona", + .id_table = kysona_devices, + .probe = kysona_probe, + .raw_event = kysona_raw_event +}; +module_hid_driver(kysona_driver); + +MODULE_LICENSE("GPL"); +MODULE_DESCRIPTION("HID driver for Kysona devices"); +MODULE_AUTHOR("Lode Willems "); From 94ec1cd82f55ebbf5c0d63c8aa7f849fdab2b535 Mon Sep 17 00:00:00 2001 From: Lode Willems Date: Sat, 5 Oct 2024 13:57:05 +0200 Subject: [PATCH 23/40] HID: Kysona: check battery status every 5s using a workqueue Use a workqueue to periodically check the battery status Signed-off-by: Lode Willems Signed-off-by: Jiri Kosina --- drivers/hid/hid-kysona.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-kysona.c b/drivers/hid/hid-kysona.c index 91e359c31fa1..403bdc4a5e12 100644 --- a/drivers/hid/hid-kysona.c +++ b/drivers/hid/hid-kysona.c @@ -12,6 +12,8 @@ #include "hid-ids.h" +#define BATTERY_TIMEOUT_MS 5000 + #define BATTERY_REPORT_ID 4 struct kysona_drvdata { @@ -21,6 +23,7 @@ struct kysona_drvdata { u8 battery_capacity; bool battery_charging; u16 battery_voltage; + struct delayed_work battery_work; }; static enum power_supply_property kysona_battery_props[] = { @@ -104,6 +107,17 @@ static void kysona_fetch_battery(struct hid_device *hdev) "Battery query failed (err: %d)\n", ret); } +static void kysona_battery_timer_tick(struct work_struct *work) +{ + struct kysona_drvdata *drv_data = container_of(work, + struct kysona_drvdata, battery_work.work); + struct hid_device *hdev = drv_data->hdev; + + kysona_fetch_battery(hdev); + schedule_delayed_work(&drv_data->battery_work, + msecs_to_jiffies(BATTERY_TIMEOUT_MS)); +} + static int kysona_battery_probe(struct hid_device *hdev) { struct kysona_drvdata *drv_data = hid_get_drvdata(hdev); @@ -134,7 +148,11 @@ static int kysona_battery_probe(struct hid_device *hdev) } power_supply_powers(drv_data->battery, &hdev->dev); + + INIT_DELAYED_WORK(&drv_data->battery_work, kysona_battery_timer_tick); kysona_fetch_battery(hdev); + schedule_delayed_work(&drv_data->battery_work, + msecs_to_jiffies(BATTERY_TIMEOUT_MS)); return ret; } @@ -188,6 +206,16 @@ static int kysona_raw_event(struct hid_device *hdev, return 0; } +static void kysona_remove(struct hid_device *hdev) +{ + struct kysona_drvdata *drv_data = hid_get_drvdata(hdev); + + if (drv_data->battery) + cancel_delayed_work_sync(&drv_data->battery_work); + + hid_hw_stop(hdev); +} + static const struct hid_device_id kysona_devices[] = { { HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_DONGLE) }, { HID_USB_DEVICE(USB_VENDOR_ID_KYSONA, USB_DEVICE_ID_KYSONA_M600_WIRED) }, @@ -199,7 +227,8 @@ static struct hid_driver kysona_driver = { .name = "kysona", .id_table = kysona_devices, .probe = kysona_probe, - .raw_event = kysona_raw_event + .raw_event = kysona_raw_event, + .remove = kysona_remove }; module_hid_driver(kysona_driver); From 9372b6c4ed80f474810d562058b625309827e36e Mon Sep 17 00:00:00 2001 From: Lode Willems Date: Sat, 5 Oct 2024 13:57:06 +0200 Subject: [PATCH 24/40] HID: Kysona: add basic online status Wait for a response to the battery status request to set the device as online. This prevent wrong power info when the dongle is connected but the mouse is turned off. Signed-off-by: Lode Willems Signed-off-by: Jiri Kosina --- drivers/hid/hid-kysona.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-kysona.c b/drivers/hid/hid-kysona.c index 403bdc4a5e12..d4c0406b3323 100644 --- a/drivers/hid/hid-kysona.c +++ b/drivers/hid/hid-kysona.c @@ -18,6 +18,8 @@ struct kysona_drvdata { struct hid_device *hdev; + bool online; + struct power_supply_desc battery_desc; struct power_supply *battery; u8 battery_capacity; @@ -32,7 +34,8 @@ static enum power_supply_property kysona_battery_props[] = { POWER_SUPPLY_PROP_CAPACITY, POWER_SUPPLY_PROP_SCOPE, POWER_SUPPLY_PROP_MODEL_NAME, - POWER_SUPPLY_PROP_VOLTAGE_NOW + POWER_SUPPLY_PROP_VOLTAGE_NOW, + POWER_SUPPLY_PROP_ONLINE }; static int kysona_battery_get_property(struct power_supply *psy, @@ -46,11 +49,16 @@ static int kysona_battery_get_property(struct power_supply *psy, case POWER_SUPPLY_PROP_PRESENT: val->intval = 1; break; + case POWER_SUPPLY_PROP_ONLINE: + val->intval = drv_data->online; + break; case POWER_SUPPLY_PROP_STATUS: - // TODO: check if device is online - val->intval = drv_data->battery_charging ? - POWER_SUPPLY_STATUS_CHARGING : - POWER_SUPPLY_STATUS_DISCHARGING; + if (drv_data->online) + val->intval = drv_data->battery_charging ? + POWER_SUPPLY_STATUS_CHARGING : + POWER_SUPPLY_STATUS_DISCHARGING; + else + val->intval = POWER_SUPPLY_STATUS_UNKNOWN; break; case POWER_SUPPLY_PROP_SCOPE: val->intval = POWER_SUPPLY_SCOPE_DEVICE; @@ -124,7 +132,9 @@ static int kysona_battery_probe(struct hid_device *hdev) struct power_supply_config pscfg = { .drv_data = drv_data }; int ret = 0; + drv_data->online = false; drv_data->battery_capacity = 100; + drv_data->battery_voltage = 4200; drv_data->battery_desc.properties = kysona_battery_props; drv_data->battery_desc.num_properties = ARRAY_SIZE(kysona_battery_props); @@ -201,6 +211,7 @@ static int kysona_raw_event(struct hid_device *hdev, drv_data->battery_capacity = data[6]; drv_data->battery_charging = data[7]; drv_data->battery_voltage = (data[8] << 8) | data[9]; + drv_data->online = true; } return 0; From a025b0dbd83f85cf81ff447431cfd8b2d3cacb0a Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Thu, 17 Oct 2024 11:31:13 -0700 Subject: [PATCH 25/40] HID: wacom: Set eraser status when either 'Eraser' or 'Invert' usage is set Microsoft defines two slightly different behaviors for pens that are being used to erase. The first one, for pens that can be used while inverted specifies that both 'Invert' and 'Eraser' usages should be set while the pen is in contact and erasing. For pens that use an eraser button though, they specify that only the 'Eraser' usage should be set (while hovering, only the 'Invert' usage is to be set). We used our internal 'invert_state' flag to determine if a pen has an intent to erase (whether hovering or not). That flag was previously only depending on the 'Invert' usage, which was sufficient for the first type of pen (EMR) but not the second type (AES). This commit makes the flag depend on either usage being set, and also renames it to make its function more clear. This change should not normally have an impact on userspace due to both the existing driver and firmware design. The driver already only determines tool type based on the first event in an interaction (e.g. it will see the 'Invert' bit set when the eraser comes into prox and then report BTN_TOOL_RUBBER for the rest of the interaction, even if 'Invert' is cleared). AES firmware is also careful to send reports that work through a set of defined state transitions, even in the corner-case where the eraser button is pressed when the pen is already in contact with the display (Prox|Tip -> Prox -> 0 -> Invert -> Eraser). Regardless, it seems reasonable to ensure the driver's state variables match programmer expectation. Link: https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/windows-pen-states Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 7 +++++-- drivers/hid/wacom_wac.h | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 413606bdf476..fd0de9bae3d9 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2422,9 +2422,11 @@ static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field wacom_wac->hid_data.sense_state = value; return; case HID_DG_INVERT: - wacom_wac->hid_data.invert_state = value; + wacom_wac->hid_data.eraser |= value; return; case HID_DG_ERASER: + wacom_wac->hid_data.eraser |= value; + fallthrough; case HID_DG_TIPSWITCH: wacom_wac->hid_data.tipswitch |= value; return; @@ -2565,7 +2567,7 @@ static void wacom_wac_pen_report(struct hid_device *hdev, if (entering_range) { /* first in range */ /* Going into range select tool */ - if (wacom_wac->hid_data.invert_state) + if (wacom_wac->hid_data.eraser) wacom_wac->tool[0] = BTN_TOOL_RUBBER; else if (wacom_wac->features.quirks & WACOM_QUIRK_AESPEN) wacom_wac->tool[0] = BTN_TOOL_PEN; @@ -2619,6 +2621,7 @@ static void wacom_wac_pen_report(struct hid_device *hdev, } wacom_wac->hid_data.tipswitch = false; + wacom_wac->hid_data.eraser = false; input_sync(input); } diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h index c8803d5c6a71..0c3c6a6aaae9 100644 --- a/drivers/hid/wacom_wac.h +++ b/drivers/hid/wacom_wac.h @@ -300,7 +300,7 @@ struct hid_data { __s16 inputmode_index; /* InputMode HID feature index in the report */ bool sense_state; bool inrange_state; - bool invert_state; + bool eraser; bool tipswitch; bool barrelswitch; bool barrelswitch2; From 2e592244c4874c5e7808ac27f62443e8ff87d901 Mon Sep 17 00:00:00 2001 From: He Lugang Date: Wed, 25 Sep 2024 14:28:24 +0800 Subject: [PATCH 26/40] HID: replace BUG_ON() with WARN_ON() There is no need to kill the system entirely in HID with BUG_ON, use WARN_ON to handle fault more gracefully which allow the system to keep running. Signed-off-by: He Lugang Signed-off-by: Jiri Kosina --- drivers/hid/hid-cp2112.c | 3 ++- drivers/hid/hid-lg4ff.c | 3 ++- drivers/hid/hid-sony.c | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index dae2b84a1490..f4c8d981aa0a 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -852,7 +852,8 @@ static int cp2112_set_usb_config(struct hid_device *hdev, { int ret; - BUG_ON(cfg->report != CP2112_USB_CONFIG); + if (WARN_ON(cfg->report != CP2112_USB_CONFIG)) + return -EINVAL; ret = cp2112_hid_output(hdev, (u8 *)cfg, sizeof(*cfg), HID_FEATURE_REPORT); diff --git a/drivers/hid/hid-lg4ff.c b/drivers/hid/hid-lg4ff.c index e3fcf1353fb3..c0a138f21ca4 100644 --- a/drivers/hid/hid-lg4ff.c +++ b/drivers/hid/hid-lg4ff.c @@ -1350,7 +1350,8 @@ int lg4ff_init(struct hid_device *hid) /* Initialize device properties */ if (mmode_ret == LG4FF_MMODE_IS_MULTIMODE) { - BUG_ON(mmode_idx == -1); + if (WARN_ON(mmode_idx == -1)) + return -EINVAL; mmode_wheel = &lg4ff_multimode_wheels[mmode_idx]; } lg4ff_init_wheel_data(&entry->wdata, &lg4ff_devices[i], mmode_wheel, real_product_id); diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c index d2486f3734f0..5258b45684e8 100644 --- a/drivers/hid/hid-sony.c +++ b/drivers/hid/hid-sony.c @@ -1379,7 +1379,8 @@ static int sony_leds_init(struct sony_sc *sc) u8 max_brightness[MAX_LEDS] = { [0 ... (MAX_LEDS - 1)] = 1 }; u8 use_hw_blink[MAX_LEDS] = { 0 }; - BUG_ON(!(sc->quirks & SONY_LED_SUPPORT)); + if (WARN_ON(!(sc->quirks & SONY_LED_SUPPORT))) + return -EINVAL; if (sc->quirks & BUZZ_CONTROLLER) { sc->led_count = 4; From 9bc089307e8dff7797233308372b4a90ce8f79be Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 17 Oct 2024 18:34:58 +0200 Subject: [PATCH 27/40] HID: bpf: Fix NKRO on Mistel MD770 Mistel MD770 keyboard (using Holtek Semiconductor, Inc. controller) has a quirk in report descriptor in one of its interfaces (more detail in the source file). Fix up the descriptor to allow NKRO to work again. Tested by loading the BPF program and confirming that 8 simultaneous keypresses work. Link: https://bugzilla.kernel.org/show_bug.cgi?id=218495 Link: https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/122 Signed-off-by: Tatsuyuki Ishi Acked-by: Jiri Kosina Link: https://patch.msgid.link/20241017-import_bpf_6-13-v2-1-6a7acb89a97f@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/bpf/progs/Mistel__MD770.bpf.c | 154 ++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 drivers/hid/bpf/progs/Mistel__MD770.bpf.c diff --git a/drivers/hid/bpf/progs/Mistel__MD770.bpf.c b/drivers/hid/bpf/progs/Mistel__MD770.bpf.c new file mode 100644 index 000000000000..fb8b5a6968b1 --- /dev/null +++ b/drivers/hid/bpf/progs/Mistel__MD770.bpf.c @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2024 Tatsuyuki Ishi + */ + +#include "vmlinux.h" +#include "hid_bpf.h" +#include "hid_bpf_helpers.h" +#include + +#define VID_HOLTEK 0x04D9 +#define PID_MD770 0x0339 +#define RDESC_SIZE 203 + +HID_BPF_CONFIG( + HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, VID_HOLTEK, PID_MD770) +); + +/* + * The Mistel MD770 keyboard reports the first 6 simultaneous key presses + * through the first interface, and anything beyond that through a second + * interface. Unfortunately, the second interface's report descriptor has an + * error, causing events to be malformed and ignored. This HID-BPF driver + * fixes the descriptor to allow NKRO to work again. + * + * For reference, this is the original report descriptor: + * + * 0x05, 0x01, // Usage Page (Generic Desktop) 0 + * 0x09, 0x80, // Usage (System Control) 2 + * 0xa1, 0x01, // Collection (Application) 4 + * 0x85, 0x01, // Report ID (1) 6 + * 0x19, 0x81, // Usage Minimum (129) 8 + * 0x29, 0x83, // Usage Maximum (131) 10 + * 0x15, 0x00, // Logical Minimum (0) 12 + * 0x25, 0x01, // Logical Maximum (1) 14 + * 0x95, 0x03, // Report Count (3) 16 + * 0x75, 0x01, // Report Size (1) 18 + * 0x81, 0x02, // Input (Data,Var,Abs) 20 + * 0x95, 0x01, // Report Count (1) 22 + * 0x75, 0x05, // Report Size (5) 24 + * 0x81, 0x01, // Input (Cnst,Arr,Abs) 26 + * 0xc0, // End Collection 28 + * 0x05, 0x0c, // Usage Page (Consumer Devices) 29 + * 0x09, 0x01, // Usage (Consumer Control) 31 + * 0xa1, 0x01, // Collection (Application) 33 + * 0x85, 0x02, // Report ID (2) 35 + * 0x15, 0x00, // Logical Minimum (0) 37 + * 0x25, 0x01, // Logical Maximum (1) 39 + * 0x95, 0x12, // Report Count (18) 41 + * 0x75, 0x01, // Report Size (1) 43 + * 0x0a, 0x83, 0x01, // Usage (AL Consumer Control Config) 45 + * 0x0a, 0x8a, 0x01, // Usage (AL Email Reader) 48 + * 0x0a, 0x92, 0x01, // Usage (AL Calculator) 51 + * 0x0a, 0x94, 0x01, // Usage (AL Local Machine Browser) 54 + * 0x09, 0xcd, // Usage (Play/Pause) 57 + * 0x09, 0xb7, // Usage (Stop) 59 + * 0x09, 0xb6, // Usage (Scan Previous Track) 61 + * 0x09, 0xb5, // Usage (Scan Next Track) 63 + * 0x09, 0xe2, // Usage (Mute) 65 + * 0x09, 0xea, // Usage (Volume Down) 67 + * 0x09, 0xe9, // Usage (Volume Up) 69 + * 0x0a, 0x21, 0x02, // Usage (AC Search) 71 + * 0x0a, 0x23, 0x02, // Usage (AC Home) 74 + * 0x0a, 0x24, 0x02, // Usage (AC Back) 77 + * 0x0a, 0x25, 0x02, // Usage (AC Forward) 80 + * 0x0a, 0x26, 0x02, // Usage (AC Stop) 83 + * 0x0a, 0x27, 0x02, // Usage (AC Refresh) 86 + * 0x0a, 0x2a, 0x02, // Usage (AC Bookmarks) 89 + * 0x81, 0x02, // Input (Data,Var,Abs) 92 + * 0x95, 0x01, // Report Count (1) 94 + * 0x75, 0x0e, // Report Size (14) 96 + * 0x81, 0x01, // Input (Cnst,Arr,Abs) 98 + * 0xc0, // End Collection 100 + * 0x05, 0x01, // Usage Page (Generic Desktop) 101 + * 0x09, 0x02, // Usage (Mouse) 103 + * 0xa1, 0x01, // Collection (Application) 105 + * 0x09, 0x01, // Usage (Pointer) 107 + * 0xa1, 0x00, // Collection (Physical) 109 + * 0x85, 0x03, // Report ID (3) 111 + * 0x05, 0x09, // Usage Page (Button) 113 + * 0x19, 0x01, // Usage Minimum (1) 115 + * 0x29, 0x08, // Usage Maximum (8) 117 + * 0x15, 0x00, // Logical Minimum (0) 119 + * 0x25, 0x01, // Logical Maximum (1) 121 + * 0x75, 0x01, // Report Size (1) 123 + * 0x95, 0x08, // Report Count (8) 125 + * 0x81, 0x02, // Input (Data,Var,Abs) 127 + * 0x05, 0x01, // Usage Page (Generic Desktop) 129 + * 0x09, 0x30, // Usage (X) 131 + * 0x09, 0x31, // Usage (Y) 133 + * 0x16, 0x01, 0x80, // Logical Minimum (-32767) 135 + * 0x26, 0xff, 0x7f, // Logical Maximum (32767) 138 + * 0x75, 0x10, // Report Size (16) 141 + * 0x95, 0x02, // Report Count (2) 143 + * 0x81, 0x06, // Input (Data,Var,Rel) 145 + * 0x09, 0x38, // Usage (Wheel) 147 + * 0x15, 0x81, // Logical Minimum (-127) 149 + * 0x25, 0x7f, // Logical Maximum (127) 151 + * 0x75, 0x08, // Report Size (8) 153 + * 0x95, 0x01, // Report Count (1) 155 + * 0x81, 0x06, // Input (Data,Var,Rel) 157 + * 0x05, 0x0c, // Usage Page (Consumer Devices) 159 + * 0x0a, 0x38, 0x02, // Usage (AC Pan) 161 + * 0x95, 0x01, // Report Count (1) 164 + * 0x81, 0x06, // Input (Data,Var,Rel) 166 + * 0xc0, // End Collection 168 + * 0xc0, // End Collection 169 + * 0x05, 0x01, // Usage Page (Generic Desktop) 170 + * 0x09, 0x06, // Usage (Keyboard) 172 + * 0xa1, 0x01, // Collection (Application) 174 + * 0x85, 0x04, // Report ID (4) 176 + * 0x05, 0x07, // Usage Page (Keyboard) 178 + * 0x95, 0x01, // Report Count (1) 180 + * 0x75, 0x08, // Report Size (8) 182 + * 0x81, 0x03, // Input (Cnst,Var,Abs) 184 + * 0x95, 0xe8, // Report Count (232) 186 + * 0x75, 0x01, // Report Size (1) 188 + * 0x15, 0x00, // Logical Minimum (0) 190 + * 0x25, 0x01, // Logical Maximum (1) 192 + * 0x05, 0x07, // Usage Page (Keyboard) 194 + * 0x19, 0x00, // Usage Minimum (0) 196 + * 0x29, 0xe7, // Usage Maximum (231) 198 + * 0x81, 0x00, // Input (Data,Arr,Abs) 200 <- change to 0x81, 0x02 (Data,Var,Abs) + * 0xc0, // End Collection 202 + */ + +SEC(HID_BPF_RDESC_FIXUP) +int BPF_PROG(hid_rdesc_fixup_mistel_md770, struct hid_bpf_ctx *hctx) +{ + __u8 *data = hid_bpf_get_data(hctx, 0, HID_MAX_DESCRIPTOR_SIZE); + + if (!data) + return 0; /* EPERM check */ + + if (data[201] == 0x00) + data[201] = 0x02; + + return 0; +} + +HID_BPF_OPS(mistel_md770) = { + .hid_rdesc_fixup = (void *)hid_rdesc_fixup_mistel_md770, +}; + +SEC("syscall") +int probe(struct hid_bpf_probe_args *ctx) +{ + ctx->retval = ctx->rdesc_size != RDESC_SIZE; + if (ctx->retval) + ctx->retval = -EINVAL; + + return 0; +} + +char _license[] SEC("license") = "GPL"; From cee9faff2f65d00fe8a56c1223c9c52435eb525d Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 17 Oct 2024 18:34:59 +0200 Subject: [PATCH 28/40] HID: bpf: Fix Rapoo M50 Plus Silent side buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rapoo M50 Plus Silent mouse has 2 side buttons in addition to the left, right and middles buttons. However, its original HID descriptor has a Usage Maximum of 3, preventing the side buttons to work. This HID-BPF driver changes that usage to 5. Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/1015 Link: https://gitlab.freedesktop.org/libevdev/udev-hid-bpf/-/merge_requests/116 Signed-off-by: José Expósito Acked-by: Jiri Kosina Link: https://patch.msgid.link/20241017-import_bpf_6-13-v2-2-6a7acb89a97f@kernel.org Signed-off-by: Benjamin Tissoires --- .../bpf/progs/Rapoo__M50-Plus-Silent.bpf.c | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 drivers/hid/bpf/progs/Rapoo__M50-Plus-Silent.bpf.c diff --git a/drivers/hid/bpf/progs/Rapoo__M50-Plus-Silent.bpf.c b/drivers/hid/bpf/progs/Rapoo__M50-Plus-Silent.bpf.c new file mode 100644 index 000000000000..6b379e45f531 --- /dev/null +++ b/drivers/hid/bpf/progs/Rapoo__M50-Plus-Silent.bpf.c @@ -0,0 +1,148 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* Copyright (c) 2024 José Expósito + */ + +#include "vmlinux.h" +#include "hid_bpf.h" +#include "hid_bpf_helpers.h" +#include + +#define VID_RAPOO 0x24AE +#define PID_M50 0x2015 +#define RDESC_SIZE 186 + +HID_BPF_CONFIG( + HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, VID_RAPOO, PID_M50) +); + +/* + * The Rapoo M50 Plus Silent mouse has 2 side buttons in addition to the left, + * right and middle buttons. However, its original HID descriptor has a Usage + * Maximum of 3, preventing the side buttons to work. This HID-BPF driver + * changes that usage to 5. + * + * For reference, this is the original report descriptor: + * + * 0x05, 0x01, // Usage Page (Generic Desktop) 0 + * 0x09, 0x02, // Usage (Mouse) 2 + * 0xa1, 0x01, // Collection (Application) 4 + * 0x85, 0x01, // Report ID (1) 6 + * 0x09, 0x01, // Usage (Pointer) 8 + * 0xa1, 0x00, // Collection (Physical) 10 + * 0x05, 0x09, // Usage Page (Button) 12 + * 0x19, 0x01, // Usage Minimum (1) 14 + * 0x29, 0x03, // Usage Maximum (3) 16 <- change to 0x05 + * 0x15, 0x00, // Logical Minimum (0) 18 + * 0x25, 0x01, // Logical Maximum (1) 20 + * 0x75, 0x01, // Report Size (1) 22 + * 0x95, 0x05, // Report Count (5) 24 + * 0x81, 0x02, // Input (Data,Var,Abs) 26 + * 0x75, 0x03, // Report Size (3) 28 + * 0x95, 0x01, // Report Count (1) 30 + * 0x81, 0x01, // Input (Cnst,Arr,Abs) 32 + * 0x05, 0x01, // Usage Page (Generic Desktop) 34 + * 0x09, 0x30, // Usage (X) 36 + * 0x09, 0x31, // Usage (Y) 38 + * 0x16, 0x01, 0x80, // Logical Minimum (-32767) 40 + * 0x26, 0xff, 0x7f, // Logical Maximum (32767) 43 + * 0x75, 0x10, // Report Size (16) 46 + * 0x95, 0x02, // Report Count (2) 48 + * 0x81, 0x06, // Input (Data,Var,Rel) 50 + * 0x09, 0x38, // Usage (Wheel) 52 + * 0x15, 0x81, // Logical Minimum (-127) 54 + * 0x25, 0x7f, // Logical Maximum (127) 56 + * 0x75, 0x08, // Report Size (8) 58 + * 0x95, 0x01, // Report Count (1) 60 + * 0x81, 0x06, // Input (Data,Var,Rel) 62 + * 0xc0, // End Collection 64 + * 0xc0, // End Collection 65 + * 0x05, 0x0c, // Usage Page (Consumer Devices) 66 + * 0x09, 0x01, // Usage (Consumer Control) 68 + * 0xa1, 0x01, // Collection (Application) 70 + * 0x85, 0x02, // Report ID (2) 72 + * 0x75, 0x10, // Report Size (16) 74 + * 0x95, 0x01, // Report Count (1) 76 + * 0x15, 0x01, // Logical Minimum (1) 78 + * 0x26, 0x8c, 0x02, // Logical Maximum (652) 80 + * 0x19, 0x01, // Usage Minimum (1) 83 + * 0x2a, 0x8c, 0x02, // Usage Maximum (652) 85 + * 0x81, 0x00, // Input (Data,Arr,Abs) 88 + * 0xc0, // End Collection 90 + * 0x05, 0x01, // Usage Page (Generic Desktop) 91 + * 0x09, 0x80, // Usage (System Control) 93 + * 0xa1, 0x01, // Collection (Application) 95 + * 0x85, 0x03, // Report ID (3) 97 + * 0x09, 0x82, // Usage (System Sleep) 99 + * 0x09, 0x81, // Usage (System Power Down) 101 + * 0x09, 0x83, // Usage (System Wake Up) 103 + * 0x15, 0x00, // Logical Minimum (0) 105 + * 0x25, 0x01, // Logical Maximum (1) 107 + * 0x19, 0x01, // Usage Minimum (1) 109 + * 0x29, 0x03, // Usage Maximum (3) 111 + * 0x75, 0x01, // Report Size (1) 113 + * 0x95, 0x03, // Report Count (3) 115 + * 0x81, 0x02, // Input (Data,Var,Abs) 117 + * 0x95, 0x05, // Report Count (5) 119 + * 0x81, 0x01, // Input (Cnst,Arr,Abs) 121 + * 0xc0, // End Collection 123 + * 0x05, 0x01, // Usage Page (Generic Desktop) 124 + * 0x09, 0x00, // Usage (Undefined) 126 + * 0xa1, 0x01, // Collection (Application) 128 + * 0x85, 0x05, // Report ID (5) 130 + * 0x06, 0x00, 0xff, // Usage Page (Vendor Defined Page 1) 132 + * 0x09, 0x01, // Usage (Vendor Usage 1) 135 + * 0x15, 0x81, // Logical Minimum (-127) 137 + * 0x25, 0x7f, // Logical Maximum (127) 139 + * 0x75, 0x08, // Report Size (8) 141 + * 0x95, 0x07, // Report Count (7) 143 + * 0xb1, 0x02, // Feature (Data,Var,Abs) 145 + * 0xc0, // End Collection 147 + * 0x06, 0x00, 0xff, // Usage Page (Vendor Defined Page 1) 148 + * 0x09, 0x0e, // Usage (Vendor Usage 0x0e) 151 + * 0xa1, 0x01, // Collection (Application) 153 + * 0x85, 0xba, // Report ID (186) 155 + * 0x95, 0x1f, // Report Count (31) 157 + * 0x75, 0x08, // Report Size (8) 159 + * 0x26, 0xff, 0x00, // Logical Maximum (255) 161 + * 0x15, 0x00, // Logical Minimum (0) 164 + * 0x09, 0x01, // Usage (Vendor Usage 1) 166 + * 0x91, 0x02, // Output (Data,Var,Abs) 168 + * 0x85, 0xba, // Report ID (186) 170 + * 0x95, 0x1f, // Report Count (31) 172 + * 0x75, 0x08, // Report Size (8) 174 + * 0x26, 0xff, 0x00, // Logical Maximum (255) 176 + * 0x15, 0x00, // Logical Minimum (0) 179 + * 0x09, 0x01, // Usage (Vendor Usage 1) 181 + * 0x81, 0x02, // Input (Data,Var,Abs) 183 + * 0xc0, // End Collection 185 + */ + +SEC(HID_BPF_RDESC_FIXUP) +int BPF_PROG(hid_rdesc_fixup_rapoo_m50, struct hid_bpf_ctx *hctx) +{ + __u8 *data = hid_bpf_get_data(hctx, 0, HID_MAX_DESCRIPTOR_SIZE); + + if (!data) + return 0; /* EPERM check */ + + if (data[17] == 0x03) + data[17] = 0x05; + + return 0; +} + +HID_BPF_OPS(rapoo_m50) = { + .hid_rdesc_fixup = (void *)hid_rdesc_fixup_rapoo_m50, +}; + +SEC("syscall") +int probe(struct hid_bpf_probe_args *ctx) +{ + ctx->retval = ctx->rdesc_size != RDESC_SIZE; + if (ctx->retval) + ctx->retval = -EINVAL; + + return 0; +} + +char _license[] SEC("license") = "GPL"; From b6d8c474e26517f944208e4645a2a09a6eeee88e Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 17 Oct 2024 18:35:00 +0200 Subject: [PATCH 29/40] HID: bpf: drop use of Logical|Physical|UsageRange Replace with individual Minimum/Maximum calls to match the HID report descriptor - HID doesn't have a Range field. Abstracting this is good for hand-written descriptors but almost all tools will output min/max instead so let's stick with that. Signed-off-by: Peter Hutterer Acked-by: Jiri Kosina Link: https://patch.msgid.link/20241017-import_bpf_6-13-v2-3-6a7acb89a97f@kernel.org Signed-off-by: Benjamin Tissoires --- drivers/hid/bpf/progs/Huion__Dial-2.bpf.c | 66 ++++++++++++------- .../hid/bpf/progs/Huion__Inspiroy-2-S.bpf.c | 60 +++++++++++------ drivers/hid/bpf/progs/hid_report_helpers.h | 36 +++++++--- 3 files changed, 111 insertions(+), 51 deletions(-) diff --git a/drivers/hid/bpf/progs/Huion__Dial-2.bpf.c b/drivers/hid/bpf/progs/Huion__Dial-2.bpf.c index 2411dec6db08..9670e5ef8d54 100644 --- a/drivers/hid/bpf/progs/Huion__Dial-2.bpf.c +++ b/drivers/hid/bpf/progs/Huion__Dial-2.bpf.c @@ -214,7 +214,8 @@ static const __u8 fixed_rdesc_pad[] = { CollectionApplication( // -- Byte 0 in report ReportId(PAD_REPORT_ID) - LogicalRange_i8(0, 1) + LogicalMaximum_i8(0) + LogicalMaximum_i8(1) UsagePage_Digitizers Usage_Dig_TabletFunctionKeys CollectionPhysical( @@ -234,14 +235,17 @@ static const __u8 fixed_rdesc_pad[] = { Input(Var|Abs) // Byte 4 in report is the dial Usage_GD_Wheel - LogicalRange_i8(-1, 1) + LogicalMinimum_i8(-1) + LogicalMaximum_i8(1) ReportCount(1) ReportSize(8) Input(Var|Rel) // Byte 5 is the button state UsagePage_Button - UsageRange_i8(0x01, 0x8) - LogicalRange_i8(0x0, 0x1) + UsageMinimum_i8(0x01) + UsageMaximum_i8(0x08) + LogicalMinimum_i8(0x0) + LogicalMaximum_i8(0x1) ReportCount(7) ReportSize(1) Input(Var|Abs) @@ -265,7 +269,8 @@ static const __u8 fixed_rdesc_pen[] = { Usage_Dig_TipSwitch Usage_Dig_BarrelSwitch Usage_Dig_SecondaryBarrelSwitch // maps eraser to BTN_STYLUS2 - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) ReportSize(1) ReportCount(3) Input(Var|Abs) @@ -280,22 +285,28 @@ static const __u8 fixed_rdesc_pen[] = { UsagePage_GenericDesktop Unit(cm) UnitExponent(-1) - PhysicalRange_i16(0, 266) - LogicalRange_i16(0, 32767) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(266) + LogicalMinimum_i16(0) + LogicalMaximum_i16(32767) Usage_GD_X Input(Var|Abs) // Bytes 2+3 - PhysicalRange_i16(0, 166) - LogicalRange_i16(0, 32767) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(166) + LogicalMinimum_i16(0) + LogicalMaximum_i16(32767) Usage_GD_Y Input(Var|Abs) // Bytes 4+5 ) UsagePage_Digitizers Usage_Dig_TipPressure - LogicalRange_i16(0, 8191) + LogicalMinimum_i16(0) + LogicalMaximum_i16(8191) Input(Var|Abs) // Byte 6+7 ReportSize(8) ReportCount(2) - LogicalRange_i8(-60, 60) + LogicalMinimum_i8(-60) + LogicalMaximum_i8(60) Usage_Dig_XTilt Usage_Dig_YTilt Input(Var|Abs) // Byte 8+9 @@ -313,7 +324,8 @@ static const __u8 fixed_rdesc_vendor[] = { Usage_Dig_Pen CollectionPhysical( // Byte 1 are the buttons - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) ReportSize(1) Usage_Dig_TipSwitch Usage_Dig_BarrelSwitch @@ -333,25 +345,31 @@ static const __u8 fixed_rdesc_vendor[] = { UnitExponent(-1) // Note: reported logical range differs // from the pen report ID for x and y - LogicalRange_i16(0, 53340) - PhysicalRange_i16(0, 266) + LogicalMinimum_i16(0) + LogicalMaximum_i16(53340) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(266) // Bytes 2/3 in report Usage_GD_X Input(Var|Abs) - LogicalRange_i16(0, 33340) - PhysicalRange_i16(0, 166) + LogicalMinimum_i16(0) + LogicalMaximum_i16(33340) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(166) // Bytes 4/5 in report Usage_GD_Y Input(Var|Abs) ) // Bytes 6/7 in report - LogicalRange_i16(0, 8191) + LogicalMinimum_i16(0) + LogicalMaximum_i16(8191) Usage_Dig_TipPressure Input(Var|Abs) // Bytes 8/9 in report ReportCount(1) // Padding Input(Const) - LogicalRange_i8(-60, 60) + LogicalMinimum_i8(-60) + LogicalMaximum_i8(60) // Byte 10 in report Usage_Dig_XTilt // Byte 11 in report @@ -366,7 +384,8 @@ static const __u8 fixed_rdesc_vendor[] = { CollectionApplication( // Byte 0 ReportId(PAD_REPORT_ID) - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) UsagePage_Digitizers Usage_Dig_TabletFunctionKeys CollectionPhysical( @@ -386,15 +405,18 @@ static const __u8 fixed_rdesc_vendor[] = { Input(Var|Abs) // Byte 4 is the button state UsagePage_Button - UsageRange_i8(0x01, 0x8) - LogicalRange_i8(0x0, 0x1) + UsageMinimum_i8(0x1) + UsageMaximum_i8(0x8) + LogicalMinimum_i8(0x0) + LogicalMaximum_i8(0x1) ReportCount(8) ReportSize(1) Input(Var|Abs) // Byte 5 is the top dial UsagePage_GenericDesktop Usage_GD_Wheel - LogicalRange_i8(-1, 1) + LogicalMinimum_i8(-1) + LogicalMaximum_i8(1) ReportCount(1) ReportSize(8) Input(Var|Rel) diff --git a/drivers/hid/bpf/progs/Huion__Inspiroy-2-S.bpf.c b/drivers/hid/bpf/progs/Huion__Inspiroy-2-S.bpf.c index b09b80132368..13f64fb49800 100644 --- a/drivers/hid/bpf/progs/Huion__Inspiroy-2-S.bpf.c +++ b/drivers/hid/bpf/progs/Huion__Inspiroy-2-S.bpf.c @@ -170,7 +170,8 @@ static const __u8 fixed_rdesc_pad[] = { CollectionApplication( // -- Byte 0 in report ReportId(PAD_REPORT_ID) - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) UsagePage_Digitizers Usage_Dig_TabletFunctionKeys CollectionPhysical( @@ -190,14 +191,17 @@ static const __u8 fixed_rdesc_pad[] = { Input(Var|Abs) // Byte 4 in report is the wheel Usage_GD_Wheel - LogicalRange_i8(-1, 1) + LogicalMinimum_i8(-1) + LogicalMaximum_i8(1) ReportCount(1) ReportSize(8) Input(Var|Rel) // Byte 5 is the button state UsagePage_Button - UsageRange_i8(0x01, 0x6) - LogicalRange_i8(0x01, 0x6) + UsageMinimum_i8(0x1) + UsageMaximum_i8(0x6) + LogicalMinimum_i8(0x1) + LogicalMaximum_i8(0x6) ReportCount(1) ReportSize(8) Input(Arr|Abs) @@ -219,7 +223,8 @@ static const __u8 fixed_rdesc_pen[] = { Usage_Dig_TipSwitch Usage_Dig_BarrelSwitch Usage_Dig_SecondaryBarrelSwitch // maps eraser to BTN_STYLUS2 - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) ReportSize(1) ReportCount(3) Input(Var|Abs) @@ -234,18 +239,23 @@ static const __u8 fixed_rdesc_pen[] = { UsagePage_GenericDesktop Unit(cm) UnitExponent(-1) - PhysicalRange_i16(0, 160) - LogicalRange_i16(0, 32767) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(160) + LogicalMinimum_i16(0) + LogicalMaximum_i16(32767) Usage_GD_X Input(Var|Abs) // Bytes 2+3 - PhysicalRange_i16(0, 100) - LogicalRange_i16(0, 32767) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(100) + LogicalMinimum_i16(0) + LogicalMaximum_i16(32767) Usage_GD_Y Input(Var|Abs) // Bytes 4+5 ) UsagePage_Digitizers Usage_Dig_TipPressure - LogicalRange_i16(0, 8191) + LogicalMinimum_i16(0) + LogicalMaximum_i16(8191) Input(Var|Abs) // Byte 6+7 // Two bytes padding so we don't need to change the report at all ReportSize(8) @@ -265,7 +275,8 @@ static const __u8 fixed_rdesc_vendor[] = { Usage_Dig_Pen CollectionPhysical( // Byte 1 are the buttons - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) ReportSize(1) Usage_Dig_TipSwitch Usage_Dig_BarrelSwitch @@ -285,19 +296,24 @@ static const __u8 fixed_rdesc_vendor[] = { UnitExponent(-1) // Note: reported logical range differs // from the pen report ID for x and y - LogicalRange_i16(0, 32000) - PhysicalRange_i16(0, 160) + LogicalMinimum_i16(0) + LogicalMaximum_i16(32000) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(160) // Bytes 2/3 in report Usage_GD_X Input(Var|Abs) - LogicalRange_i16(0, 20000) - PhysicalRange_i16(0, 100) + LogicalMinimum_i16(0) + LogicalMaximum_i16(20000) + PhysicalMinimum_i16(0) + PhysicalMaximum_i16(100) // Bytes 4/5 in report Usage_GD_Y Input(Var|Abs) ) // Bytes 6/7 in report - LogicalRange_i16(0, 8192) + LogicalMinimum_i16(0) + LogicalMaximum_i16(8192) Usage_Dig_TipPressure Input(Var|Abs) ) @@ -307,7 +323,8 @@ static const __u8 fixed_rdesc_vendor[] = { CollectionApplication( // Byte 0 ReportId(PAD_REPORT_ID) - LogicalRange_i8(0, 1) + LogicalMinimum_i8(0) + LogicalMaximum_i8(1) UsagePage_Digitizers Usage_Dig_TabletFunctionKeys CollectionPhysical( @@ -327,8 +344,10 @@ static const __u8 fixed_rdesc_vendor[] = { Input(Var|Abs) // Byte 4 is the button state UsagePage_Button - UsageRange_i8(0x01, 0x6) - LogicalRange_i8(0x0, 0x1) + UsageMinimum_i8(0x1) + UsageMaximum_i8(0x6) + LogicalMinimum_i8(0x0) + LogicalMaximum_i8(0x1) ReportCount(6) ReportSize(1) Input(Var|Abs) @@ -337,7 +356,8 @@ static const __u8 fixed_rdesc_vendor[] = { // Byte 5 is the wheel UsagePage_GenericDesktop Usage_GD_Wheel - LogicalRange_i8(-1, 1) + LogicalMinimum_i8(-1) + LogicalMaximum_i8(1) ReportCount(1) ReportSize(8) Input(Var|Rel) diff --git a/drivers/hid/bpf/progs/hid_report_helpers.h b/drivers/hid/bpf/progs/hid_report_helpers.h index 0aa1df438eeb..9b2a48e4a311 100644 --- a/drivers/hid/bpf/progs/hid_report_helpers.h +++ b/drivers/hid/bpf/progs/hid_report_helpers.h @@ -52,7 +52,8 @@ * Usage_GD_Keyboard * CollectionApplication( ← Open the collection * ReportId(3) - * LogicalRange_i8(0, 1) + * LogicalMinimum_i8(0) + * LogicalMaximum_i8(1) * // other fields * ) ← End EndCollection * @@ -74,26 +75,43 @@ #define Arr 0x0 #define Abs 0x0 #define Rel 0x4 +#define Null 0x40 +#define Buff 0x0100 /* Use like this: Input(Var|Abs) */ #define Input(i_) 0x081, i8(i_), #define Output(i_) 0x091, i8(i_), #define Feature(i_) 0x0b1, i8(i_), +#define Input_i16(i_) 0x082, LE16(i_), +#define Output_i16(i_) 0x092, LE16(i_), +#define Feature_i16(i_) 0x0b2, LE16(i_), + #define ReportId(id_) 0x85, i8(id_), #define ReportSize(sz_) 0x75, i8(sz_), #define ReportCount(cnt_) 0x95, i8(cnt_), -#define LogicalRange_i8(min_, max_) 0x15, i8(min_), 0x25, i8(max_), -#define LogicalRange_i16(min_, max_) 0x16, LE16(min_), 0x26, LE16(max_), -#define LogicalRange_i32(min_, max_) 0x17, LE32(min_), 0x27, LE32(max_), +#define LogicalMinimum_i8(min_) 0x15, i8(min_), +#define LogicalMinimum_i16(min_) 0x16, LE16(min_), +#define LogicalMinimum_i32(min_) 0x17, LE32(min_), -#define PhysicalRange_i8(min_, max_) 0x35, i8(min_), 0x45, i8(max_), -#define PhysicalRange_i16(min_, max_) 0x36, LE16(min_), 0x46, LE16(max_), -#define PhysicalRange_i32(min_, max_) 0x37, LE32(min_), 0x47, LE32(max_), +#define LogicalMaximum_i8(max_) 0x25, i8(max_), +#define LogicalMaximum_i16(max_) 0x26, LE16(max_), +#define LogicalMaximum_i32(max_) 0x27, LE32(max_), -#define UsageRange_i8(min_, max_) 0x19, i8(min_), 0x29, i8(max_), -#define UsageRange_i16(min_, max_) 0x1a, LE16(min_), 0x2a, LE16(max_), +#define PhysicalMinimum_i8(min_) 0x35, i8(min_), +#define PhysicalMinimum_i16(min_) 0x36, LE16(min_), +#define PhysicalMinimum_i32(min_) 0x37, LE32(min_), + +#define PhysicalMaximum_i8(max_) 0x45, i8(max_), +#define PhysicalMaximum_i16(max_) 0x46, LE16(max_), +#define PhysicalMaximum_i32(max_) 0x47, LE32(max_), + +#define UsageMinimum_i8(min_) 0x19, i8(min_), +#define UsageMinimum_i16(min_) 0x1a, LE16(min_), + +#define UsageMaximum_i8(max_) 0x29, i8(max_), +#define UsageMaximum_i16(max_) 0x2a, LE16(max_), #define UsagePage_i8(p_) 0x05, i8(p_), #define UsagePage_i16(p_) 0x06, LE16(p_), From be8f7f2281a26abbda26ce37c5ee4674022a7a2f Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Thu, 31 Oct 2024 20:31:12 +0800 Subject: [PATCH 30/40] HID: hid-goodix: Return 0 when receiving an empty HID feature package Align with the i2c-hid driver by returning 0 instead of -EINVAL when an empty response is received, ensuring that userspace programs utilizing the hidraw node receive consistent return values. Signed-off-by: Charles Wang Signed-off-by: Jiri Kosina --- drivers/hid/hid-goodix-spi.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c index 0f87bf9c67cf..077a91ee1d37 100644 --- a/drivers/hid/hid-goodix-spi.c +++ b/drivers/hid/hid-goodix-spi.c @@ -356,7 +356,7 @@ static int goodix_hid_check_ack_status(struct goodix_ts_data *ts, u32 *resp_len) dev_err(ts->dev, "hrd.size too short: %d", len); return -EINVAL; } - *resp_len = len; + *resp_len = len - GOODIX_HID_PKG_LEN_SIZE; return 0; } @@ -446,7 +446,10 @@ static int goodix_hid_get_raw_report(struct hid_device *hid, if (error) return error; - len = min(len, response_data_len - GOODIX_HID_PKG_LEN_SIZE); + /* Empty reprot response */ + if (!response_data_len) + return 0; + len = min(len, response_data_len); /* Step3: read response data(skip 2bytes of hid pkg length) */ error = goodix_spi_read(ts, ts->hid_report_addr + GOODIX_HID_ACK_HEADER_SIZE + From 253ed2740be0267ef75b181767b1999ab755bb84 Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Thu, 31 Oct 2024 20:31:13 +0800 Subject: [PATCH 31/40] HID: hid-goodix: Fix HID get/set feature operation overwritten problem Implement the hid get/set feature report function using a separate address, rather than sharing an address with coordinate reporting, to prevent feature events from being overwritten by coordinate events. Signed-off-by: Charles Wang Signed-off-by: Jiri Kosina --- drivers/hid/hid-goodix-spi.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c index 077a91ee1d37..6ae2300a603e 100644 --- a/drivers/hid/hid-goodix-spi.c +++ b/drivers/hid/hid-goodix-spi.c @@ -19,6 +19,7 @@ #define GOODIX_HID_DESC_ADDR 0x1058C #define GOODIX_HID_REPORT_DESC_ADDR 0x105AA #define GOODIX_HID_SIGN_ADDR 0x10D32 +#define GOODIX_HID_CMD_ADDR 0x10364 #define GOODIX_HID_GET_REPORT_CMD 0x02 #define GOODIX_HID_SET_REPORT_CMD 0x03 @@ -348,7 +349,7 @@ static int goodix_hid_check_ack_status(struct goodix_ts_data *ts, u32 *resp_len) * - byte 0: Ack flag, value of 1 for data ready * - bytes 1-2: Response data length */ - error = goodix_spi_read(ts, ts->hid_report_addr, + error = goodix_spi_read(ts, GOODIX_HID_CMD_ADDR, &hdr, sizeof(hdr)); if (!error && (hdr.flag & GOODIX_HID_ACK_READY_FLAG)) { len = le16_to_cpu(hdr.size); @@ -431,7 +432,7 @@ static int goodix_hid_get_raw_report(struct hid_device *hid, tx_len += args_len; /* Step1: write report request info */ - error = goodix_spi_write(ts, ts->hid_report_addr, tmp_buf, tx_len); + error = goodix_spi_write(ts, GOODIX_HID_CMD_ADDR, tmp_buf, tx_len); if (error) { dev_err(ts->dev, "failed send read feature cmd, %d", error); return error; @@ -451,7 +452,7 @@ static int goodix_hid_get_raw_report(struct hid_device *hid, return 0; len = min(len, response_data_len); /* Step3: read response data(skip 2bytes of hid pkg length) */ - error = goodix_spi_read(ts, ts->hid_report_addr + + error = goodix_spi_read(ts, GOODIX_HID_CMD_ADDR + GOODIX_HID_ACK_HEADER_SIZE + GOODIX_HID_PKG_LEN_SIZE, buf, len); if (error) { @@ -521,7 +522,7 @@ static int goodix_hid_set_raw_report(struct hid_device *hid, memcpy(tmp_buf + tx_len, buf, len); tx_len += len; - error = goodix_spi_write(ts, ts->hid_report_addr, tmp_buf, tx_len); + error = goodix_spi_write(ts, GOODIX_HID_CMD_ADDR, tmp_buf, tx_len); if (error) { dev_err(ts->dev, "failed send report: %*ph", tx_len, tmp_buf); return error; @@ -752,7 +753,7 @@ static int goodix_spi_set_power(struct goodix_ts_data *ts, int power_state) power_control_cmd[5] = power_state; guard(mutex)(&ts->hid_request_lock); - error = goodix_spi_write(ts, ts->hid_report_addr, power_control_cmd, + error = goodix_spi_write(ts, GOODIX_HID_CMD_ADDR, power_control_cmd, sizeof(power_control_cmd)); if (error) { dev_err(ts->dev, "failed set power mode: %s", From 138a339e39bb5acf6b0d0d1da91387f5f0a7dabd Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Wed, 23 Oct 2024 13:24:37 +0200 Subject: [PATCH 32/40] HID: steelseries: Fix battery requests stopping after some time In some cases, the headset receiver will answer one of our requests with garbage, or not at all. This is a problem when we only request battery information once we've received a battery response, as we might never get to request battery information again. If the data from the receiver could not be parsed, and there's no pending battery requests, schedule a new request. Signed-off-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-steelseries.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index 7e83fee1ffa0..16138f7dae17 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -603,8 +603,11 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, hid_dbg(sd->hdev, "Parsing raw event for Arctis 1 headset (%*ph)\n", size, read_buf); if (size < ARCTIS_1_BATTERY_RESPONSE_LEN || - memcmp (read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request))) + memcmp(read_buf, arctis_1_battery_request, sizeof(arctis_1_battery_request))) { + if (!delayed_work_pending(&sd->battery_work)) + goto request_battery; return 0; + } if (read_buf[2] == 0x01) { connected = false; capacity = 100; @@ -631,6 +634,7 @@ static int steelseries_headset_raw_event(struct hid_device *hdev, power_supply_changed(sd->battery); } +request_battery: spin_lock_irqsave(&sd->lock, flags); if (!sd->removed) schedule_delayed_work(&sd->battery_work, From 8ee0f23e2672f004e217b38d4fac956cee251c7f Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Wed, 23 Oct 2024 13:24:38 +0200 Subject: [PATCH 33/40] HID: steelseries: Add capacity_level mapping The capacity level mappings are taken from: https://support.steelseries.com/hc/en-us/articles/360049205612-How-do-I-know-the-Arctis-battery-level-how-do-I-charge-the-Arctis Even if we have a percentage, exporting a capacity_level that matches with the hardware warning levels means that upower can show warnings at the same time as the hardware. So the headset starts beeping at the same time as the critical warning notification appears :eyeroll: Signed-off-by: Bastien Nocera Signed-off-by: Jiri Kosina --- drivers/hid/hid-steelseries.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c index 16138f7dae17..f9ff5be94309 100644 --- a/drivers/hid/hid-steelseries.c +++ b/drivers/hid/hid-steelseries.c @@ -411,6 +411,15 @@ static void steelseries_headset_fetch_battery(struct hid_device *hdev) "Battery query failed (err: %d)\n", ret); } +static int battery_capacity_to_level(int capacity) +{ + if (capacity >= 50) + return POWER_SUPPLY_CAPACITY_LEVEL_NORMAL; + if (capacity >= 20) + return POWER_SUPPLY_CAPACITY_LEVEL_LOW; + return POWER_SUPPLY_CAPACITY_LEVEL_CRITICAL; +} + static void steelseries_headset_battery_timer_tick(struct work_struct *work) { struct steelseries_device *sd = container_of(work, @@ -442,6 +451,9 @@ static int steelseries_headset_battery_get_property(struct power_supply *psy, case POWER_SUPPLY_PROP_CAPACITY: val->intval = sd->battery_capacity; break; + case POWER_SUPPLY_PROP_CAPACITY_LEVEL: + val->intval = battery_capacity_to_level(sd->battery_capacity); + break; default: ret = -EINVAL; break; @@ -469,6 +481,7 @@ static enum power_supply_property steelseries_headset_battery_props[] = { POWER_SUPPLY_PROP_STATUS, POWER_SUPPLY_PROP_SCOPE, POWER_SUPPLY_PROP_CAPACITY, + POWER_SUPPLY_PROP_CAPACITY_LEVEL, }; static int steelseries_headset_battery_register(struct steelseries_device *sd) From 49a397ad24ee5e2c53a59dada2780d7e71bd3f77 Mon Sep 17 00:00:00 2001 From: Jason Gerecke Date: Mon, 28 Oct 2024 10:39:14 -0700 Subject: [PATCH 34/40] HID: wacom: Interpret tilt data from Intuos Pro BT as signed values The tilt data contained in the Bluetooth packets of an Intuos Pro are supposed to be interpreted as signed values. Simply casting the values to type `char` is not guaranteed to work since it is implementation- defined whether it is signed or unsigned. At least one user has noticed the data being reported incorrectly on their system. To ensure that the data is interpreted properly, we specifically cast to `signed char` instead. Link: https://github.com/linuxwacom/input-wacom/issues/445 Fixes: 4922cd26f03c ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface") CC: stable@vger.kernel.org # 4.11+ Signed-off-by: Jason Gerecke Signed-off-by: Jiri Kosina --- drivers/hid/wacom_wac.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 413606bdf476..5a599c90e7a2 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -1353,9 +1353,9 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom) rotation -= 1800; input_report_abs(pen_input, ABS_TILT_X, - (char)frame[7]); + (signed char)frame[7]); input_report_abs(pen_input, ABS_TILT_Y, - (char)frame[8]); + (signed char)frame[8]); input_report_abs(pen_input, ABS_Z, rotation); input_report_abs(pen_input, ABS_WHEEL, get_unaligned_le16(&frame[11])); From ae117e622a9290354615a541a18b008442799ed5 Mon Sep 17 00:00:00 2001 From: Vincent Huang Date: Wed, 16 Oct 2024 10:55:49 +0000 Subject: [PATCH 35/40] HID: rmi: Add select RMI4_F3A in Kconfig Add `select RMI4_F3A` under `HID_RMI` in Kconfig to support buttons and GPIOs on newer Synaptics HID RMI devices. Future devices will use F3A instead of F30, but F30 is still selected for backward compatibility. Signed-off-by: Vincent Huang Signed-off-by: Jiri Kosina --- drivers/hid/Kconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig index f8a56d631242..f2c4f9e89cac 100644 --- a/drivers/hid/Kconfig +++ b/drivers/hid/Kconfig @@ -1096,6 +1096,7 @@ config HID_RMI select RMI4_F11 select RMI4_F12 select RMI4_F30 + select RMI4_F3A help Support for Synaptics RMI4 touchpads. Say Y here if you have a Synaptics RMI4 touchpads over i2c-hid or usbhid From 87a2f10395c82c2b4687bb8611a6c5663a12f9e7 Mon Sep 17 00:00:00 2001 From: Callahan Kovacs Date: Mon, 11 Nov 2024 22:49:28 +0100 Subject: [PATCH 36/40] HID: magicmouse: Apple Magic Trackpad 2 USB-C driver support Adds driver support for the USB-C model of Apple's Magic Trackpad 2. The 2024 USB-C model is compatible with the existing Magic Trackpad 2 driver but has a different hardware ID. Link: https://bugzilla.kernel.org/show_bug.cgi?id=219470 Signed-off-by: Callahan Kovacs Signed-off-by: Jiri Kosina --- drivers/hid/hid-ids.h | 1 + drivers/hid/hid-magicmouse.c | 56 ++++++++++++++++++++++++++---------- 2 files changed, 42 insertions(+), 15 deletions(-) diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h index 92cff3f2658c..0f23be98c56e 100644 --- a/drivers/hid/hid-ids.h +++ b/drivers/hid/hid-ids.h @@ -94,6 +94,7 @@ #define USB_DEVICE_ID_APPLE_MAGICMOUSE2 0x0269 #define USB_DEVICE_ID_APPLE_MAGICTRACKPAD 0x030e #define USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 0x0265 +#define USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC 0x0324 #define USB_DEVICE_ID_APPLE_FOUNTAIN_ANSI 0x020e #define USB_DEVICE_ID_APPLE_FOUNTAIN_ISO 0x020f #define USB_DEVICE_ID_APPLE_GEYSER_ANSI 0x0214 diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c index 8a73b59e0827..ec110dea8772 100644 --- a/drivers/hid/hid-magicmouse.c +++ b/drivers/hid/hid-magicmouse.c @@ -227,7 +227,9 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda touch_minor = tdata[4]; state = tdata[7] & TOUCH_STATE_MASK; down = state != TOUCH_STATE_NONE; - } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { id = tdata[8] & 0xf; x = (tdata[1] << 27 | tdata[0] << 19) >> 19; y = -((tdata[3] << 30 | tdata[2] << 22 | tdata[1] << 14) >> 19); @@ -259,8 +261,9 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda /* If requested, emulate a scroll wheel by detecting small * vertical touch motions. */ - if (emulate_scroll_wheel && (input->id.product != - USB_DEVICE_ID_APPLE_MAGICTRACKPAD2)) { + if (emulate_scroll_wheel && + input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 && + input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { unsigned long now = jiffies; int step_x = msc->touches[id].scroll_x - x; int step_y = msc->touches[id].scroll_y - y; @@ -359,7 +362,9 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda input_report_abs(input, ABS_MT_POSITION_X, x); input_report_abs(input, ABS_MT_POSITION_Y, y); - if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) + if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) input_report_abs(input, ABS_MT_PRESSURE, pressure); if (report_undeciphered) { @@ -367,7 +372,9 @@ static void magicmouse_emit_touch(struct magicmouse_sc *msc, int raw_id, u8 *tda input->id.product == USB_DEVICE_ID_APPLE_MAGICMOUSE2) input_event(input, EV_MSC, MSC_RAW, tdata[7]); else if (input->id.product != - USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 && + input->id.product != + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) input_event(input, EV_MSC, MSC_RAW, tdata[8]); } } @@ -493,7 +500,9 @@ static int magicmouse_raw_event(struct hid_device *hdev, magicmouse_emit_buttons(msc, clicks & 3); input_report_rel(input, REL_X, x); input_report_rel(input, REL_Y, y); - } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { input_mt_sync_frame(input); input_report_key(input, BTN_MOUSE, clicks & 1); } else { /* USB_DEVICE_ID_APPLE_MAGICTRACKPAD */ @@ -545,7 +554,9 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd __set_bit(REL_WHEEL_HI_RES, input->relbit); __set_bit(REL_HWHEEL_HI_RES, input->relbit); } - } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { /* If the trackpad has been connected to a Mac, the name is * automatically personalized, e.g., "José Expósito's Trackpad". * When connected through Bluetooth, the personalized name is @@ -621,7 +632,9 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd MOUSE_RES_X); input_abs_set_res(input, ABS_MT_POSITION_Y, MOUSE_RES_Y); - } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + } else if (input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { input_set_abs_params(input, ABS_MT_PRESSURE, 0, 253, 0, 0); input_set_abs_params(input, ABS_PRESSURE, 0, 253, 0, 0); input_set_abs_params(input, ABS_MT_ORIENTATION, -3, 4, 0, 0); @@ -660,7 +673,8 @@ static int magicmouse_setup_input(struct input_dev *input, struct hid_device *hd input_set_events_per_packet(input, 60); if (report_undeciphered && - input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 && + input->id.product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { __set_bit(EV_MSC, input->evbit); __set_bit(MSC_RAW, input->mscbit); } @@ -685,7 +699,9 @@ static int magicmouse_input_mapping(struct hid_device *hdev, /* Magic Trackpad does not give relative data after switching to MT */ if ((hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD || - hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) && + hi->input->id.product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + hi->input->id.product == + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) && field->flags & HID_MAIN_ITEM_RELATIVE) return -1; @@ -721,7 +737,8 @@ static int magicmouse_enable_multitouch(struct hid_device *hdev) int ret; int feature_size; - if (hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + if (hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { if (hdev->vendor == BT_VENDOR_ID_APPLE) { feature_size = sizeof(feature_mt_trackpad2_bt); feature = feature_mt_trackpad2_bt; @@ -766,7 +783,8 @@ static int magicmouse_fetch_battery(struct hid_device *hdev) if (!hdev->battery || hdev->vendor != USB_VENDOR_ID_APPLE || (hdev->product != USB_DEVICE_ID_APPLE_MAGICMOUSE2 && - hdev->product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2)) + hdev->product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 && + hdev->product != USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC)) return -1; report_enum = &hdev->report_enum[hdev->battery_report_type]; @@ -835,7 +853,9 @@ static int magicmouse_probe(struct hid_device *hdev, if (id->vendor == USB_VENDOR_ID_APPLE && (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || - (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 && hdev->type != HID_TYPE_USBMOUSE))) + ((id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) && + hdev->type != HID_TYPE_USBMOUSE))) return 0; if (!msc->input) { @@ -850,7 +870,8 @@ static int magicmouse_probe(struct hid_device *hdev, else if (id->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2) report = hid_register_report(hdev, HID_INPUT_REPORT, MOUSE2_REPORT_ID, 0); - else if (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) { + else if (id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + id->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) { if (id->vendor == BT_VENDOR_ID_APPLE) report = hid_register_report(hdev, HID_INPUT_REPORT, TRACKPAD2_BT_REPORT_ID, 0); @@ -920,7 +941,8 @@ static const __u8 *magicmouse_report_fixup(struct hid_device *hdev, __u8 *rdesc, */ if (hdev->vendor == USB_VENDOR_ID_APPLE && (hdev->product == USB_DEVICE_ID_APPLE_MAGICMOUSE2 || - hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2) && + hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2 || + hdev->product == USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC) && *rsize == 83 && rdesc[46] == 0x84 && rdesc[58] == 0x85) { hid_info(hdev, "fixing up magicmouse battery report descriptor\n"); @@ -951,6 +973,10 @@ static const struct hid_device_id magic_mice[] = { USB_DEVICE_ID_APPLE_MAGICTRACKPAD2), .driver_data = 0 }, { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, USB_DEVICE_ID_APPLE_MAGICTRACKPAD2), .driver_data = 0 }, + { HID_BLUETOOTH_DEVICE(BT_VENDOR_ID_APPLE, + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC), .driver_data = 0 }, + { HID_USB_DEVICE(USB_VENDOR_ID_APPLE, + USB_DEVICE_ID_APPLE_MAGICTRACKPAD2_USBC), .driver_data = 0 }, { } }; MODULE_DEVICE_TABLE(hid, magic_mice); From 66ef47faa90d838cda131fe1f7776456cc3b59f2 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Mon, 11 Nov 2024 14:12:40 +0100 Subject: [PATCH 37/40] HID: hyperv: streamline driver probe to avoid devres issues It was found that unloading 'hid_hyperv' module results in a devres complaint: ... hv_vmbus: unregistering driver hid_hyperv ------------[ cut here ]------------ WARNING: CPU: 2 PID: 3983 at drivers/base/devres.c:691 devres_release_group+0x1f2/0x2c0 ... Call Trace: ? devres_release_group+0x1f2/0x2c0 ? __warn+0xd1/0x1c0 ? devres_release_group+0x1f2/0x2c0 ? report_bug+0x32a/0x3c0 ? handle_bug+0x53/0xa0 ? exc_invalid_op+0x18/0x50 ? asm_exc_invalid_op+0x1a/0x20 ? devres_release_group+0x1f2/0x2c0 ? devres_release_group+0x90/0x2c0 ? rcu_is_watching+0x15/0xb0 ? __pfx_devres_release_group+0x10/0x10 hid_device_remove+0xf5/0x220 device_release_driver_internal+0x371/0x540 ? klist_put+0xf3/0x170 bus_remove_device+0x1f1/0x3f0 device_del+0x33f/0x8c0 ? __pfx_device_del+0x10/0x10 ? cleanup_srcu_struct+0x337/0x500 hid_destroy_device+0xc8/0x130 mousevsc_remove+0xd2/0x1d0 [hid_hyperv] device_release_driver_internal+0x371/0x540 driver_detach+0xc5/0x180 bus_remove_driver+0x11e/0x2a0 ? __mutex_unlock_slowpath+0x160/0x5e0 vmbus_driver_unregister+0x62/0x2b0 [hv_vmbus] ... And the issue seems to be that the corresponding devres group is not allocated. Normally, devres_open_group() is called from __hid_device_probe() but Hyper-V HID driver overrides 'hid_dev->driver' with 'mousevsc_hid_driver' stub and basically re-implements __hid_device_probe() by calling hid_parse() and hid_hw_start() but not devres_open_group(). hid_device_probe() does not call __hid_device_probe() for it. Later, when the driver is removed, hid_device_remove() calls devres_release_group() as it doesn't check whether hdev->driver was initially overridden or not. The issue seems to be related to the commit 62c68e7cee33 ("HID: ensure timely release of driver-allocated resources") but the commit itself seems to be correct. Fix the issue by dropping the 'hid_dev->driver' override and using hid_register_driver()/hid_unregister_driver() instead. Alternatively, it would have been possible to rely on the default handling but HID_CONNECT_DEFAULT implies HID_CONNECT_HIDRAW and it doesn't seem to work for mousevsc as-is. Fixes: 62c68e7cee33 ("HID: ensure timely release of driver-allocated resources") Suggested-by: Michael Kelley Signed-off-by: Vitaly Kuznetsov Reviewed-by: Michael Kelley Tested-by: Saurabh Sengar Signed-off-by: Jiri Kosina --- drivers/hid/hid-hyperv.c | 58 ++++++++++++++++++++++++++++------------ 1 file changed, 41 insertions(+), 17 deletions(-) diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c index f33485d83d24..0fb210e40a41 100644 --- a/drivers/hid/hid-hyperv.c +++ b/drivers/hid/hid-hyperv.c @@ -422,6 +422,25 @@ static int mousevsc_hid_raw_request(struct hid_device *hid, return 0; } +static int mousevsc_hid_probe(struct hid_device *hid_dev, const struct hid_device_id *id) +{ + int ret; + + ret = hid_parse(hid_dev); + if (ret) { + hid_err(hid_dev, "parse failed\n"); + return ret; + } + + ret = hid_hw_start(hid_dev, HID_CONNECT_HIDINPUT | HID_CONNECT_HIDDEV); + if (ret) { + hid_err(hid_dev, "hw start failed\n"); + return ret; + } + + return 0; +} + static const struct hid_ll_driver mousevsc_ll_driver = { .parse = mousevsc_hid_parse, .open = mousevsc_hid_open, @@ -431,7 +450,16 @@ static const struct hid_ll_driver mousevsc_ll_driver = { .raw_request = mousevsc_hid_raw_request, }; -static struct hid_driver mousevsc_hid_driver; +static const struct hid_device_id mousevsc_devices[] = { + { HID_DEVICE(BUS_VIRTUAL, HID_GROUP_ANY, 0x045E, 0x0621) }, + { } +}; + +static struct hid_driver mousevsc_hid_driver = { + .name = "hid-hyperv", + .id_table = mousevsc_devices, + .probe = mousevsc_hid_probe, +}; static int mousevsc_probe(struct hv_device *device, const struct hv_vmbus_device_id *dev_id) @@ -473,7 +501,6 @@ static int mousevsc_probe(struct hv_device *device, } hid_dev->ll_driver = &mousevsc_ll_driver; - hid_dev->driver = &mousevsc_hid_driver; hid_dev->bus = BUS_VIRTUAL; hid_dev->vendor = input_dev->hid_dev_info.vendor; hid_dev->product = input_dev->hid_dev_info.product; @@ -488,20 +515,6 @@ static int mousevsc_probe(struct hv_device *device, if (ret) goto probe_err2; - - ret = hid_parse(hid_dev); - if (ret) { - hid_err(hid_dev, "parse failed\n"); - goto probe_err2; - } - - ret = hid_hw_start(hid_dev, HID_CONNECT_HIDINPUT | HID_CONNECT_HIDDEV); - - if (ret) { - hid_err(hid_dev, "hw start failed\n"); - goto probe_err2; - } - device_init_wakeup(&device->device, true); input_dev->connected = true; @@ -579,12 +592,23 @@ static struct hv_driver mousevsc_drv = { static int __init mousevsc_init(void) { - return vmbus_driver_register(&mousevsc_drv); + int ret; + + ret = hid_register_driver(&mousevsc_hid_driver); + if (ret) + return ret; + + ret = vmbus_driver_register(&mousevsc_drv); + if (ret) + hid_unregister_driver(&mousevsc_hid_driver); + + return ret; } static void __exit mousevsc_exit(void) { vmbus_driver_unregister(&mousevsc_drv); + hid_unregister_driver(&mousevsc_hid_driver); } MODULE_LICENSE("GPL"); From 20bcb2734bafa2adfbf8fc62542c742df9c46cfd Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Mon, 11 Nov 2024 15:49:59 +0800 Subject: [PATCH 38/40] dt-bindings: input: Goodix GT7986U SPI HID Touchscreen The Goodix GT7986U touch controller report touch data according to the HID protocol through the SPI bus. However, it is incompatible with Microsoft's HID-over-SPI protocol. NOTE: these bindings are distinct from the bindings used with the GT7986U when the chip is running I2C firmware. For some background, see discussion on the mailing lists in the thread: https://lore.kernel.org/r/20241018020815.3098263-2-charles.goodix@gmail.com Signed-off-by: Charles Wang Reviewed-by: Douglas Anderson Reviewed-by: Rob Herring (Arm) Signed-off-by: Jiri Kosina --- .../bindings/input/goodix,gt7986u-spifw.yaml | 69 +++++++++++++++++++ 1 file changed, 69 insertions(+) create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u-spifw.yaml diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u-spifw.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u-spifw.yaml new file mode 100644 index 000000000000..92bd0041feba --- /dev/null +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u-spifw.yaml @@ -0,0 +1,69 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/input/goodix,gt7986u-spifw.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Goodix GT7986U SPI HID Touchscreen + +maintainers: + - Charles Wang + +description: | + Supports the Goodix GT7986U touchscreen. + This touch controller reports data packaged according to the HID protocol + over the SPI bus, but it is incompatible with Microsoft's HID-over-SPI protocol. + + NOTE: these bindings are distinct from the bindings used with the + GT7986U when the chip is running I2C firmware. This is because there's + not a single device that talks over both I2C and SPI but rather + distinct touchscreens that happen to be built with the same ASIC but + that are distinct products running distinct firmware. + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + +properties: + compatible: + enum: + - goodix,gt7986u-spifw + + reg: + maxItems: 1 + + interrupts: + maxItems: 1 + + reset-gpios: + maxItems: 1 + + spi-max-frequency: true + +required: + - compatible + - reg + - interrupts + - reset-gpios + +unevaluatedProperties: false + +examples: + - | + #include + #include + + spi { + #address-cells = <1>; + #size-cells = <0>; + + touchscreen@0 { + compatible = "goodix,gt7986u-spifw"; + reg = <0>; + interrupt-parent = <&gpio>; + interrupts = <25 IRQ_TYPE_LEVEL_LOW>; + reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>; + spi-max-frequency = <10000000>; + }; + }; + +... From c8eb2faef11866be7802ff2ce9852890bcfcde16 Mon Sep 17 00:00:00 2001 From: Charles Wang Date: Mon, 11 Nov 2024 15:50:00 +0800 Subject: [PATCH 39/40] HID: hid-goodix-spi: Add OF supports This patch introduces the following changes: - Adds OF match table. - Hardcodes hid-report-addr in the driver rather than fetching it from the device property. Signed-off-by: Charles Wang Reviewed-by: Douglas Anderson Signed-off-by: Jiri Kosina --- drivers/hid/hid-goodix-spi.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/drivers/hid/hid-goodix-spi.c b/drivers/hid/hid-goodix-spi.c index 6ae2300a603e..80c0288a3a38 100644 --- a/drivers/hid/hid-goodix-spi.c +++ b/drivers/hid/hid-goodix-spi.c @@ -20,6 +20,7 @@ #define GOODIX_HID_REPORT_DESC_ADDR 0x105AA #define GOODIX_HID_SIGN_ADDR 0x10D32 #define GOODIX_HID_CMD_ADDR 0x10364 +#define GOODIX_HID_REPORT_ADDR 0x22C8C #define GOODIX_HID_GET_REPORT_CMD 0x02 #define GOODIX_HID_SET_REPORT_CMD 0x03 @@ -701,12 +702,7 @@ static int goodix_spi_probe(struct spi_device *spi) return dev_err_probe(dev, PTR_ERR(ts->reset_gpio), "failed to request reset gpio\n"); - error = device_property_read_u32(dev, "goodix,hid-report-addr", - &ts->hid_report_addr); - if (error) - return dev_err_probe(dev, error, - "failed get hid report addr\n"); - + ts->hid_report_addr = GOODIX_HID_REPORT_ADDR; error = goodix_dev_confirm(ts); if (error) return error; @@ -790,6 +786,14 @@ static const struct acpi_device_id goodix_spi_acpi_match[] = { MODULE_DEVICE_TABLE(acpi, goodix_spi_acpi_match); #endif +#ifdef CONFIG_OF +static const struct of_device_id goodix_spi_of_match[] = { + { .compatible = "goodix,gt7986u-spifw", }, + { } +}; +MODULE_DEVICE_TABLE(of, goodix_spi_of_match); +#endif + static const struct spi_device_id goodix_spi_ids[] = { { "gt7986u" }, { }, @@ -800,6 +804,7 @@ static struct spi_driver goodix_spi_driver = { .driver = { .name = "goodix-spi-hid", .acpi_match_table = ACPI_PTR(goodix_spi_acpi_match), + .of_match_table = of_match_ptr(goodix_spi_of_match), .pm = pm_sleep_ptr(&goodix_spi_pm_ops), }, .probe = goodix_spi_probe, From e8a0581914bd2e28f7af8d333ddc73fd78b1ef84 Mon Sep 17 00:00:00 2001 From: Dmitry Torokhov Date: Mon, 28 Oct 2024 12:07:50 -0700 Subject: [PATCH 40/40] HID: multitouch: make mt_set_mode() less cryptic mt_set_mode() accepts 2 boolean switches indicating whether the device (if it follows Windows Precision Touchpad specification) should report hardware buttons and/or surface contacts. For a casual reader it is completely not clear, as they look at the call site, which exact mode is being requested. Define report_mode enum and change mt_set_mode() to accept is as an argument instead. This allows to write: mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL); or mt_set_modes(hdev, HID_LATENCY_HIGH, TOUCHPAD_REPORT_BUTTONS); which makes intent much more clear. Reviewed-by: Benjamin Tissoires Signed-off-by: Dmitry Torokhov Link: https://patch.msgid.link/Zx_hBvg5Qa3KU3ta@google.com Signed-off-by: Benjamin Tissoires --- drivers/hid/hid-multitouch.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c index e936019d21fe..785743036647 100644 --- a/drivers/hid/hid-multitouch.c +++ b/drivers/hid/hid-multitouch.c @@ -31,6 +31,7 @@ * [1] https://gitlab.freedesktop.org/libevdev/hid-tools */ +#include #include #include #include @@ -83,6 +84,13 @@ enum latency_mode { HID_LATENCY_HIGH = 1, }; +enum report_mode { + TOUCHPAD_REPORT_NONE = 0, + TOUCHPAD_REPORT_BUTTONS = BIT(0), + TOUCHPAD_REPORT_CONTACTS = BIT(1), + TOUCHPAD_REPORT_ALL = TOUCHPAD_REPORT_BUTTONS | TOUCHPAD_REPORT_CONTACTS, +}; + #define MT_IO_FLAGS_RUNNING 0 #define MT_IO_FLAGS_ACTIVE_SLOTS 1 #define MT_IO_FLAGS_PENDING_SLOTS 2 @@ -1493,8 +1501,7 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev, struct hid_field *field, struct hid_usage *usage, enum latency_mode latency, - bool surface_switch, - bool button_switch, + enum report_mode report_mode, bool *inputmode_found) { struct mt_device *td = hid_get_drvdata(hdev); @@ -1549,11 +1556,11 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev, return true; case HID_DG_SURFACESWITCH: - field->value[index] = surface_switch; + field->value[index] = !!(report_mode & TOUCHPAD_REPORT_CONTACTS); return true; case HID_DG_BUTTONSWITCH: - field->value[index] = button_switch; + field->value[index] = !!(report_mode & TOUCHPAD_REPORT_BUTTONS); return true; } @@ -1561,7 +1568,7 @@ static bool mt_need_to_apply_feature(struct hid_device *hdev, } static void mt_set_modes(struct hid_device *hdev, enum latency_mode latency, - bool surface_switch, bool button_switch) + enum report_mode report_mode) { struct hid_report_enum *rep_enum; struct hid_report *rep; @@ -1586,8 +1593,7 @@ static void mt_set_modes(struct hid_device *hdev, enum latency_mode latency, rep->field[i], usage, latency, - surface_switch, - button_switch, + report_mode, &inputmode_found)) update_report = true; } @@ -1830,7 +1836,7 @@ static int mt_probe(struct hid_device *hdev, const struct hid_device_id *id) dev_warn(&hdev->dev, "Cannot allocate sysfs group for %s\n", hdev->name); - mt_set_modes(hdev, HID_LATENCY_NORMAL, true, true); + mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL); return 0; } @@ -1842,9 +1848,9 @@ static int mt_suspend(struct hid_device *hdev, pm_message_t state) /* High latency is desirable for power savings during S3/S0ix */ if ((td->mtclass.quirks & MT_QUIRK_DISABLE_WAKEUP) || !hid_hw_may_wakeup(hdev)) - mt_set_modes(hdev, HID_LATENCY_HIGH, false, false); + mt_set_modes(hdev, HID_LATENCY_HIGH, TOUCHPAD_REPORT_NONE); else - mt_set_modes(hdev, HID_LATENCY_HIGH, true, true); + mt_set_modes(hdev, HID_LATENCY_HIGH, TOUCHPAD_REPORT_ALL); return 0; } @@ -1852,7 +1858,7 @@ static int mt_suspend(struct hid_device *hdev, pm_message_t state) static int mt_reset_resume(struct hid_device *hdev) { mt_release_contacts(hdev); - mt_set_modes(hdev, HID_LATENCY_NORMAL, true, true); + mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL); return 0; } @@ -1864,7 +1870,7 @@ static int mt_resume(struct hid_device *hdev) hid_hw_idle(hdev, 0, 0, HID_REQ_SET_IDLE); - mt_set_modes(hdev, HID_LATENCY_NORMAL, true, true); + mt_set_modes(hdev, HID_LATENCY_NORMAL, TOUCHPAD_REPORT_ALL); return 0; }