[PATCH 0/4] HID: implement new transport-driver callbacks

[PATCH 0/4] HID: implement new transport-driver callbacks

[Frank Praznik]

These patches are originally the work of David Herrmann who suggested that I 
update and submit them as their functionality is required for some pending
patches to the hid-sony driver.

These patches implement the SET/GET_REPORT and raw intr OUTPUT requests for all
transport drivers.  It adds two callbacks to the hid_ll_driver struct:

int (*raw_request)(struct hid_device *hdev, unsigned char reportnum,
		   __u8 *buf, size_t len, unsigned char rtype, int reqtype);

int (*output_report)(struct hid_device *hdev, __u8 *buf, size_t len);

along with the necessary support fuctions in the USBHID, and HIDP drivers.

UHID is not converted yet.

[PATCH 1/4] HID: Add transport-driver callbacks to the hid_ll_driver struct

[Frank Praznik]

Add raw_request and output_report callbacks to the hid_ll_driver struct.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>

---
 include/linux/hid.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 31b9d29..003cc8e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -700,8 +700,14 @@ struct hid_ll_driver {
 			struct hid_report *report, int reqtype);
 
 	int (*wait)(struct hid_device *hdev);
-	int (*idle)(struct hid_device *hdev, int report, int idle, int reqtype);
 
+	int (*raw_request) (struct hid_device *hdev, unsigned char reportnum,
+			    __u8 *buf, size_t len, unsigned char rtype,
+			    int reqtype);
+
+	int (*output_report) (struct hid_device *hdev, __u8 *buf, size_t len);
+
+	int (*idle)(struct hid_device *hdev, int report, int idle, int reqtype);
 };
 
 #define	PM_HINT_FULLON	1<<5
-- 
1.8.4.2

[PATCH 2/4] HID: Add transport-driver functions to the USB HID interface.

[Frank Praznik]

Add raw_request, set_raw_report and output_report transport-driver functions to
the USB HID driver.
 
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>

---
 drivers/hid/usbhid/hid-core.c | 78 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 78 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 44df131..f8ca312 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -884,6 +884,38 @@ static int usbhid_get_raw_report(struct hid_device *hid,
 	return ret;
 }
 
+static int usbhid_set_raw_report(struct hid_device *hid, unsigned int reportnum,
+				 __u8 *buf, size_t count, unsigned char rtype)
+{
+	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, skipped_report_id = 0;
+
+	/* Byte 0 is the report number. Report data starts at byte 1.*/
+	buf[0] = reportnum;
+	if (buf[0] == 0x0) {
+		/* Don't send the Report ID */
+		buf++;
+		count--;
+		skipped_report_id = 1;
+	}
+
+	ret = usb_control_msg(dev, usb_sndctrlpipe(dev, 0),
+			HID_REQ_SET_REPORT,
+			USB_DIR_OUT | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+			((rtype + 1) << 8) | reportnum,
+			interface->desc.bInterfaceNumber, buf, count,
+			USB_CTRL_SET_TIMEOUT);
+	/* count also the report id, if this was a numbered report. */
+	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)
 {
@@ -936,6 +968,36 @@ static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf, size_t co
 	return ret;
 }
 
+static int usbhid_output_report(struct hid_device *hid, __u8 *buf, size_t count)
+{
+	struct usbhid_device *usbhid = hid->driver_data;
+	struct usb_device *dev = hid_to_usb_dev(hid);
+	int actual_length, skipped_report_id = 0, ret;
+
+	if (!usbhid->urbout)
+		return -EIO;
+
+	if (buf[0] == 0x0) {
+		/* Don't send the Report ID */
+		buf++;
+		count--;
+		skipped_report_id = 1;
+	}
+
+	ret = usb_interrupt_msg(dev, usbhid->urbout->pipe,
+				buf, count, &actual_length,
+				USB_CTRL_SET_TIMEOUT);
+	/* return the number of bytes transferred */
+	if (ret == 0) {
+		ret = actual_length;
+		/* count also the report id */
+		if (skipped_report_id)
+			ret++;
+	}
+
+	return ret;
+}
+
 static void usbhid_restart_queues(struct usbhid_device *usbhid)
 {
 	if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
@@ -1200,6 +1262,20 @@ static void usbhid_request(struct hid_device *hid, struct hid_report *rep, int r
 	}
 }
 
