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

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

[Sven Eckelmann]

Sony Dualshock 3 controllers have two motors which can be used to provide
simple force feedback rumble effects. The right motor is can be used to create
a weak rumble effect but does not allow to set the force. The left motor is
used to create a strong rumble effect with adjustable intensity.

The state of both motors can be changed using HID_OUTPUT_REPORT packets and
have no timing information. FF memless is used to keep track of the timing and
the sony driver just generates the necessary URBs.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/Kconfig    |  8 ++++++++
 drivers/hid/hid-sony.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 60 insertions(+)

The device will not have any reports. Thus the payload is generated on the fly
as it is already done in different other places in the driver.

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index a27e531..329fbb9 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -618,6 +618,14 @@ config HID_SONY
 	  * Sony PS3 Blue-ray Disk Remote Control (Bluetooth)
 	  * Logitech Harmony adapter for Sony Playstation 3 (Bluetooth)
 
+config SONY_FF
+	bool "Sony PS2/3 accessories force feedback support"
+	depends on HID_SONY
+	select INPUT_FF_MEMLESS
+	---help---
+	Say Y here if you have a Sony PS2/3 accessory and want to enable force
+	feedback support for it.
+
 config HID_SPEEDLINK
 	tristate "Speedlink VAD Cezanne mouse support"
 	depends on HID
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index bc37a18..da551d1 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -614,6 +614,54 @@ static void buzz_remove(struct hid_device *hdev)
 	drv_data->extra = NULL;
 }
 
+#ifdef CONFIG_SONY_FF
+static int sony_play_effect(struct input_dev *dev, void *data,
+			    struct ff_effect *effect)
+{
+	unsigned char buf[] = {
+		0x01,
+		0x00, 0xff, 0x00, 0xff, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x03,
+		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
+	};
+	__u8 left;
+	__u8 right;
+	struct hid_device *hid = input_get_drvdata(dev);
+
+	if (effect->type != FF_RUMBLE)
+		return 0;
+
+	left = effect->u.rumble.strong_magnitude / 256;
+	right = effect->u.rumble.weak_magnitude ? 1 : 0;
+
+	buf[3] = right;
+	buf[5] = left;
+
+	return hid->hid_output_raw_report(hid, buf, sizeof(buf),
+					  HID_OUTPUT_REPORT);
+}
+
+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;
+
+	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
+	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
+}
+
+#else
+static int sony_init_ff(struct hid_device *hdev)
+{
+	return 0;
+}
+#endif
+
 static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	int ret;
@@ -663,6 +711,10 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (ret < 0)
 		goto err_stop;
 
+	ret = sony_init_ff(hdev);
+	if (ret < 0)
+		goto err_stop;
+
 	return 0;
 err_stop:
 	hid_hw_stop(hdev);
-- 
1.8.4.2

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

[Jiri Kosina]

On Sat, 9 Nov 2013, Sven Eckelmann wrote:

> Sony Dualshock 3 controllers have two motors which can be used to provide
> simple force feedback rumble effects. The right motor is can be used to create
> a weak rumble effect but does not allow to set the force. The left motor is
> used to create a strong rumble effect with adjustable intensity.
> 
> The state of both motors can be changed using HID_OUTPUT_REPORT packets and
> have no timing information. FF memless is used to keep track of the timing and
> the sony driver just generates the necessary URBs.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

Applied, thanks.

-- 
Jiri Kosina
SUSE Labs

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

[simon]

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

> Sony Dualshock 3 controllers have two motors which can be used to provide
> simple force feedback rumble effects. The right motor is can be used to
> create
> a weak rumble effect but does not allow to set the force. The left motor
> is
> used to create a strong rumble effect with adjustable intensity.

Hi,
This patch appears to work OK with my DualShock/SixAxis controller (USB
connection), but causes a machine lockup when used with my Intec wired
controller.

The Intec controller works OK up to the point when I start rumble,
sometimes the motor runs some times it doesn't. I am using SDL2's
'testhaptic' application.

Syslog log attached detail controller.

If you have any specific tests to run, please advise,
Simon

[-- Attachment #2: intec.txt --]
[-- Type: text/plain, Size: 1305 bytes --]

Nov 16 14:26:21 womble kernel: [   95.296039] usb 3-1: new full-speed USB device number 2 using uhci_hcd
Nov 16 14:26:22 womble kernel: [   95.463964] usb 3-1: New USB device found, idVendor=054c, idProduct=0268
Nov 16 14:26:22 womble kernel: [   95.463973] usb 3-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
Nov 16 14:26:22 womble kernel: [   95.463978] usb 3-1: Product: PLAYSTATION(R)3 Controller
Nov 16 14:26:22 womble kernel: [   95.463982] usb 3-1: Manufacturer: GASIA CORP.
Nov 16 14:26:22 womble mtp-probe: checking bus 3, device 2: "/sys/devices/pci0000:00/0000:00:1d.1/usb3/3-1"
Nov 16 14:26:22 womble mtp-probe: bus: 3, device: 2 was not an MTP device
Nov 16 14:26:22 womble kernel: [   95.770400] usbcore: registered new interface driver usbhid
Nov 16 14:26:22 womble kernel: [   95.770408] usbhid: USB HID core driver
Nov 16 14:26:22 womble kernel: [   95.838174] sony 0003:054C:0268.0001: Fixing up Sony Sixaxis report descriptor
Nov 16 14:26:22 womble kernel: [   95.848763] input: GASIA CORP. PLAYSTATION(R)3 Controller as /devices/pci0000:00/0000:00:1d.1/usb3/3-1/3-1:1.0/input/input7
Nov 16 14:26:22 womble kernel: [   95.849784] sony 0003:054C:0268.0001: input,hiddev0,hidraw0: USB HID v1.11 Joystick [GASIA CORP. PLAYSTATION(R)3 Controller] on usb-0000:00:1d.1-1/input0

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

[simon]

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


> The Intec controller works OK up to the point when I start rumble,
> sometimes the motor runs some times it doesn't. I am using SDL2's
> 'testhaptic' application.

That's not quite true. Both SixAxis and Intec lock up machine with
'testhaptic'.

Using 'testrumble' the SixAxis works multiple times, Intec locks up
straight away (sometimes starts motor).

USBMon shows Testrumble does (on SixAxis)
--
e9b23e40 2764496125 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff01ff
7f000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b23e40 2764507425 C Co:002:00 0 35 >
e99fe840 2766509714 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff00ff
00000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e99fe840 2766521086 C Co:002:00 0 35 >
e9b23cc0 2768521311 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff01ff
4c000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b23cc0 2768532755 C Co:002:00 0 35 >
e9b8e3c0 2770532954 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff00ff
00000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b8e3c0 2770544425 C Co:002:00 0 35 >
e9b8f780 2770545424 C Ii:002:01 -2 0
--

Attached is a script from a while ago, when I was trying to figure out how
to get the Intec to rumble. Which runs without crashing machine.

Simon

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: intec_rumble.py --]
[-- Type: text/x-python; name="intec_rumble.py", Size: 3000 bytes --]

#!/usr/bin/python
#
# Small script to test the rumble on a PS3 gamepad
#

import usb
import time

rumble = 0

busses = usb.busses()
for bus in busses:      
  devices = bus.devices   
  for dev in devices:
    if dev.idVendor == 0x054c and dev.idProduct == 0x0268:
      print "Found RumblePad"
      rumble = dev

if rumble == 0:
  print "RumblePad not found"
  exit(0)

# Get a device handler for the usb device and detach it from the kernel
dh = rumble.open()
try:
        dh.detachKernelDriver(0)
except:
        print "Already detached?"
dh.claimInterface(0)

print "Intec - Weak Rumble"
command='\x01\x00\xfe\x80\xfe\x40\x00\x00\x00\x00\x12\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)

print "Intec - Strong Rumble"
command='\x01\x00\xfe\xff\xfe\xff\x00\x00\x00\x00\x0C\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)

'''
New kernel driver - USBMon
e9b23e40 2764496125 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff01ff 7f000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b23e40 2764507425 C Co:002:00 0 35 >
e99fe840 2766509714 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff00ff 00000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e99fe840 2766521086 C Co:002:00 0 35 >
e9b23cc0 2768521311 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff01ff 4c000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b23cc0 2768532755 C Co:002:00 0 35 >
e9b8e3c0 2770532954 S Co:002:00 s 21 09 0201 0000 0023 35 = 00ff00ff 00000000 0003ff27 100032ff 27100032 ff271000 32ff2710 00320000
e9b8e3c0 2770544425 C Co:002:00 0 35 >
e9b8f780 2770545424 C Ii:002:01 -2 0

But code below does
eb2d3480 2972495664 S Io:002:02 -115 36 = 0100ff01 ff4c0000 000003ff 27100032 ff271000 32ff2710 0032ff27 10003200
eb2d3480 2972497404 C Io:002:02 0 36 >
e9b8dd80 2975502053 S Io:002:02 -115 36 = 0100ff01 ff7f0000 000003ff 27100032 ff271000 32ff2710 0032ff27 10003200
e9b8dd80 2975503910 C Io:002:02 0 36 >
e99e2600 2978507120 S Io:002:02 -115 36 = 0100fe00 fe000000 000020ff 27100032 ff271000 32ff2710 0032ff27 10003200
e99e2600 2978508406 C Io:002:02 0 36 >
'''

