[PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Benjamin Valentin]

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

This will add rumble support for xbox 1 compatible controllers.
I am wondering why XTYPE_XBOX360W is excluded from the rumbling action,
however as I have no device to test, I have not touched this.

--- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c	2010-01-08 02:56:59.365851076 +0100
+++ xpad.c	2010-01-08 03:13:38.477835651 +0100
@@ -505,7 +505,7 @@
 	struct usb_endpoint_descriptor *ep_irq_out;
 	int error = -ENOMEM;
 
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
@@ -535,13 +535,13 @@
 
 static void xpad_stop_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360)
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)
 		usb_kill_urb(xpad->irq_out);
 }
 
 static void xpad_deinit_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360) {
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX) {
 		usb_free_urb(xpad->irq_out);
 		usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
 				xpad->odata, xpad->odata_dma);
@@ -554,24 +554,43 @@
 #endif
 
 #ifdef CONFIG_JOYSTICK_XPAD_FF
-static int xpad_play_effect(struct input_dev *dev, void *data,
-			    struct ff_effect *effect)
+static int xpad_send_rumble(struct usb_xpad *xpad, unsigned char left, unsigned char right) {
+	switch(xpad->xtype) {
+		case XTYPE_XBOX:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x06;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator
+			xpad->odata[4] = 0x00;
+			xpad->odata[5] = right; // right actuator
+			xpad->irq_out->transfer_buffer_length = 6;
+			return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		case XTYPE_XBOX360:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x08;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator?
+			xpad->odata[4] = right; // right actuator?
+			xpad->odata[5] = 0x00;
+			xpad->odata[6] = 0x00;
+			xpad->odata[7] = 0x00;
+			xpad->irq_out->transfer_buffer_length = 8;
+			return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		default:
+			dbg("%s - rumble command sent to unsupported xpad type: %d", 
+				__func__, xpad->xtype);
+			return 0;
+		}
+}
+
+static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct usb_xpad *xpad = input_get_drvdata(dev);
 
 	if (effect->type == FF_RUMBLE) {
-		__u16 strong = effect->u.rumble.strong_magnitude;
-		__u16 weak = effect->u.rumble.weak_magnitude;
-		xpad->odata[0] = 0x00;
-		xpad->odata[1] = 0x08;
-		xpad->odata[2] = 0x00;
-		xpad->odata[3] = strong / 256;
-		xpad->odata[4] = weak / 256;
-		xpad->odata[5] = 0x00;
-		xpad->odata[6] = 0x00;
-		xpad->odata[7] = 0x00;
-		xpad->irq_out->transfer_buffer_length = 8;
-		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		unsigned char strong = effect->u.rumble.strong_magnitude / 256;
+		unsigned char weak   = effect->u.rumble.weak_magnitude / 256;
+		xpad_send_rumble(xpad, strong, weak);
 	}
 
 	return 0;
@@ -579,7 +598,7 @@
 
 static int xpad_init_ff(struct usb_xpad *xpad)
 {
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Dmitry Torokhov]

Hi Benjamin,

On Fri, Jan 08, 2010 at 03:32:15AM +0100, Benjamin Valentin wrote:
> This will add rumble support for xbox 1 compatible controllers.
> I am wondering why XTYPE_XBOX360W is excluded from the rumbling action,
> however as I have no device to test, I have not touched this.

Thank you for your patch. Could I please have your "Signed-off-by: " so
I can apply it? Also, if you have any more patches ofr in put devices,
could you please CC linux-input@vger.kernel.org?

Thanks!

-- 
Dmitry

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Benjamin Valentin]

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

On Thu, 7 Jan 2010 23:50:54 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> Thank you for your patch. Could I please have your "Signed-off-by: "
> so I can apply it? Also, if you have any more patches ofr in put
> devices, could you please CC linux-input@vger.kernel.org?

This way?

Signed-off-by: Benjamin Valentin <benpicco@zedat.fu-berlin.de>

--- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c
2010-01-08 02:56:59.365851076 +0100 +++ xpad.c	2010-01-08
03:13:38.477835651 +0100 @@ -505,7 +505,7 @@
 	struct usb_endpoint_descriptor *ep_irq_out;
 	int error = -ENOMEM;
 
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
@@ -535,13 +535,13 @@
 
 static void xpad_stop_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360)
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)
 		usb_kill_urb(xpad->irq_out);
 }
 
 static void xpad_deinit_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360) {
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)
{ usb_free_urb(xpad->irq_out);
 		usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
 				xpad->odata, xpad->odata_dma);
