[PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver

[PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver

[Frank Praznik]

v2 of this patch set addresses the code review issues raised in v1 as well as
adding a patch that fixes the styling of multi-line comments to conform to
kernel coding standards, adds a check to protect against a potential
out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
that a device module now depends on the current behavior of the uniq string.

[PATCH v2 1/5] HID: sony: Fix multi-line comment styling

[Frank Praznik]

Convert multi-line comments to comply with the kernel coding style.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 drivers/hid/hid-sony.c | 45 ++++++++++++++++++++++++++++++---------------
 1 file changed, 30 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 526705f..26992e1 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -17,7 +17,8 @@
  * any later version.
  */
 
-/* NOTE: in order for the Sony PS3 BD Remote Control to be found by
+/*
+ * NOTE: in order for the Sony PS3 BD Remote Control to be found by
  * a Bluetooth host, the key combination Start+Enter has to be kept pressed
  * for about 7 seconds with the Bluetooth Host Controller in discovering mode.
  *
@@ -81,7 +82,8 @@ static const u8 sixaxis_rdesc_fixup2[] = {
 	0xb1, 0x02, 0xc0, 0xc0,
 };
 
-/* The default descriptor doesn't provide mapping for the accelerometers
+/*
+ * The default descriptor doesn't provide mapping for the accelerometers
  * or orientation sensors.  This fixed descriptor maps the accelerometers
  * to usage values 0x40, 0x41 and 0x42 and maps the orientation sensors
  * to usage values 0x43, 0x44 and 0x45.
@@ -340,7 +342,8 @@ static u8 dualshock4_usb_rdesc[] = {
 	0xC0                /*  End Collection                      */
 };
 
-/* The default behavior of the Dualshock 4 is to send reports using report
+/*
+ * 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
@@ -667,7 +670,8 @@ static const unsigned int ps3remote_keymap_remote_buttons[] = {
 };
 
 static const unsigned int buzz_keymap[] = {
-	/* The controller has 4 remote buzzers, each with one LED and 5
+	/*
+	 * The controller has 4 remote buzzers, each with one LED and 5
 	 * buttons.
 	 * 
 	 * We use the mapping chosen by the controller, which is:
@@ -838,7 +842,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
 	unsigned long flags;
 	__u8 cable_state, battery_capacity, battery_charging;
 
-	/* The sixaxis is charging if the battery value is 0xee
+	/*
+	 * The sixaxis is charging if the battery value is 0xee
 	 * and it is fully charged if the value is 0xef.
 	 * It does not report the actual level while charging so it
 	 * is set to 100% while charging is in progress.
@@ -868,18 +873,21 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
 	int n, offset;
 	__u8 cable_state, battery_capacity, battery_charging;
 
-	/* Battery and touchpad data starts at byte 30 in the USB report and
+	/*
+	 * 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
+	/*
+	 * The lower 4 bits of byte 30 contain the battery level
 	 * and the 5th bit contains the USB cable state.
 	 */
 	cable_state = (rd[offset] >> 4) & 0x01;
 	battery_capacity = rd[offset] & 0x0F;
 
-	/* When a USB power source is connected the battery level ranges from
+	/*
+	 * 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.
 	 */
@@ -903,7 +911,8 @@ static void dualshock4_parse_report(struct sony_sc *sc, __u8 *rd, int size)
 
 	offset += 5;
 
-	/* The Dualshock 4 multi-touch trackpad data starts at offset 35 on USB
+	/*
+	 * 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.
@@ -932,7 +941,8 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	/* Sixaxis HID report has acclerometers/gyro with MSByte first, this
+	/*
+	 * Sixaxis HID report has acclerometers/gyro with MSByte first, this
 	 * has to be BYTE_SWAPPED before passing up to joystick interface
 	 */
 	if ((sc->quirks & SIXAXIS_CONTROLLER) && rd[0] == 0x01 && size == 49) {
@@ -1057,7 +1067,8 @@ static int sixaxis_set_operational_bt(struct hid_device *hdev)
 				     HID_FEATURE_REPORT);
 }
 
-/* Requesting feature report 0x02 in Bluetooth mode changes the state of the
+/*
+ * 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)
@@ -1211,9 +1222,11 @@ static int sony_leds_init(struct hid_device *hdev)
 		name_fmt = "%s::sony%d";
 	}
 
-	/* Clear LEDs as we have no way of reading their initial state. This is
+	/*
+	 * Clear LEDs as we have no way of reading their initial state. This is
 	 * only relevant if the driver is loaded after somebody actively set the
-	 * LEDs to on */
+	 * LEDs to on
+	 */
 	sony_set_leds(hdev, initial_values, drv_data->led_count);
 
 	name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
@@ -1420,7 +1433,8 @@ 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
+	/*
+	 * 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;
@@ -1533,7 +1547,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 				goto err_stop;
 			}
 		}
-		/* The Dualshock 4 touchpad supports 2 touches and has a
+		/*
+		 * The Dualshock 4 touchpad supports 2 touches and has a
 		 * resolution of 1920x940.
 		 */
 		ret = sony_register_touchpad(sc, 2, 1920, 940);
-- 
1.8.5.3

[PATCH v2 2/5] HID: sony: Fix work queue issues

[Frank Praznik]

Only initialize force feedback for devices that actually support it (Sixaxis and
Dualshock 4) to prevent calls to schedule_work() with an uninitialized work
queue.

Move the cancel_work_sync() call out of sony_destroy_ff() since the state worker
is used for the LEDs even when force-feedback is disabled.

Remove the sony_destroy_ff() function since it is no longer used.

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

 This is functionally the same as the 3.14 fix but updated for the 3.15 codebase

 drivers/hid/hid-sony.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 26992e1..a51a9c0 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -51,6 +51,7 @@
 #define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER | BUZZ_CONTROLLER |\
 				DUALSHOCK4_CONTROLLER)
 #define SONY_BATTERY_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER)
+#define SONY_FF_SUPPORT (SIXAXIS_CONTROLLER | DUALSHOCK4_CONTROLLER)
 
 #define MAX_LEDS 4
 
@@ -729,6 +730,7 @@ struct sony_sc {
 	__u8 right;
 #endif
 
+	__u8 worker_initialized;
 	__u8 cable_state;
 	__u8 battery_charging;
 	__u8 battery_capacity;
@@ -1367,22 +1369,12 @@ static int sony_init_ff(struct hid_device *hdev)
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
 }
 
-static void sony_destroy_ff(struct hid_device *hdev)
-{
-	struct sony_sc *sc = hid_get_drvdata(hdev);
-
-	cancel_work_sync(&sc->state_worker);
-}
-
 #else
 static int sony_init_ff(struct hid_device *hdev)
 {
 	return 0;
 }
 
-static void sony_destroy_ff(struct hid_device *hdev)
-{
-}
 #endif
 
 static int sony_battery_get_property(struct power_supply *psy,
@@ -1535,9 +1527,11 @@ 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;
 		ret = sixaxis_set_operational_usb(hdev);
+		sc->worker_initialized = 1;
 		INIT_WORK(&sc->state_worker, sixaxis_state_worker);
 	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT) {
 		ret = sixaxis_set_operational_bt(hdev);
+		sc->worker_initialized = 1;
 		INIT_WORK(&sc->state_worker, sixaxis_state_worker);
 	} else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
 		if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) {
@@ -1555,6 +1549,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		if (ret < 0)
 			goto err_stop;
 
+		sc->worker_initialized = 1;
 		INIT_WORK(&sc->state_worker, dualshock4_state_worker);
 	} else {
 		ret = 0;
@@ -1582,9 +1577,11 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		}
 	}
 
-	ret = sony_init_ff(hdev);
-	if (ret < 0)
-		goto err_close;
+	if (sc->quirks & SONY_FF_SUPPORT) {
+		ret = sony_init_ff(hdev);
+		if (ret < 0)
+			goto err_close;
+	}
 
 	return 0;
 err_close:
@@ -1594,6 +1591,8 @@ err_stop:
 		sony_leds_remove(hdev);
 	if (sc->quirks & SONY_BATTERY_SUPPORT)
 		sony_battery_remove(sc);
+	if (sc->worker_initialized)
+		cancel_work_sync(&sc->state_worker);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -1610,7 +1609,8 @@ static void sony_remove(struct hid_device *hdev)
 		sony_battery_remove(sc);
 	}
 
-	sony_destroy_ff(hdev);
+	if (sc->worker_initialized)
+		cancel_work_sync(&sc->state_worker);
 
 	hid_hw_stop(hdev);
 }
-- 
1.8.5.3

[PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index.

[Frank Praznik]

Make sure that an out-of-bounds read doesn't occur in the Sixaxis battery level
lookup table in the event that the controller sends an invalid battery status
value in the report.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 drivers/hid/hid-sony.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a51a9c0..b39e3ab 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -854,7 +854,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
 		battery_capacity = 100;
 		battery_charging = !(rd[30] & 0x01);
 	} else {
-		battery_capacity = sixaxis_battery_capacity[rd[30]];
+		__u8 index = rd[30] <= 5 ? rd[30] : 5;
+		battery_capacity = sixaxis_battery_capacity[index];
 		battery_charging = 0;
 	}
 	cable_state = !((rd[31] >> 4) & 0x01);
-- 
1.8.5.3

[PATCH v2 4/5] HID: sony: Prevent duplicate controller connections.

[Frank Praznik]

If a Sixaxis or Dualshock 4 controller is connected via USB while already
connected via Bluetooth it will cause duplicate devices to be added to the
input device list.

To prevent this a global list of controllers and their MAC addresses is
maintained and new controllers are checked against this list.  If a duplicate
is found, the probe function will exit with -EEXIST.

On USB the MAC is retrieved via a feature report.  On Bluetooth neither
controller reports the MAC address in a feature report so the MAC is parsed from
the uniq string.  As uniq cannot be guaranteed to be a MAC address in every case
(uHID or the behavior of HIDP changing) a parsing failure will not prevent the
connection.

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

 v2 addresses the issues mentioned in the v1 code review.
 
 To preempt a potential question, byte-swapping the Sixaxis MAC is correct
 behavior since addresses in the Bluetooth protocol are natively little-endian.

 drivers/hid/hid-sony.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 140 insertions(+)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index b39e3ab..5dfa45c 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,6 +33,7 @@
 #include <linux/leds.h>
 #include <linux/power_supply.h>
 #include <linux/spinlock.h>
+#include <linux/list.h>
 #include <linux/input/mt.h>
 
 #include "hid-ids.h"
@@ -717,8 +718,12 @@ static enum power_supply_property sony_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 };
 
+static spinlock_t sony_dev_list_lock;
+static LIST_HEAD(sony_device_list);
+
 struct sony_sc {
 	spinlock_t lock;
+	struct list_head list_node;
 	struct hid_device *hdev;
 	struct led_classdev *leds[MAX_LEDS];
 	unsigned long quirks;
@@ -730,6 +735,7 @@ struct sony_sc {
 	__u8 right;
 #endif
 
+	__u8 mac_address[6];
 	__u8 worker_initialized;
 	__u8 cable_state;
 	__u8 battery_charging;
@@ -1489,6 +1495,133 @@ static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
 	return 0;
 }
 
+/*
+ * If a controller is plugged in via USB while already connected via Bluetooth
+ * it will show up as two devices. A global list of connected controllers and
+ * their MAC addresses is maintained to ensure that a device is only connected
+ * once.
+ */
+static int sony_check_add_dev_list(struct sony_sc *sc)
+{
+	struct sony_sc *entry;
+	unsigned long flags;
+	int ret;
+
+	spin_lock_irqsave(&sony_dev_list_lock, flags);
+
+	list_for_each_entry(entry, &sony_device_list, list_node) {
+		ret = memcmp(sc->mac_address, entry->mac_address,
+				sizeof(sc->mac_address));
+		if (!ret) {
+			ret = -EEXIST;
+			hid_info(sc->hdev, "controller with MAC address %pMR already connected\n",
+				sc->mac_address);
+			goto unlock;
+		}
+	}
+
+	ret = 0;
+	list_add(&(sc->list_node), &sony_device_list);
+
+unlock:
+	spin_unlock_irqrestore(&sony_dev_list_lock, flags);
+	return ret;
+}
+
+static void sony_remove_dev_list(struct sony_sc *sc)
+{
+	unsigned long flags;
+
+	if (sc->list_node.next) {
+		spin_lock_irqsave(&sony_dev_list_lock, flags);
+		list_del(&(sc->list_node));
+		spin_unlock_irqrestore(&sony_dev_list_lock, flags);
+	}
+}
+
+static int sony_get_bt_devaddr(struct sony_sc *sc)
+{
+	int ret;
+
+	/* HIDP stores the device MAC address as a string in the uniq field. */
+	ret = strlen(sc->hdev->uniq);
+	if (ret != 17)
+		return -EINVAL;
+
+	ret = sscanf(sc->hdev->uniq,
+		"%02hhx:%02hhx:%02hhx:%02hhx:%02hhx:%02hhx",
+		&sc->mac_address[5], &sc->mac_address[4], &sc->mac_address[3],
+		&sc->mac_address[2], &sc->mac_address[1], &sc->mac_address[0]);
+
+	if (ret != 6)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int sony_check_add(struct sony_sc *sc)
+{
+	int n, ret;
+
+	if ((sc->quirks & DUALSHOCK4_CONTROLLER_BT) ||
+	    (sc->quirks & SIXAXIS_CONTROLLER_BT)) {
+		/*
+		 * sony_get_bt_devaddr() attempts to parse the Bluetooth MAC
+		 * address from the uniq string where HIDP stores it.
+		 * As uniq cannot be guaranteed to be a MAC address in all cases
+		 * a failure of this function should not prevent the connection.
+		 */
+		if (sony_get_bt_devaddr(sc) < 0) {
+			hid_warn(sc->hdev, "UNIQ does not contain a MAC address; duplicate check skipped\n");
+			return 0;
+		}
+	} else if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
+		__u8 buf[7];
+
+		/*
+		 * The MAC address of a DS4 controller connected via USB can be
+		 * retrieved with feature report 0x81. The address begins at
+		 * offset 1.
+		 */
+		ret = hid_hw_raw_request(sc->hdev, 0x81, buf, sizeof(buf),
+				HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+		if (ret != 7) {
+			hid_err(sc->hdev, "failed to retrieve feature report 0x81 with the DualShock 4 MAC address\n");
+			return ret < 0 ? ret : -EINVAL;
+		}
+
+		memcpy(sc->mac_address, &buf[1], sizeof(sc->mac_address));
+	} else if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
+		__u8 buf[18];
+
+		/*
+		 * The MAC address of a Sixaxis controller connected via USB can
+		 * be retrieved with feature report 0xf2. The address begins at
+		 * offset 4.
+		 */
+		ret = hid_hw_raw_request(sc->hdev, 0xf2, buf, sizeof(buf),
+				HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+
+		if (ret != 18) {
+			hid_err(sc->hdev, "failed to retrieve feature report 0xf2 with the Sixaxis MAC address\n");
+			return ret < 0 ? ret : -EINVAL;
+		}
+
+		/*
+		 * The Sixaxis device MAC in the report is big-endian and must
+		 * be byte-swapped.
+		 */
+		for (n = 0; n < 6; n++)
+			sc->mac_address[5-n] = buf[4+n];
+	} else {
+		return 0;
+	}
+
+	return sony_check_add_dev_list(sc);
+}
+
+
 static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
@@ -1559,6 +1692,10 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (ret < 0)
 		goto err_stop;
 
+	ret = sony_check_add(sc);
+	if (ret < 0)
+		goto err_stop;
+
 	if (sc->quirks & SONY_LED_SUPPORT) {
 		ret = sony_leds_init(hdev);
 		if (ret < 0)
@@ -1594,6 +1731,7 @@ err_stop:
 		sony_battery_remove(sc);
 	if (sc->worker_initialized)
 		cancel_work_sync(&sc->state_worker);
+	sony_remove_dev_list(sc);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -1613,6 +1751,8 @@ static void sony_remove(struct hid_device *hdev)
 	if (sc->worker_initialized)
 		cancel_work_sync(&sc->state_worker);
 
+	sony_remove_dev_list(sc);
+
 	hid_hw_stop(hdev);
 }
 
-- 
1.8.5.3

[PATCH v2 5/5] HID: hidp: Add a comment that some devices depend on the current behavior of uniq

[Frank Praznik]

Add a comment noting that some devices depend on the destination address being
stored in uniq.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 net/bluetooth/hidp/core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 77c4bad..98e4840 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -767,6 +767,9 @@ static int hidp_setup_hid(struct hidp_session *session,
 	snprintf(hid->phys, sizeof(hid->phys), "%pMR",
 		 &l2cap_pi(session->ctrl_sock->sk)->chan->src);
 
+	/* NOTE: Some device modules depend on the dst address being stored in
+	 * uniq. Please be aware of this before making changes to this behavior.
+	 */
 	snprintf(hid->uniq, sizeof(hid->uniq), "%pMR",
 		 &l2cap_pi(session->ctrl_sock->sk)->chan->dst);
 
-- 
1.8.5.3

Re: [PATCH v2 3/5] HID: sony: Perform a boundry check on the sixaxis battery level index.

[David Herrmann]

Hi

On Thu, Feb 20, 2014 at 5:36 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> Make sure that an out-of-bounds read doesn't occur in the Sixaxis battery level
> lookup table in the event that the controller sends an invalid battery status
> value in the report.
>
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
> ---
>  drivers/hid/hid-sony.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index a51a9c0..b39e3ab 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -854,7 +854,8 @@ static void sixaxis_parse_report(struct sony_sc *sc, __u8 *rd, int size)
>                 battery_capacity = 100;
>                 battery_charging = !(rd[30] & 0x01);
>         } else {
> -               battery_capacity = sixaxis_battery_capacity[rd[30]];
> +               __u8 index = rd[30] <= 5 ? rd[30] : 5;
> +               battery_capacity = sixaxis_battery_capacity[index];

Does it make sense to read something else on invalid reports? Sounds
weird to me to just read at a lower index in case it's too short.
Shouldn't you just bail out? But the worst that can happen is wrong
battery values reported to user-space, so I'm fine with it.

Thanks
David

>                 battery_charging = 0;
>         }
>         cable_state = !((rd[31] >> 4) & 0x01);
> --
> 1.8.5.3
>

Re: [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver

[David Herrmann]

Hi

On Thu, Feb 20, 2014 at 5:35 PM, Frank Praznik <frank.praznik@oh.rr.com> wrote:
> v2 of this patch set addresses the code review issues raised in v1 as well as
> adding a patch that fixes the styling of multi-line comments to conform to
> kernel coding standards, adds a check to protect against a potential
> out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
> that a device module now depends on the current behavior of the uniq string.

Besides some minor comments, the series looks good to me:

Reviewed-by: David Herrmann <dh.herrmann@gmail.com>

Thanks
David

Re: [PATCH v2 0/5] HID: sony: Various fixes and improvements for the Sony driver

[Jiri Kosina]

On Thu, 20 Feb 2014, Frank Praznik wrote:

> v2 of this patch set addresses the code review issues raised in v1 as well as
> adding a patch that fixes the styling of multi-line comments to conform to
> kernel coding standards, adds a check to protect against a potential
> out-of-bounds read in the Sixaxis battery code and adds a note in hidp/core.c
> that a device module now depends on the current behavior of the uniq string.

Queued in for-3.15/sony, thanks.

-- 
Jiri Kosina
SUSE Labs