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 09.10.24 um 11:55 schrieb Werner Sembach:

Resend because HTML mail ..., but I think I now know when Thunderbird
does it: Every time I include a link it gets converted.

Hi

Am 08.10.24 um 17:21 schrieb Benjamin Tissoires:
On Oct 08 2024, Werner Sembach wrote:
[...]
Yeah, it just means that you can query or send the data. You can also
use HIDIOCGINPUT() and HIDIOCSOUTPUT() to get a current input report and
set an output report through the hidraw ioctl...

Internally, HIDIOCGINPUT() uses the same code path than
HIDIOCGFEATURE(), but with the report type being an Input instead of a
Feature. Same for HIDIOCSOUTPUT() and HIDIOCSFEATURE().

Ok so just a difference in definition not in implementation.

Then I use a get feature report for the device status function and use
it as input and output at the same time, and use a set output report
for the led update function (which technically has a return value but
i think it's always 0 anyway).

I scoured the old thread about exposing WMI calls to userspace,
because I remembered that something here came up already.

1.
https://lore.kernel.org/all/6b32fb73-0544-4a68-95ba-e82406a4b188@xxxxxx/
-> Should be no problem? Because this is not generally exposing wmi
calls, just mapping two explicitly with sanitized input (whitelisting
basically).

It would be OK to expose a selected set of WMI calls to userspace and sanitizing the input of protect potentially buggy firmware from userspace.


2.
https://lore.kernel.org/all/b6d79727-ae94-44b1-aa88-069416435c14@xxxxxxxxxx/
-> Do this concerns this apply here? The actual API to be used is
LampArray and the HID mapped WMI calls are just an "internal"
interface for the BPF driver, but technically UAPI.

I see no benefit of using BPF for creating the whole HID reports. Otherwise the HID interface exported by the driver to userspace would be a HID-mapped IOCTL interface
with no real benefit.

I think it would make more sense for the driver to export a generic HID LampArray interface, which contains placeholder values for the dimensions. Those values can then
be supplied by a HID-BPF snipped for each individual machine model. This would indeed be a suitable use of HID-BPF, as this would allow us to omit having a large quirk
table inside the kernel driver.

Regarding the basic idea of having a virtual HID interface: i would prefer to create a illumination subsystem instead, but i have to agree that we should be doing this
only after enough drivers are inside the kernel, so we can design a suitable interface for them. For now, creating a virtual HID interface seems to be good enough.

Thanks,
Armin Wolf

Also at Armin and Hans: Do you have comments on this approach?

(well as far as I can tell the hut doesn't actual specify, if they
need to
be feature reports, or am I missing something?)
They can be both actually. The HUT is missing what's expected here :(.

However, looking at the HUT RR 84:
https://www.usb.org/sites/default/files/hutrr84_-_lighting_and_illumination_page.pdf


There is an example of a report descriptor, and they are using Features.
Not Input+Output.

And looking even further (above), in 3.5 Usage Definitions:
3.5.2, 3.5.3 and 3.5.5 all of them are meant to be a feature, like:
LampArrayAttributesReport CL – Feature -
LampAttributesRequestReport CL – Feature –
LampAttributesResponseReport CL – Feature –
LampArrayControlReport CL – Feature –

3.5.4: can be either feature or output, like:
LampMultiUpdateReport CL – Feature/Output –
LampRangeUpdateReport CL – Feature/ Output –

So I guess the MS implementation can handle Feature only for all but the
update commands.
Thanks for the link, I guess for the BPF driver I will stick to
feature reports for the LampArray part until there is actually a hid
descriptor spotted in the wild defining LampMultiUpdateReport and
LampRangeUpdateReport as Output and not feature.
and there is the pair with LampAttributesRequestReport and
LampAttributesResponseReport.
Yeah, not a big deal. The bold IN and OUT are just to say that calling a
setReport on a LampAttributesResponseReport is just ignored AFAIU.

Sorry for my confusion over the hid spec.
No worries. It is definitely confusing :)

On this note as I fathom:

Input Report (usually always get report): Interrupts (the ioctl just
there to repeat the last one?)

Output Report (usually always set report): Async write, no return
value (Buffer should stay untouched)

Feature report set: Sync write, no return value (Buffer should stay
untouched)

Feature report get: Sync read/write (intended only for read, but not
limited to it, uses singular buffer for both input and output)

I kind of don't get why feature report set exists, but well it's the
specs ^^.

Regards,

Werner

[*snip*]






[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux