From c23e5fc1f7dba228558b4a46e68f7af89515b13c Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Wed, 5 May 2010 23:58:13 +0200 Subject: [PATCH] USB: serial: remove multi-urb write from generic driver Remove multi-urb write from the generic driver and simplify the prepare_write_buffer prototype: int (*prepare_write_buffer)(struct usb_serial_port *port, void *dest, size_t size); The default implementation simply fills dest with data from port write fifo but drivers can override it if they need to process the outgoing data (e.g. add headers). Turn ftdi_sio into a generic fifo-based driver, which lowers CPU usage significantly for small writes while retaining maximum throughput. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/serial/aircable.c | 7 +- drivers/usb/serial/ftdi_sio.c | 59 ++++++----------- drivers/usb/serial/generic.c | 150 +++++++----------------------------------- include/linux/usb/serial.h | 8 +-- 4 files changed, 48 insertions(+), 176 deletions(-) diff --git a/drivers/usb/serial/aircable.c b/drivers/usb/serial/aircable.c index cac51629..8a990a7 100644 --- a/drivers/usb/serial/aircable.c +++ b/drivers/usb/serial/aircable.c @@ -84,9 +84,10 @@ static const struct usb_device_id id_table[] = { MODULE_DEVICE_TABLE(usb, id_table); static int aircable_prepare_write_buffer(struct usb_serial_port *port, - void **dest, size_t size, const void *src, size_t count) + void *dest, size_t size) { - unsigned char *buf = *dest; + int count; + unsigned char *buf = dest; count = kfifo_out_locked(&port->write_fifo, buf + HCI_HEADER_LENGTH, size - HCI_HEADER_LENGTH, &port->lock); @@ -185,8 +186,6 @@ static struct usb_serial_driver aircable_device = { .id_table = id_table, .num_ports = 1, .bulk_out_size = HCI_COMPLETE_FRAME, - /* Must modify prepare_write_buffer if multi_urb_write is changed. */ - .multi_urb_write = 0, .probe = aircable_probe, .process_read_urb = aircable_process_read_urb, .prepare_write_buffer = aircable_prepare_write_buffer, diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index f515f32..14f7a34 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -781,7 +781,7 @@ static void ftdi_close(struct usb_serial_port *port); static void ftdi_dtr_rts(struct usb_serial_port *port, int on); static void ftdi_process_read_urb(struct urb *urb); static int ftdi_prepare_write_buffer(struct usb_serial_port *port, - void **dest, size_t size, const void *buf, size_t count); + void *dest, size_t size); static void ftdi_set_termios(struct tty_struct *tty, struct usb_serial_port *port, struct ktermios *old); static int ftdi_tiocmget(struct tty_struct *tty, struct file *file); @@ -808,8 +808,7 @@ static struct usb_serial_driver ftdi_sio_device = { .id_table = id_table_combined, .num_ports = 1, .bulk_in_size = 512, - /* Must modify prepare_write_buffer if multi_urb_write is changed. */ - .multi_urb_write = 1, + .bulk_out_size = 256, .probe = ftdi_sio_probe, .port_probe = ftdi_sio_port_probe, .port_remove = ftdi_sio_port_remove, @@ -1531,15 +1530,6 @@ static int ftdi_sio_port_probe(struct usb_serial_port *port) quirk->port_probe(priv); priv->port = port; - - /* Free port's existing write urb and transfer buffer. */ - if (port->write_urb) { - usb_free_urb(port->write_urb); - port->write_urb = NULL; - } - kfree(port->bulk_out_buffer); - port->bulk_out_buffer = NULL; - usb_set_serial_port_data(port, priv); ftdi_determine_type(port); @@ -1734,8 +1724,7 @@ static void ftdi_close(struct usb_serial_port *port) dbg("%s", __func__); - /* shutdown our bulk read */ - usb_kill_urb(port->read_urb); + usb_serial_generic_close(port); kref_put(&priv->kref, ftdi_sio_priv_release); } @@ -1747,40 +1736,34 @@ static void ftdi_close(struct usb_serial_port *port) * The new devices do not require this byte */ static int ftdi_prepare_write_buffer(struct usb_serial_port *port, - void **dest, size_t size, const void *src, size_t count) + void *dest, size_t size) { struct ftdi_private *priv; - unsigned char *buffer; - int len; + int count; + unsigned long flags; priv = usb_get_serial_port_data(port); - len = count; - if (priv->chip_type == SIO && count != 0) - len += ((count - 1) / (priv->max_packet_size - 1)) + 1; - - buffer = kmalloc(len, GFP_ATOMIC); - if (!buffer) { - dev_err(&port->dev, "%s - could not allocate buffer\n", - __func__); - return -ENOMEM; - } - if (priv->chip_type == SIO) { - int i, msg_len; - - for (i = 0; i < len; i += priv->max_packet_size) { - msg_len = min_t(int, len - i, priv->max_packet_size) - 1; - buffer[i] = (msg_len << 2) + 1; - memcpy(&buffer[i + 1], src, msg_len); - src += msg_len; + unsigned char *buffer = dest; + int i, len, c; + + count = 0; + spin_lock_irqsave(&port->lock, flags); + for (i = 0; i < size - 1; i += priv->max_packet_size) { + len = min_t(int, size - i, priv->max_packet_size) - 1; + buffer[i] = (len << 2) + 1; + c = kfifo_out(&port->write_fifo, &buffer[i + 1], len); + if (!c) + break; + count += c + 1; } + spin_unlock_irqrestore(&port->lock, flags); } else { - memcpy(buffer, src, count); + count = kfifo_out_locked(&port->write_fifo, dest, size, + &port->lock); } - *dest = buffer; - return count; } diff --git a/drivers/usb/serial/generic.c b/drivers/usb/serial/generic.c index 3ae1784..fcd30b8 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -26,8 +26,6 @@ static int debug; -#define MAX_TX_URBS 40 - #ifdef CONFIG_USB_SERIAL_GENERIC static int generic_probe(struct usb_interface *interface, @@ -172,90 +170,9 @@ void usb_serial_generic_close(struct usb_serial_port *port) EXPORT_SYMBOL_GPL(usb_serial_generic_close); int usb_serial_generic_prepare_write_buffer(struct usb_serial_port *port, - void **dest, size_t size, const void *src, size_t count) + void *dest, size_t size) { - if (!*dest) { - size = count; - *dest = kmalloc(count, GFP_ATOMIC); - if (!*dest) { - dev_err(&port->dev, "%s - could not allocate buffer\n", - __func__); - return -ENOMEM; - } - } - if (src) { - count = size; - memcpy(*dest, src, size); - } else { - count = kfifo_out_locked(&port->write_fifo, *dest, size, - &port->lock); - } - return count; -} -EXPORT_SYMBOL_GPL(usb_serial_generic_prepare_write_buffer); - -static int usb_serial_multi_urb_write(struct tty_struct *tty, - struct usb_serial_port *port, const unsigned char *buf, int count) -{ - unsigned long flags; - struct urb *urb; - void *buffer; - int status; - - spin_lock_irqsave(&port->lock, flags); - if (port->tx_urbs == MAX_TX_URBS) { - spin_unlock_irqrestore(&port->lock, flags); - dbg("%s - write limit hit", __func__); - return 0; - } - port->tx_urbs++; - spin_unlock_irqrestore(&port->lock, flags); - - urb = usb_alloc_urb(0, GFP_ATOMIC); - if (!urb) { - dev_err(&port->dev, "%s - no free urbs available\n", __func__); - status = -ENOMEM; - goto err_urb; - } - - buffer = NULL; - count = min_t(int, count, PAGE_SIZE); - count = port->serial->type->prepare_write_buffer(port, &buffer, 0, - buf, count); - if (count < 0) { - status = count; - goto err_buf; - } - usb_serial_debug_data(debug, &port->dev, __func__, count, buffer); - usb_fill_bulk_urb(urb, port->serial->dev, - usb_sndbulkpipe(port->serial->dev, - port->bulk_out_endpointAddress), - buffer, count, - port->serial->type->write_bulk_callback, port); - - status = usb_submit_urb(urb, GFP_ATOMIC); - if (status) { - dev_err(&port->dev, "%s - error submitting urb: %d\n", - __func__, status); - goto err; - } - spin_lock_irqsave(&port->lock, flags); - port->tx_bytes += urb->transfer_buffer_length; - spin_unlock_irqrestore(&port->lock, flags); - - usb_free_urb(urb); - - return count; -err: - kfree(buffer); -err_buf: - usb_free_urb(urb); -err_urb: - spin_lock_irqsave(&port->lock, flags); - port->tx_urbs--; - spin_unlock_irqrestore(&port->lock, flags); - - return status; + return kfifo_out_locked(&port->write_fifo, dest, size, &port->lock); } /** @@ -286,8 +203,8 @@ retry: urb = port->write_urbs[i]; count = port->serial->type->prepare_write_buffer(port, - &urb->transfer_buffer, - port->bulk_out_size, NULL, 0); + urb->transfer_buffer, + port->bulk_out_size); urb->transfer_buffer_length = count; usb_serial_debug_data(debug, &port->dev, __func__, count, urb->transfer_buffer); @@ -328,7 +245,6 @@ retry: int usb_serial_generic_write(struct tty_struct *tty, struct usb_serial_port *port, const unsigned char *buf, int count) { - struct usb_serial *serial = port->serial; int result; dbg("%s - port %d", __func__, port->number); @@ -340,23 +256,18 @@ int usb_serial_generic_write(struct tty_struct *tty, if (!count) return 0; - if (serial->type->multi_urb_write) - return usb_serial_multi_urb_write(tty, port, buf, count); - count = kfifo_in_locked(&port->write_fifo, buf, count, &port->lock); result = usb_serial_generic_write_start(port); + if (result) + return result; - if (result >= 0) - result = count; - - return result; + return count; } EXPORT_SYMBOL_GPL(usb_serial_generic_write); int usb_serial_generic_write_room(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; - struct usb_serial *serial = port->serial; unsigned long flags; int room; @@ -366,10 +277,7 @@ int usb_serial_generic_write_room(struct tty_struct *tty) return 0; spin_lock_irqsave(&port->lock, flags); - if (serial->type->multi_urb_write) - room = (MAX_TX_URBS - port->tx_urbs) * PAGE_SIZE; - else - room = kfifo_avail(&port->write_fifo); + room = kfifo_avail(&port->write_fifo); spin_unlock_irqrestore(&port->lock, flags); dbg("%s - returns %d", __func__, room); @@ -379,7 +287,6 @@ int usb_serial_generic_write_room(struct tty_struct *tty) int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) { struct usb_serial_port *port = tty->driver_data; - struct usb_serial *serial = port->serial; unsigned long flags; int chars; @@ -389,10 +296,7 @@ int usb_serial_generic_chars_in_buffer(struct tty_struct *tty) return 0; spin_lock_irqsave(&port->lock, flags); - if (serial->type->multi_urb_write) - chars = port->tx_bytes; - else - chars = kfifo_len(&port->write_fifo) + port->tx_bytes; + chars = kfifo_len(&port->write_fifo) + port->tx_bytes; spin_unlock_irqrestore(&port->lock, flags); dbg("%s - returns %d", __func__, chars); @@ -479,35 +383,25 @@ void usb_serial_generic_write_bulk_callback(struct urb *urb) dbg("%s - port %d", __func__, port->number); - if (port->serial->type->multi_urb_write) { - kfree(urb->transfer_buffer); + for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) + if (port->write_urbs[i] == urb) + break; - spin_lock_irqsave(&port->lock, flags); - port->tx_bytes -= urb->transfer_buffer_length; - port->tx_urbs--; - spin_unlock_irqrestore(&port->lock, flags); - } else { - for (i = 0; i < ARRAY_SIZE(port->write_urbs); ++i) - if (port->write_urbs[i] == urb) - break; + spin_lock_irqsave(&port->lock, flags); + port->tx_bytes -= urb->transfer_buffer_length; + set_bit(i, &port->write_urbs_free); + spin_unlock_irqrestore(&port->lock, flags); + + if (status) { + dbg("%s - non-zero urb status: %d", __func__, status); spin_lock_irqsave(&port->lock, flags); - port->tx_bytes -= urb->transfer_buffer_length; - set_bit(i, &port->write_urbs_free); + kfifo_reset_out(&port->write_fifo); spin_unlock_irqrestore(&port->lock, flags); - - if (status) { - spin_lock_irqsave(&port->lock, flags); - kfifo_reset_out(&port->write_fifo); - spin_unlock_irqrestore(&port->lock, flags); - } else { - usb_serial_generic_write_start(port); - } + } else { + usb_serial_generic_write_start(port); } - if (status) - dbg("%s - non-zero urb status: %d", __func__, status); - usb_serial_port_softint(port); } EXPORT_SYMBOL_GPL(usb_serial_generic_write_bulk_callback); diff --git a/include/linux/usb/serial.h b/include/linux/usb/serial.h index 70b6d6b2..061c997 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -67,7 +67,6 @@ enum port_dev_state { * @write_urbs: pointers to the bulk out urbs for this port * @write_urbs_free: status bitmap the for bulk out urbs * @tx_bytes: number of bytes currently in host stack queues - * @tx_urbs: number of urbs currently in host stack queues * @bulk_out_endpointAddress: endpoint address for the bulk out pipe for this * port. * @flags: usb serial port flags @@ -112,7 +111,6 @@ struct usb_serial_port { __u8 bulk_out_endpointAddress; int tx_bytes; - int tx_urbs; unsigned long flags; wait_queue_head_t write_wait; @@ -238,8 +236,6 @@ struct usb_serial_driver { struct usb_driver *usb_driver; struct usb_dynids dynids; - unsigned char multi_urb_write:1; - size_t bulk_in_size; size_t bulk_out_size; @@ -291,7 +287,7 @@ struct usb_serial_driver { void (*process_read_urb)(struct urb *urb); /* Called by the generic write implementation */ int (*prepare_write_buffer)(struct usb_serial_port *port, - void **dest, size_t size, const void *src, size_t count); + void *dest, size_t size); }; #define to_usb_serial_driver(d) \ container_of(d, struct usb_serial_driver, driver) @@ -345,7 +341,7 @@ extern int usb_serial_generic_submit_read_urb(struct usb_serial_port *port, gfp_t mem_flags); extern void usb_serial_generic_process_read_urb(struct urb *urb); extern int usb_serial_generic_prepare_write_buffer(struct usb_serial_port *port, - void **dest, size_t size, const void *src, size_t count); + void *dest, size_t size); extern int usb_serial_handle_sysrq_char(struct tty_struct *tty, struct usb_serial_port *port, unsigned int ch); -- 2.7.4