[PATCH 1/1] Add variable USB status polling delay to fix issues with USB flash drives.

[PATCH 1/1] Add variable USB status polling delay to fix issues with USB flash drives.

[Michael D. Burkey]

Due to numerous problems that we have experienced with some USB flash
drives on multiple platforms (e.g. iMX35 and iMX6) a variable startup
delay is being added prior to starting to poll the usb devices for
status information. While current versions of the Linux kernel include
multiple retries on the poll operation (with associated timeouts),
adding a proper retry operation would require passing more detailed
error information back from the USB status polling command. So, this
patch simply adds a default delay (1100ms) in barebox equal to the
maximum period in Linux once all retries and timeouts are exhausted.
The length of this delay matches one that has been added to some newer
versions of u-Boot for the iMX6 platform.

This is specifically added to address two different, similar problems:
The first is that MOSFET power distribution switches (e.g. the Micrel
MIC2076) add an additional power-on time to the outputs of a USB hub
that is greater than what is typically reported in the "Power On to
Power Good" field. The second issue is that some USB flash drives
(e.g. Corsair and Transcend) use onboard micro-controllers that have
relatively long boot times before they begin properly responding to
USB bus commands.

Since the added delay time may be neither needed nor desirable in some
cases, a global magic variable of USB_POLL_DELAY has been created. The
value of this delay is in milliseconds and can be adjusted to whatever
value is desired prior to calling the barebox "usb" command. As
mentioned before, the current default value is set to 1100ms.

Signed-off-by: Michael Burkey <mdburkey at gmail.com>
---
diff -rupN a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
--- a/drivers/usb/core/usb.c	2014-05-05 04:33:13.000000000 -0400
+++ b/drivers/usb/core/usb.c	2014-05-08 14:39:09.590616304 -0400
@@ -49,6 +49,8 @@
 #include <xfuncs.h>
 #include <init.h>
 #include <dma.h>
+#include <environment.h>
+#include <magicvar.h>

 #include <usb/usb.h>

@@ -1124,6 +1126,7 @@ static int usb_hub_configure(struct usb_
 	struct usb_hub_status *hubsts;
 	int i;
 	struct usb_hub_device *hub;
+	unsigned polling_delay;

 	hub = xzalloc(sizeof (*hub));
 	dev->hub = hub;
@@ -1235,6 +1238,24 @@ static int usb_hub_configure(struct usb_
 		(le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \
 		"" : "no ");
 	usb_hub_power_on(hub);
+
+    /* Delay a user defined period to allow devices to startup
properly.              */
+    /* This is specifically added to address two different, similar
problems:         */
+    /* The first is that MOSFET power distribution switches (e.g. the
Micrel MIC2076) */
+    /* add an additional power-on time to the outputs of a USB hub
that is greater    */
+    /* than what is typically reported in the "Power On to Power
Good" field.         */
+    /* The second issue is that some USB flash drives (e.g. Corsair
and Transcend)    */
+    /* use onboard micro-controllers that have relatively long boot
times before they */
+    /* begin properly responding to USB bus commands.
                */
+
+    /* The default value of 1100ms has been experimentally determined
to work with    */
+    /* basically every mass storage device I have been able to test.
If you are using */
+    /* devices that have shorter startup times or that are on a
permanently powered   */
+    /* connection, this time can safely be reduced (potentially to
zero).             */
+	if (!getenv_uint("USB_POLLING_DELAY", &polling_delay))
+	{
+		mdelay(polling_delay);
+	}

 	for (i = 0; i < dev->maxchild; i++) {
 		struct usb_port_status portsts;
@@ -1463,6 +1484,12 @@ struct bus_type usb_bus_type = {

 static int usb_bus_init(void)
 {
+    /* Add Global MagicVar containing device polling delay. */
+	setenv("USB_POLLING_DELAY", "1100");
+	export("USB_POLLING_DELAY");
+
 	return bus_register(&usb_bus_type);
 }
 pure_initcall(usb_bus_init);
+
+BAREBOX_MAGICVAR(USB_POLLING_DELAY, "Delay in ms after hub power on
before devices are polled.");

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/1] Add variable USB status polling delay to fix issues with USB flash drives.

[Sascha Hauer]

Hi Michael,

On Thu, May 08, 2014 at 02:41:38PM -0400, Michael D. Burkey wrote:
> Due to numerous problems that we have experienced with some USB flash
> drives on multiple platforms (e.g. iMX35 and iMX6) a variable startup
> delay is being added prior to starting to poll the usb devices for
> status information. While current versions of the Linux kernel include
> multiple retries on the poll operation (with associated timeouts),
> adding a proper retry operation would require passing more detailed
> error information back from the USB status polling command. So, this
> patch simply adds a default delay (1100ms) in barebox equal to the
> maximum period in Linux once all retries and timeouts are exhausted.
> The length of this delay matches one that has been added to some newer
> versions of u-Boot for the iMX6 platform.
> 
> This is specifically added to address two different, similar problems:
> The first is that MOSFET power distribution switches (e.g. the Micrel
> MIC2076) add an additional power-on time to the outputs of a USB hub
> that is greater than what is typically reported in the "Power On to
> Power Good" field. The second issue is that some USB flash drives
> (e.g. Corsair and Transcend) use onboard micro-controllers that have
> relatively long boot times before they begin properly responding to
> USB bus commands.
> 
> Since the added delay time may be neither needed nor desirable in some
> cases, a global magic variable of USB_POLL_DELAY has been created. The
> value of this delay is in milliseconds and can be adjusted to whatever
> value is desired prior to calling the barebox "usb" command. As
> mentioned before, the current default value is set to 1100ms.
> 
> Signed-off-by: Michael Burkey <mdburkey at gmail.com>
> ---
> diff -rupN a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> --- a/drivers/usb/core/usb.c	2014-05-05 04:33:13.000000000 -0400
> +++ b/drivers/usb/core/usb.c	2014-05-08 14:39:09.590616304 -0400
> @@ -49,6 +49,8 @@
>  #include <xfuncs.h>
>  #include <init.h>
>  #include <dma.h>
> +#include <environment.h>
> +#include <magicvar.h>
> 
>  #include <usb/usb.h>
> 
> @@ -1124,6 +1126,7 @@ static int usb_hub_configure(struct usb_
>  	struct usb_hub_status *hubsts;
>  	int i;
>  	struct usb_hub_device *hub;
> +	unsigned polling_delay;
> 
>  	hub = xzalloc(sizeof (*hub));
>  	dev->hub = hub;
> @@ -1235,6 +1238,24 @@ static int usb_hub_configure(struct usb_
>  		(le16_to_cpu(hubsts->wHubStatus) & HUB_STATUS_OVERCURRENT) ? \
>  		"" : "no ");
>  	usb_hub_power_on(hub);
> +
> +    /* Delay a user defined period to allow devices to startup
> properly.              */
> +    /* This is specifically added to address two different, similar
> problems:         */
> +    /* The first is that MOSFET power distribution switches (e.g. the
> Micrel MIC2076) */
> +    /* add an additional power-on time to the outputs of a USB hub
> that is greater    */
> +    /* than what is typically reported in the "Power On to Power
> Good" field.         */
> +    /* The second issue is that some USB flash drives (e.g. Corsair
> and Transcend)    */
> +    /* use onboard micro-controllers that have relatively long boot
> times before they */
> +    /* begin properly responding to USB bus commands.
>                 */
> +
> +    /* The default value of 1100ms has been experimentally determined
> to work with    */
> +    /* basically every mass storage device I have been able to test.
> If you are using */
> +    /* devices that have shorter startup times or that are on a
> permanently powered   */
> +    /* connection, this time can safely be reduced (potentially to
> zero).             */
> +	if (!getenv_uint("USB_POLLING_DELAY", &polling_delay))
> +	{
> +		mdelay(polling_delay);
> +	}
> 
>  	for (i = 0; i < dev->maxchild; i++) {
>  		struct usb_port_status portsts;
> @@ -1463,6 +1484,12 @@ struct bus_type usb_bus_type = {
> 
>  static int usb_bus_init(void)
>  {
> +    /* Add Global MagicVar containing device polling delay. */
> +	setenv("USB_POLLING_DELAY", "1100");
> +	export("USB_POLLING_DELAY");

I would prefer a globalvar (globalvar_add_simple_int) as this allows
some namespace to the variable, like this:

static int hub_polling_delay = 1100;

globalvar_add_simple_int("usb.hub_polling_delay", &hub_polling_delay, "%d");

Your mailer wraps the lines which makes it hard to apply this patch.
Could you have a look into it?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox