[PATCH 00/11] HID: spring cleaning

STALE4562d

31 messages, 3 authors, 2014-02-03 · open the first message on its own page

[PATCH 00/11] HID: spring cleaning

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:26 

Hi guys,

This is an attempt to complete the branch for-3.15/ll-driver-new-callbacks:
- try to implement as much as possible ll_driver callbacks (some are still
  missing, but I did not had the time to complete it)
- add inliners hid_hw_* for all the ll_driver callbacks
- remove transport dependant callbacks in struct hid_device
- remove the so called "obsolete" hidinput_input_event handler which was used
  in 2/4 transport drivers

Bonus point: 14 files changed, 213 insertions(+), 272 deletions(-)
Yay!

I made sure I kept the PS3 controller working and the LEDs (on logitech-dj and
on bluetooth keyboard) working. The rest do not mostly need further testing,
the code path did not changed. But still, a review (and some tests) would be a
good idea :)

Cheers,
Benjamin

PS: obviously, this goes on top on the branch for-3.15/ll-driver-new-callbacks

Benjamin Tissoires (11):
  HID: uHID: implement .raw_request
  HID: i2c-hid: implement ll_driver transport-layer callbacks
  HID: add inliners for ll_driver transport-layer callbacks
  HID: logitech-dj: remove hidinput_input_event
  HID: HIDp: remove hidp_hidinput_event
  HID: remove hidinput_input_event handler
  HID: HIDp: remove duplicated coded
  HID: usbhid: remove duplicated code
  HID: remove hid_get_raw_report in struct hid_device
  HID: introduce helper to access hid_output_raw_report()
  HID: move hid_output_raw_report to hid_ll_driver

 drivers/hid/hid-input.c        |  12 ++---
 drivers/hid/hid-lg.c           |   6 ++-
 drivers/hid/hid-logitech-dj.c  | 101 +++++++++++++---------------------
 drivers/hid/hid-magicmouse.c   |   2 +-
 drivers/hid/hid-sony.c         |  19 +++++--
 drivers/hid/hid-thingm.c       |   4 +-
 drivers/hid/hid-wacom.c        |  16 +++---
 drivers/hid/hid-wiimote-core.c |   4 +-
 drivers/hid/hidraw.c           |  11 ++--
 drivers/hid/i2c-hid/i2c-hid.c  |  27 +++++++++-
 drivers/hid/uhid.c             |  20 ++++++-
 drivers/hid/usbhid/hid-core.c  |  67 +++++------------------
 include/linux/hid.h            |  77 ++++++++++++++++++++++----
 net/bluetooth/hidp/core.c      | 119 +++++------------------------------------
 14 files changed, 213 insertions(+), 272 deletions(-)

-- 
1.8.3.1

[PATCH 01/11] HID: uHID: implement .raw_request

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:50:43 

It was missing, so adding it.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/uhid.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index f5a2b19..438c9f1 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -270,6 +270,22 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
 	return count;
 }
 
+static int uhid_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 uhid_hid_get_raw(hid, reportnum, buf, len, rtype);
+	case HID_REQ_SET_REPORT:
+		if (buf[0] != reportnum)
+			return -EINVAL;
+		return uhid_hid_output_raw(hid, buf, len, rtype);
+	default:
+		return -EIO;
+	}
+}
+
 static struct hid_ll_driver uhid_hid_driver = {
 	.start = uhid_hid_start,
 	.stop = uhid_hid_stop,
@@ -277,6 +293,7 @@ static struct hid_ll_driver uhid_hid_driver = {
 	.close = uhid_hid_close,
 	.parse = uhid_hid_parse,
 	.output_report = uhid_hid_output_report,
+	.raw_request = uhid_raw_request,
 };
 
 #ifdef CONFIG_COMPAT
-- 
1.8.3.1

Re: [PATCH 01/11] HID: uHID: implement .raw_request

From: David Herrmann <hidden>
Date: 2014-02-03 15:26:23 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted hunk
It was missing, so adding it.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/uhid.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index f5a2b19..438c9f1 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -270,6 +270,22 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
        return count;
 }

+static int uhid_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 uhid_hid_get_raw(hid, reportnum, buf, len, rtype);
+       case HID_REQ_SET_REPORT:
+               if (buf[0] != reportnum)
+                       return -EINVAL;
+               return uhid_hid_output_raw(hid, buf, len, rtype);
But that one looks wrong. UHID does not have any SET_REPORT query in
the protocol, yet. You turn a SET_REPORT into an OUTPUT_REPORT here.
So if user-space gets the UHID_OUTPUT event, it doesn't know whether
to send a SET_REPORT@ctrl to the device, or an async
OUTPUT_REPORT@intr. This at least matters for low-energy BT in bluez,
which uses uhid.

So we'd have to add an UHID_SET_REPORT event. Note that currently
UHID_FEATURE is the equivalent of UHID_GET_REPORT, but just horribly
named..

Thanks
David
quoted hunk
+       default:
+               return -EIO;
+       }
+}
+
 static struct hid_ll_driver uhid_hid_driver = {
        .start = uhid_hid_start,
        .stop = uhid_hid_stop,
@@ -277,6 +293,7 @@ static struct hid_ll_driver uhid_hid_driver = {
        .close = uhid_hid_close,
        .parse = uhid_hid_parse,
        .output_report = uhid_hid_output_report,
+       .raw_request = uhid_raw_request,
 };

 #ifdef CONFIG_COMPAT
--
1.8.3.1

Re: [PATCH 01/11] HID: uHID: implement .raw_request

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 19:08:00 

On Mon, Feb 3, 2014 at 10:26 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
It was missing, so adding it.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/uhid.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index f5a2b19..438c9f1 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -270,6 +270,22 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
        return count;
 }

+static int uhid_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 uhid_hid_get_raw(hid, reportnum, buf, len, rtype);
+       case HID_REQ_SET_REPORT:
+               if (buf[0] != reportnum)
+                       return -EINVAL;
+               return uhid_hid_output_raw(hid, buf, len, rtype);
But that one looks wrong. UHID does not have any SET_REPORT query in
the protocol, yet. You turn a SET_REPORT into an OUTPUT_REPORT here.
So if user-space gets the UHID_OUTPUT event, it doesn't know whether
to send a SET_REPORT@ctrl to the device, or an async
OUTPUT_REPORT@intr. This at least matters for low-energy BT in bluez,
which uses uhid.
right. So we can drop this for now.

So we'd have to add an UHID_SET_REPORT event. Note that currently
UHID_FEATURE is the equivalent of UHID_GET_REPORT, but just horribly
named..
ouch. I think this is something which can be fixed quite easily (by
marking UHID_FEATURE obsolete and creating the two events).
However, I don't think I will have the time to make the change and do
proper testings in the next few days/weeks.

Can somebody else take this?

Cheers,
Benjamin

Thanks
David
quoted
+       default:
+               return -EIO;
+       }
+}
+
 static struct hid_ll_driver uhid_hid_driver = {
        .start = uhid_hid_start,
        .stop = uhid_hid_stop,
@@ -277,6 +293,7 @@ static struct hid_ll_driver uhid_hid_driver = {
        .close = uhid_hid_close,
        .parse = uhid_hid_parse,
        .output_report = uhid_hid_output_report,
+       .raw_request = uhid_raw_request,
 };

 #ifdef CONFIG_COMPAT
--
1.8.3.1

[PATCH 04/11] HID: logitech-dj: remove hidinput_input_event

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:28 

hid-logitech-dj uses its own ->hidinput_input_event() instead of
the generic binding in hid-input.
Moving the handling of LEDs towards logi_dj_output_hidraw_report()
allows two things:
- remove hidinput_input_event in struct hid_device
- hidraw user space programs can also set the LEDs

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 64 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f45279c..61d2bbf 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
 	0x19, 0xE0,		/*   USAGE_MINIMUM (Left Control)   */
 	0x29, 0xE7,		/*   USAGE_MAXIMUM (Right GUI)      */
 	0x81, 0x02,		/*   INPUT (Data,Var,Abs)       */
-	0x95, 0x05,		/*   REPORT COUNT (5)           */
-	0x05, 0x08,		/*   USAGE PAGE (LED page)      */
-	0x19, 0x01,		/*   USAGE MINIMUM (1)          */
-	0x29, 0x05,		/*   USAGE MAXIMUM (5)          */
-	0x91, 0x02,		/*   OUTPUT (Data, Variable, Absolute)  */
-	0x95, 0x01,		/*   REPORT COUNT (1)           */
-	0x75, 0x03,		/*   REPORT SIZE (3)            */
-	0x91, 0x01,		/*   OUTPUT (Constant)          */
 	0x95, 0x06,		/*   REPORT_COUNT (6)           */
 	0x75, 0x08,		/*   REPORT_SIZE (8)            */
 	0x15, 0x00,		/*   LOGICAL_MINIMUM (0)        */
@@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
 	0x19, 0x00,		/*   USAGE_MINIMUM (no event)       */
 	0x2A, 0xFF, 0x00,	/*   USAGE_MAXIMUM (reserved)       */
 	0x81, 0x00,		/*   INPUT (Data,Ary,Abs)       */
+	0x85, 0x0e,		/* REPORT_ID (14)               */
+	0x05, 0x08,		/*   USAGE PAGE (LED page)      */
+	0x95, 0x05,		/*   REPORT COUNT (5)           */
+	0x75, 0x01,		/*   REPORT SIZE (1)            */
+	0x15, 0x00,		/*   LOGICAL_MINIMUM (0)        */
+	0x25, 0x01,		/*   LOGICAL_MAXIMUM (1)        */
+	0x19, 0x01,		/*   USAGE MINIMUM (1)          */
+	0x29, 0x05,		/*   USAGE MAXIMUM (5)          */
+	0x91, 0x02,		/*   OUTPUT (Data, Variable, Absolute)  */
+	0x95, 0x01,		/*   REPORT COUNT (1)           */
+	0x75, 0x03,		/*   REPORT SIZE (3)            */
+	0x91, 0x01,		/*   OUTPUT (Constant)          */
 	0xC0
 };
 
@@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
 					size_t count,
 					unsigned char report_type)
 {
-	/* Called by hid raw to send data */
-	dbg_hid("%s\n", __func__);
+	struct dj_device *djdev = hid->driver_data;
+	struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
+	u8 *out_buf;
+	int ret;
 
-	return 0;
+	if (buf[0] != REPORT_TYPE_LEDS)
+		return -EINVAL;
+
+	out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
+	if (!out_buf)
+		return -ENOMEM;
+
+	if (count < DJREPORT_SHORT_LENGTH - 2)
+		count = DJREPORT_SHORT_LENGTH - 2;
+
+	out_buf[0] = REPORT_ID_DJ_SHORT;
+	out_buf[1] = djdev->device_index;
+	memcpy(out_buf + 2, buf, count);
+
+	ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+		DJREPORT_SHORT_LENGTH, report_type);
+
+	kfree(out_buf);
+	return ret;
 }
 
 static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
@@ -613,58 +637,6 @@ static int logi_dj_ll_parse(struct hid_device *hid)
 	return retval;
 }
 
-static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
-				  unsigned int code, int value)
-{
-	/* Sent by the input layer to handle leds and Force Feedback */
-	struct hid_device *dj_hiddev = input_get_drvdata(dev);
-	struct dj_device *dj_dev = dj_hiddev->driver_data;
-
-	struct dj_receiver_dev *djrcv_dev =
-	    dev_get_drvdata(dj_hiddev->dev.parent);
-	struct hid_device *dj_rcv_hiddev = djrcv_dev->hdev;
-	struct hid_report_enum *output_report_enum;
-
-	struct hid_field *field;
-	struct hid_report *report;
-	unsigned char *data;
-	int offset;
-
-	dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
-		__func__, dev->phys, type, code, value);
-
-	if (type != EV_LED)
-		return -1;
-
-	offset = hidinput_find_field(dj_hiddev, type, code, &field);
-
-	if (offset == -1) {
-		dev_warn(&dev->dev, "event field not found\n");
-		return -1;
-	}
-	hid_set_field(field, offset, value);
-
-	data = hid_alloc_report_buf(field->report, GFP_ATOMIC);
-	if (!data) {
-		dev_warn(&dev->dev, "failed to allocate report buf memory\n");
-		return -1;
-	}
-
-	hid_output_report(field->report, &data[0]);
-
-	output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
-	report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
-	hid_set_field(report->field[0], 0, dj_dev->device_index);
-	hid_set_field(report->field[0], 1, REPORT_TYPE_LEDS);
-	hid_set_field(report->field[0], 2, data[1]);
-
-	hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
-
-	kfree(data);
-
-	return 0;
-}
-
 static int logi_dj_ll_start(struct hid_device *hid)
 {
 	dbg_hid("%s\n", __func__);
@@ -683,7 +655,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
 	.stop = logi_dj_ll_stop,
 	.open = logi_dj_ll_open,
 	.close = logi_dj_ll_close,
-	.hidinput_input_event = logi_dj_ll_input_event,
 };
 
 
