On Wed, 5 Dec 2012, Sebastian Andrzej Siewior wrote:

> On Mon, Dec 03, 2012 at 04:28:38PM -0500, Alan Stern wrote:
> > On Mon, 3 Dec 2012, Sarah Sharp wrote:
> > > This code in hcd.c:usb_add_hcd needs to change:
> > > 
> > >         /* starting here, usbcore will pay attention to this root hub */
> > >         rhdev->bus_mA = min(500u, hcd->power_budget);
> > 
> > In fact, this line is wrong.  The 500u should be multiplied by the 
> > number of ports -- and of course, it should be larger for SuperSpeed 
> > root hubs.
> 
> But I don't know the number of ports here.

Ah, right.  That's probably a large part of the reason it wasn't done 
correctly.  Or maybe I just wasn't thinking clearly when I wrote it...

>  I could set this here to 500/900
> and multiple later in hub_configure() by the number of ports.

Better yet, move that entire bus_mA calculation from usb_add_hcd to 
hub_configure.  It should be something like this:

        if (hdev == hdev->bus->root_hub) {
+               if (hcd->power_budget > 0)
+                       hdev->bus_mA = hcd->power_budget;
+               else
+                       hdev->bus_mA = full_load * hdev->maxchild;
+               if (hdev->bus_mA >= full_load)
-               if (hdev->bus_mA == 0 || hdev->bus_mA >= full_load)
                        hub->mA_per_port = full_load;
                else {
                        hub->mA_per_port = hdev->bus_mA;
                        hub->limited_power = 1;
                }

Leaving rhdev->bus_mA set to 0 for a while shouldn't hurt anything,
because our config descriptors for root hubs all have bMaxPower equal
to 0.

In case this isn't clear (which seems quite likely), hdev->bus_mA
represents the total amount of current available for use by the hub
itself plus its downstream devices.  So on a regular PC, where each
root-hub port can supply a full load, we say that the root hub uses no
power and hdev->bus_mA is a full load times the number of ports.

This all makes more sense if you think about external hubs.

> Anyway, do you think this is correct? Anything > 500 for non-SS will replaced
> with 500 in hub_configure:
> 
> |      if (hdev == hdev->bus->root_hub) {
> |                 if (hdev->bus_mA == 0 || hdev->bus_mA >= full_load)
> |                         hub->mA_per_port = full_load;
> |                 else {
> |                         hub->mA_per_port = hdev->bus_mA;
> |                         hub->limited_power = 1;
> |                 }

True, hub->mA_per_port isn't affected by the multiplication.  But look
at the calculation in hub_power_remaining -- it won't work right if 
hdev->bus_mA isn't set correctly.

> going through the tree I saw most people set it to something around 200 (if at
> all). There are a few omap/musb boards setting it to 500.

People haven't paid all that much attention to it.  Most likely it 
doesn't matter all that much except for OTG hosts.

> The interresting part is davinci of course. So there is comment that says
> |"irlml6401 switches over 1A in under 8 msec"
> and it calls davinci_setup_usb(1000, 8); which writes 255 in the power member
> which musb in turn multiplies by 2. So we have 500mA again ;) I have no idea
> how many ports they have.

Hopefully they will work correctly with these changes regardless.

Alan Stern

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

Reply via email to