@@ -554,24 +554,43 @@
 #endif
 
 #ifdef CONFIG_JOYSTICK_XPAD_FF
-static int xpad_play_effect(struct input_dev *dev, void *data,
-			    struct ff_effect *effect)
+static int xpad_send_rumble(struct usb_xpad *xpad, unsigned char left,
unsigned char right) {
+	switch(xpad->xtype) {
+		case XTYPE_XBOX:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x06;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator
+			xpad->odata[4] = 0x00;
+			xpad->odata[5] = right; // right actuator
+			xpad->irq_out->transfer_buffer_length = 6;
+			return usb_submit_urb(xpad->irq_out,
GFP_KERNEL);
+		case XTYPE_XBOX360:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x08;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator?
+			xpad->odata[4] = right; // right actuator?
+			xpad->odata[5] = 0x00;
+			xpad->odata[6] = 0x00;
+			xpad->odata[7] = 0x00;
+			xpad->irq_out->transfer_buffer_length = 8;
+			return usb_submit_urb(xpad->irq_out,
GFP_KERNEL);
+		default:
+			dbg("%s - rumble command sent to unsupported
xpad type: %d", 
+				__func__, xpad->xtype);
+			return 0;
+		}
+}
+
+static int xpad_play_effect(struct input_dev *dev, void *data, struct
ff_effect *effect) {
 	struct usb_xpad *xpad = input_get_drvdata(dev);
 
 	if (effect->type == FF_RUMBLE) {
-		__u16 strong = effect->u.rumble.strong_magnitude;
-		__u16 weak = effect->u.rumble.weak_magnitude;
-		xpad->odata[0] = 0x00;
-		xpad->odata[1] = 0x08;
-		xpad->odata[2] = 0x00;
-		xpad->odata[3] = strong / 256;
-		xpad->odata[4] = weak / 256;
-		xpad->odata[5] = 0x00;
-		xpad->odata[6] = 0x00;
-		xpad->odata[7] = 0x00;
-		xpad->irq_out->transfer_buffer_length = 8;
-		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		unsigned char strong =
effect->u.rumble.strong_magnitude / 256;
+		unsigned char weak   =
effect->u.rumble.weak_magnitude / 256;
+		xpad_send_rumble(xpad, strong, weak);
 	}
 
 	return 0;
@@ -579,7 +598,7 @@
 
 static int xpad_init_ff(struct usb_xpad *xpad)
 {
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Dmitry Torokhov]

On Fri, Jan 08, 2010 at 11:26:10AM +0100, Benjamin Valentin wrote:
> On Thu, 7 Jan 2010 23:50:54 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > Thank you for your patch. Could I please have your "Signed-off-by: "
> > so I can apply it? Also, if you have any more patches ofr in put
> > devices, could you please CC linux-input@vger.kernel.org?
> 
> This way?
> 
> Signed-off-by: Benjamin Valentin <benpicco@zedat.fu-berlin.de>

Yep, thanks.

> 
> --- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c
> 2010-01-08 02:56:59.365851076 +0100 +++ xpad.c	2010-01-08
> 03:13:38.477835651 +0100 @@ -505,7 +505,7 @@
>  	struct usb_endpoint_descriptor *ep_irq_out;
>  	int error = -ENOMEM;
>  
> -	if (xpad->xtype != XTYPE_XBOX360)
> +	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
>  		return 0;
>  
>  	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
> @@ -535,13 +535,13 @@
>  
>  static void xpad_stop_output(struct usb_xpad *xpad)
>  {
> -	if (xpad->xtype == XTYPE_XBOX360)
> +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)

This should cretainly be "... || xpad->xtype == XTYPE_XBOX)", I'll fix
it up locally.

>  		usb_kill_urb(xpad->irq_out);
>  }
>  
>  static void xpad_deinit_output(struct usb_xpad *xpad)
>  {
> -	if (xpad->xtype == XTYPE_XBOX360) {
> +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)

Same here.

BTW, your mailer line-wraps e-mail which is bad when sending patches.

-- 
Dmitry

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Benjamin Valentin]

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

On Sat, 9 Jan 2010 23:56:16 -0800
Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:

> > --- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c
> > 2010-01-08 02:56:59.365851076 +0100 +++ xpad.c	2010-01-08
> > 03:13:38.477835651 +0100 @@ -505,7 +505,7 @@
> >  	struct usb_endpoint_descriptor *ep_irq_out;
> >  	int error = -ENOMEM;
> >  
> > -	if (xpad->xtype != XTYPE_XBOX360)
> > +	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype !=
> > XTYPE_XBOX) return 0;
> >  
> >  	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
> > @@ -535,13 +535,13 @@
> >  
> >  static void xpad_stop_output(struct usb_xpad *xpad)
> >  {
> > -	if (xpad->xtype == XTYPE_XBOX360)
> > +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype !=
> > XTYPE_XBOX)
> 
> This should cretainly be "... || xpad->xtype == XTYPE_XBOX)", I'll fix
> it up locally.

Thank you, this made me discover another bug that eventually leads to a
kernel oops when the device is unplugged while an effect is playing
(or the effect is somehow else interrupted).
This should be fixed by taking the mutex when modifying xpad->odata as
well as checking whether it has been freed before writing to it.

Signed-off-by: Benjamin Valentin <benpicco@zedat.fu-berlin.de>

diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -535,16 +535,24 @@
 
 static void xpad_stop_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)
-		usb_kill_urb(xpad->irq_out);
+	if (xpad->xtype != XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX)
+		return;
+		
+	mutex_lock(&xpad->odata_mutex);
+	usb_kill_urb(xpad->irq_out);
+	mutex_unlock(&xpad->odata_mutex);
 }
 
 static void xpad_deinit_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype != XTYPE_XBOX) {
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX) {
+		mutex_lock(&xpad->odata_mutex);
+		
 		usb_free_urb(xpad->irq_out);
-		usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
+		usb_buffer_free(xpad->udev, XPAD_PKT_LEN, 
 				xpad->odata, xpad->odata_dma);
+				
+		mutex_unlock(&xpad->odata_mutex);		
 	}
 }
 #else
@@ -555,32 +563,46 @@
 
 #ifdef CONFIG_JOYSTICK_XPAD_FF
 static int xpad_send_rumble(struct usb_xpad *xpad, unsigned char left, unsigned char right) {
-	switch(xpad->xtype) {
-		case XTYPE_XBOX:
-			xpad->odata[0] = 0x00;
-			xpad->odata[1] = 0x06;
-			xpad->odata[2] = 0x00;
-			xpad->odata[3] = left;  // left actuator
-			xpad->odata[4] = 0x00;
-			xpad->odata[5] = right; // right actuator
-			xpad->irq_out->transfer_buffer_length = 6;
-			return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
-		case XTYPE_XBOX360:
-			xpad->odata[0] = 0x00;
-			xpad->odata[1] = 0x08;
-			xpad->odata[2] = 0x00;
-			xpad->odata[3] = left;  // left actuator?
-			xpad->odata[4] = right; // right actuator?
-			xpad->odata[5] = 0x00;
-			xpad->odata[6] = 0x00;
-			xpad->odata[7] = 0x00;
-			xpad->irq_out->transfer_buffer_length = 8;
-			return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
-		default:
-			dbg("%s - rumble command sent to unsupported xpad type: %d", 
-				__func__, xpad->xtype);
-			return 0;
+	int result = 0;
+	
+	mutex_lock(&xpad->odata_mutex);	
+	if(xpad->odata) {		
+		switch(xpad->xtype) {
+			case XTYPE_XBOX:
+				xpad->odata[0] = 0x00;
+				xpad->odata[1] = 0x06;
+				xpad->odata[2] = 0x00;
+				xpad->odata[3] = left;  // left actuator
+				xpad->odata[4] = 0x00;
+				xpad->odata[5] = right; // right actuator
+				xpad->irq_out->transfer_buffer_length = 6;
+				break;
+			case XTYPE_XBOX360:
+				xpad->odata[0] = 0x00;
+				xpad->odata[1] = 0x08;
+				xpad->odata[2] = 0x00;
+				xpad->odata[3] = left;  // left actuator?
+				xpad->odata[4] = right; // right actuator?
+				xpad->odata[5] = 0x00;
+				xpad->odata[6] = 0x00;
+				xpad->odata[7] = 0x00;
+				xpad->irq_out->transfer_buffer_length = 8;			
+				break;
+			default:
+				dbg("%s - rumble command sent to unsupported xpad type: %d", 
+					__func__, xpad->xtype);
+				result = -1;
+			}
+		} else {
+			dbg("%s - xpad->odata already freed", __func__);
+			result = -1;
 		}
+		
+		if(result == 0)
+			result = usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+			
+		mutex_unlock(&xpad->odata_mutex);		
+		return result;
 }
 
 static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Dmitry Torokhov]

