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

Stefan Roese sr at denx.de
Tue Mar 15 13:59:15 CET 2016


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 code does not wait
using mdelay, instead usb_hub_power_on() will wait before querying
the device in the scanning loop later. 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)
Don't delay the minimum delay (for power to stabilize) in
usb_hub_power_on(). Instead skip querying these devices in the scannig
loop until the delay time is reached.

c)
The ports are now scanned in a quasi 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 all USB hubs quasi simultaneously. For this, all ports are added
to a scanning list. This list is scanned until all ports are ready
by either a) reaching the connection timeout (calculated earlier), or
by b) detecting a USB device. This results in a faster USB scan time as
the recursive scanning of USB hubs connected to the hub that's currently
being scanned will start earlier.

One small functional change to the original code is, that ports with
overcurrent detection will now get rescanned multiple times
(PORT_OVERCURRENT_MAX_SCAN_COUNT).

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

time: 20.163 seconds

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

time: 1.822 seconds

So ~18.3 seconds of USB scanning time reduction.

Signed-off-by: Stefan Roese <sr at denx.de>
Acked-by: Hans de Goede <hdegoede at redhat.com>
Tested-by: Stephen Warren <swarren at nvidia.com>

---

Changes in v5:
- Removed superfluous debug output from usb_scan_port()
- Replaced usb_hub_power_on() with usb_set_port_feature() in case
  of overcurrent detection. This is whats really needed here.
- Added a per-port overcurrent counter and stop re-scanning of
  a device that exceeds the PORT_OVERCURRENT_MAX_SCAN_COUNT
- Moved list_del() to end of loop in usb_scan_port()
- Moved timeout / delay variables from USB dev to hub struct
  as they are hub specific
- Moved state_get_skip_delays() to usb_hub_power_on() so that
  the timeouts are not set there

Changes in v4:
- Moved check for query_delay into usb_scan_port() as suggested by Hans
- Correct list handling (drop INIT_LIST_HEAD)
- Added some missing free() calls
- Changed connect_timeout calculation as suggested by Stephen
- Moved usb_scan_list to other globals to be cleaned up in a later patch
- Added timeout check for non-functional ports (usb_get_port_status
  return error
- Reverted if logic in loop to remove an indentation level
- Moved debug() output
- Removed unnecessary if when already connected
- Added Hans's Acked-by
- Added Stephen's Tested-by
- Minor rewording / fixes of the commit text

Changes in v3:
- Introduced scanning list containing all USB devices of one USB
  controller that need to get scanned
- Don't delay in usb_hub_power_on(). Instead skip querying these devices
  until the delay time is reached.

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

 common/usb_hub.c | 317 ++++++++++++++++++++++++++++++++++++++-----------------
 include/usb.h    |   4 +
 2 files changed, 227 insertions(+), 94 deletions(-)

diff --git a/common/usb_hub.c b/common/usb_hub.c
index d621f50..e6a2cdb 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -30,6 +30,7 @@
 #include <asm/processor.h>
 #include <asm/unaligned.h>
 #include <linux/ctype.h>
+#include <linux/list.h>
 #include <asm/byteorder.h>
 #ifdef CONFIG_SANDBOX
 #include <asm/state.h>
@@ -49,9 +50,19 @@ DECLARE_GLOBAL_DATA_PTR;
 #define HUB_SHORT_RESET_TIME	20
 #define HUB_LONG_RESET_TIME	200
 
+#define PORT_OVERCURRENT_MAX_SCAN_COUNT		3
+
+struct usb_device_scan {
+	struct usb_device *dev;		/* USB hub device to scan */
+	struct usb_hub_device *hub;	/* USB hub struct */
+	int port;			/* USB port to scan */
+	struct list_head list;
+};
+
 /* 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;
+static LIST_HEAD(usb_scan_list);
 
 __weak void usb_hub_reset_devices(int port)
 {
@@ -109,6 +120,15 @@ static void usb_hub_power_on(struct usb_hub_device *hub)
 		debug("port %d returns %lX\n", i + 1, dev->status);
 	}
 
+#ifdef CONFIG_SANDBOX
+	/*
+	 * Don't set timeout / delay values here. This results
+	 * in these values still being reset to 0.
+	 */
+	if (state_get_skip_delays())
+		return;
+#endif
+
 	/*
 	 * Wait for power to become stable,
 	 * plus spec-defined max time for device to connect
@@ -120,12 +140,30 @@ 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);
+
+	/*
+	 * Do a minimum delay of the larger value of 100ms or pgood_delay
+	 * so that the power can stablize before the devices are queried
+	 */
+	hub->query_delay = get_timer(0) + max(100, (int)pgood_delay);
+
+	/*
+	 * 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.
+	 */
+	hub->connect_timeout = hub->query_delay + 1000;
+	debug("devnum=%d poweron: query_delay=%d connect_timeout=%d\n",
+	      dev->devnum, max(100, (int)pgood_delay),
+	      max(100, (int)pgood_delay) + 1000);
 }
 
 void usb_hub_reset(void)
 {
 	usb_hub_index = 0;
+
+	/* Zero out global hub_dev in case its re-used again */
+	memset(hub_dev, 0, sizeof(hub_dev));
 }
 
 static struct usb_hub_device *usb_hub_allocate(void)
@@ -332,6 +370,168 @@ int usb_hub_port_connect_change(struct usb_device *dev, int port)
 	return ret;
 }
 
+static int usb_scan_port(struct usb_device_scan *usb_scan)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
+	unsigned short portstatus;
+	unsigned short portchange;
+	struct usb_device *dev;
+	struct usb_hub_device *hub;
+	int ret = 0;
+	int i;
+
+	dev = usb_scan->dev;
+	hub = usb_scan->hub;
+	i = usb_scan->port;
+
+	/*
+	 * Don't talk to the device before the query delay is expired.
+	 * This is needed for voltages to stabalize.
+	 */
+	if (get_timer(0) < hub->query_delay)
+		return 0;
+
+	ret = usb_get_port_status(dev, i + 1, portsts);
+	if (ret < 0) {
+		debug("get_port_status failed\n");
+		if (get_timer(0) >= hub->connect_timeout) {
+			debug("devnum=%d port=%d: timeout\n",
+			      dev->devnum, i + 1);
+			/* Remove this device from scanning list */
+			list_del(&usb_scan->list);
+			free(usb_scan);
+			return 0;
+		}
+	}
+
+	portstatus = le16_to_cpu(portsts->wPortStatus);
+	portchange = le16_to_cpu(portsts->wPortChange);
+	debug("Port %d Status %X Change %X\n", i + 1, portstatus, portchange);
+
+	/* No connection change happened, wait a bit more. */
+	if (!(portchange & USB_PORT_STAT_C_CONNECTION)) {
+		if (get_timer(0) >= hub->connect_timeout) {
+			debug("devnum=%d port=%d: timeout\n",
+			      dev->devnum, i + 1);
+			/* Remove this device from scanning list */
+			list_del(&usb_scan->list);
+			free(usb_scan);
+			return 0;
+		}
+		return 0;
+	}
+
+	/* Test if the connection came up, and if not exit */
+	if (!(portstatus & USB_PORT_STAT_CONNECTION))
+		return 0;
+
+	/* A new USB device is ready at this point */
+	debug("devnum=%d port=%d: USB dev found\n", dev->devnum, i + 1);
+
+	usb_hub_port_connect_change(dev, i);
+
+	if (portchange & USB_PORT_STAT_C_ENABLE) {
+		debug("port %d enable change, status %x\n", i + 1, portstatus);
+		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_ENABLE);
+		/*
+		 * The following hack causes a ghost device problem
+		 * to Faraday EHCI
+		 */
+#ifndef CONFIG_USB_EHCI_FARADAY
+		/*
+		 * EM interference sometimes causes bad shielded USB
+		 * devices to be shutdown by the hub, this hack enables
+		 * them again. Works at least with mouse driver
+		 */
+		if (!(portstatus & USB_PORT_STAT_ENABLE) &&
+		    (portstatus & USB_PORT_STAT_CONNECTION) &&
+		    usb_device_has_child_on_port(dev, i)) {
+			debug("already running port %i disabled by hub (EMI?), re-enabling...\n",
+			      i + 1);
+			usb_hub_port_connect_change(dev, i);
+		}
+#endif
+	}
+
+	if (portstatus & USB_PORT_STAT_SUSPEND) {
+		debug("port %d suspend change\n", i + 1);
+		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_SUSPEND);
+	}
+
+	if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
+		debug("port %d over-current change\n", i + 1);
+		usb_clear_port_feature(dev, i + 1,
+				       USB_PORT_FEAT_C_OVER_CURRENT);
+		/* Only power-on this one port */
+		usb_set_port_feature(dev, i + 1, USB_PORT_FEAT_POWER);
+		hub->overcurrent_count[i]++;
+
+		/*
+		 * If the max-scan-count is not reached, return without removing
+		 * the device from scan-list. This will re-issue a new scan.
+		 */
+		if (hub->overcurrent_count[i] <=
+		    PORT_OVERCURRENT_MAX_SCAN_COUNT)
+			return 0;
+
+		/* Otherwise the device will get removed */
+		printf("Port %d over-current occured %d times\n", i + 1,
+		       hub->overcurrent_count[i]);
+	}
+
+	if (portchange & USB_PORT_STAT_C_RESET) {
+		debug("port %d reset change\n", i + 1);
+		usb_clear_port_feature(dev, i + 1, USB_PORT_FEAT_C_RESET);
+	}
+
+	/*
+	 * We're done with this device, so let's remove this device from
+	 * scanning list
+	 */
+	list_del(&usb_scan->list);
+	free(usb_scan);
+
+	return 0;
+}
+
+static int usb_device_list_scan(void)
+{
+	struct usb_device_scan *usb_scan;
+	struct usb_device_scan *tmp;
+	static int running;
+	int ret = 0;
+
+	/* Only run this loop once for each controller */
+	if (running)
+		return 0;
+
+	running = 1;
+
+	while (1) {
+		/* We're done, once the list is empty again */
+		if (list_empty(&usb_scan_list))
+			goto out;
+
+		list_for_each_entry_safe(usb_scan, tmp, &usb_scan_list, list) {
+			int ret;
+
+			/* Scan this port */
+			ret = usb_scan_port(usb_scan);
+			if (ret)
+				goto out;
+		}
+	}
+
+out:
+	/*
+	 * This USB controller has finished scanning all its connected
+	 * USB devices. Set "running" back to 0, so that other USB controllers
+	 * will scan their devices too.
+	 */
+	running = 0;
+
+	return ret;
+}
 
 static int usb_hub_configure(struct usb_device *dev)
 {
@@ -466,104 +666,33 @@ static int usb_hub_configure(struct usb_device *dev)
 	for (i = 0; i < dev->maxchild; i++)
 		usb_hub_reset_devices(i + 1);
 
+	/*
+	 * Only add the connected USB devices, including potential hubs,
+	 * to a scanning list. This list will get scanned and devices that
+	 * are detected (either via port connected or via port timeout)
+	 * will get removed from this list. Scanning of the devices on this
+	 * list will continue until all devices are removed.
+	 */
 	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;
-#endif
-#ifdef CONFIG_DM_USB
-		debug("\n\nScanning '%s' port %d\n", dev->dev->name, i + 1);
-#else
-		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");
-				break;
-			}
-
-			portstatus = le16_to_cpu(portsts->wPortStatus);
-			portchange = le16_to_cpu(portsts->wPortChange);
-
-			/* No connection change happened, wait a bit more. */
-			if (!(portchange & USB_PORT_STAT_C_CONNECTION))
-				continue;
-
-			/* Test if the connection came up, and if so, exit. */
-			if (portstatus & USB_PORT_STAT_CONNECTION)
-				break;
-
-		} while (get_timer(start) < delay);
-
-		if (ret < 0)
-			continue;
+		struct usb_device_scan *usb_scan;
 