+static int usbhid_raw_request(struct hid_device *hid, unsigned char reportnum,
+			      __u8 *buf, size_t len, unsigned char rtype,
+			      int reqtype)
+{
+	switch (reqtype) {
+	case HID_REQ_GET_REPORT:
+		return usbhid_get_raw_report(hid, reportnum, buf, len, rtype);
+	case HID_REQ_SET_REPORT:
+		return usbhid_set_raw_report(hid, reportnum, buf, len, rtype);
+	default:
+		return -EIO;
+	}
+}
+
 static int usbhid_idle(struct hid_device *hid, int report, int idle,
 		int reqtype)
 {
@@ -1223,6 +1299,8 @@ static struct hid_ll_driver usb_hid_driver = {
 	.power = usbhid_power,
 	.request = usbhid_request,
 	.wait = usbhid_wait_io,
+	.raw_request = usbhid_raw_request,
+	.output_report = usbhid_output_report,
 	.idle = usbhid_idle,
 };
 
-- 
1.8.4.2

[PATCH 3/4] HID: Add the transport-driver function to the uhid driver

[Frank Praznik]

Add the uhid_output_report transport-driver function to the uhid driver.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>

---
 drivers/hid/uhid.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index cedc6da..f5a2b19 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -244,12 +244,39 @@ static int uhid_hid_output_raw(struct hid_device *hid, __u8 *buf, size_t count,
 	return count;
 }
 
+static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
+				  size_t count)
+{
+	struct uhid_device *uhid = hid->driver_data;
+	unsigned long flags;
+	struct uhid_event *ev;
+
+	if (count < 1 || count > UHID_DATA_MAX)
+		return -EINVAL;
+
+	ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+	if (!ev)
+		return -ENOMEM;
+
+	ev->type = UHID_OUTPUT;
+	ev->u.output.size = count;
+	ev->u.output.rtype = UHID_OUTPUT_REPORT;
+	memcpy(ev->u.output.data, buf, count);
+
+	spin_lock_irqsave(&uhid->qlock, flags);
+	uhid_queue(uhid, ev);
+	spin_unlock_irqrestore(&uhid->qlock, flags);
+
+	return count;
+}
+
 static struct hid_ll_driver uhid_hid_driver = {
 	.start = uhid_hid_start,
 	.stop = uhid_hid_stop,
 	.open = uhid_hid_open,
 	.close = uhid_hid_close,
 	.parse = uhid_hid_parse,
+	.output_report = uhid_hid_output_report,
 };
 
 #ifdef CONFIG_COMPAT
-- 
1.8.4.2

[PATCH 4/4] HID: Add the transport-driver functions to the HIDP driver.

[Frank Praznik]

Add raw_request, set_raw_report and output_report transport-driver functions to
the HIDP driver.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>

---
 net/bluetooth/hidp/core.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 292e619..b062cee 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -353,6 +353,71 @@ err:
 	return ret;
 }
 
+static int hidp_set_raw_report(struct hid_device *hid, unsigned char reportnum,
+			       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;
+		break;
+	case HID_INPUT_REPORT:
+		report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_INPUT;
+		break;
+	case HID_OUTPUT_REPORT:
+		report_type = HIDP_TRANS_SET_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. */
+	data[0] = reportnum;
+	set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
+	ret = hidp_send_ctrl_message(session, report_type, data, count);
+	if (ret)
+		goto err;
+
+	/* Wait for the ACK from the device. */
+	while (test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags) &&
+	       !atomic_read(&session->terminate)) {
+		int res;
+
+		res = wait_event_interruptible_timeout(session->report_queue,
+			!test_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags)
+				|| atomic_read(&session->terminate),
+			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 int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
 		unsigned char report_type)
 {
@@ -411,6 +476,29 @@ err:
 	return ret;
 }
 
