[U-Boot] [PATCH v5 0/4] usb: Reduce USB scanning time
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: 1.822 seconds
As you can see, the time is reduced from 28.4 to 1.8 seconds!
Please find more details to the changes in the patch description.
Testing and comments welcome!
Thanks, Stefan
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: - Minor rewording / fixes of the commit text - Add Acked-by / Tested-by from Hans and Stephen - Minor rewording / fixes of the commit text - Add Acked-by from Hans - 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: - Changed small timeout from 10ms to 20ms as this results in a much faster USB scanning time (10ms too small and 20ms enough in many cases) - 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: - 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 | 329 ++++++++++++++++++++++++++++++++++++++----------------- include/usb.h | 4 + 3 files changed, 235 insertions(+), 111 deletions(-)
Start with a short USB hub reset delay of 20ms. This can be enough for some configurations.
The 2nd delay at the end 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 v5: None Changes in v4: - Minor rewording / fixes of the commit text
Changes in v3: - Changed small timeout from 10ms to 20ms as this results in a much faster USB scanning time (10ms too small and 20ms enough in many cases)
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..2089e20 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 20 +#define HUB_LONG_RESET_TIME 200 + /* TODO(sjg@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) {
On Tue, Mar 15, 2016 at 8:59 PM, Stefan Roese sr@denx.de wrote:
Start with a short USB hub reset delay of 20ms. This can be enough for some configurations.
The 2nd delay at the end 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 v5: None Changes in v4:
- Minor rewording / fixes of the commit text
Changes in v3:
- Changed small timeout from 10ms to 20ms as this results in a much faster USB scanning time (10ms too small and 20ms enough in many cases)
Changes in v2:
- Add Acked-by / Tested-by from Hans and Stephen
common/usb_hub.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Tested-by: Bin Meng bmeng.cn@gmail.com
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.003 seconds
So ~4.5 seconds of USB scanning time reduction.
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 v5: None Changes in v4: - Add Acked-by / Tested-by from Hans and Stephen
Changes in v3: None 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 2089e20..d621f50 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;
On Tue, Mar 15, 2016 at 8:59 PM, Stefan Roese sr@denx.de wrote:
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.003 seconds
So ~4.5 seconds of USB scanning time reduction.
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 v5: None Changes in v4:
- Add Acked-by / Tested-by from Hans and Stephen
Changes in v3: None 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(-)
Tested-by: Bin Meng bmeng.cn@gmail.com
Debugging has shown, that all USB hubs are being reset 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.003 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: 20.392 seconds
So ~3.6 seconds of USB scanning time reduction.
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 v5: None Changes in v4: - Minor rewording / fixes of the commit text - Add Acked-by from Hans
Changes in v3: None 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; }
On Tue, Mar 15, 2016 at 8:59 PM, Stefan Roese sr@denx.de wrote:
Debugging has shown, that all USB hubs are being reset 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.003 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: 20.392 seconds
So ~3.6 seconds of USB scanning time reduction.
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 v5: None Changes in v4:
- Minor rewording / fixes of the commit text
- Add Acked-by from Hans
Changes in v3: None Changes in v2:
- Make this change unconditional
- Add Tested-by from Stephen
common/usb.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-)
Tested-by: Bin Meng bmeng.cn@gmail.com
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@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@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
Hi,
On 15-03-16 13:59, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@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
Looks good to me, my Acked-by still stands. Thanks for doing a v5.
Regards,
Hans
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@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.*/
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
On 03/15/2016 06:59 AM, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@nvidia.com
This version still works for me (the timing numbers I sent a few minutes ago were with this version too).
On Tue, Mar 15, 2016 at 8:59 PM, Stefan Roese sr@denx.de wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@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(-)
Tested-by: Bin Meng bmeng.cn@gmail.com
On 03/15/2016 01:59 PM, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@nvidia.com
This breaks DWC2 on SoCkit, I can no longer detect any USB device. USB works without this patch though. Ideas?
Best regards, Marek Vasut
Hi,
On 04/02/2016 12:22 AM, Marek Vasut wrote:
On 03/15/2016 01:59 PM, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@nvidia.com
This breaks DWC2 on SoCkit, I can no longer detect any USB device. USB works without this patch though. Ideas?
Have you tried simply adding a large sleep before the initial uart, or doing an "usb reset" after the initial scan ?
The biggest change here is the change in timing ...
Also try defining debug printf in usb_hub.c and see what output you get ?
Regards,
Hans
On 04/02/2016 11:21 PM, Hans de Goede wrote:
Hi,
Hi!
On 04/02/2016 12:22 AM, Marek Vasut wrote:
On 03/15/2016 01:59 PM, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@nvidia.com
This breaks DWC2 on SoCkit, I can no longer detect any USB device. USB works without this patch though. Ideas?
Have you tried simply adding a large sleep before the initial uart, or doing an "usb reset" after the initial scan ?
Yeah, that doesn't help. But I also sent a few fixes for the DWC2 controller, so let's see. I also finally got Stefan a working board, so he can start poking around :)
The biggest change here is the change in timing ...
Also try defining debug printf in usb_hub.c and see what output you get ?
I'll be offloading this to Stefan :)
Regards,
Hans
On 28.04.2016 01:07, Marek Vasut wrote:
On 04/02/2016 11:21 PM, Hans de Goede wrote:
Hi,
Hi!
On 04/02/2016 12:22 AM, Marek Vasut wrote:
On 03/15/2016 01:59 PM, Stefan Roese wrote:
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@denx.de Acked-by: Hans de Goede hdegoede@redhat.com Tested-by: Stephen Warren swarren@nvidia.com
This breaks DWC2 on SoCkit, I can no longer detect any USB device. USB works without this patch though. Ideas?
Have you tried simply adding a large sleep before the initial uart, or doing an "usb reset" after the initial scan ?
Yeah, that doesn't help. But I also sent a few fixes for the DWC2 controller, so let's see. I also finally got Stefan a working board, so he can start poking around :)
Okay, here my collection of tests with numerous USB keys on SoCFPGA (SoCrates). All these tests were made with current mainline U-Boot and the following patches applied:
http://patchwork.ozlabs.org/patch/614747/
http://patchwork.ozlabs.org/patch/615647/ http://patchwork.ozlabs.org/patch/615650/ http://patchwork.ozlabs.org/patch/615649/ http://patchwork.ozlabs.org/patch/615651/
a) With current mainline (USB scanning enhancement patch included)
and
b) USB scanning enhancement patch reverted (usb: Change power-on / scanning timeout handling
I've used the following command to test the USB sticks:
dcache off;time usb start;usb tree
1. Kingston DataTraveler 2.0 (64GiB): ------------------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... Device NOT ready Request Sense returned 00 00 00 2 USB Device(s) found
time: 2 minutes, 1.880 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) Kingston DataTraveler 2.0 50E549C688C4BE7189942766
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... Device NOT ready Request Sense returned 00 00 00 2 USB Device(s) found
time: 2 minutes, 2.784 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) Kingston DataTraveler 2.0 50E549C688C4BE7189942766
2. "Paradise" (USBest Technology USB Mass Storage Device) 4GiB: --------------------------------------------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 0.936 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 98mA) USBest Technology USB Mass Storage Device 09092207fbf0c4
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.571 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 98mA) USBest Technology USB Mass Storage Device 09092207fbf0c4
3. Intenso Alu Line 16GiB: -------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 1 USB Device(s) found
time: 1.390 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) U-Boot Root Hub
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 2.447 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) 6989 Intenso Alu Line EE706F5E
4. Transcent USB 3.0 JF780 (JetFlash) 16GiB: -------------------------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.317 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) JetFlash Mass Storage Device 3281440601
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.398 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) JetFlash Mass Storage Device 3281440601
5. Transcend (JetFlash) 16GiB: ------------------------------ a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 1 USB Device(s) found
time: 1.390 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) U-Boot Root Hub
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.880 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 200mA) JetFlash Mass Storage Device 55DECDA5
6. Kingston DataTraveler DTSE9 16GiB: ------------------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 0.580 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 300mA) Kingston DataTraveler 2.0 C860008863CCBFC07A0668FC
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.485 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 300mA) Kingston DataTraveler 2.0 C860008863CCBFC07A0668FC
7. microSD card in very "cheap" USB-microSD card reader: -------------------------------------------------------- a) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 0.512 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 500mA)
b) starting USB... USB0: Core Release: 2.93a scanning bus 0 for devices... 2 USB Device(s) found
time: 1.397 seconds USB device tree: 1 Hub (480 Mb/s, 0mA) | U-Boot Root Hub | +-2 Mass Storage (480 Mb/s, 500mA)
In summary, I have only 2 USB sticks here, where I can see a regression with the USB scanning patch. These are the following USB sticks:
3. Intenso Alu Line 16GiB 5. Transcend (JetFlash) 16GiB
I've experimented with "usb_pgood_delay" a bit and both USB keys start to work with a value of 700ms (600 does not work always). So 1000 seems to be a safe value for "usb_pgood_delay" on SoCFPGA right now.
My current feeling is that this problem is more related to the DWC2 driver than the general USB scanning code that my patch enhances. As such USB key detection regressions have not been observed on other platforms so far. Even using the same problematic USB sticks listed above. Reverting this USB scanning patch will only paper over the main issue that is very likely hidden in the DWC2 driver (or the SoCFPGA specific part of it). So I'm definitely against revering this patch but instead using "usb_pgood_delay=1000" on SoCFPGA for this release.
Thanks, Stefan
On 03/15/2016 01:59 PM, 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: 1.822 seconds
As you can see, the time is reduced from 28.4 to 1.8 seconds!
Please find more details to the changes in the patch description.
Testing and comments welcome!
Thanks, Stefan
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:
- Minor rewording / fixes of the commit text
- Add Acked-by / Tested-by from Hans and Stephen
- Minor rewording / fixes of the commit text
- Add Acked-by from Hans
- 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:
- Changed small timeout from 10ms to 20ms as this results in a much faster USB scanning time (10ms too small and 20ms enough in many cases)
- 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:
- 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 | 329 ++++++++++++++++++++++++++++++++++++++----------------- include/usb.h | 4 + 3 files changed, 235 insertions(+), 111 deletions(-)
Applied to u-boot-usb/master , thanks!
Hi Stefan,
On 15 March 2016 at 13:53, Marek Vasut marex@denx.de wrote:
On 03/15/2016 01:59 PM, 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!
That is truly amazing - nice work!
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: 1.822 seconds
As you can see, the time is reduced from 28.4 to 1.8 seconds!
Please find more details to the changes in the patch description.
Testing and comments welcome!
Thanks, Stefan
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:
- Minor rewording / fixes of the commit text
- Add Acked-by / Tested-by from Hans and Stephen
- Minor rewording / fixes of the commit text
- Add Acked-by from Hans
- 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:
- Changed small timeout from 10ms to 20ms as this results in a much faster USB scanning time (10ms too small and 20ms enough in many cases)
- 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:
- 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 | 329 ++++++++++++++++++++++++++++++++++++++----------------- include/usb.h | 4 + 3 files changed, 235 insertions(+), 111 deletions(-)
Applied to u-boot-usb/master , thanks!
-- Best regards, Marek Vasut
Regards, Simon
Hi Stefan,
On Tue, Mar 15, 2016 at 8:59 PM, Stefan Roese sr@denx.de 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: 1.822 seconds
As you can see, the time is reduced from 28.4 to 1.8 seconds!
Please find more details to the changes in the patch description.
Testing and comments welcome!
Thanks, Stefan
This is a great improvement. Thanks!
Some numbers FYI.
I've tested the v5 patch on Intel Crown Bay. The 'usb start' command takes about 3 seconds to finish, compared to previously 9 seconds.
Regards, Bin
participants (6)
-
Bin Meng -
Hans de Goede -
Marek Vasut -
Simon Glass -
Stefan Roese -
Stephen Warren