diff mbox

Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB

Message ID 2105006.tGLFZqe9L3@sven-desktop (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

Sven Eckelmann Nov. 18, 2013, 12:26 a.m. UTC 
On Monday 18 November 2013 00:53:25 Sven Eckelmann wrote:
[..]
> Ok, just tried it and it seems this byte is really for the LEDs. But
> unfortunately, it is enabling/disabling the LEDs completely and not the
> configuration.
> 
> And sending less bytes just lets everything fail.

Forgot to add my proof of concept patch for the LED control. I've attached it 
so you can also try it out.

@Simon: This may also be interesting for you because you've asked for it in 
8c40cb08b7c44f373c2c533614d70b6a.squirrel@mungewell.org

Kind regards,
	Sven

Comments

simon@mungewell.org Nov. 18, 2013, 1:21 a.m. UTC | #1 
> @Simon: This may also be interesting for you because you've asked for it
> in 8c40cb08b7c44f373c2c533614d70b6a.squirrel@mungewell.org

Yes, very interesting. Builds on top of the other 2 patches.

This seems to work, however something is causing the LEDs to go out once
they have been set. This seems to happen a few seconds after they are set,
but there does not appear too be USB activity (via usbmon).

If 'testrumble' is used then the previously set LEDs come back on.


I note that the LED class directory is based on USB ID and node, which
changes each time the controller is plugged in. This makes assigning LEDs
in game configurations a pain.

From my point of view a 'fixed UID' (such as a serial number) would be
better, but most controllers don't report these. Any suggestions?

Simon


--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
simon@mungewell.org Nov. 18, 2013, 3:54 a.m. UTC | #2 
> This seems to work, however something is causing the LEDs to go out once
> they have been set. This seems to happen a few seconds after they are set,
> but there does not appear too be USB activity (via usbmon).

I tested this on a real machine (as opposed to VM box), and the LEDs
remain set and do not auto cancel. Suspect host OS was doing something to
device.

There is another bit of weirdness though, when plugged in my Intec
controller slow flashes the LEDs. This is overwritten when I set any LED
through the LED class, but returns to flashing when I clear all LEDs via
the class.

IE. there is no way to turn all the the LEDs off.

The DualShock3/SixAxis behaves slightly differently in that the flashing
does not return when all the LEDs are turn off.

From some of my earlier investigations I had some notes about LED settings
which might be of help.
--
# \x01                          Header
# \x00\x00\x00\x00\x00          ForceFeedback
(\x00\xfe\x00+<rumble,40=weak,80=strong>
# \x00\x00\x00\x00\x1E          Which LED to enable(LED1..4 << 1, 0x20=none)
# \xff\x27\x10\x00\x32          LED 1 (rate,duty factor
# \xff\x27\x10\x00\x32          LED 2
# \xff\x27\x10\x00\x32          LED 3
# \xff\x27\x10\x00\x32          LED 4
# \x00\x00\x00\x00\x00          Footer

#        * the total time the led is active (0xff means forever)
#        * |     duty_length: how long a cycle is in deciseconds (0 means
"blink really fast")
#        * |     |     ??? (Some form of phase shift or duty_length
multiplier?)
#        * |     |     |     % of duty_length the led is off (0xff means
100%)
#        * |     |     |     |     % of duty_length the led is on (0xff
mean 100%)
#        * |     |     |     |     |
#        * 0xff, 0x27, 0x10, 0x00, 0x32,
--

Attached is a script which works with my Intec, but not with the SixAxis.
Simon
Antonio Ospite Nov. 18, 2013, 10:27 a.m. UTC | #3 
On Sun, 17 Nov 2013 20:21:04 -0500
simon@mungewell.org wrote:

[...]
> I note that the LED class directory is based on USB ID and node, which
> changes each time the controller is plugged in. This makes assigning LEDs
> in game configurations a pain.
> 
> From my point of view a 'fixed UID' (such as a serial number) would be
> better, but most controllers don't report these. Any suggestions?
> 

