On Tue, 08 Jun 2010 09:42:55 -0400 Alan Ott <alan@xxxxxxxxxxx> wrote: > On 06/08/2010 02:32 AM, Antonio Ospite wrote: > > On Mon, 7 Jun 2010 23:51:48 -0400 > > Alan Ott<alan@xxxxxxxxxxx> wrote: > > > > > >> Per the HID Specification, Feature reports must be sent and received on > >> the Configuration endpoint (EP 0) through the Set_Report/Get_Report > >> interfaces. This patch adds two ioctls to hidraw to set and get feature > >> reports to and from the device. Modifications were made to hidraw and > >> usbhid. > >> > >> New hidraw ioctls: > >> HIDIOCSFEATURE - Perform a Set_Report transfer of a Feature report. > >> HIDIOCGFEATURE - Perform a Get_Report transfer of a Feature report. > >> > >> Signed-off-by: Alan Ott<alan@xxxxxxxxxxx> > >> --- > >> > > Thanks Alan, I am going to test this quite soon. > > > > TBH, when I was thinking about how to extend hidraw I thought we could > > have added a new report_type field to struct hidraw_report_descriptor, > > in order to re-use the HIDIOCGRDESC ioctl handler itself, adding then a > > HIDIOCSRDESC for setting the report. This looked cleaner to my eyes, > > > Thanks for the feedback, Antonio. The HIDIOCGRDESC ioctl copies the > existing descriptor from the hid_device structure. Since it does not > initiate a Get_Report transfer, I'm not sure how much re-use there could > have been using that method. In my estimation, a Set_Report/Get_Report > was more similar to the call to write(). > I was only thinking about the interface to userspace, HIDIOCGRDESC/HIDIOCSRDESC sounded more general to me (and look like HIDIOCGREPORT/HIDIOCSREPORT from hiddev) if they could be made to work with different report types, but as I said I didn't look at the current code very well, so my remark are surely quite naive. > > but I didn't actually implement this, so I don't know if it was > > feasible, for instance one problem I didn't investigate further was > > about the default value of the aforementioned report_type field in > > order to keep the current behavior of HIDIOCGRDESC. > > > I'm not sure what you mean here, as the report_type field is not part of > hidraw_report_descriptor. > I was thinking about _adding_ that field, but again, pretty arbitrarily thought. > Thanks for testing my patch. Please let me know if you have problems > with it. > It works basically ok for my needs, thanks again, waiting for comments from usb/HID people. Note that there are some checkpatch.pl errors in the current patch, and also a style fix mixed with functional ones (@@ -315,7 +411,7 @@), you may want to sort these out in a v2. After this gets in, some more style fixes to hidraw.c could be made, I'll do these. Maybe some naming cleanup can be made too, hid_output_raw_report could become hid_set_raw_report for instance, but I am waiting for the topic to settle first. Thanks, Antonio -- Antonio Ospite http://ao2.it PGP public key ID: 0x4553B001 A: Because it messes up the order in which people normally read text. See http://en.wikipedia.org/wiki/Posting_style Q: Why is top-posting such a bad thing? A: Top-posting. Q: What is the most annoying thing in e-mail?
Attachment:
pgppLWnI8pLKi.pgp
Description: PGP signature
