[PATCH v4 0/7] HID: sony: More Sony controller fixes and improvements.

[PATCH v4 0/7] HID: sony: More Sony controller fixes and improvements.

[Frank Praznik]

v4 of this series has been rebased against jikos/hid.git/for-linus

There was another bit of mismerge code duplication in for-linus that caused
merge conflicts with patches 3 and above in v3 of the series.

The first patch in v4 fixes the mismerge and the remaining patches now apply
cleanly.

Patch 1 should be queued for 3.15 as soon as possible.

[PATCH v4 1/7] HID: sony: Fix cancel_work_sync mismerge

[Frank Praznik]

Remove redundant cancel_work_sync() call caused by mismerge.

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

 Apply against jikos/hid.git/for-linus to fix mismerge code duplication

 drivers/hid/hid-sony.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 69204af..908de27 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1721,8 +1721,6 @@ static void sony_remove(struct hid_device *hdev)
 	if (sc->quirks & SONY_LED_SUPPORT)
 		sony_leds_remove(hdev);
 
-	if (sc->worker_initialized)
-		cancel_work_sync(&sc->state_worker);
 	if (sc->quirks & SONY_BATTERY_SUPPORT) {
 		hid_hw_close(hdev);
 		sony_battery_remove(sc);
-- 
1.8.3.2

[PATCH v4 2/7] HID: sony: Use inliners for work queue initialization and cancellation

[Frank Praznik]

Use inliners to make sure that the work queue initialization flag is always
checked and set correctly when initializing or cancelling the work queue.

Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>
---
 
 v2 doesn't set worker_initialized to 0 when cancelling work since
 cancel_work_sync doesn't actually deinitialize the work queue.
 
 drivers/hid/hid-sony.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 908de27..3df3306 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1578,6 +1578,20 @@ static int sony_check_add(struct sony_sc *sc)
 	return sony_check_add_dev_list(sc);
 }
 
+static inline void sony_init_work(struct sony_sc *sc,
+					void(*worker)(struct work_struct *))
+{
+	if (!sc->worker_initialized)
+		INIT_WORK(&sc->state_worker, worker);
+
+	sc->worker_initialized = 1;
+}
+
+static inline void sony_cancel_work_sync(struct sony_sc *sc)
+{
+	if (sc->worker_initialized)
+		cancel_work_sync(&sc->state_worker);
+}
 
 static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
@@ -1629,8 +1643,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hdev->quirks |= HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP;
 		hdev->quirks |= HID_QUIRK_SKIP_OUTPUT_REPORT_ID;
 		ret = sixaxis_set_operational_usb(hdev);
-		sc->worker_initialized = 1;
-		INIT_WORK(&sc->state_worker, sixaxis_state_worker);
+		sony_init_work(sc, sixaxis_state_worker);
 	} else if (sc->quirks & SIXAXIS_CONTROLLER_BT) {
 		/*
 		 * The Sixaxis wants output reports sent on the ctrl endpoint
@@ -1638,8 +1651,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		 */
 		hdev->quirks |= HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP;
 		ret = sixaxis_set_operational_bt(hdev);
-		sc->worker_initialized = 1;
-		INIT_WORK(&sc->state_worker, sixaxis_state_worker);
+		sony_init_work(sc, sixaxis_state_worker);
 	} else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
 		if (sc->quirks & DUALSHOCK4_CONTROLLER_BT) {
 			/*
@@ -1661,8 +1673,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);
+		sony_init_work(sc, dualshock4_state_worker);
 	} else {
 		ret = 0;
 	}
@@ -1707,8 +1718,7 @@ 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);
+	sony_cancel_work_sync(sc);
 	sony_remove_dev_list(sc);
 	hid_hw_stop(hdev);
 	return ret;
@@ -1726,8 +1736,7 @@ static void sony_remove(struct hid_device *hdev)
 		sony_battery_remove(sc);
 	}
 
-	if (sc->worker_initialized)
-		cancel_work_sync(&sc->state_worker);
+	sony_cancel_work_sync(sc);
 
 	sony_remove_dev_list(sc);
 
-- 
1.8.3.2

[PATCH v4 3/7] HID: sony: Use a struct for the Sixaxis output report.

[Frank Praznik]

Use a struct for the Sixaxis output report that uses named members to set the
report fields.

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

 drivers/hid/hid-sony.c | 66 +++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 3df3306..ed68962 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -717,6 +717,36 @@ static enum power_supply_property sony_battery_props[] = {
 	POWER_SUPPLY_PROP_STATUS,
 };
 
+struct sixaxis_led {
+	__u8 time_enabled; /* the total time the led is active (0xff means forever) */
+	__u8 duty_length;  /* how long a cycle is in deciseconds (0 means "really fast") */
+	__u8 enabled;
+	__u8 duty_off; /* % of duty_length the led is off (0xff means 100%) */
+	__u8 duty_on;  /* % of duty_length the led is on (0xff mean 100%) */
+} __packed;
+
+struct sixaxis_rumble {
+	__u8 padding;
+	__u8 right_duration; /* Right motor duration (0xff means forever) */
+	__u8 right_motor_on; /* Right (small) motor on/off, only supports values of 0 or 1 (off/on) */
+	__u8 left_duration; /* Left motor duration (0xff means forever) */
+	__u8 left_motor_force; /* left (large) motor, supports force values from 0 to 255 */
+} __packed;
+
+struct sixaxis_output_report {
+	__u8 report_id;
+	struct sixaxis_rumble rumble;
+	__u8 padding[4];
+	__u8 leds_bitmap; /* bitmap of enabled LEDs: LED_1 = 0x02, LED_2 = 0x04, ... */
+	struct sixaxis_led led[4]; /* LEDx at (4 - x) */
+	struct sixaxis_led _reserved; /* LED5, not actually soldered */
+} __packed;
+
+union sixaxis_output_report_01 {
+	struct sixaxis_output_report data;
+	__u8 buf[36];
+};
+
 static spinlock_t sony_dev_list_lock;
 static LIST_HEAD(sony_device_list);
 