On Sun, Jan 10, 2010 at 07:11:12PM +0100, Benjamin Valentin wrote:
> On Sat, 9 Jan 2010 23:56:16 -0800
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> 
> > > --- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c
> > > 2010-01-08 02:56:59.365851076 +0100 +++ xpad.c	2010-01-08
> > > 03:13:38.477835651 +0100 @@ -505,7 +505,7 @@
> > >  	struct usb_endpoint_descriptor *ep_irq_out;
> > >  	int error = -ENOMEM;
> > >  
> > > -	if (xpad->xtype != XTYPE_XBOX360)
> > > +	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype !=
> > > XTYPE_XBOX) return 0;
> > >  
> > >  	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
> > > @@ -535,13 +535,13 @@
> > >  
> > >  static void xpad_stop_output(struct usb_xpad *xpad)
> > >  {
> > > -	if (xpad->xtype == XTYPE_XBOX360)
> > > +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype !=
> > > XTYPE_XBOX)
> > 
> > This should cretainly be "... || xpad->xtype == XTYPE_XBOX)", I'll fix
> > it up locally.
> 
> Thank you, this made me discover another bug that eventually leads to a
> kernel oops when the device is unplugged while an effect is playing
> (or the effect is somehow else interrupted).
> This should be fixed by taking the mutex when modifying xpad->odata as
> well as checking whether it has been freed before writing to it.
> 

No, you may not take mutex in xpad_play_effect because it is called with
interrupts off and dev->event_lock spinlock held. We probably need a
workqueue-based solution to queue play requests and ensure that 2
different requests do not "step" on each other.

-- 
Dmitry

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Benjamin Valentin]

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

Am Sun, 10 Jan 2010 11:56:41 -0800
schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>:

> On Sun, Jan 10, 2010 at 07:11:12PM +0100, Benjamin Valentin wrote:
> > On Sat, 9 Jan 2010 23:56:16 -0800
> > Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > 
> > > > --- /usr/src/linux-source-2.6.33/drivers/input/joystick/xpad.c
> > > > 2010-01-08 02:56:59.365851076 +0100 +++ xpad.c	2010-01-08
> > > > 03:13:38.477835651 +0100 @@ -505,7 +505,7 @@
> > > >  	struct usb_endpoint_descriptor *ep_irq_out;
> > > >  	int error = -ENOMEM;
> > > >  
> > > > -	if (xpad->xtype != XTYPE_XBOX360)
> > > > +	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype !=
> > > > XTYPE_XBOX) return 0;
> > > >  
> > > >  	xpad->odata = usb_buffer_alloc(xpad->udev,
> > > > XPAD_PKT_LEN, @@ -535,13 +535,13 @@
> > > >  
> > > >  static void xpad_stop_output(struct usb_xpad *xpad)
> > > >  {
> > > > -	if (xpad->xtype == XTYPE_XBOX360)
> > > > +	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype !=
> > > > XTYPE_XBOX)
> > > 
> > > This should cretainly be "... || xpad->xtype == XTYPE_XBOX)",
> > > I'll fix it up locally.
> > 
> > Thank you, this made me discover another bug that eventually leads
> > to a kernel oops when the device is unplugged while an effect is
> > playing (or the effect is somehow else interrupted).
> > This should be fixed by taking the mutex when modifying xpad->odata
> > as well as checking whether it has been freed before writing to it.
> > 
> 
> No, you may not take mutex in xpad_play_effect because it is called
> with interrupts off and dev->event_lock spinlock held.

Does that mean that xpad_disconnect and xpad_play_effect never can get
executed concurrently? In that case, none of the introduced mutexes
would be necessary. (I was already wondering since there were none in
other joystick drivers)
I think it's possible that the strange behaviour when unplugging the
device while playing several effects might be introduced by VirtualBox
- I was unable to reproduce it on a physical machine.

[  968.247266] BUG: scheduling while atomic: swapper/0/0x10000100
[  968.247271] Modules linked in: joydev xpad led_class ff_memless binfmt_misc snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi iptable_filter snd_seq_midi_event ip_tables x_tables snd_seq snd_timer snd_seq_device ppdev parport_pc snd psmouse serio_raw i2c_piix4 soundcore lp snd_page_alloc parport floppy pcnet32 mii
[  968.247357] Modules linked in: joydev xpad led_class ff_memless binfmt_misc snd_intel8x0 snd_ac97_codec ac97_bus snd_pcm_oss snd_mixer_oss snd_pcm snd_seq_dummy snd_seq_oss snd_seq_midi snd_rawmidi iptable_filter snd_seq_midi_event ip_tables x_tables snd_seq snd_timer snd_seq_device ppdev parport_pc snd psmouse serio_raw i2c_piix4 soundcore lp snd_page_alloc parport floppy pcnet32 mii
[  968.247386] 
[  968.247409] Pid: 0, comm: swapper Not tainted 2.6.33-999-generic #201001091003 /VirtualBox
[  968.247413] EIP: 0060:[<c010a400>] EFLAGS: 00000246 CPU: 0
[  968.247448] EIP is at mwait_idle+0x50/0x90
[  968.247451] EAX: 00000000 EBX: c0790000 ECX: 00000000 EDX: 00000000
[  968.247454] ESI: 00000000 EDI: c0793000 EBP: c0791f9c ESP: c0791f98
[  968.247457]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
[  968.247461] Process swapper (pid: 0, ti=c0790000 task=c0797120 task.ti=c0790000)
[  968.247464] Stack:
[  968.247466]  c07dc384 c0791fb4 c0101bb4 9ecc1b86 5298b144 f616da4d 0008d800 c0791fbc
[  968.247480] <0> c0581ee2 c0791fe0 c07df9ca 000000c0 c07df530 00100000 504f88cd f616da4d
[  968.247487] <0> c0820900 00000000 c0791ff8 c07df08b 37feff9d 00000000 c06e5c40 00000800
[  968.247495] Call Trace:
[  968.247501]  [<c0101bb4>] ? cpu_idle+0x84/0xc0
[  968.247517]  [<c0581ee2>] ? rest_init+0x62/0x70
[  968.247532]  [<c07df9ca>] ? start_kernel+0x26a/0x300
[  968.247537]  [<c07df530>] ? unknown_bootoption+0x0/0x150
[  968.247542]  [<c07df08b>] ? i386_start_kernel+0x5b/0x90
[  968.247544] Code: c0 f6 44 02 27 02 75 26 89 e3 31 c9 81 e3 00 e0 ff ff 89 ca 8d 43 08 0f 01 c8 0f ae f0 89 f6 f6 43 08 08 75 16 89 c8 fb 0f 01 c9 <5b> 5d c3 89 e0 25 00 e0 ff ff 0f ae 78 08 eb cd fb 90 8d 74 26 
[  968.247584] Call Trace:
[  968.247587]  [<c0101bb4>] cpu_idle+0x84/0xc0
[  968.247599]  [<c0581ee2>] rest_init+0x62/0x70
[  968.247603]  [<c07df9ca>] start_kernel+0x26a/0x300
[  968.247607]  [<c07df530>] ? unknown_bootoption+0x0/0x150
[  968.247611]  [<c07df08b>] i386_start_kernel+0x5b/0x90
[  975.023581] usb 2-1: USB disconnect, address 6

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]

Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller

[Benjamin Valentin]

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

To avoid confusion, I am resubmitting a cleaned up version of the patch
against 2.6.33-r4.
I have changed GFP_KERNEL to GFP_ATOMIC as you did in your tree [1],
this seems to clear the oops in Virtualbox when unplugging the device
while sending an effect.

[1]
http://git.kernel.org/gitweb.cgi?p=linux/kernel/git/dtor/input.git;a=commit;h=dd38d6889dc5dae2014d9eac72fae32f477f294e)

Signed-off-by: Benjamin Valentin <benpicco@zedat.fu-berlin.de>
--- a/drivers/input/joystick/xpad.c	2010-01-13 21:35:35.081651802 +0100
+++ b/drivers/input/joystick/xpad.c	2010-01-13 22:02:29.313757056 +0100
@@ -446,7 +446,7 @@ static void xpad_irq_in(struct urb *urb)
 	}
 
 exit:
-	retval = usb_submit_urb (urb, GFP_ATOMIC);
+	retval = usb_submit_urb(urb, GFP_ATOMIC);
 	if (retval)
 		err ("%s - usb_submit_urb failed with result %d",
 		     __func__, retval);
@@ -505,7 +505,7 @@ static int xpad_init_output(struct usb_i
 	struct usb_endpoint_descriptor *ep_irq_out;
 	int error = -ENOMEM;
 
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	xpad->odata = usb_buffer_alloc(xpad->udev, XPAD_PKT_LEN,
@@ -535,13 +535,13 @@ static int xpad_init_output(struct usb_i
 
 static void xpad_stop_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360)
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX)
 		usb_kill_urb(xpad->irq_out);
 }
 
 static void xpad_deinit_output(struct usb_xpad *xpad)
 {
-	if (xpad->xtype == XTYPE_XBOX360) {
+	if (xpad->xtype == XTYPE_XBOX360 || xpad->xtype == XTYPE_XBOX) {
 		usb_free_urb(xpad->irq_out);
 		usb_buffer_free(xpad->udev, XPAD_PKT_LEN,
 				xpad->odata, xpad->odata_dma);
@@ -554,24 +554,45 @@ static void xpad_stop_output(struct usb_
 #endif
 
 #ifdef CONFIG_JOYSTICK_XPAD_FF
-static int xpad_play_effect(struct input_dev *dev, void *data,
-			    struct ff_effect *effect)
+static int xpad_send_rumble(struct usb_xpad *xpad, __u8 left, __u8 right) {
+	switch(xpad->xtype) {
+		case XTYPE_XBOX:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x06;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator (strong)
+			xpad->odata[4] = 0x00;
+			xpad->odata[5] = right; // right actuator (weak)
+			xpad->irq_out->transfer_buffer_length = 6;
+			break;
+		case XTYPE_XBOX360:
+			xpad->odata[0] = 0x00;
+			xpad->odata[1] = 0x08;
+			xpad->odata[2] = 0x00;
+			xpad->odata[3] = left;  // left actuator (strong)
+			xpad->odata[4] = right; // right actuator (weak)
+			xpad->odata[5] = 0x00;
+			xpad->odata[6] = 0x00;
+			xpad->odata[7] = 0x00;
+			xpad->irq_out->transfer_buffer_length = 8;
+			break;
+		default:
+			dbg("%s - rumble command sent to unsupported xpad type: %d", 
+				__func__, xpad->xtype);
+			return 0;
+		}
+		
+		return usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
+}
+
+static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
 {
 	struct usb_xpad *xpad = input_get_drvdata(dev);
 
 	if (effect->type == FF_RUMBLE) {
-		__u16 strong = effect->u.rumble.strong_magnitude;
-		__u16 weak = effect->u.rumble.weak_magnitude;
-		xpad->odata[0] = 0x00;
-		xpad->odata[1] = 0x08;
-		xpad->odata[2] = 0x00;
-		xpad->odata[3] = strong / 256;
-		xpad->odata[4] = weak / 256;
-		xpad->odata[5] = 0x00;
-		xpad->odata[6] = 0x00;
-		xpad->odata[7] = 0x00;
-		xpad->irq_out->transfer_buffer_length = 8;
-		usb_submit_urb(xpad->irq_out, GFP_KERNEL);
+		__u8 strong = effect->u.rumble.strong_magnitude / 256;
+		__u8 weak   = effect->u.rumble.weak_magnitude / 256;
+		xpad_send_rumble(xpad, strong, weak);
 	}
 
 	return 0;
@@ -579,7 +600,7 @@ static int xpad_play_effect(struct input
 
 static int xpad_init_ff(struct usb_xpad *xpad)
 {
-	if (xpad->xtype != XTYPE_XBOX360)
+	if (xpad->xtype != XTYPE_XBOX360 && xpad->xtype != XTYPE_XBOX)
 		return 0;
 
 	input_set_capability(xpad->dev, EV_FF, FF_RUMBLE);

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 197 bytes --]