HI Julius,
On Sat, Apr 20, 2013 at 12:30 AM, Julius Werner <jwer...@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...@samsung.com> >> Signed-off-by: Vivek Gautam <gautam.vi...@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); >> } -- Best Regards Vivek _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot