Linux Input/HID development
 help / color / mirror / Atom feed
* Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17 23:53 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <5434362.OvDlhOjSJd@sven-edge>

[-- 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 --]

^ permalink raw reply

* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17 23:12 UTC (permalink / raw)
  To: Antonio Ospite; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <20131117232559.d2791ac2c278f56630aa41a7@studenti.unina.it>

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

^ permalink raw reply

* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Antonio Ospite @ 2013-11-17 22:25 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: simon, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <1567991.TNa7UhjX8Q@sven-desktop>

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?

^ permalink raw reply

* [PATCH] usbkbd.c: trivial typo ('released' -> 'pressed')
From: Adam Cozzette @ 2013-11-17 21:51 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-usb, linux-input, linux-kernel, Adam Cozzette

It looks like this typo was introduced by a mistake in a copy-and-paste in
commit ddbe32491951.

Signed-off-by: Adam Cozzette <acozzette@gmail.com>
---
 drivers/hid/usbhid/usbkbd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
index 7960869..9a332e6 100644
--- a/drivers/hid/usbhid/usbkbd.c
+++ b/drivers/hid/usbhid/usbkbd.c
@@ -146,7 +146,7 @@ static void usb_kbd_irq(struct urb *urb)
 				input_report_key(kbd->dev, usb_kbd_keycode[kbd->new[i]], 1);
 			else
 				hid_info(urb->dev,
-					 "Unknown key (scancode %#x) released.\n",
+					 "Unknown key (scancode %#x) pressed.\n",
 					 kbd->new[i]);
 		}
 	}
-- 
1.8.3.2


^ permalink raw reply related

* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: simon @ 2013-11-17 21:49 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: simon, linux-input, Jiri Kosina
In-Reply-To: <2281545.UHfeV6KrOG@sven-edge>

> On Sunday 17 November 2013 14:46:32 simon@mungewell.org wrote:
>> >  drivers/hid/hid-sony.c | 53
>> >
>> > +++++++++++++++++++++++++++++++++++++++-----------
>> >
>> >  1 file changed, 42 insertions(+), 11 deletions(-)
>>
>> Doesn't this miss patching Kconfig (previous changes were reverted).
>> Simon
>
> This was meant as a "replacement"/v2 of the revert patch as said earlier
> (directly bellow the commit message):
>
>> > This patch can replace
>> > 'Revert "HID: sony: Add force feedback support for Dualshock3 USB"'
>
> And if the maintainer doesn't like the work_queue workaround then he can
> still
> apply the revert patch or discuss/propose a different approach.

Patching the original Nov9 patch ([PATCH] HID: sony: Add force feedback
support for Dualshock3 USB) and then the 2nd revert ([PATCHv2] HID: sony:
Send FF commands in non-atomic context) works for me.

Both Sony DualShock2/SixAxis and my wired Itec controller work as expected
with 'testrumble' and 'testhaptic'.

Thanks for getting this fixed.
Simon

Signed-off-by: Simon Wood <simon@mungewell.org>


^ permalink raw reply

* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Sven Eckelmann @ 2013-11-17 19:51 UTC (permalink / raw)
  To: simon; +Cc: linux-input, Jiri Kosina
In-Reply-To: <b5aa869c12a432d275c51f9130ba697b.squirrel@mungewell.org>

On Sunday 17 November 2013 14:46:32 simon@mungewell.org wrote:
> >  drivers/hid/hid-sony.c | 53
> > 
> > +++++++++++++++++++++++++++++++++++++++-----------
> > 
> >  1 file changed, 42 insertions(+), 11 deletions(-)
> 
> Doesn't this miss patching Kconfig (previous changes were reverted).
> Simon

This was meant as a "replacement"/v2 of the revert patch as said earlier 
(directly bellow the commit message):

> > This patch can replace
> > 'Revert "HID: sony: Add force feedback support for Dualshock3 USB"'

And if the maintainer doesn't like the work_queue workaround then he can still 
apply the revert patch or discuss/propose a different approach.

Kind regards,
	Sven

^ permalink raw reply

* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: simon @ 2013-11-17 19:46 UTC (permalink / raw)
  Cc: linux-input, Jiri Kosina, simon, Sven Eckelmann
In-Reply-To: <1384717101-7027-1-git-send-email-sven@narfation.org>


>  drivers/hid/hid-sony.c | 53
> +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 42 insertions(+), 11 deletions(-)

Doesn't this miss patching Kconfig (previous changes were reverted).
Simon


^ permalink raw reply

* [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Sven Eckelmann @ 2013-11-17 19:38 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, simon, Sven Eckelmann
In-Reply-To: <1384682880-5960-1-git-send-email-sven@narfation.org>

The ff_memless has a timer running which gets run in an atomic context and
calls the play_effect callback. The callback function for sony uses the
hid_output_raw_report (overwritten by sixaxis_usb_output_raw_report) function
to handle differences in the control message format. It is not safe for an
atomic context because it may sleep later in usb_start_wait_urb.

This "scheduling while atomic" can cause the system to lock up. A workaround is
to make the force feedback state update using work_queues and use the
play_effect function only to enqueue the work item.

Reported-by: Simon Wood <simon@mungewell.org>
Reported-by: David Herrmann <dh.herrmann@gmail.com>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
This patch can replace
 'Revert "HID: sony: Add force feedback support for Dualshock3 USB"'

It doesn't contain the command changes from <2014555.nmU692BQMt@sven-desktop>.
It would be nice when Simon Wood could test it again with his Intec Wired
controller. When it doesn't work:

 * Try to change to usb_interrupt_msg instead of sc->hdev->hid_output_raw_report
 * Send a interrupt message using buf[10] = 0x02
   (only when buf[3] != 0 || buf[5] != 0) followed by a message with
   buf[10] = 0x1e (always)

 drivers/hid/hid-sony.c | 53 +++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index da551d1..098af2f8 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -225,6 +225,13 @@ static const unsigned int buzz_keymap[] = {
 struct sony_sc {
 	unsigned long quirks;
 
+#ifdef CONFIG_SONY_FF
+	struct work_struct rumble_worker;
+	struct hid_device *hdev;
+	__u8 left;
+	__u8 right;
+#endif
+
 	void *extra;
 };
 
@@ -615,9 +622,9 @@ static void buzz_remove(struct hid_device *hdev)
 }
 
 #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,
@@ -628,21 +635,28 @@ static int sony_play_effect(struct input_dev *dev, void *data,
 		0xff, 0x27, 0x10, 0x00, 0x32,
 		0x00, 0x00, 0x00, 0x00, 0x00
 	};
-	__u8 left;
-	__u8 right;
+
+	buf[3] = sc->right;
+	buf[5] = sc->left;
+
+	sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
+					HID_OUTPUT_REPORT);
+}
+
+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,16 +664,31 @@ 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);
 }
 
+static void sony_destroy_ff(struct hid_device *hdev)
+{
+	struct sony_sc *sc = hid_get_drvdata(hdev);
+
+	cancel_work_sync(&sc->rumble_worker);
+}
+
 #else
 static int sony_init_ff(struct hid_device *hdev)
 {
 	return 0;
 }
+
+static void sony_destroy_ff(struct hid_device *hdev)
+{
+}
 #endif
 
 static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
@@ -728,6 +757,8 @@ static void sony_remove(struct hid_device *hdev)
 	if (sc->quirks & BUZZ_CONTROLLER)
 		buzz_remove(hdev);
 
+	sony_destroy_ff(hdev);
+
 	hid_hw_stop(hdev);
 }
 
-- 
1.8.4.3


^ permalink raw reply related

* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-17 19:11 UTC (permalink / raw)
  To: Sven Eckelmann
  Cc: David Herrmann, Simon Wood, open list:HID CORE LAYER, Jiri Kosina,
	Colin Leitner
