Regression due to 020bbcb "usb: hub: Power-cycle on root-hub ports"

U-Boot Stephen Warren 3 participants 15 messages Jun 13, 2013 – Jul 9, 2013 1 message not in the archive

lore

Share & tools

lore

Stephen Warren

author

2013-06-13 22:03 UTC #1 lore raw

In reply to a message not in the archive

(Sorry to those on to/cc; I'm resending this so it goes to the correct mailing list)

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!

Marek Vasut

2013-06-30 16:38 UTC #2 lore raw

Dear Stephen Warren,

Quoted 16 lines

(Sorry to those on to/cc; I'm resending this so it goes to the correct mailing list)
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.

Quoted 7 lines

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.

Best regards,
Marek Vasut

Vivek Gautam

2013-07-01 13:49 UTC #3 lore raw

Hi Marek,

Quoted 4 lines · Marek Vasut

Dear Stephen Warren,
Quoted 2 lines
(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.

Quoted 17 lines

Quoted 17 lines
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.)

Quoted 6 lines

Quoted 4 lines
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.

common/usb_hub.c +2 -4

index 774ba63..437a51f 100644
@@ -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.

Signature · 2 lines

Best Regards
Vivek

Stephen Warren

author

2013-07-01 16:41 UTC #4 lore raw

Quoted 35 lines · Vivek Gautam

Hi Marek,
Quoted 4 lines · Marek Vasut
Dear Stephen Warren,
Quoted 2 lines
(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.
Quoted 23 lines
Quoted 14 lines
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.
Quoted 7 lines
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.

...

Quoted 6 lines

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.

...

Quoted 2 lines

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

I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf. Below is the log:

U-Boot 2013.04-00281-g0e8ef51 (Jul 01 2013 - 10:33:36)

TEGRA20

Board: NVIDIA Seaboard
DRAM:  1 GiB
NAND:  512 MiB
MMC:   Tegra SD/MMC: 0, Tegra SD/MMC: 1
In:    tegra-kbc
Out:   lcd
Err:   lcd
Net:   Net Initialization Skipped

No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 2 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices...
Warning: asx0 using MAC address from net device
1 Ethernet Device(s) found
Hit any key to stop autoboot: 0

Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found

(there's a much longer pause when scanning this bus every time except the very first)

USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found

Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found

Vivek Gautam

2013-07-02 17:01 UTC #5 lore raw

Quoted 83 lines · Stephen Warren

Quoted 35 lines · Vivek Gautam
Hi Marek,
Quoted 4 lines · Marek Vasut
Dear Stephen Warren,
Quoted 2 lines
(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.
Quoted 23 lines
Quoted 14 lines
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.
Quoted 7 lines
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.
...
Quoted 6 lines
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.
...
Quoted 2 lines
can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports.
I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf. Below is the log:
U-Boot 2013.04-00281-g0e8ef51 (Jul 01 2013 - 10:33:36)
TEGRA20
Board: NVIDIA Seaboard
DRAM:  1 GiB
NAND:  512 MiB
MMC:   Tegra SD/MMC: 0, Tegra SD/MMC: 1
In:    tegra-kbc
Out:   lcd
Err:   lcd
Net:   Net Initialization Skipped
No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 2 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices...
Warning: asx0 using MAC address from net device
1 Ethernet Device(s) found
Hit any key to stop autoboot: 0
Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found

Quoted 2 lines

(there's a much longer pause when scanning this bus every time except the very first)

This long pause could be from the 10sec delay present in common/usb_hub.c: usb_hub_configure(): line 475
(the do-while loop present to check Current Connect Status and Connect Status Change bits)

I could actually see somewhat similar issue of long pause on xHCI port, if we didn't apply patches:
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports

This was because, once usb_hub_power_on() was called, Connect Status Change bit of xHC port was
getting cleared though Current Connect Status was still asserted, even untill that no code handles that bit.

For xHC, setting the Port_power bit, in case that bit was initially asserted clears CSC bit.

Quoted 6 lines

USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found

Quoted 9 lines

Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found

I think we should be checking EHCI registers now, PORTSC register to be specific to see how the
port power is getting affected.
On smdk5250 i am unable see this behavior, which is having only one controller unlike
seaboard which i can see has 3 controllers.

Signature · 2 lines

Best Regards
Vivek

Marek Vasut

2013-07-08 13:03 UTC #6 lore raw

Hi guys,

Quoted 126 lines · Stephen Warren

Quoted 88 lines · Vivek Gautam
Quoted 34 lines
Hi Marek,
Quoted 4 lines · Marek Vasut
Dear Stephen Warren,
Quoted 2 lines
(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.
Quoted 23 lines
Quoted 14 lines
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.
Quoted 7 lines
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.
...
Quoted 6 lines
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.
...
Quoted 2 lines
can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports.
I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf. Below is the log:
U-Boot 2013.04-00281-g0e8ef51 (Jul 01 2013 - 10:33:36)
TEGRA20
Board: NVIDIA Seaboard
DRAM:  1 GiB
NAND:  512 MiB
MMC:   Tegra SD/MMC: 0, Tegra SD/MMC: 1
In:    tegra-kbc
Out:   lcd
Err:   lcd
Net:   Net Initialization Skipped
No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 2 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices...
Warning: asx0 using MAC address from net device
1 Ethernet Device(s) found
Hit any key to stop autoboot: 0
Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
Quoted 2 lines
(there's a much longer pause when scanning this bus every time except the very first)
This long pause could be from the 10sec delay present in common/usb_hub.c: usb_hub_configure(): line 475
(the do-while loop present to check Current Connect Status and Connect Status Change bits)
I could actually see somewhat similar issue of long pause on xHCI port, if we didn't apply patches:
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
This was because, once usb_hub_power_on() was called, Connect Status Change bit of xHC port was
getting cleared though Current Connect Status was still asserted, even untill that no code handles that bit.
For xHC, setting the Port_power bit, in case that bit was initially asserted clears CSC bit.
Quoted 13 lines
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found
Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
USB2: lowlevel init failed
Quoted 2 lines
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found

Quoted 6 lines

I think we should be checking EHCI registers now, PORTSC register to be specific to see how the
port power is getting affected.
On smdk5250 i am unable see this behavior, which is having only one controller unlike
seaboard which i can see has 3 controllers.

Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.

Best regards,
Marek Vasut

Vivek Gautam

2013-07-08 13:25 UTC #7 lore raw

Hi Marek,

Quoted 135 lines · Marek Vasut

Hi guys,
Quoted 126 lines · Stephen Warren
Quoted 88 lines · Vivek Gautam
Quoted 34 lines
Hi Marek,
Quoted 4 lines · Marek Vasut
Dear Stephen Warren,
Quoted 2 lines
(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.
Quoted 23 lines
Quoted 14 lines
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.
Quoted 7 lines
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.
...
Quoted 6 lines
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.
...
Quoted 2 lines
can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports.
I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf. Below is the log:
U-Boot 2013.04-00281-g0e8ef51 (Jul 01 2013 - 10:33:36)
TEGRA20
Board: NVIDIA Seaboard
DRAM:  1 GiB
NAND:  512 MiB
MMC:   Tegra SD/MMC: 0, Tegra SD/MMC: 1
In:    tegra-kbc
Out:   lcd
Err:   lcd
Net:   Net Initialization Skipped
No ethernet found.
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 2 USB Device(s) found
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices...
Warning: asx0 using MAC address from net device
1 Ethernet Device(s) found
Hit any key to stop autoboot: 0
Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
Quoted 2 lines
(there's a much longer pause when scanning this bus every time except the very first)
This long pause could be from the 10sec delay present in common/usb_hub.c: usb_hub_configure(): line 475
(the do-while loop present to check Current Connect Status and Connect Status Change bits)
I could actually see somewhat similar issue of long pause on xHCI port, if we didn't apply patches:
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
This was because, once usb_hub_power_on() was called, Connect Status Change bit of xHC port was
getting cleared though Current Connect Status was still asserted, even untill that no code handles that bit.
For xHC, setting the Port_power bit, in case that bit was initially asserted clears CSC bit.
Quoted 13 lines
USB2: lowlevel init failed
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found
Tegra20 (SeaBoard) # usb start
(Re)start USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 2 USB Device(s) found
USB1: USB EHCI 1.00
scanning bus 1 for devices... 1 USB Device(s) found
USB2: lowlevel init failed
Quoted 2 lines
scanning usb for storage devices... 0 Storage Device(s) found scanning usb for ethernet devices... 0 Ethernet Device(s) found
Quoted 6 lines
I think we should be checking EHCI registers now, PORTSC register to be specific to see how the
port power is getting affected.
On smdk5250 i am unable see this behavior, which is having only one controller unlike
seaboard which i can see has 3 controllers.

Quoted 2 lines

Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.

0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports

Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.

I will have to figure out some other way to handle xHC ports. ;-)

Signature · 2 lines

Best Regards
Vivek

Stephen Warren

author

2013-07-08 15:58 UTC #8 lore raw

Quoted 16 lines · Vivek Gautam

Quoted 15 lines · Marek Vasut
Quoted 15 lines · Stephen Warren
Quoted 14 lines · Vivek Gautam
Quoted 13 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
(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.
Quoted 6 lines
Quoted 6 lines
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.

...

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
BUMP? Vivek, any ideas? Otherwise I'm reverting this.

...

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
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.

...

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports.
I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf.

...

Quoted 5 lines

Quoted 4 lines
Quoted 1 line
seaboard which i can see has 3 controllers.
Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.

Quoted 7 lines

0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.

Yes, I have been reverting those two commits locally for a while, and it solves the problem for me.

Marek Vasut

2013-07-08 17:03 UTC #9 lore raw

Dear Stephen Warren,

Quoted 3 lines · Vivek Gautam

Quoted 2 lines · Marek Vasut
Quoted 1 line
Quoted 1 line
On Mon, Jul 1, 2013 at 10:11 PM, Stephen Warren <[email protected]>

Quoted 46 lines · wrote:

Quoted 14 lines
Quoted 14 lines
Quoted 14 lines
Quoted 14 lines · Vivek Gautam
Quoted 13 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
(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.
Quoted 6 lines
Quoted 6 lines
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.
...
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
BUMP? Vivek, any ideas? Otherwise I'm reverting this.
...
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
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.
...
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 2 lines
can you please confirm if you problem is related to this BUG in the sequence of power-cycling the ports.
I applied that change, and it does not solve the problem on Tegra, nor do I see any of the messages that were changed from debug to printf.
...
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
seaboard which i can see has 3 controllers.
Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports

Quoted 7 lines

Quoted 4 lines
Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.
Yes, I have been reverting those two commits locally for a while, and it solves the problem for me.

Reverted, please test u-boot-usb/master .

Best regards,
Marek Vasut

Stephen Warren

author

2013-07-08 18:03 UTC #10 lore raw

Quoted 16 lines · Marek Vasut

Dear Stephen Warren,
Quoted 3 lines · Vivek Gautam
Quoted 2 lines · Marek Vasut
Quoted 1 line
Quoted 1 line
On Mon, Jul 1, 2013 at 10:11 PM, Stephen Warren <[email protected]>
Quoted 10 lines · wrote:
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines · Vivek Gautam
Quoted 9 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
(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.
Quoted 2 lines
Quoted 2 lines
Commit 020bbcb "usb: hub: Power-cycle on root-hub ports" causes a regression on Tegra systems.

...

Quoted 11 lines

Quoted 11 lines
Quoted 10 lines
Quoted 2 lines
Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.

Quoted 4 lines

Quoted 2 lines
Yes, I have been reverting those two commits locally for a while, and it solves the problem for me.
Reverted, please test u-boot-usb/master .

Well, it works, but it turns out the reverts aren't needed. Simon Glass already found the problem, and fixed it with:

ed10e66 usb: Correct CLEAR_FEATURE code in ehci-hcd

Sorry for not noticing this earlier, but since there hadn't been any news in this thread, and there weren't any relevant changes to the power-cycling code affected by the problematic patches, it didn't occur to me that the problem may have already been fixed elsewhere, so I didn't ever retest the issue with a newer commit than that one where I originally found the problem:-(

Marek Vasut

2013-07-08 18:25 UTC #11 lore raw

Dear Stephen Warren,

Quoted 41 lines · Marek Vasut

Quoted 18 lines
Dear Stephen Warren,
Quoted 4 lines · Vivek Gautam
Quoted 3 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
On Mon, Jul 1, 2013 at 10:11 PM, Stephen Warren
<[email protected]>
Quoted 10 lines · wrote:
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines · Vivek Gautam
Quoted 9 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
(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.
Quoted 2 lines
Quoted 2 lines
Commit 020bbcb "usb: hub: Power-cycle on root-hub ports" causes a regression on Tegra systems.
...
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 2 lines
Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.
Quoted 4 lines
Quoted 2 lines
Yes, I have been reverting those two commits locally for a while, and it solves the problem for me.
Reverted, please test u-boot-usb/master .
Well, it works, but it turns out the reverts aren't needed. Simon Glass already found the problem, and fixed it with:

Quoted 8 lines

ed10e66 usb: Correct CLEAR_FEATURE code in ehci-hcd
Sorry for not noticing this earlier, but since there hadn't been any news in this thread, and there weren't any relevant changes to the power-cycling code affected by the problematic patches, it didn't occur to me that the problem may have already been fixed elsewhere, so I didn't ever retest the issue with a newer commit than that one where I originally found the problem:-(

OK, I dropped the reverts, retest again please.

Best regards,
Marek Vasut

Stephen Warren

author

2013-07-08 18:28 UTC #12 lore raw

Quoted 45 lines · Marek Vasut

Dear Stephen Warren,
Quoted 43 lines · Marek Vasut
Quoted 18 lines
Dear Stephen Warren,
Quoted 4 lines · Vivek Gautam
Quoted 3 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
On Mon, Jul 1, 2013 at 10:11 PM, Stephen Warren
<[email protected]>
Quoted 10 lines · wrote:
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines · Vivek Gautam
Quoted 9 lines · Marek Vasut
Quoted 2 lines
Quoted 2 lines
(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.
Quoted 2 lines
Quoted 2 lines
Commit 020bbcb "usb: hub: Power-cycle on root-hub ports" causes a regression on Tegra systems.
...
Quoted 11 lines
Quoted 11 lines
Quoted 10 lines
Quoted 2 lines
Vivek, what do I have to revert to fix this flub? I will do that now, since this discussion is stalled.
0bf796f usb: hub: Parallelize power-cycling of root-hub ports 020bbcb usb: hub: Power-cycle on root-hub ports
Above two patches are the one which changed the hub_power_on() functionality. If Stephen can confirm that reverting these patches really solves the problem on Tegra,
we can revert them.
Quoted 4 lines
Quoted 2 lines
Yes, I have been reverting those two commits locally for a while, and it solves the problem for me.
Reverted, please test u-boot-usb/master .
Well, it works, but it turns out the reverts aren't needed. Simon Glass already found the problem, and fixed it with:
ed10e66 usb: Correct CLEAR_FEATURE code in ehci-hcd

Quoted 8 lines

Quoted 6 lines
Sorry for not noticing this earlier, but since there hadn't been any news in this thread, and there weren't any relevant changes to the power-cycling code affected by the problematic patches, it didn't occur to me that the problem may have already been fixed elsewhere, so I didn't ever retest the issue with a newer commit than that one where I originally found the problem:-(
OK, I dropped the reverts, retest again please.

I had already tested the commit in your tree right before the reverts (a36466c50b1b3614c3cfdae194227f7dd8e2c592); that's how I noticed that the reverts weren't necessary, since I'd expected that commit to fail but it didn't.

Marek Vasut

2013-07-08 19:50 UTC #13 lore raw

Dear Stephen Warren,

[...]

Quoted 4 lines

I had already tested the commit in your tree right before the reverts (a36466c50b1b3614c3cfdae194227f7dd8e2c592); that's how I noticed that the reverts weren't necessary, since I'd expected that commit to fail but it didn't.

So we are now all good? Ready for release, no problems anywhere?

Best regards,
Marek Vasut

Stephen Warren

author

2013-07-08 19:53 UTC #14 lore raw

Quoted 10 lines · Marek Vasut

Dear Stephen Warren,
[...]
Quoted 4 lines
I had already tested the commit in your tree right before the reverts (a36466c50b1b3614c3cfdae194227f7dd8e2c592); that's how I noticed that the reverts weren't necessary, since I'd expected that commit to fail but it didn't.
So we are now all good? Ready for release, no problems anywhere?

As far as I know, yes.

(Although I haven't tested u-boot/master recently, but anyway)

Vivek Gautam

2013-07-09 07:46 UTC #15 lore raw

Quoted 9 lines · Stephen Warren

Quoted 8 lines · Marek Vasut
Dear Stephen Warren,
[...]
Quoted 4 lines
I had already tested the commit in your tree right before the reverts (a36466c50b1b3614c3cfdae194227f7dd8e2c592); that's how I noticed that the reverts weren't necessary, since I'd expected that commit to fail but it didn't.

Good to hear that things are working now. :-)
Sorry, i missed to notice that patch from Simon, i should have caught that earlier on u-boot-usb/master.

Quoted 6 lines

Quoted 2 lines
So we are now all good? Ready for release, no problems anywhere?
As far as I know, yes.
(Although I haven't tested u-boot/master recently, but anyway)

Signature · 2 lines

Best Regards
Vivek