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 <[email protected]>
---
Instead of creating a new function to handle Set_Report(Feature) requests,
and to promote a bit of re-use, I re-named hidraw_write() to
hidraw_send_report() and made hidraw_write() call hidraw_send_report().
hidraw_send_report() takes one additional parameter, allowing it to handle
OUTPUT and FEATURE reports. Since hidraw_send_report() is called from both
hidraw_write() and from hidraw_ioctl(), the locking of minors_mutex had to
be moved outside of hidraw_send_report() into hidraw_write(), because
hidraw_send_report() locks minors_mutex.
To implement the Get_Report(Feature) request (ioctl HIDIOCGFEATURE), I made
a new function in usbhid/hid-core.c (usbhid_get_raw_report()) to
perform the transfer. To make it available to hidraw, I added an additional
function pointer into include/linux/hid.h, similar to the existing
hid_output_raw_report() pointer.
The steps I used to create this patch are as follows:
1. In hidraw.c Rename the current hidraw_write() function to
hidraw_send_report(), and add report_type parameter to it.
2. Change the call to hid->hid_output_raw_report() to use the new passed-in
report_type parameter.
3. Create a new hidraw_write() which calls hidraw_send_report() with the
minors_mutex held.
4. Remove the minors_mutex locking from hidraw_send_report() because it will
now be called from hidraw_write() and from hidraw_ioctl(). Locking must be
done outside it now (the reason for step 3).
5. Create a hidraw_get_report() function which is similar to
hidraw_set_report() except that it calls hid->hid_get_raw_report() instead
of hid->hid_output_raw_report().
6. Modify hidraw_ioctl() to accept read/write string arguments (remove the
check for _IOC_DIR(cmd) being _IOC_READ).
7. Add the two new ioctls, HIDIOCSFEATURE(len) and HIDIOCGFEATURE(len) which
make calls to hidraw_send_report() and hidraw_get_report(), respectively.
8. In usbhid/hid-core.c, create a new function usb_hid_get_raw_report()
which calls usb_control_msg with a GET_REPORT command.
9. Modify usbhid_output_raw_report() to NOT use the interrupt OUT endpoint
for Feature Reports (as dictated by the USB HID standard).
10. Create a new pointer in hid.h suitable for handling our new
*_get_raw_report() function.
11. Back in usbhid/hid-core.c, set the pointer to hid_get_raw_report to
usbhid_get_raw_report().
12. In include/linux/hidraw.h, add the two new ioctls for our Set and Get
Feature Reports.
Alan.
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 3ccd478..f611300 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -103,14 +103,14 @@ out:
}
/* the first byte is expected to be a report number */
-static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+/* This function is to be called with the minors_lock mutex held */
+static ssize_t hidraw_send_report(struct file *file, const char __user *buffer, size_t count, unsigned char report_type)
{
unsigned int minor = iminor(file->f_path.dentry->d_inode);
struct hid_device *dev;
__u8 *buf;
int ret = 0;
- mutex_lock(&minors_lock);
dev = hidraw_table[minor]->hid;
if (!dev->hid_output_raw_report) {
@@ -143,14 +143,93 @@ static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t
goto out_free;
}
- ret = dev->hid_output_raw_report(dev, buf, count, HID_OUTPUT_REPORT);
+ ret = dev->hid_output_raw_report(dev, buf, count, report_type);
out_free:
kfree(buf);
out:
+ return ret;
+}
+
+/* the first byte is expected to be a report number */
+static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+ mutex_lock(&minors_lock);
+ ret = hidraw_send_report(file, buffer, count, HID_OUTPUT_REPORT);
mutex_unlock(&minors_lock);
return ret;
}
+
+/* This function performs a Get_Report transfer over the control endpoint
+ per section 7.2.1 of the HID specification, version 1.1. The first byte
+ of buffer is the report number to request, or 0x0 if the defice does not
+ use numbered reports. The report_type parameter can be HID_FEATURE_REPORT
+ or HID_INPUT_REPORT. This function is to be called with the minors_lock
+ mutex held. */
+static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t count, unsigned char report_type)
+{
+ unsigned int minor = iminor(file->f_path.dentry->d_inode);
+ struct hid_device *dev;
+ __u8 *buf;
+ int ret = 0, len;
+ unsigned char report_number;
+
+ dev = hidraw_table[minor]->hid;
+
+ if (!dev->hid_get_raw_report) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ if (count > HID_MAX_BUFFER_SIZE) {
+ printk(KERN_WARNING "hidraw: pid %d passed too large report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (count < 2) {
+ printk(KERN_WARNING "hidraw: pid %d passed too short report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ buf = kmalloc(count * sizeof(__u8), GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Read the first byte from the user. This is the report number,
+ which is passed to dev->hid_get_raw_report(). */
+ if (copy_from_user(&report_number, buffer, 1)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = dev->hid_get_raw_report(dev, report_number, buf, count, report_type);
+
+ if (ret < 0) {
+ goto out_free;
+ }
+
+ len = (ret < count)? ret: count;
+
+ if (copy_to_user(buffer, buf, len)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = len;
+
+out_free:
+ kfree(buf);
+out:
+ return ret;
+}
+
static unsigned int hidraw_poll(struct file *file, poll_table *wait)
{
struct hidraw_list *list = file->private_data;
@@ -283,7 +362,24 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
default:
{
struct hid_device *hid = dev->hid;
- if (_IOC_TYPE(cmd) != 'H' || _IOC_DIR(cmd) != _IOC_READ) {
+ if (_IOC_TYPE(cmd) != 'H') {
+ ret = -EINVAL;
+ break;
+ }
+
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+
+ /* Begin Read-only ioctls. */
+ if (_IOC_DIR(cmd) != _IOC_READ) {
ret = -EINVAL;
break;
}
@@ -315,7 +411,7 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
-EFAULT : len;
break;
}
- }
+ }
ret = -ENOTTY;
}
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1ebd324..986b5ac 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -798,6 +798,34 @@ static int hid_alloc_buffers(struct usb_device *dev, struct hid_device *hid)
return 0;
}
+static int usbhid_get_raw_report(struct hid_device *hid,
+ unsigned char report_number, __u8 *buf, size_t count,
+ unsigned char report_type)
+{
+ struct usbhid_device *usbhid = hid->driver_data;
+ struct usb_device *dev = hid_to_usb_dev(hid);
+ struct usb_interface *intf = usbhid->intf;
+ struct usb_host_interface *interface = intf->cur_altsetting;
+ int ret;
+
+ /* Byte 0 is the report number. Report data starts at byte 1.*/
+ buf[0] = report_number;
+
+ ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+ HID_REQ_GET_REPORT,
+ USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ ((report_type + 1) << 8) | report_number,
+ interface->desc.bInterfaceNumber, buf + 1, count - 1,
+ USB_CTRL_SET_TIMEOUT);
+
+ /* count also the report id */
+ if (ret > 0) {
+ ret++;
+ }
+
+ return ret;
+}
+
static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count,
unsigned char report_type)
{
@@ -807,7 +835,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
struct usb_host_interface *interface = intf->cur_altsetting;
int ret;
- if (usbhid->urbout) {
+ if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
int actual_length;
int skipped_report_id = 0;
if (buf[0] == 0x0) {
@@ -1142,6 +1170,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
usb_set_intfdata(intf, hid);
hid->ll_driver = &usb_hid_driver;
+ hid->hid_get_raw_report = usbhid_get_raw_report;
hid->hid_output_raw_report = usbhid_output_raw_report;
hid->ff_init = hid_pidff_init;
#ifdef CONFIG_USB_HIDDEV
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 895001f..e6796c5 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -502,6 +502,9 @@ struct hid_device { /* device report descriptor */
struct hid_usage *, __s32);
void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
+ /* handler for raw input (Get_Report) data, used by hidraw */
+ int (*hid_get_raw_report) (struct hid_device *, unsigned char, __u8 *, size_t, unsigned char);
+
/* handler for raw output data, used by hidraw */
int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t, unsigned char);
diff --git a/include/linux/hidraw.h b/include/linux/hidraw.h
index dd8d692..4b88e69 100644
--- a/include/linux/hidraw.h
+++ b/include/linux/hidraw.h
@@ -35,6 +35,9 @@ struct hidraw_devinfo {
#define HIDIOCGRAWINFO _IOR('H', 0x03, struct hidraw_devinfo)
#define HIDIOCGRAWNAME(len) _IOC(_IOC_READ, 'H', 0x04, len)
#define HIDIOCGRAWPHYS(len) _IOC(_IOC_READ, 'H', 0x05, len)
+/* The first byte of SFEATURE and GFEATURE is the report number */
+#define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
+#define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
#define HIDRAW_FIRST_MINOR 0
#define HIDRAW_MAX_DEVICES 64
On Mon, 7 Jun 2010 23:51:48 -0400
Alan Ott <[email protected]> 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 <[email protected]>
> ---
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,
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.
Regards,
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?
On 06/08/2010 02:32 AM, Antonio Ospite wrote:
> On Mon, 7 Jun 2010 23:51:48 -0400
> Alan Ott<[email protected]> 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<[email protected]>
>> ---
>>
> 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().
> 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.
Thanks for testing my patch. Please let me know if you have problems
with it.
Alan.
On Tue, 08 Jun 2010 09:42:55 -0400
Alan Ott <[email protected]> wrote:
> On 06/08/2010 02:32 AM, Antonio Ospite wrote:
> > On Mon, 7 Jun 2010 23:51:48 -0400
> > Alan Ott<[email protected]> 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<[email protected]>
> >> ---
> >>
> > 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?
On 06/09/2010 04:42 AM, Antonio Ospite wrote:
> 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.
>
I see what you mean, re-using the _struct_ not the code. You would have to add
a report_id, like you said, but it would break the current userspace.
Along those lines, I wouldn't be opposed to making the ioctl instead of taking
a variable-length buffer, take a struct. Something like:
struct hidraw_report {
int report_number;
int size;
char data[HID_MAX_BUFFER_SIZE /*or something similar*/];
};
The thing I ran into is that the user would most conveniently create
the entire struct in userspace, using much more space than is probably necessary
(since most reports aren't going to be anywhere close to the max allowable
size, and the max allowable size must be sufficiently large).
The user _could_ do something like:
char *buf = malloc(sizeof(int)*2 + requried_data_length);
ioctl(fd, ...SREPORT, (hidraw_report*)buf);
but I don't envision most users doing that as it would be error-prone.
This would require hidraw to copy_from_user() only size bytes from the data
field, not the entire thing.
I'm open to doing it this way if it seems to fit existing paradigms better,
but like I said, it will (for most users) require more memory in userspace.
>>> 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.
>
>
I didn't worry about the 80 column warnings, because many of them are
copy/paste from existing code in the same file, and fixing them would have
meant either making the code inconsistent in format with the code around it,
or making formatting corrections which would have violated the "make the
patch do only one thing" rule. I would appreciate some guidance as to what
is the best way to handle this kind of thing.
That said, I had fixed the trailing whitespace errors, but apparently messed
up when generating my patch. I'll put out a v2 shortly.
> 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.
>
Agreed. I've made note of a handful of other (small) things which could
be made a bit better as well. I'm holding off on that stuff until we get this
functionality ironed out.
Thanks,
Alan.
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 <[email protected]>
---
drivers/hid/hidraw.c | 105 +++++++++++++++++++++++++++++++++++++++--
drivers/hid/usbhid/hid-core.c | 30 +++++++++++-
include/linux/hid.h | 3 +
include/linux/hidraw.h | 3 +
4 files changed, 135 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 3ccd478..7669423 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -103,14 +103,14 @@ out:
}
/* the first byte is expected to be a report number */
-static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+/* This function is to be called with the minors_lock mutex held */
+static ssize_t hidraw_send_report(struct file *file, const char __user *buffer, size_t count, unsigned char report_type)
{
unsigned int minor = iminor(file->f_path.dentry->d_inode);
struct hid_device *dev;
__u8 *buf;
int ret = 0;
- mutex_lock(&minors_lock);
dev = hidraw_table[minor]->hid;
if (!dev->hid_output_raw_report) {
@@ -143,14 +143,92 @@ static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t
goto out_free;
}
- ret = dev->hid_output_raw_report(dev, buf, count, HID_OUTPUT_REPORT);
+ ret = dev->hid_output_raw_report(dev, buf, count, report_type);
out_free:
kfree(buf);
out:
+ return ret;
+}
+
+/* the first byte is expected to be a report number */
+static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+ mutex_lock(&minors_lock);
+ ret = hidraw_send_report(file, buffer, count, HID_OUTPUT_REPORT);
mutex_unlock(&minors_lock);
return ret;
}
+
+/* This function performs a Get_Report transfer over the control endpoint
+ per section 7.2.1 of the HID specification, version 1.1. The first byte
+ of buffer is the report number to request, or 0x0 if the defice does not
+ use numbered reports. The report_type parameter can be HID_FEATURE_REPORT
+ or HID_INPUT_REPORT. This function is to be called with the minors_lock
+ mutex held. */
+static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t count, unsigned char report_type)
+{
+ unsigned int minor = iminor(file->f_path.dentry->d_inode);
+ struct hid_device *dev;
+ __u8 *buf;
+ int ret = 0, len;
+ unsigned char report_number;
+
+ dev = hidraw_table[minor]->hid;
+
+ if (!dev->hid_get_raw_report) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ if (count > HID_MAX_BUFFER_SIZE) {
+ printk(KERN_WARNING "hidraw: pid %d passed too large report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (count < 2) {
+ printk(KERN_WARNING "hidraw: pid %d passed too short report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ buf = kmalloc(count * sizeof(__u8), GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Read the first byte from the user. This is the report number,
+ which is passed to dev->hid_get_raw_report(). */
+ if (copy_from_user(&report_number, buffer, 1)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = dev->hid_get_raw_report(dev, report_number, buf, count, report_type);
+
+ if (ret < 0)
+ goto out_free;
+
+ len = (ret < count) ? ret : count;
+
+ if (copy_to_user(buffer, buf, len)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = len;
+
+out_free:
+ kfree(buf);
+out:
+ return ret;
+}
+
static unsigned int hidraw_poll(struct file *file, poll_table *wait)
{
struct hidraw_list *list = file->private_data;
@@ -283,7 +361,24 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
default:
{
struct hid_device *hid = dev->hid;
- if (_IOC_TYPE(cmd) != 'H' || _IOC_DIR(cmd) != _IOC_READ) {
+ if (_IOC_TYPE(cmd) != 'H') {
+ ret = -EINVAL;
+ break;
+ }
+
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+
+ /* Begin Read-only ioctls. */
+ if (_IOC_DIR(cmd) != _IOC_READ) {
ret = -EINVAL;
break;
}
@@ -315,7 +410,7 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
-EFAULT : len;
break;
}
- }
+ }
ret = -ENOTTY;
}
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1ebd324..deef816 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -798,6 +798,33 @@ static int hid_alloc_buffers(struct usb_device *dev, struct hid_device *hid)
return 0;
}
+static int usbhid_get_raw_report(struct hid_device *hid,
+ unsigned char report_number, __u8 *buf, size_t count,
+ unsigned char report_type)
+{
+ struct usbhid_device *usbhid = hid->driver_data;
+ struct usb_device *dev = hid_to_usb_dev(hid);
+ struct usb_interface *intf = usbhid->intf;
+ struct usb_host_interface *interface = intf->cur_altsetting;
+ int ret;
+
+ /* Byte 0 is the report number. Report data starts at byte 1.*/
+ buf[0] = report_number;
+
+ ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+ HID_REQ_GET_REPORT,
+ USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ ((report_type + 1) << 8) | report_number,
+ interface->desc.bInterfaceNumber, buf + 1, count - 1,
+ USB_CTRL_SET_TIMEOUT);
+
+ /* count also the report id */
+ if (ret > 0)
+ ret++;
+
+ return ret;
+}
+
static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count,
unsigned char report_type)
{
@@ -807,7 +834,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
struct usb_host_interface *interface = intf->cur_altsetting;
int ret;
- if (usbhid->urbout) {
+ if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
int actual_length;
int skipped_report_id = 0;
if (buf[0] == 0x0) {
@@ -1142,6 +1169,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
usb_set_intfdata(intf, hid);
hid->ll_driver = &usb_hid_driver;
+ hid->hid_get_raw_report = usbhid_get_raw_report;
hid->hid_output_raw_report = usbhid_output_raw_report;
hid->ff_init = hid_pidff_init;
#ifdef CONFIG_USB_HIDDEV
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 895001f..e6796c5 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -502,6 +502,9 @@ struct hid_device { /* device report descriptor */
struct hid_usage *, __s32);
void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
+ /* handler for raw input (Get_Report) data, used by hidraw */
+ int (*hid_get_raw_report) (struct hid_device *, unsigned char, __u8 *, size_t, unsigned char);
+
/* handler for raw output data, used by hidraw */
int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t, unsigned char);
diff --git a/include/linux/hidraw.h b/include/linux/hidraw.h
index dd8d692..4b88e69 100644
--- a/include/linux/hidraw.h
+++ b/include/linux/hidraw.h
@@ -35,6 +35,9 @@ struct hidraw_devinfo {
#define HIDIOCGRAWINFO _IOR('H', 0x03, struct hidraw_devinfo)
#define HIDIOCGRAWNAME(len) _IOC(_IOC_READ, 'H', 0x04, len)
#define HIDIOCGRAWPHYS(len) _IOC(_IOC_READ, 'H', 0x05, len)
+/* The first byte of SFEATURE and GFEATURE is the report number */
+#define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
+#define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
#define HIDRAW_FIRST_MINOR 0
#define HIDRAW_MAX_DEVICES 64
--
1.7.0.4
On Wed, 9 Jun 2010 11:54:28 -0400
Alan Ott <[email protected]> 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 <[email protected]>
Tested-by: Antonio Ospite <[email protected]>
Regards,
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?
On Wed, 9 Jun 2010, Alan Ott 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.
Hi Alan,
thanks for the patch. Could you please also update the Bluetooth
implementation (in net/bluetooth/hidp/).
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
On Wed, 9 Jun 2010, Alan Ott 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.
Applied, thanks Alan. The Bluetooth version I will still like to get
either Acked or merged by Marcel.
--
Jiri Kosina
SUSE Labs, Novell Inc.
Version 3 of the patch. Fixing the problem reported by Amit Nagal where
Report IDs were not getting sent to the device when using Set_Report control
tranfers.
Alan Ott (1):
HID: Add Support for Setting and Getting Feature Reports from hidraw
drivers/hid/hidraw.c | 105 +++++++++++++++++++++++++++++++++++++++--
drivers/hid/usbhid/hid-core.c | 31 ++++++++++--
include/linux/hid.h | 3 +
include/linux/hidraw.h | 3 +
4 files changed, 132 insertions(+), 10 deletions(-)
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 <[email protected]>
---
drivers/hid/hidraw.c | 105 +++++++++++++++++++++++++++++++++++++++--
drivers/hid/usbhid/hid-core.c | 31 ++++++++++--
include/linux/hid.h | 3 +
include/linux/hidraw.h | 3 +
4 files changed, 132 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 3ccd478..7669423 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -103,14 +103,14 @@ out:
}
/* the first byte is expected to be a report number */
-static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+/* This function is to be called with the minors_lock mutex held */
+static ssize_t hidraw_send_report(struct file *file, const char __user *buffer, size_t count, unsigned char report_type)
{
unsigned int minor = iminor(file->f_path.dentry->d_inode);
struct hid_device *dev;
__u8 *buf;
int ret = 0;
- mutex_lock(&minors_lock);
dev = hidraw_table[minor]->hid;
if (!dev->hid_output_raw_report) {
@@ -143,14 +143,92 @@ static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t
goto out_free;
}
- ret = dev->hid_output_raw_report(dev, buf, count, HID_OUTPUT_REPORT);
+ ret = dev->hid_output_raw_report(dev, buf, count, report_type);
out_free:
kfree(buf);
out:
+ return ret;
+}
+
+/* the first byte is expected to be a report number */
+static ssize_t hidraw_write(struct file *file, const char __user *buffer, size_t count, loff_t *ppos)
+{
+ ssize_t ret;
+ mutex_lock(&minors_lock);
+ ret = hidraw_send_report(file, buffer, count, HID_OUTPUT_REPORT);
mutex_unlock(&minors_lock);
return ret;
}
+
+/* This function performs a Get_Report transfer over the control endpoint
+ per section 7.2.1 of the HID specification, version 1.1. The first byte
+ of buffer is the report number to request, or 0x0 if the defice does not
+ use numbered reports. The report_type parameter can be HID_FEATURE_REPORT
+ or HID_INPUT_REPORT. This function is to be called with the minors_lock
+ mutex held. */
+static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t count, unsigned char report_type)
+{
+ unsigned int minor = iminor(file->f_path.dentry->d_inode);
+ struct hid_device *dev;
+ __u8 *buf;
+ int ret = 0, len;
+ unsigned char report_number;
+
+ dev = hidraw_table[minor]->hid;
+
+ if (!dev->hid_get_raw_report) {
+ ret = -ENODEV;
+ goto out;
+ }
+
+ if (count > HID_MAX_BUFFER_SIZE) {
+ printk(KERN_WARNING "hidraw: pid %d passed too large report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (count < 2) {
+ printk(KERN_WARNING "hidraw: pid %d passed too short report\n",
+ task_pid_nr(current));
+ ret = -EINVAL;
+ goto out;
+ }
+
+ buf = kmalloc(count * sizeof(__u8), GFP_KERNEL);
+ if (!buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
+ /* Read the first byte from the user. This is the report number,
+ which is passed to dev->hid_get_raw_report(). */
+ if (copy_from_user(&report_number, buffer, 1)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = dev->hid_get_raw_report(dev, report_number, buf, count, report_type);
+
+ if (ret < 0)
+ goto out_free;
+
+ len = (ret < count) ? ret : count;
+
+ if (copy_to_user(buffer, buf, len)) {
+ ret = -EFAULT;
+ goto out_free;
+ }
+
+ ret = len;
+
+out_free:
+ kfree(buf);
+out:
+ return ret;
+}
+
static unsigned int hidraw_poll(struct file *file, poll_table *wait)
{
struct hidraw_list *list = file->private_data;
@@ -283,7 +361,24 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
default:
{
struct hid_device *hid = dev->hid;
- if (_IOC_TYPE(cmd) != 'H' || _IOC_DIR(cmd) != _IOC_READ) {
+ if (_IOC_TYPE(cmd) != 'H') {
+ ret = -EINVAL;
+ break;
+ }
+
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCSFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_send_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+ if (_IOC_NR(cmd) == _IOC_NR(HIDIOCGFEATURE(0))) {
+ int len = _IOC_SIZE(cmd);
+ ret = hidraw_get_report(file, user_arg, len, HID_FEATURE_REPORT);
+ break;
+ }
+
+ /* Begin Read-only ioctls. */
+ if (_IOC_DIR(cmd) != _IOC_READ) {
ret = -EINVAL;
break;
}
@@ -315,7 +410,7 @@ static long hidraw_ioctl(struct file *file, unsigned int cmd,
-EFAULT : len;
break;
}
- }
+ }
ret = -ENOTTY;
}
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 1ebd324..099eb81 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -798,6 +798,29 @@ static int hid_alloc_buffers(struct usb_device *dev, struct hid_device *hid)
return 0;
}
+static int usbhid_get_raw_report(struct hid_device *hid,
+ unsigned char report_number, __u8 *buf, size_t count,
+ unsigned char report_type)
+{
+ struct usbhid_device *usbhid = hid->driver_data;
+ struct usb_device *dev = hid_to_usb_dev(hid);
+ struct usb_interface *intf = usbhid->intf;
+ struct usb_host_interface *interface = intf->cur_altsetting;
+ int ret;
+
+ /* Byte 0 is the report number. Report data starts at byte 1.*/
+ buf[0] = report_number;
+
+ ret = usb_control_msg(dev, usb_rcvctrlpipe(dev, 0),
+ HID_REQ_GET_REPORT,
+ USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ ((report_type + 1) << 8) | report_number,
+ interface->desc.bInterfaceNumber, buf, count,
+ USB_CTRL_SET_TIMEOUT);
+
+ return ret;
+}
+
static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count,
unsigned char report_type)
{
@@ -807,7 +830,7 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
struct usb_host_interface *interface = intf->cur_altsetting;
int ret;
- if (usbhid->urbout) {
+ if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
int actual_length;
int skipped_report_id = 0;
if (buf[0] == 0x0) {
@@ -831,11 +854,8 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
HID_REQ_SET_REPORT,
USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
((report_type + 1) << 8) | *buf,
- interface->desc.bInterfaceNumber, buf + 1, count - 1,
+ interface->desc.bInterfaceNumber, buf, count,
USB_CTRL_SET_TIMEOUT);
- /* count also the report id */
- if (ret > 0)
- ret++;
}
return ret;
@@ -1142,6 +1162,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
usb_set_intfdata(intf, hid);
hid->ll_driver = &usb_hid_driver;
+ hid->hid_get_raw_report = usbhid_get_raw_report;
hid->hid_output_raw_report = usbhid_output_raw_report;
hid->ff_init = hid_pidff_init;
#ifdef CONFIG_USB_HIDDEV
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 895001f..e6796c5 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -502,6 +502,9 @@ struct hid_device { /* device report descriptor */
struct hid_usage *, __s32);
void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
+ /* handler for raw input (Get_Report) data, used by hidraw */
+ int (*hid_get_raw_report) (struct hid_device *, unsigned char, __u8 *, size_t, unsigned char);
+
/* handler for raw output data, used by hidraw */
int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t, unsigned char);
diff --git a/include/linux/hidraw.h b/include/linux/hidraw.h
index dd8d692..4b88e69 100644
--- a/include/linux/hidraw.h
+++ b/include/linux/hidraw.h
@@ -35,6 +35,9 @@ struct hidraw_devinfo {
#define HIDIOCGRAWINFO _IOR('H', 0x03, struct hidraw_devinfo)
#define HIDIOCGRAWNAME(len) _IOC(_IOC_READ, 'H', 0x04, len)
#define HIDIOCGRAWPHYS(len) _IOC(_IOC_READ, 'H', 0x05, len)
+/* The first byte of SFEATURE and GFEATURE is the report number */
+#define HIDIOCSFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
+#define HIDIOCGFEATURE(len) _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
#define HIDRAW_FIRST_MINOR 0
#define HIDRAW_MAX_DEVICES 64
--
1.7.0.4