From 507ca9bc0476662f3463888d583864834eab1e11 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Sat, 23 Apr 2005 12:49:16 -0700 Subject: [PATCH] [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: Greg Kroah-Hartman --- drivers/usb/serial/cyberjack.c | 19 ++++++++++--------- drivers/usb/serial/generic.c | 24 ++++++++++++++++-------- drivers/usb/serial/ipaq.c | 5 ----- drivers/usb/serial/ipw.c | 14 ++++++++++---- drivers/usb/serial/ir-usb.c | 16 +++++++++++----- drivers/usb/serial/keyspan_pda.c | 19 +++++++++++++------ drivers/usb/serial/omninet.c | 17 ++++++++++++----- drivers/usb/serial/safe_serial.c | 13 +++++++++---- drivers/usb/serial/usb-serial.c | 1 + drivers/usb/serial/usb-serial.h | 3 +++ 10 files changed, 85 insertions(+), 46 deletions(-) diff --git a/drivers/usb/serial/cyberjack.c b/drivers/usb/serial/cyberjack.c index 46a204cd40e1..b5b431067b08 100644 --- a/drivers/usb/serial/cyberjack.c +++ b/drivers/usb/serial/cyberjack.c @@ -213,10 +213,14 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b 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__); - return (0); + return 0; } + port->write_urb_busy = 1; + spin_unlock(&port->lock); spin_lock_irqsave(&priv->lock, flags); @@ -224,6 +228,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b /* To much data for buffer. Reset buffer. */ priv->wrfilled=0; spin_unlock_irqrestore(&priv->lock, flags); + port->write_urb_busy = 0; return (0); } @@ -268,6 +273,7 @@ static int cyberjack_write (struct usb_serial_port *port, const unsigned char *b priv->wrfilled=0; priv->wrsent=0; spin_unlock_irqrestore(&priv->lock, flags); + port->write_urb_busy = 0; return 0; } @@ -412,7 +418,8 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs struct cyberjack_private *priv = usb_get_serial_port_data(port); dbg("%s - port %d", __FUNCTION__, port->number); - + + port->write_urb_busy = 0; if (urb->status) { dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); return; @@ -424,12 +431,6 @@ static void cyberjack_write_bulk_callback (struct urb *urb, struct pt_regs *regs if( priv->wrfilled ) { 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__); length = ((priv->wrfilled - priv->wrsent) > port->bulk_out_size) ? diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index 99214aa3cd19..ddde5fb13f6b 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -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 */ 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__); - return (0); + return 0; } + port->write_urb_busy = 1; + spin_unlock(&port->lock); 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 * usb_serial_generic_write_bulk_callback), port); /* send the data out the bulk port */ + port->write_urb_busy = 1; 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); - else + /* don't have to grab the lock here, as we will retry if != 0 */ + port->write_urb_busy = 0; + } else result = count; return result; } /* no bulk out, so return 0 bytes written */ - return (0); + return 0; } int usb_serial_generic_write_room (struct usb_serial_port *port) @@ -214,9 +221,9 @@ int usb_serial_generic_write_room (struct usb_serial_port *port) int room = 0; dbg("%s - port %d", __FUNCTION__, port->number); - + if (serial->num_bulk_out) { - if (port->write_urb->status != -EINPROGRESS) + if (port->write_urb_busy) room = port->bulk_out_size; } @@ -232,7 +239,7 @@ int usb_serial_generic_chars_in_buffer (struct usb_serial_port *port) dbg("%s - port %d", __FUNCTION__, port->number); if (serial->num_bulk_out) { - if (port->write_urb->status == -EINPROGRESS) + if (port->write_urb_busy) 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 dbg("%s - port %d", __FUNCTION__, port->number); + port->write_urb_busy = 0; if (urb->status) { dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); return; diff --git a/drivers/usb/serial/ipaq.c b/drivers/usb/serial/ipaq.c index 3bd69c4ef24b..c05c2a2a0f31 100644 --- a/drivers/usb/serial/ipaq.c +++ b/drivers/usb/serial/ipaq.c @@ -818,11 +818,6 @@ static void ipaq_write_gather(struct usb_serial_port *port) struct ipaq_packet *pkt, *tmp; 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; list_for_each_entry_safe(pkt, tmp, &priv->queue, list) { count = min(room, (int)(pkt->len - pkt->written)); diff --git a/drivers/usb/serial/ipw.c b/drivers/usb/serial/ipw.c index 11105d74f461..85e242459c27 100644 --- a/drivers/usb/serial/ipw.c +++ b/drivers/usb/serial/ipw.c @@ -399,16 +399,21 @@ static int ipw_write(struct usb_serial_port *port, const unsigned char *buf, int dbg("%s - write request of 0 bytes", __FUNCTION__); return 0; } - - /* Racy and broken, FIXME properly! */ - if (port->write_urb->status == -EINPROGRESS) + + spin_lock(&port->lock); + if (port->write_urb_busy) { + spin_unlock(&port->lock); + dbg("%s - already writing", __FUNCTION__); return 0; + } + port->write_urb_busy = 1; + spin_unlock(&port->lock); count = min(count, port->bulk_out_size); memcpy(port->bulk_out_buffer, buf, count); dbg("%s count now:%d", __FUNCTION__, count); - + usb_fill_bulk_urb(port->write_urb, dev, usb_sndbulkpipe(dev, port->bulk_out_endpointAddress), port->write_urb->transfer_buffer, @@ -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); if (ret != 0) { + port->write_urb_busy = 0; dbg("%s - usb_submit_urb(write bulk) failed with error = %d", __FUNCTION__, ret); return ret; } diff --git a/drivers/usb/serial/ir-usb.c b/drivers/usb/serial/ir-usb.c index 59f234df5f89..937b2fdd7171 100644 --- a/drivers/usb/serial/ir-usb.c +++ b/drivers/usb/serial/ir-usb.c @@ -341,10 +341,14 @@ static int ir_write (struct usb_serial_port *port, const unsigned char *buf, int if (count == 0) return 0; - if (port->write_urb->status == -EINPROGRESS) { - dbg ("%s - already writing", __FUNCTION__); + spin_lock(&port->lock); + if (port->write_urb_busy) { + spin_unlock(&port->lock); + dbg("%s - already writing", __FUNCTION__); return 0; } + port->write_urb_busy = 1; + spin_unlock(&port->lock); transfer_buffer = port->write_urb->transfer_buffer; 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 port->write_urb->transfer_flags = URB_ZERO_PACKET; 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); - else + } else result = transfer_size; return result; @@ -387,7 +392,8 @@ static void ir_write_bulk_callback (struct urb *urb, struct pt_regs *regs) struct usb_serial_port *port = (struct usb_serial_port *)urb->context; dbg("%s - port %d", __FUNCTION__, port->number); - + + port->write_urb_busy = 0; if (urb->status) { dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); return; diff --git a/drivers/usb/serial/keyspan_pda.c b/drivers/usb/serial/keyspan_pda.c index 7fd0aa9eccf6..635c384cb15a 100644 --- a/drivers/usb/serial/keyspan_pda.c +++ b/drivers/usb/serial/keyspan_pda.c @@ -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 device is full (wait until it says there is room) */ - if (port->write_urb->status == -EINPROGRESS || priv->tx_throttled ) { - return( 0 ); + spin_lock(&port->lock); + 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 again (the only sudden transition was the one from EINPROGRESS to @@ -570,7 +574,7 @@ static int keyspan_pda_write(struct usb_serial_port *port, memcpy (port->write_urb->transfer_buffer, buf, count); /* send the data out the bulk port */ port->write_urb->transfer_buffer_length = count; - + priv->tx_room -= count; port->write_urb->dev = port->serial->dev; @@ -593,6 +597,8 @@ static int keyspan_pda_write(struct usb_serial_port *port, rc = count; exit: + if (rc < 0) + port->write_urb_busy = 0; return rc; } @@ -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 keyspan_pda_private *priv; + port->write_urb_busy = 0; priv = usb_get_serial_port_data(port); /* queue up a wakeup at scheduler time */ @@ -626,12 +633,12 @@ static int keyspan_pda_write_room (struct usb_serial_port *port) static int keyspan_pda_chars_in_buffer (struct usb_serial_port *port) { struct keyspan_pda_private *priv; - + priv = usb_get_serial_port_data(port); - + /* when throttled, return at least WAKEUP_CHARS to tell select() (via 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 0; } diff --git a/drivers/usb/serial/omninet.c b/drivers/usb/serial/omninet.c index b5f2c06d4f3e..6a99ae192df1 100644 --- a/drivers/usb/serial/omninet.c +++ b/drivers/usb/serial/omninet.c @@ -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__); 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__); - return (0); + return 0; } + port->write_urb_busy = 1; + spin_unlock(&port->lock); count = (count > OMNINET_BULKOUTSIZE) ? OMNINET_BULKOUTSIZE : count; @@ -275,9 +280,10 @@ static int omninet_write (struct usb_serial_port *port, const unsigned char *buf wport->write_urb->dev = serial->dev; 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); - else + } else result = count; return result; @@ -291,7 +297,7 @@ static int omninet_write_room (struct usb_serial_port *port) int room = 0; // Default: no room - if (wport->write_urb->status != -EINPROGRESS) + if (wport->write_urb_busy) room = wport->bulk_out_size - OMNINET_HEADERLEN; // dbg("omninet_write_room returns %d", room); @@ -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); + port->write_urb_busy = 0; if (urb->status) { dbg("%s - nonzero write bulk status received: %d", __FUNCTION__, urb->status); return; diff --git a/drivers/usb/serial/safe_serial.c b/drivers/usb/serial/safe_serial.c index 0e85ed6c6c19..96a17568cbf1 100644 --- a/drivers/usb/serial/safe_serial.c +++ b/drivers/usb/serial/safe_serial.c @@ -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__); return (0); } - if (port->write_urb->status == -EINPROGRESS) { - dbg ("%s - already writing", __FUNCTION__); - return (0); + spin_lock(&port->lock); + if (port->write_urb_busy) { + 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 @@ -354,6 +358,7 @@ static int safe_write (struct usb_serial_port *port, const unsigned char *buf, i #endif port->write_urb->dev = port->serial->dev; 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); return 0; } @@ -368,7 +373,7 @@ static int safe_write_room (struct usb_serial_port *port) dbg ("%s", __FUNCTION__); - if (port->write_urb->status != -EINPROGRESS) + if (port->write_urb_busy) room = port->bulk_out_size - (safe ? 2 : 0); if (room) { diff --git a/drivers/usb/serial/usb-serial.c b/drivers/usb/serial/usb-serial.c index 5da76dd8fb28..0267b26dde18 100644 --- a/drivers/usb/serial/usb-serial.c +++ b/drivers/usb/serial/usb-serial.c @@ -1047,6 +1047,7 @@ int usb_serial_probe(struct usb_interface *interface, memset(port, 0x00, sizeof(struct usb_serial_port)); port->number = i + serial->minor; port->serial = serial; + spin_lock_init(&port->lock); INIT_WORK(&port->work, usb_serial_port_softint, port); serial->port[i] = port; } diff --git a/drivers/usb/serial/usb-serial.h b/drivers/usb/serial/usb-serial.h index d1f0c4057fa6..57f92f054c75 100644 --- a/drivers/usb/serial/usb-serial.h +++ b/drivers/usb/serial/usb-serial.h @@ -69,6 +69,7 @@ * usb_serial_port: structure for the specific ports of a device. * @serial: pointer back to the struct usb_serial owner of 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). * @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. @@ -98,6 +99,7 @@ struct usb_serial_port { struct usb_serial * serial; struct tty_struct * tty; + spinlock_t lock; unsigned char number; unsigned char * interrupt_in_buffer; @@ -117,6 +119,7 @@ struct usb_serial_port { unsigned char * bulk_out_buffer; int bulk_out_size; struct urb * write_urb; + int write_urb_busy; __u8 bulk_out_endpointAddress; wait_queue_head_t write_wait; -- 2.30.2