usb: hub: allow pgood_delay to be specified via env

PATCH New Tim Harvey 3 participants 10 messages Apr 6, 2015 – Apr 11, 2015

Share & tools

Tim Harvey

author

2015-04-06 14:05 UTC #0 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.

I have encountered this specficically with several different sized/branded USB sticks with VID:PID 058f:6387 (Alcor Micro Corp. Transcend JetFlash) where I need to set usb_pgood_delay to 2000ms to make them detectable.

Signed-off-by Tim Harvey <[email protected]>

---
 common/usb_hub.c | 7 +++++++
 1 file changed, 7 insertions(+)

common/usb_hub.c +7 -0

index 66b4a72..a0ef058 100644
@@ -86,6 +86,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
 	int i;
 	struct usb_device *dev;
 	unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
+	const char *env;
 
 	dev = hub->pusb_dev;

@@ -98,7 +99,13 @@ 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
+	 * but allow this time to be increased via env variable as some
+	 * devices break the spec and require longer warm-up times
 	 */
+	env = getenv("usb_pgood_delay");
+	if (env)
+		pgood_delay = max(pgood_delay, simple_strtol(env, NULL, 0));
+	debug("pgood_delay=%dms\n", pgood_delay);
 	mdelay(pgood_delay + 1000);
 }

Signature · 1 line

1.9.1

Marek Vasut

2015-04-06 14:58 UTC #1 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 8 lines · Tim Harvey

Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
I have encountered this specficically with several different sized/branded USB sticks with VID:PID 058f:6387 (Alcor Micro Corp. Transcend JetFlash) where I need to set usb_pgood_delay to 2000ms to make them detectable.
Signed-off-by Tim Harvey <[email protected]>

Hi!

this looks like a good idea. I have a hint for improvement -- in case you get a timeout waiting for the device to be detected, maybe the error message should mention that you can try setting this env variable and try again ?

Best regards,
Marek Vasut

Tim Harvey

author

2015-04-07 14:52 UTC #2 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 10 lines · Marek Vasut

Quoted 8 lines · Tim Harvey
Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
I have encountered this specficically with several different sized/branded USB sticks with VID:PID 058f:6387 (Alcor Micro Corp. Transcend JetFlash) where I need to set usb_pgood_delay to 2000ms to make them detectable.
Signed-off-by Tim Harvey <[email protected]>

Quoted 8 lines

Hi!
this looks like a good idea. I have a hint for improvement -- in case you get a timeout waiting for the device to be detected, maybe the error message should mention that you can try setting this env variable and try again ?
Best regards,
Marek Vasut

Marek,

I'm not sure what 'failed detect' would be appropriate though. In my case I was dealing with storage devices so I think what you are proposing is that on a 'usb start' if 0 storage devices are found display a message such as:

No storage devices found - if a connected device is not being detected try setting usb_pgood_delay env var to something above the 1000ms time the USB spec allows

Sounds a bit verbose and I'm not clear if it should be limited to USB storage devices or other supported devices (I'm betting its just the storage ones that often break the spec).

Stephen, what do you think? You and I discussed adding such an env var about a year ago.

Tim

Stephen Warren

2015-04-07 15:34 UTC #3 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 22 lines · Tim Harvey

Quoted 10 lines · Marek Vasut
Quoted 8 lines · Tim Harvey
Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
I have encountered this specficically with several different sized/branded USB sticks with VID:PID 058f:6387 (Alcor Micro Corp. Transcend JetFlash) where I need to set usb_pgood_delay to 2000ms to make them detectable.
Signed-off-by Tim Harvey <[email protected]>
Quoted 8 lines
Hi!
this looks like a good idea. I have a hint for improvement -- in case you get a timeout waiting for the device to be detected, maybe the error message should mention that you can try setting this env variable and try again ?
Best regards,
Marek Vasut
Marek,

Quoted 8 lines

I'm not sure what 'failed detect' would be appropriate though. In my case I was dealing with storage devices so I think what you are proposing is that on a 'usb start' if 0 storage devices are found display a message such as:
No storage devices found - if a connected device is not being detected try setting usb_pgood_delay env var to something above the 1000ms time the USB spec allows

That seems like something better for the documentation that at run-time. There's no reason to believe that the user has any USB storage devices plugged in.

Quoted 6 lines

Sounds a bit verbose and I'm not clear if it should be limited to USB storage devices or other supported devices (I'm betting its just the storage ones that often break the spec).
Stephen, what do you think? You and I discussed adding such an env var about a year ago.

If there is some kind of error that's actively detected, it may make sense to print some kind of message. However, if the symptoms are that devices simply don't appear on the bus and there's no reason to believe any are actually plugged in, that fells different.

Tim Harvey

author

2015-04-07 16:04 UTC #4 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 48 lines · Stephen Warren

Quoted 29 lines · Tim Harvey
Quoted 16 lines · Marek Vasut
Quoted 10 lines · Tim Harvey
Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
I have encountered this specficically with several different sized/branded
USB sticks with VID:PID 058f:6387 (Alcor Micro Corp. Transcend JetFlash) where I need to set usb_pgood_delay to 2000ms to make them detectable.
Signed-off-by Tim Harvey <[email protected]>
Hi!
Quoted 7 lines
this looks like a good idea. I have a hint for improvement -- in case you get a timeout waiting for the device to be detected, maybe the error message
should mention that you can try setting this env variable and try again ?
Best regards,
Marek Vasut
Marek,
Quoted 8 lines
I'm not sure what 'failed detect' would be appropriate though. In my case I was dealing with storage devices so I think what you are proposing is that on a 'usb start' if 0 storage devices are found display a message such as:
No storage devices found - if a connected device is not being detected try setting usb_pgood_delay env var to something above the 1000ms time the USB spec allows
That seems like something better for the documentation that at run-time. There's no reason to believe that the user has any USB storage devices plugged in.
Quoted 4 lines
Sounds a bit verbose and I'm not clear if it should be limited to USB storage devices or other supported devices (I'm betting its just the storage ones that often break the spec).

Quoted 8 lines

Quoted 2 lines
Stephen, what do you think? You and I discussed adding such an env var about a year ago.
If there is some kind of error that's actively detected, it may make sense to print some kind of message. However, if the symptoms are that devices simply don't appear on the bus and there's no reason to believe any are actually plugged in, that fells different.

No error... just failure to detect storage device. I agree such a message for people using 'usb start' for some other usb device is not cool.

Let's leave it as-is then if there are no complaints.

Tim

Marek Vasut

2015-04-07 18:55 UTC #5 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

On Tuesday, April 07, 2015 at 06:04:36 PM, Tim Harvey wrote:
[...]

Quoted 10 lines

Quoted 4 lines
If there is some kind of error that's actively detected, it may make sense to print some kind of message. However, if the symptoms are that devices simply don't appear on the bus and there's no reason to believe any are actually plugged in, that fells different.
No error... just failure to detect storage device. I agree such a message for people using 'usb start' for some other usb device is not cool.
Let's leave it as-is then if there are no complaints.

Hi,

Is the device recognised on the bus and just not identified OR is the device not even recognised on the bus ?

Best regards,
Marek Vasut

Tim Harvey

author

2015-04-08 16:50 UTC #6 lore raw

PATCH 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 14 lines · Marek Vasut

On Tuesday, April 07, 2015 at 06:04:36 PM, Tim Harvey wrote:
[...]
Quoted 10 lines
Quoted 4 lines
If there is some kind of error that's actively detected, it may make sense to print some kind of message. However, if the symptoms are that devices simply don't appear on the bus and there's no reason to believe any are actually plugged in, that fells different.
No error... just failure to detect storage device. I agree such a message for people using 'usb start' for some other usb device is not cool.
Let's leave it as-is then if there are no complaints.

Quoted 7 lines

Hi,
Is the device recognised on the bus and just not identified OR is the device not even recognised on the bus ?
Best regards,
Marek Vasut

Marek,

No timeout - just failure to see Port status change (portstatus/portchange reflect no change on port).

I did notice there is a compiler warning with my patch that I missed so I'll re-post it with that resolved if everything else looks ok.

Tim

Tim Harvey

author

2015-04-08 19:21 UTC #7 lore raw

PATCH v2 1/1 usb: hub: allow pgood_delay to be specified via env

Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.

Signed-off-by Tim Harvey <[email protected]>

---
v2:
- added cast to fix compiler warning

---
 common/usb_hub.c | 8 ++++++++
 1 file changed, 8 insertions(+)

common/usb_hub.c +8 -0

index 66b4a72..f54a404 100644
@@ -86,6 +86,7 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
 	int i;
 	struct usb_device *dev;
 	unsigned pgood_delay = hub->desc.bPwrOn2PwrGood * 2;
+	const char *env;
 
 	dev = hub->pusb_dev;

@@ -98,7 +99,14 @@ 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
+	 * but allow this time to be increased via env variable as some
+	 * devices break the spec and require longer warm-up times
 	 */
+	env = getenv("usb_pgood_delay");
+	if (env)
+		pgood_delay = max(pgood_delay,
+			          (unsigned)simple_strtol(env, NULL, 0));
+	debug("pgood_delay=%dms\n", pgood_delay);
 	mdelay(pgood_delay + 1000);
 }

Signature · 1 line

1.9.1

Marek Vasut

2015-04-11 16:59 UTC #8 lore raw

PATCH v2 1/1 usb: hub: allow pgood_delay to be specified via env

Quoted 4 lines · Tim Harvey

Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
Signed-off-by Tim Harvey <[email protected]>

It'd be nice if this variable was documented somewhere. Can you please do that ? Subsequent patch would be fine, though I'm not sure where exactly to place such documentation. Any ideas ?

Best regards,
Marek Vasut

Marek Vasut

2015-04-11 17:08 UTC #9 lore raw

PATCH v2 1/1 usb: hub: allow pgood_delay to be specified via env

In reply to Tim Harvey

Quoted 4 lines · Tim Harvey

Some USB devices break the spec and require longer warm-up times. Allow the usb_pgood_delay env variable to override the calculated time.
Signed-off-by Tim Harvey <[email protected]>

Applied to u-boot-usb/next , thanks!

Best regards,
Marek Vasut