On Fri, 2020-11-27 at 08:30 +1100, Dean Camera wrote: > Hi Filipe, > > Comments inline. > > - Dean > > On 27/11/2020 6:05 am, Filipe Laíns wrote: > > Hi, > > > > What is the difference between V1, V2 and V3? I think generally you would > > add a > > small summary. > > > > Sorry, that's my fault -- the contents are identical. I am more used to > modern tooling with code review platforms, pull-requests or even emailed > attached patches, so the old tooling took me a few goes to get right. > > V1 was mangled by Thunderbird, while V2 was missing the cover letter > when I submitted it via git send-email from my test machine. V3 is where > I (think) I beat the tooling into submission. No worries, I was just wondering what the differences were, given that the patches were identical. *snip* > > I am not sure using the same approach as HIDIOCGFEATURE is a good design > > choice. > > The first byte of the supplied buffer is the report ID, but you can set is > > to 0 > > if you don't want to use numbered reports. From my understanding, this makes > > it > > impossible to use the ioctl with report ID 0, which valid per the HID spec. > > > > Report ID 0 is reserved by the HID specification and may not be used in > a device with multiple reports (see "Device Class Definition for HID > 1.11", section "6.2.2.7 Global Items" where it states "Report ID zero is > reserved and should not be used."). Yup, you are right! I missed that. > I think the designers of HID forsaw a sane future where in userspace > everyone just assumed the report ID was present at all times, and the > HID driver would just omit it on the wire if it was zero. Unfortunatly > every platform seems to handle that differently now, with some always > requring it, and others selectively omitting it in their APIs. Yes, it's unfortunate. Perhaps if the HID designers made the spec easier to read we wouldn't have this issue. > > My suggestion would be to automatically use numbered reports or not > > depending if > > the device uses them. A HID endpoint either uses numbered reports or not, it > > doesn't make much sense to me to let users try to use numbered reports on > > devices that do not use them or the other way round. > > > > But I guess this is a question for Benjamin. > > I'm *strongly* in favour of always having them at least in the > `ioctl()`, with a (reserved) zero value indicating it is unused - like > it is now. That makes userspace easier to deal with, and covers the > quirk case where a device does not list report IDs in its HID report > descriptor properly, but requires them anyway. > > It also makes the new requests consistent with the existing request, so > there's no extra cognitive load from working with one then switching to > the others. Yeah, it makes sense given that report 0 is reserved :) > > > > I tried to track down the discussion about the addition of the > > HIDIOCGFEATURE > > ioctl but from what I saw there was no mention of this design flaw. > > > > Am I missing something here? I was :D You can have my Signed-off-by: Filipe Laíns <lains@xxxxxxxxxxxxx> Perhaps just split the hid-example.c change to another patch. *snip* Cheers, Filipe Laíns
Attachment:
signature.asc
Description: This is a digitally signed message part
