Messages in this thread Patch in this message |  | | | Date | Sun, 10 Jan 2010 19:11:12 +0100 | | From | Benjamin Valentin <> | | Subject | Re: [PATCH] drivers/input/joystick/xpad.c: Add rumble support for original xbox controller |
| |
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) [unhandled content-type:application/pgp-signature] |  |