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 22.10.24 um 11:05 schrieb Benjamin Tissoires:
Sorry I should have answered earlier...

On Oct 09 2024, Werner Sembach wrote:
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).
not quite. You can not use a get feature to set something on the device.

The semantic is:
Set -> "write" something on the device (from host to device)
Get -> "read" something from the device (from device to host)

Features can be set/get.
Input can only be get.
Output can only be set.

The implementation in the kernel should enforce that.

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

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.

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?)
yeah, but from hidraw the kernel calls the device directly to query the
report, so some device don't like that and just hang.

Rule of thumbs: never use get_report on an input report, unless the
specification explicitely says that the device is supposed to support
it for the given usage.

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

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

Feature report get: Sync read/write (intended only for read, but not limited
to it, uses singular buffer for both input and output)
sync read only, no write. The existing values in the incoming buffer are
just overwritten.
Sorry I'm still confused: You said i could do input and output in a singular feature report, but now you say i can't do input or i can't do output, so i still need to use 2?

I kind of don't get why feature report set exists, but well it's the specs ^^.
if "feature report set" doesn't exist, you can not write a vlaue to a
feature on a device (because get doesn't allow you to write).

Anyway, it's a USB implementation detail: input/output are using URB, so
direct USB read/write, when Features are using the control endpoint,
which allows for a slightly different approach.

And this transfered as output being async, when features are
synchronous.

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