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

[Marek Vasut]

Hi guys,

> On Mon, Jul 1, 2013 at 10:11 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> > On 07/01/2013 07:49 AM, Vivek Gautam wrote:
> >> 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.
> > 
> > ...
> > 
> >> 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.
> > 
> > ...
> > 
> >> 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)
> 
> 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.
> 
> > 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
> 
> 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