-- 
1.8.3.1

Re: [PATCH 04/11] HID: logitech-dj: remove hidinput_input_event

From: David Herrmann <hidden>
Date: 2014-02-03 16:06:56 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted hunk
hid-logitech-dj uses its own ->hidinput_input_event() instead of
the generic binding in hid-input.
Moving the handling of LEDs towards logi_dj_output_hidraw_report()
allows two things:
- remove hidinput_input_event in struct hid_device
- hidraw user space programs can also set the LEDs

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 64 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f45279c..61d2bbf 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
        0x19, 0xE0,             /*   USAGE_MINIMUM (Left Control)   */
        0x29, 0xE7,             /*   USAGE_MAXIMUM (Right GUI)      */
        0x81, 0x02,             /*   INPUT (Data,Var,Abs)       */
-       0x95, 0x05,             /*   REPORT COUNT (5)           */
-       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
-       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
-       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
-       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
-       0x95, 0x01,             /*   REPORT COUNT (1)           */
-       0x75, 0x03,             /*   REPORT SIZE (3)            */
-       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0x95, 0x06,             /*   REPORT_COUNT (6)           */
        0x75, 0x08,             /*   REPORT_SIZE (8)            */
        0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
@@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
        0x19, 0x00,             /*   USAGE_MINIMUM (no event)       */
        0x2A, 0xFF, 0x00,       /*   USAGE_MAXIMUM (reserved)       */
        0x81, 0x00,             /*   INPUT (Data,Ary,Abs)       */
+       0x85, 0x0e,             /* REPORT_ID (14)               */
+       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
+       0x95, 0x05,             /*   REPORT COUNT (5)           */
+       0x75, 0x01,             /*   REPORT SIZE (1)            */
+       0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
+       0x25, 0x01,             /*   LOGICAL_MAXIMUM (1)        */
+       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
+       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
+       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
+       0x95, 0x01,             /*   REPORT COUNT (1)           */
+       0x75, 0x03,             /*   REPORT SIZE (3)            */
+       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0xC0
 };

@@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
                                        size_t count,
                                        unsigned char report_type)
 {
-       /* Called by hid raw to send data */
-       dbg_hid("%s\n", __func__);
+       struct dj_device *djdev = hid->driver_data;
+       struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
+       u8 *out_buf;
+       int ret;

-       return 0;
+       if (buf[0] != REPORT_TYPE_LEDS)
+               return -EINVAL;
+
+       out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
+       if (!out_buf)
+               return -ENOMEM;
+
+       if (count < DJREPORT_SHORT_LENGTH - 2)
+               count = DJREPORT_SHORT_LENGTH - 2;
+
+       out_buf[0] = REPORT_ID_DJ_SHORT;
+       out_buf[1] = djdev->device_index;
+       memcpy(out_buf + 2, buf, count);
+
+       ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+               DJREPORT_SHORT_LENGTH, report_type);
Strictly speaking the code below uses HID_REQ_SET_REPORT and you
replace it with hid_output_raw_report() here (which at least for BTHID
is not equivalent). Anyhow, HID-core uses hid_output_raw_report(),
too, so this should be fine.

Thanks
David
quoted hunk
+
+       kfree(out_buf);
+       return ret;
 }

 static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
@@ -613,58 +637,6 @@ static int logi_dj_ll_parse(struct hid_device *hid)
        return retval;
 }

-static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
-                                 unsigned int code, int value)
-{
-       /* Sent by the input layer to handle leds and Force Feedback */
-       struct hid_device *dj_hiddev = input_get_drvdata(dev);
-       struct dj_device *dj_dev = dj_hiddev->driver_data;
-
-       struct dj_receiver_dev *djrcv_dev =
-           dev_get_drvdata(dj_hiddev->dev.parent);
-       struct hid_device *dj_rcv_hiddev = djrcv_dev->hdev;
-       struct hid_report_enum *output_report_enum;
-
-       struct hid_field *field;
-       struct hid_report *report;
-       unsigned char *data;
-       int offset;
-
-       dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
-               __func__, dev->phys, type, code, value);
-
-       if (type != EV_LED)
-               return -1;
-
-       offset = hidinput_find_field(dj_hiddev, type, code, &field);
-
-       if (offset == -1) {
-               dev_warn(&dev->dev, "event field not found\n");
-               return -1;
-       }
-       hid_set_field(field, offset, value);
-
-       data = hid_alloc_report_buf(field->report, GFP_ATOMIC);
-       if (!data) {
-               dev_warn(&dev->dev, "failed to allocate report buf memory\n");
-               return -1;
-       }
-
-       hid_output_report(field->report, &data[0]);
-
-       output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
-       report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
-       hid_set_field(report->field[0], 0, dj_dev->device_index);
-       hid_set_field(report->field[0], 1, REPORT_TYPE_LEDS);
-       hid_set_field(report->field[0], 2, data[1]);
-
-       hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
-
-       kfree(data);
-
-       return 0;
-}
-
 static int logi_dj_ll_start(struct hid_device *hid)
 {
        dbg_hid("%s\n", __func__);
@@ -683,7 +655,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
        .stop = logi_dj_ll_stop,
        .open = logi_dj_ll_open,
        .close = logi_dj_ll_close,
-       .hidinput_input_event = logi_dj_ll_input_event,
 };


--
1.8.3.1

Re: [PATCH 04/11] HID: logitech-dj: remove hidinput_input_event

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 16:21:29 

On Mon, Feb 3, 2014 at 11:06 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
hid-logitech-dj uses its own ->hidinput_input_event() instead of
the generic binding in hid-input.
Moving the handling of LEDs towards logi_dj_output_hidraw_report()
allows two things:
- remove hidinput_input_event in struct hid_device
- hidraw user space programs can also set the LEDs

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 64 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f45279c..61d2bbf 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
        0x19, 0xE0,             /*   USAGE_MINIMUM (Left Control)   */
        0x29, 0xE7,             /*   USAGE_MAXIMUM (Right GUI)      */
        0x81, 0x02,             /*   INPUT (Data,Var,Abs)       */
-       0x95, 0x05,             /*   REPORT COUNT (5)           */
-       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
-       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
-       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
-       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
-       0x95, 0x01,             /*   REPORT COUNT (1)           */
-       0x75, 0x03,             /*   REPORT SIZE (3)            */
-       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0x95, 0x06,             /*   REPORT_COUNT (6)           */
        0x75, 0x08,             /*   REPORT_SIZE (8)            */
        0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
@@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
        0x19, 0x00,             /*   USAGE_MINIMUM (no event)       */
        0x2A, 0xFF, 0x00,       /*   USAGE_MAXIMUM (reserved)       */
        0x81, 0x00,             /*   INPUT (Data,Ary,Abs)       */
+       0x85, 0x0e,             /* REPORT_ID (14)               */
+       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
+       0x95, 0x05,             /*   REPORT COUNT (5)           */
+       0x75, 0x01,             /*   REPORT SIZE (1)            */
+       0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
+       0x25, 0x01,             /*   LOGICAL_MAXIMUM (1)        */
+       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
+       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
+       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
+       0x95, 0x01,             /*   REPORT COUNT (1)           */
+       0x75, 0x03,             /*   REPORT SIZE (3)            */
+       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0xC0
 };

@@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
                                        size_t count,
                                        unsigned char report_type)
 {
-       /* Called by hid raw to send data */
-       dbg_hid("%s\n", __func__);
+       struct dj_device *djdev = hid->driver_data;
+       struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
+       u8 *out_buf;
+       int ret;

-       return 0;
+       if (buf[0] != REPORT_TYPE_LEDS)
+               return -EINVAL;
+
+       out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
+       if (!out_buf)
+               return -ENOMEM;
+
+       if (count < DJREPORT_SHORT_LENGTH - 2)
+               count = DJREPORT_SHORT_LENGTH - 2;
+
+       out_buf[0] = REPORT_ID_DJ_SHORT;
+       out_buf[1] = djdev->device_index;
+       memcpy(out_buf + 2, buf, count);
+
+       ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+               DJREPORT_SHORT_LENGTH, report_type);
Strictly speaking the code below uses HID_REQ_SET_REPORT and you
replace it with hid_output_raw_report() here (which at least for BTHID
is not equivalent). Anyhow, HID-core uses hid_output_raw_report(),
too, so this should be fine.
Yes, you are right. This fires back on me yesterday when I tried to
remove the hid_output_raw_report() calls.
It occurs it works because usbhid_out_raw_report() uses set_report
when there is no urbout, which is the case with logitech receivers.
So I guess an ideal solution would be to implement a hid_hw_request call.

However, the only concern I have with hid_hw_request is that it does
not allow hidraw to play with the LEDs given that hidraw does not have
an ioctl for SET_REPORT (or GET_REPORT). I already seen this issue
regarding i2c_hid at some point, but it was "solved" in i2c_hid by
using the same ugly trick that we have in USB-hid.

Cheers,
Benjamin

Thanks
David
quoted
+
+       kfree(out_buf);
+       return ret;
 }

 static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
@@ -613,58 +637,6 @@ static int logi_dj_ll_parse(struct hid_device *hid)
        return retval;
 }

-static int logi_dj_ll_input_event(struct input_dev *dev, unsigned int type,
-                                 unsigned int code, int value)
-{
-       /* Sent by the input layer to handle leds and Force Feedback */
-       struct hid_device *dj_hiddev = input_get_drvdata(dev);
-       struct dj_device *dj_dev = dj_hiddev->driver_data;
-
-       struct dj_receiver_dev *djrcv_dev =
-           dev_get_drvdata(dj_hiddev->dev.parent);
-       struct hid_device *dj_rcv_hiddev = djrcv_dev->hdev;
-       struct hid_report_enum *output_report_enum;
-
-       struct hid_field *field;
-       struct hid_report *report;
-       unsigned char *data;
-       int offset;
-
-       dbg_hid("%s: %s, type:%d | code:%d | value:%d\n",
-               __func__, dev->phys, type, code, value);
-
-       if (type != EV_LED)
-               return -1;
-
-       offset = hidinput_find_field(dj_hiddev, type, code, &field);
-
-       if (offset == -1) {
-               dev_warn(&dev->dev, "event field not found\n");
-               return -1;
-       }
-       hid_set_field(field, offset, value);
-
-       data = hid_alloc_report_buf(field->report, GFP_ATOMIC);
-       if (!data) {
-               dev_warn(&dev->dev, "failed to allocate report buf memory\n");
-               return -1;
-       }
-
-       hid_output_report(field->report, &data[0]);
-
-       output_report_enum = &dj_rcv_hiddev->report_enum[HID_OUTPUT_REPORT];
-       report = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
-       hid_set_field(report->field[0], 0, dj_dev->device_index);
-       hid_set_field(report->field[0], 1, REPORT_TYPE_LEDS);
-       hid_set_field(report->field[0], 2, data[1]);
-
-       hid_hw_request(dj_rcv_hiddev, report, HID_REQ_SET_REPORT);
-
-       kfree(data);
-
-       return 0;
-}
-
 static int logi_dj_ll_start(struct hid_device *hid)
 {
        dbg_hid("%s\n", __func__);
@@ -683,7 +655,6 @@ static struct hid_ll_driver logi_dj_ll_driver = {
        .stop = logi_dj_ll_stop,
        .open = logi_dj_ll_open,
        .close = logi_dj_ll_close,
-       .hidinput_input_event = logi_dj_ll_input_event,
 };


--
1.8.3.1

Re: [PATCH 04/11] HID: logitech-dj: remove hidinput_input_event

From: David Herrmann <hidden>
Date: 2014-02-03 17:07:36 

Hi

On Mon, Feb 3, 2014 at 5:21 PM, Benjamin Tissoires
[off-list ref] wrote:
On Mon, Feb 3, 2014 at 11:06 AM, David Herrmann [off-list ref] wrote:
quoted
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
hid-logitech-dj uses its own ->hidinput_input_event() instead of
the generic binding in hid-input.
Moving the handling of LEDs towards logi_dj_output_hidraw_report()
allows two things:
- remove hidinput_input_event in struct hid_device
- hidraw user space programs can also set the LEDs

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-logitech-dj.c | 99 +++++++++++++++----------------------------
 1 file changed, 35 insertions(+), 64 deletions(-)

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f45279c..61d2bbf 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -44,14 +44,6 @@ static const char kbd_descriptor[] = {
        0x19, 0xE0,             /*   USAGE_MINIMUM (Left Control)   */
        0x29, 0xE7,             /*   USAGE_MAXIMUM (Right GUI)      */
        0x81, 0x02,             /*   INPUT (Data,Var,Abs)       */
-       0x95, 0x05,             /*   REPORT COUNT (5)           */
-       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
-       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
-       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
-       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
-       0x95, 0x01,             /*   REPORT COUNT (1)           */
-       0x75, 0x03,             /*   REPORT SIZE (3)            */
-       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0x95, 0x06,             /*   REPORT_COUNT (6)           */
        0x75, 0x08,             /*   REPORT_SIZE (8)            */
        0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
@@ -60,6 +52,18 @@ static const char kbd_descriptor[] = {
        0x19, 0x00,             /*   USAGE_MINIMUM (no event)       */
        0x2A, 0xFF, 0x00,       /*   USAGE_MAXIMUM (reserved)       */
        0x81, 0x00,             /*   INPUT (Data,Ary,Abs)       */
+       0x85, 0x0e,             /* REPORT_ID (14)               */
+       0x05, 0x08,             /*   USAGE PAGE (LED page)      */
+       0x95, 0x05,             /*   REPORT COUNT (5)           */
+       0x75, 0x01,             /*   REPORT SIZE (1)            */
+       0x15, 0x00,             /*   LOGICAL_MINIMUM (0)        */
+       0x25, 0x01,             /*   LOGICAL_MAXIMUM (1)        */
+       0x19, 0x01,             /*   USAGE MINIMUM (1)          */
+       0x29, 0x05,             /*   USAGE MAXIMUM (5)          */
+       0x91, 0x02,             /*   OUTPUT (Data, Variable, Absolute)  */
+       0x95, 0x01,             /*   REPORT COUNT (1)           */
+       0x75, 0x03,             /*   REPORT SIZE (3)            */
+       0x91, 0x01,             /*   OUTPUT (Constant)          */
        0xC0
 };

@@ -544,10 +548,30 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
                                        size_t count,
                                        unsigned char report_type)
 {
-       /* Called by hid raw to send data */
-       dbg_hid("%s\n", __func__);
+       struct dj_device *djdev = hid->driver_data;
+       struct dj_receiver_dev *djrcv_dev = djdev->dj_receiver_dev;
+       u8 *out_buf;
+       int ret;

-       return 0;
+       if (buf[0] != REPORT_TYPE_LEDS)
+               return -EINVAL;
+
+       out_buf = kzalloc(DJREPORT_SHORT_LENGTH, GFP_ATOMIC);
+       if (!out_buf)
+               return -ENOMEM;
+
+       if (count < DJREPORT_SHORT_LENGTH - 2)
+               count = DJREPORT_SHORT_LENGTH - 2;
+
+       out_buf[0] = REPORT_ID_DJ_SHORT;
+       out_buf[1] = djdev->device_index;
+       memcpy(out_buf + 2, buf, count);
+
+       ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+               DJREPORT_SHORT_LENGTH, report_type);
Strictly speaking the code below uses HID_REQ_SET_REPORT and you
replace it with hid_output_raw_report() here (which at least for BTHID
is not equivalent). Anyhow, HID-core uses hid_output_raw_report(),
too, so this should be fine.
Yes, you are right. This fires back on me yesterday when I tried to
remove the hid_output_raw_report() calls.
It occurs it works because usbhid_out_raw_report() uses set_report
when there is no urbout, which is the case with logitech receivers.
So I guess an ideal solution would be to implement a hid_hw_request call.

However, the only concern I have with hid_hw_request is that it does
not allow hidraw to play with the LEDs given that hidraw does not have
an ioctl for SET_REPORT (or GET_REPORT). I already seen this issue
regarding i2c_hid at some point, but it was "solved" in i2c_hid by
using the same ugly trick that we have in USB-hid.
Hah! That's why it worked all the years, USB-HID checks for !urbout..
and yeah, that's what I just criticized in your i2c-hid patch.. Ok,
I'm fine if we make output_report() fall back to SET_REPORT if not
output-channel is supported. But we it gets ugly if we rely on that
from the upper stack... hmm, don't know how to make that work but
adding a hid quirk to use SET_REPORT for hidinput_input_report().

Thanks
David

[PATCH 03/11] HID: add inliners for ll_driver transport-layer callbacks

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:31 

Those callbacks are not mandatory, so it's better to add inliners
to use them safely.

Signed-off-by: Benjamin Tissoires <redacted>
---
 include/linux/hid.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 003cc8e..dddcad0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -680,6 +680,8 @@ struct hid_driver {
  *	   shouldn't allocate anything to not leak memory
  * @request: send report request to device (e.g. feature report)
  * @wait: wait for buffered io to complete (send/recv reports)
+ * @raw_request: send raw report request to device (e.g. feature report)
+ * @output_report: send output report to device
  * @idle: send idle request to device
  */
 struct hid_ll_driver {
@@ -974,6 +976,49 @@ static inline void hid_hw_request(struct hid_device *hdev,
 }
 
 /**
+ * hid_hw_raw_request - send report request to device
+ *
+ * @hdev: hid device
+ * @reportnum: report ID
+ * @buf: in/out data to transfer
+ * @len: length of buf
+ * @rtype: HID report type
+ * @reqtype: HID_REQ_GET_REPORT or HID_REQ_SET_REPORT
+ *
+ * @return: count of data transfered, negative if error
+ *
+ * Same behavior as hid_hw_request, but with raw buffers instead.
+ */
+static inline int hid_hw_raw_request(struct hid_device *hdev,
+				  unsigned char reportnum, __u8 *buf,
+				  size_t len, unsigned char rtype, int reqtype)
+{
+	if (hdev->ll_driver->raw_request)
+		return hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
+						    rtype, reqtype);
+
+	return -ENOSYS;
+}
+
+/**
+ * hid_hw_output_report - send output report to device
+ *
+ * @hdev: hid device
+ * @buf: raw data to transfer
+ * @len: length of buf
+ *
+ * @return: count of data transfered, negative if error
+ */
+static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
+					size_t len)
+{
+	if (hdev->ll_driver->output_report)
+		return hdev->ll_driver->output_report(hdev, buf, len);
+
+	return -ENOSYS;
+}
+
+/**
  * hid_hw_idle - send idle request to device
  *
  * @hdev: hid device
-- 
1.8.3.1

Re: [PATCH 03/11] HID: add inliners for ll_driver transport-layer callbacks

From: David Herrmann <hidden>
Date: 2014-02-03 15:29:26 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
Those callbacks are not mandatory, so it's better to add inliners
to use them safely.
Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 include/linux/hid.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 003cc8e..dddcad0 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -680,6 +680,8 @@ struct hid_driver {
  *        shouldn't allocate anything to not leak memory
  * @request: send report request to device (e.g. feature report)
  * @wait: wait for buffered io to complete (send/recv reports)
+ * @raw_request: send raw report request to device (e.g. feature report)
+ * @output_report: send output report to device
  * @idle: send idle request to device
  */
 struct hid_ll_driver {
@@ -974,6 +976,49 @@ static inline void hid_hw_request(struct hid_device *hdev,
 }

 /**
+ * hid_hw_raw_request - send report request to device
+ *
+ * @hdev: hid device
+ * @reportnum: report ID
+ * @buf: in/out data to transfer
+ * @len: length of buf
+ * @rtype: HID report type
+ * @reqtype: HID_REQ_GET_REPORT or HID_REQ_SET_REPORT
+ *
+ * @return: count of data transfered, negative if error
+ *
+ * Same behavior as hid_hw_request, but with raw buffers instead.
+ */
+static inline int hid_hw_raw_request(struct hid_device *hdev,
+                                 unsigned char reportnum, __u8 *buf,
+                                 size_t len, unsigned char rtype, int reqtype)
+{
+       if (hdev->ll_driver->raw_request)
+               return hdev->ll_driver->raw_request(hdev, reportnum, buf, len,
+                                                   rtype, reqtype);
+
+       return -ENOSYS;
+}
+
+/**
+ * hid_hw_output_report - send output report to device
+ *
+ * @hdev: hid device
+ * @buf: raw data to transfer
+ * @len: length of buf
+ *
+ * @return: count of data transfered, negative if error
+ */
+static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
+                                       size_t len)
+{
+       if (hdev->ll_driver->output_report)
+               return hdev->ll_driver->output_report(hdev, buf, len);
+
+       return -ENOSYS;
+}
+
+/**
  * hid_hw_idle - send idle request to device
  *
  * @hdev: hid device
--
1.8.3.1

[PATCH 07/11] HID: HIDp: remove duplicated coded

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:33 

- Move hidp_output_report() above
- Removed duplicated code in hidp_output_raw_report()

Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 469e61b..02670b3 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -373,62 +373,25 @@ err:
 	return ret;
 }
 
-static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
-		unsigned char report_type)
+static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
 {
 	struct hidp_session *session = hid->driver_data;
-	int ret;
 
+	return hidp_send_intr_message(session,
+				      HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+				      data, count);
+}
+
+static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
+		size_t count, unsigned char report_type)
+{
 	if (report_type == HID_OUTPUT_REPORT) {
-		report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-		return hidp_send_intr_message(session, report_type,
-					      data, count);
+		return hidp_output_report(hid, data, count);
 	} else if (report_type != HID_FEATURE_REPORT) {
 		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);
-	report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
-	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;
+	return hidp_set_raw_report(hid, data[0], data, count, report_type);
 }
 
 static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
@@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
 	}
 }
 
-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;
-- 
1.8.3.1

Re: [PATCH 07/11] HID: HIDp: remove duplicated coded

From: David Herrmann <hidden>
Date: 2014-02-03 15:02:15 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted hunk
- Move hidp_output_report() above
- Removed duplicated code in hidp_output_raw_report()

Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 469e61b..02670b3 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -373,62 +373,25 @@ err:
        return ret;
 }

-static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
-               unsigned char report_type)
+static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
 {
        struct hidp_session *session = hid->driver_data;
-       int ret;

+       return hidp_send_intr_message(session,
+                                     HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+                                     data, count);
+}
+
+static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
+               size_t count, unsigned char report_type)
+{
        if (report_type == HID_OUTPUT_REPORT) {
-               report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-               return hidp_send_intr_message(session, report_type,
-                                             data, count);
+               return hidp_output_report(hid, data, count);
NAK, that does not work with BT. hidp_output_raw_report() can do both,
a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
RTYPE_FEATURE the ctrl-channel.

This function is exactly the reason why we added raw_report() and
output_report(). So I'd like to keep it as it and just remove it once
we have no more users of hidp_output_raw_report().

Thanks
David
quoted hunk
        } else if (report_type != HID_FEATURE_REPORT) {
                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);
-       report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
-       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;
+       return hidp_set_raw_report(hid, data[0], data, count, report_type);
 }

 static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
@@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
        }
 }

-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;
--
1.8.3.1

Re: [PATCH 07/11] HID: HIDp: remove duplicated coded

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 15:11:33 

On Mon, Feb 3, 2014 at 10:02 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
- Move hidp_output_report() above
- Removed duplicated code in hidp_output_raw_report()

Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 469e61b..02670b3 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -373,62 +373,25 @@ err:
        return ret;
 }

-static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
-               unsigned char report_type)
+static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
 {
        struct hidp_session *session = hid->driver_data;
-       int ret;

+       return hidp_send_intr_message(session,
+                                     HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+                                     data, count);
+}
+
+static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
+               size_t count, unsigned char report_type)
+{
        if (report_type == HID_OUTPUT_REPORT) {
-               report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-               return hidp_send_intr_message(session, report_type,
-                                             data, count);
+               return hidp_output_report(hid, data, count);
NAK, that does not work with BT. hidp_output_raw_report() can do both,
a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
RTYPE_FEATURE the ctrl-channel.
David, please, re-review it carefully, and see that there is _no_
change in the code path with this refactoring:
hidp_output_report() calls exactly one hidp_send_intr_message() with
the same arguments.

And for the rest, you will see that the code path is still the same also.

This function is exactly the reason why we added raw_report() and
output_report(). So I'd like to keep it as it and just remove it once
we have no more users of hidp_output_raw_report().
I worked on that yesterday, so I can make this happening soon (not
today though).

Cheers,
Benjamin

Thanks
David
quoted
        } else if (report_type != HID_FEATURE_REPORT) {
                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);
-       report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
-       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;
+       return hidp_set_raw_report(hid, data[0], data, count, report_type);
 }

 static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
@@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
        }
 }

-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;
--
1.8.3.1

Re: [PATCH 07/11] HID: HIDp: remove duplicated coded

From: David Herrmann <hidden>
Date: 2014-02-03 15:19:06 

Hi

On Mon, Feb 3, 2014 at 4:11 PM, Benjamin Tissoires
[off-list ref] wrote:
On Mon, Feb 3, 2014 at 10:02 AM, David Herrmann [off-list ref] wrote:
quoted
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
- Move hidp_output_report() above
- Removed duplicated code in hidp_output_raw_report()

Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 68 ++++++++---------------------------------------
 1 file changed, 11 insertions(+), 57 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 469e61b..02670b3 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -373,62 +373,25 @@ err:
        return ret;
 }

-static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data, size_t count,
-               unsigned char report_type)
+static int hidp_output_report(struct hid_device *hid, __u8 *data, size_t count)
 {
        struct hidp_session *session = hid->driver_data;
-       int ret;

+       return hidp_send_intr_message(session,
+                                     HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT,
+                                     data, count);
+}
+
+static int hidp_output_raw_report(struct hid_device *hid, unsigned char *data,
+               size_t count, unsigned char report_type)
+{
        if (report_type == HID_OUTPUT_REPORT) {
-               report_type = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-               return hidp_send_intr_message(session, report_type,
-                                             data, count);
+               return hidp_output_report(hid, data, count);
NAK, that does not work with BT. hidp_output_raw_report() can do both,
a synchronous SET_REPORT and an async OUTPUT_REPORT. It's currently
screwed up, as for RTYPE_OUTPUT it uses the intr-channel, for
RTYPE_FEATURE the ctrl-channel.
David, please, re-review it carefully, and see that there is _no_
change in the code path with this refactoring:
hidp_output_report() calls exactly one hidp_send_intr_message() with
the same arguments.

And for the rest, you will see that the code path is still the same also.
Sorry, my bad. I thought you changed hidp_output_raw_report() to call
hidp_output_report() unconditionally. You're indeed right, the
code-paths are the same and they now highlight the very horrible hack
hid_output_raw_report() has been in the past for HIDP.

Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted
This function is exactly the reason why we added raw_report() and
output_report(). So I'd like to keep it as it and just remove it once
we have no more users of hidp_output_raw_report().
I worked on that yesterday, so I can make this happening soon (not
today though).

Cheers,
Benjamin
quoted
Thanks
David
quoted
        } else if (report_type != HID_FEATURE_REPORT) {
                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);
-       report_type = HIDP_TRANS_SET_REPORT | HIDP_DATA_RTYPE_FEATURE;
-       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;
+       return hidp_set_raw_report(hid, data[0], data, count, report_type);
 }

 static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
@@ -445,15 +408,6 @@ static int hidp_raw_request(struct hid_device *hid, unsigned char reportnum,
        }
 }

-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;
--
1.8.3.1

[PATCH 02/11] HID: i2c-hid: implement ll_driver transport-layer callbacks

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:35 

Add output_report and raw_request to i2c-hid.
Hopefully, we will manage to have the same transport level between
all the transport drivers.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/i2c-hid/i2c-hid.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index ce68a12..5099f1f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -574,6 +574,28 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf,
 	return ret;
 }
 
+static int i2c_hid_output_report(struct hid_device *hid, __u8 *buf,
+		size_t count)
+{
+	return i2c_hid_output_raw_report(hid, buf, count, HID_OUTPUT_REPORT);
+}
+
+static int i2c_hid_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 i2c_hid_get_raw_report(hid, reportnum, buf, len, rtype);
+	case HID_REQ_SET_REPORT:
+		if (buf[0] != reportnum)
+			return -EINVAL;
+		return i2c_hid_output_raw_report(hid, buf, len, rtype);
+	default:
+		return -EIO;
+	}
+}
+
 static void i2c_hid_request(struct hid_device *hid, struct hid_report *rep,
 		int reqtype)
 {
@@ -761,6 +783,8 @@ static struct hid_ll_driver i2c_hid_ll_driver = {
 	.close = i2c_hid_close,
 	.power = i2c_hid_power,
 	.request = i2c_hid_request,
+	.output_report = i2c_hid_output_report,
+	.raw_request = i2c_hid_raw_request,
 };
 
 static int i2c_hid_init_irq(struct i2c_client *client)
-- 
1.8.3.1

Re: [PATCH 02/11] HID: i2c-hid: implement ll_driver transport-layer callbacks

From: David Herrmann <hidden>
Date: 2014-02-03 16:04:21 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted hunk
Add output_report and raw_request to i2c-hid.
Hopefully, we will manage to have the same transport level between
all the transport drivers.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/i2c-hid/i2c-hid.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index ce68a12..5099f1f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -574,6 +574,28 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf,
        return ret;
 }

+static int i2c_hid_output_report(struct hid_device *hid, __u8 *buf,
+               size_t count)
+{
+       return i2c_hid_output_raw_report(hid, buf, count, HID_OUTPUT_REPORT);
+}
+
+static int i2c_hid_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 i2c_hid_get_raw_report(hid, reportnum, buf, len, rtype);
+       case HID_REQ_SET_REPORT:
+               if (buf[0] != reportnum)
+                       return -EINVAL;
+               return i2c_hid_output_raw_report(hid, buf, len, rtype);
I just skimmed the I2C-HID specs and it defines three methods for
input/output reports:

1) Section 6.2:
raw async output-reports can be sent by writing the data at any time
to wOutputRegister.
This should be used as method for hid->output_report().

2) Section 7.1:
SET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and the data into wDataRegister.
This should be used as method for hid->raw_request() + HID_REQ_SET_REPORT.

3) Section 7.1:
GET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and then waiting for the device to write the data
into wDataRegister.
This should be used for hid->raw_request() + HID_REQ_GET_REPORT


The GET_REPORT implementation looks fine to me, but the
i2c_hid_set_report() seems to support both 1) and 2) depending on the
passed type and capabilities:
 - it uses 2) for FEATURE_REPORT reports or if the max OUTPUT_LENGTH is 0
 - it uses 1) otherwise

I'm not sure whether the i2c-hid-spec mandates this behavior, so I am
not saying it's wrong. I just wanna understand what we do here. So if
we use hid->output_report() with HID_FEATURE_REPORT, the current code
turns this into a SET_REPORT. Likewise, an hid->raw_request() with
HID_REQ_SET_REPORT but with HID_OUTPUT_REPORT turns into an
output-report.

I'd rather expect this behavior:

hid->output_report() should always do this:
  args[index++] = outputRegister & 0xFF;
  args[index++] = outputRegister >> 8;
  hidcmd = &hid_no_cmd;

while hid->raw_request() should always do this:
  args[index++] = dataRegister & 0xFF;
  args[index++] = dataRegister >> 8;

The special case for maxOutputLength==0 seems fine to me, but the
"reportType == 0x03" looks weird.


Thanks
David
quoted hunk
+       default:
+               return -EIO;
+       }
+}
+
 static void i2c_hid_request(struct hid_device *hid, struct hid_report *rep,
                int reqtype)
 {
@@ -761,6 +783,8 @@ static struct hid_ll_driver i2c_hid_ll_driver = {
        .close = i2c_hid_close,
        .power = i2c_hid_power,
        .request = i2c_hid_request,
+       .output_report = i2c_hid_output_report,
+       .raw_request = i2c_hid_raw_request,
 };

 static int i2c_hid_init_irq(struct i2c_client *client)
--
1.8.3.1

Re: [PATCH 02/11] HID: i2c-hid: implement ll_driver transport-layer callbacks

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 19:00:04 

On Mon, Feb 3, 2014 at 11:04 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
Add output_report and raw_request to i2c-hid.
Hopefully, we will manage to have the same transport level between
all the transport drivers.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/i2c-hid/i2c-hid.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index ce68a12..5099f1f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -574,6 +574,28 @@ static int i2c_hid_output_raw_report(struct hid_device *hid, __u8 *buf,
        return ret;
 }

+static int i2c_hid_output_report(struct hid_device *hid, __u8 *buf,
+               size_t count)
+{
+       return i2c_hid_output_raw_report(hid, buf, count, HID_OUTPUT_REPORT);
+}
+
+static int i2c_hid_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 i2c_hid_get_raw_report(hid, reportnum, buf, len, rtype);
+       case HID_REQ_SET_REPORT:
+               if (buf[0] != reportnum)
+                       return -EINVAL;
+               return i2c_hid_output_raw_report(hid, buf, len, rtype);
I just skimmed the I2C-HID specs and it defines three methods for
input/output reports:

1) Section 6.2:
raw async output-reports can be sent by writing the data at any time
to wOutputRegister.
This should be used as method for hid->output_report().

2) Section 7.1:
SET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and the data into wDataRegister.
This should be used as method for hid->raw_request() + HID_REQ_SET_REPORT.

3) Section 7.1:
GET_REPORT can be issued by writing the right OPCODE + report-ID into
wCommandRegister and then waiting for the device to write the data
into wDataRegister.
This should be used for hid->raw_request() + HID_REQ_GET_REPORT


The GET_REPORT implementation looks fine to me, but the
i2c_hid_set_report() seems to support both 1) and 2) depending on the
passed type and capabilities:
 - it uses 2) for FEATURE_REPORT reports or if the max OUTPUT_LENGTH is 0
 - it uses 1) otherwise

I'm not sure whether the i2c-hid-spec mandates this behavior, so I am
not saying it's wrong. I just wanna understand what we do here. So if
we use hid->output_report() with HID_FEATURE_REPORT, the current code
turns this into a SET_REPORT. Likewise, an hid->raw_request() with
HID_REQ_SET_REPORT but with HID_OUTPUT_REPORT turns into an
output-report.
Ok, just for the record (I think that you already answered this in the
4/11 review):
this behavior has been added in commit
811adb9622de310efbb661531c3ec0ae5d2b2bc0 for Synaptics so they can
talk to their HID/I2C firmware.
The use they are making is through hdev->hid_output_raw_report(), and
at that time output_report() did not exist.

I guess that with the new API (output_report() and raw_request() ) we
can revert this and it will not bother them given that rmi-hid.c is
still not upstream.

I have no clue if anyone else is using hdev->hid_output_raw_report()
for I2C devices, but we could still try to clean this right now given
that those devices are not well spread (besides some hid-multitouch
touchscreens which do not use OUTPUT reports).

I am still a little bit concerned by hidraw not able to call
set_report in this case, but this could be fixed also by adding an
ioctl.

How does that sound?

Cheers,
Benjamin

I'd rather expect this behavior:

hid->output_report() should always do this:
  args[index++] = outputRegister & 0xFF;
  args[index++] = outputRegister >> 8;
  hidcmd = &hid_no_cmd;

while hid->raw_request() should always do this:
  args[index++] = dataRegister & 0xFF;
  args[index++] = dataRegister >> 8;

The special case for maxOutputLength==0 seems fine to me, but the
"reportType == 0x03" looks weird.


Thanks
David
quoted
+       default:
+               return -EIO;
+       }
+}
+
 static void i2c_hid_request(struct hid_device *hid, struct hid_report *rep,
                int reqtype)
 {
@@ -761,6 +783,8 @@ static struct hid_ll_driver i2c_hid_ll_driver = {
        .close = i2c_hid_close,
        .power = i2c_hid_power,
        .request = i2c_hid_request,
+       .output_report = i2c_hid_output_report,
+       .raw_request = i2c_hid_raw_request,
 };

 static int i2c_hid_init_irq(struct i2c_client *client)
--
1.8.3.1

[PATCH 05/11] HID: HIDp: remove hidp_hidinput_event

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:37 

hidp uses its own ->hidinput_input_event() instead of the generic binding
in hid-input.
Moving the handling of LEDs towards hidp_hidinput_event() allows two things:
- remove hidinput_input_event definitively from struct hid_device
- hidraw user space programs can also set the LEDs

Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 46 ----------------------------------------------
 1 file changed, 46 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index b062cee..469e61b 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -223,51 +223,6 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
 	input_sync(dev);
 }
 
-static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
-{
-	unsigned char hdr;
-	u8 *buf;
-	int rsize, ret;
-
-	buf = hid_alloc_report_buf(report, GFP_ATOMIC);
-	if (!buf)
-		return -EIO;
-
-	hid_output_report(report, buf);
-	hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-
-	rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
-	ret = hidp_send_intr_message(session, hdr, buf, rsize);
-
-	kfree(buf);
-	return ret;
-}
-
-static int hidp_hidinput_event(struct input_dev *dev, unsigned int type,
-			       unsigned int code, int value)
-{
-	struct hid_device *hid = input_get_drvdata(dev);
-	struct hidp_session *session = hid->driver_data;
-	struct hid_field *field;
-	int offset;
-
-	BT_DBG("session %p type %d code %d value %d",
-	       session, type, code, value);
-
-	if (type != EV_LED)
-		return -1;
-
-	offset = hidinput_find_field(hid, type, code, &field);
-	if (offset == -1) {
-		hid_warn(dev, "event field not found\n");
-		return -1;
-	}
-
-	hid_set_field(field, offset, value);
-
-	return hidp_send_report(session, field->report);
-}
-
 static int hidp_get_raw_report(struct hid_device *hid,
 		unsigned char report_number,
 		unsigned char *data, size_t count,
@@ -817,7 +772,6 @@ static struct hid_ll_driver hidp_hid_driver = {
 	.close = hidp_close,
 	.raw_request = hidp_raw_request,
 	.output_report = hidp_output_report,
-	.hidinput_input_event = hidp_hidinput_event,
 };
 
 /* This function sets up the hid device. It does not add it
-- 
1.8.3.1

Re: [PATCH 05/11] HID: HIDp: remove hidp_hidinput_event

From: David Herrmann <hidden>
Date: 2014-02-03 15:10:58 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
hidp uses its own ->hidinput_input_event() instead of the generic binding
in hid-input.
Moving the handling of LEDs towards hidp_hidinput_event() allows two things:
- remove hidinput_input_event definitively from struct hid_device
- hidraw user space programs can also set the LEDs
This one looks good. hid-input uses output_raw_report(), which is on
INTR for BT-HID, which is equivalent to what hidp_send_report() does.
So:

Reviewed-by: David Herrmann <redacted>

Btw., you might have to keep hidp_send_report() if you drop earlier
patches (haven't exactly looked at the order), but feel free to keep
my r-b anyway.

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 46 ----------------------------------------------
 1 file changed, 46 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index b062cee..469e61b 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -223,51 +223,6 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
        input_sync(dev);
 }

-static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
-{
-       unsigned char hdr;
-       u8 *buf;
-       int rsize, ret;
-
-       buf = hid_alloc_report_buf(report, GFP_ATOMIC);
-       if (!buf)
-               return -EIO;
-
-       hid_output_report(report, buf);
-       hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-
-       rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
-       ret = hidp_send_intr_message(session, hdr, buf, rsize);
-
-       kfree(buf);
-       return ret;
-}
-
-static int hidp_hidinput_event(struct input_dev *dev, unsigned int type,
-                              unsigned int code, int value)
-{
-       struct hid_device *hid = input_get_drvdata(dev);
-       struct hidp_session *session = hid->driver_data;
-       struct hid_field *field;
-       int offset;
-
-       BT_DBG("session %p type %d code %d value %d",
-              session, type, code, value);
-
-       if (type != EV_LED)
-               return -1;
-
-       offset = hidinput_find_field(hid, type, code, &field);
-       if (offset == -1) {
-               hid_warn(dev, "event field not found\n");
-               return -1;
-       }
-
-       hid_set_field(field, offset, value);
-
-       return hidp_send_report(session, field->report);
-}
-
 static int hidp_get_raw_report(struct hid_device *hid,
                unsigned char report_number,
                unsigned char *data, size_t count,
@@ -817,7 +772,6 @@ static struct hid_ll_driver hidp_hid_driver = {
        .close = hidp_close,
        .raw_request = hidp_raw_request,
        .output_report = hidp_output_report,
-       .hidinput_input_event = hidp_hidinput_event,
 };

 /* This function sets up the hid device. It does not add it
--
1.8.3.1

Re: [PATCH 05/11] HID: HIDp: remove hidp_hidinput_event

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 15:28:21 

On Mon, Feb 3, 2014 at 10:10 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
hidp uses its own ->hidinput_input_event() instead of the generic binding
in hid-input.
Moving the handling of LEDs towards hidp_hidinput_event() allows two things:
- remove hidinput_input_event definitively from struct hid_device
- hidraw user space programs can also set the LEDs
This one looks good. hid-input uses output_raw_report(), which is on
INTR for BT-HID, which is equivalent to what hidp_send_report() does.
So:

Reviewed-by: David Herrmann <redacted>
Thanks

Btw., you might have to keep hidp_send_report() if you drop earlier
patches (haven't exactly looked at the order), but feel free to keep
my r-b anyway.
No need to keep it, patches 1 to 4 do not touch HIDp, so no one else
use hidp_send_report().

Cheers,
Benjamin

Thanks
David
quoted
Signed-off-by: Benjamin Tissoires <redacted>
---
 net/bluetooth/hidp/core.c | 46 ----------------------------------------------
 1 file changed, 46 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index b062cee..469e61b 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -223,51 +223,6 @@ static void hidp_input_report(struct hidp_session *session, struct sk_buff *skb)
        input_sync(dev);
 }

-static int hidp_send_report(struct hidp_session *session, struct hid_report *report)
-{
-       unsigned char hdr;
-       u8 *buf;
-       int rsize, ret;
-
-       buf = hid_alloc_report_buf(report, GFP_ATOMIC);
-       if (!buf)
-               return -EIO;
-
-       hid_output_report(report, buf);
-       hdr = HIDP_TRANS_DATA | HIDP_DATA_RTYPE_OUPUT;
-
-       rsize = ((report->size - 1) >> 3) + 1 + (report->id > 0);
-       ret = hidp_send_intr_message(session, hdr, buf, rsize);
-
-       kfree(buf);
-       return ret;
-}
-
-static int hidp_hidinput_event(struct input_dev *dev, unsigned int type,
-                              unsigned int code, int value)
-{
-       struct hid_device *hid = input_get_drvdata(dev);
-       struct hidp_session *session = hid->driver_data;
-       struct hid_field *field;
-       int offset;
-
-       BT_DBG("session %p type %d code %d value %d",
-              session, type, code, value);
-
-       if (type != EV_LED)
-               return -1;
-
-       offset = hidinput_find_field(hid, type, code, &field);
-       if (offset == -1) {
-               hid_warn(dev, "event field not found\n");
-               return -1;
-       }
-
-       hid_set_field(field, offset, value);
-
-       return hidp_send_report(session, field->report);
-}
-
 static int hidp_get_raw_report(struct hid_device *hid,
                unsigned char report_number,
                unsigned char *data, size_t count,
@@ -817,7 +772,6 @@ static struct hid_ll_driver hidp_hid_driver = {
        .close = hidp_close,
        .raw_request = hidp_raw_request,
        .output_report = hidp_output_report,
-       .hidinput_input_event = hidp_hidinput_event,
 };

 /* This function sets up the hid device. It does not add it
--
1.8.3.1

[PATCH 06/11] HID: remove hidinput_input_event handler

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:39 

All the different transport drivers use now the generic event handling
in hid-input. We can remove the handler definitively now.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c | 4 +---
 include/linux/hid.h     | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index a713e62..594722d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1263,9 +1263,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
 	}
 
 	input_set_drvdata(input_dev, hid);
-	if (hid->ll_driver->hidinput_input_event)
-		input_dev->event = hid->ll_driver->hidinput_input_event;
-	else if (hid->ll_driver->request || hid->hid_output_raw_report)
+	if (hid->ll_driver->request || hid->hid_output_raw_report)
 		input_dev->event = hidinput_input_event;
 	input_dev->open = hidinput_open;
 	input_dev->close = hidinput_close;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dddcad0..38c307b 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -675,7 +675,6 @@ struct hid_driver {
  * @stop: called on remove
  * @open: called by input layer on open
  * @close: called by input layer on close
- * @hidinput_input_event: event input event (e.g. ff or leds)
  * @parse: this method is called only once to parse the device data,
  *	   shouldn't allocate anything to not leak memory
  * @request: send report request to device (e.g. feature report)
@@ -693,9 +692,6 @@ struct hid_ll_driver {
 
 	int (*power)(struct hid_device *hdev, int level);
 
-	int (*hidinput_input_event) (struct input_dev *idev, unsigned int type,
-			unsigned int code, int value);
-
 	int (*parse)(struct hid_device *hdev);
 
 	void (*request)(struct hid_device *hdev,
-- 
1.8.3.1

Re: [PATCH 06/11] HID: remove hidinput_input_event handler

From: David Herrmann <hidden>
Date: 2014-02-03 16:10:42 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
All the different transport drivers use now the generic event handling
in hid-input. We can remove the handler definitively now.
\o/

Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c | 4 +---
 include/linux/hid.h     | 4 ----
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index a713e62..594722d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1263,9 +1263,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
        }

        input_set_drvdata(input_dev, hid);
-       if (hid->ll_driver->hidinput_input_event)
-               input_dev->event = hid->ll_driver->hidinput_input_event;
-       else if (hid->ll_driver->request || hid->hid_output_raw_report)
+       if (hid->ll_driver->request || hid->hid_output_raw_report)
                input_dev->event = hidinput_input_event;
        input_dev->open = hidinput_open;
        input_dev->close = hidinput_close;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index dddcad0..38c307b 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -675,7 +675,6 @@ struct hid_driver {
  * @stop: called on remove
  * @open: called by input layer on open
  * @close: called by input layer on close
- * @hidinput_input_event: event input event (e.g. ff or leds)
  * @parse: this method is called only once to parse the device data,
  *        shouldn't allocate anything to not leak memory
  * @request: send report request to device (e.g. feature report)
@@ -693,9 +692,6 @@ struct hid_ll_driver {

        int (*power)(struct hid_device *hdev, int level);

-       int (*hidinput_input_event) (struct input_dev *idev, unsigned int type,
-                       unsigned int code, int value);
-
        int (*parse)(struct hid_device *hdev);

        void (*request)(struct hid_device *hdev,
--
1.8.3.1

[PATCH 08/11] HID: usbhid: remove duplicated code

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:41 

Well, no use to keep twice the same code.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/usbhid/hid-core.c | 64 ++++++++-----------------------------------
 1 file changed, 11 insertions(+), 53 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index f8ca312..406497b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -915,59 +915,6 @@ static int usbhid_set_raw_report(struct hid_device *hid, unsigned int reportnum,
 	return ret;
 }
 
-
-static int usbhid_output_raw_report(struct hid_device *hid, __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;
-
-	if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
-		int actual_length;
-		int skipped_report_id = 0;
-
-		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++;
-		}
-	} else {
-		int skipped_report_id = 0;
-		int report_id = buf[0];
-		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,
-			((report_type + 1) << 8) | report_id,
-			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_report(struct hid_device *hid, __u8 *buf, size_t count)
 {
 	struct usbhid_device *usbhid = hid->driver_data;
@@ -998,6 +945,17 @@ static int usbhid_output_report(struct hid_device *hid, __u8 *buf, size_t count)
 	return ret;
 }
 
+static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf,
+		size_t count, unsigned char report_type)
+{
+	struct usbhid_device *usbhid = hid->driver_data;
+
+	if (usbhid->urbout && report_type != HID_FEATURE_REPORT)
+		return usbhid_output_report(hid, buf, count);
+
+	return usbhid_set_raw_report(hid, buf[0], buf, count, report_type);
+}
+
 static void usbhid_restart_queues(struct usbhid_device *usbhid)
 {
 	if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
-- 
1.8.3.1

Re: [PATCH 08/11] HID: usbhid: remove duplicated code

From: David Herrmann <hidden>
Date: 2014-02-03 16:09:59 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
Well, no use to keep twice the same code.
Yepp, I actually copied the code from that, so this is fine.

Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/usbhid/hid-core.c | 64 ++++++++-----------------------------------
 1 file changed, 11 insertions(+), 53 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index f8ca312..406497b 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -915,59 +915,6 @@ static int usbhid_set_raw_report(struct hid_device *hid, unsigned int reportnum,
        return ret;
 }

-
-static int usbhid_output_raw_report(struct hid_device *hid, __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;
-
-       if (usbhid->urbout && report_type != HID_FEATURE_REPORT) {
-               int actual_length;
-               int skipped_report_id = 0;
-
-               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++;
-               }
-       } else {
-               int skipped_report_id = 0;
-               int report_id = buf[0];
-               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,
-                       ((report_type + 1) << 8) | report_id,
-                       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_report(struct hid_device *hid, __u8 *buf, size_t count)
 {
        struct usbhid_device *usbhid = hid->driver_data;
@@ -998,6 +945,17 @@ static int usbhid_output_report(struct hid_device *hid, __u8 *buf, size_t count)
        return ret;
 }

+static int usbhid_output_raw_report(struct hid_device *hid, __u8 *buf,
+               size_t count, unsigned char report_type)
+{
+       struct usbhid_device *usbhid = hid->driver_data;
+
+       if (usbhid->urbout && report_type != HID_FEATURE_REPORT)
+               return usbhid_output_report(hid, buf, count);
+
+       return usbhid_set_raw_report(hid, buf[0], buf, count, report_type);
+}
+
 static void usbhid_restart_queues(struct usbhid_device *usbhid)
 {
        if (usbhid->urbout && !test_bit(HID_OUT_RUNNING, &usbhid->iofl))
--
1.8.3.1

[PATCH 09/11] HID: remove hid_get_raw_report in struct hid_device

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:42 

dev->hid_get_raw_report(X) and hid_hw_raw_request(X, HID_REQ_GET_REPORT)
are strictly equivalent. Switch the hid subsystem to the hid_hw notation
and remove the field .hid_get_raw_report in struct hid_device.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c       | 6 +++---
 drivers/hid/hid-sony.c        | 3 ++-
 drivers/hid/hidraw.c          | 7 ++++---
 drivers/hid/i2c-hid/i2c-hid.c | 1 -
 drivers/hid/uhid.c            | 1 -
 drivers/hid/usbhid/hid-core.c | 1 -
 include/linux/hid.h           | 3 ---
 net/bluetooth/hidp/core.c     | 1 -
 8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 594722d..1de5997 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -350,9 +350,9 @@ static int hidinput_get_battery_property(struct power_supply *psy,
 			ret = -ENOMEM;
 			break;
 		}
-		ret = dev->hid_get_raw_report(dev, dev->battery_report_id,
-					      buf, 2,
-					      dev->battery_report_type);
+		ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2,
+					 dev->battery_report_type,
+					 HID_REQ_GET_REPORT);
 
 		if (ret != 2) {
 			ret = -ENODATA;
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 1235405..3930acb 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -706,7 +706,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 	if (!buf)
 		return -ENOMEM;
 
-	ret = hdev->hid_get_raw_report(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT);
+	ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
+				 HID_REQ_GET_REPORT);
 
 	if (ret < 0)
 		hid_err(hdev, "can't set operational mode\n");
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index cb0137b..4b2dc95 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -189,7 +189,7 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t
 
 	dev = hidraw_table[minor]->hid;
 
-	if (!dev->hid_get_raw_report) {
+	if (!dev->ll_driver->raw_request) {
 		ret = -ENODEV;
 		goto out;
 	}
@@ -216,14 +216,15 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t
 
 	/*
 	 * Read the first byte from the user. This is the report number,
-	 * which is passed to dev->hid_get_raw_report().
+	 * which is passed to hid_hw_raw_request().
 	 */
 	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);
+	ret = hid_hw_raw_request(dev, report_number, buf, count, report_type,
+				 HID_REQ_GET_REPORT);
 
 	if (ret < 0)
 		goto out_free;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 5099f1f..fe3b392 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1029,7 +1029,6 @@ static int i2c_hid_probe(struct i2c_client *client,
 
 	hid->driver_data = client;
 	hid->ll_driver = &i2c_hid_ll_driver;
-	hid->hid_get_raw_report = i2c_hid_get_raw_report;
 	hid->hid_output_raw_report = i2c_hid_output_raw_report;
 	hid->dev.parent = &client->dev;
 	ACPI_COMPANION_SET(&hid->dev, ACPI_COMPANION(&client->dev));
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 438c9f1..7358346 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -421,7 +421,6 @@ static int uhid_dev_create(struct uhid_device *uhid,
 	hid->uniq[63] = 0;
 
 	hid->ll_driver = &uhid_hid_driver;
-	hid->hid_get_raw_report = uhid_hid_get_raw;
 	hid->hid_output_raw_report = uhid_hid_output_raw;
 	hid->bus = ev->u.create.bus;
 	hid->vendor = ev->u.create.vendor;
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 406497b..b9a770f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1289,7 +1289,6 @@ 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 38c307b..c56681a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -508,9 +508,6 @@ 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/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 02670b3..77c4bad 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -773,7 +773,6 @@ static int hidp_setup_hid(struct hidp_session *session,
 	hid->dev.parent = &session->conn->hcon->dev;
 	hid->ll_driver = &hidp_hid_driver;
 
-	hid->hid_get_raw_report = hidp_get_raw_report;
 	hid->hid_output_raw_report = hidp_output_raw_report;
 
 	/* True if device is blacklisted in drivers/hid/hid-core.c */
-- 
1.8.3.1

Re: [PATCH 09/11] HID: remove hid_get_raw_report in struct hid_device

From: David Herrmann <hidden>
Date: 2014-02-03 16:12:31 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
dev->hid_get_raw_report(X) and hid_hw_raw_request(X, HID_REQ_GET_REPORT)
are strictly equivalent. Switch the hid subsystem to the hid_hw notation
and remove the field .hid_get_raw_report in struct hid_device.
Finally we can remove that beast:

Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c       | 6 +++---
 drivers/hid/hid-sony.c        | 3 ++-
 drivers/hid/hidraw.c          | 7 ++++---
 drivers/hid/i2c-hid/i2c-hid.c | 1 -
 drivers/hid/uhid.c            | 1 -
 drivers/hid/usbhid/hid-core.c | 1 -
 include/linux/hid.h           | 3 ---
 net/bluetooth/hidp/core.c     | 1 -
 8 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 594722d..1de5997 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -350,9 +350,9 @@ static int hidinput_get_battery_property(struct power_supply *psy,
                        ret = -ENOMEM;
                        break;
                }
-               ret = dev->hid_get_raw_report(dev, dev->battery_report_id,
-                                             buf, 2,
-                                             dev->battery_report_type);
+               ret = hid_hw_raw_request(dev, dev->battery_report_id, buf, 2,
+                                        dev->battery_report_type,
+                                        HID_REQ_GET_REPORT);

                if (ret != 2) {
                        ret = -ENODATA;
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 1235405..3930acb 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -706,7 +706,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
        if (!buf)
                return -ENOMEM;

-       ret = hdev->hid_get_raw_report(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT);
+       ret = hid_hw_raw_request(hdev, 0xf2, buf, 17, HID_FEATURE_REPORT,
+                                HID_REQ_GET_REPORT);

        if (ret < 0)
                hid_err(hdev, "can't set operational mode\n");
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index cb0137b..4b2dc95 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -189,7 +189,7 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t

        dev = hidraw_table[minor]->hid;

-       if (!dev->hid_get_raw_report) {
+       if (!dev->ll_driver->raw_request) {
                ret = -ENODEV;
                goto out;
        }
@@ -216,14 +216,15 @@ static ssize_t hidraw_get_report(struct file *file, char __user *buffer, size_t

        /*
         * Read the first byte from the user. This is the report number,
-        * which is passed to dev->hid_get_raw_report().
+        * which is passed to hid_hw_raw_request().
         */
        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);
+       ret = hid_hw_raw_request(dev, report_number, buf, count, report_type,
+                                HID_REQ_GET_REPORT);

        if (ret < 0)
                goto out_free;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 5099f1f..fe3b392 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1029,7 +1029,6 @@ static int i2c_hid_probe(struct i2c_client *client,

        hid->driver_data = client;
        hid->ll_driver = &i2c_hid_ll_driver;
-       hid->hid_get_raw_report = i2c_hid_get_raw_report;
        hid->hid_output_raw_report = i2c_hid_output_raw_report;
        hid->dev.parent = &client->dev;
        ACPI_COMPANION_SET(&hid->dev, ACPI_COMPANION(&client->dev));
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 438c9f1..7358346 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -421,7 +421,6 @@ static int uhid_dev_create(struct uhid_device *uhid,
        hid->uniq[63] = 0;

        hid->ll_driver = &uhid_hid_driver;
-       hid->hid_get_raw_report = uhid_hid_get_raw;
        hid->hid_output_raw_report = uhid_hid_output_raw;
        hid->bus = ev->u.create.bus;
        hid->vendor = ev->u.create.vendor;
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 406497b..b9a770f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1289,7 +1289,6 @@ 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 38c307b..c56681a 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -508,9 +508,6 @@ 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/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 02670b3..77c4bad 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -773,7 +773,6 @@ static int hidp_setup_hid(struct hidp_session *session,
        hid->dev.parent = &session->conn->hcon->dev;
        hid->ll_driver = &hidp_hid_driver;

-       hid->hid_get_raw_report = hidp_get_raw_report;
        hid->hid_output_raw_report = hidp_output_raw_report;

        /* True if device is blacklisted in drivers/hid/hid-core.c */
--
1.8.3.1

[PATCH 10/11] HID: introduce helper to access hid_output_raw_report()

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:44 

Add a helper to access hdev->hid_output_raw_report().

To convert the drivers, use the following snippets:

for i in drivers/hid/*.c
do
  sed -i.bak "s/[^ \t]*->hid_output_raw_report(/hid_output_raw_report(/g" $i
done

Then manually fix for checkpatch.pl

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c        |  2 +-
 drivers/hid/hid-lg.c           |  6 ++++--
 drivers/hid/hid-logitech-dj.c  |  2 +-
 drivers/hid/hid-magicmouse.c   |  2 +-
 drivers/hid/hid-sony.c         |  5 +++--
 drivers/hid/hid-thingm.c       |  4 ++--
 drivers/hid/hid-wacom.c        | 16 +++++++---------
 drivers/hid/hid-wiimote-core.c |  2 +-
 drivers/hid/hidraw.c           |  2 +-
 include/linux/hid.h            | 16 ++++++++++++++++
 10 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1de5997..78293c3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1184,7 +1184,7 @@ static void hidinput_led_worker(struct work_struct *work)
 
 	hid_output_report(report, buf);
 	/* synchronous output report */
-	hid->hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
+	hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
 	kfree(buf);
 }
 
diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index 9fe9d4a..76ed7e5 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -692,7 +692,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (hdev->product == USB_DEVICE_ID_LOGITECH_WII_WHEEL) {
 		unsigned char buf[] = { 0x00, 0xAF,  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
 
-		ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+		ret = hid_output_raw_report(hdev, buf, sizeof(buf),
+					    HID_FEATURE_REPORT);
 
 		if (ret >= 0) {
 			/* insert a little delay of 10 jiffies ~ 40ms */
@@ -704,7 +705,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			buf[1] = 0xB2;
 			get_random_bytes(&buf[2], 2);
 
-			ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+			ret = hid_output_raw_report(hdev, buf, sizeof(buf),
+						    HID_FEATURE_REPORT);
 		}
 	}
 
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 61d2bbf..9347625 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -567,7 +567,7 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
 	out_buf[1] = djdev->device_index;
 	memcpy(out_buf + 2, buf, count);
 
-	ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+	ret = hid_output_raw_report(djrcv_dev->hdev, out_buf,
 		DJREPORT_SHORT_LENGTH, report_type);
 
 	kfree(out_buf);
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 3b43d1c..cb5db3a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -538,7 +538,7 @@ static int magicmouse_probe(struct hid_device *hdev,
 	 * but there seems to be no other way of switching the mode.
 	 * Thus the super-ugly hacky success check below.
 	 */
-	ret = hdev->hid_output_raw_report(hdev, feature, sizeof(feature),
+	ret = hid_output_raw_report(hdev, feature, sizeof(feature),
 			HID_FEATURE_REPORT);
 	if (ret != -EIO && ret != sizeof(feature)) {
 		hid_err(hdev, "unable to request touch data (%d)\n", ret);
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 3930acb..8494b8c 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -720,7 +720,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 static int sixaxis_set_operational_bt(struct hid_device *hdev)
 {
 	unsigned char buf[] = { 0xf4,  0x42, 0x03, 0x00, 0x00 };
-	return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+	return hid_output_raw_report(hdev, buf, sizeof(buf),
+				     HID_FEATURE_REPORT);
 }
 
 static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
@@ -942,7 +943,7 @@ static void sixaxis_state_worker(struct work_struct *work)
 	buf[10] |= sc->led_state[2] << 3;
 	buf[10] |= sc->led_state[3] << 4;
 
-	sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+	hid_output_raw_report(sc->hdev, buf, sizeof(buf),
 					HID_OUTPUT_REPORT);
 }
 
diff --git a/drivers/hid/hid-thingm.c b/drivers/hid/hid-thingm.c
index 99342cf..7dd3197 100644
--- a/drivers/hid/hid-thingm.c
+++ b/drivers/hid/hid-thingm.c
@@ -48,8 +48,8 @@ static int blink1_send_command(struct blink1_data *data,
 			buf[0], buf[1], buf[2], buf[3], buf[4],
 			buf[5], buf[6], buf[7], buf[8]);
 
-	ret = data->hdev->hid_output_raw_report(data->hdev, buf,
-			BLINK1_CMD_SIZE, HID_FEATURE_REPORT);
+	ret = hid_output_raw_report(data->hdev, buf, BLINK1_CMD_SIZE,
+				    HID_FEATURE_REPORT);
 
 	return ret < 0 ? ret : 0;
 }
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 60c75dc..c720db9 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -128,8 +128,7 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,
 
 	rep_data[0] = WAC_CMD_ICON_START_STOP;
 	rep_data[1] = 0;
-	ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
-				HID_FEATURE_REPORT);
+	ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);
 	if (ret < 0)
 		goto err;
 
@@ -143,15 +142,14 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,
 			rep_data[j + 3] = p[(i << 6) + j];
 
 		rep_data[2] = i;
-		ret = hdev->hid_output_raw_report(hdev, rep_data, 67,
+		ret = hid_output_raw_report(hdev, rep_data, 67,
 					HID_FEATURE_REPORT);
 	}
 
 	rep_data[0] = WAC_CMD_ICON_START_STOP;
 	rep_data[1] = 0;
 
-	ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
-				HID_FEATURE_REPORT);
+	ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);
 
 err:
 	return;
@@ -183,7 +181,7 @@ static void wacom_leds_set_brightness(struct led_classdev *led_dev,
 		buf[3] = value;
 		/* use fixed brightness for OLEDs */
 		buf[4] = 0x08;
-		hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
+		hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
 		kfree(buf);
 	}
 
@@ -339,7 +337,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
 		rep_data[0] = 0x03 ; rep_data[1] = 0x00;
 		limit = 3;
 		do {
-			ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+			ret = hid_output_raw_report(hdev, rep_data, 2,
 					HID_FEATURE_REPORT);
 		} while (ret < 0 && limit-- > 0);
 
@@ -352,7 +350,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
 			rep_data[1] = 0x00;
 			limit = 3;
 			do {
-				ret = hdev->hid_output_raw_report(hdev,
+				ret = hid_output_raw_report(hdev,
 					rep_data, 2, HID_FEATURE_REPORT);
 			} while (ret < 0 && limit-- > 0);
 
@@ -378,7 +376,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
 		rep_data[0] = 0x03;
 		rep_data[1] = wdata->features;
 
-		ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+		ret = hid_output_raw_report(hdev, rep_data, 2,
 					HID_FEATURE_REPORT);
 		if (ret >= 0)
 			wdata->high_speed = speed;
diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index abb20db..d7dc6c5b 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -35,7 +35,7 @@ static int wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
 	if (!buf)
 		return -ENOMEM;
 
-	ret = hdev->hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);
+	ret = hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);
 
 	kfree(buf);
 	return ret;
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 4b2dc95..f8708c9 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -153,7 +153,7 @@ static ssize_t hidraw_send_report(struct file *file, const char __user *buffer,
 		goto out_free;
 	}
 
-	ret = dev->hid_output_raw_report(dev, buf, count, report_type);
+	ret = hid_output_raw_report(dev, buf, count, report_type);
 out_free:
 	kfree(buf);
 out:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index c56681a..a837ede 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1012,6 +1012,22 @@ static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
 }
 
 /**
+ * hid_output_raw_report - send an output or a feature report to the device
+ *
+ * @hdev: hid device
+ * @buf: raw data to transfer
+ * @len: length of buf
+ * @report_type: HID_FEATURE_REPORT or HID_OUTPUT_REPORT
+ *
+ * @return: count of data transfered, negative if error
+ */
+static inline int hid_output_raw_report(struct hid_device *hdev, __u8 *buf,
+					size_t len, unsigned char report_type)
+{
+	return hdev->hid_output_raw_report(hdev, buf, len, report_type);
+}
+
+/**
  * hid_hw_idle - send idle request to device
  *
  * @hdev: hid device
-- 
1.8.3.1

Re: [PATCH 10/11] HID: introduce helper to access hid_output_raw_report()

From: David Herrmann <hidden>
Date: 2014-02-03 16:14:51 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
Add a helper to access hdev->hid_output_raw_report().

To convert the drivers, use the following snippets:

for i in drivers/hid/*.c
do
  sed -i.bak "s/[^ \t]*->hid_output_raw_report(/hid_output_raw_report(/g" $i
done

Then manually fix for checkpatch.pl
Looks all good. I guess you didn't use the hid_hw_* prefix as it's not
a hdrv but hdev callback? But we hopefully can remove this soon,
anyway. So:

Reviewed-by: David Herrmann <redacted>

Thanks
David
quoted hunk
Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c        |  2 +-
 drivers/hid/hid-lg.c           |  6 ++++--
 drivers/hid/hid-logitech-dj.c  |  2 +-
 drivers/hid/hid-magicmouse.c   |  2 +-
 drivers/hid/hid-sony.c         |  5 +++--
 drivers/hid/hid-thingm.c       |  4 ++--
 drivers/hid/hid-wacom.c        | 16 +++++++---------
 drivers/hid/hid-wiimote-core.c |  2 +-
 drivers/hid/hidraw.c           |  2 +-
 include/linux/hid.h            | 16 ++++++++++++++++
 10 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 1de5997..78293c3 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1184,7 +1184,7 @@ static void hidinput_led_worker(struct work_struct *work)

        hid_output_report(report, buf);
        /* synchronous output report */
-       hid->hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
+       hid_output_raw_report(hid, buf, len, HID_OUTPUT_REPORT);
        kfree(buf);
 }

diff --git a/drivers/hid/hid-lg.c b/drivers/hid/hid-lg.c
index 9fe9d4a..76ed7e5 100644
--- a/drivers/hid/hid-lg.c
+++ b/drivers/hid/hid-lg.c
@@ -692,7 +692,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
        if (hdev->product == USB_DEVICE_ID_LOGITECH_WII_WHEEL) {
                unsigned char buf[] = { 0x00, 0xAF,  0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };

-               ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+               ret = hid_output_raw_report(hdev, buf, sizeof(buf),
+                                           HID_FEATURE_REPORT);

                if (ret >= 0) {
                        /* insert a little delay of 10 jiffies ~ 40ms */
@@ -704,7 +705,8 @@ static int lg_probe(struct hid_device *hdev, const struct hid_device_id *id)
                        buf[1] = 0xB2;
                        get_random_bytes(&buf[2], 2);

-                       ret = hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+                       ret = hid_output_raw_report(hdev, buf, sizeof(buf),
+                                                   HID_FEATURE_REPORT);
                }
        }

diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 61d2bbf..9347625 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -567,7 +567,7 @@ static int logi_dj_output_hidraw_report(struct hid_device *hid, u8 * buf,
        out_buf[1] = djdev->device_index;
        memcpy(out_buf + 2, buf, count);

-       ret = djrcv_dev->hdev->hid_output_raw_report(djrcv_dev->hdev, out_buf,
+       ret = hid_output_raw_report(djrcv_dev->hdev, out_buf,
                DJREPORT_SHORT_LENGTH, report_type);

        kfree(out_buf);
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 3b43d1c..cb5db3a 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -538,7 +538,7 @@ static int magicmouse_probe(struct hid_device *hdev,
         * but there seems to be no other way of switching the mode.
         * Thus the super-ugly hacky success check below.
         */
-       ret = hdev->hid_output_raw_report(hdev, feature, sizeof(feature),
+       ret = hid_output_raw_report(hdev, feature, sizeof(feature),
                        HID_FEATURE_REPORT);
        if (ret != -EIO && ret != sizeof(feature)) {
                hid_err(hdev, "unable to request touch data (%d)\n", ret);
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 3930acb..8494b8c 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -720,7 +720,8 @@ static int sixaxis_set_operational_usb(struct hid_device *hdev)
 static int sixaxis_set_operational_bt(struct hid_device *hdev)
 {
        unsigned char buf[] = { 0xf4,  0x42, 0x03, 0x00, 0x00 };
-       return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
+       return hid_output_raw_report(hdev, buf, sizeof(buf),
+                                    HID_FEATURE_REPORT);
 }

 static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
@@ -942,7 +943,7 @@ static void sixaxis_state_worker(struct work_struct *work)
        buf[10] |= sc->led_state[2] << 3;
        buf[10] |= sc->led_state[3] << 4;

-       sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+       hid_output_raw_report(sc->hdev, buf, sizeof(buf),
                                        HID_OUTPUT_REPORT);
 }

diff --git a/drivers/hid/hid-thingm.c b/drivers/hid/hid-thingm.c
index 99342cf..7dd3197 100644
--- a/drivers/hid/hid-thingm.c
+++ b/drivers/hid/hid-thingm.c
@@ -48,8 +48,8 @@ static int blink1_send_command(struct blink1_data *data,
                        buf[0], buf[1], buf[2], buf[3], buf[4],
                        buf[5], buf[6], buf[7], buf[8]);

-       ret = data->hdev->hid_output_raw_report(data->hdev, buf,
-                       BLINK1_CMD_SIZE, HID_FEATURE_REPORT);
+       ret = hid_output_raw_report(data->hdev, buf, BLINK1_CMD_SIZE,
+                                   HID_FEATURE_REPORT);

        return ret < 0 ? ret : 0;
 }
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 60c75dc..c720db9 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -128,8 +128,7 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,

        rep_data[0] = WAC_CMD_ICON_START_STOP;
        rep_data[1] = 0;
-       ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
-                               HID_FEATURE_REPORT);
+       ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);
        if (ret < 0)
                goto err;

@@ -143,15 +142,14 @@ static void wacom_set_image(struct hid_device *hdev, const char *image,
                        rep_data[j + 3] = p[(i << 6) + j];

                rep_data[2] = i;
-               ret = hdev->hid_output_raw_report(hdev, rep_data, 67,
+               ret = hid_output_raw_report(hdev, rep_data, 67,
                                        HID_FEATURE_REPORT);
        }

        rep_data[0] = WAC_CMD_ICON_START_STOP;
        rep_data[1] = 0;

-       ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
-                               HID_FEATURE_REPORT);
+       ret = hid_output_raw_report(hdev, rep_data, 2, HID_FEATURE_REPORT);

 err:
        return;
@@ -183,7 +181,7 @@ static void wacom_leds_set_brightness(struct led_classdev *led_dev,
                buf[3] = value;
                /* use fixed brightness for OLEDs */
                buf[4] = 0x08;
-               hdev->hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
+               hid_output_raw_report(hdev, buf, 9, HID_FEATURE_REPORT);
                kfree(buf);
        }

@@ -339,7 +337,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
                rep_data[0] = 0x03 ; rep_data[1] = 0x00;
                limit = 3;
                do {
-                       ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+                       ret = hid_output_raw_report(hdev, rep_data, 2,
                                        HID_FEATURE_REPORT);
                } while (ret < 0 && limit-- > 0);

@@ -352,7 +350,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
                        rep_data[1] = 0x00;
                        limit = 3;
                        do {
-                               ret = hdev->hid_output_raw_report(hdev,
+                               ret = hid_output_raw_report(hdev,
                                        rep_data, 2, HID_FEATURE_REPORT);
                        } while (ret < 0 && limit-- > 0);

@@ -378,7 +376,7 @@ static void wacom_set_features(struct hid_device *hdev, u8 speed)
                rep_data[0] = 0x03;
                rep_data[1] = wdata->features;

-               ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+               ret = hid_output_raw_report(hdev, rep_data, 2,
                                        HID_FEATURE_REPORT);
                if (ret >= 0)
                        wdata->high_speed = speed;
diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index abb20db..d7dc6c5b 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -35,7 +35,7 @@ static int wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
        if (!buf)
                return -ENOMEM;

-       ret = hdev->hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);
+       ret = hid_output_raw_report(hdev, buf, count, HID_OUTPUT_REPORT);

        kfree(buf);
        return ret;
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index 4b2dc95..f8708c9 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -153,7 +153,7 @@ static ssize_t hidraw_send_report(struct file *file, const char __user *buffer,
                goto out_free;
        }

-       ret = dev->hid_output_raw_report(dev, buf, count, report_type);
+       ret = hid_output_raw_report(dev, buf, count, report_type);
 out_free:
        kfree(buf);
 out:
diff --git a/include/linux/hid.h b/include/linux/hid.h
index c56681a..a837ede 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1012,6 +1012,22 @@ static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
 }

 /**
+ * hid_output_raw_report - send an output or a feature report to the device
+ *
+ * @hdev: hid device
+ * @buf: raw data to transfer
+ * @len: length of buf
+ * @report_type: HID_FEATURE_REPORT or HID_OUTPUT_REPORT
+ *
+ * @return: count of data transfered, negative if error
+ */
+static inline int hid_output_raw_report(struct hid_device *hdev, __u8 *buf,
+                                       size_t len, unsigned char report_type)
+{
+       return hdev->hid_output_raw_report(hdev, buf, len, report_type);
+}
+
+/**
  * hid_hw_idle - send idle request to device
  *
  * @hdev: hid device
--
1.8.3.1

[PATCH 11/11] HID: move hid_output_raw_report to hid_ll_driver

From: Benjamin Tissoires <hidden>
Date: 2014-02-02 04:51:45 

struct hid_ll_driver is responsible for the transport communication.
Move hid_output_raw_report from hid_device to the transport layer then.

Signed-off-by: Benjamin Tissoires <redacted>
---
 drivers/hid/hid-input.c        |  2 +-
 drivers/hid/hid-logitech-dj.c  |  2 +-
 drivers/hid/hid-sony.c         | 11 ++++++++++-
 drivers/hid/hid-wiimote-core.c |  2 +-
 drivers/hid/hidraw.c           |  2 +-
 drivers/hid/i2c-hid/i2c-hid.c  |  2 +-
 drivers/hid/uhid.c             |  2 +-
 drivers/hid/usbhid/hid-core.c  |  2 +-
 include/linux/hid.h            | 11 +++++++----
 net/bluetooth/hidp/core.c      |  4 ++--
 10 files changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 78293c3..3125155 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1263,7 +1263,7 @@ static struct hid_input *hidinput_allocate(struct hid_device *hid)
 	}
 
 	input_set_drvdata(input_dev, hid);
-	if (hid->ll_driver->request || hid->hid_output_raw_report)
+	if (hid->ll_driver->request || hid->ll_driver->hid_output_raw_report)
 		input_dev->event = hidinput_input_event;
 	input_dev->open = hidinput_open;
 	input_dev->close = hidinput_close;
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 9347625..bdfa1db 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -262,7 +262,6 @@ static void logi_dj_recv_add_djhid_device(struct dj_receiver_dev *djrcv_dev,
 	}
 
 	dj_hiddev->ll_driver = &logi_dj_ll_driver;
-	dj_hiddev->hid_output_raw_report = logi_dj_output_hidraw_report;
 
 	dj_hiddev->dev.parent = &djrcv_hdev->dev;
 	dj_hiddev->bus = BUS_USB;
@@ -655,6 +654,7 @@ static struct hid_ll_driver logi_dj_ll_driver = {
 	.stop = logi_dj_ll_stop,
 	.open = logi_dj_ll_open,
 	.close = logi_dj_ll_close,
+	.hid_output_raw_report = logi_dj_output_hidraw_report,
 };
 
 
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 8494b8c..9dd37ff 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -494,6 +494,8 @@ struct sony_sc {
 	unsigned long quirks;
 	struct work_struct state_worker;
 
+	struct hid_ll_driver *ll_driver;
+
 #ifdef CONFIG_SONY_FF
 	__u8 left;
 	__u8 right;
@@ -1077,7 +1079,14 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
-		hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
+		sc->ll_driver = devm_kzalloc(&hdev->dev, sizeof(*sc->ll_driver),
+					     GFP_KERNEL);
+		if (sc->ll_driver == NULL)
+			return -ENOMEM;
+		*sc->ll_driver = *hdev->ll_driver;
+		hdev->ll_driver = sc->ll_driver;
+		sc->ll_driver->hid_output_raw_report =
+						sixaxis_usb_output_raw_report;
 		ret = sixaxis_set_operational_usb(hdev);
 		INIT_WORK(&sc->state_worker, sixaxis_state_worker);
 	}
diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
index d7dc6c5b..715a3ab 100644
--- a/drivers/hid/hid-wiimote-core.c
+++ b/drivers/hid/hid-wiimote-core.c
@@ -28,7 +28,7 @@ static int wiimote_hid_send(struct hid_device *hdev, __u8 *buffer,
 	__u8 *buf;
 	int ret;
 
-	if (!hdev->hid_output_raw_report)
+	if (!hdev->ll_driver->hid_output_raw_report)
 		return -ENODEV;
 
 	buf = kmemdup(buffer, count, GFP_KERNEL);
diff --git a/drivers/hid/hidraw.c b/drivers/hid/hidraw.c
index f8708c9..c60c530 100644
--- a/drivers/hid/hidraw.c
+++ b/drivers/hid/hidraw.c
@@ -123,7 +123,7 @@ static ssize_t hidraw_send_report(struct file *file, const char __user *buffer,
 
 	dev = hidraw_table[minor]->hid;
 
-	if (!dev->hid_output_raw_report) {
+	if (!dev->ll_driver->hid_output_raw_report) {
 		ret = -ENODEV;
 		goto out;
 	}
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index fe3b392..fabb388 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -785,6 +785,7 @@ static struct hid_ll_driver i2c_hid_ll_driver = {
 	.request = i2c_hid_request,
 	.output_report = i2c_hid_output_report,
 	.raw_request = i2c_hid_raw_request,
+	.hid_output_raw_report = i2c_hid_output_raw_report,
 };
 
 static int i2c_hid_init_irq(struct i2c_client *client)
@@ -1029,7 +1030,6 @@ static int i2c_hid_probe(struct i2c_client *client,
 
 	hid->driver_data = client;
 	hid->ll_driver = &i2c_hid_ll_driver;
-	hid->hid_output_raw_report = i2c_hid_output_raw_report;
 	hid->dev.parent = &client->dev;
 	ACPI_COMPANION_SET(&hid->dev, ACPI_COMPANION(&client->dev));
 	hid->bus = BUS_I2C;
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 7358346..8e99a5a 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -294,6 +294,7 @@ static struct hid_ll_driver uhid_hid_driver = {
 	.parse = uhid_hid_parse,
 	.output_report = uhid_hid_output_report,
 	.raw_request = uhid_raw_request,
+	.hid_output_raw_report = uhid_hid_output_raw,
 };
 
 #ifdef CONFIG_COMPAT
@@ -421,7 +422,6 @@ static int uhid_dev_create(struct uhid_device *uhid,
 	hid->uniq[63] = 0;
 
 	hid->ll_driver = &uhid_hid_driver;
-	hid->hid_output_raw_report = uhid_hid_output_raw;
 	hid->bus = ev->u.create.bus;
 	hid->vendor = ev->u.create.vendor;
 	hid->product = ev->u.create.product;
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index b9a770f..9c3c244 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1260,6 +1260,7 @@ static struct hid_ll_driver usb_hid_driver = {
 	.raw_request = usbhid_raw_request,
 	.output_report = usbhid_output_report,
 	.idle = usbhid_idle,
+	.hid_output_raw_report = usbhid_output_raw_report,
 };
 
 static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id)
@@ -1289,7 +1290,6 @@ 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_output_raw_report = usbhid_output_raw_report;
 	hid->ff_init = hid_pidff_init;
 #ifdef CONFIG_USB_HIDDEV
 	hid->hiddev_connect = hiddev_connect;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index a837ede..eb588e9 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -508,9 +508,6 @@ struct hid_device {							/* device report descriptor */
 				  struct hid_usage *, __s32);
 	void (*hiddev_report_event) (struct hid_device *, struct hid_report *);
 
-	/* handler for raw output data, used by hidraw */
-	int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t, unsigned char);
-
 	/* debugging support via debugfs */
 	unsigned short debug;
 	struct dentry *debug_dir;
@@ -678,6 +675,7 @@ struct hid_driver {
  * @wait: wait for buffered io to complete (send/recv reports)
  * @raw_request: send raw report request to device (e.g. feature report)
  * @output_report: send output report to device
+ * @hid_output_raw_report: send report to device (e.g. feature report)
  * @idle: send idle request to device
  */
 struct hid_ll_driver {
@@ -702,6 +700,10 @@ struct hid_ll_driver {
 
 	int (*output_report) (struct hid_device *hdev, __u8 *buf, size_t len);
 
+	/* handler for raw output data, used by hidraw */
+	int (*hid_output_raw_report) (struct hid_device *, __u8 *, size_t,
+				      unsigned char);
+
 	int (*idle)(struct hid_device *hdev, int report, int idle, int reqtype);
 };
 
@@ -1024,7 +1026,8 @@ static inline int hid_hw_output_report(struct hid_device *hdev, __u8 *buf,
 static inline int hid_output_raw_report(struct hid_device *hdev, __u8 *buf,
 					size_t len, unsigned char report_type)
 {
-	return hdev->hid_output_raw_report(hdev, buf, len, report_type);
+	return hdev->ll_driver->hid_output_raw_report(hdev, buf, len,
+						      report_type);
 }
 
 /**
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 77c4bad..6189b54 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -726,6 +726,8 @@ static struct hid_ll_driver hidp_hid_driver = {
 	.close = hidp_close,
 	.raw_request = hidp_raw_request,
 	.output_report = hidp_output_report,
+	.hid_output_raw_report = hidp_output_raw_report,
+
 };
 
 /* This function sets up the hid device. It does not add it
@@ -773,8 +775,6 @@ static int hidp_setup_hid(struct hidp_session *session,
 	hid->dev.parent = &session->conn->hcon->dev;
 	hid->ll_driver = &hidp_hid_driver;
 
-	hid->hid_output_raw_report = hidp_output_raw_report;
-
 	/* True if device is blacklisted in drivers/hid/hid-core.c */
 	if (hid_ignore(hid)) {
 		hid_destroy_device(session->hid);
-- 
1.8.3.1

Re: [PATCH 00/11] HID: spring cleaning

From: David Herrmann <hidden>
Date: 2014-02-03 16:48:51 

Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
Hi guys,

This is an attempt to complete the branch for-3.15/ll-driver-new-callbacks:
- try to implement as much as possible ll_driver callbacks (some are still
  missing, but I did not had the time to complete it)
- add inliners hid_hw_* for all the ll_driver callbacks
- remove transport dependant callbacks in struct hid_device
- remove the so called "obsolete" hidinput_input_event handler which was used
  in 2/4 transport drivers

Bonus point: 14 files changed, 213 insertions(+), 272 deletions(-)
Yay!

I made sure I kept the PS3 controller working and the LEDs (on logitech-dj and
on bluetooth keyboard) working. The rest do not mostly need further testing,
the code path did not changed. But still, a review (and some tests) would be a
good idea :)
Thanks a lot! Looks all good except for the uhid and i2c changes. But
I have commented on those.

Cheers
David

Cheers,
Benjamin

PS: obviously, this goes on top on the branch for-3.15/ll-driver-new-callbacks

Benjamin Tissoires (11):
  HID: uHID: implement .raw_request
  HID: i2c-hid: implement ll_driver transport-layer callbacks
  HID: add inliners for ll_driver transport-layer callbacks
  HID: logitech-dj: remove hidinput_input_event
  HID: HIDp: remove hidp_hidinput_event
  HID: remove hidinput_input_event handler
  HID: HIDp: remove duplicated coded
  HID: usbhid: remove duplicated code
  HID: remove hid_get_raw_report in struct hid_device
  HID: introduce helper to access hid_output_raw_report()
  HID: move hid_output_raw_report to hid_ll_driver

 drivers/hid/hid-input.c        |  12 ++---
 drivers/hid/hid-lg.c           |   6 ++-
 drivers/hid/hid-logitech-dj.c  | 101 +++++++++++++---------------------
 drivers/hid/hid-magicmouse.c   |   2 +-
 drivers/hid/hid-sony.c         |  19 +++++--
 drivers/hid/hid-thingm.c       |   4 +-
 drivers/hid/hid-wacom.c        |  16 +++---
 drivers/hid/hid-wiimote-core.c |   4 +-
 drivers/hid/hidraw.c           |  11 ++--
 drivers/hid/i2c-hid/i2c-hid.c  |  27 +++++++++-
 drivers/hid/uhid.c             |  20 ++++++-
 drivers/hid/usbhid/hid-core.c  |  67 +++++------------------
 include/linux/hid.h            |  77 ++++++++++++++++++++++----
 net/bluetooth/hidp/core.c      | 119 +++++------------------------------------
 14 files changed, 213 insertions(+), 272 deletions(-)

--
1.8.3.1

Re: [PATCH 00/11] HID: spring cleaning

From: Benjamin Tissoires <hidden>
Date: 2014-02-03 19:20:01 

On Mon, Feb 3, 2014 at 11:48 AM, David Herrmann [off-list ref] wrote:
Hi

On Sun, Feb 2, 2014 at 5:50 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
Hi guys,

This is an attempt to complete the branch for-3.15/ll-driver-new-callbacks:
- try to implement as much as possible ll_driver callbacks (some are still
  missing, but I did not had the time to complete it)
- add inliners hid_hw_* for all the ll_driver callbacks
- remove transport dependant callbacks in struct hid_device
- remove the so called "obsolete" hidinput_input_event handler which was used
  in 2/4 transport drivers

Bonus point: 14 files changed, 213 insertions(+), 272 deletions(-)
Yay!

I made sure I kept the PS3 controller working and the LEDs (on logitech-dj and
on bluetooth keyboard) working. The rest do not mostly need further testing,
the code path did not changed. But still, a review (and some tests) would be a
good idea :)
Thanks a lot! Looks all good except for the uhid and i2c changes. But
I have commented on those.
Thanks David for the review. Very appreciated.

Jiri, I think the simpler way to handle this is that I send out a v2
of the patches with the reviewed-by David:
I will drop 1, 2 and 11 and rework a little on 4 (hid-logitech-dj).

Then, I'll send out the second series where I will remove
hid_output_raw_report, implement a generic .request() on top of
.raw_request(), and I have some more cleanup to come (less aggressive
this time I think).

Cheers,
Benjamin

Cheers
David
quoted
Cheers,
Benjamin

PS: obviously, this goes on top on the branch for-3.15/ll-driver-new-callbacks

Benjamin Tissoires (11):
  HID: uHID: implement .raw_request
  HID: i2c-hid: implement ll_driver transport-layer callbacks
  HID: add inliners for ll_driver transport-layer callbacks
  HID: logitech-dj: remove hidinput_input_event
  HID: HIDp: remove hidp_hidinput_event
  HID: remove hidinput_input_event handler
  HID: HIDp: remove duplicated coded
  HID: usbhid: remove duplicated code
  HID: remove hid_get_raw_report in struct hid_device
  HID: introduce helper to access hid_output_raw_report()
  HID: move hid_output_raw_report to hid_ll_driver

 drivers/hid/hid-input.c        |  12 ++---
 drivers/hid/hid-lg.c           |   6 ++-
 drivers/hid/hid-logitech-dj.c  | 101 +++++++++++++---------------------
 drivers/hid/hid-magicmouse.c   |   2 +-
 drivers/hid/hid-sony.c         |  19 +++++--
 drivers/hid/hid-thingm.c       |   4 +-
 drivers/hid/hid-wacom.c        |  16 +++---
 drivers/hid/hid-wiimote-core.c |   4 +-
 drivers/hid/hidraw.c           |  11 ++--
 drivers/hid/i2c-hid/i2c-hid.c  |  27 +++++++++-
 drivers/hid/uhid.c             |  20 ++++++-
 drivers/hid/usbhid/hid-core.c  |  67 +++++------------------
 include/linux/hid.h            |  77 ++++++++++++++++++++++----
 net/bluetooth/hidp/core.c      | 119 +++++------------------------------------
 14 files changed, 213 insertions(+), 272 deletions(-)

--
1.8.3.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help