[PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[Sven Eckelmann]

Hi,

this is a resent of the patchset "[PATCHv3 0/5] HID: sony: Dualshock3 USB FF
deadlock fix and LED support". It is necessary because the patches 2-5 of that
patchset cannot be applied anymore. This happened because the v2 of the patch
"HID: sony: Send FF commands in non-atomic context" instead of the v3 was
applied in the repository jikos/hid.git. The differences between these two
patch versions can now be found in the new patch "[PATCHv4 1/5] HID: sony:
Rename rumble_* functions/variables to state_*"

Kind regards,
	Sven

[PATCHv4 1/5] HID: sony: Rename rumble_* functions/variables to state_*

[Sven Eckelmann]

The commands used to modify the rumble motor state also modifies the LEDs at
the same time. The functionality used to modify this state in the driver has to
be shared between the rumble and LED part. It is therefore better to replace
the "rumble" part of the names with "state".

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 098af2f8..28b847a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -226,7 +226,7 @@ struct sony_sc {
 	unsigned long quirks;
 
 #ifdef CONFIG_SONY_FF
-	struct work_struct rumble_worker;
+	struct work_struct state_worker;
 	struct hid_device *hdev;
 	__u8 left;
 	__u8 right;
@@ -622,9 +622,9 @@ static void buzz_remove(struct hid_device *hdev)
 }
 
 #ifdef CONFIG_SONY_FF
-static void sony_rumble_worker(struct work_struct *work)
+static void sony_state_worker(struct work_struct *work)
 {
-	struct sony_sc *sc = container_of(work, struct sony_sc, rumble_worker);
+	struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
 	unsigned char buf[] = {
 		0x01,
 		0x00, 0xff, 0x00, 0xff, 0x00,
@@ -655,7 +655,7 @@ static int sony_play_effect(struct input_dev *dev, void *data,
 	sc->left = effect->u.rumble.strong_magnitude / 256;
 	sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
 
-	schedule_work(&sc->rumble_worker);
+	schedule_work(&sc->state_worker);
 	return 0;
 }
 
@@ -667,7 +667,7 @@ static int sony_init_ff(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	sc->hdev = hdev;
-	INIT_WORK(&sc->rumble_worker, sony_rumble_worker);
+	INIT_WORK(&sc->state_worker, sony_state_worker);
 
 	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -677,7 +677,7 @@ static void sony_destroy_ff(struct hid_device *hdev)
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	cancel_work_sync(&sc->rumble_worker);
+	cancel_work_sync(&sc->state_worker);
 }
 
 #else
-- 
1.8.4.3

[PATCHv4 2/5] HID: sony: Use BIT(x) macro to define quirks constants

[Sven Eckelmann]

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 28b847a..0677292 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,11 +33,11 @@
 
 #include "hid-ids.h"
 
-#define VAIO_RDESC_CONSTANT     (1 << 0)
-#define SIXAXIS_CONTROLLER_USB  (1 << 1)
-#define SIXAXIS_CONTROLLER_BT   (1 << 2)
-#define BUZZ_CONTROLLER         (1 << 3)
-#define PS3REMOTE		(1 << 4)
+#define VAIO_RDESC_CONSTANT     BIT(0)
+#define SIXAXIS_CONTROLLER_USB  BIT(1)
+#define SIXAXIS_CONTROLLER_BT   BIT(2)
+#define BUZZ_CONTROLLER         BIT(3)
+#define PS3REMOTE		BIT(4)
 
 static const u8 sixaxis_rdesc_fixup[] = {
 	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
-- 
1.8.4.3

[PATCHv4 3/5] HID: sony: Rename buzz_* functions to sony_led_*

[Sven Eckelmann]

More controllers managed by the hid-sony module have 4 LEDs. These can share
most of the functionality provided by the buzz functions.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 0677292..cdb2419 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -466,7 +466,7 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
-static void buzz_led_set_brightness(struct led_classdev *led,
+static void sony_led_set_brightness(struct led_classdev *led,
 				    enum led_brightness value)
 {
 	struct device *dev = led->dev->parent;
@@ -498,7 +498,7 @@ static void buzz_led_set_brightness(struct led_classdev *led,
 	}
 }
 
-static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
+static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 {
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
@@ -525,7 +525,7 @@ static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
 	return on ? LED_FULL : LED_OFF;
 }
 
-static int buzz_init(struct hid_device *hdev)
+static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
 	struct buzz_extra *buzz;
@@ -567,8 +567,8 @@ static int buzz_init(struct hid_device *hdev)
 		led->name = name;
 		led->brightness = 0;
 		led->max_brightness = 1;
-		led->brightness_get = buzz_led_get_brightness;
-		led->brightness_set = buzz_led_set_brightness;
+		led->brightness_get = sony_led_get_brightness;
+		led->brightness_set = sony_led_set_brightness;
 
 		if (led_classdev_register(&hdev->dev, led)) {
 			hid_err(hdev, "Failed to register LED %d\n", n);
@@ -596,7 +596,7 @@ error_leds:
 	return ret;
 }
 
-static void buzz_remove(struct hid_device *hdev)
+static void sony_leds_remove(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
 	struct buzz_extra *buzz;
@@ -733,7 +733,7 @@ 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 & BUZZ_CONTROLLER)
-		ret = buzz_init(hdev);
+		ret = sony_leds_init(hdev);
 	else
 		ret = 0;
 
@@ -755,7 +755,7 @@ static void sony_remove(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	if (sc->quirks & BUZZ_CONTROLLER)
-		buzz_remove(hdev);
+		sony_leds_remove(hdev);
 
 	sony_destroy_ff(hdev);
 
-- 
1.8.4.3

[PATCHv4 4/5] HID: sony: Move LED data to the main structure

[Sven Eckelmann]

It is not necessary to keep the LED information in an extra struct which is
only used by the Buzz device. It can also be used by other devices.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 58 +++++++++++++++-----------------------------------
 1 file changed, 17 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index cdb2419..2f93aab 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -223,6 +223,7 @@ static const unsigned int buzz_keymap[] = {
 };
 
 struct sony_sc {
+	struct led_classdev *leds[4];
 	unsigned long quirks;
 
 #ifdef CONFIG_SONY_FF
@@ -232,12 +233,7 @@ struct sony_sc {
 	__u8 right;
 #endif
 
-	void *extra;
-};
-
-struct buzz_extra {
-	int led_state;
-	struct led_classdev *leds[4];
+	__u8 led_state;
 };
 
 static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -472,26 +468,24 @@ static void sony_led_set_brightness(struct led_classdev *led,
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 
 	int n;
 
 	drv_data = hid_get_drvdata(hdev);
-	if (!drv_data || !drv_data->extra) {
+	if (!drv_data) {
 		hid_err(hdev, "No device data\n");
 		return;
 	}
-	buzz = drv_data->extra;
 
 	for (n = 0; n < 4; n++) {
-		if (led == buzz->leds[n]) {
-			int on = !! (buzz->led_state & (1 << n));
+		if (led == drv_data->leds[n]) {
+			int on = !!(drv_data->led_state & (1 << n));
 			if (value == LED_OFF && on) {
-				buzz->led_state &= ~(1 << n);
-				buzz_set_leds(hdev, buzz->led_state);
+				drv_data->led_state &= ~(1 << n);
+				buzz_set_leds(hdev, drv_data->led_state);
 			} else if (value != LED_OFF && !on) {
-				buzz->led_state |= (1 << n);
-				buzz_set_leds(hdev, buzz->led_state);
+				drv_data->led_state |= (1 << n);
+				buzz_set_leds(hdev, drv_data->led_state);
 			}
 			break;
 		}
@@ -503,21 +497,19 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 
 	int n;
 	int on = 0;
 
 	drv_data = hid_get_drvdata(hdev);
-	if (!drv_data || !drv_data->extra) {
+	if (!drv_data) {
 		hid_err(hdev, "No device data\n");
 		return LED_OFF;
 	}
-	buzz = drv_data->extra;
 
 	for (n = 0; n < 4; n++) {
-		if (led == buzz->leds[n]) {
-			on = !! (buzz->led_state & (1 << n));
+		if (led == drv_data->leds[n]) {
+			on = !!(drv_data->led_state & (1 << n));
 			break;
 		}
 	}
@@ -528,7 +520,6 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 	int n, ret = 0;
 	struct led_classdev *led;
 	size_t name_sz;
@@ -541,13 +532,6 @@ static int sony_leds_init(struct hid_device *hdev)
 	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
 		return -ENODEV;
 
-	buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
-	if (!buzz) {
-		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
-		return -ENOMEM;
-	}
-	drv_data->extra = buzz;
-
 	/* 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 */
@@ -576,49 +560,41 @@ static int sony_leds_init(struct hid_device *hdev)
 			goto error_leds;
 		}
 
-		buzz->leds[n] = led;
+		drv_data->leds[n] = led;
 	}
 
 	return ret;
 
 error_leds:
 	for (n = 0; n < 4; n++) {
-		led = buzz->leds[n];
-		buzz->leds[n] = NULL;
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
 		if (!led)
 			continue;
 		led_classdev_unregister(led);
 		kfree(led);
 	}
 
-	kfree(drv_data->extra);
-	drv_data->extra = NULL;
 	return ret;
 }
 
 static void sony_leds_remove(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 	struct led_classdev *led;
 	int n;
 
 	drv_data = hid_get_drvdata(hdev);
 	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
 
-	buzz = drv_data->extra;
-
 	for (n = 0; n < 4; n++) {
-		led = buzz->leds[n];
-		buzz->leds[n] = NULL;
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
 		if (!led)
 			continue;
 		led_classdev_unregister(led);
 		kfree(led);
 	}
-
-	kfree(drv_data->extra);
-	drv_data->extra = NULL;
 }
 
 #ifdef CONFIG_SONY_FF
-- 
1.8.4.3

[PATCHv4 5/5] HID: sony: Add LED support for Sixaxis/Dualshock3 USB

[Sven Eckelmann]

The PS3 Sixaxis controller has 4 LEDs which can be controlled using the same
command as the rumble functionality. It seems not to be possible to only change
the LED without modifying the rumble motor state. Thus both have to be stored
on the host and retransmitted when either the LED or rumble state is changed.

Third party controllers may not support to disable all LEDs at once. These
controllers automatically switch to blinking of all LEDs when no LED is active
anymore.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Tested-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c | 118 +++++++++++++++++++++++++++++--------------------
 1 file changed, 71 insertions(+), 47 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 2f93aab..b60bc38 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -39,6 +39,8 @@
 #define BUZZ_CONTROLLER         BIT(3)
 #define PS3REMOTE		BIT(4)
 
+#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)
+
 static const u8 sixaxis_rdesc_fixup[] = {
 	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
 	0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF,
@@ -223,12 +225,12 @@ static const unsigned int buzz_keymap[] = {
 };
 
 struct sony_sc {
+	struct hid_device *hdev;
 	struct led_classdev *leds[4];
 	unsigned long quirks;
+	struct work_struct state_worker;
 
 #ifdef CONFIG_SONY_FF
-	struct work_struct state_worker;
-	struct hid_device *hdev;
 	__u8 left;
 	__u8 right;
 #endif
@@ -462,6 +464,18 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
+static void sony_set_leds(struct hid_device *hdev, __u8 leds)
+{
+	struct sony_sc *drv_data = hid_get_drvdata(hdev);
+
+	if (drv_data->quirks & BUZZ_CONTROLLER) {
+		buzz_set_leds(hdev, leds);
+	} else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) {
+		drv_data->led_state = leds;
+		schedule_work(&drv_data->state_worker);
+	}
+}
+
 static void sony_led_set_brightness(struct led_classdev *led,
 				    enum led_brightness value)
 {
@@ -482,10 +496,10 @@ static void sony_led_set_brightness(struct led_classdev *led,
 			int on = !!(drv_data->led_state & (1 << n));
 			if (value == LED_OFF && on) {
 				drv_data->led_state &= ~(1 << n);
-				buzz_set_leds(hdev, drv_data->led_state);
+				sony_set_leds(hdev, drv_data->led_state);
 			} else if (value != LED_OFF && !on) {
 				drv_data->led_state |= (1 << n);
-				buzz_set_leds(hdev, drv_data->led_state);
+				sony_set_leds(hdev, drv_data->led_state);
 			}
 			break;
 		}
@@ -517,6 +531,25 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	return on ? LED_FULL : LED_OFF;
 }
 
+static void sony_leds_remove(struct hid_device *hdev)
+{
+	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));
+
+	for (n = 0; n < 4; n++) {
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
+		if (!led)
+			continue;
+		led_classdev_unregister(led);
+		kfree(led);
+	}
+}
+
 static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
@@ -524,20 +557,29 @@ static int sony_leds_init(struct hid_device *hdev)
 	struct led_classdev *led;
 	size_t name_sz;
 	char *name;
+	size_t name_len;
+	const char *name_fmt;
 
 	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+	BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
 
-	/* Validate expected report characteristics. */
-	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
-		return -ENODEV;
+	if (drv_data->quirks & BUZZ_CONTROLLER) {
+		name_len = strlen("::buzz#");
+		name_fmt = "%s::buzz%d";
+		/* Validate expected report characteristics. */
+		if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+			return -ENODEV;
+	} else {
+		name_len = strlen("::sony#");
+		name_fmt = "%s::sony%d";
+	}
 
 	/* 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 */
-	buzz_set_leds(hdev, 0x00);
+	sony_set_leds(hdev, 0x00);
 
-	name_sz = strlen(dev_name(&hdev->dev)) + strlen("::buzz#") + 1;
+	name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
 
 	for (n = 0; n < 4; n++) {
 		led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
@@ -547,7 +589,7 @@ static int sony_leds_init(struct hid_device *hdev)
 		}
 
 		name = (void *)(&led[1]);
-		snprintf(name, name_sz, "%s::buzz%d", dev_name(&hdev->dev), n + 1);
+		snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1);
 		led->name = name;
 		led->brightness = 0;
 		led->max_brightness = 1;
@@ -566,45 +608,18 @@ static int sony_leds_init(struct hid_device *hdev)
 	return ret;
 
 error_leds:
-	for (n = 0; n < 4; n++) {
-		led = drv_data->leds[n];
-		drv_data->leds[n] = NULL;
-		if (!led)
-			continue;
-		led_classdev_unregister(led);
-		kfree(led);
-	}
+	sony_leds_remove(hdev);
 
 	return ret;
 }
 
-static void sony_leds_remove(struct hid_device *hdev)
-{
-	struct sony_sc *drv_data;
-	struct led_classdev *led;
-	int n;
-
-	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
-
-	for (n = 0; n < 4; n++) {
-		led = drv_data->leds[n];
-		drv_data->leds[n] = NULL;
-		if (!led)
-			continue;
-		led_classdev_unregister(led);
-		kfree(led);
-	}
-}
-
-#ifdef CONFIG_SONY_FF
 static void sony_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, 0x03,
+		0x00, 0x00, 0x00, 0x00, 0x00,
 		0xff, 0x27, 0x10, 0x00, 0x32,
 		0xff, 0x27, 0x10, 0x00, 0x32,
 		0xff, 0x27, 0x10, 0x00, 0x32,
@@ -612,13 +627,18 @@ static void sony_state_worker(struct work_struct *work)
 		0x00, 0x00, 0x00, 0x00, 0x00
 	};
 
+#ifdef CONFIG_SONY_FF
 	buf[3] = sc->right;
 	buf[5] = sc->left;
+#endif
+
+	buf[10] |= (sc->led_state & 0xf) << 1;
 
 	sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
 					HID_OUTPUT_REPORT);
 }
 
+#ifdef CONFIG_SONY_FF
 static int sony_play_effect(struct input_dev *dev, void *data,
 			    struct ff_effect *effect)
 {
@@ -640,10 +660,6 @@ static int sony_init_ff(struct hid_device *hdev)
 	struct hid_input *hidinput = list_entry(hdev->inputs.next,
 						struct hid_input, list);
 	struct input_dev *input_dev = hidinput->input;
-	struct sony_sc *sc = hid_get_drvdata(hdev);
-
-	sc->hdev = hdev;
-	INIT_WORK(&sc->state_worker, sony_state_worker);
 
 	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -682,6 +698,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	sc->quirks = quirks;
 	hid_set_drvdata(hdev, sc);
+	sc->hdev = hdev;
 
 	ret = hid_parse(hdev);
 	if (ret) {
@@ -705,23 +722,30 @@ 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);
+		INIT_WORK(&sc->state_worker, sony_state_worker);
 	}
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
-	else if (sc->quirks & BUZZ_CONTROLLER)
-		ret = sony_leds_init(hdev);
 	else
 		ret = 0;
 
 	if (ret < 0)
 		goto err_stop;
 
+	if (sc->quirks & SONY_LED_SUPPORT) {
+		ret = sony_leds_init(hdev);
+		if (ret < 0)
+			goto err_stop;
+	}
+
 	ret = sony_init_ff(hdev);
 	if (ret < 0)
 		goto err_stop;
 
 	return 0;
 err_stop:
+	if (sc->quirks & SONY_LED_SUPPORT)
+		sony_leds_remove(hdev);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -730,7 +754,7 @@ static void sony_remove(struct hid_device *hdev)
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	if (sc->quirks & BUZZ_CONTROLLER)
+	if (sc->quirks & SONY_LED_SUPPORT)
 		sony_leds_remove(hdev);
 
 	sony_destroy_ff(hdev);
-- 
1.8.4.3

Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[Jiri Kosina]

On Tue, 19 Nov 2013, Sven Eckelmann wrote:

> this is a resent of the patchset "[PATCHv3 0/5] HID: sony: Dualshock3 USB FF
> deadlock fix and LED support". It is necessary because the patches 2-5 of that
> patchset cannot be applied anymore. This happened because the v2 of the patch
> "HID: sony: Send FF commands in non-atomic context" instead of the v3 was
> applied in the repository jikos/hid.git. The differences between these two
> patch versions can now be found in the new patch "[PATCHv4 1/5] HID: sony:
> Rename rumble_* functions/variables to state_*"

Queued for 3.14. Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[simon]

> On Tue, 19 Nov 2013, Sven Eckelmann wrote:
>
>> this is a resent of the patchset "[PATCHv3 0/5] HID: sony: Dualshock3
>> USB FF
>> deadlock fix and LED support". It is necessary because the patches 2-5
>> of that
>> patchset cannot be applied anymore. This happened because the v2 of the
>> patch
>> "HID: sony: Send FF commands in non-atomic context" instead of the v3
>> was
>> applied in the repository jikos/hid.git. The differences between these
>> two
>> patch versions can now be found in the new patch "[PATCHv4 1/5] HID:
>> sony:
>> Rename rumble_* functions/variables to state_*"
>
> Queued for 3.14. Thanks,

I'm a little confused about which kernel this should be in. I believe the
first in this patch series is required for 3.13 (but does not seemed to be
included (1)).

Without this, the kernel will be subject to lockups if anyone actually
uses the force feedback.
Simon

(1) -
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/drivers/hid/hid-sony.c

Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[Sven Eckelmann]

On Friday 22 November 2013 21:35:28 simon@mungewell.org wrote:
> I'm a little confused about which kernel this should be in. I believe the
> first in this patch series is required for 3.13 (but does not seemed to be
> included (1)).
> 
> Without this, the kernel will be subject to lockups if anyone actually
> uses the force feedback.
> Simon

No, the first patch is for 3.14 and the rest is also for 3.13. The fix for the
lockup is in the patch "[PATCHv3 0/5] HID: sony: Dualshock3 USB FF deadlock
fix and LED support" and its v2 version. Unfortunately, the v2 was applied for
3.13 and the first patch in this series just contains the renaming stuff
between v2 and v3 of this patch.

The pull request may still have to be made by Jiri. But it is already queued up
in his 3.13 fixes branch [1]

Kind regards,
	Sven

[1] https://git.kernel.org/cgit/linux/kernel/git/jikos/hid.git/log/?h=for-3.13/upstream-fixes

Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support

[Jiri Kosina]

On Fri, 22 Nov 2013, simon@mungewell.org wrote:

> I'm a little confused about which kernel this should be in. I believe the
> first in this patch series is required for 3.13 (but does not seemed to be
> included (1)).
> 
> Without this, the kernel will be subject to lockups if anyone actually
> uses the force feedback.

Yes, the lockup fix is queued for 3.13, the rest for 3.14. See 
for-3.13/upstream-fixes and for-3.14/sony branches in hid.git.

for-3.13/upstream-fixes will be going to Linus this week.

-- 
Jiri Kosina
SUSE Labs