In-Reply-To: <2014555.nmU692BQMt@sven-desktop>


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


^ permalink raw reply

* Re: [PATCHv3 1/2] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-11-17 18:28 UTC (permalink / raw)
  To: Dmitry Torokhov, Dmitry Torokhov, linux-input, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Rob Landley, Grant Likely, devicetree, linux-doc, linux-kernel
In-Reply-To: <20131112032506.GA30028@earth.universe>

Hi!

> On Mon, Nov 11, 2013 at 11:19:41PM +0100, Pavel Machek wrote:
> > > +	if (of_get_property(np, "linux,input-no-autorepeat", NULL))
> > > +		keypad_data->no_autorepeat = true;
> > 
> > From 2/2:
> > 
> > +Optional Properties specific to linux:
> > +- linux,keypad-no-autorepeat: do not enable autorepeat feature.
> > 
> > I'm confused now.
> 
> good catch! That happens when one tries to mimic other drivers :/
> 
> I just checked all DT input drivers for autorepeat keyword:
> 
> DRIVER              CODE                        DOCUMENTATION
> twl4030-keypad      linux,input-no-autorepeat   linux,keypad-no-autorepeat
> omap4-keypad        linux,input-no-autorepeat   linux,keypad-no-autorepeat
> samsung-keypad      linux,input-no-autorepeat   linux,keypad-no-autorepeat
> stmpe-keypad        st,no-autorepeat            st,no-autorepeat
> spear-keyboard      autorepeat                  autorepeat
> tca8418-keypad      keypad,autorepeat           --- not documented ---
> gpio-matrix-keypad  linux,no-autorepeat         linux,no-autorepeat
> gpio-keys-polled    autorepeat                  autorepeat
> gpio-keys           autorepeat                  --- no documentation ---
> 
> Any suggestions how to continue fixing this mess? I guess first of
> all the documentation of omap4-keypad, samsung-keypad and of course
> the new twl4030-keypad driver should be fixed.
> 
> Next it would be nice to choose one standard property name for this
> and use it for twl4030-keypad. I suggest to use "linux,input-no-autorepeat".

I'd suggest just simple "autorepeat", so that we get rid of ugly double-negation.

But I guess this is not too important as long as it is consistent... And I'm not
even sure consistency is huge issue.
								Pavel	

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17 18:08 UTC (permalink / raw)
  To: David Herrmann
  Cc: Simon Wood, open list:HID CORE LAYER, Jiri Kosina, Colin Leitner
In-Reply-To: <CANq1E4RN2vpFvLcT6jdPw59wyJN-cBfFRQhMFzak7kJpPL86Nw@mail.gmail.com>


[-- 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 --]

^ permalink raw reply related

* Re: Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17 17:41 UTC (permalink / raw)
  To: simon; +Cc: linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <35271d19bf566dcc8d07fdb07d3c5830.squirrel@mungewell.org>

[-- 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 --]

^ permalink raw reply

* Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-17 17:38 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <1567991.TNa7UhjX8Q@sven-desktop>


> 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


^ permalink raw reply

* Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: David Herrmann @ 2013-11-17 16:30 UTC (permalink / raw)
  To: Sven Eckelmann
  Cc: Simon Wood, open list:HID CORE LAYER, Jiri Kosina, Colin Leitner
In-Reply-To: <1567991.TNa7UhjX8Q@sven-desktop>

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

^ permalink raw reply

* Re: conflict of wiimote and hid-sony
From: David Herrmann @ 2013-11-17 16:26 UTC (permalink / raw)
  To: Michel Kraus, Jiri Kosina; +Cc: open list:HID CORE LAYER
In-Reply-To: <CADwLDZsUsHduucFXrb218wemsGaWM76BCqQUY_OobuSf0UEP1Q@mail.gmail.com>

Hi