'''
print "Sony Six Axis - Weak"
command='\x01\x00\xff\x01\xff\x4c\x00\x00\x00\x00\x03\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)

print "Sony Six Axis - Strong"
command='\x01\x00\xff\x01\xff\x7f\x00\x00\x00\x00\x03\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)
'''

print "Cancel Rumble"
command='\x01\x00\xfe\x00\xfe\x00\x00\x00\x00\x00\x20\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))

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

[Sven Eckelmann]

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

On Saturday 16 November 2013 17:30:25 simon@mungewell.org wrote:
> This patch appears to work OK with my DualShock/SixAxis controller (USB
> connection), but causes a machine lockup when used with my Intec wired
> controller.
> 
> The Intec controller works OK up to the point when I start rumble,
> sometimes the motor runs some times it doesn't. I am using SDL2's
> 'testhaptic' application.

Thanks a lot for this bug report. The testhaptic was a good testcase because 
it is a heavy user and can reproduce the problem quite easily. I've only 
tested it using testrumble and own programs which didn't seem to trigger the 
problem here. The problem is easy to explain:

 * usb_control_msg/usb_interrupt_msg/usb_bulk_msg/... may sleep
 * sony_play_effect may gets called in an softirq context (atomic)

So it is a bad choice to use hid_output_raw_report (which calls 
usb_control_msg) in a ff_memless control function. I will just send a revert 
patch in some minutes.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[David Herrmann]

Hi

On Sun, Nov 17, 2013 at 10:36 AM, Sven Eckelmann <sven@narfation.org> wrote:
> On Saturday 16 November 2013 17:30:25 simon@mungewell.org wrote:
>> This patch appears to work OK with my DualShock/SixAxis controller (USB
>> connection), but causes a machine lockup when used with my Intec wired
>> controller.
>>
>> The Intec controller works OK up to the point when I start rumble,
>> sometimes the motor runs some times it doesn't. I am using SDL2's
>> 'testhaptic' application.
>
> Thanks a lot for this bug report. The testhaptic was a good testcase because
> it is a heavy user and can reproduce the problem quite easily. I've only
> tested it using testrumble and own programs which didn't seem to trigger the
> problem here. The problem is easy to explain:
>
>  * usb_control_msg/usb_interrupt_msg/usb_bulk_msg/... may sleep
>  * sony_play_effect may gets called in an softirq context (atomic)
>
> So it is a bad choice to use hid_output_raw_report (which calls
> usb_control_msg) in a ff_memless control function. I will just send a revert
> patch in some minutes.

Yeah, the input-ff callbacks cannot be handled inline. You also get
deadlocks with the input-spinlock. In the wiimote driver I simply
dispatch the ff-events to a workqueue. You can have a look at
drivers/hid/hid-wiimote-modules.c. You can get some ordering-problems
then, but these can usually be ignored as they just collapse events.

The related commit was:

commit f50f9aabf32db7414551ffdfdccc71be5f3d6e7d
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Wed Oct 2 13:47:28 2013 +0200

    HID: wiimote: fix FF deadlock

Thanks
David

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

[simon]

> So it is a bad choice to use hid_output_raw_report (which calls
> usb_control_msg) in a ff_memless control function. I will just send a
> revert
> patch in some minutes.

Rather than revert, can't we just replace the raw call with
'hid_hw_request()' to send the data?

This is what we have in hid-lg4ff:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-lg4ff.c?id=refs/tags/v3.12#n402

Simon

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

[Sven Eckelmann]

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

On Sunday 17 November 2013 12:38:43 simon@mungewell.org wrote:
> > So it is a bad choice to use hid_output_raw_report (which calls
> > usb_control_msg) in a ff_memless control function. I will just send a
> > revert
> > patch in some minutes.
> 
> Rather than revert, can't we just replace the raw call with
> 'hid_hw_request()' to send the data?

This device doesn't have any report and I had not the time to check how to 
correctly generate the fields manually for the report/check why it doesn't 
have any.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Sven Eckelmann]

[-- Attachment #1.1: Type: text/plain, Size: 1038 bytes --]

On Sunday 17 November 2013 17:30:51 David Herrmann wrote:
> Yeah, the input-ff callbacks cannot be handled inline. You also get
> deadlocks with the input-spinlock. In the wiimote driver I simply
> dispatch the ff-events to a workqueue. You can have a look at
> drivers/hid/hid-wiimote-modules.c. You can get some ordering-problems
> then, but these can usually be ignored as they just collapse events.
> 
> The related commit was:
> 
> commit f50f9aabf32db7414551ffdfdccc71be5f3d6e7d
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Wed Oct 2 13:47:28 2013 +0200
> 
>     HID: wiimote: fix FF deadlock

Yes, I've played around with the linux kernel usb message api and came to the 
same conclusion (for now). I've only tested it with a small proof of concept 
patch and it didn't hang anymore with testhaptic.

Maybe Simon Wood can test his devices because I am unsure whether PS3 
dualshock clones will work with the interrupt or control urbs. For example the 
script from Simon didn't work at all for me.

Kind regards,
	Sven

[-- Attachment #1.2: proof_of_concept_wq_rumble.patch --]
[-- Type: text/x-patch, Size: 3182 bytes --]

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index da551d1..d509447 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -224,6 +224,10 @@ static const unsigned int buzz_keymap[] = {
 
 struct sony_sc {
 	unsigned long quirks;
+	struct work_struct rumble_worker;
+	struct hid_device *hdev;
+	__u8 left;
+	__u8 right;
 
 	void *extra;
 };
@@ -614,35 +618,53 @@ static void buzz_remove(struct hid_device *hdev)
 	drv_data->extra = NULL;
 }
 
-#ifdef CONFIG_SONY_FF
-static int sony_play_effect(struct input_dev *dev, void *data,
-			    struct ff_effect *effect)
+static void sony_rumble_worker(struct work_struct *work)
 {
+	struct sony_sc *sc = container_of(work, struct sony_sc, rumble_worker);
 	unsigned char buf[] = {
 		0x01,
 		0x00, 0xff, 0x00, 0xff, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x03,
+		0x00, 0x00, 0x00, 0x00, 0x02,
 		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
+		0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00, 0x00, 0x00,
+		0x00, 0x00, 0x00
 	};
-	__u8 left;
-	__u8 right;
+	struct usb_interface *intf = to_usb_interface(sc->hdev->dev.parent);
+	struct usb_device *usbdev = interface_to_usbdev(intf);
+
+	buf[3] = sc->right;
+	buf[5] = sc->left;
+
+	if (sc->right || sc->left)
+		usb_interrupt_msg(usbdev, usb_sndctrlpipe(usbdev, 2), buf,
+				  sizeof(buf), NULL, USB_CTRL_SET_TIMEOUT);
+
+	buf[10] = 0x1e;
+	usb_interrupt_msg(usbdev, usb_sndctrlpipe(usbdev, 2), buf, sizeof(buf),
+			  NULL, USB_CTRL_SET_TIMEOUT);
+}
+
+#ifdef CONFIG_SONY_FF
+static int sony_play_effect(struct input_dev *dev, void *data,
+			    struct ff_effect *effect)
+{
 	struct hid_device *hid = input_get_drvdata(dev);
+	struct sony_sc *sc = hid_get_drvdata(hid);
 
 	if (effect->type != FF_RUMBLE)
 		return 0;
 
-	left = effect->u.rumble.strong_magnitude / 256;
-	right = effect->u.rumble.weak_magnitude ? 1 : 0;
+	sc->left = effect->u.rumble.strong_magnitude / 256;
+	sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
 
-	buf[3] = right;
-	buf[5] = left;
 
-	return hid->hid_output_raw_report(hid, buf, sizeof(buf),
-					  HID_OUTPUT_REPORT);
+	schedule_work(&sc->rumble_worker);
+	return 0;
 }
 
 static int sony_init_ff(struct hid_device *hdev)
@@ -650,6 +672,9 @@ 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);
+
+	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);
@@ -711,6 +736,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (ret < 0)
 		goto err_stop;
 
