Greg,
I didn't mean to personally offend you or any of the other usb-uhci authors with
my opinion. I was only saying that, for both uhci and usb-uhci, I tried to
follow the flow of a URB (from submit to completion), and I thought that uhci
was easier to follow. It had less conditionals, different execution paths based
on URB info, etc. In my opinion, less complicated is usually better. It very
well may be that usb-uhci performs better, I really can't comment on that. But
I think in many cases, code that is difficult to follow, is difficult to modify.
And code that's difficult to modify is difficult to maintain, and it winds up
that very few people really understand what it's doing.
I certainly did not mean to imply that it is bad; I only was trying to say, if
the time comes that for whatever reason one of the UHCI drivers goes away, I
recommend the one that's easier to follow, because more people will be able to
understand exactly what it's doing.
Anyway, that really was a side note to my email. Do you have an opinion on
the patch? If you could take a look at what I did in usb-uhci I'd appreciate
it.
Thanks.
On Wed, 16 May 2001, Georg Acher wrote:
>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...>
>
>
--
Dan Streetman
[EMAIL PROTECTED]
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
http://lists.sourceforge.net/lists/listinfo/linux-usb-devel