On Sun, Nov 17, 2013 at 2:13 AM, Michel Kraus <mksolpa@gmail.com> wrote:
> Hi,
>
> commit 86b84167 HID: wiimote: add LEGO-wiimote VID
>
> introduced a VID/PID conflict with its original owner: hid-wiimote got
> hid:b0005g*v0000054Cp00000306 added but hid-sony already has this id for
> the PS3 Remote.
>
> I think the original and legitimate owner should remain the default for this
> VID/PID combination.

Thanks for the report! It's indeed officially allocated as USB ID to Sony.

@Jiri, can you revert the following commit (note: it was already
backported to stable):

commit 86b84167d4e67372376a57ea9955c5d53dae232f
Author: David Herrmann <dh.herrmann@gmail.com>
Date:   Fri Oct 18 16:26:22 2013 +0200

    HID: wiimote: add LEGO-wiimote VID

I will write a bluez-patch to force the official wiimote-id on the
device, based on a name-comparison. This should allow us to support
both devices.

Thanks
David

^ permalink raw reply

* [PATCH] Revert "HID: sony: Add force feedback support for Dualshock3 USB"
From: Sven Eckelmann @ 2013-11-17 10:08 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, simon, Sven Eckelmann

The ff_memless has a timer running which gets run in an atomic context and
calls the play_effect callback. The callback function for sony uses the
hid_output_raw_report (overwritten by sixaxis_usb_output_raw_report) function
to handle differences in the control message format. It is not safe for an
atomic context because it may sleep later in usb_start_wait_urb.

[  619.529530] BUG: scheduling while atomic: swapper/5/0/0x00000100
[...]
[  619.529707] Call Trace:
[  619.529710]  <IRQ>  [<ffffffff81474a72>] ? dump_stack+0x41/0x51
[  619.529723]  [<ffffffff814723d8>] ? __schedule_bug+0x44/0x51
[  619.529731]  [<ffffffff814782fa>] ? __schedule+0x53a/0x720
[  619.529739]  [<ffffffff81476669>] ? schedule_timeout+0x1c9/0x2d0
[  619.529756]  [<ffffffffa00c0c6a>] ? ohci_urb_enqueue+0x1ea/0xa40 [ohci_hcd]
[  619.529763]  [<ffffffff810494ba>] ? gart_map_page+0x5a/0x80
[  619.529786]  [<ffffffffa000f7b5>] ? usb_hcd_map_urb_for_dma+0x425/0x4b0 [usbcore]
[  619.529794]  [<ffffffff81478c91>] ? wait_for_completion_timeout+0xa1/0x120
[  619.529801]  [<ffffffff81086090>] ? wake_up_state+0x10/0x10
[  619.529825]  [<ffffffffa0011d0d>] ? usb_start_wait_urb+0x6d/0xd0 [usbcore]
[  619.529848]  [<ffffffffa0011e2d>] ? usb_control_msg+0xbd/0x100 [usbcore]
[  619.529856]  [<ffffffffa1026620>] ? sony_play_effect+0x170/0x180 [hid_sony]
[  619.529864]  [<ffffffffa101c593>] ? ml_play_effects+0xf3/0x610 [ff_memless]
[  619.529872]  [<ffffffffa101cb90>] ? ml_ff_playback+0xa0/0xa0 [ff_memless]
[  619.529880]  [<ffffffffa101cbb9>] ? ml_effect_timer+0x29/0x470 [ff_memless]
[  619.529888]  [<ffffffff81063a51>] ? call_timer_fn+0x31/0x100
[  619.529895]  [<ffffffffa101cb90>] ? ml_ff_playback+0xa0/0xa0 [ff_memless]
[  619.529903]  [<ffffffff81064549>] ? run_timer_softirq+0x1c9/0x2b0
[  619.529910]  [<ffffffff8105d4e6>] ? __do_softirq+0xf6/0x240
[  619.529917]  [<ffffffff8148369c>] ? call_softirq+0x1c/0x30
[  619.529924]  [<ffffffff810143a5>] ? do_softirq+0x55/0x90
[  619.529930]  [<ffffffff8105d785>] ? irq_exit+0x95/0xa0
[  619.529937]  [<ffffffff81013ed9>] ? do_IRQ+0x49/0xb0
[  619.529944]  [<ffffffff8147a4ad>] ? common_interrupt+0x6d/0x6d
[  619.529947]  <EOI>  [<ffffffff810a2779>] ? ktime_get+0x39/0xc0
[  619.529959]  [<ffffffff810472a2>] ? native_safe_halt+0x2/0x10
[  619.529966]  [<ffffffff8101a349>] ? default_idle+0x19/0xa0
[  619.529973]  [<ffffffff8101a43b>] ? amd_e400_idle+0x6b/0xf0
[  619.529980]  [<ffffffff810a0ae8>] ? cpu_startup_entry+0xc8/0x270
[  619.529987]  [<ffffffff8103b962>] ? start_secondary+0x1d2/0x230

This reverts commit a08c22c0df0ad23d0df10ae1a9df26643589b3cc.

Reported-by: simon@mungewell.org
Signed-of-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/Kconfig    |  8 --------
 drivers/hid/hid-sony.c | 52 --------------------------------------------------
 2 files changed, 60 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 329fbb9..a27e531 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -618,14 +618,6 @@ 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 da551d1..bc37a18 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -614,54 +614,6 @@ 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;
@@ -711,10 +663,6 @@ 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.3


^ permalink raw reply related

* Re: Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: Sven Eckelmann @ 2013-11-17  9:36 UTC (permalink / raw)
  To: simon; +Cc: linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <f26febe04a776d22c913ed3ade3a8d90.squirrel@mungewell.org>

[-- 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 --]

^ permalink raw reply

* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-17  1:48 UTC (permalink / raw)
  To: simon; +Cc: Sven Eckelmann, linux-input, Jiri Kosina, Colin Leitner
In-Reply-To: <f26febe04a776d22c913ed3ade3a8d90.squirrel@mungewell.org>

[-- 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))

^ permalink raw reply

* conflict of wiimote and hid-sony
From: Michel Kraus @ 2013-11-17  1:13 UTC (permalink / raw)
  To: linux-input

Hi,

commit 86b84167 HID: wiimote: add LEGO-wiimote VID

introduced a VID/PID conflict with its original owner: hid-wiimote got
hid:b0005g*v0000054Cp00000306 added but hid-sony already has this id for
the PS3 Remote.

I think the original and legitimate owner should remain the default for this
VID/PID combination.

Michel Kraus

^ permalink raw reply

* Re: [PATCH] HID: sony: Add force feedback support for Dualshock3 USB
From: simon @ 2013-11-16 22:30 UTC (permalink / raw)
  Cc: linux-input, Jiri Kosina, Colin Leitner, Sven Eckelmann
In-Reply-To: <1384021557-24106-1-git-send-email-sven@narfation.org>

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

^ permalink raw reply

* Re: [PATCH 04/13] Input: add motion-tracking ABS_* bits and docs
From: David Herrmann @ 2013-11-16 18:07 UTC (permalink / raw)
  To: Antonio Ospite
  Cc: open list:HID CORE LAYER, Dmitry Torokhov, Jiri Kosina,
	Peter Hutterer, Benjamin Tissoires
In-Reply-To: <20131116185337.0d2c80ff184ea582d69858ed@studenti.unina.it>

Hi

