HI Julius,
On Sat, Apr 20, 2013 at 12:30 AM, Julius Werner jwerner@chromium.org wrote:
XHCI ports are powered on after a H/W reset, however EHCI ports are not. So disabling and re-enabling power on all ports invariably.
Signed-off-by: Amar amarendra.xt@samsung.com Signed-off-by: Vivek Gautam gautam.vivek@samsung.com
Changes from v2:
- Replaced USB_HUB_PRINTFs to debug()
common/usb_hub.c | 34 ++++++++++++++++++++++++++++++++++ 1 files changed, 34 insertions(+), 0 deletions(-)
diff --git a/common/usb_hub.c b/common/usb_hub.c index f2a0285..e4f4e3c 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -100,11 +100,45 @@ static void usb_hub_power_on(struct usb_hub_device *hub) int i; struct usb_device *dev; unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);unsigned short portstatus;int ret; dev = hub->pusb_dev; /* Enable power to the ports */ debug("enabling power on all ports\n"); for (i = 0; i < dev->maxchild; i++) {/** Power-cycle the ports here: aka,* turning them off and turning on again.*/usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);debug("port %d returns %lX\n", i + 1, dev->status);/* Wait at least 2*bPwrOn2PwrGood for PP to change */mdelay(pgood_delay);This adds 20ms per port to USB boot times... is it really necessary? Why do we need to powercycle XHCI ports? Couldn't we just check if the ports are powered and only power them on if they aren't?
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).
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).
So we arrived at solution that either we don't power-up XHCI ports or do a power-cycle on them.
If you insist, please at least parallelize this. Disable power on all ports, wait, then check and reenable it.
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 ?
ret = usb_get_port_status(dev, i + 1, portsts);if (ret < 0) {debug("port %d: get_port_status failed\n", i + 1);This is a severe error (because the ports won't come up again), so I think it should be a printf() instead of a debug().
Sure, will change this to pritnf()
return;}/** Check to confirm the state of Port Power:* xHCI says "After modifying PP, s/w shall read* PP and confirm that it has reached the desired state* before modifying it again, undefined behavior may occur* if this procedure is not followed".* EHCI doesn't say anything like this, but no harm in keeping* this.*/portstatus = le16_to_cpu(portsts->wPortStatus);if (portstatus & (USB_PORT_STAT_POWER << 1)) {debug("port %d: Port power change failed\n", i + 1);Same as above.
Sure, will change this.
return;}usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER); debug("port %d returns %lX\n", i + 1, dev->status); }