This patch adds Feature Report support for USB and Bluetooth HID devices
through hidraw.
The first two patches prepare the bluetooth side for the change.
a. Make sure the hidp_session() thread is started before
device's probe() functions are called.
b. Wait for ACK/NAK on sent reports, and return proper
error codes.
The third patch is the hidraw core and USB changes.
The fourth patch is the Bluetooth changes.
Thanks to Antonio Ospite and Bill Good for providing testing and feedback.
Alan Ott (4):
bt hidp: Move hid_add_device() call to after hidp_session() has
started.
bt hidp: Wait for ACK on Sent Reports
HID: Add Support for Setting and Getting Feature Reports from hidraw
Bluetooth hidp: Add support for hidraw HIDIOCGFEATURE and
HIDIOCSFEATURE
drivers/hid/hidraw.c | 106 +++++++++++++++++++-
drivers/hid/usbhid/hid-core.c | 35 +++++++
include/linux/hid.h | 3 +
include/linux/hidraw.h | 3 +
net/bluetooth/hidp/core.c | 214 ++++++++++++++++++++++++++++++++++++++---
net/bluetooth/hidp/hidp.h | 15 +++
6 files changed, 355 insertions(+), 21 deletions(-)
On Tue, 18 Jan 2011, Alan Ott wrote:
> This patch adds Feature Report support for USB and Bluetooth HID devices
> through hidraw.
>
> The first two patches prepare the bluetooth side for the change.
> a. Make sure the hidp_session() thread is started before
> device's probe() functions are called.
> b. Wait for ACK/NAK on sent reports, and return proper
> error codes.
> The third patch is the hidraw core and USB changes.
> The fourth patch is the Bluetooth changes.
>
> Thanks to Antonio Ospite and Bill Good for providing testing and feedback.
>
>
> Alan Ott (4):
> bt hidp: Move hid_add_device() call to after hidp_session() has
> started.
> bt hidp: Wait for ACK on Sent Reports
> HID: Add Support for Setting and Getting Feature Reports from hidraw
> Bluetooth hidp: Add support for hidraw HIDIOCGFEATURE and
> HIDIOCSFEATURE
>
> drivers/hid/hidraw.c | 106 +++++++++++++++++++-
> drivers/hid/usbhid/hid-core.c | 35 +++++++
> include/linux/hid.h | 3 +
> include/linux/hidraw.h | 3 +
> net/bluetooth/hidp/core.c | 214 ++++++++++++++++++++++++++++++++++++++---
> net/bluetooth/hidp/hidp.h | 15 +++
Before proceeding with these patches, I'd really like to have comment
(ideally 'Acked-by') from Marcel on the net/bluetooth/hidp part,
obviously.
Marcel?
Thanks,
--
Jiri Kosina
SUSE Labs, Novell Inc.
This patch adds support or getting and setting feature reports for bluetooth
HID devices from HIDRAW.
Signed-off-by: Alan Ott <[email protected]>
---
net/bluetooth/hidp/core.c | 113 +++++++++++++++++++++++++++++++++++++++++++--
net/bluetooth/hidp/hidp.h | 8 +++
2 files changed, 117 insertions(+), 4 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 5383e6c..6df8ea1 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -36,6 +36,7 @@
#include <linux/file.h>
#include <linux/init.h>
#include <linux/wait.h>
+#include <linux/mutex.h>
#include <net/sock.h>
#include <linux/input.h>
@@ -313,6 +314,86 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
return hidp_queue_report(session, buf, rsize);
}
+static int hidp_get_raw_report(struct hid_device *hid,
+ unsigned char report_number,
+ unsigned char *data, size_t count,
+ unsigned char report_type)
+{
+ struct hidp_session *session = hid->driver_data;
+ struct sk_buff *skb;
+ size_t len;
+ int numbered_reports = hid->report_enum[report_type].numbered;
+
+ switch (report_type) {
+ case HID_FEATURE_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_FEATURE;
+ break;
+ case HID_INPUT_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_INPUT;
+ break;
+ case HID_OUTPUT_REPORT:
+ report_type = HIDP_TRANS_GET_REPORT | HIDP_DATA_RTYPE_OUPUT;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ if (mutex_lock_interruptible(&session->report_mutex))
+ return -ERESTARTSYS;
+
+ /* Set up our wait, and send the report request to the device. */
+ session->waiting_report_type = report_type & HIDP_DATA_RTYPE_MASK;
+ session->waiting_report_number = numbered_reports ? report_number : -1;
+ set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ data[0] = report_number;
+ if (hidp_send_ctrl_message(hid->driver_data, report_type, data, 1))
+ goto err_eio;
+
+ /* Wait for the return of the report. The returned report
+ gets put in session->report_return. */
+ while (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
+ int res;
+
+ res = wait_event_interruptible_timeout(session->report_queue,
+ !test_bit(HIDP_WAITING_FOR_RETURN, &session->flags),
+ 5*HZ);
+ if (res == 0) {
+ /* timeout */
+ goto err_eio;
+ }
+ if (res < 0) {
+ /* signal */
+ goto err_restartsys;
+ }
+ }
+
+ skb = session->report_return;
+ if (skb) {
+ len = skb->len < count ? skb->len : count;
+ memcpy(data, skb->data, len);
+
+ kfree_skb(skb);
+ session->report_return = NULL;
+ } else {
+ /* Device returned a HANDSHAKE, indicating protocol error. */
+ len = -EIO;
+ }
+
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+
+ return len;
+
+err_restartsys:
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+ return -ERESTARTSYS;
+err_eio:
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ mutex_unlock(&session->report_mutex);
+ return -EIO;
+}
+
static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
unsigned char report_type)
{
@@ -409,6 +490,10 @@ static void hidp_process_handshake(struct hidp_session *session,
case HIDP_HSHK_ERR_INVALID_REPORT_ID:
case HIDP_HSHK_ERR_UNSUPPORTED_REQUEST:
case HIDP_HSHK_ERR_INVALID_PARAMETER:
+ if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags)) {
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ wake_up_interruptible(&session->report_queue);
+ }
/* FIXME: Call into SET_ GET_ handlers here */
break;
@@ -451,9 +536,11 @@ static void hidp_process_hid_control(struct hidp_session *session,
}
}
-static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
+/* Returns true if the passed-in skb should be freed by the caller. */
+static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
unsigned char param)
{
+ int done_with_skb = 1;
BT_DBG("session %p skb %p len %d param 0x%02x", session, skb, skb->len, param);
switch (param) {
@@ -465,7 +552,6 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
if (session->hid)
hid_input_report(session->hid, HID_INPUT_REPORT, skb->data, skb->len, 0);
-
break;
case HIDP_DATA_RTYPE_OTHER:
@@ -477,12 +563,27 @@ static void hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
__hidp_send_ctrl_message(session,
HIDP_TRANS_HANDSHAKE | HIDP_HSHK_ERR_INVALID_PARAMETER, NULL, 0);
}
+
+ if (test_bit(HIDP_WAITING_FOR_RETURN, &session->flags) &&
+ param == session->waiting_report_type) {
+ if (session->waiting_report_number < 0 ||
+ session->waiting_report_number == skb->data[0]) {
+ /* hidp_get_raw_report() is waiting on this report. */
+ session->report_return = skb;
+ done_with_skb = 0;
+ clear_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
+ wake_up_interruptible(&session->report_queue);
+ }
+ }
+
+ return done_with_skb;
}
static void hidp_recv_ctrl_frame(struct hidp_session *session,
struct sk_buff *skb)
{
unsigned char hdr, type, param;
+ int free_skb = 1;
BT_DBG("session %p skb %p len %d", session, skb, skb->len);
@@ -502,7 +603,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
break;
case HIDP_TRANS_DATA:
- hidp_process_data(session, skb, param);
+ free_skb = hidp_process_data(session, skb, param);
break;
default:
@@ -511,7 +612,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
break;
}
- kfree_skb(skb);
+ if (free_skb)
+ kfree_skb(skb);
}
static void hidp_recv_intr_frame(struct hidp_session *session,
@@ -845,6 +947,7 @@ static int hidp_setup_hid(struct hidp_session *session,
hid->dev.parent = hidp_get_device(session);
hid->ll_driver = &hidp_hid_driver;
+ hid->hid_get_raw_report = hidp_get_raw_report;
hid->hid_output_raw_report = hidp_output_raw_report;
return 0;
@@ -897,6 +1000,8 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
skb_queue_head_init(&session->ctrl_transmit);
skb_queue_head_init(&session->intr_transmit);
+ mutex_init(&session->report_mutex);
+ init_waitqueue_head(&session->report_queue);
init_waitqueue_head(&session->startup_queue);
session->waiting_for_startup = 1;
session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID);
diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
index 92e093e..13de5fa 100644
--- a/net/bluetooth/hidp/hidp.h
+++ b/net/bluetooth/hidp/hidp.h
@@ -80,6 +80,7 @@
#define HIDP_VIRTUAL_CABLE_UNPLUG 0
#define HIDP_BOOT_PROTOCOL_MODE 1
#define HIDP_BLUETOOTH_VENDOR_ID 9
+#define HIDP_WAITING_FOR_RETURN 10
#define HIDP_WAITING_FOR_SEND_ACK 11
struct hidp_connadd_req {
@@ -155,6 +156,13 @@ struct hidp_session {
struct sk_buff_head ctrl_transmit;
struct sk_buff_head intr_transmit;
+ /* Used in hidp_get_raw_report() */
+ int waiting_report_type; /* HIDP_DATA_RTYPE_* */
+ int waiting_report_number; /* -1 for not numbered */
+ struct mutex report_mutex;
+ struct sk_buff *report_return;
+ wait_queue_head_t report_queue;
+
/* Used in hidp_output_raw_report() */
int output_report_success; /* boolean */
--
1.7.0.4
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]>
Signed-off-by: Antonio Ospite <[email protected]>
---
drivers/hid/hidraw.c | 106 ++++++++++++++++++++++++++++++++++++++--
drivers/hid/usbhid/hid-core.c | 35 +++++++++++++
include/linux/hid.h | 3 +
include/linux/hidraw.h | 3 +
4 files changed, 141 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 468e87b..8f06044 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -102,15 +102,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);
-
if (!hidraw_table[minor]) {
ret = -ENODEV;
goto out;
@@ -148,14 +147,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;
@@ -295,7 +372,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;
}
@@ -327,7 +421,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 b336dd8..38c261a 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -799,6 +799,40 @@ 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 skipped_report_id = 0;
+ int ret;
+
+ /* Byte 0 is the report number. Report data starts at byte 1.*/
+ buf[0] = report_number;
+ if (report_number == 0x0) {
+ /* Offset the return buffer by 1, so that the report ID
+ will remain in byte 0. */
+ buf++;
+ count--;
+ skipped_report_id = 1;
+ }
+ 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);
+
+ /* count also the report id */
+ if (ret > 0 && skipped_report_id)
+ ret++;
+
+ return ret;
+}
+
static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t count,
unsigned char report_type)
{
@@ -1139,6 +1173,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 d91c25e..e8ee0a9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -504,6 +504,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
Wait for an ACK from the device before returning from
hidp_output_raw_report(). This way, failures can be returned to the user
application. Also, it prevents ACK/NAK packets from an output packet from
being confused with ACK/NAK packets from an input request packet.
Signed-off-by: Alan Ott <[email protected]>
---
net/bluetooth/hidp/core.c | 54 ++++++++++++++++++++++++++++++++++++++++++--
net/bluetooth/hidp/hidp.h | 4 +++
2 files changed, 55 insertions(+), 3 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 67cc4bc..5383e6c 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -316,6 +316,9 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep
static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
unsigned char report_type)
{
+ struct hidp_session *session = hid->driver_data;
+ int ret;
+
switch (report_type) {
case HID_FEATURE_REPORT:
report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
@@ -327,10 +330,47 @@ static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, s
return -EINVAL;
}
+ if (mutex_lock_interruptible(&session->report_mutex))
+ return -ERESTARTSYS;
+
+ /* Set up our wait, and send the report request to the device. */
+ set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
if (hidp_send_ctrl_message(hid->driver_data, report_type,
- data, count))
- return -ENOMEM;
- return count;
+ data, count)) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
+ /* Wait for the ACK from the device. */
+ while (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)) {
+ int res;
+
+ res = wait_event_interruptible_timeout(session->report_queue,
+ !test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags),
+ 10*HZ);
+ if (res == 0) {
+ /* timeout */
+ ret = -EIO;
+ goto err;
+ }
+ if (res < 0) {
+ /* signal */
+ ret = -ERESTARTSYS;
+ goto err;
+ }
+ }
+
+ if (!session->output_report_success) {
+ ret = -EIO;
+ goto err;
+ }
+
+ ret = count;
+
+err:
+ clear_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
+ mutex_unlock(&session->report_mutex);
+ return ret;
}
static void hidp_idle_timeout(unsigned long arg)
@@ -357,10 +397,12 @@ static void hidp_process_handshake(struct hidp_session *session,
unsigned char param)
{
BT_DBG("session %p param 0x%02x", session, param);
+ session->output_report_success = 0; /* default condition */
switch (param) {
case HIDP_HSHK_SUCCESSFUL:
/* FIXME: Call into SET_ GET_ handlers here */
+ session->output_report_success = 1;
break;
case HIDP_HSHK_NOT_READY:
@@ -385,6 +427,12 @@ static void hidp_process_handshake(struct hidp_session *session,
HIDP_TRANS_HANDSHAKE | HIDP_HSHK_ERR_INVALID_PARAMETER, NULL, 0);
break;
}
+
+ /* Wake up the waiting thread. */
+ if (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)) {
+ clear_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
+ wake_up_interruptible(&session->report_queue);
+ }
}
static void hidp_process_hid_control(struct hidp_session *session,
diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
index 2cc35dc..92e093e 100644
--- a/net/bluetooth/hidp/hidp.h
+++ b/net/bluetooth/hidp/hidp.h
@@ -80,6 +80,7 @@
#define HIDP_VIRTUAL_CABLE_UNPLUG 0
#define HIDP_BOOT_PROTOCOL_MODE 1
#define HIDP_BLUETOOTH_VENDOR_ID 9
+#define HIDP_WAITING_FOR_SEND_ACK 11
struct hidp_connadd_req {
int ctrl_sock; // Connected control socket
@@ -154,6 +155,9 @@ struct hidp_session {
struct sk_buff_head ctrl_transmit;
struct sk_buff_head intr_transmit;
+ /* Used in hidp_output_raw_report() */
+ int output_report_success; /* boolean */
+
/* Report descriptor */
__u8 *rd_data;
uint rd_size;
--
1.7.0.4
Move the call to hid_add_device() (which calls a device's probe() function)
to after the kernel_thread() call which starts the hidp_session() thread.
This ensures the Bluetooth receive socket is fully running by the time a
device's probe() function is called. This way, a device can communicate
(send and receive) with the Bluetooth device from its probe() function.
Signed-off-by: Alan Ott <[email protected]>
---
net/bluetooth/hidp/core.c | 28 ++++++++++++++++++++--------
net/bluetooth/hidp/hidp.h | 3 +++
2 files changed, 23 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 29544c2..67cc4bc 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -563,6 +563,8 @@ static int hidp_session(void *arg)
init_waitqueue_entry(&intr_wait, current);
add_wait_queue(sk_sleep(ctrl_sk), &ctrl_wait);
add_wait_queue(sk_sleep(intr_sk), &intr_wait);
+ session->waiting_for_startup = 0;
+ wake_up_interruptible(&session->startup_queue);
while (!atomic_read(&session->terminate)) {
set_current_state(TASK_INTERRUPTIBLE);
@@ -754,6 +756,8 @@ static struct hid_ll_driver hidp_hid_driver = {
.hidinput_input_event = hidp_hidinput_event,
};
+/* This function sets up the hid device. It does not add it
+ to the HID system. That is done in hidp_add_connection(). */
static int hidp_setup_hid(struct hidp_session *session,
struct hidp_connadd_req *req)
{
@@ -795,16 +799,8 @@ static int hidp_setup_hid(struct hidp_session *session,
hid->hid_output_raw_report = hidp_output_raw_report;
- err = hid_add_device(hid);
- if (err < 0)
- goto failed;
-
return 0;
-failed:
- hid_destroy_device(hid);
- session->hid = NULL;
-
fault:
kfree(session->rd_data);
session->rd_data = NULL;
@@ -853,6 +849,8 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
skb_queue_head_init(&session->ctrl_transmit);
skb_queue_head_init(&session->intr_transmit);
+ init_waitqueue_head(&session->startup_queue);
+ session->waiting_for_startup = 1;
session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID);
session->idle_to = req->idle_to;
@@ -875,6 +873,14 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
err = kernel_thread(hidp_session, session, CLONE_KERNEL);
if (err < 0)
goto unlink;
+ while (session->waiting_for_startup) {
+ wait_event_interruptible(session->startup_queue,
+ !session->waiting_for_startup);
+ }
+
+ err = hid_add_device(session->hid);
+ if (err < 0)
+ goto err_add_device;
if (session->input) {
hidp_send_ctrl_message(session,
@@ -888,6 +894,12 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock,
up_write(&hidp_session_sem);
return 0;
+err_add_device:
+ hid_destroy_device(session->hid);
+ session->hid = NULL;
+ atomic_inc(&session->terminate);
+ hidp_schedule(session);
+
unlink:
hidp_del_timer(session);
diff --git a/net/bluetooth/hidp/hidp.h b/net/bluetooth/hidp/hidp.h
index 8d934a1..2cc35dc 100644
--- a/net/bluetooth/hidp/hidp.h
+++ b/net/bluetooth/hidp/hidp.h
@@ -157,6 +157,9 @@ struct hidp_session {
/* Report descriptor */
__u8 *rd_data;
uint rd_size;
+
+ wait_queue_head_t startup_queue;
+ int waiting_for_startup;
};
static inline void hidp_schedule(struct hidp_session *session)
--
1.7.0.4
On Thu, 10 Feb 2011, Gustavo F. Padovan wrote:
> > > This patch adds Feature Report support for USB and Bluetooth HID devices
> > > through hidraw.
> > >
> > > The first two patches prepare the bluetooth side for the change.
> > > a. Make sure the hidp_session() thread is started before
> > > device's probe() functions are called.
> > > b. Wait for ACK/NAK on sent reports, and return proper
> > > error codes.
> > > The third patch is the hidraw core and USB changes.
> > > The fourth patch is the Bluetooth changes.
> > >
> > > Thanks to Antonio Ospite and Bill Good for providing testing and feedback.
> > >
> > >
> > > Alan Ott (4):
> > > bt hidp: Move hid_add_device() call to after hidp_session() has
> > > started.
> > > bt hidp: Wait for ACK on Sent Reports
> > > HID: Add Support for Setting and Getting Feature Reports from hidraw
> > > Bluetooth hidp: Add support for hidraw HIDIOCGFEATURE and
> > > HIDIOCSFEATURE
> > >
> > > drivers/hid/hidraw.c | 106 +++++++++++++++++++-
> > > drivers/hid/usbhid/hid-core.c | 35 +++++++
> > > include/linux/hid.h | 3 +
> > > include/linux/hidraw.h | 3 +
> > > net/bluetooth/hidp/core.c | 214 ++++++++++++++++++++++++++++++++++++++---
> > > net/bluetooth/hidp/hidp.h | 15 +++
> >
> > Before proceeding with these patches, I'd really like to have comment
> > (ideally 'Acked-by') from Marcel on the net/bluetooth/hidp part,
> > obviously.
>
> I have tested it and it seems ok to me and to Marcel. For net/bluetooth/
>
> Acked-by: Gustavo F. Padovan <[email protected]>
Awesome, thanks a lot everyone for having this sorted out finally!
I will be taking the patchset through my tree.
--
Jiri Kosina
SUSE Labs, Novell Inc.
Hi Jiri,
* Jiri Kosina <[email protected]> [2011-01-19 13:17:00 +0100]:
> On Tue, 18 Jan 2011, Alan Ott wrote:
>
> > This patch adds Feature Report support for USB and Bluetooth HID devices
> > through hidraw.
> >
> > The first two patches prepare the bluetooth side for the change.
> > a. Make sure the hidp_session() thread is started before
> > device's probe() functions are called.
> > b. Wait for ACK/NAK on sent reports, and return proper
> > error codes.
> > The third patch is the hidraw core and USB changes.
> > The fourth patch is the Bluetooth changes.
> >
> > Thanks to Antonio Ospite and Bill Good for providing testing and feedback.
> >
> >
> > Alan Ott (4):
> > bt hidp: Move hid_add_device() call to after hidp_session() has
> > started.
> > bt hidp: Wait for ACK on Sent Reports
> > HID: Add Support for Setting and Getting Feature Reports from hidraw
> > Bluetooth hidp: Add support for hidraw HIDIOCGFEATURE and
> > HIDIOCSFEATURE
> >
> > drivers/hid/hidraw.c | 106 +++++++++++++++++++-
> > drivers/hid/usbhid/hid-core.c | 35 +++++++
> > include/linux/hid.h | 3 +
> > include/linux/hidraw.h | 3 +
> > net/bluetooth/hidp/core.c | 214 ++++++++++++++++++++++++++++++++++++++---
> > net/bluetooth/hidp/hidp.h | 15 +++
>
> Before proceeding with these patches, I'd really like to have comment
> (ideally 'Acked-by') from Marcel on the net/bluetooth/hidp part,
> obviously.
I have tested it and it seems ok to me and to Marcel. For net/bluetooth/
Acked-by: Gustavo F. Padovan <[email protected]>
Regards,
--
Gustavo F. Padovan
http://profusion.mobi
On Tue, 18 Jan 2011 03:04:36 -0500
Alan Ott <[email protected]> wrote:
> This patch adds Feature Report support for USB and Bluetooth HID devices
> through hidraw.
>
> The first two patches prepare the bluetooth side for the change.
> a. Make sure the hidp_session() thread is started before
> device's probe() functions are called.
> b. Wait for ACK/NAK on sent reports, and return proper
> error codes.
> The third patch is the hidraw core and USB changes.
In particular, this third patch is a prerequisite for the bluez
sixaxis-cable plugin, originally written by Bastien Nocera (added to
CC) which I plan to submit when these changes are merged in mainline
linux.
[...]
--
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?