The bluez plugin uses libudev, so it knows how to address the right
device when setting the LED.

Ciao,
   Antonio
diff mbox

Patch

From 94d33d53718268d3f41f9ec38105e221e0ba3585 Mon Sep 17 00:00:00 2001
From: Sven Eckelmann <sven@narfation.org>
Date: Mon, 18 Nov 2013 01:22:39 +0100
Subject: [RFC] HID: sony: Make sixaxis usb LEDs configurable

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

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 098af2f8..d1d99bb 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -224,18 +224,13 @@  static const unsigned int buzz_keymap[] = {
 
 struct sony_sc {
 	unsigned long quirks;
+	struct work_struct state_worker;
+	struct hid_device *hdev;
 
 #ifdef CONFIG_SONY_FF
-	struct work_struct rumble_worker;
-	struct hid_device *hdev;
 	__u8 left;
 	__u8 right;
 #endif
-
-	void *extra;
-};
-
-struct buzz_extra {
 	int led_state;
 	struct led_classdev *leds[4];
 };
@@ -466,58 +461,66 @@  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_set_leds(struct hid_device *hdev, int 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)
 {
 	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);
+				sony_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);
+				sony_set_leds(hdev, drv_data->led_state);
 			}
 			break;
 		}
 	}
 }
 
-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);
 	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;
 		}
 	}
@@ -525,35 +528,36 @@  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;
 	int n, ret = 0;
 	struct led_classdev *led;
 	size_t name_sz;
 	char *name;
+	size_t name_len;
+	const char *name_format;
 
 	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+	BUG_ON(!(drv_data->quirks & (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)));
 
-	/* Validate expected report characteristics. */
-	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;
+	if (drv_data->quirks & BUZZ_CONTROLLER) {
+		name_len = strlen("::buzz#");
+		name_format = "%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_format = "%s::sony%d";
 	}
-	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 */
-	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);
@@ -563,12 +567,12 @@  static int buzz_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_format, dev_name(&hdev->dev), n + 1);
 		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);
@@ -576,73 +580,71 @@  static int buzz_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 buzz_remove(struct hid_device *hdev)
+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));
+	BUG_ON(!(drv_data->quirks & (SIXAXIS_CONTROLLER_USB | 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
-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,
-		0x00, 0x00, 0x00, 0x00, 0x03,
+		0x00, 0xff, 0x00, 0xf0, 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
 	};
+	size_t len = sizeof(buf);
 
+#ifdef CONFIG_SONY_FF
 	buf[3] = sc->right;
 	buf[5] = sc->left;
+#endif
 
-	sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+	buf[10] |= (sc->led_state & 0xf) << 1;
+
+	sc->hdev->hid_output_raw_report(sc->hdev, buf, len,
 					HID_OUTPUT_REPORT);
 }
 
+#ifdef CONFIG_SONY_FF
 static int sony_play_effect(struct input_dev *dev, void *data,
 			    struct ff_effect *effect)
 {
@@ -655,7 +657,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;
 }
 
@@ -664,10 +666,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->rumble_worker, sony_rumble_worker);
 
 	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -677,7 +675,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
@@ -706,6 +704,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) {
@@ -729,17 +728,22 @@  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 = buzz_init(hdev);
 	else
 		ret = 0;
 
 	if (ret < 0)
 		goto err_stop;
 
+	if (sc->quirks & (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_USB)) {
+		ret = sony_leds_init(hdev);
+		if (ret < 0)
+			goto err_stop;
+	}
+
 	ret = sony_init_ff(hdev);
 	if (ret < 0)
 		goto err_stop;
@@ -754,8 +758,8 @@  static void sony_remove(struct hid_device *hdev)
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	if (sc->quirks & BUZZ_CONTROLLER)
-		buzz_remove(hdev);
+	if (sc->quirks & (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER))
+		sony_leds_remove(hdev);
 
 	sony_destroy_ff(hdev);
 
-- 
1.8.4.3