Hi Tero, On Fri, Nov 26, 2021 at 2:02 PM Tero Kristo <tero.kristo@xxxxxxxxxxxxxxx> wrote: > > Hi, > > This series is an update based on comments from Benjamin. What is done > is this series is to ditch the separate hid-driver for USI, and add the > generic support to core layers. This part basically brings the support > for providing USI events, without programmability (patches 1-6). That part seems to be almost good for now. I have a few things to check: - patch2: "HID: hid-input: Add suffix also for HID_DG_PEN" I need to ensure there are no touchscreens affected by this (there used to be a mess with some vendors where they would not declare things properly) - patch5: "HID: core: map USI pen style reports directly" this one feels plain wrong. I would need to have a look at the report descriptor but this is too specific in a very generic code > > Additionally, a HID-BPF based sample is provided which can be used to > program / query pen parameters in comparison to the old driver level > implementation (patches 7-8, patch #8 is an incremental change on top of > patch #7 which just converts the fifo to socket so that the client can > also get results back from the server.) After a few more thoughts, I wondered what your input is on this. We should be able to do the very same with plain hidraw... However, you added a `hid/raw_event` processing that will still be kept in the kernel, so maybe bpf would be useful for that at least. > > The whole series is based on top of Benjamin's hid-bpf support work, and > I've pushed a branch at [1] with a series that works and brings in > the dependency. There are also a few separate patches in this series to > fix the problems I found from Benjamin's initial work for hid-bpf; I > wasn't able to get things working without those. The branch is also > based on top of 5.16-rc2 which required some extra changes to the > patches from Benjamin. Yeah, I also rebased on top of 5.16 shortly after sharing that branch and got roughly the same last fix (HID: bpf: compile fix for bpf_hid_foreach_rdesc_item). I am *very* interested in your "HID: bpf: execute BPF programs in proper context" because that is something that was bothering me a lot :) "HID: bpf: add expected_attach_type to bpf prog during detach" is something I'll need to bring in too but "HID: bpf: fix file mapping" is actually wrong. I initially wanted to attach BPF programs to hidraw, but shortly realized that this is not working because the `hid/rdesc_fixup` kills the hidraw node and so releases the BPF programs. The way I am now attaching it is to use the fd associated with the modalias in the sysfs file (for instance: `sudo ./hid_surface_dial /sys/bus/hid/devices/0005:045E:091B.*/modalias`). This way, the reference to the struct hid_device is kept even if we disconnect the device and reprobe it. Thanks again for your work, and I'd be curious to have your thoughts on hid-bpf and if you think it is better than hidraw/evdev write/new ioctls for your use case. Cheers, Benjamin > > -Tero > > [1] https://github.com/t-kristo/linux/tree/usi-5.16-rfc-v2-bpf > >
