Am Samstag, 19. M�rz 2005 23:22 schrieb Pete Zaitcev:
> On Sat, 19 Mar 2005 10:25:43 +0100 Oliver Neukum <[EMAIL PROTECTED]> wrote:
> > Am Samstag, 19. M�rz 2005 06:41 schrieb Pete Zaitcev:
>
> > > I would like someone who cares about cdc-acm to consider this patch.
> > >
> > > A customer of mine hits this problem. They use a modem for remote access.
> > > User connects, logs in. Then, bash prompt does not scroll, just overwrites
> > > lowest line. This happens because LF is lost, only CR goes out. There's
> > > a comment in the patch about it.
> >
> > Upon further consideration, is this a comprehensive fix? As far as I can see
> > it will work only for two byte combinations like CR/LF. A three or more byte
> > combination will still fail. Isn't it possible to tell the line disciplines
> > how many buffers there are?
>
> Normally the line disciplines test for fit before every write. I do not
> think another case exists where a single test is followed with several
> smaller writes. So, the patch is limited this way on purpose. The thing
> is, once we have a software FIFO, where do we stop expanding it? I thought
> I needed to draw a line in the sand.
This makes sense, but:
+static void acm_wb_free(struct acm *acm, int wbn)
+{
+ acm->wb[wbn].use = 0;
+}
Why isn't this also inline?
+static int acm_wb_is_avail(struct acm *acm)
+{
+ int i, wbn, n;
+
+ n = 0;
+ wbn = acm->write_current;
+ for (i = 0; i < ACM_NWB; i++) {
+ if (!acm->wb[wbn].use)
+ n++;
+ wbn = (wbn + 1) % ACM_NWB;
+ }
+ return n;
+}
Why two counters?
+ if (acm->dev == NULL) {
+ spin_unlock_irqrestore(&acm->write_lock, flags);
+ return -ENODEV;
+ }
Should be tested first.
+ acm->write_ready = 0;
+
+ acm->writeurb->transfer_buffer = wb->buf;
+ acm->writeurb->transfer_dma = wb->dmah;
+ acm->writeurb->transfer_buffer_length = wb->len;
+ acm->writeurb->dev = acm->dev;
+ spin_unlock_irqrestore(&acm->write_lock, flags);
Why is the spinlock released exactly there?
+ /*
+ * This is inaccurate (overcounts), but it works.
+ */
+ return (ACM_NWB - acm_wb_is_avail(acm)) * acm->writesize;
Conservative would be to return one buffer size.
- acm->write_buffer, acm->writesize, acm_write_bulk,
acm);
+ NULL, acm->writesize, acm_write_bulk, acm);
Why?
+ 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);
Why unconditionally switching to the next buffer?
It would be more efficient to switch buffers only at need.
Regards
Oliver
-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_ide95&alloc_id396&op=click
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
