On 10/21/2015 06:12 AM, Oliver Neukum wrote: > On Tue, 2015-10-20 at 14:16 -0400, Peter Hurley wrote: >> ECHO is on by default and the cdc-acm driver does not implement the >> put_char() and flush_chars() tty driver methods, which made the >> problem >> _way worse_, since every echoed char is sent as it's own URB. > > That can be fixed. How can I test this?
I'm working on getting my tty tests organized for upstreaming, but it's not there yet. Let me see if I can get you a standalone test for accuracy, at least. > Will the tty layer use write() and put_char()? Echoing uses put_char() + flush_chars() for echoing, if the driver supports it; otherwise, it uses write(). Be aware that while the N_TTY line discipline ensures that put_char() and write() usage will not be interleaved without flush_chars(), other line disciplines might not, so at least be sure that driver won't crash if that happens. Regards, Peter Hurley > From ac75a2c9ea67ec22c704a6bcaa30e6fc415d64d3 Mon Sep 17 00:00:00 2001 > From: Oliver Neukum <[email protected]> > Date: Wed, 21 Oct 2015 12:10:07 +0200 > Subject: [PATCH] cdc-acm: implement put_char() and flush_chars() > > This should cut down latencies and waste if the tty layer writes single bytes. > > Signed-off-by: Oliver Neukum >[email protected]> > --- > drivers/usb/class/cdc-acm.c | 58 > +++++++++++++++++++++++++++++++++++++++++++++ > drivers/usb/class/cdc-acm.h | 1 + > 2 files changed, 59 insertions(+) > > diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c > index b30e742..262f179 100644 > --- a/drivers/usb/class/cdc-acm.c > +++ b/drivers/usb/class/cdc-acm.c > @@ -725,6 +725,62 @@ static int acm_tty_write(struct tty_struct *tty, > return count; > } > > +static void acm_tty_flush_chars(struct tty_struct *tty) > +{ > + struct acm *acm = tty->driver_data; > + struct acm_wb *cur = acm->putbuffer; > + int err; > + unsigned long flags; > + > + acm->putbuffer = NULL; > + err = usb_autopm_get_interface_async(acm->control); > + spin_lock_irqsave(&acm->write_lock, flags); > + if (err < 0) { > + cur->use = 0; > + goto out; > + } > + > + if (acm->susp_count) { > + usb_anchor_urb(cur->urb, &acm->delayed); > + goto out; > + } > + > + acm_start_wb(acm, cur); > +out: > + spin_unlock_irqrestore(&acm->write_lock, flags); > + return; > +} > + > +static int acm_tty_put_char(struct tty_struct *tty, unsigned char ch) > +{ > + struct acm *acm = tty->driver_data; > + struct acm_wb *cur; > + int wbn; > + unsigned long flags; > + > +overflow: > + cur = acm->putbuffer; > + if (!cur) { > + spin_lock_irqsave(&acm->write_lock, flags); > + wbn = acm_wb_alloc(acm); > + if (wbn >= 0) { > + cur = &acm->wb[wbn]; > + acm->putbuffer = cur; > + } > + spin_unlock_irqrestore(&acm->write_lock, flags); > + if (!cur) > + return 0; > + } > + > + if (cur->len == acm->writesize) { > + acm_tty_flush_chars(tty); > + goto overflow; > + } > + > + cur->buf[cur->len++] = ch; > + return 1; > +} > + > static int acm_tty_write_room(struct tty_struct *tty) > { > struct acm *acm = tty->driver_data; > @@ -1888,6 +1944,8 @@ static const struct tty_operations acm_ops = { > .cleanup = acm_tty_cleanup, > .hangup = acm_tty_hangup, > .write = acm_tty_write, > + .put_char = acm_tty_put_char, > + .flush_chars = acm_tty_flush_chars, > .write_room = acm_tty_write_room, > .ioctl = acm_tty_ioctl, > .throttle = acm_tty_throttle, > diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h > index dd9af38..648a6f7 100644 > --- a/drivers/usb/class/cdc-acm.h > +++ b/drivers/usb/class/cdc-acm.h > @@ -94,6 +94,7 @@ struct acm { > unsigned long read_urbs_free; > struct urb *read_urbs[ACM_NR]; > struct acm_rb read_buffers[ACM_NR]; > + struct acm_wb *putbuffer; /* for > acm_tty_put_char() */ > int rx_buflimit; > int rx_endpoint; > spinlock_t read_lock; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
