[U-Boot] [PATCH v2 0/4] usb: Reduce USB scanning time

[U-Boot] [PATCH v2 0/4] usb: Reduce USB scanning time

[Stefan Roese]

My current x86 platform (Bay Trail, not in mainline yet) has a quite
complex USB infrastructure with many USB hubs. Here the USB scan takes
an incredible huge amount of time:

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

time: 28.415 seconds

This is of course not acceptable on platforms, where USB needs to get
scanned at every bootup. As this increases the bootup time of this
device by nearly 30 seconds!

This patch series greatly reduces the USB scanning time. This is done
by multiple means:

- Remove or reduce delays and timeouts
- Remove a 2nd reset of the USB hubs
- Change USB port timeout handling and introduce quasi parallel USB
  port scanning

As a result, the USB scanning time is greatly reduced:

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

time: 4.606 seconds

As you can see, the time is reduced from 28.4 to 4.6 seconds!

Please find more details to the changes in the patch description.

Testing and comments welcome!

Thanks,
Stefan

Changes in v2:
- Add Acked-by / Tested-by from Hans and Stephen
- Make this change unconditional
- Add Acked-by / Tested-by from Hans and Stephen
- Make this change unconditional
- Add Tested-by from Stephen
- Remove static USB port configuration patch (for now)

Stefan Roese (4):
  usb: legacy_hub_port_reset(): Speedup hub reset handling
  usb: Remove 200 ms delay in usb_hub_port_connect_change()
  usb: Don't reset the USB hub a 2nd time
  usb: Change power-on / scanning timeout handling

 common/usb.c     |  13 +------
 common/usb_hub.c | 110 +++++++++++++++++++++++++++++++++++++++----------------
 include/usb.h    |   2 +
 3 files changed, 81 insertions(+), 44 deletions(-)

-- 
2.7.3

[U-Boot] [PATCH v2 1/4] usb: legacy_hub_port_reset(): Speedup hub reset handling

[Stefan Roese]

Start with a short USB hub reset delay of 10ms. This can be enough for
some configurations.

The 2nd delay at the of the loop is completely removed. Since the delay
hasn't been long enough, a longer delay time of 200ms is assigned. And
will be used in the next loop round.

This hub reset handling is also used in the v4.4 Linux USB driver,
hub_port_reset().

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Acked-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Cc: Marek Vasut <marex@denx.de>

---

Changes in v2:
- Add Acked-by / Tested-by from Hans and Stephen

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

diff --git a/common/usb_hub.c b/common/usb_hub.c
index e1de813..10fdd3c 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -46,6 +46,9 @@ DECLARE_GLOBAL_DATA_PTR;
 
 #define USB_BUFSIZ	512
 
+#define HUB_SHORT_RESET_TIME	10
+#define HUB_LONG_RESET_TIME	200
+
 /* TODO(sjg at chromium.org): Remove this when CONFIG_DM_USB is defined */
 static struct usb_hub_device hub_dev[USB_MAX_HUB];
 static int usb_hub_index;