On Sat, Nov 16, 2013 at 6:53 PM, Antonio Ospite
<ospite@studenti.unina.it> wrote:
> On Fri, 15 Nov 2013 11:17:31 +0100
> David Herrmann <dh.herrmann@gmail.com> wrote:
>
>> Hi Antonio
>>
>> On Fri, Nov 15, 2013 at 11:11 AM, Antonio Ospite
>> <ospite@studenti.unina.it> wrote:
>> > On Fri,  1 Nov 2013 21:16:15 +0100
>> > David Herrmann <dh.herrmann@gmail.com> wrote:
>> >
>> >> Motion sensors are getting quite common in mobile devices. To avoid
>> >> returning accelerometer data via ABS_X/Y/Z and irritating the Xorg
>> >> mouse-driver, this adds separate ABS_* bits for that.
>> >>
>> >> This is needed if gaming devices want to report their normal data plus
>> >> accelerometer/gyro data. Usually, ABS_X/Y are already used by analog
>> >> sticks, so need separate definitions, anyway.
>> >>
>> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
>> >
>> > Hi, what's the status of this patch?
>> > I cannot see it in Dmitry's repository, David maybe you have another
>> > repository I can pull from?
>>
>> Please see patch 1/13 and 2/13. They fix the kernel ABI to support new
>> ABS_* bits (the current ABI is broken in that regard). As this is
>> quite critical and caused breaks during the last merge-window, we
>> haven't merged it yet. I hope to get it into 3.14, though. The current
>> patches seem fine and work for me. I will try to get the user-space
>> part working in the next weeks.
>>
>> Any help is welcome!
>>
>
> I can try updating evtest, any idea about how I should check at
> runtime whether ABS_MAX2 is supported or not?

That's still my current issue that I need to fix. My intention is to
increase EVDEV_VERSION, so EVIOCGVERSION will suffice to test for it.

Thanks
David

^ permalink raw reply

* Re: [PATCH v3] add sur40 driver for Samsung SUR40 (aka MS Surface 2.0/Pixelsense)
From: Henrik Rydberg @ 2013-11-16 18:08 UTC (permalink / raw)
  To: Florian Echtler
  Cc: Dmitry Torokhov, linux-input, benjamin.tissoires, dh.herrmann
In-Reply-To: <52875AEF.2010109@butterbrot.org>

Hi Florian,

> Looks like the sur40 driver didn't make it into the 3.13 pull request,
> anything still missing/broken? I was hoping this might still make it into
> Ubuntu 14.04...

Nothing is broken, don't worry. If you want to make sure to hit a
certain merge window, then everything should be set and applied
several weeks before that, to allow for automatic testing and
administration. It could be that Dmitry is stacking up for a second
pull request in a couple of weeks, but I would not count on it. If you
want Ubuntu to pick your patches up, you can always try to send them
to kernel-team@lists.ubuntu.com; they might be willing to carry your
patches to the next kernel release. It helps to refer to the
appropriate commits in linux-next.

Thanks,
Henrik

^ permalink raw reply

* Re: [PATCH 04/13] Input: add motion-tracking ABS_* bits and docs
From: Antonio Ospite @ 2013-11-16 17:53 UTC (permalink / raw)
  To: David Herrmann
  Cc: open list:HID CORE LAYER, Dmitry Torokhov, Jiri Kosina,
	Peter Hutterer, Benjamin Tissoires
In-Reply-To: <CANq1E4Szv+0-GSkekH_Q5HXDv+hq7LS6tYbZFTtmxsUp1-ALkA@mail.gmail.com>

On Fri, 15 Nov 2013 11:17:31 +0100
David Herrmann <dh.herrmann@gmail.com> wrote:

