On Oct 08 2024, Werner Sembach wrote: > > Am 08.10.24 um 11:53 schrieb Benjamin Tissoires: > > On Oct 07 2024, Werner Sembach wrote: > > > Hi, > > > > > > Am 02.10.24 um 10:31 schrieb Benjamin Tissoires: > > > > On Oct 01 2024, Werner Sembach wrote: > > > > > Hi Benjamin, > > > > > > > > > > Am 01.10.24 um 15:41 schrieb Benjamin Tissoires: > > > > > > [...] > > > > > > PPS: sorry for pushing that hard on HID-BPF, but I can see that it fits > > > > > > all of the requirements here: > > > > > > - need to be dynamic > > > > > > - still unsure of the userspace implementation, meaning that userspace > > > > > > might do something wrong, which might require kernel changes > > > > > Well the reference implementetion for the arduiono macropad from microsoft > > > > > ignores the intensity (brightness) channel on rgb leds contrary to the HID > > > > > spec, soo yeah you have a point here ... > > > > Heh :) > > > > > > > > > > - possibility to extend later the kernel API > > > > > > - lots of fun :) > > > > > You advertise it good ;). More work for me now but maybe less work for me > > > > > later, I will look into it. > > > > Again, I'm pushing this because I see the benefits and because I can > > > > probably reuse the same code on my Corsair and Logitech keyboards. But > > > > also, keep in mind that it's not mandatory because you can actually > > > > attach the BPF code on top of your existing driver to change the way it > > > > behaves. It'll be slightly more complex if you don't let a couple of > > > > vendor passthrough reports that we can use to directly talk to the > > > > device without any tampering, but that's doable. But if you want to keep > > > > the current implementation and have a different layout, this can easily > > > > be done in BPF on top. > > > > > > > > Cheers, > > > > Benjamin > > > > > > > > > > > > [0] https://lore.kernel.org/linux-input/20241001-hid-bpf-hid-generic-v3-0-2ef1019468df@xxxxxxxxxx/T/#t > > > Thinking about the minimal WMI to HID today, but found a problem: a HID > > > feature report is either strictly input or output afaik, but the WMI > > > interface has both in some functions. > > Not sure you are talking about feature reports, because they are > > read/write. It's just that they are synchronous over the USB control > > endpoint (on USB). > > I'm confused about the split between get and send feature reports > https://www.kernel.org/doc/html/latest/hid/hidraw.html > > I guess then a get feature report can also carry input data and the > difference is that a send feature report doesn't wait for a reply? but then > what is it's reason of existence in contrast to an output report? I'm under the impression you are mixing the 3 types of reports (just re-stating that here in case I wasn't clear). - Input reports: `Input()` in the report descriptor -> data emitted by the device to the host, and notified through an IRQ mechanism -> obtained in hidraw through a blocking read() operation - Output reports: `Output()` in the report descriptor -> data sent asynchronously by the host to the device. -> sent from hidraw by calling write() on the dev node (no feedback except how many bytes were sent) - Feature reports: `Feature()` in the report descriptor -> way to synchronously configure the device. Think of it like a register on the device: you can read it, write it, but you never get an interrupt when there is a change -> read/written by using an ioctl on the hidraw node And BTW, it's perfectly fine to have a dedicated report ID which has Input, Output and Feature attached to it :) > > > > > An input report is strictly directed from the device, and an output > > report is from the host to the device. > > > > But a feature report is bidirectional. > > > > > How would I map that? > > Depending on the WMI interface, if you want this to be synchronous, > > defining a Feature report is correct, otherwise (if you don't need > > feedback from WMI), you can declare the commands to WMI as Output > > reports. > Thanks for reminding me that output reports exist xD. hehe > > > > > If I split everything in input and output the new interface wouldn't > > > actually be much smaller. > > The HID report descriptor doesn't need to be smaller. The fact that by > > default it exposes only one or two LEDs so we don't have the micrometers > > arrays is the only purpose. > > > > But if we also implement a not-full HID implementation of LampArray, we > > should be able to strip out the parts that we don't care in the LED > > class implementation, like the exact positioning, or the multiupdate. > > > > > Also what would I write for the usage for the reserved padding in the report > > > descriptor. Usage: 0x00? > > padding are ignored by HID. So whatever current usage you have is fine. > > > > However, if you are talking about the custom WMI vendor access, I'd go > > with a vendor collection (usage page 0xff00, usage 0x08 for the 8 bytes > > long WMI command for instance, 0x10 for the 16 bytes long one). > > > > Side note: in drivers/hid/bpf/progs/hid_report_helpers.h we have some > > autogenerated macros to help writing report descriptors (see > > drivers/hid/bpf/progs/Huion__Dial-2.bpf.c for an example of usage). It's > > in the hid-bpf tree but I think we might be able to include this in > > other drivers (or do a minimal rewrite/move into include). > > I'm not asking you to use it on your code right now, but this has the > > advantage of becoming less "binary blob" in your code, and prevent > > mistakes where you edit the comments but not the values. > > I will look into it. > > Since the interface is fixed I don't need to flesh out the whole descriptor > (which i thought i must do) and usage page (0xff42, because NB04 and the wmi > has 2 other ec controlling wmi interfaces besides the AB one), report usage > (matching the wmi comand id's) and report size should be enough. I'm a little confused by that last sentence. But yeah, I would expect some minimal sanity check before handing over the HID report to the WMI interface :) Cheers, Benjamin
