Hi,
On Tue, Apr 23, 2013 at 3:32 AM, Julius Werner jwerner@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.