Re: [PATCH 1/1] platform/x86/tuxedo: Add virtual LampArray for TUXEDO NB04 devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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?


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.

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.

Regards,

Werner


Cheers,
Benjamin





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux