* Re: [PATCH] Bluetooth: hidp: make sure input buffers are big enough
From: Jiri Kosina @ 2014-02-05 15:49 UTC (permalink / raw)
To: Marcel Holtmann, Gustavo F. Padovan
Cc: David Herrmann, open list:HID CORE LAYER,
linux-bluetooth-u79uwXL29TY76Z2rM5mHXA@public.gmane.org development
In-Reply-To: <20573666-E2A0-4D6B-986A-09DAFC0FB64B-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
On Tue, 4 Feb 2014, Marcel Holtmann wrote:
> >>>>> just got back to this, finally ... did you have time to work on this at
> >>>>> all, or should I just start from scratch?
> >>>>
> >>>> Sorry, no. Fosdem is this weekend and I needed to get my code ready
> >>>> for that. But I'll finally have time again next week.
> >>>
> >>> Okay, thanks. I then guess we should proceed with this bandaid (double
> >>> allocation) for 3.14
> >>
> >> It would be really nice if we could get the HIDP patch into 3.14-rc2
> >> and backported to stable. There have been quite a bunch of reports now
> >> and I dislike adding a GFP_ATOMIC allocation in HID core.
> >
> > I agree with David; Gustavo, what is your take on this, please?
>
> I leave this up to Gustavo to get this into wireless tree for 3.14-rc2.
>
> Acked-by: Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
Thanks a lot.
Gustavo, what is your take on this please? I can take it through hid.git
in case you don't have anything else queued for -rc2.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* BUG: HID: hid-sony - LEDs on 3rd party controller
From: simon @ 2014-02-05 18:10 UTC (permalink / raw)
To: linux-input
Tested 3.14rc1 on a collection of PS3/PS4 controllers, and noticed that
behaviour of the LEDs is 'odd' on a third party controller.
The controller shows 4 leds, which flash together when first plugged in.
Individual leds can be controlled via the '/sys/class/led/' interface,
however when all leds are set to off (0) the 'all flashing' restarts.
This is different to the official controllers (SixAxis and Dualshock4)
which will extingush all leds as expected.
The controller in question is made by Intec and identifies as
--
Nov 16 14:26:21 womble kernel: [ 95.296039] usb 3-1: new full-speed USB
device
number 2 using uhci_hcd
Nov 16 14:26:22 womble kernel: [ 95.463964] usb 3-1: New USB device found,
idVendor=054c, idProduct=0268
Nov 16 14:26:22 womble kernel: [ 95.463973] usb 3-1: New USB device
strings:
Mfr=1, Product=2, SerialNumber=0
Nov 16 14:26:22 womble kernel: [ 95.463978] usb 3-1: Product:
PLAYSTATION(R)3
Controller
Nov 16 14:26:22 womble kernel: [ 95.463982] usb 3-1: Manufacturer: GASIA
CORP.
Nov 16 14:26:22 womble mtp-probe: checking bus 3, device 2:
"/sys/devices/pci0000:00/0000:00:1d.1/usb3/3-1"
Nov 16 14:26:22 womble mtp-probe: bus: 3, device: 2 was not an MTP device
Nov 16 14:26:22 womble kernel: [ 95.770400] usbcore: registered new
interface
driver usbhid
Nov 16 14:26:22 womble kernel: [ 95.770408] usbhid: USB HID core driver
Nov 16 14:26:22 womble kernel: [ 95.838174] sony 0003:054C:0268.0001:
Fixing up
Sony Sixaxis report descriptor
Nov 16 14:26:22 womble kernel: [ 95.848763] input: GASIA CORP.
PLAYSTATION(R)3
Controller as /devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/input/input7
Nov 16 14:26:22 womble kernel: [ 95.849784] sony 0003:054C:0268.0001:
input,hiddev0,hidraw0: USB HID v1.11 Joystick [GASIA CORP. PLAYSTATION(R)3
Controller] on usb-0000:00:1d.1-1/input0
--
I made some comments previously which may be of use in fixing this:
http://www.spinics.net/lists/linux-input/msg28271.html
Cheers,
Simon.
^ permalink raw reply
* [PATCH] HID: hid-sony Report actual brightness value when reading LED
From: Simon Wood @ 2014-02-05 19:34 UTC (permalink / raw)
To: linux-input; +Cc: Jiri Kosina, linux-kernel, simon
The Dualshock4 controller contains a RGB LED, which is enabled via
the '/sys/class/leds' interface. At present the driver only returns
whether each of the RGB LEDs is lit (ie not off), but no indication
of it's brightness.
This patch fixes the reading of the current brightnes so that it
returns the value (rather than just off=0, on=LED_FULL).
Tested on the DS4 and SixAxis (for compatibility).
Signed-off-by: Simon Wood <simon@mungewell.org>
---
drivers/hid/hid-sony.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 1235405..f9a7610 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -790,7 +790,6 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
struct sony_sc *drv_data;
int n;
- int on = 0;
drv_data = hid_get_drvdata(hdev);
if (!drv_data) {
@@ -799,13 +798,11 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
}
for (n = 0; n < drv_data->led_count; n++) {
- if (led == drv_data->leds[n]) {
- on = !!(drv_data->led_state[n]);
- break;
- }
+ if (led == drv_data->leds[n])
+ return drv_data->led_state[n];
}
- return on ? LED_FULL : LED_OFF;
+ return LED_OFF;
}
static void sony_leds_remove(struct hid_device *hdev)
--
1.8.1.2
^ permalink raw reply related
* [PATCH v2 1/9] HID: add inliners for ll_driver transport-layer callbacks
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
Those callbacks are not mandatory, so it's better to add inliners
to use them safely.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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
^ permalink raw reply related
* [PATCH v2 3/9] HID: HIDp: remove hidp_hidinput_event
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
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
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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
^ permalink raw reply related
* [PATCH v2 4/9] HID: remove hidinput_input_event handler
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
All the different transport drivers use now the generic event handling
in hid-input. We can remove the handler definitively now.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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 1f0d49c..68db2db 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1266,9 +1266,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
^ permalink raw reply related
* [PATCH v2 6/9] HID: usbhid: remove duplicated code
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
Well, no use to keep twice the same code.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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
^ permalink raw reply related
* [PATCH v2 7/9] HID: remove hid_get_raw_report in struct hid_device
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
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.
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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 68db2db..6253e95 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 2bd3f13..b51db79 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -810,7 +810,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 923ff81..f57de7f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1003,7 +1003,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 f5a2b19..12439e1 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -404,7 +404,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
^ permalink raw reply related
* [PATCH v2 8/9] HID: introduce helper to access hid_output_raw_report()
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
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
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-input.c | 2 +-
drivers/hid/hid-lg.c | 6 ++++--
drivers/hid/hid-magicmouse.c | 2 +-
drivers/hid/hid-sony.c | 6 +++---
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 ++++++++++++++++
9 files changed, 36 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 6253e95..eb00a5b 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1187,7 +1187,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-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 b51db79..d275c0d 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -824,7 +824,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)
@@ -1046,8 +1047,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_REPORT);
+ hid_output_raw_report(sc->hdev, buf, sizeof(buf), HID_OUTPUT_REPORT);
}
static void dualshock4_state_worker(struct work_struct *work)
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
^ permalink raw reply related
* [PATCH v2 0/9] HID: spring cleanup v2
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
Hi guys,
well, here comes the promised v2 of the ll_transport cleanup.
As I said, I removed patches which need some more work, and kept only the
trivial ones. I also added David's documentation, which gives us a net
difference of +210 lines of code :(
Let's say that we still have a net worth of -106 lines of actual code :)
Cheers,
Benjamin
Changes since v1:
- removed uhid, i2c-hid patches
- removed the previous 11/11 (move hid_output_raw_report to hid_ll_driver)
- hid-logitech-dj: use hid_hw_raw_request instead of hid_output_report (2/9)
- add documentation - I removed the hid_input_event() doc (9/9)
Benjamin Tissoires (9):
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: Add HID transport driver documentation
Documentation/hid/hid-transport.txt | 316 ++++++++++++++++++++++++++++++++++++
drivers/hid/hid-input.c | 12 +-
drivers/hid/hid-lg.c | 6 +-
drivers/hid/hid-logitech-dj.c | 106 +++++-------
drivers/hid/hid-magicmouse.c | 2 +-
drivers/hid/hid-sony.c | 9 +-
drivers/hid/hid-thingm.c | 4 +-
drivers/hid/hid-wacom.c | 16 +-
drivers/hid/hid-wiimote-core.c | 2 +-
drivers/hid/hidraw.c | 9 +-
drivers/hid/i2c-hid/i2c-hid.c | 1 -
drivers/hid/uhid.c | 1 -
drivers/hid/usbhid/hid-core.c | 65 ++------
include/linux/hid.h | 68 +++++++-
net/bluetooth/hidp/core.c | 115 ++-----------
15 files changed, 471 insertions(+), 261 deletions(-)
create mode 100644 Documentation/hid/hid-transport.txt
--
1.8.3.1
^ permalink raw reply
* [PATCH v2 2/9] HID: logitech-dj: remove hidinput_input_event
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
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 <benjamin.tissoires@redhat.com>
---
drivers/hid/hid-logitech-dj.c | 106 +++++++++++++++++-------------------------
1 file changed, 42 insertions(+), 64 deletions(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index f45279c..980ede5 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,37 @@ 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);
+
+ /*
+ * hid-generic calls us with hid_output_raw_report(), but the LEDs
+ * are set through a SET_REPORT command. It works for USB-HID devices
+ * because usbhid either calls a SET_REPORT or directly send the output
+ * report depending if the device presents an urbout.
+ * Let be simple, send a SET_REPORT request.
+ */
+ ret = hid_hw_raw_request(djrcv_dev->hdev, out_buf[0], out_buf,
+ DJREPORT_SHORT_LENGTH, report_type, HID_REQ_SET_REPORT);
+
+ kfree(out_buf);
+ return ret;
}
static void rdcat(char *rdesc, unsigned int *rsize, const char *data, unsigned int size)
@@ -613,58 +644,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 +662,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
^ permalink raw reply related
* [PATCH v2 5/9] HID: HIDp: remove duplicated coded
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
- Move hidp_output_report() above
- Removed duplicated code in hidp_output_raw_report()
Reviewed-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
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
^ permalink raw reply related
* [PATCH v2 9/9] HID: Add HID transport driver documentation
From: Benjamin Tissoires @ 2014-02-05 21:33 UTC (permalink / raw)
To: Benjamin Tissoires, David Herrmann, Jiri Kosina, Frank Praznik,
linux-input, linux-kernel
In-Reply-To: <1391636004-29354-1-git-send-email-benjamin.tissoires@redhat.com>
Add David Herrmann's documentation for the new low-level HID transport driver
functions.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
Documentation/hid/hid-transport.txt | 316 ++++++++++++++++++++++++++++++++++++
1 file changed, 316 insertions(+)
create mode 100644 Documentation/hid/hid-transport.txt
diff --git a/Documentation/hid/hid-transport.txt b/Documentation/hid/hid-transport.txt
new file mode 100644
index 0000000..9dbbcea
--- /dev/null
+++ b/Documentation/hid/hid-transport.txt
@@ -0,0 +1,316 @@
+ HID I/O Transport Drivers
+ ===========================
+
+The HID subsystem is independent of the underlying transport driver. Initially,
+only USB was supported, but other specifications adopted the HID design and
+provided new transport drivers. The kernel includes at least support for USB,
+Bluetooth, I2C and user-space I/O drivers.
+
+1) HID Bus
+==========
+
+The HID subsystem is designed as a bus. Any I/O subsystem may provide HID
+devices and register them with the HID bus. HID core then loads generic device
+drivers on top of it. The transport drivers are responsible of raw data
+transport and device setup/management. HID core is responsible of
+report-parsing, report interpretation and the user-space API. Device specifics
+and quirks are handled by all layers depending on the quirk.
+
+ +-----------+ +-----------+ +-----------+ +-----------+
+ | Device #1 | | Device #i | | Device #j | | Device #k |
+ +-----------+ +-----------+ +-----------+ +-----------+
+ \\ // \\ //
+ +------------+ +------------+
+ | I/O Driver | | I/O Driver |
+ +------------+ +------------+
+ || ||
+ +------------------+ +------------------+
+ | Transport Driver | | Transport Driver |
+ +------------------+ +------------------+
+ \___ ___/
+ \ /
+ +----------------+
+ | HID Core |
+ +----------------+
+ / | | \
+ / | | \
+ ____________/ | | \_________________
+ / | | \
+ / | | \
+ +----------------+ +-----------+ +------------------+ +------------------+
+ | Generic Driver | | MT Driver | | Custom Driver #1 | | Custom Driver #2 |
+ +----------------+ +-----------+ +------------------+ +------------------+
+
+Example Drivers:
+ I/O: USB, I2C, Bluetooth-l2cap
+ Transport: USB-HID, I2C-HID, BT-HIDP
+
+Everything below "HID Core" is simplified in this graph as it is only of
+interest to HID device drivers. Transport drivers do not need to know the
+specifics.
+
+1.1) Device Setup
+-----------------
+
+I/O drivers normally provide hotplug detection or device enumeration APIs to the
+transport drivers. Transport drivers use this to find any suitable HID device.
+They allocate HID device objects and register them with HID core. Transport
+drivers are not required to register themselves with HID core. HID core is never
+aware of which transport drivers are available and is not interested in it. It
+is only interested in devices.
+
+Transport drivers attach a constant "struct hid_ll_driver" object with each
+device. Once a device is registered with HID core, the callbacks provided via
+this struct are used by HID core to communicate with the device.
+
+Transport drivers are responsible of detecting device failures and unplugging.
+HID core will operate a device as long as it is registered regardless of any
+device failures. Once transport drivers detect unplug or failure events, they
+must unregister the device from HID core and HID core will stop using the
+provided callbacks.
+
+1.2) Transport Driver Requirements
+----------------------------------
+
+The terms "asynchronous" and "synchronous" in this document describe the
+transmission behavior regarding acknowledgements. An asynchronous channel must
+not perform any synchronous operations like waiting for acknowledgements or
+verifications. Generally, HID calls operating on asynchronous channels must be
+running in atomic-context just fine.
+On the other hand, synchronous channels can be implemented by the transport
+driver in whatever way they like. They might just be the same as asynchronous
+channels, but they can also provide acknowledgement reports, automatic
+retransmission on failure, etc. in a blocking manner. If such functionality is
+required on asynchronous channels, a transport-driver must implement that via
+its own worker threads.
+
+HID core requires transport drivers to follow a given design. A Transport
+driver must provide two bi-directional I/O channels to each HID device. These
+channels must not necessarily be bi-directional in the hardware itself. A
+transport driver might just provide 4 uni-directional channels. Or it might
+multiplex all four on a single physical channel. However, in this document we
+will describe them as two bi-directional channels as they have several
+properties in common.
+
+ - Interrupt Channel (intr): The intr channel is used for asynchronous data
+ reports. No management commands or data acknowledgements are sent on this
+ channel. Any unrequested incoming or outgoing data report must be sent on
+ this channel and is never acknowledged by the remote side. Devices usually
+ send their input events on this channel. Outgoing events are normally
+ not send via intr, except if high throughput is required.
+ - Control Channel (ctrl): The ctrl channel is used for synchronous requests and
+ device management. Unrequested data input events must not be sent on this
+ channel and are normally ignored. Instead, devices only send management
+ events or answers to host requests on this channel.
+ The control-channel is used for direct blocking queries to the device
+ independent of any events on the intr-channel.
+ Outgoing reports are usually sent on the ctrl channel via synchronous
+ SET_REPORT requests.
+
+Communication between devices and HID core is mostly done via HID reports. A
+report can be of one of three types:
+
+ - INPUT Report: Input reports provide data from device to host. This
+ data may include button events, axis events, battery status or more. This
+ data is generated by the device and sent to the host with or without
+ requiring explicit requests. Devices can choose to send data continuously or
+ only on change.
+ - OUTPUT Report: Output reports change device states. They are sent from host
+ to device and may include LED requests, rumble requests or more. Output
+ reports are never sent from device to host, but a host can retrieve their
+ current state.
+ Hosts may choose to send output reports either continuously or only on
+ change.
+ - FEATURE Report: Feature reports are used for specific static device features
+ and never reported spontaneously. A host can read and/or write them to access
+ data like battery-state or device-settings.
+ Feature reports are never sent without requests. A host must explicitly set
+ or retrieve a feature report. This also means, feature reports are never sent
+ on the intr channel as this channel is asynchronous.
+
+INPUT and OUTPUT reports can be sent as pure data reports on the intr channel.
+For INPUT reports this is the usual operational mode. But for OUTPUT reports,
+this is rarely done as OUTPUT reports are normally quite scarce. But devices are
+free to make excessive use of asynchronous OUTPUT reports (for instance, custom
+HID audio speakers make great use of it).
+
+Plain reports must not be sent on the ctrl channel, though. Instead, the ctrl
+channel provides synchronous GET/SET_REPORT requests. Plain reports are only
+allowed on the intr channel and are the only means of data there.
+
+ - GET_REPORT: A GET_REPORT request has a report ID as payload and is sent
+ from host to device. The device must answer with a data report for the
+ requested report ID on the ctrl channel as a synchronous acknowledgement.
+ Only one GET_REPORT request can be pending for each device. This restriction
+ is enforced by HID core as several transport drivers don't allow multiple
+ simultaneous GET_REPORT requests.
+ Note that data reports which are sent as answer to a GET_REPORT request are
+ not handled as generic device events. That is, if a device does not operate
+ in continuous data reporting mode, an answer to GET_REPORT does not replace
+ the raw data report on the intr channel on state change.
+ GET_REPORT is only used by custom HID device drivers to query device state.
+ Normally, HID core caches any device state so this request is not necessary
+ on devices that follow the HID specs except during device initialization to
+ retrieve the current state.
+ GET_REPORT requests can be sent for any of the 3 report types and shall
+ return the current report state of the device. However, OUTPUT reports as
+ payload may be blocked by the underlying transport driver if the
+ specification does not allow them.
+ - SET_REPORT: A SET_REPORT request has a report ID plus data as payload. It is
+ sent from host to device and a device must update it's current report state
+ according to the given data. Any of the 3 report types can be used. However,
+ INPUT reports as payload might be blocked by the underlying transport driver
+ if the specification does not allow them.
+ A device must answer with a synchronous acknowledgement. However, HID core
+ does not require transport drivers to forward this acknowledgement to HID
+ core.
+ Same as for GET_REPORT, only one SET_REPORT can be pending at a time. This
+ restriction is enforced by HID core as some transport drivers do not support
+ multiple synchronous SET_REPORT requests.
+
+Other ctrl-channel requests are supported by USB-HID but are not available
+(or deprecated) in most other transport level specifications:
+
+ - GET/SET_IDLE: Only used by USB-HID and I2C-HID.
+ - GET/SET_PROTOCOL: Not used by HID core.
+ - RESET: Used by I2C-HID, not hooked up in HID core.
+ - SET_POWER: Used by I2C-HID, not hooked up in HID core.
+
+2) HID API
+==========
+
+2.1) Initialization
+-------------------
+
+Transport drivers normally use the following procedure to register a new device
+with HID core:
+
+ struct hid_device *hid;
+ int ret;
+
+ hid = hid_allocate_device();
+ if (IS_ERR(hid)) {
+ ret = PTR_ERR(hid);
+ goto err_<...>;
+ }
+
+ strlcpy(hid->name, <device-name-src>, 127);
+ strlcpy(hid->phys, <device-phys-src>, 63);
+ strlcpy(hid->uniq, <device-uniq-src>, 63);
+
+ hid->ll_driver = &custom_ll_driver;
+ hid->bus = <device-bus>;
+ hid->vendor = <device-vendor>;
+ hid->product = <device-product>;
+ hid->version = <device-version>;
+ hid->country = <device-country>;
+ hid->dev.parent = <pointer-to-parent-device>;
+ hid->driver_data = <transport-driver-data-field>;
+
+ ret = hid_add_device(hid);
+ if (ret)
+ goto err_<...>;
+
+Once hid_add_device() is entered, HID core might use the callbacks provided in
+"custom_ll_driver". Note that fields like "country" can be ignored by underlying
+transport-drivers if not supported.
+
+To unregister a device, use:
+
+ hid_destroy_device(hid);
+
+Once hid_destroy_device() returns, HID core will no longer make use of any
+driver callbacks.
+
+2.2) hid_ll_driver operations
+-----------------------------
+
+The available HID callbacks are:
+ - int (*start) (struct hid_device *hdev)
+ Called from HID device drivers once they want to use the device. Transport
+ drivers can choose to setup their device in this callback. However, normally
+ devices are already set up before transport drivers register them to HID core
+ so this is mostly only used by USB-HID.
+
+ - void (*stop) (struct hid_device *hdev)
+ Called from HID device drivers once they are done with a device. Transport
+ drivers can free any buffers and deinitialize the device. But note that
+ ->start() might be called again if another HID device driver is loaded on the
+ device.
+ Transport drivers are free to ignore it and deinitialize devices after they
+ destroyed them via hid_destroy_device().
+
+ - int (*open) (struct hid_device *hdev)
+ Called from HID device drivers once they are interested in data reports.
+ Usually, while user-space didn't open any input API/etc., device drivers are
+ not interested in device data and transport drivers can put devices asleep.
+ However, once ->open() is called, transport drivers must be ready for I/O.
+ ->open() calls are nested for each client that opens the HID device.
+
+ - void (*close) (struct hid_device *hdev)
+ Called from HID device drivers after ->open() was called but they are no
+ longer interested in device reports. (Usually if user-space closed any input
+ devices of the driver).
+ Transport drivers can put devices asleep and terminate any I/O of all
+ ->open() calls have been followed by a ->close() call. However, ->start() may
+ be called again if the device driver is interested in input reports again.
+
+ - int (*parse) (struct hid_device *hdev)
+ Called once during device setup after ->start() has been called. Transport
+ drivers must read the HID report-descriptor from the device and tell HID core
+ about it via hid_parse_report().
+
+ - int (*power) (struct hid_device *hdev, int level)
+ Called by HID core to give PM hints to transport drivers. Usually this is
+ analogical to the ->open() and ->close() hints and redundant.
+
+ - void (*request) (struct hid_device *hdev, struct hid_report *report,
+ int reqtype)
+ Send an HID request on the ctrl channel. "report" contains the report that
+ should be sent and "reqtype" the request type. Request-type can be
+ HID_REQ_SET_REPORT or HID_REQ_GET_REPORT.
+ This callback is optional. If not provided, HID core will assemble a raw
+ report following the HID specs and send it via the ->raw_request() callback.
+ The transport driver is free to implement this asynchronously.
+
+ - int (*wait) (struct hid_device *hdev)
+ Used by HID core before calling ->request() again. A transport driver can use
+ it to wait for any pending requests to complete if only one request is
+ allowed at a time.
+
+ - int (*raw_request) (struct hid_device *hdev, unsigned char reportnum,
+ __u8 *buf, size_t count, unsigned char rtype,
+ int reqtype)
+ Same as ->request() but provides the report as raw buffer. This request shall
+ be synchronous. A transport driver must not use ->wait() to complete such
+ requests.
+
+ - int (*output_report) (struct hid_device *hdev, __u8 *buf, size_t len)
+ Send raw output report via intr channel. Used by some HID device drivers
+ which require high throughput for outgoing requests on the intr channel. This
+ must not cause SET_REPORT calls! This must be implemented as asynchronous
+ output report on the intr channel!
+
+ - int (*idle) (struct hid_device *hdev, int report, int idle, int reqtype)
+ Perform SET/GET_IDLE request. Only used by USB-HID, do not implement!
+
+2.3) Data Path
+--------------
+
+Transport drivers are responsible of reading data from I/O devices. They must
+handle any I/O-related state-tracking themselves. HID core does not implement
+protocol handshakes or other management commands which can be required by the
+given HID transport specification.
+
+Every raw data packet read from a device must be fed into HID core via
+hid_input_report(). You must specify the channel-type (intr or ctrl) and report
+type (input/output/feature). Under normal conditions, only input reports are
+provided via this API.
+
+Responses to GET_REPORT requests via ->request() must also be provided via this
+API. Responses to ->raw_request() are synchronous and must be intercepted by the
+transport driver and not passed to hid_input_report().
+Acknowledgements to SET_REPORT requests are not of interest to HID core.
+
+----------------------------------------------------
+Written 2013, David Herrmann <dh.herrmann@gmail.com>
--
1.8.3.1
^ permalink raw reply related
* [PATCH v3 0/6] HID: sony: Add full Bluetooth support for the Dualshock 4
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
v3 of this patch series rebases the code against Benjamin Tissoires' HID
transport layer changes to use the safe inline hid_hw_* functions which
eliminates the need to check the function pointers manually.
It adds an explicit Bluetooth initialization function instead of the controller
input report state being set as a side effect of initializing the LED system.
It also uses a new DUALSHOCK4_CONTROLLER macro to simplify conditional cases
where the connection type is irrelevant.
^ permalink raw reply
* [PATCH v3 1/6] HID: sony: Use low-level transport driver functions
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
Switch to the low-level transport driver functions.
sony_set_output_report is removed since it is no longer used.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v3 uses the inline hid_hw_output_report function
drivers/hid/hid-sony.c | 56 +++++++++++++-------------------------------------
1 file changed, 14 insertions(+), 42 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 2bd3f13..71fd17f 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -502,7 +502,6 @@ struct sony_sc {
spinlock_t lock;
struct hid_device *hdev;
struct led_classdev *leds[MAX_LEDS];
- struct hid_report *output_report;
unsigned long quirks;
struct work_struct state_worker;
struct power_supply battery;
@@ -1053,21 +1052,26 @@ static void dualshock4_state_worker(struct work_struct *work)
{
struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
struct hid_device *hdev = sc->hdev;
- struct hid_report *report = sc->output_report;
- __s32 *value = report->field[0]->value;
+ int offset;
+
+ __u8 buf[32] = { 0 };
- value[0] = 0x03;
+ buf[0] = 0x05;
+ buf[1] = 0x03;
+ offset = 4;
#ifdef CONFIG_SONY_FF
- value[3] = sc->right;
- value[4] = sc->left;
+ buf[offset++] = sc->right;
+ buf[offset++] = sc->left;
+#else
+ offset += 2;
#endif
- value[5] = sc->led_state[0];
- value[6] = sc->led_state[1];
- value[7] = sc->led_state[2];
+ buf[offset++] = sc->led_state[0];
+ buf[offset++] = sc->led_state[1];
+ buf[offset++] = sc->led_state[2];
- hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
+ hid_hw_output_report(hdev, buf, sizeof(buf));
}
#ifdef CONFIG_SONY_FF
@@ -1200,33 +1204,6 @@ static void sony_battery_remove(struct sony_sc *sc)
sc->battery.name = NULL;
}
-static int sony_set_output_report(struct sony_sc *sc, int req_id, int req_size)
-{
- struct list_head *head, *list;
- struct hid_report *report;
- struct hid_device *hdev = sc->hdev;
-
- list = &hdev->report_enum[HID_OUTPUT_REPORT].report_list;
-
- list_for_each(head, list) {
- report = list_entry(head, struct hid_report, list);
-
- if (report->id == req_id) {
- if (report->size < req_size) {
- hid_err(hdev, "Output report 0x%02x (%i bits) is smaller than requested size (%i bits)\n",
- req_id, report->size, req_size);
- return -EINVAL;
- }
- sc->output_report = report;
- return 0;
- }
- }
-
- hid_err(hdev, "Unable to locate output report 0x%02x\n", req_id);
-
- return -EINVAL;
-}
-
static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
int w, int h)
{
@@ -1291,11 +1268,6 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
ret = sixaxis_set_operational_bt(hdev);
else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
- /* Report 5 (31 bytes) is used to send data to the controller via USB */
- ret = sony_set_output_report(sc, 0x05, 248);
- if (ret < 0)
- goto err_stop;
-
/* The Dualshock 4 touchpad supports 2 touches and has a
* resolution of 1920x940.
*/
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 3/6] HID: sony: Add Dualshock 4 Bluetooth output report formatting
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
Add formating for the Dualshock 4 output report data in Bluetooth mode.
In Bluetooth mode the Dualshock 4 wants output reports sent on the control
channel.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v3 uses the inline hid_hw_raw_request inline function
drivers/hid/hid-sony.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index e9775a1..59cd9d7 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1268,11 +1268,18 @@ static void dualshock4_state_worker(struct work_struct *work)
struct hid_device *hdev = sc->hdev;
int offset;
- __u8 buf[32] = { 0 };
+ __u8 buf[78] = { 0 };
- buf[0] = 0x05;
- buf[1] = 0x03;
- offset = 4;
+ if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ buf[0] = 0x05;
+ buf[1] = 0x03;
+ offset = 4;
+ } else {
+ buf[0] = 0x11;
+ buf[1] = 0xB0;
+ buf[3] = 0x0F;
+ offset = 6;
+ }
#ifdef CONFIG_SONY_FF
buf[offset++] = sc->right;
@@ -1285,7 +1292,11 @@ static void dualshock4_state_worker(struct work_struct *work)
buf[offset++] = sc->led_state[1];
buf[offset++] = sc->led_state[2];
- hid_hw_output_report(hdev, buf, sizeof(buf));
+ if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
+ hid_hw_output_report(hdev, buf, 32);
+ else
+ hid_hw_raw_request(hdev, 0x11, buf, 78,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
}
#ifdef CONFIG_SONY_FF
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 4/6] HID: sony: Add Dualshock 4 Bluetooth battery and touchpad parsing
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
Add Dualshock 4 battery and touchpad parsing for Bluetooth reports.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v2 adds more robust battery data parsing
drivers/hid/hid-sony.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 59cd9d7..4b8cb98 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -861,25 +861,34 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
struct hid_input, list);
struct input_dev *input_dev = hidinput->input;
unsigned long flags;
- int n, offset = 35;
+ int n, offset;
__u8 cable_state, battery_capacity, battery_charging;
+ /* Battery and touchpad data starts at byte 30 in the USB report and
+ * 32 in Bluetooth report.
+ */
+ offset = (sc->quirks & DUALSHOCK4_CONTROLLER_USB) ? 30 : 32;
+
/* The lower 4 bits of byte 30 contain the battery level
* and the 5th bit contains the USB cable state.
*/
- cable_state = (rd[30] >> 4) & 0x01;
- battery_capacity = rd[30] & 0x0F;
+ cable_state = (rd[offset] >> 4) & 0x01;
+ battery_capacity = rd[offset] & 0x0F;
- /* On USB the Dualshock 4 battery level goes from 0 to 11.
- * A battery level of 11 means fully charged.
+ /* When a USB power source is connected the battery level ranges from
+ * 0 to 10, and when running on battery power it ranges from 0 to 9.
+ * A battery level above 10 when plugged in means charge completed.
*/
- if (cable_state && battery_capacity == 11)
+ if (!cable_state || battery_capacity > 10)
battery_charging = 0;
else
battery_charging = 1;
+ if (!cable_state)
+ battery_capacity++;
if (battery_capacity > 10)
- battery_capacity--;
+ battery_capacity = 10;
+
battery_capacity *= 10;
spin_lock_irqsave(&sc->lock, flags);
@@ -888,7 +897,10 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
sc->battery_charging = battery_charging;
spin_unlock_irqrestore(&sc->lock, flags);
- /* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB.
+ offset += 5;
+
+ /* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB
+ * and 37 on Bluetooth.
* The first 7 bits of the first byte is a counter and bit 8 is a touch
* indicator that is 0 when pressed and 1 when not pressed.
* The next 3 bytes are two 12 bit touch coordinates, X and Y.
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 2/6] HID: sony: Add modified Dualshock 4 Bluetooth HID descriptor
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
By default, the Dualshock 4 sends controller data via report 1. Once a valid
output report 0x11 is received or a feature report of type 0x02 is requested the
controller changes from sending data in report 1 to sending data in report 17,
which is unmapped in the default descriptor. The mappings have to be moved to
report 17 to let the HID driver properly process the incoming reports.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 214 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 214 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 71fd17f..e9775a1 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -336,6 +336,216 @@ static u8 dualshock4_usb_rdesc[] = {
0xC0 /* End Collection */
};
+/* The default behavior of the Dualshock 4 is to send reports using report
+ * type 1 when running over Bluetooth. However, as soon as it receives a
+ * report of type 17 to set the LEDs or rumble it starts returning it's state
+ * in report 17 instead of 1. Since report 17 is undefined in the default HID
+ * descriptor the button and axis definitions must be moved to report 17 or
+ * the HID layer won't process the received input once a report is sent.
+ */
+static u8 dualshock4_bt_rdesc[] = {
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x05, /* Usage (Gamepad), */
+ 0xA1, 0x01, /* Collection (Application), */
+ 0x85, 0x01, /* Report ID (1), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x0A, /* Report Count (9), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x06, 0x04, 0xFF, /* Usage Page (FF04h), */
+ 0x85, 0x02, /* Report ID (2), */
+ 0x09, 0x24, /* Usage (24h), */
+ 0x95, 0x24, /* Report Count (36), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xA3, /* Report ID (163), */
+ 0x09, 0x25, /* Usage (25h), */
+ 0x95, 0x30, /* Report Count (48), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x05, /* Report ID (5), */
+ 0x09, 0x26, /* Usage (26h), */
+ 0x95, 0x28, /* Report Count (40), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x06, /* Report ID (6), */
+ 0x09, 0x27, /* Usage (27h), */
+ 0x95, 0x34, /* Report Count (52), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x07, /* Report ID (7), */
+ 0x09, 0x28, /* Usage (28h), */
+ 0x95, 0x30, /* Report Count (48), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x08, /* Report ID (8), */
+ 0x09, 0x29, /* Usage (29h), */
+ 0x95, 0x2F, /* Report Count (47), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x06, 0x03, 0xFF, /* Usage Page (FF03h), */
+ 0x85, 0x03, /* Report ID (3), */
+ 0x09, 0x21, /* Usage (21h), */
+ 0x95, 0x26, /* Report Count (38), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x04, /* Report ID (4), */
+ 0x09, 0x22, /* Usage (22h), */
+ 0x95, 0x2E, /* Report Count (46), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xF0, /* Report ID (240), */
+ 0x09, 0x47, /* Usage (47h), */
+ 0x95, 0x3F, /* Report Count (63), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xF1, /* Report ID (241), */
+ 0x09, 0x48, /* Usage (48h), */
+ 0x95, 0x3F, /* Report Count (63), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xF2, /* Report ID (242), */
+ 0x09, 0x49, /* Usage (49h), */
+ 0x95, 0x0F, /* Report Count (15), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x11, /* Report ID (17), */
+ 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+ 0x09, 0x20, /* Usage (20h), */
+ 0x95, 0x02, /* Report Count (2), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x30, /* Usage (X), */
+ 0x09, 0x31, /* Usage (Y), */
+ 0x09, 0x32, /* Usage (Z), */
+ 0x09, 0x35, /* Usage (Rz), */
+ 0x15, 0x00, /* Logical Minimum (0), */
+ 0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x04, /* Report Count (4), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x39, /* Usage (Hat Switch), */
+ 0x15, 0x00, /* Logical Minimum (0), */
+ 0x25, 0x07, /* Logical Maximum (7), */
+ 0x75, 0x04, /* Report Size (4), */
+ 0x95, 0x01, /* Report Count (1), */
+ 0x81, 0x42, /* Input (Variable, Null State), */
+ 0x05, 0x09, /* Usage Page (Button), */
+ 0x19, 0x01, /* Usage Minimum (01h), */
+ 0x29, 0x0E, /* Usage Maximum (0Eh), */
+ 0x15, 0x00, /* Logical Minimum (0), */
+ 0x25, 0x01, /* Logical Maximum (1), */
+ 0x75, 0x01, /* Report Size (1), */
+ 0x95, 0x0E, /* Report Count (14), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x75, 0x06, /* Report Size (6), */
+ 0x95, 0x01, /* Report Count (1), */
+ 0x81, 0x01, /* Input (Constant), */
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x09, 0x33, /* Usage (Rx), */
+ 0x09, 0x34, /* Usage (Ry), */
+ 0x15, 0x00, /* Logical Minimum (0), */
+ 0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x02, /* Report Count (2), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+ 0x09, 0x20, /* Usage (20h), */
+ 0x95, 0x03, /* Report Count (3), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x05, 0x01, /* Usage Page (Desktop), */
+ 0x19, 0x40, /* Usage Minimum (40h), */
+ 0x29, 0x42, /* Usage Maximum (42h), */
+ 0x16, 0x00, 0x80, /* Logical Minimum (-32768), */
+ 0x26, 0x00, 0x7F, /* Logical Maximum (32767), */
+ 0x75, 0x10, /* Report Size (16), */
+ 0x95, 0x03, /* Report Count (3), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x19, 0x43, /* Usage Minimum (43h), */
+ 0x29, 0x45, /* Usage Maximum (45h), */
+ 0x16, 0xFF, 0xBF, /* Logical Minimum (-16385), */
+ 0x26, 0x00, 0x40, /* Logical Maximum (16384), */
+ 0x95, 0x03, /* Report Count (3), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */
+ 0x09, 0x20, /* Usage (20h), */
+ 0x15, 0x00, /* Logical Minimum (0), */
+ 0x26, 0xFF, 0x00, /* Logical Maximum (255), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x31, /* Report Count (51), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x21, /* Usage (21h), */
+ 0x75, 0x08, /* Report Size (8), */
+ 0x95, 0x4D, /* Report Count (77), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x12, /* Report ID (18), */
+ 0x09, 0x22, /* Usage (22h), */
+ 0x95, 0x8D, /* Report Count (141), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x23, /* Usage (23h), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x13, /* Report ID (19), */
+ 0x09, 0x24, /* Usage (24h), */
+ 0x95, 0xCD, /* Report Count (205), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x25, /* Usage (25h), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x14, /* Report ID (20), */
+ 0x09, 0x26, /* Usage (26h), */
+ 0x96, 0x0D, 0x01, /* Report Count (269), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x27, /* Usage (27h), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x15, /* Report ID (21), */
+ 0x09, 0x28, /* Usage (28h), */
+ 0x96, 0x4D, 0x01, /* Report Count (333), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x29, /* Usage (29h), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x16, /* Report ID (22), */
+ 0x09, 0x2A, /* Usage (2Ah), */
+ 0x96, 0x8D, 0x01, /* Report Count (397), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x2B, /* Usage (2Bh), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x17, /* Report ID (23), */
+ 0x09, 0x2C, /* Usage (2Ch), */
+ 0x96, 0xCD, 0x01, /* Report Count (461), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x2D, /* Usage (2Dh), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x18, /* Report ID (24), */
+ 0x09, 0x2E, /* Usage (2Eh), */
+ 0x96, 0x0D, 0x02, /* Report Count (525), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x2F, /* Usage (2Fh), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x85, 0x19, /* Report ID (25), */
+ 0x09, 0x30, /* Usage (30h), */
+ 0x96, 0x22, 0x02, /* Report Count (546), */
+ 0x81, 0x02, /* Input (Variable), */
+ 0x09, 0x31, /* Usage (31h), */
+ 0x91, 0x02, /* Output (Variable), */
+ 0x06, 0x80, 0xFF, /* Usage Page (FF80h), */
+ 0x85, 0x82, /* Report ID (130), */
+ 0x09, 0x22, /* Usage (22h), */
+ 0x95, 0x3F, /* Report Count (63), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x83, /* Report ID (131), */
+ 0x09, 0x23, /* Usage (23h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x84, /* Report ID (132), */
+ 0x09, 0x24, /* Usage (24h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x90, /* Report ID (144), */
+ 0x09, 0x30, /* Usage (30h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x91, /* Report ID (145), */
+ 0x09, 0x31, /* Usage (31h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x92, /* Report ID (146), */
+ 0x09, 0x32, /* Usage (32h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0x93, /* Report ID (147), */
+ 0x09, 0x33, /* Usage (33h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xA0, /* Report ID (160), */
+ 0x09, 0x40, /* Usage (40h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0x85, 0xA4, /* Report ID (164), */
+ 0x09, 0x44, /* Usage (44h), */
+ 0xB1, 0x02, /* Feature (Variable), */
+ 0xC0 /* End Collection */
+};
+
static __u8 ps3remote_rdesc[] = {
0x05, 0x01, /* GUsagePage Generic Desktop */
0x09, 0x05, /* LUsage 0x05 [Game Pad] */
@@ -591,6 +801,10 @@ static __u8 *sony_report_fixup(struct hid_device *hdev, __u8 *rdesc,
hid_info(hdev, "Using modified Dualshock 4 report descriptor with gyroscope axes\n");
rdesc = dualshock4_usb_rdesc;
*rsize = sizeof(dualshock4_usb_rdesc);
+ } else if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) && *rsize == 357) {
+ hid_info(hdev, "Using modified Dualshock 4 Bluetooth report descriptor\n");
+ rdesc = dualshock4_bt_rdesc;
+ *rsize = sizeof(dualshock4_bt_rdesc);
}
/* The HID descriptor exposed over BT has a trailing zero byte */
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 5/6] HID: sony: Set initial battery level to 100% to avoid false low battery warnings
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
Set the initial battery level to 100% to avoid false low battery warnings if
the battery state is polled before a report with the actual battery level is
received.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
drivers/hid/hid-sony.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 4b8cb98..84633bf 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1404,6 +1404,11 @@ static int sony_battery_probe(struct sony_sc *sc)
struct hid_device *hdev = sc->hdev;
int ret;
+ /* Set the default battery level to 100% to avoid low battery warnings
+ * if the battery is polled before the first device report is received.
+ */
+ sc->battery_capacity = 100;
+
power_id = (unsigned long)atomic_inc_return(&power_id_seq);
sc->battery.properties = sony_battery_props;
--
1.8.5.3
^ permalink raw reply related
* [PATCH v3 6/6] HID: sony: Add conditionals to enable all features in Bluetooth mode
From: Frank Praznik @ 2014-02-06 1:03 UTC (permalink / raw)
To: linux-input; +Cc: jkosina, dh.herrmann, Frank Praznik
In-Reply-To: <1391648629-3077-1-git-send-email-frank.praznik@oh.rr.com>
Add the conditionals to enable rumble, battery reporting, LED and touchpad
support for the Dualshock 4 in Bluetooth mode.
Add dualshock4_set_operational_bt to initialize the controller to the proper
operational state.
Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
v3 adds a DUALSHOCK4_CONTROLLER macro for conditional cases where the
connection type is irrelevant as well as dualshock4_set_operational_bt to
explicitly initialize the controller instead of the device state being set as a
side effect of initializing the LED system.
drivers/hid/hid-sony.c | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 84633bf..1b150d8 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -44,8 +44,12 @@
#define DUALSHOCK4_CONTROLLER_USB BIT(5)
#define DUALSHOCK4_CONTROLLER_BT BIT(6)
-#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER | DUALSHOCK4_CONTROLLER_USB)
-#define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT | DUALSHOCK4_CONTROLLER_USB)
+#define DUALSHOCK4_CONTROLLER (DUALSHOCK4_CONTROLLER_USB |\
+ DUALSHOCK4_CONTROLLER_BT)
+#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER |\
+ DUALSHOCK4_CONTROLLER)
+#define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT |\
+ DUALSHOCK4_CONTROLLER)
#define MAX_LEDS 4
@@ -939,8 +943,9 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
swap(rd[47], rd[48]);
sixaxis_parse_report(sc, rd, size);
- } else if ((sc->quirks & DUALSHOCK4_CONTROLLER_USB) && rd[0] == 0x01 &&
- size == 64) {
+ } else if (((sc->quirks & DUALSHOCK4_CONTROLLER_USB) && rd[0] == 0x01 &&
+ size == 64) || ((sc->quirks & DUALSHOCK4_CONTROLLER_BT)
+ && rd[0] == 0x11 && size == 78)) {
dualshock4_parse_report(sc, rd, size);
}
@@ -1051,6 +1056,17 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
return hdev->hid_output_raw_report(hdev, buf, sizeof(buf), HID_FEATURE_REPORT);
}
+/* Requesting feature report 0x02 in Bluetooth mode changes the state of the
+ * controller so that it sends full input reports of type 0x11.
+ */
+static int dualshock4_set_operational_bt(struct hid_device *hdev)
+{
+ __u8 buf[37] = { 0 };
+
+ return hid_hw_raw_request(hdev, 0x02, buf, sizeof(buf),
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+}
+
static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
{
struct list_head *report_list =
@@ -1079,7 +1095,7 @@ static void sony_set_leds(struct hid_device *hdev, const __u8 *leds, int count)
if (drv_data->quirks & BUZZ_CONTROLLER && count == 4) {
buzz_set_leds(hdev, leds);
} else if ((drv_data->quirks & SIXAXIS_CONTROLLER_USB) ||
- (drv_data->quirks & DUALSHOCK4_CONTROLLER_USB)) {
+ (drv_data->quirks & DUALSHOCK4_CONTROLLER)) {
for (n = 0; n < count; n++)
drv_data->led_state[n] = leds[n];
schedule_work(&drv_data->state_worker);
@@ -1184,7 +1200,7 @@ static int sony_leds_init(struct hid_device *hdev)
/* Validate expected report characteristics. */
if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
return -ENODEV;
- } else if (drv_data->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ } else if (drv_data->quirks & DUALSHOCK4_CONTROLLER) {
drv_data->led_count = 3;
max_brightness = 255;
use_colors = 1;
@@ -1509,7 +1525,14 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
}
else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
ret = sixaxis_set_operational_bt(hdev);
- else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+ else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
+ if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) {
+ ret = dualshock4_set_operational_bt(hdev);
+ if (ret < 0) {
+ hid_err(hdev, "failed to set the Dualshock 4 operational mode\n");
+ goto err_stop;
+ }
+ }
/* The Dualshock 4 touchpad supports 2 touches and has a
* resolution of 1920x940.
*/
--
1.8.5.3
^ permalink raw reply related
* Re: [PATCH 01/15] Input: synaptics-rmi4 - fix checkpatch.pl, sparse and GCC warnings
From: Dmitry Torokhov @ 2014-02-06 1:09 UTC (permalink / raw)
To: Christopher Heiny; +Cc: Courtney Cavin, linux-input
In-Reply-To: <52F172DC.5090900@synaptics.com>
On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote:
> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >Cc: Christopher Heiny <cheiny@synaptics.com>
> >Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
> >---
> > drivers/input/rmi4/rmi_bus.c | 4 ++--
> > drivers/input/rmi4/rmi_bus.h | 2 +-
> > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++-----
> > drivers/input/rmi4/rmi_f11.c | 4 +++-
> > 4 files changed, 18 insertions(+), 9 deletions(-)
> >
> >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
> >index 96a76e7..8a939f3 100644
> >--- a/drivers/input/rmi4/rmi_bus.c
> >+++ b/drivers/input/rmi4/rmi_bus.c
> >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev)
> > kfree(rmi_dev);
> > }
> >
> >-struct device_type rmi_device_type = {
> >+static struct device_type rmi_device_type = {
> > .name = "rmi_sensor",
> > .release = rmi_release_device,
> > };
>
> This struct is used by diagnostic modules to identify sensor
> devices, so it cannot be static.
Then we need to declare it somewhere or provide an accessor function.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3] input synaptics-rmi4: PDT scan cleanup
From: Dmitry Torokhov @ 2014-02-06 1:13 UTC (permalink / raw)
To: Christopher Heiny
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires, David Herrmann, Jiri Kosina
In-Reply-To: <1391563997-11155-1-git-send-email-cheiny@synaptics.com>
On Tue, Feb 04, 2014 at 05:33:17PM -0800, Christopher Heiny wrote:
> This builds on Dmitry's patch of 2014-01-27, with some bug fixes. It's been
> tested on a Nexus 4.
>
> Eliminates copy-paste code that handled scans of the Page Descriptor
> Table, replacing it with a single PDT scan routine that invokes a
> callback function.
>
> Updated the copyright dates and eliminate C++ style comments while we
> were at it.
>
> Signed-off-by: Christopher Heiny <cheiny@synaptics.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 15/15] Input: synaptics-rmi4 - correct RMI4 spec url
From: Dmitry Torokhov @ 2014-02-06 1:14 UTC (permalink / raw)
To: Christopher Heiny; +Cc: Courtney Cavin, linux-input
In-Reply-To: <52F1737E.4080509@synaptics.com>
On Tue, Feb 04, 2014 at 03:10:54PM -0800, Christopher Heiny wrote:
> On 01/23/2014 04:00 PM, Courtney Cavin wrote:
> >Cc: Christopher Heiny <cheiny@synaptics.com>
> >Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >Signed-off-by: Courtney Cavin <courtney.cavin@sonymobile.com>
>
> Acked-by: Christopher Heiny <cheiny@synaptics.com>
Applied, thank you.
>
> >---
> > drivers/input/rmi4/rmi_driver.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> >diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
> >index 9d8d6ec..9d845a4 100644
> >--- a/drivers/input/rmi4/rmi_driver.c
> >+++ b/drivers/input/rmi4/rmi_driver.c
> >@@ -7,7 +7,7 @@
> > * The RMI4 specification can be found here (URL split for line length):
> > *
> > * http://www.synaptics.com/sites/default/files/
> >- * 511-000136-01-Rev-E-RMI4%20Intrfacing%20Guide.pdf
> >+ * 511-000136-01-Rev-E-RMI4-Interfacing-Guide.pdf
> > *
> > * This program is free software; you can redistribute it and/or modify it
> > * under the terms of the GNU General Public License version 2 as published by
> >
>
>
> --
>
> Christopher Heiny
> Senior Staff Firmware Engineer
> Synaptics Incorporated
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: synaptics-rmi4 - Count IRQs before creating functions; save F01 container.
From: Christopher Heiny @ 2014-02-06 1:28 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Linux Input, Andrew Duggan, Vincent Huang, Vivian Ly,
Daniel Rosenberg, Jean Delvare, Joerie de Gram, Linus Walleij,
Benjamin Tissoires
In-Reply-To: <20140129225241.GA14709@core.coreip.homeip.net>
On 01/29/2014 02:52 PM, Dmitry Torokhov wrote:
> On Wed, Jan 29, 2014 at 10:30:48PM +0000, Christopher Heiny wrote:
>> Sorry for the delay on this. The mail problems from earlier this week continue to plague me.
>>
>> On 01/26/2014 10:36 PM, Dmitry Torokhov wrote:
>>> Hi Christopher,
>>>
>>> On Fri, Jan 10, 2014 at 01:53:34PM -0800, Christopher Heiny wrote:
>>>>
>>>> err_free_data:
>>>> + rmi_free_function_list(rmi_dev);
>>>> + if (gpio_is_valid(pdata->attn_gpio))
>>>> + gpio_free(pdata->attn_gpio);
>>>> + devm_kfree(&rmi_dev->dev, data->irq_status);
>>>> + devm_kfree(&rmi_dev->dev, data->current_irq_mask);
>>>> + devm_kfree(&rmi_dev->dev, data->irq_mask_store);
>>>> + devm_kfree(&rmi_dev->dev, data);
>>>
>>> It is rarely makes sense to explicitly free devm-managed data. In
>>> general I find that RMI code is very confused about when devm-managed
>>> resources are released (they only released after failed probe or remove,
>>> but we use devm_kzalloc to allocate function's interrupt masks during
>>> device creation and they will get freed only when parent unbinds, etc).
>>
>
>> Yeah, we've gotten a metric ton of confusing advice/recommendations
>> regarding devm_* (most of it offline from linux-input) and it shows.
>> At one point I was pretty much ready to just bag it all and write our
>> own garbage collecting storage manager, but figured that would be
>> unlikely to make it upstream :-)
>
> Yeah, some people see mistake screws for nails when they get a hold of a
> hammer. Managed resources are nice as long as you have clear
> understanding on what happens.
>
>>
>>> Given that you mentioned firmware flash in the work and I expect we'll
>>> be destroying and re-creating functions and other data structures at
>>> will I think we should limit use of devm APIs so that lifetime
>>> management is explicit and clear.
>>
>> Sounds good.
>>
>>> I tried adjusting the patch so that it works with the version of PDT
>>> cleanup patch I posted a few minutes ago, please let me know what you
>>> think.
>>
>> There's some comments below. After making those changes, I've applied this to my test tree, and it works well.
>>
>> I can send updated versions of your two patches, if you'd like.
>
> Yes, please, I always prefer applying something that was tested. You
> can also sent more of the pending stuff my way, no need to limit to 1
> patch at a time - I usually able to cherry-pick patches that I do not
> have questions about and we can work on ones that I need some
> clarification on. Just don't mailbomb me with 100 ;)
OK - we'll have some more on the way in a bit.
>>> +static int rmi_check_bootloader_mode(struct rmi_device *rmi_dev,
>>> + const struct pdt_entry *pdt)
>>> +{
>>> + int error;
>>> + u8 device_status;
>>> +
>>> + error = rmi_read(rmi_dev, pdt->data_base_addr + pdt->page_start,
>>> + &device_status);
>>
>> Since this is applied after your previous patch, then this statement should be:
>> error = rmi_read(rmi_dev, pdt->data_base_addr, &device_status);
>
> Hmm, I did not think I adjusted data_base_addr in PDT, I only stored the
> page start in there... The updated addresses as in function
> structures.
I went back and double checked. This was correct, it was page_start
that was bogus. Yesterday's patch fixed that.
^ permalink raw reply
* Re: [PATCH 01/15] Input: synaptics-rmi4 - fix checkpatch.pl, sparse and GCC warnings
From: Christopher Heiny @ 2014-02-06 1:36 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Courtney Cavin, linux-input
In-Reply-To: <20140206010938.GA27770@core.coreip.homeip.net>
On 02/05/2014 05:09 PM, Dmitry Torokhov wrote:
> On Tue, Feb 04, 2014 at 03:08:12PM -0800, Christopher Heiny wrote:
>> >On 01/23/2014 04:00 PM, Courtney Cavin wrote:
>>> > >Cc: Christopher Heiny<cheiny@synaptics.com>
>>> > >Cc: Dmitry Torokhov<dmitry.torokhov@gmail.com>
>>> > >Signed-off-by: Courtney Cavin<courtney.cavin@sonymobile.com>
>>> > >---
>>> > > drivers/input/rmi4/rmi_bus.c | 4 ++--
>>> > > drivers/input/rmi4/rmi_bus.h | 2 +-
>>> > > drivers/input/rmi4/rmi_driver.c | 17 ++++++++++++-----
>>> > > drivers/input/rmi4/rmi_f11.c | 4 +++-
>>> > > 4 files changed, 18 insertions(+), 9 deletions(-)
>>> > >
>>> > >diff --git a/drivers/input/rmi4/rmi_bus.c b/drivers/input/rmi4/rmi_bus.c
>>> > >index 96a76e7..8a939f3 100644
>>> > >--- a/drivers/input/rmi4/rmi_bus.c
>>> > >+++ b/drivers/input/rmi4/rmi_bus.c
>>> > >@@ -37,7 +37,7 @@ static void rmi_release_device(struct device *dev)
>>> > > kfree(rmi_dev);
>>> > > }
>>> > >
>>> > >-struct device_type rmi_device_type = {
>>> > >+static struct device_type rmi_device_type = {
>>> > > .name = "rmi_sensor",
>>> > > .release = rmi_release_device,
>>> > > };
>> >
>> >This struct is used by diagnostic modules to identify sensor
>> >devices, so it cannot be static.
>
> Then we need to declare it somewhere or provide an accessor function.
Currently it's in a header not included in the patches. We'll move it
to rmi_bus.h.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox