[U-Boot] [U-Boot, v3, 3/8] usb: hub: Power-cycle on root-hub ports

[Vivek Gautam]

Hi,


On Tue, Apr 23, 2013 at 3:32 AM, Julius Werner <jwerner at chromium.org> wrote:
>> This 20ms delay is truely being taken to be on safer side (twice of
>> Power-on-to-power-good),
>> its not observational.
>> In my earlier patch-series, we were doing the same way as you are
>> suggesting here (power-on
>> ports only if they aren't), however Marek suggested to power-cycle the
>> ports. This would ensure
>> that we don't have any spurious Port status (telling us that port is
>> powered up).
>
> Fair enough... I guess 20ms overall is not a big deal in the whole
> picture. I sometimes tend to overoptimize things...
>
>> Actually i was seeing a strage behavior on USB 2.0 protocol ports
>> available with XHCI.
>> Since all ports with XHCI are powered-up after a Chip-reset, the
>> instant we do a power-on
>> on these ports (as with original code - simply setting the PORT_POWER
>> feature), the Connect status
>> change bit used to get cleared (however Current connect status bit was
>> still set).
>
> This is a bug in your XHCI code I hadn't spotted before: in
> xhci_submit_root(SET_FEATURE) you read the PORTSC register, add a bit,
> and write it again... without calling xhci_port_state_to_neutral()
> inbetween. Thus you clear any pending change events when you set
> PORT_POWER.

Right, we need to do that.

> (Seems the EHCI code has a similar bug in CLEAR_FEATURE,
> now that I'm looking at it... someone should put a 'reg &=
> ~EHCI_PS_CLEAR;' in there.)

True, EHCI has it for SET_FEATURE but not for CLEAR_FEATURE.

>
>> Hmm, so right now we are doing this for one port at a time.
>> I am sure parallelising things as you suggested would be best to do here, but
>> can you please explain, would handling one port at a time lead to unwanted
>> behavior from Host's side somehow ?
>
> It doesn't affect correctness, just the amount of time "wasted". Doing
> it one port at a time means you wait 100ms on a 5 port root hub, while
> you could get by with 20ms overall by parallelizing it.

True, will amend this as required.



-- 
Best Regards
Vivek