> Hi Antonio
> 
> On Fri, Nov 15, 2013 at 11:11 AM, Antonio Ospite
> <ospite@studenti.unina.it> wrote:
> > On Fri,  1 Nov 2013 21:16:15 +0100
> > David Herrmann <dh.herrmann@gmail.com> wrote:
> >
> >> Motion sensors are getting quite common in mobile devices. To avoid
> >> returning accelerometer data via ABS_X/Y/Z and irritating the Xorg
> >> mouse-driver, this adds separate ABS_* bits for that.
> >>
> >> This is needed if gaming devices want to report their normal data plus
> >> accelerometer/gyro data. Usually, ABS_X/Y are already used by analog
> >> sticks, so need separate definitions, anyway.
> >>
> >> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
> >
> > Hi, what's the status of this patch?
> > I cannot see it in Dmitry's repository, David maybe you have another
> > repository I can pull from?
> 
> Please see patch 1/13 and 2/13. They fix the kernel ABI to support new
> ABS_* bits (the current ABI is broken in that regard). As this is
> quite critical and caused breaks during the last merge-window, we
> haven't merged it yet. I hope to get it into 3.14, though. The current
> patches seem fine and work for me. I will try to get the user-space
> part working in the next weeks.
> 
> Any help is welcome!
> 

I can try updating evtest, any idea about how I should check at
runtime whether ABS_MAX2 is supported or not?

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?

^ permalink raw reply

* Re: About the PS3 Sixaxis linux driver
From: Antonio Ospite @ 2013-11-16 17:06 UTC (permalink / raw)
  To: simon; +Cc: Benjamin Tissoires, case, linux-input
In-Reply-To: <6ead5599102745d03c2c28b640b11fb4.squirrel@mungewell.org>

On Mon, 4 Nov 2013 16:40:29 -0500
simon@mungewell.org wrote:

Hi Simon, sorry for the delay on this one.
 
> > Actually I intended to rewrite the whole HID descriptor and add a sane
> > mapping for all the buttons (maybe following the Gamepad API and
> > even using the new ABS_ACCEL_* and ABS_GYRO_* events from David
> > Hermann).
> 
> I think that the root of the problem here is simply the large number of
> axis, and that the keycodes start at 'axis-x' then overflow into/past the
> MultiTouch keycode.
>

Yes, and the code that maps HID usages to event keycodes also comes into
play I guess.

> This problem is going to be a more common one as HID drivers present more
> buttons/axis.
> 
> We did some patching here:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/hid/hid-sony.c?id=refs/tags/v3.12#n301
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/hid/hid-sony.c?id=61ab44bebdefab296487e7cd723a634849278827
> 
> Do we need to fix this is the HID descriptor, or in 'hid-sony' input stuff?

I think both:

  1. adjust the HID descriptor into something that can be more easily
     remapped;

  2. remap the keycodes in hid-sony to those of the gamepad API, because
     AFAICT that can't be achieved by solely deciding HID usage codes in
     the descriptor.

> Do you have example of another driver to copy?

I am doing some work already in this direction, on the lines of what we
did for the ps3remote in hid-sony, but it needs some input bits not
merged yet, I'll send an RFC as soon as this becomes more easily
testable.

Regards,
   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?

^ permalink raw reply

* Re: [PATCH v3] add sur40 driver for Samsung SUR40 (aka MS Surface 2.0/Pixelsense)
From: Florian Echtler @ 2013-11-16 11:45 UTC (permalink / raw)
  To: Henrik Rydberg, Dmitry Torokhov
  Cc: linux-input, benjamin.tissoires, dh.herrmann
In-Reply-To: <52822E8B.6030906@butterbrot.org>

Looks like the sur40 driver didn't make it into the 3.13 pull request, 
anything still missing/broken? I was hoping this might still make it 
into Ubuntu 14.04...

BR, Florian

Florian Echtler <floe@butterbrot.org> wrote:
>On 11.11.2013 11:27, Henrik Rydberg wrote:
>>  >> Also, does the patch below mess up or device or it still works?
>>> I'll test this tomorrow (no access to SUR40 today). If everything
>works,
>>> should I resubmit my patch with yours included or just tell you that
>>> it's fine?
>> No need to resubmit, a confirmation will do nicely. Thanks!
>> Henrik
>
>I've tested Dmitry's additional patch, confirmed to work. Just for the
>record:
>
>Tested-by: Florian Echtler <floe@butterbrot.org>
>
>BR, Florian

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox