On Thursday 03 January 2008, Karsten Wiese wrote:
> Am Donnerstag, 3. Januar 2008 schrieb David Brownell:
> > On Wednesday 02 January 2008, Karsten Wiese wrote:
> > > 
> > > In scan_periodic()'s case Q_TYPE_ITD:
> > > those lines:
> > >           *q_p = q.itd->itd_next;
> > >           *hw_p = q.itd->hw_next;
> > > IMO should only execute once the frame has elapsed.
> > 
> > And that would be ... why?
> > 
> > Absolutely the strongest statement you can make in this area
> > is that some silicon *might* cache that part of a frame's
> > schedule but not have any invalidation mechanism.
> > 
> > Now, if it doesn't do that caching, or invalidates sanely,
> > then it's obviously safe.
> > 
> > And if it doesn't, and those assumptions are met, then how
> > could anything go wrong?

[ For clarity:  those assumptions were "that only completions will
add to the schedule and that drivers aren't doing silly stuff like
more than two URBs per frame (for a given endpoint)". ]

 
> My snd-usb-us122l driver tries to be too clever.
> it resubmits a completed earlier urb when it runs the completion of
> a second urb:

Hmm, so there might actually be two issues you're seeing, then.

I can see this issue you're outlining ... basically violating the
first assumption by having some code outside the endpoint's own
completion handler adding to its schedule.

But there's evidently another issue, seen more often with full speed
ISO, kicking in when multiple ISO transfers complete in one frame.
If they hit a small window at end-of-frame, the schedule scanning
might wrongly advance into the next frame while before the HC finishes
writing back status for later ISO transfers.  Result:  a transfer gets
skipped.  Not unlike the "sooner" issue you noted.  That should be
addressed by a updated patch I'll send in a bit.  (Does that sound
to you like a failure you might be seeing?)


> In- and Out-urbs are submitted so they complete at the same uFrame.
> Of such an In- Out- pair
> - The first completing urb isn't submitted from the completion callback.
> - The second completing urb's completion callback resubmits both urbs.
> 
> Why? Because Out-urb's packet-lengths and nr_of_packets are taken from 
> in-urb (and its predecessor) and in-urb's nr_of_packets is set to
> out-urb's.

That still leaves me wondering "why" though.  :)

What's the purpose of coordinating ISO streams in that way,
and why shouldn't they all just use predefined values?  If they
didn't try to coordinate between endpoints, this issue wouldn't
be coming up.


> Your patch "ehci delays recycling ISO DMA descriptors" doesn't help
> here. To let it help, the completion callback has to resubmit.

Doesn't hurt either, and it *would* help drivers following the
more conventional idioms!

 
> An idea to make snd-usb-us122l work with your patches would depend on a
> "complete iso urbs sharing equal start- and stop-uFrames in the sequence
>  they were submitted in." ehci patch.

We've only guaranteed that in the context of a single endpoint;
there are currently no between-endpoints guarantees I'm aware of
in the Linux-USB stack.

Your code might have similar issues if it were trying to work
with OHCI or most other HCDs.  I don't think *any* of them make
the kind of assumptions you want ...

It may be worth exploring what making those guarantees might
involve, but for the moment it would be a lot better if your
driver didn't want those things!

- Dave
-
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to