RE: What to do with all of the USB UHCI drivers in the kernel ?

RE: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

Greg,

I'm gonna speak for Bluetooth USB devices.
I do have bunch of things like Kodak digi camera, Sony DV camcorder, CF 
reader, etc. But they don't
seem to care much about which HCD is used and work equally well with both 
usb-uhci and uhci drivers.

I used to be a uhci driver fan :). But starting somewhere from 2.4.16 or so 
Bluetooth devices work much better
with usb-uhci driver (not all devices but most of them). Even thought 
Bluetooth is pretty slow (about 700kbps)
performance difference is sometimes pretty significant 20-30% (ie usb-uhci 
driver is faster).

So basically I vote for usb-uhci. However some things will have to be 
fixed. We (Bluetooth folks) have couple
of devices that refuse to work with usb-uhci (I didn't test the latest 
usb-uhci though).

Thanks
Max

>Ok, now that 2.5.16 is out, we have a total of 4 different USB UHCI
>controller drivers in the kernel!  That's about 3 too many for me :)
>
>So what to do?  I propose the following:
>
>   From now until July 1, I want everyone to test out both the uhci-hcd
>   and usb-uhci-hcd drivers on just about every piece of hardware they
>   can find.  This includes SMP, UP, preempt kernels, big and little
>   endian machines, and loads of different types of USB devices.
>
>   (Note, for those who don't realize it, the uhci-hcd driver is based
>   off of the uhci.c driver, and the usb-uhci-hcd driver is based off of
>   the usb-uhci.c driver.  Both of these drivers now use the USB HCD
>   interface, hopefully reducing some code complexity and size because of
>   this.  So this means that the uhci.c and usb-uhci.c drivers are
>   going to go away, just like the usb-ohci.c driver did in 2.5.16.)
>
>   Let me (and the linux-usb-devel list) know about any thoughts you have
>   pertaining to liking one of the drivers over the other one.  Speed
>   tests, size tests, code pretty tests, comment spelling tests,
>   documentation tests, you name it, I want to know about it.  If you
>   don't want your comments to be public, send them to me directly and I
>   will not let anyone else know what you said, but will use the info to
>   try to pick which one should stay.
>
>   I will be doing the same thing (running speed tests, and hardware
>   tests) and will be publishing those results on the linux-usb-devel
>   list for others to verify.
>
>   Then, the week of July 1, I will be taking everyone's comments and
>   making a decision about which driver to keep.
>
>thanks,
>
>greg k-h



Max

http://bluez.sf.net
http://vtun.sf.net

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Greg KH]

On Tue, May 21, 2002 at 12:41:39PM -0700, Maksim (Max) Krasnyanskiy wrote:
> Greg,
> 
> I'm gonna speak for Bluetooth USB devices.
> I do have bunch of things like Kodak digi camera, Sony DV camcorder, CF 
> reader, etc. But they don't
> seem to care much about which HCD is used and work equally well with both 
> usb-uhci and uhci drivers.
> 
> I used to be a uhci driver fan :). But starting somewhere from 2.4.16 or so 
> Bluetooth devices work much better
> with usb-uhci driver (not all devices but most of them). Even thought 
> Bluetooth is pretty slow (about 700kbps)
> performance difference is sometimes pretty significant 20-30% (ie usb-uhci 
> driver is faster).
> 
> So basically I vote for usb-uhci. However some things will have to be 
> fixed. We (Bluetooth folks) have couple
> of devices that refuse to work with usb-uhci (I didn't test the latest 
> usb-uhci though).

Sorry for the confusion, but both usb-uhci.c and uhci.c will be deleted
anyway :)

I am more interested in usb-uhci-hcd.c and uhci-hcd.c drivers, which both
showed up in 2.5.16.  Yes they are based on the previous usb-uhci.c and
uhci.c drivers respectivly, but they are a bit different (they use the
hcd core code which reduces the size of the driver.)

You also might want to check out uhci.c again in 2.4.19-pre.  It has had
a lot of previous bugs fixed and works _much_ better for me than before.

thanks,

greg k-h

Re: [linux-usb-devel] RE: What to do with all of the USB UHCI drivers in the kernel ?

[Johannes Erdfelt]

On Tue, May 21, 2002, Maksim (Max) Krasnyanskiy <maxk@qualcomm.com> wrote:
> I'm gonna speak for Bluetooth USB devices.
> I do have bunch of things like Kodak digi camera, Sony DV camcorder, CF 
> reader, etc. But they don't
> seem to care much about which HCD is used and work equally well with both 
> usb-uhci and uhci drivers.
> 
> I used to be a uhci driver fan :). But starting somewhere from 2.4.16 or so 
> Bluetooth devices work much better
> with usb-uhci driver (not all devices but most of them). Even thought 
> Bluetooth is pretty slow (about 700kbps)
> performance difference is sometimes pretty significant 20-30% (ie usb-uhci 
> driver is faster).
> 
> So basically I vote for usb-uhci. However some things will have to be 
> fixed. We (Bluetooth folks) have couple
> of devices that refuse to work with usb-uhci (I didn't test the latest 
> usb-uhci though).

Please test the latest version of the drivers. Both uhci and usb-uhci
have had various bug fixes. I'm sure the performance problems you've had
with uhci have been fixed for a little while now.

Also, feedback about the -hcd variants would useful too since one of
those will be the only ones left for 2.5.

JE

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

> > So basically I vote for usb-uhci. However some things will have to be
> > fixed. We (Bluetooth folks) have couple
> > of devices that refuse to work with usb-uhci (I didn't test the latest
> > usb-uhci though).
>
>Sorry for the confusion, but both usb-uhci.c and uhci.c will be deleted 
>anyway :)
I thought that usb-uhci-hcd and uhci-hcd are direct derivatives of usb-uhci 
and uhci
(ie just minor API changes). And therefor perform exactly the same.

>I am more interested in usb-uhci-hcd.c and uhci-hcd.c drivers, which both
>showed up in 2.5.16.  Yes they are based on the previous usb-uhci.c and
>uhci.c drivers respectivly, but they are a bit different (they use the
>hcd core code which reduces the size of the driver.)
I see. Ok. I'll try 2.5.17 on one of my machines with UHCI controller.

>You also might want to check out uhci.c again in 2.4.19-pre.  It has had
>a lot of previous bugs fixed and works _much_ better for me than before.
Ok.

Thanks
Max

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Johannes Erdfelt]

On Tue, May 21, 2002, Maksim (Max) Krasnyanskiy <maxk@qualcomm.com> wrote:
> 
> > > So basically I vote for usb-uhci. However some things will have to be
> > > fixed. We (Bluetooth folks) have couple
> > > of devices that refuse to work with usb-uhci (I didn't test the latest
> > > usb-uhci though).
> >
> >Sorry for the confusion, but both usb-uhci.c and uhci.c will be deleted 
> >anyway :)
> I thought that usb-uhci-hcd and uhci-hcd are direct derivatives of usb-uhci 
> and uhci
> (ie just minor API changes). And therefor perform exactly the same.

I wouldn't consider it a minor API change, but theoretically they should
perform identically. Since some changes were non trivial, I wouldn't
guarantee that they behave identically :)

However, I'm not sure that's all that interesting. The code is a
straight enough port over that if there are bugs, they'll be there in
both versions, except for some trivial porting mistakes. Those are easy
to find and easy to fix normally.

The other kinds of bugs, like fundamental design flaws or bugs that
have always been there, are more interesting and likely to be in both.

IMO, I think testing with usb-uhci.c and uhci.c is still useful, but
testing with the -hcd variants is the most ideal since that will be the
final code base.

JE

Re: [linux-usb-devel] RE: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

>Please test the latest version of the drivers. Both uhci and usb-uhci
>have had various bug fixes. I'm sure the performance problems you've had
>with uhci have been fixed for a little while now.
Yep. I did a quick test with Intel, Ericsson and Broadcom devices.
Both HCDs have the same performance in 2.4.19-pre8.

>Also, feedback about the -hcd variants would useful too since one of
>those will be the only ones left for 2.5.
Will do.

Max

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

>IMO, I think testing with usb-uhci.c and uhci.c is still useful, but
>testing with the -hcd variants is the most ideal since that will be the
>final code base.

Ok. Here is feedback on 2.5.17 uhci-hcd and usb-uhci-hcd.
I did not notice any difference in behavior. Both have the same 
performance, just like 2.4.19-pre8.

One-shot interrupt transfers are broken in *-hcd drivers. core/hcd.c 
returns EINVAL if urb->interval==0.
My Broadcom FW loader (uses usbdevfs) needs one-shot interrupts. So in 
order to test Broadcom devices
I changed to hcd.c to allow urb->interval==0. With that change uhci-hcd 
works just fine, I can load fw and
use the device. But usb-uhci-hcd kills the machine pretty hard (hw reset 
needed).

Here is a patch for hcd.c.

--- hcd.c.orig  Tue May 21 17:50:09 2002
+++ hcd.c       Tue May 21 17:01:44 2002
@@ -1456,11 +1456,9 @@
          * supports different values... this uses EHCI/UHCI defaults (and
          * EHCI can use smaller non-default values).
          */
-       switch (temp) {
-       case PIPE_ISOCHRONOUS:
-       case PIPE_INTERRUPT:
+       if (urb->interval && (temp == PIPE_ISOCHRONOUS || temp == 
PIPE_INTERRUPT)) {
                 /* too small? */
-               if (urb->interval <= 0)
+               if (urb->interval < 0)
                         return -EINVAL;
                 /* too big? */
                 switch (urb->dev->speed) {

usb-uhci-hcd has to be fixed.
btw It tries to round interval value even thought it's done by hcd.c

So, Bluetooth USB devices should work fine with either usb-uhci-hcd or 
uhci-hcd.
(assuming that above patch is applied and one-shot is fixed in usb-uhci-hcd)

On a side note. Why are URBs still not SLABified ?
Drivers still have those silly urb pools and stuff. I thought you guys were 
gonna fix that.

Max

Re: [linux-usb-devel] Re: What to do with all of the USB UHCI drivers in the kernel ?

[David Brownell]

Maksim (Max) Krasnyanskiy wrote:
> 
> One-shot interrupt transfers are broken in *-hcd drivers. core/hcd.c 
> returns EINVAL if urb->interval==0.

Hmm, eventually that automagic resubmit model needs to go away,
in favor of a straight queued transfer model -- where in effect
there are _only_ "one shot" transfers, which for interrupts are
just going to hang out on endpoint queues that are properly
scheduled.  That's needed to make interrupt reads and writes
have the same transfer model ... right now interrupt OUT transfers
don't work very well.  (And they'll be the most common type when
we eventually get those device/target side driver APIs sorted.)

Meanwhile, I suppose I can see wanting access to that UHCI-ism.
However your patch would do that wrong, since it should only
apply to interrupt transfers.


> usb-uhci-hcd has to be fixed.
> btw It tries to round interval value even thought it's done by hcd.c

That was one of my quick-review comments too.  It doesn't hurt,
it's just one of several URB sanity-check/preprocessing steps
that doesn't need to be in every driver.


> On a side note. Why are URBs still not SLABified ?

Hasn't seemed to be necessary.  kmalloc() is slabified already,
so it's not clear that a control/bulk/interrupt URB pool shared
between drivers -- size, maybe a handful -- would be better than
sharing that same memory with other kernel code.

- Dave

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Greg KH]

On Tue, May 21, 2002 at 06:04:21PM -0700, Maksim (Max) Krasnyanskiy wrote:
> 
> >IMO, I think testing with usb-uhci.c and uhci.c is still useful, but
> >testing with the -hcd variants is the most ideal since that will be the
> >final code base.
> 
> Ok. Here is feedback on 2.5.17 uhci-hcd and usb-uhci-hcd.
> I did not notice any difference in behavior. Both have the same 
> performance, just like 2.4.19-pre8.
> 
> One-shot interrupt transfers are broken in *-hcd drivers. core/hcd.c 
> returns EINVAL if urb->interval==0.
> My Broadcom FW loader (uses usbdevfs) needs one-shot interrupts. So in 
> order to test Broadcom devices
> I changed to hcd.c to allow urb->interval==0. With that change uhci-hcd 
> works just fine, I can load fw and
> use the device. But usb-uhci-hcd kills the machine pretty hard (hw reset 
> needed).
> 
> Here is a patch for hcd.c.

Thanks for the patch.

> On a side note. Why are URBs still not SLABified ?
> Drivers still have those silly urb pools and stuff. I thought you guys were 
> gonna fix that.

It hasn't been proven that it's really needed.  95% of the current
drivers create their urbs when the device is plugged in, and then free
them when they are removed.  Making that kind of allocation into a slab
is a bit silly :)

Now if more drivers start doing fun stuff like the visor.c driver does
in the 2.5 tree, then it might make more sense to create a URB specific
slab.

thanks,

greg k-h

Re: [linux-usb-devel] Re: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

At 09:33 PM 5/21/2002 -0700, David Brownell wrote:
>Maksim (Max) Krasnyanskiy wrote:
>>One-shot interrupt transfers are broken in *-hcd drivers. core/hcd.c 
>>returns EINVAL if urb->interval==0.
><....>
>Meanwhile, I suppose I can see wanting access to that UHCI-ism.
>However your patch would do that wrong, since it should only
>apply to interrupt transfers.
Yeah, I guess it doesn't make sense to do one-shot iso.

>>On a side note. Why are URBs still not SLABified ?
>
>Hasn't seemed to be necessary.  kmalloc() is slabified already,
>so it's not clear that a control/bulk/interrupt URB pool shared
>between drivers -- size, maybe a handful -- would be better than
>sharing that same memory with other kernel code.
Well, kmalloc has slabs of a fixed size and will round requested amount
to the nearest value. For example on x86 sizeof(struct urb) == 84 which
will be rounded to 96 by kmalloc ie 12 bytes unused (on Sparc64 it's about 
20 bytes).
Also we could use slab constructor to pre-initialize some urb fields.

Max

Re: What to do with all of the USB UHCI drivers in the kernel ?

[Maksim (Max) Krasnyanskiy]

At 10:06 PM 5/21/2002 -0700, Greg KH wrote:
> > On a side note. Why are URBs still not SLABified ?
> > Drivers still have those silly urb pools and stuff. I thought you guys 
> were
> > gonna fix that.
>
>It hasn't been proven that it's really needed.  95% of the current
>drivers create their urbs when the device is plugged in, and then free
>them when they are removed.  Making that kind of allocation into a slab
>is a bit silly :)
Well, I'm claiming that those drivers are wrong ;)
It makes sense to pre-allocate intr in, iso and bulk in URB. But (imo) it 
doesn't make much sense
to pre-allocate ctrl and bulk out. For example one might never send stuff 
out of USB serial port,
because it used only for logging or something, ut write_urb will always be 
allocated. Same
goes for ctrl, one sends ctrl requests only once in a while but URB is 
always allocated.

Also as Dave pointed out kmalloc is already slabified anyway. And as I 
mentioned kmalloc
has fixed size slabs and therefor will waste some memory.

>Now if more drivers start doing fun stuff like the visor.c driver does
>in the 2.5 tree, then it might make more sense to create a URB specific
>slab.
Some drivers are still maintaining bulk URB pools which is just unnecessary 
code and mem waste.
They should allocate/free bulk urbs on demand.
visor, usbnet, hci_usb and some other driver allocate URB on demand.

I guess the reason for having driver specific URB pools is because 
usb_alloc_urb (using kmalloc) was
slow. Now if it's replaced with slab cache it's no longer the case. So 
drivers should not pre-allocate
stuff unnecessary.

It definitely doesn't hurt to have URB specific cache. You get better stats 
(like URB slab utilization, etc).
You could chose to do slab poisoning and debug URB allocations and stuff. 
You could use slab constructor
to pre-initialize some URB fields.

Max

Re: [linux-usb-devel] Re: What to do with all of the USB UHCI drivers in the kernel?

[David Brownell]

> Could you please just do me a small favour and drop something
> in to linux/Documentation. Becouse I'm right now already confused
> about which driver to use and which alias to put in /etc/modules.conf
> so kudzu stops hollering about not knowing what to do
> if I out of a sudden reboot in to 2.5.xx kernel.

I don't think that belongs in Linux/Documentation since it's
specific to RedHat.  Apart from the "which UHCI" confusion,
you could use the /etc/rc.d/init.d/hotplug script to start
up USB. (That's not part of RedHat though; they use kudzu to
promote what seems like more of a static config model.)

I have an RH 7.3 system that has this ... though I had to
tweak it by hand since their setup stored multiple entries
for some drivers, and didn't put EHCI first (so devices on
USB 2.0 adapters would start out with one driver, disconnect,
then reconnect using the "right" host controller):

	alias usb-controller  ehci-hcd
	alias usb-controller1 ohci-hcd
	alias usb-controller2 uhci-hcd
	alias usb-controller3 usb-ohci
	alias usb-controller4 usb-uhci

Note that the "uhci-hcd" would be "usb-uhci-hcd" if you're trying
to experiment with that version on the 2.5 kernel, if you're not
doing manual rmmod/modprobe (which I usually end up doing).

That causes silly boot messages for the hardware or drivers you
may not have, but works reliably/correctly for all current kernels.

- Dave