Hi Hans,

On 10.03.2016 20:12, Hans de Goede wrote:
On 10-03-16 16:50, Stefan Roese wrote:
In a system with a complex USB infrastrcture (many USB hubs), the
power-on delay of mininimum 1 second for each USB hub results in a quite
big USB scanning time. Many USB devices can deal with much lower
power-on delays. In my test system, even 10ms seems to be enough
for the USB device to get detected correctly. This patch now
reduces the minimum 1 second delay to a minumum of 100ms instead.
This results in a big USB scan time reduction:

Here the numbers for my current board:

Without this patch:
starting USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 9 USB Device(s) found

time: 10.822 seconds

With this patch:
starting USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 9 USB Device(s) found

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

I'm not really sure if this patch can be accepted in general as is.

In patch 0d437bca [usb: hub: fix power good delay timing] by Stephen
Warren, this delay was changes according to "Connect Timing ECN.pdf"
to a total of 1 second plus the hub port's power good time. Now its
changes back to 100ms which is a violation of this spec. But still,
for most USB devices this 100ms is more than enough. So its a valid
use-case to lower this time in special (perhaps static) USB
environments.

Actually that 1 second + poweron delay is why we had the 1 sec
delay in usb_hub_configure(), that is where we wait for a device
to show up. Note that we can already save a lot of time, by
first powering up all ports and then doing the 1 sec wait
and then checking all ports without needing to delay any further.

Or even better:

1) turn on all ports
2) do power-on-delay (either 2ms * bPwrOn2PwrGood from descriptor, or
    100ms which ever one is LARGER)
3) set a timeout val 1 sec from now, loop over all ports and quit
    the loop when either all ports are connected (we already skip the
    per port delay in this connected case now), or when the 1 sec
    expires. There is no reason for the 1 sec per port delay,
    one sec + bPwrOn2PwrGood delay is enough for all ports

One question here: Do we really need to do this power-on-delay in
step 2) at all? Shouldn't it be sufficient to do the port scanning
loop with a "2ms * bPwrOn2PwrGood + 1 sec" timeout instead? As
I've done in the patch I've attached (which works just fine on
my platform).

Note that I will also dig into the parallelizing idea as well. I'm
just trying to implement the timeout handling correctly first.

Thanks,
Stefan

>From 7e490d2d532929113ceb4e8bd5bb9d39b1d5836a Mon Sep 17 00:00:00 2001
From: Stefan Roese <s...@denx.de>
Date: Fri, 11 Mar 2016 10:53:45 +0100
Subject: [PATCH] usb: Change power-on / scanning timeout handling

This patch removes the power-on delay in usb_hub_power_on(). Instead
the timeout is calculated and used in the following per-port scanning
loop as the timeout to detect active USB devices.

The resulting power-on timeout / delay for each port is 1 second
plus "hub->desc.bPwrOn2PwrGood * 2".

Without this patch:
starting USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 9 USB Device(s) found

time: 22.266 seconds

With this patch:
starting USB...
USB0:   USB EHCI 1.00
scanning bus 0 for devices... 9 USB Device(s) found

time: 5.723 seconds

Signed-off-by: Stefan Roese <s...@denx.de>
---
 common/usb_hub.c | 14 +++++++++-----
 include/usb.h    |  2 ++
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index 721cfd8..f651781 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -120,7 +120,13 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
 		pgood_delay = max(pgood_delay,
 			          (unsigned)simple_strtol(env, NULL, 0));
 	debug("pgood_delay=%dms\n", pgood_delay);
-	mdelay(pgood_delay + 1000);
+
+	/*
+	 * Only record the power-on timeout here. The max. delay (timeout)
+	 * will be done based on this value in the USB port loop in
+	 * usb_hub_configure() later.
+	 */
+	dev->poweron_timeout = get_timer(0) + pgood_delay + 1000;
 }
 
 void usb_hub_reset(void)
@@ -470,12 +476,10 @@ static int usb_hub_configure(struct usb_device *dev)
 		ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 		unsigned short portstatus, portchange;
 		int ret;
-		ulong start = get_timer(0);
-		uint delay = CONFIG_SYS_HZ;
 
 #ifdef CONFIG_SANDBOX
 		if (state_get_skip_delays())
-			delay = 0;
+			dev->poweron_timeout = 0;
 #endif
 #ifdef CONFIG_DM_USB
 		debug("\n\nScanning '%s' port %d\n", dev->dev->name, i + 1);
@@ -508,7 +512,7 @@ static int usb_hub_configure(struct usb_device *dev)
 			if (portstatus & USB_PORT_STAT_CONNECTION)
 				break;
 
-		} while (get_timer(start) < delay);
+		} while (get_timer(0) < dev->poweron_timeout);
 
 		if (ret < 0)
 			continue;
diff --git a/include/usb.h b/include/usb.h
index 0b410b6..42edab6 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -153,6 +153,8 @@ struct usb_device {
 	struct udevice *dev;		/* Pointer to associated device */
 	struct udevice *controller_dev;	/* Pointer to associated controller */
 #endif
+
+	ulong poweron_timeout;		/* Power-On timeout value in ms */
 };
 
 struct int_queue;
-- 
2.7.3

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to