Remove multi-urb writes 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 <jhovold@xxxxxxxxx> --- drivers/usb/serial/aircable.c | 9 +-- drivers/usb/serial/ftdi_sio.c | 60 ++++++----------- drivers/usb/serial/generic.c | 149 ++++++----------------------------------- include/linux/usb/serial.h | 7 +-- 4 files changed, 49 insertions(+), 176 deletions(-) diff --git a/drivers/usb/serial/aircable.c b/drivers/usb/serial/aircable.c index 10ecb54..eadb84a 100644 --- a/drivers/usb/serial/aircable.c +++ b/drivers/usb/serial/aircable.c @@ -83,13 +83,12 @@ 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; - - BUG_ON(port->serial->type->multi_urb_write); /* not implemented */ + int count; + unsigned char *buf = dest; - count = kfifo_out_locked(&port->write_fifo, *dest + HCI_HEADER_LENGTH, + count = kfifo_out_locked(&port->write_fifo, buf + HCI_HEADER_LENGTH, size - HCI_HEADER_LENGTH, &port->lock); buf[0] = TX_HEADER_0; buf[1] = TX_HEADER_1; diff --git a/drivers/usb/serial/ftdi_sio.c b/drivers/usb/serial/ftdi_sio.c index f792d57..4f91e2f 100644 --- a/drivers/usb/serial/ftdi_sio.c +++ b/drivers/usb/serial/ftdi_sio.c @@ -779,7 +779,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); @@ -806,7 +806,7 @@ static struct usb_serial_driver ftdi_sio_device = { .id_table = id_table_combined, .num_ports = 1, .bulk_in_size = 512, - .multi_urb_write = 1, + .bulk_out_size = 256, .probe = ftdi_sio_probe, .port_probe = ftdi_sio_port_probe, .port_remove = ftdi_sio_port_remove, @@ -1528,15 +1528,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); @@ -1731,8 +1722,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); } @@ -1744,42 +1734,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; - - BUG_ON(!port->serial->type->multi_urb_write); /* not implemented */ + 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 afc61aa..4defe2b 100644 --- a/drivers/usb/serial/generic.c +++ b/drivers/usb/serial/generic.c @@ -24,8 +24,6 @@ static int debug; -#define MAX_TX_URBS 40 - #ifdef CONFIG_USB_SERIAL_GENERIC static int generic_probe(struct usb_interface *interface, @@ -170,92 +168,12 @@ 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; + return kfifo_out_locked(&port->write_fifo, dest, size, &port->lock); } 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; -} - /** * usb_serial_generic_write_start - kick off an URB write * @port: Pointer to the &struct usb_serial_port data @@ -284,8 +202,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); @@ -326,7 +244,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); @@ -338,23 +255,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; @@ -364,10 +276,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); @@ -377,7 +286,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; @@ -387,10 +295,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); @@ -477,35 +382,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 b88afbc..a7befd0 100644 --- a/include/linux/usb/serial.h +++ b/include/linux/usb/serial.h @@ -110,7 +110,6 @@ struct usb_serial_port { unsigned long flags; int tx_bytes; - int tx_urbs; wait_queue_head_t write_wait; struct work_struct work; @@ -235,8 +234,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; @@ -288,7 +285,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) @@ -342,7 +339,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); -- 1.7.0.2 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html