On Tue, May 15, 2001 at 02:00:04PM -0400, Dan Streetman wrote:
<...>
> uhci.c and usb-ohci.c were easy to follow; I'm pretty confident they are ok.
> usb-uhci.c was rather difficult to follow, and has many complicated paths
> depending on certain states of the URB. I think I got it right tho. If
> the authors of the HCDs could review the patch I'd appreciate it :-)
<...>
> Note that usb-ohci.c was not treating 0-interval interrupts as one-shot; I
> changed it to treat 0-interval interrupts as 1-shot. However the documentation
> (URB.txt) doesn't mention 0-interval/one-shot interrupts at all! So either the
> uhci HCDs or the documentation is wrong...?
As far as I can recall, these one-shot interrupts were a speciality of uhci
and not mentioned in our original URB "spec". I preferred a bulk replacement
at that time, but since a driver author (I can't remember who) wanted them,
I added this behavior afterwards.
> <opinion>
> I think if the HCDs are overhauled in 2.5, I would suggest some changes:
>
> -Allow all URB types to be resubmitted (have a urb->resubmit flag).
That's exactly the case I don't understand and why the patches for *uhci are
actually needed. Why do you need a resubmit for interrupts? Interrupt URBs
run as long as you don't kill them. Actually resubmission seems pretty
useless to me.
> -Allow all URB types to be queued! (I think OHCI already does this?)
This is a hardware issue. OHCI can do this in hardware, UHCI needs much help
with a complicated descriptor structure and pointer handling.
For *uhci, only control transfers currently lack the the queue ability,
isochrounous did it since beginning and for queued interrupts I simply see no
use (a sort of semantic nonsense...).
> -Last opinion: if one of the UHCI HCDs is dropped, I think 'uhci' is cleaner
> than 'usb-uhci'. I would recommend keeping 'uhci'.
> </opinion>
<my opinion, and you can flame me to death, burn me, think of me as arrogant
etc, but there is a time to say it and that is now>
- It is no wonder that uhci look a bit cleaner/simpler, since it based on a
great part on the "algorithms" and dataflows we (T.Sailer&D.Fliegl&Me)
developed in our uhci-rewrite (better: keep the only the init stuff)
project. Rewriting by cloning usually tends to clean up the code, but
doesn't necessarily mean that the core issues were understood (examples
below).
Remember, we designed the URB stuff and thus had (usually ;-) a very good
idea how and why this works like that and why the simpler solutions failed
in our opinion. Each one of us (maybe Thomas is the most Linux-famous in
that class) had written dozens of hardware drivers, so the way usb-uhci
looks like reflects the experiences we made ("simple, but not too
simple...").
Example 1:
If you would have compared the bulk queueing of both uhcis a few weeks ago,
you would have wondered while looking at usb-uhci: "why the hell they are
making so much complicated pointer and descriptor linking, uhci looks much
cleaner and it seems to work, too". I got a few (personal) mails like
that...
Well, the cause is, that the uhci-method (as it turned out) was simple, but
simply false and (obviously) based on the wrong understanding how the
UHCI-HW processes its descriptors. When I started with the queuing stuff
more than one year ago, I never considered that method seriously longer than
a minute and I was a bit surprised to find it actually implemented...
The usb-uhci-queuing method took a few nights to develop and to code, it
looks/is difficult and it had (of course) a few ugly implementation bugs in
it, but it was in its basis correct, there is absolutely no possibility
of a race.
And now it turns up that uhci has races with queuing. Martin Diehl found out
why (the TD pointers are zeroed out by the QH-update), I commented, how and why
usb-uhci handles this in a much more complicated way, and -PLOP- uhci now
seems to work very similar (I haven't analyzed it yet). Huh? Am I missing
something? Should I have been quiet to see how long it takes to solve the
problem for uhci?
Example 2:
There were a lot of sanity check and debugging outputs in usb-uhci in the
beginning and now there are even more (max. interrupt load, descriptor
cleanup, max. ->next count etc.), and they blow up the code and don't help
the readability. But they indicate early problems and avoid a lot of hard
lockups if something goes deadly wrong. So what should I do? Should I remove
them just for the sake of readability?
In the beginning, we had a warning that you wanted more data from an
isochronous EP than specified in the descriptor, and it actually flagged
that for a driver. That was considered more as a flaw of usb-uhci than of
the provoking driver, and we disabled it. Strange world...
<cooling down. Now I feel better...>
--
Georg Acher, [EMAIL PROTECTED]
http://www.in.tum.de/~acher/
"Oh no, not again !" The bowl of petunias
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel