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

[Vivek Gautam]

Hi Marek,


On Mon, Jul 8, 2013 at 6:33 PM, Marek Vasut <marex at denx.de> wrote:
> 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.

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. ;-)



-- 
Best Regards
Vivek