From: "Filipe Laíns" <lains@archlinux.org>
To: Dean Camera <dean@fourwalledcubicle.com>,
linux-input@vger.kernel.org, jikos@kernel.org,
benjamin.tissoires@redhat.com
Subject: Re: [V3, PATCH] Add additional hidraw input/output report ioctls.
Date: Thu, 26 Nov 2020 21:42:37 +0000 [thread overview]
Message-ID: <c8d4d17798e2c9fca417223d4edf5f4b1aebf75d.camel@archlinux.org> (raw)
In-Reply-To: <2c2bfd55-3a03-9839-22f5-16058ac24e35@fourwalledcubicle.com>
[-- Attachment #1: Type: text/plain, Size: 3312 bytes --]
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@archlinux.org>
Perhaps just split the hid-example.c change to another patch.
*snip*
Cheers,
Filipe Laíns
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-11-26 21:42 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-25 22:39 [V3, PATCH] Add additional hidraw input/output report ioctls Dean Camera
2020-11-26 19:05 ` Filipe Laíns
2020-11-26 21:30 ` Dean Camera
2020-11-26 21:42 ` Filipe Laíns [this message]
2020-11-27 4:05 ` Dean Camera
2020-11-27 20:11 ` Filipe Laíns
2020-11-28 7:02 ` Dean Camera
2020-11-29 20:01 ` Filipe Laíns
2020-11-27 14:49 ` Jiri Kosina
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c8d4d17798e2c9fca417223d4edf5f4b1aebf75d.camel@archlinux.org \
--to=lains@archlinux.org \
--cc=benjamin.tissoires@redhat.com \
--cc=dean@fourwalledcubicle.com \
--cc=jikos@kernel.org \
--cc=linux-input@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).