On Wed, Aug 31, 2005 at 02:02:23PM +0200, Oliver Neukum wrote: > On Wed, 31 Aug 2005, Petr Pisar wrote: > > Oliver Neukum <oliver <at> neukum.org> writes: > > > > > Do you happen to have an URL? > > > > > I'd like to include it in 2.6.14. > > > > > > > > > > > informed. The latest version is at > > > > [http://dave.ok.cz/cdc-acm_release/linux-2.6.12-rc5_stable-cdc-acm.patch]. > > > > > > > > It should run in 2.6.12-rc5 but I'm not sure, I have not yet test this > > > > > > I'll have a good look at it. > > > > So, David Kubicek (author if the patch) said he's going to prepare patch > > against > > 2.6.13 in one week and post it here. > > Very good. I'll look at it as soon as possible. Hello all,
I am sorry for being so slow with the new patch - didn't really have time for hacking on that last month. This patch is against the latest stable kernel version, 2.6.13.2. I have addressed all of Olivers' notes and suggestions from the last time, so there shouldn't be any work with it's inclusion in 2.6.14 (usb_alloc_buffer instead kmalloc, debugging messages and a couple of checks). I am planning similar changes to the usb-serial driver, which is also limited to 256 kbps, as many modems used in modern 3G wireless networks do not conform to the CDC ACM specifications and work with this driver only. -- David Kubíèek System Specialist gedas ÈR s.r.o. Mladá Boleslav, Husova 217 Phone: (420) 326 329 359 Mobile: (420) 724 073 280 Email: [EMAIL PROTECTED] Web: http://www.awk.cz
diff -Nru linux-2.6.13.2/drivers/usb/class/cdc-acm.c linux-2.6.13.2-fastcdc/drivers/usb/class/cdc-acm.c --- linux-2.6.13.2/drivers/usb/class/cdc-acm.c 2005-09-17 03:02:12.000000000 +0200 +++ linux-2.6.13.2-fastcdc/drivers/usb/class/cdc-acm.c 2005-09-21 13:45:46.000000000 +0200 @@ -6,6 +6,7 @@ * Copyright (c) 1999 Johannes Erdfelt <[EMAIL PROTECTED]> * Copyright (c) 2000 Vojtech Pavlik <[EMAIL PROTECTED]> * Copyright (c) 2004 Oliver Neukum <[EMAIL PROTECTED]> + * Copyright (c) 2005 David Kubicek <[EMAIL PROTECTED]> * * USB Abstract Control Model driver for USB modems and ISDN adapters * @@ -29,6 +30,7 @@ * config we want, sysadmin changes bConfigurationValue in sysfs. * v0.23 - use softirq for rx processing, as needed by tty layer * v0.24 - change probe method to evaluate CDC union descriptor + * v0.25 - downstream tasks paralelized to maximize throughput */ /* @@ -63,14 +65,15 @@ #include <linux/usb_cdc.h> #include <asm/byteorder.h> #include <asm/unaligned.h> +#include <linux/list.h> #include "cdc-acm.h" /* * Version Information */ -#define DRIVER_VERSION "v0.23" -#define DRIVER_AUTHOR "Armin Fuerst, Pavel Machek, Johannes Erdfelt, Vojtech Pavlik" +#define DRIVER_VERSION "v0.25" +#define DRIVER_AUTHOR "Armin Fuerst, Pavel Machek, Johannes Erdfelt, Vojtech Pavlik, David Kubicek" #define DRIVER_DESC "USB Abstract Control Model driver for USB modems and ISDN adapters" static struct usb_driver acm_driver; @@ -284,7 +287,10 @@ /* data interface returns incoming bytes, or we got unthrottled */ static void acm_read_bulk(struct urb *urb, struct pt_regs *regs) { - struct acm *acm = urb->context; + struct acm_read_buffer *buf; + struct acm_read_urb *rcv = urb->context; + struct acm *acm = rcv->instance; + unsigned long flags; dbg("Entering acm_read_bulk with status %d\n", urb->status); if (!ACM_READY(acm)) @@ -293,49 +299,108 @@ if (urb->status) dev_dbg(&acm->data->dev, "bulk rx status %d\n", urb->status); - /* calling tty_flip_buffer_push() in_irq() isn't allowed */ - tasklet_schedule(&acm->bh); + buf = rcv->buffer; + buf->size = urb->actual_length; + + spin_lock_irqsave(&acm->read_lock, flags); + list_add_tail(&rcv->list, &acm->spare_read_urbs); + list_add_tail(&buf->list, &acm->filled_read_bufs); + spin_unlock_irqrestore(&acm->read_lock, flags); + + tasklet_schedule(&acm->urb_task); } static void acm_rx_tasklet(unsigned long _acm) { struct acm *acm = (void *)_acm; - struct urb *urb = acm->readurb; + struct acm_read_buffer *buf; struct tty_struct *tty = acm->tty; - unsigned char *data = urb->transfer_buffer; + struct acm_read_urb *rcv; + unsigned long flags; int i = 0; dbg("Entering acm_rx_tasklet"); - if (urb->actual_length > 0 && !acm->throttle) { - for (i = 0; i < urb->actual_length && !acm->throttle; i++) { - /* if we insert more than TTY_FLIPBUF_SIZE characters, - * we drop them. */ - if (tty->flip.count >= TTY_FLIPBUF_SIZE) { - tty_flip_buffer_push(tty); - } - tty_insert_flip_char(tty, data[i], 0); - } - dbg("Handed %d bytes to tty layer", i+1); - tty_flip_buffer_push(tty); - } + if (!ACM_READY(acm) || acm->throttle) + return; + +next_buffer: + spin_lock_irq(&acm->read_lock); + if (list_empty(&acm->filled_read_bufs)) { + spin_unlock_irq(&acm->read_lock); + goto urbs; + } + buf = list_entry(acm->filled_read_bufs.next, + struct acm_read_buffer, list); + list_del(&buf->list); + spin_unlock_irq(&acm->read_lock); + + dbg("acm_rx_tasklet: procesing buf 0x%p, size = %d\n", buf, buf->size); + + for (i = 0; i < buf->size && !acm->throttle; i++) { + /* if we insert more than TTY_FLIPBUF_SIZE characters, + we drop them. */ + if (tty->flip.count >= TTY_FLIPBUF_SIZE) { + tty_flip_buffer_push(tty); + } + tty_insert_flip_char(tty, buf->base[i], 0); + } + tty_flip_buffer_push(tty); spin_lock(&acm->throttle_lock); if (acm->throttle) { dbg("Throtteling noticed"); - memmove(data, data + i, urb->actual_length - i); - urb->actual_length -= i; - acm->resubmit_to_unthrottle = 1; + memmove(buf->base, buf->base + i, buf->size - i); + buf->size -= i; spin_unlock(&acm->throttle_lock); + spin_lock_irq(&acm->read_lock); + list_add(&buf->list, &acm->filled_read_bufs); + spin_unlock_irq(&acm->read_lock); return; } spin_unlock(&acm->throttle_lock); - urb->actual_length = 0; - urb->dev = acm->dev; - - i = usb_submit_urb(urb, GFP_ATOMIC); - if (i) - dev_dbg(&acm->data->dev, "bulk rx resubmit %d\n", i); + spin_lock_irqsave(&acm->read_lock, flags); + list_add(&buf->list, &acm->spare_read_bufs); + spin_unlock_irqrestore(&acm->read_lock, flags); + goto next_buffer; + +urbs: + while (!list_empty(&acm->spare_read_bufs)) { + spin_lock_irq(&acm->read_lock); + if (list_empty(&acm->spare_read_urbs)) { + spin_unlock_irq(&acm->read_lock); + return; + } + rcv = list_entry(acm->spare_read_urbs.next, + struct acm_read_urb, list); + list_del(&rcv->list); + spin_unlock_irq(&acm->read_lock); + + buf = list_entry(acm->spare_read_bufs.next, + struct acm_read_buffer, list); + list_del(&buf->list); + + rcv->buffer = buf; + + usb_fill_bulk_urb(rcv->urb, acm->dev, + acm->rx_endpoint, + buf->base, + acm->readsize, + acm_read_bulk, rcv); + rcv->urb->transfer_dma = buf->dma; + + dbg("acm_rx_tasklet: sending urb 0x%p, rcv 0x%p, buf 0x%p\n", rcv->urb, rcv, buf); + + /* This shouldn't kill the driver as unsuccessful URBs are returned to the + free-urbs-pool and resubmited ASAP */ + if (usb_submit_urb(rcv->urb, GFP_ATOMIC) < 0) { + list_add(&buf->list, &acm->spare_read_bufs); + spin_lock_irq(&acm->read_lock); + list_add(&rcv->list, &acm->spare_read_urbs); + spin_unlock_irq(&acm->read_lock); + return; + } + } } /* data interface wrote those outgoing bytes */ @@ -369,6 +434,7 @@ { struct acm *acm; int rv = -EINVAL; + int i; dbg("Entering acm_tty_open.\n"); down(&open_sem); @@ -382,7 +448,9 @@ tty->driver_data = acm; acm->tty = tty; - + /* force low_latency on so that our tty_push actually forces the data through, + otherwise it is scheduled, and with high data rates data can get lost. */ + tty->low_latency = 1; if (acm->used++) { goto done; @@ -394,18 +462,20 @@ goto bail_out; } - acm->readurb->dev = acm->dev; - if (usb_submit_urb(acm->readurb, GFP_KERNEL)) { - dbg("usb_submit_urb(read bulk) failed"); - goto bail_out_and_unlink; - } - if (0 > acm_set_control(acm, acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS)) goto full_bailout; - /* force low_latency on so that our tty_push actually forces the data through, - otherwise it is scheduled, and with high data rates data can get lost. */ - tty->low_latency = 1; + INIT_LIST_HEAD(&acm->spare_read_urbs); + INIT_LIST_HEAD(&acm->spare_read_bufs); + INIT_LIST_HEAD(&acm->filled_read_bufs); + for (i = 0; i < ACM_MAX_READ_URBS; i++) { + list_add(&(acm->read_urbs[i].list), &acm->spare_read_urbs); + } + for (i = 0; i < ACM_MAX_READ_BUFS; i++) { + list_add(&(acm->read_bufs[i].list), &acm->spare_read_bufs); + } + + tasklet_schedule(&acm->urb_task); done: err_out: @@ -413,8 +483,6 @@ return rv; full_bailout: - usb_kill_urb(acm->readurb); -bail_out_and_unlink: usb_kill_urb(acm->ctrlurb); bail_out: acm->used--; @@ -424,18 +492,22 @@ static void acm_tty_unregister(struct acm *acm) { + int i; + tty_unregister_device(acm_tty_driver, acm->minor); usb_put_intf(acm->control); acm_table[acm->minor] = NULL; usb_free_urb(acm->ctrlurb); - usb_free_urb(acm->readurb); usb_free_urb(acm->writeurb); + for (i = 0; i < ACM_MAX_READ_URBS; i++) + usb_free_urb(acm->read_urbs[i].urb); kfree(acm); } static void acm_tty_close(struct tty_struct *tty, struct file *filp) { struct acm *acm = tty->driver_data; + int i; if (!acm || !acm->used) return; @@ -446,7 +518,8 @@ acm_set_control(acm, acm->ctrlout = 0); usb_kill_urb(acm->ctrlurb); usb_kill_urb(acm->writeurb); - usb_kill_urb(acm->readurb); + for (i = 0; i < ACM_MAX_READ_URBS; i++) + usb_kill_urb(acm->read_urbs[i].urb); } else acm_tty_unregister(acm); } @@ -528,10 +601,7 @@ spin_lock_bh(&acm->throttle_lock); acm->throttle = 0; spin_unlock_bh(&acm->throttle_lock); - if (acm->resubmit_to_unthrottle) { - acm->resubmit_to_unthrottle = 0; - acm_read_bulk(acm->readurb, NULL); - } + tasklet_schedule(&acm->urb_task); } static void acm_tty_break_ctl(struct tty_struct *tty, int state) @@ -694,6 +764,7 @@ int call_interface_num = -1; int data_interface_num; unsigned long quirks; + int i; /* handle quirks deadly to normal probing*/ quirks = (unsigned long)id->driver_info; @@ -834,7 +905,7 @@ memset(acm, 0, sizeof(struct acm)); ctrlsize = le16_to_cpu(epctrl->wMaxPacketSize); - readsize = le16_to_cpu(epread->wMaxPacketSize); + readsize = le16_to_cpu(epread->wMaxPacketSize)*2; acm->writesize = le16_to_cpu(epwrite->wMaxPacketSize); acm->control = control_interface; acm->data = data_interface; @@ -843,12 +914,14 @@ acm->ctrl_caps = ac_management_function; acm->ctrlsize = ctrlsize; acm->readsize = readsize; - acm->bh.func = acm_rx_tasklet; - acm->bh.data = (unsigned long) acm; + acm->urb_task.func = acm_rx_tasklet; + acm->urb_task.data = (unsigned long) acm; INIT_WORK(&acm->work, acm_softint, acm); spin_lock_init(&acm->throttle_lock); spin_lock_init(&acm->write_lock); + spin_lock_init(&acm->read_lock); acm->write_ready = 1; + acm->rx_endpoint = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress); buf = usb_buffer_alloc(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); if (!buf) { @@ -857,13 +930,6 @@ } acm->ctrl_buffer = buf; - buf = usb_buffer_alloc(usb_dev, readsize, GFP_KERNEL, &acm->read_dma); - if (!buf) { - dev_dbg(&intf->dev, "out of memory (read buffer alloc)\n"); - goto alloc_fail3; - } - acm->read_buffer = buf; - if (acm_write_buffers_alloc(acm) < 0) { dev_dbg(&intf->dev, "out of memory (write buffer alloc)\n"); goto alloc_fail4; @@ -874,10 +940,25 @@ dev_dbg(&intf->dev, "out of memory (ctrlurb kmalloc)\n"); goto alloc_fail5; } - acm->readurb = usb_alloc_urb(0, GFP_KERNEL); - if (!acm->readurb) { - dev_dbg(&intf->dev, "out of memory (readurb kmalloc)\n"); - goto alloc_fail6; + for (i = 0; i < ACM_MAX_READ_URBS; i++) { + struct acm_read_urb *rcv = &(acm->read_urbs[i]); + + if (!(rcv->urb = usb_alloc_urb(0, GFP_KERNEL))) { + dev_dbg(&intf->dev, "out of memory (read_urbs kmalloc)\n"); + goto alloc_fail7; + } + + rcv->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + rcv->instance = acm; + } + for (i = 0; i < ACM_MAX_READ_BUFS; i++) { + struct acm_read_buffer *buf = &(acm->read_bufs[i]); + + // Using usb_buffer_alloc instead of kmalloc as Oliver suggested + if (!(buf->base = usb_buffer_alloc(acm->dev, readsize, GFP_KERNEL, &buf->dma))) { + dev_dbg(&intf->dev, "out of memory (read_bufs usb_buffer_alloc)\n"); + goto alloc_fail7; + } } acm->writeurb = usb_alloc_urb(0, GFP_KERNEL); if (!acm->writeurb) { @@ -890,15 +971,9 @@ acm->ctrlurb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; acm->ctrlurb->transfer_dma = acm->ctrl_dma; - usb_fill_bulk_urb(acm->readurb, usb_dev, usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress), - acm->read_buffer, readsize, acm_read_bulk, acm); - acm->readurb->transfer_flags |= URB_NO_FSBR | URB_NO_TRANSFER_DMA_MAP; - acm->readurb->transfer_dma = acm->read_dma; - usb_fill_bulk_urb(acm->writeurb, usb_dev, usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress), NULL, acm->writesize, acm_write_bulk, acm); acm->writeurb->transfer_flags |= URB_NO_FSBR | URB_NO_TRANSFER_DMA_MAP; - /* acm->writeurb->transfer_dma = 0; */ dev_info(&intf->dev, "ttyACM%d: USB ACM device\n", minor); @@ -918,14 +993,14 @@ return 0; alloc_fail7: - usb_free_urb(acm->readurb); -alloc_fail6: + for (i = 0; i < ACM_MAX_READ_BUFS; i++) + usb_buffer_free(usb_dev, acm->readsize, acm->read_bufs[i].base, acm->read_bufs[i].dma); + for (i = 0; i < ACM_MAX_READ_URBS; i++) + usb_free_urb(acm->read_urbs[i].urb); usb_free_urb(acm->ctrlurb); alloc_fail5: acm_write_buffers_free(acm); alloc_fail4: - usb_buffer_free(usb_dev, readsize, acm->read_buffer, acm->read_dma); -alloc_fail3: usb_buffer_free(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma); alloc_fail2: kfree(acm); @@ -937,6 +1012,7 @@ { struct acm *acm = usb_get_intfdata (intf); struct usb_device *usb_dev = interface_to_usbdev(intf); + int i; if (!acm || !acm->dev) { dbg("disconnect on nonexisting interface"); @@ -947,15 +1023,24 @@ acm->dev = NULL; usb_set_intfdata (intf, NULL); + tasklet_disable(&acm->urb_task); + usb_kill_urb(acm->ctrlurb); - usb_kill_urb(acm->readurb); usb_kill_urb(acm->writeurb); + for (i = 0; i < ACM_MAX_READ_URBS; i++) + usb_kill_urb(acm->read_urbs[i].urb); + + INIT_LIST_HEAD(&acm->filled_read_bufs); + INIT_LIST_HEAD(&acm->spare_read_bufs); + + tasklet_enable(&acm->urb_task); flush_scheduled_work(); /* wait for acm_softint */ acm_write_buffers_free(acm); - usb_buffer_free(usb_dev, acm->readsize, acm->read_buffer, acm->read_dma); usb_buffer_free(usb_dev, acm->ctrlsize, acm->ctrl_buffer, acm->ctrl_dma); + for (i = 0; i < ACM_MAX_READ_BUFS; i++) + usb_buffer_free(usb_dev, acm->readsize, acm->read_bufs[i].base, acm->read_bufs[i].dma); usb_driver_release_interface(&acm_driver, acm->data); diff -Nru linux-2.6.13.2/drivers/usb/class/cdc-acm.h linux-2.6.13.2-fastcdc/drivers/usb/class/cdc-acm.h --- linux-2.6.13.2/drivers/usb/class/cdc-acm.h 2005-09-17 03:02:12.000000000 +0200 +++ linux-2.6.13.2-fastcdc/drivers/usb/class/cdc-acm.h 2005-09-21 14:09:18.000000000 +0200 @@ -66,22 +66,46 @@ int use; }; +#define ACM_MAX_READ_URBS 16 +#define ACM_MAX_READ_BUFS 16 + +struct acm_read_buffer { + struct list_head list; + int size; + unsigned char *base; + dma_addr_t dma; +}; + +struct acm_read_urb { + struct list_head list; + struct acm_read_buffer *buffer; + struct urb *urb; + struct acm *instance; +}; + struct acm { struct usb_device *dev; /* the corresponding usb device */ struct usb_interface *control; /* control interface */ struct usb_interface *data; /* data interface */ struct tty_struct *tty; /* the corresponding tty */ - struct urb *ctrlurb, *readurb, *writeurb; /* urbs */ - u8 *ctrl_buffer, *read_buffer; /* buffers of urbs */ - dma_addr_t ctrl_dma, read_dma; /* dma handles of buffers */ + struct urb *ctrlurb, *writeurb; /* urbs */ + u8 *ctrl_buffer; /* buffers of urbs */ + dma_addr_t ctrl_dma; /* dma handles of buffers */ struct acm_wb wb[ACM_NWB]; + struct acm_read_urb read_urbs[ACM_MAX_READ_URBS]; + struct acm_read_buffer read_bufs[ACM_MAX_READ_BUFS]; + int rx_endpoint; + spinlock_t read_lock; + struct list_head spare_read_urbs; + struct list_head spare_read_bufs; + struct list_head filled_read_bufs; int write_current; /* current write buffer */ int write_used; /* number of non-empty write buffers */ int write_ready; /* write urb is not running */ spinlock_t write_lock; struct usb_cdc_line_coding line; /* bits, stop, parity */ struct work_struct work; /* work queue entry for line discipline waking up */ - struct tasklet_struct bh; /* rx processing */ + struct tasklet_struct urb_task; /* rx processing */ spinlock_t throttle_lock; /* synchronize throtteling and read callback */ unsigned int ctrlin; /* input control lines (DCD, DSR, RI, break, overruns) */ unsigned int ctrlout; /* output control lines (DTR, RTS) */ @@ -91,7 +115,6 @@ unsigned int minor; /* acm minor number */ unsigned char throttle; /* throttled by tty layer */ unsigned char clocal; /* termios CLOCAL */ - unsigned char resubmit_to_unthrottle; /* throtteling has disabled the read urb */ unsigned int ctrl_caps; /* control capabilities from the class specific header */ };