@@ -1244,29 +1274,31 @@ error_leds:
 static void sixaxis_state_worker(struct work_struct *work)
 {
 	struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
-	unsigned char buf[] = {
-		0x01,
-		0x00, 0xff, 0x00, 0xff, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x00,
-		0xff, 0x27, 0x10, 0x00, 0x32,
-		0xff, 0x27, 0x10, 0x00, 0x32,
-		0xff, 0x27, 0x10, 0x00, 0x32,
-		0xff, 0x27, 0x10, 0x00, 0x32,
-		0x00, 0x00, 0x00, 0x00, 0x00
+	union sixaxis_output_report_01 report = {
+		.buf = {
+			0x01,
+			0x00, 0xff, 0x00, 0xff, 0x00,
+			0x00, 0x00, 0x00, 0x00, 0x00,
+			0xff, 0x27, 0x10, 0x00, 0x32,
+			0xff, 0x27, 0x10, 0x00, 0x32,
+			0xff, 0x27, 0x10, 0x00, 0x32,
+			0xff, 0x27, 0x10, 0x00, 0x32,
+			0x00, 0x00, 0x00, 0x00, 0x00
+		}
 	};
 
 #ifdef CONFIG_SONY_FF
-	buf[3] = sc->right ? 1 : 0;
-	buf[5] = sc->left;
+	report.data.rumble.right_motor_on = sc->right ? 1 : 0;
+	report.data.rumble.left_motor_force = sc->left;
 #endif
 
-	buf[10] |= sc->led_state[0] << 1;
-	buf[10] |= sc->led_state[1] << 2;
-	buf[10] |= sc->led_state[2] << 3;
-	buf[10] |= sc->led_state[3] << 4;
+	report.data.leds_bitmap |= sc->led_state[0] << 1;
+	report.data.leds_bitmap |= sc->led_state[1] << 2;
+	report.data.leds_bitmap |= sc->led_state[2] << 3;
+	report.data.leds_bitmap |= sc->led_state[3] << 4;
 
-	hid_hw_raw_request(sc->hdev, 0x01, buf, sizeof(buf), HID_OUTPUT_REPORT,
-			HID_REQ_SET_REPORT);
+	hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
+			sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
 }
 
 static void dualshock4_state_worker(struct work_struct *work)
-- 
1.8.3.2

[PATCH v4 4/7] HID: sony: Convert startup and shutdown functions to use a uniform parameter type

[Frank Praznik]

Convert all of the local initialization and shutdown functions to take a
parameter type of struct sony_sc* instead of using a mix of struct sony_sc* and
struct hid_device*.

Allows for the removal of some calls to hid_get_drvdata().

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

 drivers/hid/hid-sony.c | 67 ++++++++++++++++++++++++--------------------------
 1 file changed, 32 insertions(+), 35 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index ed68962..a86542a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1096,19 +1096,18 @@ static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
-static void sony_set_leds(struct hid_device *hdev, const __u8 *leds, int count)
+static void sony_set_leds(struct sony_sc *sc, const __u8 *leds, int count)
 {
-	struct sony_sc *drv_data = hid_get_drvdata(hdev);
 	int n;
 
 	BUG_ON(count > MAX_LEDS);
 
-	if (drv_data->quirks & BUZZ_CONTROLLER && count == 4) {
-		buzz_set_leds(hdev, leds);
+	if (sc->quirks & BUZZ_CONTROLLER && count == 4) {
+		buzz_set_leds(sc->hdev, leds);
 	} else {
 		for (n = 0; n < count; n++)
-			drv_data->led_state[n] = leds[n];
-		schedule_work(&drv_data->state_worker);
+			sc->led_state[n] = leds[n];
+		schedule_work(&sc->state_worker);
 	}
 }
 
@@ -1131,7 +1130,8 @@ static void sony_led_set_brightness(struct led_classdev *led,
 		if (led == drv_data->leds[n]) {
 			if (value != drv_data->led_state[n]) {
 				drv_data->led_state[n] = value;
-				sony_set_leds(hdev, drv_data->led_state, drv_data->led_count);
+				sony_set_leds(drv_data, drv_data->led_state,
+						drv_data->led_count);
 			}
 			break;
 		}
@@ -1160,30 +1160,28 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	return LED_OFF;
 }
 
-static void sony_leds_remove(struct hid_device *hdev)
+static void sony_leds_remove(struct sony_sc *sc)
 {
-	struct sony_sc *drv_data;
 	struct led_classdev *led;
 	int n;
 
-	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
+	BUG_ON(!(sc->quirks & SONY_LED_SUPPORT));
 
-	for (n = 0; n < drv_data->led_count; n++) {
-		led = drv_data->leds[n];
-		drv_data->leds[n] = NULL;
+	for (n = 0; n < sc->led_count; n++) {
+		led = sc->leds[n];
+		sc->leds[n] = NULL;
 		if (!led)
 			continue;
 		led_classdev_unregister(led);
 		kfree(led);
 	}
 
-	drv_data->led_count = 0;
+	sc->led_count = 0;
 }
 
-static int sony_leds_init(struct hid_device *hdev)
+static int sony_leds_init(struct sony_sc *sc)
 {
-	struct sony_sc *drv_data;
+	struct hid_device *hdev = sc->hdev;
 	int n, ret = 0;
 	int max_brightness;
 	int use_colors;
@@ -1195,11 +1193,10 @@ static int sony_leds_init(struct hid_device *hdev)
 	static const char * const color_str[] = { "red", "green", "blue" };
 	static const __u8 initial_values[MAX_LEDS] = { 0x00, 0x00, 0x00, 0x00 };
 
-	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
+	BUG_ON(!(sc->quirks & SONY_LED_SUPPORT));
 
-	if (drv_data->quirks & BUZZ_CONTROLLER) {
-		drv_data->led_count = 4;
+	if (sc->quirks & BUZZ_CONTROLLER) {
+		sc->led_count = 4;
 		max_brightness = 1;
 		use_colors = 0;
 		name_len = strlen("::buzz#");
@@ -1207,14 +1204,14 @@ 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) {
-		drv_data->led_count = 3;
+	} else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
+		sc->led_count = 3;
 		max_brightness = 255;
 		use_colors = 1;
 		name_len = 0;
 		name_fmt = "%s:%s";
 	} else {
-		drv_data->led_count = 4;
+		sc->led_count = 4;
 		max_brightness = 1;
 		use_colors = 0;
 		name_len = strlen("::sony#");
@@ -1226,11 +1223,11 @@ static int sony_leds_init(struct hid_device *hdev)
 	 * only relevant if the driver is loaded after somebody actively set the
 	 * LEDs to on
 	 */
-	sony_set_leds(hdev, initial_values, drv_data->led_count);
+	sony_set_leds(sc, initial_values, sc->led_count);
 
 	name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
 
-	for (n = 0; n < drv_data->led_count; n++) {
+	for (n = 0; n < sc->led_count; n++) {
 
 		if (use_colors)
 			name_sz = strlen(dev_name(&hdev->dev)) + strlen(color_str[n]) + 2;
@@ -1260,13 +1257,13 @@ static int sony_leds_init(struct hid_device *hdev)
 			goto error_leds;
 		}
 
-		drv_data->leds[n] = led;
+		sc->leds[n] = led;
 	}
 
 	return ret;
 
 error_leds:
-	sony_leds_remove(hdev);
+	sony_leds_remove(sc);
 
 	return ret;
 }
@@ -1355,9 +1352,9 @@ static int sony_play_effect(struct input_dev *dev, void *data,
 	return 0;
 }
 
-static int sony_init_ff(struct hid_device *hdev)
+static int sony_init_ff(struct sony_sc *sc)
 {
-	struct hid_input *hidinput = list_entry(hdev->inputs.next,
+	struct hid_input *hidinput = list_entry(sc->hdev->inputs.next,
 						struct hid_input, list);
 	struct input_dev *input_dev = hidinput->input;
 
@@ -1366,7 +1363,7 @@ static int sony_init_ff(struct hid_device *hdev)
 }
 
 #else
-static int sony_init_ff(struct hid_device *hdev)
+static int sony_init_ff(struct sony_sc *sc)
 {
 	return 0;
 }
@@ -1718,7 +1715,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_stop;
 
 	if (sc->quirks & SONY_LED_SUPPORT) {
-		ret = sony_leds_init(hdev);
+		ret = sony_leds_init(sc);
 		if (ret < 0)
 			goto err_stop;
 	}
@@ -1737,7 +1734,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	}
 
 	if (sc->quirks & SONY_FF_SUPPORT) {
-		ret = sony_init_ff(hdev);
+		ret = sony_init_ff(sc);
 		if (ret < 0)
 			goto err_close;
 	}
@@ -1747,7 +1744,7 @@ err_close:
 	hid_hw_close(hdev);
 err_stop:
 	if (sc->quirks & SONY_LED_SUPPORT)
-		sony_leds_remove(hdev);
+		sony_leds_remove(sc);
 	if (sc->quirks & SONY_BATTERY_SUPPORT)
 		sony_battery_remove(sc);
 	sony_cancel_work_sync(sc);
@@ -1761,7 +1758,7 @@ static void sony_remove(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	if (sc->quirks & SONY_LED_SUPPORT)
-		sony_leds_remove(hdev);
+		sony_leds_remove(sc);
 
 	if (sc->quirks & SONY_BATTERY_SUPPORT) {
 		hid_hw_close(hdev);
-- 
1.8.3.2

[PATCH v4 5/7] HID: sony: Use the controller Bluetooth MAC address as the unique value in the battery name string

[Frank Praznik]

Use the controller Bluetooth MAC address as the unique identifier in the
battery name string instead of the atomic integer that was used before.

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

 drivers/hid/hid-sony.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a86542a..c709161 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1413,8 +1413,6 @@ static int sony_battery_get_property(struct power_supply *psy,
 
 static int sony_battery_probe(struct sony_sc *sc)
 {
-	static atomic_t power_id_seq = ATOMIC_INIT(0);
-	unsigned long power_id;
 	struct hid_device *hdev = sc->hdev;
 	int ret;
 
@@ -1424,15 +1422,13 @@ static int sony_battery_probe(struct sony_sc *sc)
 	 */
 	sc->battery_capacity = 100;
 
-	power_id = (unsigned long)atomic_inc_return(&power_id_seq);
-
 	sc->battery.properties = sony_battery_props;
 	sc->battery.num_properties = ARRAY_SIZE(sony_battery_props);
 	sc->battery.get_property = sony_battery_get_property;
 	sc->battery.type = POWER_SUPPLY_TYPE_BATTERY;
 	sc->battery.use_for_apm = 0;
-	sc->battery.name = kasprintf(GFP_KERNEL, "sony_controller_battery_%lu",
-				     power_id);
+	sc->battery.name = kasprintf(GFP_KERNEL, "sony_controller_battery_%pMR",
+				     sc->mac_address);
 	if (!sc->battery.name)
 		return -ENOMEM;
 
-- 
1.8.3.2

[PATCH v4 6/7] HID: sony: Initialize the controller LEDs with a device ID value

[Frank Praznik]

Add an IDA id allocator to assign unique, sequential device ids to Sixaxis and
DualShock 4 controllers.

Use the device ID to initialize the Sixaxis and DualShock 4 controller LEDs to
default values.  The number or color of the controller is set relative to other
connected Sony controllers.

Set the LED class brightness values to the initial values and add the new led to
the array before calling led_classdev_register so that the correct brightness
value shows up in the LED sysfs entry.

Use explicit module init and exit functions since the IDA allocator must be
manually destroyed when the module is unloaded.

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

 drivers/hid/hid-sony.c | 117 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 114 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index c709161..f26f8fa 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,6 +33,7 @@
 #include <linux/power_supply.h>
 #include <linux/spinlock.h>
 #include <linux/list.h>
+#include <linux/idr.h>
 #include <linux/input/mt.h>
 
 #include "hid-ids.h"
@@ -749,6 +750,7 @@ union sixaxis_output_report_01 {
 
 static spinlock_t sony_dev_list_lock;
 static LIST_HEAD(sony_device_list);
+static DEFINE_IDA(sony_device_id_allocator);
 
 struct sony_sc {
 	spinlock_t lock;
@@ -758,6 +760,7 @@ struct sony_sc {
 	unsigned long quirks;
 	struct work_struct state_worker;
 	struct power_supply battery;
+	int device_id;
 
 #ifdef CONFIG_SONY_FF
 	__u8 left;
@@ -1078,6 +1081,52 @@ static int dualshock4_set_operational_bt(struct hid_device *hdev)
 				HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
 }
 
+static void sixaxis_set_leds_from_id(int id, __u8 values[MAX_LEDS])
+{
+	static const __u8 sixaxis_leds[10][4] = {
+				{ 0x01, 0x00, 0x00, 0x00 },
+				{ 0x00, 0x01, 0x00, 0x00 },
+				{ 0x00, 0x00, 0x01, 0x00 },
+				{ 0x00, 0x00, 0x00, 0x01 },
+				{ 0x01, 0x00, 0x00, 0x01 },
+				{ 0x00, 0x01, 0x00, 0x01 },
+				{ 0x00, 0x00, 0x01, 0x01 },
+				{ 0x01, 0x00, 0x01, 0x01 },
+				{ 0x00, 0x01, 0x01, 0x01 },
+				{ 0x01, 0x01, 0x01, 0x01 }
+	};
+
+	BUG_ON(MAX_LEDS < ARRAY_SIZE(sixaxis_leds[0]));
+
+	if (id < 0)
+		return;
+
+	id %= 10;
+	memcpy(values, sixaxis_leds[id], sizeof(sixaxis_leds[id]));
+}
+
+static void dualshock4_set_leds_from_id(int id, __u8 values[MAX_LEDS])
+{
+	/* The first 4 color/index entries match what the PS4 assigns */
+	static const __u8 color_code[7][3] = {
+			/* Blue   */	{ 0x00, 0x00, 0x01 },
+			/* Red	  */	{ 0x01, 0x00, 0x00 },
+			/* Green  */	{ 0x00, 0x01, 0x00 },
+			/* Pink   */	{ 0x02, 0x00, 0x01 },
+			/* Orange */	{ 0x02, 0x01, 0x00 },
+			/* Teal   */	{ 0x00, 0x01, 0x01 },
+			/* White  */	{ 0x01, 0x01, 0x01 }
+	};
+
+	BUG_ON(MAX_LEDS < ARRAY_SIZE(color_code[0]));
+
+	if (id < 0)
+		return;
+
+	id %= 7;
+	memcpy(values, color_code[id], sizeof(color_code[id]));
+}
+
 static void buzz_set_leds(struct hid_device *hdev, const __u8 *leds)
 {
 	struct list_head *report_list =
@@ -1191,7 +1240,7 @@ static int sony_leds_init(struct sony_sc *sc)
 	size_t name_len;
 	const char *name_fmt;
 	static const char * const color_str[] = { "red", "green", "blue" };
-	static const __u8 initial_values[MAX_LEDS] = { 0x00, 0x00, 0x00, 0x00 };
+	__u8 initial_values[MAX_LEDS] = { 0 };
 
 	BUG_ON(!(sc->quirks & SONY_LED_SUPPORT));
 
@@ -1205,12 +1254,14 @@ static int sony_leds_init(struct sony_sc *sc)
 		if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
 			return -ENODEV;
 	} else if (sc->quirks & DUALSHOCK4_CONTROLLER) {
+		dualshock4_set_leds_from_id(sc->device_id, initial_values);
 		sc->led_count = 3;
 		max_brightness = 255;
 		use_colors = 1;
 		name_len = 0;
 		name_fmt = "%s:%s";
 	} else {
+		sixaxis_set_leds_from_id(sc->device_id, initial_values);
 		sc->led_count = 4;
 		max_brightness = 1;
 		use_colors = 0;
@@ -1245,14 +1296,17 @@ static int sony_leds_init(struct sony_sc *sc)
 		else
 			snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1);
 		led->name = name;
-		led->brightness = 0;
+		led->brightness = initial_values[n];
 		led->max_brightness = max_brightness;
 		led->brightness_get = sony_led_get_brightness;
 		led->brightness_set = sony_led_set_brightness;
 
+		sc->leds[n] = led;
+
 		ret = led_classdev_register(&hdev->dev, led);
 		if (ret) {
 			hid_err(hdev, "Failed to register LED %d\n", n);
+			sc->leds[n] = NULL;
 			kfree(led);
 			goto error_leds;
 		}
@@ -1603,6 +1657,38 @@ static int sony_check_add(struct sony_sc *sc)
 	return sony_check_add_dev_list(sc);
 }
 
+static int sony_set_device_id(struct sony_sc *sc)
+{
+	int ret;
+
+	/*
+	 * Only DualShock 4 or Sixaxis controllers get an id.
+	 * All others are set to -1.
+	 */
+	if ((sc->quirks & SIXAXIS_CONTROLLER) ||
+	    (sc->quirks & DUALSHOCK4_CONTROLLER)) {
+		ret = ida_simple_get(&sony_device_id_allocator, 0, 0,
+					GFP_KERNEL);
+		if (ret < 0) {
+			sc->device_id = -1;
+			return ret;
+		}
+		sc->device_id = ret;
+	} else {
+		sc->device_id = -1;
+	}
+
+	return 0;
+}
+
+static void sony_release_device_id(struct sony_sc *sc)
+{
+	if (sc->device_id >= 0) {
+		ida_simple_remove(&sony_device_id_allocator, sc->device_id);
+		sc->device_id = -1;
+	}
+}
+
 static inline void sony_init_work(struct sony_sc *sc,
 					void(*worker)(struct work_struct *))
 {
@@ -1654,6 +1740,12 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	ret = sony_set_device_id(sc);
+	if (ret < 0) {
+		hid_err(hdev, "failed to allocate the device id\n");
+		goto err_stop;
+	}
+
 	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
 		/*
 		 * The Sony Sixaxis does not handle HID Output Reports on the
@@ -1745,6 +1837,7 @@ err_stop:
 		sony_battery_remove(sc);
 	sony_cancel_work_sync(sc);
 	sony_remove_dev_list(sc);
+	sony_release_device_id(sc);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -1765,6 +1858,8 @@ static void sony_remove(struct hid_device *hdev)
 
 	sony_remove_dev_list(sc);
 
+	sony_release_device_id(sc);
+
 	hid_hw_stop(hdev);
 }
 
@@ -1809,6 +1904,22 @@ static struct hid_driver sony_driver = {
 	.report_fixup  = sony_report_fixup,
 	.raw_event     = sony_raw_event
 };
-module_hid_driver(sony_driver);
+
+static int __init sony_init(void)
+{
+	dbg_hid("Sony:%s\n", __func__);
+
+	return hid_register_driver(&sony_driver);
+}
+
+static void __exit sony_exit(void)
+{
+	dbg_hid("Sony:%s\n", __func__);
+
+	ida_destroy(&sony_device_id_allocator);
+	hid_unregister_driver(&sony_driver);
+}
+module_init(sony_init);
+module_exit(sony_exit);
 
 MODULE_LICENSE("GPL");
-- 
1.8.3.2

[PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[Frank Praznik]

Add support for setting the blink rate of the LEDs.  The Sixaxis allows control
over each individual LED, but the Dualshock 4 only has one global control for
the light bar so changing any individual color changes the global blink rate.

Setting the brightness cancels the blinking as per the LED class specifications.

The Sixaxis and Dualshock 4 controllers accept delays in decisecond increments
from 0 to 255 (2550 milliseconds).

The value at index 1 of the DualShock 4 USB output report must be 0xFF or the
light bar won't blink.

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

 drivers/hid/hid-sony.c | 113 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 104 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index f26f8fa..aa5ece5 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -773,6 +773,8 @@ struct sony_sc {
 	__u8 battery_charging;
 	__u8 battery_capacity;
 	__u8 led_state[MAX_LEDS];
+	__u8 led_delay_on[MAX_LEDS];
+	__u8 led_delay_off[MAX_LEDS];
 	__u8 led_count;
 };
 
@@ -1167,7 +1169,7 @@ static void sony_led_set_brightness(struct led_classdev *led,
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
 	struct sony_sc *drv_data;
 
-	int n;
+	int n, blink_index;
 
 	drv_data = hid_get_drvdata(hdev);
 	if (!drv_data) {
@@ -1176,14 +1178,31 @@ static void sony_led_set_brightness(struct led_classdev *led,
 	}
 
 	for (n = 0; n < drv_data->led_count; n++) {
-		if (led == drv_data->leds[n]) {
-			if (value != drv_data->led_state[n]) {
-				drv_data->led_state[n] = value;
-				sony_set_leds(drv_data, drv_data->led_state,
-						drv_data->led_count);
-			}
+		if (led == drv_data->leds[n])
 			break;
-		}
+	}
+
+	/* This LED is not registered on this device */
+	if (n >= drv_data->led_count)
+		return;
+
+	/* The DualShock 4 has a global blink setting and always uses index 0 */
+	if (drv_data->quirks & DUALSHOCK4_CONTROLLER)
+		blink_index = 0;
+	else
+		blink_index = n;
+
+	if ((value != drv_data->led_state[n]) ||
+		drv_data->led_delay_on[blink_index] ||
+		drv_data->led_delay_off[blink_index]) {
+		drv_data->led_state[n] = value;
+
+		/* Setting the brightness stops the blinking */
+		drv_data->led_delay_on[blink_index] = 0;
+		drv_data->led_delay_off[blink_index] = 0;
+
+		sony_set_leds(drv_data, drv_data->led_state,
+				drv_data->led_count);
 	}
 }
 
@@ -1209,6 +1228,58 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	return LED_OFF;
 }
 
+static int sony_led_blink_set(struct led_classdev *led, unsigned long *delay_on,
+				unsigned long *delay_off)
+{
+	struct device *dev = led->dev->parent;
+	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+	struct sony_sc *drv_data = hid_get_drvdata(hdev);
+	int n;
+	__u8 new_on, new_off;
+
+	if (!drv_data) {
+		hid_err(hdev, "No device data\n");
+		return -EINVAL;
+	}
+
+	/* Max delay is 255 deciseconds or 2550 milliseconds */
+	if (*delay_on > 2550)
+		*delay_on = 2550;
+	if (*delay_off > 2550)
+		*delay_off = 2550;
+
+	/* Blink at 1 Hz if both values are zero */
+	if (!*delay_on && !*delay_off)
+		*delay_on = *delay_off = 1000;
+
+	new_on = *delay_on / 10;
+	new_off = *delay_off / 10;
+
+	/* The DualShock 4 has a global blink setting and always uses index 0 */
+	if (drv_data->quirks & DUALSHOCK4_CONTROLLER) {
+		n = 0;
+	} else {
+		for (n = 0; n < drv_data->led_count; n++) {
+			if (led == drv_data->leds[n])
+				break;
+		}
+	}
+
+	/* This LED is not registered on this device */
+	if (n >= drv_data->led_count)
+		return -EINVAL;
+
+	/* Don't schedule work if the values didn't change */
+	if (new_on != drv_data->led_delay_on[n] ||
+		new_off != drv_data->led_delay_off[n]) {
+		drv_data->led_delay_on[n] = new_on;
+		drv_data->led_delay_off[n] = new_off;
+		schedule_work(&drv_data->state_worker);
+	}
+
+	return 0;
+}
+
 static void sony_leds_remove(struct sony_sc *sc)
 {
 	struct led_classdev *led;
@@ -1301,6 +1372,9 @@ static int sony_leds_init(struct sony_sc *sc)
 		led->brightness_get = sony_led_get_brightness;
 		led->brightness_set = sony_led_set_brightness;
 
+		if (!(sc->quirks & BUZZ_CONTROLLER))
+			led->blink_set = sony_led_blink_set;
+
 		sc->leds[n] = led;
 
 		ret = led_classdev_register(&hdev->dev, led);
@@ -1325,6 +1399,7 @@ error_leds:
 static void sixaxis_state_worker(struct work_struct *work)
 {
 	struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
+	int n;
 	union sixaxis_output_report_01 report = {
 		.buf = {
 			0x01,
@@ -1348,6 +1423,22 @@ static void sixaxis_state_worker(struct work_struct *work)
 	report.data.leds_bitmap |= sc->led_state[2] << 3;
 	report.data.leds_bitmap |= sc->led_state[3] << 4;
 
+	/*
+	 * The LEDs in the report are indexed in reverse order to their
+	 * corresponding light on the controller.
+	 * Index 0 = LED 4, index 1 = LED 3, etc...
+	 *
+	 * In the case of both delay values being zero (blinking disabled) the
+	 * default report values should be used or the controller LED will be
+	 * always off.
+	 */
+	for (n = 0; n < 4; n++) {
+		if (sc->led_delay_on[n] || sc->led_delay_off[n]) {
+			report.data.led[3 - n].duty_off = sc->led_delay_off[n];
+			report.data.led[3 - n].duty_on = sc->led_delay_on[n];
+		}
+	}
+
 	hid_hw_raw_request(sc->hdev, report.data.report_id, report.buf,
 			sizeof(report), HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
 }
@@ -1362,7 +1453,7 @@ static void dualshock4_state_worker(struct work_struct *work)
 
 	if (sc->quirks & DUALSHOCK4_CONTROLLER_USB) {
 		buf[0] = 0x05;
-		buf[1] = 0x03;
+		buf[1] = 0xFF;
 		offset = 4;
 	} else {
 		buf[0] = 0x11;
@@ -1382,6 +1473,10 @@ static void dualshock4_state_worker(struct work_struct *work)
 	buf[offset++] = sc->led_state[1];
 	buf[offset++] = sc->led_state[2];
 
+	/* If both delay values are zero the DualShock 4 disables blinking. */
+	buf[offset++] = sc->led_delay_on[0];
+	buf[offset++] = sc->led_delay_off[0];
+
 	if (sc->quirks & DUALSHOCK4_CONTROLLER_USB)
 		hid_hw_output_report(hdev, buf, 32);
 	else
-- 
1.8.3.2

Re: [PATCH v4 0/7] HID: sony: More Sony controller fixes and improvements.

[Jiri Kosina]

On Wed, 2 Apr 2014, Frank Praznik wrote:

> v4 of this series has been rebased against jikos/hid.git/for-linus
> 
> There was another bit of mismerge code duplication in for-linus that caused
> merge conflicts with patches 3 and above in v3 of the series.
> 
> The first patch in v4 fixes the mismerge and the remaining patches now apply
> cleanly.
> 
> Patch 1 should be queued for 3.15 as soon as possible.

Hmm, you are right indeed. This conflict resolution is not something I'd 
be proud of.

Will be queuing the first patch shortly after Linus pulls, and will review 
the rest after merge window is closed.

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4 1/7] HID: sony: Fix cancel_work_sync mismerge

[Jiri Kosina]

On Wed, 2 Apr 2014, Frank Praznik wrote:

> Remove redundant cancel_work_sync() call caused by mismerge.
> 
> Signed-off-by: Frank Praznik <frank.praznik@oh.rr.com>

Queued for 3.15, thanks for spotting it, Frank.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[simon]

> Add support for setting the blink rate of the LEDs.  The Sixaxis allows
> control
> over each individual LED, but the Dualshock 4 only has one global control
> for
> the light bar so changing any individual color changes the global blink
> rate.

I was able to build this and test on the controllers I have. After
modprobing 'ledtrig-timer', I was able to 'echo timer > trigger' in the
appropriate led directory and confirm that leds blinked without additional
USB traffic seen by USBMON.

DS3 SixAxis - mostly worked, although I hade to set a led to 0, before I
could set to 1 (otherwise all leds just did automatic blinking). I was
able to:
blink single led
blink led while other was statically on
blink multiple leds with different timing
blinking sometimes got confused, setting trigger to none and back to timer
resolved.

DS4 - Worked as descibed in patch, but I feel behaviour is confusing. I
was able to:
blink single led
unable adjust other led whilst blinking (blinking stopped, or wrong colour
started blinking)

3rd Party Intec - Was unable to get any controlled blinking. As previously
mentioned all leds flash (automatic, as if first plugged in) whenever all
leds are turned off.
I can control all leds in a static on/off mode, but can't set any
blinking/timer behaviour

Let me know if there's any other testing I can do,
Simon.

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[simon]

DS3:
>> blinking sometimes got confused, setting trigger to none and back to
>> timer resolved.

> Could you elaborate on how the blinking became confused?

When blink was enabled the led would turn off (from being statically on),
but never turn on/blink... I'll try to do some more 'structured' testing
over the weekend. Seen on the 3rd led, which might be a clue.

DS4:
> Yeah, if you set different LEDs to different triggers the light can get
> stuck since the triggers will constantly override each other. I'm
> thinking that an extra "blink" LED just to control the global blink rate
> would be a better solution since the hardware blink setting isn't really
> LED specific.

Why not make the triggers/etc apply across the board, so it doesn't matter
which led the command is sent to - it's just registered/copied to the
first (red?) led data[].

IIRC the weirdness I saw was:
--
cd red
echo 80 > brightness # now red
cd blue
echo 80 > brightness # now purple
cd red
echo timer > trigger # now flashing blue....
--

>> 3rd Party Intec - Was unable to get any controlled blinking.

> It sounds like this controller just doesn't implement all of the
> behavior of the official controller.  I'm not sure how to fix it if it's
> not obeying the instructions in valid output reports and I don't have
> one to test personally.  Do the lights flash properly when the
> controller is used with a PS3?

I presume that they just reversed engineered what they thought to be
correct, I don't have a Playstation so can't confirm for certain.

In my testing from a year ago or so, I could make the blink work via
sending data with python. The script can be found here:
http://www.spinics.net/lists/linux-input/msg28271.html

Note: "Which LED to enable(LED1..4 << 1, 0x20=none)"

Cheers,
Simon

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[Frank Praznik]

On 4/4/2014 12:11, simon@mungewell.org wrote:
> Why not make the triggers/etc apply across the board, so it doesn't matter
> which led the command is sent to - it's just registered/copied to the
> first (red?) led data[].

Unfortunately, it's not that simple.  There is no API for setting 
triggers from within the module and from looking at the code for these 
triggers, some set the hardware blink rate which will blink the whole 
light bar and some just use their own software timers and send 
brightness commands which will only effect the LED they are set on which 
results in unpredictable behavior depending on how the trigger is 
implemented.

These software-only triggers are why I'm thinking it would be good to 
have an extra global control for controlling the whole light bar 
synchronously.  Even if you set all the LEDs to the same software 
trigger the colors may not flash in sync if the trigger starts a 
separate timer for each LED and in some cases the trigger may mangle the 
user defined brightness settings (ie. the heartbeat trigger doesn't read 
the current brightness and just uses the maximum).

With an extra global switch, you can set the desired brightness levels 
for each color, then just use the global control to set the on/off state 
and get consistent blink behavior even if the trigger uses software 
timers or doesn't care about the user's brightness settings.  Plus, if 
for some reason you do want to flash individual colors, setting the 
blink rate of an individual color will always blink only that color 
instead of potentially blinking the whole light bar if the trigger tries 
to set the hardware blink rate.

I think it will be more functional, more predictable and less confusing.

>
>>> 3rd Party Intec - Was unable to get any controlled blinking.
>> It sounds like this controller just doesn't implement all of the
>> behavior of the official controller.  I'm not sure how to fix it if it's
>> not obeying the instructions in valid output reports and I don't have
>> one to test personally.  Do the lights flash properly when the
>> controller is used with a PS3?
> I presume that they just reversed engineered what they thought to be
> correct, I don't have a Playstation so can't confirm for certain.
>
> In my testing from a year ago or so, I could make the blink work via
> sending data with python. The script can be found here:
> http://www.spinics.net/lists/linux-input/msg28271.html
>
> Note: "Which LED to enable(LED1..4 << 1, 0x20=none)"
>
> Cheers,
> Simon
>

I'll take a look, but without having that particular controller all I 
can do is take a shot in the dark.

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[simon]

> On 4/4/2014 12:11, simon@mungewell.org wrote:
>> Why not make the triggers/etc apply across the board, so it doesn't
>> matter
>> which led the command is sent to - it's just registered/copied to the
>> first (red?) led data[].
>
> Unfortunately, it's not that simple.

It never is.... :-(

> There is no API for setting
> triggers from within the module and from looking at the code for these
> triggers, some set the hardware blink rate which will blink the whole
> light bar and some just use their own software timers and send
> brightness commands which will only effect the LED they are set on which
> results in unpredictable behavior depending on how the trigger is
> implemented.

I see the driver has a special case/function for setting blink with
'sony_led_blink_set()' which could be made to do funky stuff but that
might very rapidly become messy.

> These software-only triggers are why I'm thinking it would be good to
> have an extra global control for controlling the whole light bar
> synchronously.

A while ago (summer 2012) I messed around with the idea of 'chaining'
(grouping) leds together into a linked list, such that any actions on the
head of the list were duplicated to all leds in the chain.

The chain was created by a new trigger 'chain' and given the parameter of
the led to follow. I got as far as some code (wanted to learn about linked
lists in the kernel), but don't remember whether it worked properly on
not.

In this use case we could 'chain' the red->blue, and then have the blue
trigger set as 'timer' (or 'heartbeat') to cause a 'off->purple' blinking.
This would remove the contention in your scheme of having two places/leds
('red' and 'all') which could be set differently.

I can see if there's still some code on my disk if others are interested
in the idea.
Simon.

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[Frank Praznik]

On 4/4/2014 17:13, simon@mungewell.org wrote:
> I see the driver has a special case/function for setting blink with 
> 'sony_led_blink_set()' which could be made to do funky stuff but that 
> might very rapidly become messy.
>> These software-only triggers are why I'm thinking it would be good to
>> have an extra global control for controlling the whole light bar
>> synchronously.
> A while ago (summer 2012) I messed around with the idea of 'chaining'
> (grouping) leds together into a linked list, such that any actions on the
> head of the list were duplicated to all leds in the chain.
>
> The chain was created by a new trigger 'chain' and given the parameter of
> the led to follow. I got as far as some code (wanted to learn about linked
> lists in the kernel), but don't remember whether it worked properly on
> not.
>
> In this use case we could 'chain' the red->blue, and then have the blue
> trigger set as 'timer' (or 'heartbeat') to cause a 'off->purple' blinking.
> This would remove the contention in your scheme of having two places/leds
> ('red' and 'all') which could be set differently.
>
> I can see if there's still some code on my disk if others are interested
> in the idea.
> Simon.

This sounds kind of like how things work now: The blink function maps 
all blink requests to any of the LEDs to one global value and uses that 
to set the hardware delay on/off values.

I'd be interested in seeing your solution if you still have it, but that 
still sounds like it still wouldn't work in the case of something like 
the heartbeat trigger which doesn't actually use the blink function, 
just runs it's own internal timer and just changes the brightness every 
so often.  The LED code in the module doesn't know where the commands 
are coming from, just that it's being told to change brightness so it 
wouldn't know which commands come from a trigger and should effect 
everything vs which are user commands and should just change the 
brightness of that specific LED.

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[simon]

[-- Attachment #1: Type: text/plain, Size: 1569 bytes --]

Unfortunately I was unable to find the 'led chaining' code I mentioned. I
guess it has been lost to the sands of time....

>> 3rd Party Intec - Was unable to get any controlled blinking. As
>> previously
>> mentioned all leds flash (automatic, as if first plugged in) whenever
>> all
>> leds are turned off.
>> I can control all leds in a static on/off mode, but can't set any
>> blinking/timer behaviour
>
> It sounds like this controller just doesn't implement all of the
> behavior of the official controller.  I'm not sure how to fix it if it's
> not obeying the instructions in valid output reports and I don't have
> one to test personally.  Do the lights flash properly when the
> controller is used with a PS3?

I was able to patch the code so that I could turn all LEDs off my Intec
controller, see attached. Tested this against the Intec and the SixAxis.

I was not able to make the LEDs flash in a controlled fashion, so you
might be right about the controller not working properly - although my
comments in python script suggests I did have something going.... must be
missing a snippet of info.


I also found that on the SixAxis, when I reported that I had to set 1st
LED off before I could set it on.... this only applies when the controller
was off and then plugged into USB. At this point the LEDs are slow
flashing, and the 'brightness' reports as 1 (driver code only writes to
device if value is changed).

If the device was already on (LEDs flashing) then only the 1st LED is set
on plug in, and I can turn LEDs off/on straight away.

Cheers,
Simon

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-HID-hid-sony-allow-3rd-party-INTEC-controller-to-tur.patch --]
[-- Type: text/x-patch; name="0001-HID-hid-sony-allow-3rd-party-INTEC-controller-to-tur.patch", Size: 1122 bytes --]

>From a0597309d26ddecb5d4662d9e3bcb4d2689b7ed5 Mon Sep 17 00:00:00 2001
From: Simon Wood <simon@mungewell.org>
Date: Tue, 8 Apr 2014 21:39:59 -0600
Subject: [PATCH] HID: hid-sony - allow 3rd party INTEC controller to turn off
 all leds

Without this patch the 3rd party INTEC (PS3) controller will blink all
leds when user turns them off, it appears to require an extra flag set.

Signed-off-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index aa5ece5..45cb8b6 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1423,6 +1423,10 @@ static void sixaxis_state_worker(struct work_struct *work)
 	report.data.leds_bitmap |= sc->led_state[2] << 3;
 	report.data.leds_bitmap |= sc->led_state[3] << 4;
 
+	/* Set flag for all leds off, required for 3rd party INTEC controller */
+	if ((report.data.leds_bitmap & 0x1E) == 0)
+		report.data.leds_bitmap |= 0x20;
+
 	/*
 	 * The LEDs in the report are indexed in reverse order to their
 	 * corresponding light on the controller.
-- 
1.8.1.2

Re: [PATCH v4 7/7] HID: sony: Add blink support to the Sixaxis and DualShock 4 LEDs

[Frank Praznik]

On 4/9/2014 01:51, simon@mungewell.org wrote:
> Unfortunately I was unable to find the 'led chaining' code I mentioned. I
> guess it has been lost to the sands of time....
>
>>> 3rd Party Intec - Was unable to get any controlled blinking. As
>>> previously
>>> mentioned all leds flash (automatic, as if first plugged in) whenever
>>> all
>>> leds are turned off.
>>> I can control all leds in a static on/off mode, but can't set any
>>> blinking/timer behaviour
>> It sounds like this controller just doesn't implement all of the
>> behavior of the official controller.  I'm not sure how to fix it if it's
>> not obeying the instructions in valid output reports and I don't have
>> one to test personally.  Do the lights flash properly when the
>> controller is used with a PS3?
> I was able to patch the code so that I could turn all LEDs off my Intec
> controller, see attached. Tested this against the Intec and the SixAxis.

Thanks for taking care of this.  I'll add it to the v5 series when I 
send it in a couple of days.

>
> I was not able to make the LEDs flash in a controlled fashion, so you
> might be right about the controller not working properly - although my
> comments in python script suggests I did have something going.... must be
> missing a snippet of info.
>
>
> I also found that on the SixAxis, when I reported that I had to set 1st
> LED off before I could set it on.... this only applies when the controller
> was off and then plugged into USB. At this point the LEDs are slow
> flashing, and the 'brightness' reports as 1 (driver code only writes to
> device if value is changed).
>
> If the device was already on (LEDs flashing) then only the 1st LED is set
> on plug in, and I can turn LEDs off/on straight away.

Yeah, the controller overriding user settings and flashing by itself 
until the PS button is pushed is a particularly annoying aspect of the 
Sixaxis on USB and there is no perfect way to handle it.  I wish that I 
could just submit a report from within the driver the first time the PS 
button is pressed to restore the LED state, but that could mess with 
hidraw applications.  The next-best solution would probably be to not 
filter redundant LED state settings for Sixaxis controllers on USB.  
This way, userland can stop the flashing without having to toggle an LED 
on/off or off/on if they are already set to the desired state.  I'll 
make the change for v5.

Thanks again for all of the testing :)