@@ -164,6 +167,7 @@ int legacy_hub_port_reset(struct usb_device *dev, int port,
 	int err, tries;
 	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 	unsigned short portstatus, portchange;
+	int delay = HUB_SHORT_RESET_TIME; /* start with short reset delay */
 
 #ifdef CONFIG_DM_USB
 	debug("%s: resetting '%s' port %d...\n", __func__, dev->dev->name,
@@ -176,7 +180,7 @@ int legacy_hub_port_reset(struct usb_device *dev, int port,
 		if (err < 0)
 			return err;
 
-		mdelay(200);
+		mdelay(delay);
 
 		if (usb_get_port_status(dev, port + 1, portsts) < 0) {
 			debug("get_port_status failed status %lX\n",
@@ -215,7 +219,8 @@ int legacy_hub_port_reset(struct usb_device *dev, int port,
 		if (portstatus & USB_PORT_STAT_ENABLE)
 			break;
 
-		mdelay(200);
+		/* Switch to long reset delay for the next round */
+		delay = HUB_LONG_RESET_TIME;
 	}
 
 	if (tries == MAX_TRIES) {
-- 
2.7.3

[U-Boot] [PATCH v2 2/4] usb: Remove 200 ms delay in usb_hub_port_connect_change()

[Stefan Roese]

This patch removes 2 mdelay(200) calls from usb_hub_port_connect_change().
These delays don't seem to be necessary. At least not in my tests. Here
the number for a custom x86 Bay Trail board (not in mainline yet) with
a quite large and complex USB hub infrastructure.

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

time: 28.415 seconds

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

time: 24.811 seconds

So ~3.5 seconds of USB scanning time reduction.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Stephen Warren <swarren@nvidia.com>
Cc: Marek Vasut <marex@denx.de>

---

Changes in v2:
- Make this change unconditional
- Add Acked-by / Tested-by from Hans and Stephen

 common/usb_hub.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index 10fdd3c..721cfd8 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -275,7 +275,6 @@ int usb_hub_port_connect_change(struct usb_device *dev, int port)
 		if (!(portstatus & USB_PORT_STAT_CONNECTION))
 			return -ENOTCONN;
 	}
-	mdelay(200);
 
 	/* Reset the port */
 	ret = legacy_hub_port_reset(dev, port, &portstatus);
@@ -285,8 +284,6 @@ int usb_hub_port_connect_change(struct usb_device *dev, int port)
 		return ret;
 	}
 
-	mdelay(200);
-
 	switch (portstatus & USB_PORT_STAT_SPEED_MASK) {
 	case USB_PORT_STAT_SUPER_SPEED:
 		speed = USB_SPEED_SUPER;
-- 
2.7.3

[U-Boot] [PATCH v2 3/4] usb: Don't reset the USB hub a 2nd time

[Stefan Roese]

Debugging has shown, that all USB hubs are being resetted twice while
USB scanning. This introduces additional delays and makes USB scanning
even more slow. Testing has shown that this 2nd USB hub reset doesn't
seem to be necessary.

This patch now removes this 2nd USB hub reset. Resulting in faster USB
scan time. Here the current numbers:

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

time: 24.811 seconds

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

time: 22.269 seconds

So ~2.5 seconds of USB scanning time reduction.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Simon Glass <sjg@chromium.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Tested-by: Stephen Warren <swarren@nvidia.com>
Cc: Marek Vasut <marex@denx.de>

---

Changes in v2:
- Make this change unconditional
- Add Tested-by from Stephen

 common/usb.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index c7b8b0e..45a5a0f 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -919,19 +919,8 @@ __weak int usb_alloc_device(struct usb_device *udev)
 
 static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub)
 {
-	if (hub) {
-		unsigned short portstatus;
-		int err;
-
-		/* reset the port for the second time */
-		err = legacy_hub_port_reset(hub, dev->portnr - 1, &portstatus);
-		if (err < 0) {
-			printf("\n     Couldn't reset port %i\n", dev->portnr);
-			return err;
-		}
-	} else {
+	if (!hub)
 		usb_reset_root_port(dev);
-	}
 
 	return 0;
 }
-- 
2.7.3

[U-Boot] [PATCH v2 4/4] usb: Change power-on / scanning timeout handling

[Stefan Roese]

This patch changes the USB port scanning procedure and timeout
handling in the following ways:

a)
The power-on delay in usb_hub_power_on() is now reduced to a value of
max(100ms, "hub->desc.bPwrOn2PwrGood * 2"). The total timeout for this
hub, which is 1 second + "hub->desc.bPwrOn2PwrGood * 2" is calculated
and will be used in the following per-port scanning loop as the timeout
to detect active USB devices on this hub.

b)
The ports are now scanned in a quasy parallel way. The current code did
wait for each (unconnected) port to reach its timeout. And only then
continue with the next port. This patch now changes this to scan all
ports of one USB hub quasi simultaniously. Resulting in a faster USB
scan time as the recursive scanning of USB hubs connected to the hub
thats currently being scanned will start earlier.

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: 4.606 seconds

So ~17.7 seconds of USB scanning time reduction.

Signed-off-by: Stefan Roese <sr@denx.de>

---

Changes in v2:
- Remove static USB port configuration patch (for now)

 common/usb_hub.c | 98 ++++++++++++++++++++++++++++++++++++++++----------------
 include/usb.h    |  2 ++
 2 files changed, 73 insertions(+), 27 deletions(-)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index 721cfd8..706e7cf 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -120,7 +120,19 @@ 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);
+
+	/*
+	 * 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;
+
+	/*
+	 * Do a minimum delay of the larger value of 100ms or pgood_delay
+	 * so that the power can stablize before the devices are queried
+	 */
+	mdelay(max(100, (int)pgood_delay));
 }
 
 void usb_hub_reset(void)
@@ -342,6 +354,8 @@ static int usb_hub_configure(struct usb_device *dev)
 	struct usb_hub_descriptor *descriptor;
 	struct usb_hub_device *hub;
 	__maybe_unused struct usb_hub_status *hubsts;
+	int device_probed[USB_MAXCHILDREN];
+	int devices_total;
 	int ret;
 
 	/* "allocate" Hub device */
@@ -466,31 +480,44 @@ static int usb_hub_configure(struct usb_device *dev)
 	for (i = 0; i < dev->maxchild; i++)
 		usb_hub_reset_devices(i + 1);
 
-	for (i = 0; i < dev->maxchild; i++) {
-		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;
+	if (state_get_skip_delays())
+		dev->poweron_timeout = 0;
 #endif
+
+	/*
+	 * Start a loop on this hub which is only exited, once all
+	 * devices (children) of this hub have been probed (either
+	 * detected of timeout)
+	 */
+	devices_total = 0;
+	memset(device_probed, 0,
+	       ARRAY_SIZE(device_probed) * sizeof(device_probed[0]));
+	while (devices_total < dev->maxchild) {
+		ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
+		unsigned short portstatus = 0;
+		unsigned short portchange = 0;
+		int ret = 0;
+
+		/*
+		 * Do one loop over all devices on this hub and check if
+		 * either a) a new device is detected or b) a timeout on
+		 * a device has been reached. If either a) or b) occurs,
+		 * mark this device as "probed" and increase the number
+		 * of probed devices of this hub (devices_total).
+		 */
+		for (i = 0; i < dev->maxchild; i++) {
+			/* Skip devices that are already detected */
+			if (device_probed[i])
+				continue;
+
 #ifdef CONFIG_DM_USB
-		debug("\n\nScanning '%s' port %d\n", dev->dev->name, i + 1);
+			debug("\n\nScanning '%s' port %d\n",
+			      dev->dev->name, i + 1);
 #else
-		debug("\n\nScanning port %d\n", i + 1);
+			debug("\n\nScanning port %d\n", i + 1);
 #endif
-		/*
-		 * Wait for (whichever finishes first)
-		 *  - A maximum of 10 seconds
-		 *    This is a purely observational value driven by connecting
-		 *    a few broken pen drives and taking the max * 1.5 approach
-		 *  - connection_change and connection state to report same
-		 *    state
-		 */
-		do {
+
 			ret = usb_get_port_status(dev, i + 1, portsts);
 			if (ret < 0) {
 				debug("get_port_status failed\n");
@@ -501,18 +528,35 @@ static int usb_hub_configure(struct usb_device *dev)
 			portchange = le16_to_cpu(portsts->wPortChange);
 
 			/* No connection change happened, wait a bit more. */
-			if (!(portchange & USB_PORT_STAT_C_CONNECTION))
+			if (!(portchange & USB_PORT_STAT_C_CONNECTION)) {
+				if (get_timer(0) >= dev->poweron_timeout) {
+					device_probed[i] = 1;
+					devices_total++;
+				}
 				continue;
+			}
 
 			/* Test if the connection came up, and if so, exit. */
-			if (portstatus & USB_PORT_STAT_CONNECTION)
+			if (portstatus & USB_PORT_STAT_CONNECTION) {
+				/*
+				 * Mark this device as ready so that it won't be
+				 * scanned in the next loop
+				 */
+				device_probed[i] = 1;
+				devices_total++;
 				break;
+			}
+		}
 
-		} while (get_timer(start) < delay);
-
-		if (ret < 0)
+		/*
+		 * Scan again if either an error has occured, or no new
+		 * USB devices have been detected
+		 */
+		if ((ret < 0) || (i >= dev->maxchild))
 			continue;
 
+		/* A new USB device is ready at this point */
+
 		debug("Port %d Status %X Change %X\n",
 		      i + 1, portstatus, portchange);
 
@@ -561,7 +605,7 @@ static int usb_hub_configure(struct usb_device *dev)
 			usb_clear_port_feature(dev, i + 1,
 						USB_PORT_FEAT_C_RESET);
 		}
-	} /* end for i all ports */
+	}
 
 	return 0;
 }
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] [PATCH v2 0/4] usb: Reduce USB scanning time

[Stephen Warren]

On 03/11/2016 07:55 AM, Stefan Roese wrote:
>
> My current x86 platform (Bay Trail, not in mainline yet) has a quite
> complex USB infrastructure with many USB hubs. Here the USB scan takes
> an incredible huge amount of time:
>
> starting USB...
> USB0:   USB EHCI 1.00
> scanning bus 0 for devices... 9 USB Device(s) found
>
> time: 28.415 seconds
>
> This is of course not acceptable on platforms, where USB needs to get
> scanned at every bootup. As this increases the bootup time of this
> device by nearly 30 seconds!
>
> This patch series greatly reduces the USB scanning time. This is done
> by multiple means:
>
> - Remove or reduce delays and timeouts
> - Remove a 2nd reset of the USB hubs
> - Change USB port timeout handling and introduce quasi parallel USB
>    port scanning
>
> As a result, the USB scanning time is greatly reduced:
>
> starting USB...
> USB0:   USB EHCI 1.00
> scanning bus 0 for devices... 9 USB Device(s) found
>
> time: 4.606 seconds
>
> As you can see, the time is reduced from 28.4 to 4.6 seconds!
>
> Please find more details to the changes in the patch description.
>
> Testing and comments welcome!

The series,
Tested-by: Stephen Warren <swarren@nvidia.com>
(same set of tests as last time)

[U-Boot] [PATCH v2 0/4] usb: Reduce USB scanning time

[Hans de Goede]

Hi,

On 11-03-16 18:53, Stephen Warren wrote:
> On 03/11/2016 07:55 AM, Stefan Roese wrote:
>>
>> My current x86 platform (Bay Trail, not in mainline yet) has a quite
>> complex USB infrastructure with many USB hubs. Here the USB scan takes
>> an incredible huge amount of time:
>>
>> starting USB...
>> USB0:   USB EHCI 1.00
>> scanning bus 0 for devices... 9 USB Device(s) found
>>
>> time: 28.415 seconds
>>
>> This is of course not acceptable on platforms, where USB needs to get
>> scanned at every bootup. As this increases the bootup time of this
>> device by nearly 30 seconds!
>>
>> This patch series greatly reduces the USB scanning time. This is done
>> by multiple means:
>>
>> - Remove or reduce delays and timeouts
>> - Remove a 2nd reset of the USB hubs
>> - Change USB port timeout handling and introduce quasi parallel USB
>>    port scanning
>>
>> As a result, the USB scanning time is greatly reduced:
>>
>> starting USB...
>> USB0:   USB EHCI 1.00
>> scanning bus 0 for devices... 9 USB Device(s) found
>>
>> time: 4.606 seconds
>>
>> As you can see, the time is reduced from 28.4 to 4.6 seconds!
>>
>> Please find more details to the changes in the patch description.
>>
>> Testing and comments welcome!
>
> The series,
> Tested-by: Stephen Warren <swarren@nvidia.com>
> (same set of tests as last time)

And the entire series looks good to me (note not tested only reviewed) :

Acked-by: Hans de Goede <hdegoede@redhat.com>

Thank you for working on this.

Regards,

Hans