[U-Boot] [PATCH 2/2] usb: hub: remove CONFIG_USB_HUB_MIN_POWER_ON_DELAY

[Tim Harvey]

On Mon, May 19, 2014 at 1:21 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
> From: Stephen Warren <swarren at nvidia.com>
>
> Now that we wait the correct specification-mandated time at the end of
> usb_hub_power_on(), I suspect that CONFIG_USB_HUB_MIN_POWER_ON_DELAY has
> no purpose.
>
> For cm_t35.h, we already wait longer than the original MIN_POWER_ON_DELAY,
> so this change is safe.
>
> For gw_ventana.h, we will wait as long as the original MIN_POWER_ON_DELAY
> iff pgood_delay was at least 200ms. I'm not sure if this is the case or
> not, hence I've CC'd relevant people to test this change.
>
> Cc: Igor Grinberg <grinberg at compulab.co.il>
> Cc: Tim Harvey <tharvey at gateworks.com>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
>  README                       | 3 ---
>  common/usb_hub.c             | 6 +-----
>  include/configs/cm_t35.h     | 2 --
>  include/configs/gw_ventana.h | 1 -
>  4 files changed, 1 insertion(+), 11 deletions(-)
>
> diff --git a/README b/README
> index d8fcd95f9423..091897227c52 100644
> --- a/README
> +++ b/README
> @@ -1432,9 +1432,6 @@ The following options need to be configured:
>                 CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the
>                 txfilltuning field in the EHCI controller on reset.
>
> -               CONFIG_USB_HUB_MIN_POWER_ON_DELAY defines the minimum
> -               interval for usb hub power-on delay.(minimum 100msec)
> -
>  - USB Device:
>                 Define the below if you wish to use the USB console.
>                 Once firmware is rebuilt from a serial console issue the
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index 4875a51aceaf..2add4b97920f 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -35,10 +35,6 @@
>  #include <asm/4xx_pci.h>
>  #endif
>
> -#ifndef CONFIG_USB_HUB_MIN_POWER_ON_DELAY
> -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY      1000
> -#endif
> -
>  #define USB_BUFSIZ     512
>
>  static struct usb_hub_device hub_dev[USB_MAX_HUB];
> @@ -142,7 +138,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
>          * Wait for power to become stable,
>          * plus spec-defined max time for device to connect
>          */
> -       mdelay(pgood_delay + CONFIG_USB_HUB_MIN_POWER_ON_DELAY);
> +       mdelay(pgood_delay + 1000);
>  }
>
>  void usb_hub_reset(void)
> diff --git a/include/configs/cm_t35.h b/include/configs/cm_t35.h
> index aae05e033303..f6acf6920524 100644
> --- a/include/configs/cm_t35.h
> +++ b/include/configs/cm_t35.h
> @@ -104,8 +104,6 @@
>  #define CONFIG_USB_DEVICE
>  #define CONFIG_USB_TTY
>  #define CONFIG_SYS_CONSOLE_IS_IN_ENV
> -/* This delay is really for slow-to-power-on USB sticks, not the hub */
> -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 500
>
>  /* commands to include */
>  #include <config_cmd_default.h>
> diff --git a/include/configs/gw_ventana.h b/include/configs/gw_ventana.h
> index 33983907f228..188871630bd3 100644
> --- a/include/configs/gw_ventana.h
> +++ b/include/configs/gw_ventana.h
> @@ -188,7 +188,6 @@
>  #define CONFIG_USB_ETH_CDC
>  #define CONFIG_NETCONSOLE
>  #define CONFIG_SYS_USB_EVENT_POLL_VIA_CONTROL_EP
> -#define CONFIG_USB_HUB_MIN_POWER_ON_DELAY 1200
>
>  /* serial console (ttymxc1,115200) */
>  #define CONFIG_CONS_INDEX              1
> --
> 1.8.1.5
>

Stephen,

Sorry for the late reply - I'm just getting around to testing this
(and I realize that Marek already committed it which is ok by me).

I have a variety of USB Mass Storage devices that I tested when I was
looking at this and out of about 12 devices I found I had 1 'usb
stick' that requires 2100ms in order to respond and be successfully
scanned: 048d:1327 Integrated Technology Express, Inc 32GB USB stick.
I also found that rotational media (ie Seagate and Western Digital USB
drives) would not respond in 1000ms either which didn't surprise me as
I figured they needed some extra spin-up time. For all other devices I
had I found that 1000ms was adequate.

So do these devices I mention simply violate the USB spec?

I wonder if the delay should be able to be overridden with an env var
or an argument to 'usb start' to account for devices like this?

Regards,

Tim