Commit 7fc595f4 authored by Pavel Rojtberg's avatar Pavel Rojtberg Committed by Dmitry Torokhov

Input: xpad - correctly handle concurrent LED and FF requests

Track the status of the irq_out URB to prevent submission iof new requests
while current one is active. Failure to do so results in the "URB submitted
while active" warning/stack trace.

Store pending brightness and FF effect in the driver structure and replace
it with the latest requests until the device is ready to process next
request. Alternate serving LED vs FF requests to make sure one does not
starve another. See [1] for discussion. Inspired by patch of Sarah Bessmer
[2].

[1]: http://www.spinics.net/lists/linux-input/msg40708.html
[2]: http://www.spinics.net/lists/linux-input/msg31450.htmlSigned-off-by: default avatarPavel Rojtberg <rojtberg@gmail.com>
Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
parent 09c8b00a
...@@ -319,6 +319,19 @@ static struct usb_device_id xpad_table[] = { ...@@ -319,6 +319,19 @@ static struct usb_device_id xpad_table[] = {
MODULE_DEVICE_TABLE(usb, xpad_table); MODULE_DEVICE_TABLE(usb, xpad_table);
struct xpad_output_packet {
u8 data[XPAD_PKT_LEN];
u8 len;
bool pending;
};
#define XPAD_OUT_CMD_IDX 0
#define XPAD_OUT_FF_IDX 1
#define XPAD_OUT_LED_IDX (1 + IS_ENABLED(CONFIG_JOYSTICK_XPAD_FF))
#define XPAD_NUM_OUT_PACKETS (1 + \
IS_ENABLED(CONFIG_JOYSTICK_XPAD_FF) + \
IS_ENABLED(CONFIG_JOYSTICK_XPAD_LEDS))
struct usb_xpad { struct usb_xpad {
struct input_dev *dev; /* input device interface */ struct input_dev *dev; /* input device interface */
struct input_dev __rcu *x360w_dev; struct input_dev __rcu *x360w_dev;
...@@ -333,9 +346,13 @@ struct usb_xpad { ...@@ -333,9 +346,13 @@ struct usb_xpad {
dma_addr_t idata_dma; dma_addr_t idata_dma;
struct urb *irq_out; /* urb for interrupt out report */ struct urb *irq_out; /* urb for interrupt out report */
bool irq_out_active; /* we must not use an active URB */
unsigned char *odata; /* output data */ unsigned char *odata; /* output data */
dma_addr_t odata_dma; dma_addr_t odata_dma;
struct mutex odata_mutex; spinlock_t odata_lock;
struct xpad_output_packet out_packets[XPAD_NUM_OUT_PACKETS];
int last_out_packet;
#if defined(CONFIG_JOYSTICK_XPAD_LEDS) #if defined(CONFIG_JOYSTICK_XPAD_LEDS)
struct xpad_led *led; struct xpad_led *led;
...@@ -711,18 +728,71 @@ static void xpad_irq_in(struct urb *urb) ...@@ -711,18 +728,71 @@ static void xpad_irq_in(struct urb *urb)
__func__, retval); __func__, retval);
} }
/* Callers must hold xpad->odata_lock spinlock */
static bool xpad_prepare_next_out_packet(struct usb_xpad *xpad)
{
struct xpad_output_packet *pkt, *packet = NULL;
int i;
for (i = 0; i < XPAD_NUM_OUT_PACKETS; i++) {
if (++xpad->last_out_packet >= XPAD_NUM_OUT_PACKETS)
xpad->last_out_packet = 0;
pkt = &xpad->out_packets[xpad->last_out_packet];
if (pkt->pending) {
dev_dbg(&xpad->intf->dev,
"%s - found pending output packet %d\n",
__func__, xpad->last_out_packet);
packet = pkt;
break;
}
}
if (packet) {
memcpy(xpad->odata, packet->data, packet->len);
xpad->irq_out->transfer_buffer_length = packet->len;
return true;
}
return false;
}
/* Callers must hold xpad->odata_lock spinlock */
static int xpad_try_sending_next_out_packet(struct usb_xpad *xpad)
{
int error;
if (!xpad->irq_out_active && xpad_prepare_next_out_packet(xpad)) {
error = usb_submit_urb(xpad->irq_out, GFP_ATOMIC);
if (error) {
dev_err(&xpad->intf->dev,
"%s - usb_submit_urb failed with result %d\n",
__func__, error);
return -EIO;
}
xpad->irq_out_active = true;
}
return 0;
}
static void xpad_irq_out(struct urb *urb) static void xpad_irq_out(struct urb *urb)
{ {
struct usb_xpad *xpad = urb->context; struct usb_xpad *xpad = urb->context;
struct device *dev = &xpad->intf->dev; struct device *dev = &xpad->intf->dev;
int retval, status; int status = urb->status;
int error;
unsigned long flags;
status = urb->status; spin_lock_irqsave(&xpad->odata_lock, flags);
switch (status) { switch (status) {
case 0: case 0:
/* success */ /* success */
return; xpad->out_packets[xpad->last_out_packet].pending = false;
xpad->irq_out_active = xpad_prepare_next_out_packet(xpad);
break;
case -ECONNRESET: case -ECONNRESET:
case -ENOENT: case -ENOENT:
...@@ -730,19 +800,26 @@ static void xpad_irq_out(struct urb *urb) ...@@ -730,19 +800,26 @@ static void xpad_irq_out(struct urb *urb)
/* this urb is terminated, clean up */ /* this urb is terminated, clean up */
dev_dbg(dev, "%s - urb shutting down with status: %d\n", dev_dbg(dev, "%s - urb shutting down with status: %d\n",
__func__, status); __func__, status);
return; xpad->irq_out_active = false;
break;
default: default:
dev_dbg(dev, "%s - nonzero urb status received: %d\n", dev_dbg(dev, "%s - nonzero urb status received: %d\n",
__func__, status); __func__, status);
goto exit; break;
} }
exit: if (xpad->irq_out_active) {
retval = usb_submit_urb(urb, GFP_ATOMIC); error = usb_submit_urb(urb, GFP_ATOMIC);
if (retval) if (error) {
dev_err(dev, "%s - usb_submit_urb failed with result %d\n", dev_err(dev,
__func__, retval); "%s - usb_submit_urb failed with result %d\n",
__func__, error);
xpad->irq_out_active = false;
}
}
spin_unlock_irqrestore(&xpad->odata_lock, flags);
} }
static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
...@@ -761,7 +838,7 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad) ...@@ -761,7 +838,7 @@ static int xpad_init_output(struct usb_interface *intf, struct usb_xpad *xpad)
goto fail1; goto fail1;
} }
mutex_init(&xpad->odata_mutex); spin_lock_init(&xpad->odata_lock);
xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL); xpad->irq_out = usb_alloc_urb(0, GFP_KERNEL);
if (!xpad->irq_out) { if (!xpad->irq_out) {
...@@ -803,27 +880,57 @@ static void xpad_deinit_output(struct usb_xpad *xpad) ...@@ -803,27 +880,57 @@ static void xpad_deinit_output(struct usb_xpad *xpad)
static int xpad_inquiry_pad_presence(struct usb_xpad *xpad) static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
{ {
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
unsigned long flags;
int retval;
spin_lock_irqsave(&xpad->odata_lock, flags);
packet->data[0] = 0x08;
packet->data[1] = 0x00;
packet->data[2] = 0x0F;
packet->data[3] = 0xC0;
packet->data[4] = 0x00;
packet->data[5] = 0x00;
packet->data[6] = 0x00;
packet->data[7] = 0x00;
packet->data[8] = 0x00;
packet->data[9] = 0x00;
packet->data[10] = 0x00;
packet->data[11] = 0x00;
packet->len = 12;
packet->pending = true;
/* Reset the sequence so we send out presence first */
xpad->last_out_packet = -1;
retval = xpad_try_sending_next_out_packet(xpad);
spin_unlock_irqrestore(&xpad->odata_lock, flags);
return retval;
}
static int xpad_start_xbox_one(struct usb_xpad *xpad)
{
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_CMD_IDX];
unsigned long flags;
int retval; int retval;
mutex_lock(&xpad->odata_mutex); spin_lock_irqsave(&xpad->odata_lock, flags);
xpad->odata[0] = 0x08; /* Xbox one controller needs to be initialized. */
xpad->odata[1] = 0x00; packet->data[0] = 0x05;
xpad->odata[2] = 0x0F; packet->data[1] = 0x20;
xpad->odata[3] = 0xC0; packet->len = 2;
xpad->odata[4] = 0x00; packet->pending = true;
xpad->odata[5] = 0x00;
xpad->odata[6] = 0x00;
xpad->odata[7] = 0x00;
xpad->odata[8] = 0x00;
xpad->odata[9] = 0x00;
xpad->odata[10] = 0x00;
xpad->odata[11] = 0x00;
xpad->irq_out->transfer_buffer_length = 12;
retval = usb_submit_urb(xpad->irq_out, GFP_KERNEL); /* Reset the sequence so we send out start packet first */
xpad->last_out_packet = -1;
retval = xpad_try_sending_next_out_packet(xpad);
mutex_unlock(&xpad->odata_mutex); spin_unlock_irqrestore(&xpad->odata_lock, flags);
return retval; return retval;
} }
...@@ -832,8 +939,11 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad) ...@@ -832,8 +939,11 @@ static int xpad_inquiry_pad_presence(struct usb_xpad *xpad)
static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect) static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect *effect)
{ {
struct usb_xpad *xpad = input_get_drvdata(dev); struct usb_xpad *xpad = input_get_drvdata(dev);
struct xpad_output_packet *packet = &xpad->out_packets[XPAD_OUT_FF_IDX];
__u16 strong; __u16 strong;
__u16 weak; __u16 weak;
int retval;
unsigned long flags;
if (effect->type != FF_RUMBLE) if (effect->type != FF_RUMBLE)
return 0; return 0;
...@@ -841,69 +951,80 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect ...@@ -841,69 +951,80 @@ static int xpad_play_effect(struct input_dev *dev, void *data, struct ff_effect
strong = effect->u.rumble.strong_magnitude; strong = effect->u.rumble.strong_magnitude;
weak = effect->u.rumble.weak_magnitude; weak = effect->u.rumble.weak_magnitude;
spin_lock_irqsave(&xpad->odata_lock, flags);
switch (xpad->xtype) { switch (xpad->xtype) {
case XTYPE_XBOX: case XTYPE_XBOX:
xpad->odata[0] = 0x00; packet->data[0] = 0x00;
xpad->odata[1] = 0x06; packet->data[1] = 0x06;
xpad->odata[2] = 0x00; packet->data[2] = 0x00;
xpad->odata[3] = strong / 256; /* left actuator */ packet->data[3] = strong / 256; /* left actuator */
xpad->odata[4] = 0x00; packet->data[4] = 0x00;
xpad->odata[5] = weak / 256; /* right actuator */ packet->data[5] = weak / 256; /* right actuator */
xpad->irq_out->transfer_buffer_length = 6; packet->len = 6;
packet->pending = true;
break; break;
case XTYPE_XBOX360: case XTYPE_XBOX360:
xpad->odata[0] = 0x00; packet->data[0] = 0x00;
xpad->odata[1] = 0x08; packet->data[1] = 0x08;
xpad->odata[2] = 0x00; packet->data[2] = 0x00;
xpad->odata[3] = strong / 256; /* left actuator? */ packet->data[3] = strong / 256; /* left actuator? */
xpad->odata[4] = weak / 256; /* right actuator? */ packet->data[4] = weak / 256; /* right actuator? */
xpad->odata[5] = 0x00; packet->data[5] = 0x00;
xpad->odata[6] = 0x00; packet->data[6] = 0x00;
xpad->odata[7] = 0x00; packet->data[7] = 0x00;
xpad->irq_out->transfer_buffer_length = 8; packet->len = 8;
packet->pending = true;
break; break;
case XTYPE_XBOX360W: case XTYPE_XBOX360W:
xpad->odata[0] = 0x00; packet->data[0] = 0x00;
xpad->odata[1] = 0x01; packet->data[1] = 0x01;
xpad->odata[2] = 0x0F; packet->data[2] = 0x0F;
xpad->odata[3] = 0xC0; packet->data[3] = 0xC0;
xpad->odata[4] = 0x00; packet->data[4] = 0x00;
xpad->odata[5] = strong / 256; packet->data[5] = strong / 256;
xpad->odata[6] = weak / 256; packet->data[6] = weak / 256;
xpad->odata[7] = 0x00; packet->data[7] = 0x00;
xpad->odata[8] = 0x00; packet->data[8] = 0x00;
xpad->odata[9] = 0x00; packet->data[9] = 0x00;
xpad->odata[10] = 0x00; packet->data[10] = 0x00;
xpad->odata[11] = 0x00; packet->data[11] = 0x00;
xpad->irq_out->transfer_buffer_length = 12; packet->len = 12;
packet->pending = true;
break; break;
case XTYPE_XBOXONE: case XTYPE_XBOXONE:
xpad->odata[0] = 0x09; /* activate rumble */ packet->data[0] = 0x09; /* activate rumble */
xpad->odata[1] = 0x08; packet->data[1] = 0x08;
xpad->odata[2] = 0x00; packet->data[2] = 0x00;
xpad->odata[3] = 0x08; /* continuous effect */ packet->data[3] = 0x08; /* continuous effect */
xpad->odata[4] = 0x00; /* simple rumble mode */ packet->data[4] = 0x00; /* simple rumble mode */
xpad->odata[5] = 0x03; /* L and R actuator only */ packet->data[5] = 0x03; /* L and R actuator only */
xpad->odata[6] = 0x00; /* TODO: LT actuator */ packet->data[6] = 0x00; /* TODO: LT actuator */
xpad->odata[7] = 0x00; /* TODO: RT actuator */ packet->data[7] = 0x00; /* TODO: RT actuator */
xpad->odata[8] = strong / 256; /* left actuator */ packet->data[8] = strong / 256; /* left actuator */
xpad->odata[9] = weak / 256; /* right actuator */ packet->data[9] = weak / 256; /* right actuator */
xpad->odata[10] = 0x80; /* length of pulse */ packet->data[10] = 0x80; /* length of pulse */
xpad->odata[11] = 0x00; /* stop period of pulse */ packet->data[11] = 0x00; /* stop period of pulse */
xpad->irq_out->transfer_buffer_length = 12; packet->len = 12;
packet->pending = true;
break; break;
default: default:
dev_dbg(&xpad->dev->dev, dev_dbg(&xpad->dev->dev,
"%s - rumble command sent to unsupported xpad type: %d\n", "%s - rumble command sent to unsupported xpad type: %d\n",
__func__, xpad->xtype); __func__, xpad->xtype);
return -EINVAL; retval = -EINVAL;
goto out;
} }
return usb_submit_urb(xpad->irq_out, GFP_ATOMIC); retval = xpad_try_sending_next_out_packet(xpad);
out:
spin_unlock_irqrestore(&xpad->odata_lock, flags);
return retval;
} }
static int xpad_init_ff(struct usb_xpad *xpad) static int xpad_init_ff(struct usb_xpad *xpad)
...@@ -954,36 +1075,44 @@ struct xpad_led { ...@@ -954,36 +1075,44 @@ struct xpad_led {
*/ */
static void xpad_send_led_command(struct usb_xpad *xpad, int command) static void xpad_send_led_command(struct usb_xpad *xpad, int command)
{ {
struct xpad_output_packet *packet =
&xpad->out_packets[XPAD_OUT_LED_IDX];
unsigned long flags;
command %= 16; command %= 16;
mutex_lock(&xpad->odata_mutex); spin_lock_irqsave(&xpad->odata_lock, flags);
switch (xpad->xtype) { switch (xpad->xtype) {
case XTYPE_XBOX360: case XTYPE_XBOX360:
xpad->odata[0] = 0x01; packet->data[0] = 0x01;
xpad->odata[1] = 0x03; packet->data[1] = 0x03;
xpad->odata[2] = command; packet->data[2] = command;
xpad->irq_out->transfer_buffer_length = 3; packet->len = 3;
packet->pending = true;
break; break;
case XTYPE_XBOX360W: case XTYPE_XBOX360W:
xpad->odata[0] = 0x00; packet->data[0] = 0x00;
xpad->odata[1] = 0x00; packet->data[1] = 0x00;
xpad->odata[2] = 0x08; packet->data[2] = 0x08;
xpad->odata[3] = 0x40 + command; packet->data[3] = 0x40 + command;
xpad->odata[4] = 0x00; packet->data[4] = 0x00;
xpad->odata[5] = 0x00; packet->data[5] = 0x00;
xpad->odata[6] = 0x00; packet->data[6] = 0x00;
xpad->odata[7] = 0x00; packet->data[7] = 0x00;
xpad->odata[8] = 0x00; packet->data[8] = 0x00;
xpad->odata[9] = 0x00; packet->data[9] = 0x00;
xpad->odata[10] = 0x00; packet->data[10] = 0x00;
xpad->odata[11] = 0x00; packet->data[11] = 0x00;
xpad->irq_out->transfer_buffer_length = 12; packet->len = 12;
packet->pending = true;
break; break;
} }
usb_submit_urb(xpad->irq_out, GFP_KERNEL); xpad_try_sending_next_out_packet(xpad);
mutex_unlock(&xpad->odata_mutex);
spin_unlock_irqrestore(&xpad->odata_lock, flags);
} }
/* /*
...@@ -1074,13 +1203,8 @@ static int xpad_open(struct input_dev *dev) ...@@ -1074,13 +1203,8 @@ static int xpad_open(struct input_dev *dev)
if (usb_submit_urb(xpad->irq_in, GFP_KERNEL)) if (usb_submit_urb(xpad->irq_in, GFP_KERNEL))
return -EIO; return -EIO;
if (xpad->xtype == XTYPE_XBOXONE) { if (xpad->xtype == XTYPE_XBOXONE)
/* Xbox one controller needs to be initialized. */ return xpad_start_xbox_one(xpad);
xpad->odata[0] = 0x05;
xpad->odata[1] = 0x20;
xpad->irq_out->transfer_buffer_length = 2;
return usb_submit_urb(xpad->irq_out, GFP_KERNEL);
}
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment