Hi, On 03-04-19 17:12, Benjamin Tissoires wrote:
Hi, On Wed, Apr 3, 2019 at 9:06 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
<snip>
I was wondering if this is something which more keyboards suffer from, a quick grep finds a couple which do exactly the same thing (but with a different new maximum value: hid-apple.c: static __u8 *apple_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { struct apple_sc *asc = hid_get_drvdata(hdev); if ((asc->quirks & APPLE_RDESC_JIS) && *rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) { hid_info(hdev, "fixing up MacBook JIS keyboard report descriptor\n"); rdesc[53] = rdesc[59] = 0xe7; } return rdesc; } hid-nti.c: static __u8 *nti_usbsun_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { if (*rsize >= 60 && rdesc[53] == 0x65 && rdesc[59] == 0x65) { hid_info(hdev, "fixing up NTI USB-SUN keyboard adapter report de rdesc[53] = rdesc[59] = 0xe7; } return rdesc; } And a few wich are close, but slightly different: hid-asus.c: static __u8 *asus_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { struct asus_drvdata *drvdata = hid_get_drvdata(hdev); if (drvdata->quirks & QUIRK_FIX_NOTEBOOK_REPORT && *rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x65) hid_info(hdev, "Fixing up Asus notebook report descriptor\n"); rdesc[55] = 0xdd; } ... (other quirks for other devices) } hid-aureal.c: static __u8 *aureal_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) { dev_info(&hdev->dev, "fixing Aureal Cy se W-01RN USB_V3.1 report rdesc[53] = 0x65; } return rdesc; } hid-ortek.c: static __u8 *ortek_report_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { if (*rsize >= 56 && rdesc[54] == 0x25 && rdesc[55] == 0x01) { hid_info(hdev, "Fixing up logical maximum in report descriptor ( rdesc[55] = 0x92; } else if (*rsize >= 54 && rdesc[52] == 0x25 && rdesc[53] == 0x01) { hid_info(hdev, "Fixing up logical maximum in report descriptor ( rdesc[53] = 0x65; } return rdesc; } So I'm wondering if we cannot come up with some generic helper for this, which drivers could then call from their report-fixup; or, even better maybe have a HID_QUIRK_FIX_KEYBOARD_MAXIMUM which causes the generic code to bump the maximum to 0xe7 (*), then the hid-nti, hid-macally and hid-ortek drivers could be dropped and replaced with a single line entry in hid-quirks.cI am not a big fan of the idea. Having a specific quirk is not going to help much IMO because it will be quite hard to grasp what it does. Right now adding a new module doesn't cost much, especially since hid-generic is nice enough to unbind itself. With Jiri, we talked (a lot) about having those report descriptors overrides provided by userspace. I know I already mentioned this to you a while ago (and more recently IIRC), but one idea could be to have the override as a firmware file that would get loaded by udev through request_firmware() (if I remember correctly the API). But I am also wondering if we could not use eBPF to actually fix those HID devices programmatically, which mean we could externalize those fixes without actually needing the full report descriptor. Anyway, until we have a generic and good enough solution, I'd rather we keep accepting those report fixups in the kernel.
Ok, fair enough. Regards, Hans
