Hi Marco, On Mon, Jun 12, 2023 at 12:29 PM Marco Morandini <marco.morandini@xxxxxxxxx> wrote: > > First of all, please bear with me for writing this. > > Should this appear email to be a criticism toward any of you, > be assured that this is not the intention. I'm > really grateful to all you guys, who keep improving the kernel > and allows us to use a free operating system. > > My questions here below are basically along the lines of > "would it make sense to write this and that?". This is not > because I'd like someone else to do the work for me, but > because I really don't know whether it makes sense or not. > Should be wrong to make such type of questions without > accompanying proofs of concept, then please ignore me, and > I'll go back into my cave. Asking questions is always fine. And given that you already did a lot of homework, not answering would be rude ;) > > Some background: I've recently bought a laser digital pointer > from HP. It connects through Bluetooth. > Since it did not work, I made a point to have it working, > even if I'm not a kernel developer, and do not plan to become a > kernel developer. > At the end, this turned out to be a two-line patch, adding > the HID_QUIRK_MULTI_INPUT for such device. Something that > would likely require less than 5 minutes > of a not-so proficient kernel developer. Yeah, right. This is the kind of situation where it's usually easy enough to detect with hid-tools[0]. We can record the device on your machine, then we can replay it locally on ours, and make several attempts. > > However, the process for me was much more cumbersome: > I had to navigate a lot o wrong or misleading documentation > in different forums, try to make sense of the kernel documentation, > understand what a HID descriptor is, > understand how to parse it, try to make sense of some > unknown kernel code (mostly unsuccessfully), > try with ebpf, try this and that... you get the idea. Heh, you tried hid-bpf :) thanks! > > Now, I'm writing because I _think_ I've learned something > in the process, and perhaps it could be useful to share it. > > Thus the questions: > > 1) do you think it would make sense to have some basic documentation > describing what a hid descriptor is, where to download the documents > defining it (https://www.usb.org/ is linked from the docs, > but this is not enough to get started, at least for someone like me), > how to actually read it from the hardware, how to parse it... ? Yes, very much yes. At least having pointers to various projects that can read HID descriptors and parse them would be already better. > Very basic things, that, if I'm not wrong, are not currently > covered by the kernel documentation, and that could allow > someone else like me to get started more quickly? > If yes, I can try to write a skeleton for that, but I'm not sure > there will not be errors and/or omissions, thus it would likely need > to be fixed by someone more knowledgeable than me. Sure. Please write (if you want) your first draft, we can review it and we can iterate from there. Do not forget to add the linux doc mailing list in CC in case some people from there want to also add things. > > 2) if I got it right, one can add a quirk like HID_QUIRK_MULTI_INPUT > while loading the usb_hid module, but not while loading the usb_generic > one (that turned out to be the module that manages my HP pointer), > even if the statically defined quirks were moved into their own file. > Would it make sense to add the possibility to > add quirks while loading hid_generic? Is this the right place for > such code? If yes, I can try in my spare time to do this, > even if I'm not sure I'll be able to get it right. I'm not 100% sure of what you mean, but currently dynamic quirks can be added to the *usbhid* module (not usb_hid or usb_generic), which is the transport layer driver for HID. This module is responsible for creating a HID device, which can be handled by HID drivers, hid_generic being one of them. As the name says, hid_generic is supposed to be generic, and I do not want to see special cases handled there, because it would not be generic anymore. However, other drivers, (hid_multitouch for instance) can have a .driver_data dynamically set too, which allows for quirking a device. But the quirk is local to the driver. Given that HID_QUIRK_MULTI_INPUT is a global HID quirk, it makes sense IMO to add it at the usbhid level because you are just using the hid-generic implementation. Furthermore, if you submit your patch to the LKML, the quirk will likely end up in driver/hid/hid-quirks.c which is exactly the static equivalent of the dynamic one from usbhid. So I don't think having such a new quirk mechanism makes sense. Furthermore, that quirk mechanism allows for quick and dirty testing of the impact on the kernel, but a proper submission as a kernel patch ensures that everybody gets the same fix, so I'd rather not have a forum that explains in details what to do for a given HID product when we can just quirk it in the tree and forget. > > 3) always if I get it right, currently it is not possible to inject quirks > using ebpf, but only to modify the HID descriptor. > Is this correct? You can also change the events and filter them out if you want, but yes that's pretty much the extent of HID_BPF nowadays. > If yes, do you think it would be feasible and reasonable > to add such a possibility? If yes, I can try in my spare time to do this, > even if I'm not sure I'll be able to get it right. Long story short: I would rather not have such bpf call. The fundamental of HID-BPF is that you take a HID thingy, and you output another HID thingy. Messing up with the kernel drivers is out of the scope. But furthermore, what would be the benefit compared to the already available dynamic quirks the kernel allows to have? You can simply add a configuration file to your system to locally have the dynamic quirk set, and then it gets re-applied on every reboot. As much as I'd like bpf to be the universal answer, this doesn't seem to be a good replacement to what we currently have. HID-BPF seems to be a good answer to replace (some) drivers, but I don't think it should replace the generic kernel processing. So that's why I don't think this is a good idea. > > Apologize again for this long email: > I understand it's full of good intentions but without any > significant contribution :( No need to apologize. You are actually proposing ideas and your help to make things better for end-users, which is extremely valuable in itself :) Cheers, Benjamin [0] https://gitlab.freedesktop.org/libevdev/hid-tools