+static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
+			    __u8 *buf, size_t len, unsigned char rtype,
+			    int reqtype)
+{
+	switch (reqtype) {
+	case HID_REQ_GET_REPORT:
+		return hidp_get_raw_report(hid, reportnum, buf, len, rtype);
+	case HID_REQ_SET_REPORT:
+		return hidp_set_raw_report(hid, reportnum, buf, len, rtype);
+	default:
+		return -EIO;
+	}
+}
+
+static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
+{
+	struct hidp_session *session = hid->driver_data;
+
+	return hidp_send_intr_message(session,
+				      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+				      data, count);
+}
+
 static void hidp_idle_timeout(unsigned long arg)
 {
 	struct hidp_session *session = (struct hidp_session *) arg;
@@ -727,6 +815,8 @@ static struct hid_ll_driver hidp_hid_driver = {
 	.stop = hidp_stop,
 	.open  = hidp_open,
 	.close = hidp_close,
+	.raw_request = hidp_raw_request,
+	.output_report = hidp_output_report,
 	.hidinput_input_event = hidp_hidinput_event,
 };
 
-- 
1.8.4.2

Re: [PATCH 0/4] HID: implement new transport-driver callbacks

[David Herrmann]

Hi

On Wed, Jan 22, 2014 at 7:49 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> These patches are originally the work of David Herrmann who suggested that I
> update and submit them as their functionality is required for some pending
> patches to the hid-sony driver.
>
> These patches implement the SET/GET_REPORT and raw intr OUTPUT requests for all
> transport drivers.  It adds two callbacks to the hid_ll_driver struct:
>
> int (*raw_request)(struct hid_device *hdev, unsigned char reportnum,
>                    __u8 *buf, size_t len, unsigned char rtype, int reqtype);
>
> int (*output_report)(struct hid_device *hdev, __u8 *buf, size_t len);
>
> along with the necessary support fuctions in the USBHID, and HIDP drivers.
>
> UHID is not converted yet.

Thanks for picking it up. As background, people should read my HID
summary which originally was part of this series:
  http://cgit.freedesktop.org/~dvdhrm/linux/tree/Documentation/hid/hid-transport.txt?h=hid&id=86c08bb28302bb31dcd3b9aaf22b222f890397e0

Our current hid_output_raw_report() callbacks are implemented
differently in the USBHID, HIDP, I2CHID and UHID backends (I even
think they're all mutually different). We cannot easily change these
as drivers actually depend on the backends to do it differently.
Therefore, I proposed the raw_request() and output_report() functions.
raw_request() is basically the same as request() but takes a raw
buffer instead of an hid_report. output_report() is what HIDP
currently does with hid_output_raw_report() and sends the report as
asynchronous intr report.

The plan should be to use request(), raw_request() and output_report()
exclusively and carefully port drivers to use them. Once we're done,
we can remove hid_output_raw_report() (and any other legacy). This
should guarantee, that drivers can choose between ctrl-SET_REPORT and
intr-OUTPUT_REPORT messages without depending on the underlying
backend to choose the right one.

I hope that information helps.
Thanks
David

Re: [PATCH 0/4] HID: implement new transport-driver callbacks

[Jiri Kosina]

On Wed, 22 Jan 2014, David Herrmann wrote:

> > These patches are originally the work of David Herrmann who suggested that I
> > update and submit them as their functionality is required for some pending
> > patches to the hid-sony driver.
> >
> > These patches implement the SET/GET_REPORT and raw intr OUTPUT requests for all
> > transport drivers.  It adds two callbacks to the hid_ll_driver struct:
> >
> > int (*raw_request)(struct hid_device *hdev, unsigned char reportnum,
> >                    __u8 *buf, size_t len, unsigned char rtype, int reqtype);
> >
> > int (*output_report)(struct hid_device *hdev, __u8 *buf, size_t len);
> >
> > along with the necessary support fuctions in the USBHID, and HIDP drivers.
> >
> > UHID is not converted yet.
> 
> Thanks for picking it up. As background, people should read my HID
> summary which originally was part of this series:
>   http://cgit.freedesktop.org/~dvdhrm/linux/tree/Documentation/hid/hid-transport.txt?h=hid&id=86c08bb28302bb31dcd3b9aaf22b222f890397e0
> 
> Our current hid_output_raw_report() callbacks are implemented
> differently in the USBHID, HIDP, I2CHID and UHID backends (I even
> think they're all mutually different). We cannot easily change these
> as drivers actually depend on the backends to do it differently.
> Therefore, I proposed the raw_request() and output_report() functions.
> raw_request() is basically the same as request() but takes a raw
> buffer instead of an hid_report. output_report() is what HIDP
> currently does with hid_output_raw_report() and sends the report as
> asynchronous intr report.
> 
> The plan should be to use request(), raw_request() and output_report()
> exclusively and carefully port drivers to use them. Once we're done,
> we can remove hid_output_raw_report() (and any other legacy). This
> should guarantee, that drivers can choose between ctrl-SET_REPORT and
> intr-OUTPUT_REPORT messages without depending on the underlying
> backend to choose the right one.

I haven't finished reviewing the patchset yet, but anyway -- David, can I 
consider your e-mail as a potential Acked-by:?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH 0/4] HID: implement new transport-driver callbacks

[David Herrmann]

Hi

On Tue, Jan 28, 2014 at 9:53 PM, Jiri Kosina <jkosina@suse.cz> wrote:
> On Wed, 22 Jan 2014, David Herrmann wrote:
>
>> > These patches are originally the work of David Herrmann who suggested that I
>> > update and submit them as their functionality is required for some pending
>> > patches to the hid-sony driver.
>> >
>> > These patches implement the SET/GET_REPORT and raw intr OUTPUT requests for all
>> > transport drivers.  It adds two callbacks to the hid_ll_driver struct:
>> >
>> > int (*raw_request)(struct hid_device *hdev, unsigned char reportnum,
>> >                    __u8 *buf, size_t len, unsigned char rtype, int reqtype);
>> >
>> > int (*output_report)(struct hid_device *hdev, __u8 *buf, size_t len);
>> >
>> > along with the necessary support fuctions in the USBHID, and HIDP drivers.
>> >
>> > UHID is not converted yet.
>>
>> Thanks for picking it up. As background, people should read my HID
>> summary which originally was part of this series:
>>   http://cgit.freedesktop.org/~dvdhrm/linux/tree/Documentation/hid/hid-transport.txt?h=hid&id=86c08bb28302bb31dcd3b9aaf22b222f890397e0
>>
>> Our current hid_output_raw_report() callbacks are implemented
>> differently in the USBHID, HIDP, I2CHID and UHID backends (I even
>> think they're all mutually different). We cannot easily change these
>> as drivers actually depend on the backends to do it differently.
>> Therefore, I proposed the raw_request() and output_report() functions.
>> raw_request() is basically the same as request() but takes a raw
>> buffer instead of an hid_report. output_report() is what HIDP
>> currently does with hid_output_raw_report() and sends the report as
>> asynchronous intr report.
>>
>> The plan should be to use request(), raw_request() and output_report()
>> exclusively and carefully port drivers to use them. Once we're done,
>> we can remove hid_output_raw_report() (and any other legacy). This
>> should guarantee, that drivers can choose between ctrl-SET_REPORT and
>> intr-OUTPUT_REPORT messages without depending on the underlying
>> backend to choose the right one.
>
> I haven't finished reviewing the patchset yet, but anyway -- David, can I
> consider your e-mail as a potential Acked-by:?

Yes, definitely. It's basically my 1-year old patch split into 3.

Thanks
David

Re: [PATCH 0/4] HID: implement new transport-driver callbacks

[Jiri Kosina]

On Wed, 22 Jan 2014, Frank Praznik wrote:

> These patches are originally the work of David Herrmann who suggested that I 
> update and submit them as their functionality is required for some pending
> patches to the hid-sony driver.
> 
> These patches implement the SET/GET_REPORT and raw intr OUTPUT requests for all
> transport drivers.  It adds two callbacks to the hid_ll_driver struct:
> 
> int (*raw_request)(struct hid_device *hdev, unsigned char reportnum,
> 		   __u8 *buf, size_t len, unsigned char rtype, int reqtype);
> 
> int (*output_report)(struct hid_device *hdev, __u8 *buf, size_t len);
> 
> along with the necessary support fuctions in the USBHID, and HIDP drivers.
> 
> UHID is not converted yet.

I have finished reviewing the patchset and don't have any objections. I am 
queuing them for 3.15.

-- 
Jiri Kosina
SUSE Labs