+	sc->hdev = hdev;
 	ret = sony_init_ff(hdev);
 	if (ret < 0)
 		goto err_stop;
@@ -728,6 +754,8 @@ static void sony_remove(struct hid_device *hdev)
 	if (sc->quirks & BUZZ_CONTROLLER)
 		buzz_remove(hdev);
 
+	cancel_work_sync(&sc->rumble_worker);
+
 	hid_hw_stop(hdev);
 }
 

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[simon]

> Maybe Simon Wood can test his devices because I am unsure whether PS3
> dualshock clones will work with the interrupt or control urbs. For example
> the script from Simon didn't work at all for me.

With the proof of concept both my devices work OK with 'testhaptic' and
'testrumble' - the controllers rumble and the computer does not lock up.

My script is from a couple of years ago, and it didn't work for me with
the dualshock.

Simon.

PS. We know that some of the bits of that data frame are used to control
the LEDs. Anyone interested on getting them working via the LED class?

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

[Antonio Ospite]

On Sun, 17 Nov 2013 10:36:55 +0100
Sven Eckelmann <sven@narfation.org> wrote:

> On Saturday 16 November 2013 17:30:25 simon@mungewell.org wrote:
> > This patch appears to work OK with my DualShock/SixAxis controller (USB
> > connection), but causes a machine lockup when used with my Intec wired
> > controller.
> > 
> > The Intec controller works OK up to the point when I start rumble,
> > sometimes the motor runs some times it doesn't. I am using SDL2's
> > 'testhaptic' application.
> 
> Thanks a lot for this bug report. The testhaptic was a good testcase because 
> it is a heavy user and can reproduce the problem quite easily. I've only 
> tested it using testrumble and own programs which didn't seem to trigger the 
> problem here. The problem is easy to explain:
> 
>  * usb_control_msg/usb_interrupt_msg/usb_bulk_msg/... may sleep
>  * sony_play_effect may gets called in an softirq context (atomic)
> 
> So it is a bad choice to use hid_output_raw_report (which calls 
> usb_control_msg) in a ff_memless control function. I will just send a revert 
> patch in some minutes.
>

