[PATCH] iso pipe support for usbatm

Roman Kagan rkagan at mail.ru
Fri Mar 25 03:27:30 EST 2005


On Thu, Mar 24, 2005 at 11:57:36PM +0100, matthieu castet wrote:
> Roman Kagan wrote:
> >On Thu, Mar 24, 2005 at 07:36:07PM +0100, matthieu castet wrote:
> >
> >>-	if (likely(!urb->status))
> >>+	if (likely(!urb->status) ||
> >>+			/* via chipset produce lot's of crc error...*/
> >>+			(usb_pipeisoc(instance->rx_endpoint) && urb->status 
> >>== -EILSEQ))
> >>		tasklet_schedule(&instance->receive_tasklet);
> >
> >
> >This test should go a bit earlier, and set ->filled_cells as we
> >discussed instead of tasklet_schedule (AFAICT calling or not
> >tasklet_schedule here doesn't change the fact that the data from this
> >urb will be processed, it just makes it wait until the next successful
> >urb completion).
> >
> >Ups, just struck me: when you see -EILSEQ, is the data in the buffer all
> >valid?  Or it's actually bogus and is being discarded later on in
> >usbatm_extract_cells()?
> in fact in case of -EILSEQ, there could be some frames that are valid, 
> but some are invalid : we check the status for each frame in the loop 
> before.

Do you actually see nonzero number of valid frames in urbs returning
-EILSEQ?  Then it doesn't sound like a good idea to discard such urbs as
a whole, which is done in this test.

> This test is to avoid in case of consecutive errors to never call 
> tasklet_schedule, and never submit again urb...

In other words, if all urbs we had submitted returned with an error
let's deadlock.  Not too good.  I guess with the small number of urbs
we're using there may be valid reasons for them to occasionally fail all
in sequence.  I believe always scheduling the tasklet is safe.

Cheers,
  Roman.



More information about the Usbatm mailing list