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

[Julius Werner]

> 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 at samsung.com>
> Signed-off-by: Vivek Gautam <gautam.vivek at 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?

If you insist, please at least parallelize this. Disable power on all ports,
wait, then check and reenable it.

> +
> +		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().

> +			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.

> +			return;
> +		}
> +
>  		usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
>  		debug("port %d returns %lX\n", i + 1, dev->status);
>  	}