[U-Boot] Regression due to 020bbcb "usb: hub: Power-cycle on root-hub ports"

[Vivek Gautam]

Hi Marek,


On Sun, Jun 30, 2013 at 10:08 PM, Marek Vasut <marex at denx.de> wrote:
> Dear Stephen Warren,
>
>> (Sorry to those on to/cc; I'm resending this so it goes to the correct
>> mailing list)

Dear Stephen,
sorry for the delay in responding to this.

>>
>> Commit 020bbcb "usb: hub: Power-cycle on root-hub ports" causes a
>> regression on Tegra systems.
>>
>> The first time "usb start" is executed, it appears to work correctly.
>> However, any subsequent time it is executed, it takes a long time, and
>> eventually fails to find any USB devices.
>>
>> This situation can happen quite often; for example, if the user forgets
>> to plug in a USB device before booting, runs "usb start", realizes that,
>> then plugs it in and runs "usb start" again. This is compounded on at
>> least one of the Tegra boards, since CONFIG_PREBOOT is set to "usb
>> start" on systems (laptops/clamshells) which have built-in USB keyboards.
>>
>> If I simply revert this patch, then everything works again. (Yes,
>> reverting requires fixing a small merge conflict.)
>>
>> Do you have any idea what the problem can be? I'm tempted to simply ask
>> for the patch to be reverted since it causes a regression.
>>
>> Thanks for any idea how to fix this!
>
> BUMP? Vivek, any ideas? Otherwise I'm reverting this.

Tried this at my end on smdk5250 board, and since we have 3 ports on
EHCI (which we haven't defined in 5250's config --
CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS);
the issue was very much reproducible.

For this following commits should be under scan:
0bf796f usb: hub: Parallelize power-cycling of root-hub ports
020bbcb usb: hub: Power-cycle on root-hub ports

There's one BUG that i could see in " 0bf796f " commit.
Now that we parallelized the sequence to power cycle ports, so if
get_port_status for any port failed,
it returns from hub_power_on() and not power-on any of the port.

Below is the change i suggest.

diff --git a/common/usb_hub.c b/common/usb_hub.c
index 774ba63..437a51f 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -127,7 +127,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
        for (i = 0; i < dev->maxchild; i++) {
                ret = usb_get_port_status(dev, i + 1, portsts);
                if (ret < 0) {
-                       debug("port %d: get_port_status failed\n", i + 1);
+                       printf("port %d: get_port_status failed\n", i + 1);
                        return;
                }

@@ -142,12 +142,10 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
                 */
                portstatus = le16_to_cpu(portsts->wPortStatus);
                if (portstatus & (USB_PORT_STAT_POWER << 1)) {
-                       debug("port %d: Port power change failed\n", i + 1);
+                       printf("port %d: Port power change failed\n", i + 1);
                        return;
                }
-       }

-       for (i = 0; i < dev->maxchild; i++) {
                usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
                debug("port %d returns %lX\n", i + 1, dev->status);
        }


Dear Stephen,

can you please confirm if you problem is related to this BUG in the
sequence of power-cycling the ports.

With this change i can see that USB 2.0 does not detect attached
device for the first time we give 'usb start',
but subsequently every time it comes and detects the device.
But similar behavior is observed when i revert both of above mentioned
commits, so this i should look into.



-- 
Best Regards
Vivek