Sven, if you are going to resubmit another patch for this functionality
(I've seen your v2 just before hitting "Send"), wouldn't it be better
to advertise just FF_RUMBLE? AFAICS your first patch results in this
(from evtest):

....
  Event type 21 (EV_FF)
    Event code 80 (FF_RUMBLE)
    Event code 81 (FF_PERIODIC)
    Event code 88 (FF_SQUARE)
    Event code 89 (FF_TRIANGLE)
    Event code 90 (FF_SINE)
    Event code 96 (FF_GAIN)

I don't know if this is how it should be, I know nothing about FF stuff.

Also please make sure that setting the rumble does not change the LEDs
status if there is any set: HID output report 1 is used for both LEDs
and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
to match the joystick number, just as the PS3 does, it would be strange
for the user if a rumble event would reset the LEDs status.

Maybe only send up to the rumble related bytes, or do a
read-modify-write if that is possible with HID output reports, if this
cannot be done we will have to design a solution to set LEDs too in the
kernel, in order to be able to keep some status around.

Last comment, if we want a conditional config what about using
CONFIG_HID_SONY_FF instead of CONFIG_SONY_FF? Not a big deal of course,
just a suggestion.

Sorry for the late comments but I got to see the patch only after Jiri
had already picked it up.

Thanks,
   Antonio

[1] http://ao2.it/tmp/playstation-peripheral-pugin-v5.x-2013-11-13.patch

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

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

[Sven Eckelmann]

On Sunday 17 November 2013 23:25:59 Antonio Ospite wrote:
> Sven, if you are going to resubmit another patch for this functionality
> (I've seen your v2 just before hitting "Send"), wouldn't it be better
> to advertise just FF_RUMBLE? AFAICS your first patch results in this
> (from evtest):
> 
> ....
>   Event type 21 (EV_FF)
>     Event code 80 (FF_RUMBLE)
>     Event code 81 (FF_PERIODIC)
>     Event code 88 (FF_SQUARE)
>     Event code 89 (FF_TRIANGLE)
>     Event code 90 (FF_SINE)
>     Event code 96 (FF_GAIN)

I don't set them, ff_memless is doing that in input_ff_create_memless:

	/* we can emulate periodic effects with RUMBLE */
	if (test_bit(FF_RUMBLE, ff->ffbit)) {
		set_bit(FF_PERIODIC, dev->ffbit);
		set_bit(FF_SINE, dev->ffbit);
		set_bit(FF_TRIANGLE, dev->ffbit);
		set_bit(FF_SQUARE, dev->ffbit);
	}

> Also please make sure that setting the rumble does not change the LEDs
> status if there is any set: HID output report 1 is used for both LEDs
> and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
> to match the joystick number, just as the PS3 does, it would be strange
> for the user if a rumble event would reset the LEDs status.

I never used the LEDs and therefore cannot say anything about it (I don't have 
a specification for the used command format). Maybe I can try to play with 
them next week.

But you're patch has some comments in set_leds. Do I correctly interpret the 
byte 10 in leds_report as "only make changes to following LEDs"? So setting it 
to 1 would make the command not change the LEDs at all?

> Maybe only send up to the rumble related bytes, or do a
> read-modify-write if that is possible with HID output reports, if this
> cannot be done we will have to design a solution to set LEDs too in the
> kernel, in order to be able to keep some status around.

The in-kernel state seems to be interesting because it is already done for the 
Sony Buzz LEDs. But I this is only a wild guess because I've never checked 
this code path and never used it.

> Last comment, if we want a conditional config what about using
> CONFIG_HID_SONY_FF instead of CONFIG_SONY_FF? Not a big deal of course,
> just a suggestion.

Most other hid devices omit the HID_ part in the _FF setting. This is also the 
reason why I've removed it too.

$ grep 'config ' ./drivers/hid/Kconfig|grep -B1 _FF
config HID_ACRUX
config HID_ACRUX_FF
--
config HID_DRAGONRISE
config DRAGONRISE_FF
config HID_EMS_FF
--
config HID_HOLTEK
config HOLTEK_FF
--
config HID_LOGITECH_DJ
config LOGITECH_FF
config LOGIRUMBLEPAD2_FF
config LOGIG940_FF
config LOGIWHEELS_FF
--
config HID_PANTHERLORD
config PANTHERLORD_FF
--
config HID_GREENASIA
config GREENASIA_FF
--
config HID_SMARTJOYPLUS
config SMARTJOYPLUS_FF
--
config HID_THRUSTMASTER
config THRUSTMASTER_FF
--
config HID_ZEROPLUS
config ZEROPLUS_FF

Kind regards,
	Sven

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

[Sven Eckelmann]

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

On Monday 18 November 2013 00:12:14 Sven Eckelmann wrote:
> > Also please make sure that setting the rumble does not change the LEDs
> > status if there is any set: HID output report 1 is used for both LEDs
> > and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
> > to match the joystick number, just as the PS3 does, it would be strange
> > for the user if a rumble event would reset the LEDs status.
> 
> I never used the LEDs and therefore cannot say anything about it (I don't
> have a specification for the used command format). Maybe I can try to play
> with them next week.
> 
> But you're patch has some comments in set_leds. Do I correctly interpret the
> byte 10 in leds_report as "only make changes to following LEDs"? So setting
> it to 1 would make the command not change the LEDs at all?

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.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Sven Eckelmann]

[-- Attachment #1.1: Type: text/plain, Size: 543 bytes --]

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

[-- Attachment #1.2: 0001-HID-sony-Make-sixaxis-usb-LEDs-configurable.patch --]
[-- Type: text/x-patch, Size: 9970 bytes --]

>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


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[simon]

> @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

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

[simon]

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

> 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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: rumble_pyusb.py --]
[-- Type: text/x-python; name="rumble_pyusb.py", Size: 2294 bytes --]

#!/usr/bin/python
#
# Small script to test the rumble on a PS3 gamepad
#

import usb
import time

rumble = 0

busses = usb.busses()
for bus in busses:      
  devices = bus.devices   
  for dev in devices:
    if dev.idVendor == 0x054c and dev.idProduct == 0x0268:
      print "Found RumblePad"
      rumble = dev

if rumble == 0:
  print "RumblePad not found"
  exit(0)

# Get a device handler for the usb device and detach it from the kernel
dh = rumble.open()
dh.detachKernelDriver(0)
dh.claimInterface(0)

#command='\x52\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1E\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\xff\x27\x10\x00\x32\x00\x00\x00\x00\x00'

# \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,

print "LED3 + LED0"
command='\x01' + \
	'\x00\xfe\x00\xfe\x00' + \
	'\x00\x00\x00\x00\x12' + \
	'\xff\x00\x00\x00\x80' + \
	'\xff\x00\x00\x00\x80' + \
	'\xff\x00\x00\x00\x00' + \
	'\xff\x00\x00\x00\x00' + \
	'\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)

print "LED2 + LED1"
command='\x01' + \
	'\x00\xfe\x00\xfe\x00' + \
	'\x00\x00\x00\x00\x0C' + \
	'\xff\x27\x10\x80\x32' + \
	'\xff\x27\x10\x80\x32' + \
	'\xff\x27\x10\x00\x32' + \
	'\xff\x27\x10\x00\x32' + \
	'\x00\x00\x00\x00\x00'

dh.bulkWrite(0x02, command, len(command))
time.sleep(3)

print "no LEDs"
command='\x01' + \
	'\x00\x00\x00\x00\x00' + \
	'\x00\x00\x00\x00\x20' + \
	'\xff\x07\x10\xff\x00' + \
	'\xff\x07\x10\xff\x00' + \
	'\xff\x07\x10\xff\x00' + \
	'\xff\x07\x10\xff\x00' + \
	'\x00\x00\x00\x00\x00'
	
dh.bulkWrite(0x02, command, len(command))

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

[Antonio Ospite]

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

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

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

[Antonio Ospite]

On Mon, 18 Nov 2013 00:12:14 +0100
Sven Eckelmann <sven@narfation.org> wrote:

> On Sunday 17 November 2013 23:25:59 Antonio Ospite wrote:
> > Sven, if you are going to resubmit another patch for this functionality
> > (I've seen your v2 just before hitting "Send"), wouldn't it be better
> > to advertise just FF_RUMBLE? AFAICS your first patch results in this
> > (from evtest):
> > 
> > ....
> >   Event type 21 (EV_FF)
> >     Event code 80 (FF_RUMBLE)
> >     Event code 81 (FF_PERIODIC)
> >     Event code 88 (FF_SQUARE)
> >     Event code 89 (FF_TRIANGLE)
> >     Event code 90 (FF_SINE)
> >     Event code 96 (FF_GAIN)
> 
> I don't set them, ff_memless is doing that in input_ff_create_memless:
> 
> 	/* we can emulate periodic effects with RUMBLE */
> 	if (test_bit(FF_RUMBLE, ff->ffbit)) {
> 		set_bit(FF_PERIODIC, dev->ffbit);
> 		set_bit(FF_SINE, dev->ffbit);
> 		set_bit(FF_TRIANGLE, dev->ffbit);
> 		set_bit(FF_SQUARE, dev->ffbit);
> 	}
>

I see now, thanks.

> > Also please make sure that setting the rumble does not change the LEDs
> > status if there is any set: HID output report 1 is used for both LEDs
> > and rumble. In the bluez plugin[1] I plan on setting LEDs from userspace
> > to match the joystick number, just as the PS3 does, it would be strange
> > for the user if a rumble event would reset the LEDs status.
> 
> I never used the LEDs and therefore cannot say anything about it (I don't have 
> a specification for the used command format). Maybe I can try to play with 
> them next week.
> 
> But you're patch has some comments in set_leds. Do I correctly interpret the 
> byte 10 in leds_report as "only make changes to following LEDs"? So setting it 
> to 1 would make the command not change the LEDs at all?
>

That would be an interesting approach, I'll do some experiment about
that too.

> > Maybe only send up to the rumble related bytes, or do a
> > read-modify-write if that is possible with HID output reports, if this
> > cannot be done we will have to design a solution to set LEDs too in the
> > kernel, in order to be able to keep some status around.
> 
> The in-kernel state seems to be interesting because it is already done for the 
> Sony Buzz LEDs. But I this is only a wild guess because I've never checked 
> this code path and never used it.
> 
> > Last comment, if we want a conditional config what about using
> > CONFIG_HID_SONY_FF instead of CONFIG_SONY_FF? Not a big deal of course,
> > just a suggestion.
> 
> Most other hid devices omit the HID_ part in the _FF setting. This is also the 
> reason why I've removed it too.
>

OK, I didn't know, I guess I am fine with that if others are doing so
too.

> $ grep 'config ' ./drivers/hid/Kconfig|grep -B1 _FF
[...]

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?