Commit 507ca9bc authored by Greg Kroah-Hartman's avatar Greg Kroah-Hartman

[PATCH] USB: add ability for usb-serial drivers to determine if their write...

[PATCH] USB: add ability for usb-serial drivers to determine if their write urb is currently being used.

This removes a lot of racy and buggy code by trying to check the status of the urb.
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent f4df0e33
...@@ -213,10 +213,14 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b ...@@ -213,10 +213,14 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
return (0); return (0);
} }
if (port->write_urb->status == -EINPROGRESS) { spin_lock(&port->lock);
if (port->write_urb_busy) {
spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__); dbg("%s - already writing", __FUNCTION__);
return (0); return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
spin_lock_irqsave(&priv->lock, flags); spin_lock_irqsave(&priv->lock, flags);
...@@ -224,6 +228,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b ...@@ -224,6 +228,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
/* To much data for buffer. Reset buffer. */ /* To much data for buffer. Reset buffer. */
priv->wrfilled=0; priv->wrfilled=0;
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock_irqrestore(&priv->lock, flags);
port->write_urb_busy = 0;
return (0); return (0);
} }
...@@ -268,6 +273,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b ...@@ -268,6 +273,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b
priv->wrfilled=0; priv->wrfilled=0;
priv->wrsent=0; priv->wrsent=0;
spin_unlock_irqrestore(&priv->lock, flags); spin_unlock_irqrestore(&priv->lock, flags);
port->write_urb_busy = 0;
return 0; return 0;
} }
...@@ -413,6 +419,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -413,6 +419,7 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
port->write_urb_busy = 0;
if (urb->status) { if (urb->status) {
dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
return; return;
...@@ -424,12 +431,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs ...@@ -424,12 +431,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs
if( priv->wrfilled ) { if( priv->wrfilled ) {
int length, blksize, result; int length, blksize, result;
if (port->write_urb->status == -EINPROGRESS) {
dbg("%s - already writing", __FUNCTION__);
spin_unlock(&priv->lock);
return;
}
dbg("%s - transmitting data (frame n)", __FUNCTION__); dbg("%s - transmitting data (frame n)", __FUNCTION__);
length = ((priv->wrfilled - priv->wrsent) > port->bulk_out_size) ? length = ((priv->wrfilled - priv->wrsent) > port->bulk_out_size) ?
......
...@@ -174,10 +174,14 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char * ...@@ -174,10 +174,14 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
/* only do something if we have a bulk out endpoint */ /* only do something if we have a bulk out endpoint */
if (serial->num_bulk_out) { if (serial->num_bulk_out) {
if (port->write_urb->status == -EINPROGRESS) { spin_lock(&port->lock);
if (port->write_urb_busy) {
spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__); dbg("%s - already writing", __FUNCTION__);
return (0); return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
count = (count > port->bulk_out_size) ? port->bulk_out_size : count; count = (count > port->bulk_out_size) ? port->bulk_out_size : count;
...@@ -195,17 +199,20 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char * ...@@ -195,17 +199,20 @@ int usb_serial_generic_write(struct usb_serial_port *port, const unsigned char *
usb_serial_generic_write_bulk_callback), port); usb_serial_generic_write_bulk_callback), port);
/* send the data out the bulk port */ /* send the data out the bulk port */
port->write_urb_busy = 1;
result = usb_submit_urb(port->write_urb, GFP_ATOMIC); result = usb_submit_urb(port->write_urb, GFP_ATOMIC);
if (result) if (result) {
dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result); dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
else /* don't have to grab the lock here, as we will retry if != 0 */
port->write_urb_busy = 0;
} else
result = count; result = count;
return result; return result;
} }
/* no bulk out, so return 0 bytes written */ /* no bulk out, so return 0 bytes written */
return (0); return 0;
} }
int usb_serial_generic_write_room (struct usb_serial_port *port) int usb_serial_generic_write_room (struct usb_serial_port *port)
...@@ -216,7 +223,7 @@ int usb_serial_generic_write_room (struct usb_serial_port *port) ...@@ -216,7 +223,7 @@ int usb_serial_generic_write_room (struct usb_serial_port *port)
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
if (serial->num_bulk_out) { if (serial->num_bulk_out) {
if (port->write_urb->status != -EINPROGRESS) if (port->write_urb_busy)
room = port->bulk_out_size; room = port->bulk_out_size;
} }
...@@ -232,7 +239,7 @@ int usb_serial_generic_chars_in_buffer (struct usb_serial_port *port) ...@@ -232,7 +239,7 @@ int usb_serial_generic_chars_in_buffer (struct usb_serial_port *port)
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
if (serial->num_bulk_out) { if (serial->num_bulk_out) {
if (port->write_urb->status == -EINPROGRESS) if (port->write_urb_busy)
chars = port->write_urb->transfer_buffer_length; chars = port->write_urb->transfer_buffer_length;
} }
...@@ -291,6 +298,7 @@ void usb_serial_generic_write_bulk_callback (struct urb *urb, struct pt_regs *re ...@@ -291,6 +298,7 @@ void usb_serial_generic_write_bulk_callback (struct urb *urb, struct pt_regs *re
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
port->write_urb_busy = 0;
if (urb->status) { if (urb->status) {
dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
return; return;
......
...@@ -818,11 +818,6 @@ static void ipaq_write_gather(struct usb_serial_port *port) ...@@ -818,11 +818,6 @@ static void ipaq_write_gather(struct usb_serial_port *port)
struct ipaq_packet *pkt, *tmp; struct ipaq_packet *pkt, *tmp;
struct urb *urb = port->write_urb; struct urb *urb = port->write_urb;
if (urb->status == -EINPROGRESS) {
/* Should never happen */
err("%s - flushing while urb is active !", __FUNCTION__);
return;
}
room = URBDATA_SIZE; room = URBDATA_SIZE;
list_for_each_entry_safe(pkt, tmp, &priv->queue, list) { list_for_each_entry_safe(pkt, tmp, &priv->queue, list) {
count = min(room, (int)(pkt->len - pkt->written)); count = min(room, (int)(pkt->len - pkt->written));
......
...@@ -400,9 +400,14 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int ...@@ -400,9 +400,14 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
return 0; return 0;
} }
/* Racy and broken, FIXME properly! */ spin_lock(&port->lock);
if (port->write_urb->status == -EINPROGRESS) if (port->write_urb_busy) {
spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0; return 0;
}
port->write_urb_busy = 1;
spin_unlock(&port->lock);
count = min(count, port->bulk_out_size); count = min(count, port->bulk_out_size);
memcpy(port->bulk_out_buffer, buf, count); memcpy(port->bulk_out_buffer, buf, count);
...@@ -418,6 +423,7 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int ...@@ -418,6 +423,7 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int
ret = usb_submit_urb(port->write_urb, GFP_ATOMIC); ret = usb_submit_urb(port->write_urb, GFP_ATOMIC);
if (ret != 0) { if (ret != 0) {
port->write_urb_busy = 0;
dbg("%s - usb_submit_urb(write bulk) failed with error = %d", __FUNCTION__, ret); dbg("%s - usb_submit_urb(write bulk) failed with error = %d", __FUNCTION__, ret);
return ret; return ret;
} }
......
...@@ -341,10 +341,14 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int ...@@ -341,10 +341,14 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
if (count == 0) if (count == 0)
return 0; return 0;
if (port->write_urb->status == -EINPROGRESS) { spin_lock(&port->lock);
dbg ("%s - already writing", __FUNCTION__); if (port->write_urb_busy) {
spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0; return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
transfer_buffer = port->write_urb->transfer_buffer; transfer_buffer = port->write_urb->transfer_buffer;
transfer_size = min(count, port->bulk_out_size - 1); transfer_size = min(count, port->bulk_out_size - 1);
...@@ -374,9 +378,10 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int ...@@ -374,9 +378,10 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int
port->write_urb->transfer_flags = URB_ZERO_PACKET; port->write_urb->transfer_flags = URB_ZERO_PACKET;
result = usb_submit_urb (port->write_urb, GFP_ATOMIC); result = usb_submit_urb (port->write_urb, GFP_ATOMIC);
if (result) if (result) {
port->write_urb_busy = 0;
dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result); dev_err(&port->dev, "%s - failed submitting write urb, error %d\n", __FUNCTION__, result);
else } else
result = transfer_size; result = transfer_size;
return result; return result;
...@@ -388,6 +393,7 @@ static void ir_write_bulk_callback (struct urb *urb, struct pt_regs *regs) ...@@ -388,6 +393,7 @@ static void ir_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
dbg("%s - port %d", __FUNCTION__, port->number); dbg("%s - port %d", __FUNCTION__, port->number);
port->write_urb_busy = 0;
if (urb->status) { if (urb->status) {
dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
return; return;
......
...@@ -520,9 +520,13 @@ static int keyspan_pda_write(struct usb_serial_port *port, ...@@ -520,9 +520,13 @@ static int keyspan_pda_write(struct usb_serial_port *port,
the TX urb is in-flight (wait until it completes) the TX urb is in-flight (wait until it completes)
the device is full (wait until it says there is room) the device is full (wait until it says there is room)
*/ */
if (port->write_urb->status == -EINPROGRESS || priv->tx_throttled ) { spin_lock(&port->lock);
return( 0 ); if (port->write_urb_busy || priv->tx_throttled) {
spin_unlock(&port->lock);
return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
/* At this point the URB is in our control, nobody else can submit it /* At this point the URB is in our control, nobody else can submit it
again (the only sudden transition was the one from EINPROGRESS to again (the only sudden transition was the one from EINPROGRESS to
...@@ -593,6 +597,8 @@ static int keyspan_pda_write(struct usb_serial_port *port, ...@@ -593,6 +597,8 @@ static int keyspan_pda_write(struct usb_serial_port *port,
rc = count; rc = count;
exit: exit:
if (rc < 0)
port->write_urb_busy = 0;
return rc; return rc;
} }
...@@ -602,6 +608,7 @@ static void keyspan_pda_write_bulk_callback (struct urb *urb, struct pt_regs *re ...@@ -602,6 +608,7 @@ static void keyspan_pda_write_bulk_callback (struct urb *urb, struct pt_regs *re
struct usb_serial_port *port = (struct usb_serial_port *)urb->context; struct usb_serial_port *port = (struct usb_serial_port *)urb->context;
struct keyspan_pda_private *priv; struct keyspan_pda_private *priv;
port->write_urb_busy = 0;
priv = usb_get_serial_port_data(port); priv = usb_get_serial_port_data(port);
/* queue up a wakeup at scheduler time */ /* queue up a wakeup at scheduler time */
...@@ -631,7 +638,7 @@ static int keyspan_pda_chars_in_buffer (struct usb_serial_port *port) ...@@ -631,7 +638,7 @@ static int keyspan_pda_chars_in_buffer (struct usb_serial_port *port)
/* when throttled, return at least WAKEUP_CHARS to tell select() (via /* when throttled, return at least WAKEUP_CHARS to tell select() (via
n_tty.c:normal_poll() ) that we're not writeable. */ n_tty.c:normal_poll() ) that we're not writeable. */
if( port->write_urb->status == -EINPROGRESS || priv->tx_throttled ) if (port->write_urb_busy || priv->tx_throttled)
return 256; return 256;
return 0; return 0;
} }
......
...@@ -254,10 +254,15 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf ...@@ -254,10 +254,15 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
dbg("%s - write request of 0 bytes", __FUNCTION__); dbg("%s - write request of 0 bytes", __FUNCTION__);
return (0); return (0);
} }
if (wport->write_urb->status == -EINPROGRESS) {
spin_lock(&port->lock);
if (port->write_urb_busy) {
spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__); dbg("%s - already writing", __FUNCTION__);
return (0); return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count; count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count;
...@@ -275,9 +280,10 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf ...@@ -275,9 +280,10 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf
wport->write_urb->dev = serial->dev; wport->write_urb->dev = serial->dev;
result = usb_submit_urb(wport->write_urb, GFP_ATOMIC); result = usb_submit_urb(wport->write_urb, GFP_ATOMIC);
if (result) if (result) {
port->write_urb_busy = 0;
err("%s - failed submitting write urb, error %d", __FUNCTION__, result); err("%s - failed submitting write urb, error %d", __FUNCTION__, result);
else } else
result = count; result = count;
return result; return result;
...@@ -291,7 +297,7 @@ static int omninet_write_room (struct usb_serial_port *port) ...@@ -291,7 +297,7 @@ static int omninet_write_room (struct usb_serial_port *port)
int room = 0; // Default: no room int room = 0; // Default: no room
if (wport->write_urb->status != -EINPROGRESS) if (wport->write_urb_busy)
room = wport->bulk_out_size - OMNINET_HEADERLEN; room = wport->bulk_out_size - OMNINET_HEADERLEN;
// dbg("omninet_write_room returns %d", room); // dbg("omninet_write_room returns %d", room);
...@@ -306,6 +312,7 @@ static void omninet_write_bulk_callback (struct urb *urb, struct pt_regs *regs) ...@@ -306,6 +312,7 @@ static void omninet_write_bulk_callback (struct urb *urb, struct pt_regs *regs)
// dbg("omninet_write_bulk_callback, port %0x\n", port); // dbg("omninet_write_bulk_callback, port %0x\n", port);
port->write_urb_busy = 0;
if (urb->status) { if (urb->status) {
dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status);
return; return;
......
...@@ -299,10 +299,14 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i ...@@ -299,10 +299,14 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
dbg ("%s - write request of 0 bytes", __FUNCTION__); dbg ("%s - write request of 0 bytes", __FUNCTION__);
return (0); return (0);
} }
if (port->write_urb->status == -EINPROGRESS) { spin_lock(&port->lock);
dbg ("%s - already writing", __FUNCTION__); if (port->write_urb_busy) {
return (0); spin_unlock(&port->lock);
dbg("%s - already writing", __FUNCTION__);
return 0;
} }
port->write_urb_busy = 1;
spin_unlock(&port->lock);
packet_length = port->bulk_out_size; // get max packetsize packet_length = port->bulk_out_size; // get max packetsize
...@@ -354,6 +358,7 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i ...@@ -354,6 +358,7 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i
#endif #endif
port->write_urb->dev = port->serial->dev; port->write_urb->dev = port->serial->dev;
if ((result = usb_submit_urb (port->write_urb, GFP_KERNEL))) { if ((result = usb_submit_urb (port->write_urb, GFP_KERNEL))) {
port->write_urb_busy = 0;
err ("%s - failed submitting write urb, error %d", __FUNCTION__, result); err ("%s - failed submitting write urb, error %d", __FUNCTION__, result);
return 0; return 0;
} }
...@@ -368,7 +373,7 @@ static int safe_write_room (struct usb_serial_port *port) ...@@ -368,7 +373,7 @@ static int safe_write_room (struct usb_serial_port *port)
dbg ("%s", __FUNCTION__); dbg ("%s", __FUNCTION__);
if (port->write_urb->status != -EINPROGRESS) if (port->write_urb_busy)
room = port->bulk_out_size - (safe ? 2 : 0); room = port->bulk_out_size - (safe ? 2 : 0);
if (room) { if (room) {
......
...@@ -1047,6 +1047,7 @@ int usb_serial_probe(struct usb_interface *interface, ...@@ -1047,6 +1047,7 @@ int usb_serial_probe(struct usb_interface *interface,
memset(port, 0x00, sizeof(struct usb_serial_port)); memset(port, 0x00, sizeof(struct usb_serial_port));
port->number = i + serial->minor; port->number = i + serial->minor;
port->serial = serial; port->serial = serial;
spin_lock_init(&port->lock);
INIT_WORK(&port->work, usb_serial_port_softint, port); INIT_WORK(&port->work, usb_serial_port_softint, port);
serial->port[i] = port; serial->port[i] = port;
} }
......
...@@ -69,6 +69,7 @@ ...@@ -69,6 +69,7 @@
* usb_serial_port: structure for the specific ports of a device. * usb_serial_port: structure for the specific ports of a device.
* @serial: pointer back to the struct usb_serial owner of this port. * @serial: pointer back to the struct usb_serial owner of this port.
* @tty: pointer to the corresponding tty for this port. * @tty: pointer to the corresponding tty for this port.
* @lock: spinlock to grab when updating portions of this structure.
* @number: the number of the port (the minor number). * @number: the number of the port (the minor number).
* @interrupt_in_buffer: pointer to the interrupt in buffer for this port. * @interrupt_in_buffer: pointer to the interrupt in buffer for this port.
* @interrupt_in_urb: pointer to the interrupt in struct urb for this port. * @interrupt_in_urb: pointer to the interrupt in struct urb for this port.
...@@ -98,6 +99,7 @@ ...@@ -98,6 +99,7 @@
struct usb_serial_port { struct usb_serial_port {
struct usb_serial * serial; struct usb_serial * serial;
struct tty_struct * tty; struct tty_struct * tty;
spinlock_t lock;
unsigned char number; unsigned char number;
unsigned char * interrupt_in_buffer; unsigned char * interrupt_in_buffer;
...@@ -117,6 +119,7 @@ struct usb_serial_port { ...@@ -117,6 +119,7 @@ struct usb_serial_port {
unsigned char * bulk_out_buffer; unsigned char * bulk_out_buffer;
int bulk_out_size; int bulk_out_size;
struct urb * write_urb; struct urb * write_urb;
int write_urb_busy;
__u8 bulk_out_endpointAddress; __u8 bulk_out_endpointAddress;
wait_queue_head_t write_wait; wait_queue_head_t write_wait;
......
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