On Oct 08 2024, Werner Sembach wrote:

Am 08.10.24 um 11:53 schrieb Benjamin Tissoires:

quoted

On Oct 07 2024, Werner Sembach wrote:

quoted

Hi,

Am 02.10.24 um 10:31 schrieb Benjamin Tissoires:

quoted

On Oct 01 2024, Werner Sembach wrote:

quoted

Hi Benjamin,

Am 01.10.24 um 15:41 schrieb Benjamin Tissoires:

quoted

[...]
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 :)

quoted

quoted

- 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@kernel.org/T/#t (local)

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 :)

quoted

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.

quoted

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

quoted

quoted

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.

quoted

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