-		debug("Port %d Status %X Change %X\n",
-		      i + 1, portstatus, portchange);
-
-		if (portchange & USB_PORT_STAT_C_CONNECTION) {
-			debug("port %d connection change\n", i + 1);
-			usb_hub_port_connect_change(dev, i);
-		}
-		if (portchange & USB_PORT_STAT_C_ENABLE) {
-			debug("port %d enable change, status %x\n",
-			      i + 1, portstatus);
-			usb_clear_port_feature(dev, i + 1,
-						USB_PORT_FEAT_C_ENABLE);
-			/*
-			 * The following hack causes a ghost device problem
-			 * to Faraday EHCI
-			 */
-#ifndef CONFIG_USB_EHCI_FARADAY
-			/* EM interference sometimes causes bad shielded USB
-			 * devices to be shutdown by the hub, this hack enables
-			 * them again. Works at least with mouse driver */
-			if (!(portstatus & USB_PORT_STAT_ENABLE) &&
-			     (portstatus & USB_PORT_STAT_CONNECTION) &&
-			     usb_device_has_child_on_port(dev, i)) {
-				debug("already running port %i "  \
-				      "disabled by hub (EMI?), " \
-				      "re-enabling...\n", i + 1);
-				      usb_hub_port_connect_change(dev, i);
-			}
-#endif
-		}
-		if (portstatus & USB_PORT_STAT_SUSPEND) {
-			debug("port %d suspend change\n", i + 1);
-			usb_clear_port_feature(dev, i + 1,
-						USB_PORT_FEAT_SUSPEND);
-		}
-
-		if (portchange & USB_PORT_STAT_C_OVERCURRENT) {
-			debug("port %d over-current change\n", i + 1);
-			usb_clear_port_feature(dev, i + 1,
-						USB_PORT_FEAT_C_OVER_CURRENT);
-			usb_hub_power_on(hub);
+		usb_scan = calloc(1, sizeof(*usb_scan));
+		if (!usb_scan) {
+			printf("Can't allocate memory for USB device!\n");
+			return -ENOMEM;
 		}
+		usb_scan->dev = dev;
+		usb_scan->hub = hub;
+		usb_scan->port = i;
+		list_add_tail(&usb_scan->list, &usb_scan_list);
+	}
 
-		if (portchange & USB_PORT_STAT_C_RESET) {
-			debug("port %d reset change\n", i + 1);
-			usb_clear_port_feature(dev, i + 1,
-						USB_PORT_FEAT_C_RESET);
-		}
-	} /* end for i all ports */
+	/*
+	 * And now call the scanning code which loops over the generated list
+	 */
+	ret = usb_device_list_scan();
 
-	return 0;
+	return ret;
 }
 
 static int usb_hub_check(struct usb_device *dev, int ifnum)
diff --git a/include/usb.h b/include/usb.h
index 0b410b6..7a82fb2 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -556,6 +556,10 @@ struct usb_hub_descriptor {
 struct usb_hub_device {
 	struct usb_device *pusb_dev;
 	struct usb_hub_descriptor desc;
+
+	ulong connect_timeout;		/* Device connection timeout in ms */
+	ulong query_delay;		/* Device query delay in ms */
+	int overcurrent_count[USB_MAXCHILDREN];	/* Over-current counter */
 };
 
 #ifdef CONFIG_DM_USB
-- 
2.7.3



More information about the U-Boot mailing list