From oliver@neukum.org Thu Apr 21 12:29:53 2005 From: Oliver Neukum To: greg@kroah.com Subject: USB: fix acm trouble with terminals Date: Thu, 21 Apr 2005 21:28:02 +0200 This patch fixes lost LF when ACM device is used with getty/login/bash, in case of a modem which takes calls. Signed-off-by: Pete Zaitcev Signed-off-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman diff -urp -X dontdiff linux-2.6.12-rc1/drivers/usb/class/cdc-acm.c linux-2.6.12-rc1-lem/drivers/usb/class/cdc-acm.c --- linux-2.6.12-rc1/drivers/usb/class/cdc-acm.c 2005-03-18 17:12:12.000000000 -0800 +++ linux-2.6.12-rc1-lem/drivers/usb/class/cdc-acm.c 2005-04-18 13:53:26.000000000 -0700 @@ -106,6 +106,111 @@ static int acm_ctrl_msg(struct acm *acm, acm_ctrl_msg(acm, USB_CDC_REQ_SEND_BREAK, ms, NULL, 0) /* + * Write buffer management. + * All of these assume proper locks taken by the caller. + */ + +static int acm_wb_alloc(struct acm *acm) +{ + int i, wbn; + struct acm_wb *wb; + + wbn = acm->write_current; + i = 0; + for (;;) { + wb = &acm->wb[wbn]; + if (!wb->use) { + wb->use = 1; + return wbn; + } + wbn = (wbn + 1) % ACM_NWB; + if (++i >= ACM_NWB) + return -1; + } +} + +static void acm_wb_free(struct acm *acm, int wbn) +{ + acm->wb[wbn].use = 0; +} + +static int acm_wb_is_avail(struct acm *acm) +{ + int i, n; + + n = 0; + for (i = 0; i < ACM_NWB; i++) { + if (!acm->wb[i].use) + n++; + } + return n; +} + +static inline int acm_wb_is_used(struct acm *acm, int wbn) +{ + return acm->wb[wbn].use; +} + +/* + * Finish write. + */ +static void acm_write_done(struct acm *acm) +{ + unsigned long flags; + int wbn; + + spin_lock_irqsave(&acm->write_lock, flags); + acm->write_ready = 1; + wbn = acm->write_current; + acm_wb_free(acm, wbn); + acm->write_current = (wbn + 1) % ACM_NWB; + spin_unlock_irqrestore(&acm->write_lock, flags); +} + +/* + * Poke write. + */ +static int acm_write_start(struct acm *acm) +{ + unsigned long flags; + int wbn; + struct acm_wb *wb; + int rc; + + spin_lock_irqsave(&acm->write_lock, flags); + if (!acm->dev) { + spin_unlock_irqrestore(&acm->write_lock, flags); + return -ENODEV; + } + + if (!acm->write_ready) { + spin_unlock_irqrestore(&acm->write_lock, flags); + return 0; /* A white lie */ + } + + wbn = acm->write_current; + if (!acm_wb_is_used(acm, wbn)) { + spin_unlock_irqrestore(&acm->write_lock, flags); + return 0; + } + wb = &acm->wb[wbn]; + + acm->write_ready = 0; + spin_unlock_irqrestore(&acm->write_lock, flags); + + acm->writeurb->transfer_buffer = wb->buf; + acm->writeurb->transfer_dma = wb->dmah; + acm->writeurb->transfer_buffer_length = wb->len; + acm->writeurb->dev = acm->dev; + + if ((rc = usb_submit_urb(acm->writeurb, GFP_ATOMIC)) < 0) { + dbg("usb_submit_urb(write bulk) failed: %d", rc); + acm_write_done(acm); + } + return rc; +} + +/* * Interrupt handlers for various ACM device responses */ @@ -237,17 +342,13 @@ static void acm_rx_tasklet(unsigned long static void acm_write_bulk(struct urb *urb, struct pt_regs *regs) { struct acm *acm = (struct acm *)urb->context; - dbg("Entering acm_write_bulk with status %d\n", urb->status); - - if (!ACM_READY(acm)) - goto out; - if (urb->status) - dbg("nonzero write bulk status received: %d", urb->status); + dbg("Entering acm_write_bulk with status %d\n", urb->status); - schedule_work(&acm->work); -out: - acm->ready_for_write = 1; + acm_write_done(acm); + acm_write_start(acm); + if (ACM_READY(acm)) + schedule_work(&acm->work); } static void acm_softint(void *private) @@ -351,32 +452,33 @@ static int acm_tty_write(struct tty_stru { struct acm *acm = tty->driver_data; int stat; + unsigned long flags; + int wbn; + struct acm_wb *wb; + dbg("Entering acm_tty_write to write %d bytes,\n", count); if (!ACM_READY(acm)) return -EINVAL; - if (!acm->ready_for_write) - return 0; if (!count) return 0; - count = (count > acm->writesize) ? acm->writesize : count; + spin_lock_irqsave(&acm->write_lock, flags); + if ((wbn = acm_wb_alloc(acm)) < 0) { + spin_unlock_irqrestore(&acm->write_lock, flags); + acm_write_start(acm); + return 0; + } + wb = &acm->wb[wbn]; + count = (count > acm->writesize) ? acm->writesize : count; dbg("Get %d bytes...", count); - memcpy(acm->write_buffer, buf, count); - dbg(" Successfully copied.\n"); + memcpy(wb->buf, buf, count); + wb->len = count; + spin_unlock_irqrestore(&acm->write_lock, flags); - acm->writeurb->transfer_buffer_length = count; - acm->writeurb->dev = acm->dev; - - acm->ready_for_write = 0; - stat = usb_submit_urb(acm->writeurb, GFP_ATOMIC); - if (stat < 0) { - dbg("usb_submit_urb(write bulk) failed"); - acm->ready_for_write = 1; + if ((stat = acm_write_start(acm)) < 0) return stat; - } - return count; } @@ -385,7 +487,11 @@ static int acm_tty_write_room(struct tty struct acm *acm = tty->driver_data; if (!ACM_READY(acm)) return -EINVAL; - return !acm->ready_for_write ? 0 : acm->writesize; + /* + * Do not let the line discipline to know that we have a reserve, + * or it might get too enthusiastic. + */ + return (acm->write_ready && acm_wb_is_avail(acm)) ? acm->writesize : 0; } static int acm_tty_chars_in_buffer(struct tty_struct *tty) @@ -393,7 +499,10 @@ static int acm_tty_chars_in_buffer(struc struct acm *acm = tty->driver_data; if (!ACM_READY(acm)) return -EINVAL; - return !acm->ready_for_write ? acm->writeurb->transfer_buffer_length : 0; + /* + * This is inaccurate (overcounts), but it works. + */ + return (ACM_NWB - acm_wb_is_avail(acm)) * acm->writesize; } static void acm_tty_throttle(struct tty_struct *tty) @@ -526,6 +635,39 @@ static void acm_tty_set_termios(struct t * USB probe and disconnect routines. */ +/* Little helper: write buffers free */ +static void acm_write_buffers_free(struct acm *acm) +{ + int i; + struct acm_wb *wb; + + for (wb = &acm->wb[0], i = 0; i < ACM_NWB; i++, wb++) { + usb_buffer_free(acm->dev, acm->writesize, wb->buf, wb->dmah); + } +} + +/* Little helper: write buffers allocate */ +static int acm_write_buffers_alloc(struct acm *acm) +{ + int i; + struct acm_wb *wb; + + for (wb = &acm->wb[0], i = 0; i < ACM_NWB; i++, wb++) { + wb->buf = usb_buffer_alloc(acm->dev, acm->writesize, GFP_KERNEL, + &wb->dmah); + if (!wb->buf) { + while (i != 0) { + --i; + --wb; + usb_buffer_free(acm->dev, acm->writesize, + wb->buf, wb->dmah); + } + return -ENOMEM; + } + } + return 0; +} + static int acm_probe (struct usb_interface *intf, const struct usb_device_id *id) { @@ -700,7 +842,8 @@ skip_normal_probe: acm->bh.data = (unsigned long) acm; INIT_WORK(&acm->work, acm_softint, acm); spin_lock_init(&acm->throttle_lock); - acm->ready_for_write = 1; + spin_lock_init(&acm->write_lock); + acm->write_ready = 1; buf = usb_buffer_alloc(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); if (!buf) { @@ -716,12 +859,10 @@ skip_normal_probe: } acm->read_buffer = buf; - buf = usb_buffer_alloc(usb_dev, acm->writesize, GFP_KERNEL, &acm->write_dma); - if (!buf) { + if (acm_write_buffers_alloc(acm) < 0) { dev_dbg(&intf->dev, "out of memory (write buffer alloc)\n"); goto alloc_fail4; } - acm->write_buffer = buf; acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL); if (!acm->ctrlurb) { @@ -750,9 +891,9 @@ skip_normal_probe: acm->readurb->transfer_dma = acm->read_dma; usb_fill_bulk_urb(acm->writeurb, usb_dev, usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress), - acm->write_buffer, acm->writesize, acm_write_bulk, acm); + NULL, acm->writesize, acm_write_bulk, acm); acm->writeurb->transfer_flags |= URB_NO_FSBR | URB_NO_TRANSFER_DMA_MAP; - acm->writeurb->transfer_dma = acm->write_dma; + /* acm->writeurb->transfer_dma = 0; */ dev_info(&intf->dev, "ttyACM%d: USB ACM device\n", minor); @@ -775,7 +916,7 @@ alloc_fail7: alloc_fail6: usb_free_urb(acm->ctrlurb); alloc_fail5: - usb_buffer_free(usb_dev, acm->writesize, acm->write_buffer, acm->write_dma); + acm_write_buffers_free(acm); alloc_fail4: usb_buffer_free(usb_dev, readsize, acm->read_buffer, acm->read_dma); alloc_fail3: @@ -806,7 +947,7 @@ static void acm_disconnect(struct usb_in flush_scheduled_work(); /* wait for acm_softint */ - usb_buffer_free(usb_dev, acm->writesize, acm->write_buffer, acm->write_dma); + 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); diff -urp -X dontdiff linux-2.6.12-rc1/drivers/usb/class/cdc-acm.h linux-2.6.12-rc1-lem/drivers/usb/class/cdc-acm.h --- linux-2.6.12-rc1/drivers/usb/class/cdc-acm.h 2005-03-18 17:12:12.000000000 -0800 +++ linux-2.6.12-rc1-lem/drivers/usb/class/cdc-acm.h 2005-03-19 17:09:44.000000000 -0800 @@ -51,14 +51,34 @@ * Internal driver structures. */ +/* + * The only reason to have several buffers is to accomodate assumptions + * in line disciplines. They ask for empty space amount, receive our URB size, + * and proceed to issue several 1-character writes, assuming they will fit. + * The very first write takes a complete URB. Fortunately, this only happens + * when processing onlcr, so we only need 2 buffers. + */ +#define ACM_NWB 2 +struct acm_wb { + unsigned char *buf; + dma_addr_t dmah; + int len; + int use; +}; + 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, *write_buffer; /* buffers of urbs */ - dma_addr_t ctrl_dma, read_dma, write_dma; /* dma handles of buffers */ + u8 *ctrl_buffer, *read_buffer; /* buffers of urbs */ + dma_addr_t ctrl_dma, read_dma; /* dma handles of buffers */ + struct acm_wb wb[ACM_NWB]; + 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 */ @@ -71,7 +91,6 @@ struct acm { unsigned int minor; /* acm minor number */ unsigned char throttle; /* throttled by tty layer */ unsigned char clocal; /* termios CLOCAL */ - unsigned char ready_for_write; /* write urb can be used */ unsigned char resubmit_to_unthrottle; /* throtteling has disabled the read urb */ unsigned int ctrl_caps; /* control capabilities from the class specific header */ }; Signed-off-by: Pete Zaitcev -- Pete