usb: Reduce USB scanning time

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 <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

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

common/usb_hub.c +7 -2

index e1de813..10fdd3c 100644
@@ -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) {

Hi,

Quoted 7 lines · 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.

Quoted 8 lines

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

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

Since this is good enough for the kernel it should be good enough for us:

Acked-by Hans de Goede <[email protected]>

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().

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.

These mdelay calls are removed if CONFIG_USB_FAST_SCAN is defined. They are not removed per default yet. It would be good to test with this option enabled on many other boards. And once we have a good testing base we can decide to remove these delays completely, including this macro.

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

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

common/usb_hub.c +4 -0

index 10fdd3c..660f4f4 100644
@@ -275,7 +275,9 @@ int usb_hub_port_connect_change(struct usb_device *dev, int port)
  		if (!(portstatus & USB_PORT_STAT_CONNECTION))
  			return -ENOTCONN;
  	}
+#if !defined(CONFIG_USB_FAST_SCAN)
  	mdelay(200);
+#endif
 
  	/* Reset the port */
  	ret = legacy_hub_port_reset(dev, port, &portstatus);

@@ -285,7 +287,9 @@ int usb_hub_port_connect_change(struct usb_device *dev, int port)
  		return ret;
  	}
 
+#if !defined(CONFIG_USB_FAST_SCAN)
  	mdelay(200);
+#endif
 
  	switch (portstatus & USB_PORT_STAT_SPEED_MASK) {
  	case USB_PORT_STAT_SUPER_SPEED:

Hi,

Quoted 20 lines · 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

Quoted 7 lines

So ~3.5 seconds of USB scanning time reduction.

These mdelay calls are removed if CONFIG_USB_FAST_SCAN is defined. They are not removed per default yet. It would be good to test with this option enabled on many other boards. And once we have a good testing base we can decide to remove these delays completely, including this macro.

There indeed is no reason at all to delay before the reset and the kernel does not wait with checking the USB_PORT_STAT_SPEED_MASK once the reset completes, so I see no reason why we should:

Acked-by Hans de Goede <[email protected]>

Hi,

Quoted 20 lines · 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

Quoted 7 lines

So ~3.5 seconds of USB scanning time reduction.

These mdelay calls are removed if CONFIG_USB_FAST_SCAN is defined. They are not removed per default yet. It would be good to test with this option enabled on many other boards. And once we have a good testing base we can decide to remove these delays completely, including this macro.

There indeed is no reason at all to delay before the reset and the kernel does not wait with checking the USB_PORT_STAT_SPEED_MASK once the reset completes, so I see no reason why we should:

Acked-by Hans de Goede <[email protected]>

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.

These mdelay calls are removed if CONFIG_USB_FAST_SCAN is defined. They are not removed per default yet. It would be good to test with this option enabled on many other boards. And once we have a good testing base we can decide to remove these delays completely, including this macro.

On complex USB infrastructures, with many hubs and therefore many (perhaps unconnected) ports, current U-Boot has a very long USB scanning time. On my current custom x86 board, this time is over 20 seconds!!! One of the biggest problems here is a 1 second delay (timeout) in usb_hub_configure() to check, if a USB device is connected to a port. As this is done for each port, this timeout sums up over all unconnected ports.

This patch removes this 1 seconds per port timeout from usb_hub_configure().

All my USB devices are still detected correctly
on my platform. And I'm using 4 different USB keys right now. Including a problematic "Intenso Alu Line" key.

Here the numbers for my current board:

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

time: 24.811 seconds

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

time: 10.822 seconds

So ~14 seconds of USB scanning time reduction.

Again, this timeout is only removed if CONFIG_USB_FAST_SCAN is defined. Once more tests are done on multiple other platforms we can decide to remove this timeout completely.

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

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

common/usb_hub.c +4 -0

index 660f4f4..780291f 100644
@@ -478,7 +478,11 @@ static int usb_hub_configure(struct usb_device *dev)
  		unsigned short portstatus, portchange;
  		int ret;
  		ulong start = get_timer(0);
+#if defined(CONFIG_USB_FAST_SCAN)
+		uint delay = 0;
+#else
  		uint delay = CONFIG_SYS_HZ;
+#endif
 
  #ifdef CONFIG_SANDBOX
  		if (state_get_skip_delays())

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

Here the numbers for my current board:

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

time: 10.822 seconds

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

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

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

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

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

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

common/usb_hub.c +4 -0

index 780291f..d5fcd27 100644
@@ -120,7 +120,11 @@ 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);
+#if defined(CONFIG_USB_FAST_SCAN)
+	mdelay(pgood_delay + 100);
+#else
  	mdelay(pgood_delay + 1000);
+#endif
  }
 
  void usb_hub_reset(void)

Hi,

Quoted 28 lines · Stefan Roese

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

Here the numbers for my current board:

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

time: 10.822 seconds

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

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

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

Quoted 7 lines

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

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

Or even better:

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

###

Note that even better would be to in 3 first calls some sort of probe_prepare function, which if the child is a hub itself, does the poweron, and records the timeout for its loop and runs its loop once (to probe_prepare its children if they come up immediately).

And then only once the loop over all ports ends, call the real probe which will then do usb_hub_configure() for its (hub) children using the timeout recorded during the probe_prepare, and will thus likely be able to skip a large part of that 1 sec waiting since that was already waited in the parent hub's usb_hub_configure() call.

Note this would speed up scanning deep trees of hubs, if we have multiple usb controllers, it would also be really good to probe them in parallel as described here:

http://lists.denx.de/pipermail/u-boot/2016-February/245243.html

(yes I'm repeating myself :)

Quoted 28 lines · Stefan Roese

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

Here the numbers for my current board:

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

time: 10.822 seconds

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

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

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

Quoted 7 lines

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

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

Or even better:

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

Hi Hans,

Quoted 45 lines · Hans de Goede

Quoted 28 lines · Stefan Roese

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

Here the numbers for my current board:

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

time: 10.822 seconds

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

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

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

Quoted 7 lines

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

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

Or even better:

Quoted 8 lines

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

One question here: Do we really need to do this power-on-delay in step 2) at all?

Shouldn't it be sufficient to do the port scanning loop with a "2ms * bPwrOn2PwrGood + 1 sec" timeout instead? As I've done in the patch I've attached (which works just fine on my platform).

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

Hi,

Quoted 58 lines · Stefan Roese

Hi Hans,

Quoted 46 lines · Hans de Goede

Quoted 29 lines · Stefan Roese

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

Here the numbers for my current board:

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

time: 10.822 seconds

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

time: 6.319 seconds

So ~4.5 seconds of USB scanning time reduction.

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

Quoted 7 lines

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

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

Or even better:

Quoted 8 lines

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

Quoted 2 lines

One question here: Do we really need to do this power-on-delay in step 2) at all?

Yes the power may bounce during this time, causing a device connect
+ disconnect + connect again, we really need to wait for the power to be stable before looking at / talking to connected devices.

Note this is also what the kernel does, it ignores the hub after powering its ports for this time.

Quoted 7 lines

Shouldn't it be sufficient to do the port scanning loop with a "2ms * bPwrOn2PwrGood + 1 sec" timeout instead? As I've done in the patch I've attached (which works just fine on my platform).

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

Cool :)

About the patch, you're waiting up to 1 sec for the first port and then scanning the other ports without waiting. For the initial implementation this is fine, but for the parallelization of child probing you will want to scan all ports as fast as possible, skipping already connected and handled ports for the duration of 1 sec, so as to also scan the children as soon as they are connected (and not delay scanning a hub on port 2 because port 1 does not have a device connected).

Nice improvement in scan time from this simple patch btw :)

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 if CONFIG_USB_FAST_SCAN is defined. Resulting in faster USB scan time. Here the current number:

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

time: 6.319 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: 3.777 seconds

So ~2.5 seconds of USB scanning time reduction.

Again, this 2nd reset is only removed if CONFIG_USB_FAST_SCAN is defined. Once more tests are done on multiple other platforms we can decide to remove this 2nd reset completely.

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

  common/usb.c | 6 ++++++
  1 file changed, 6 insertions(+)

common/usb.c +6 -0

index c7b8b0e..b96a2f6 100644
@@ -920,6 +920,11 @@ __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) {
+#if !defined(CONFIG_USB_FAST_SCAN)
+		/*
+		 * Is this 2nd hub port reset necessary? The port has already
+		 * been reset in usb_hub_port_connect_change() before.
+		 */
  		unsigned short portstatus;
  		int err;
 

@@ -929,6 +934,7 @@ static int usb_hub_port_reset(struct usb_device *dev, struct usb_device *hub)
  			printf("\n     Couldn't reset port %i\n", dev->portnr);
  			return err;
  		}
+#endif
  	} else {
  		usb_reset_root_port(dev);
  	}

Hi,

Quoted 27 lines · 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 if CONFIG_USB_FAST_SCAN is defined. Resulting in faster USB scan time. Here the current number:

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

time: 6.319 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: 3.777 seconds

So ~2.5 seconds of USB scanning time reduction.

Quoted 3 lines

Again, this 2nd reset is only removed if CONFIG_USB_FAST_SCAN is defined. Once more tests are done on multiple other platforms we can decide to remove this 2nd reset completely.

I see no reason to make the removal conditional, I believe that this is some relic workaround for likely long fixed bugs and we should just remove it completely (for v2016.07).

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 if CONFIG_USB_FAST_SCAN is defined. Resulting in faster USB scan time. Here the current number:

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

time: 6.319 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: 3.777 seconds

So ~2.5 seconds of USB scanning time reduction.

Again, this 2nd reset is only removed if CONFIG_USB_FAST_SCAN is defined. Once more tests are done on multiple other platforms we can decide to remove this 2nd reset completely.

This patch implements an optionally quasi static USB port configuration. This is done by using an environment variable, that describes the ports that shall be scanned at the next USB scans (usb start, usb reset).

The "usb_port_use" env variable is used to describe this static USB configuration. For this, each USB hub is represented by a 8-bit value. Making it possible to configure a maximum of 8 ports for each USB hub. A 64-bit representation is used, therefore 8 USB hubs can be described in total.

Here an example for this "usb_port_use" environment variable:

usb_port_use = 0000000000040301
-------------------------------
1st USB hub: 0x01 -> Use port 1 (first port)
2nd USB hub: 0x03 -> Use port 1 and 2
3rd USB hub: 0x04 -> Use port 3
4th USB hub: 0x00 -> Use no ports
...
8th USB hub: 0x00 -> Use no ports

To make this procedure of configuring this env variable less error prone and less painful, this patch also introduces another env variable that is dynamically generated at each USB scan: "usb_port_detected". This variable is similar to "usb_port_use". It has a bit enabled for each port that has been detected. This can be easily used on a new system, where the USB configuration is static in this way:

Run the USB scan (usb start, usb reset) without the "usb_port_use" variable set. This will result in all ports being scanned and the result written into the "usb_port_detected" variable. To configure the USB subsystem to only scan these specific USB ports upon the next scans, you only need to write the value from "usb_port_detected" into the "usb_port_use" variable:

=> setenv usb_port_use ${usb_port_detected}
=> saveenv

The next scans will only scan those enabled ports.

Its of course also possible to manually "tune" this env variable. If some ports are not needed in U-Boot, they can be disabled this way. This will result in less USB hub ports getting scanned and therefore in a faster USB scan time. Here an example:

With all USB devices enabled (usb_port_use not set):

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

time: 3.776 seconds
=> usb tree
USB device tree:

   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 0mA)
     |
     +-8  Mass Storage (480 Mb/s, 200mA)
     |    6989 Intenso Alu Line EE706F5E
     |
     +-9  Mass Storage (480 Mb/s, 200mA)
     |    JetFlash Mass Storage Device 3281440601
     |
     +-3  Hub (480 Mb/s, 100mA)
     | |
     | +-4  Hub (12 Mb/s, 100mA)
     |
     +-5  Hub (480 Mb/s, 0mA)
       |
       +-6  Mass Storage (480 Mb/s, 200mA)
         |    Kingston DataTraveler 2.0 50E549C688C4BE7189942766
         |
         +-7  Mass Storage (480 Mb/s, 98mA)
              USBest Technology USB Mass Storage Device 09092207fbf0c4

=> printenv usb_port_detected
usb_port_detected=0000000501080f01

With only some USB devices enabled:

=> setenv usb_port_use 0000000000000c01
=> time usb start
starting USB...
USB0: USB EHCI 1.00
scanning bus 0 for devices... 4 USB Device(s) found

time: 1.354 seconds
=> usb tree
USB device tree:

   1  Hub (480 Mb/s, 0mA)
   |  u-boot EHCI Host Controller
   |
   +-2  Hub (480 Mb/s, 0mA)
     |
     +-3  Mass Storage (480 Mb/s, 200mA)
     |    6989 Intenso Alu Line EE706F5E
     |
     +-4  Mass Storage (480 Mb/s, 200mA)
          JetFlash Mass Storage Device 3281440601

So this feature of USB port enabling via environment variable is very helpful to further reduce the USB scanning time in some configurations.

Signed-off-by Stefan Roese <[email protected]>

Cc Simon Glass <[email protected]>

Cc Hans de Goede <[email protected]>

Cc Stephen Warren <[email protected]>

Cc Marek Vasut <[email protected]>

---

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

common/usb_hub.c +44 -0

index d5fcd27..b600cfa 100644
@@ -353,6 +353,10 @@ 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;
+	static u64 port_active;
+	u32 port_use = 0xff;	/* Default: use all ports */
+	const char *env;
+	char str[18];
  	int ret;
 
  	/* "allocate" Hub device */

@@ -477,6 +481,18 @@ static int usb_hub_configure(struct usb_device *dev)
  	for (i = 0; i < dev->maxchild; i++)
  		usb_hub_reset_devices(i + 1);
 
+	/* Check if only configured ports shall be scanned / enabled */
+	env = getenv("usb_port_use");
+	if (env) {
+		port_use = (simple_strtoull(env, NULL, 16) >>
+			    ((dev->devnum - 1) * 8)) & 0xff;
+		debug("port_usb[%d]=0x%02x\n", dev->devnum, port_use);
+	}
+
+	/* Reset port_active variable on the scan of the 1st USB hub */
+	if (dev->devnum == 1)
+		port_active = 0;
+
  	for (i = 0; i < dev->maxchild; i++) {
  		ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
  		unsigned short portstatus, portchange;

@@ -497,6 +513,23 @@ static int usb_hub_configure(struct usb_device *dev)
  #else
  		debug("\n\nScanning port %d\n", i + 1);
  #endif
+
+		/*
+		 * Check if this port should be used. This can be configured
+		 * via the "usb_port_use" env variable in a flexible way. Here
+		 * an example:
+		 *
+		 * usb_port_use = 0000000004000301
+		 * 1st USB hub: 0x01 -> Use port 1 (first port)
+		 * 2nd USB hub: 0x03 -> Use port 1 and 2
+		 * 3rd USB hub: 0x00 -> Use no ports
+		 * 4th USB hub: 0x04 -> Use port 3
+		 */
+		if (!(port_use & BIT(i))) {
+			debug("Skipping port %d\n", i + 1);
+			continue;
+		}
+
  		/*
  		 * Wait for (whichever finishes first)
  		 *  - A maximum of 10 seconds

@@ -534,6 +567,9 @@ static int usb_hub_configure(struct usb_device *dev)
  		if (portchange & USB_PORT_STAT_C_CONNECTION) {
  			debug("port %d connection change\n", i + 1);
  			usb_hub_port_connect_change(dev, i);
+
+			/* Save the port as active */
+			port_active |= (u64)(BIT(i)) << ((dev->devnum - 1) * 8);
  		}
  		if (portchange & USB_PORT_STAT_C_ENABLE) {
  			debug("port %d enable change, status %x\n",

@@ -578,6 +614,14 @@ static int usb_hub_configure(struct usb_device *dev)
  		}
  	} /* end for i all ports */
 
+	/*
+	 * Save the active ports in the "usb_port_detected" env variable.
+	 * This can be used by the user to save this into the "usb_port_use"
+	 * variable to only scan the statically available ports.
+	 */
+	sprintf(str, "%016llx", port_active);
+	setenv("usb_port_detected", str);
+
  	return 0;
  }
 

Hi,

Quoted 38 lines · Stefan Roese

This patch implements an optionally quasi static USB port configuration. This is done by using an environment variable, that describes the ports that shall be scanned at the next USB scans (usb start, usb reset).

The "usb_port_use" env variable is used to describe this static USB configuration. For this, each USB hub is represented by a 8-bit value. Making it possible to configure a maximum of 8 ports for each USB hub. A 64-bit representation is used, therefore 8 USB hubs can be described in total.

Here an example for this "usb_port_use" environment variable:

usb_port_use = 0000000000040301
-------------------------------
1st USB hub: 0x01 -> Use port 1 (first port)
2nd USB hub: 0x03 -> Use port 1 and 2
3rd USB hub: 0x04 -> Use port 3
4th USB hub: 0x00 -> Use no ports
...
8th USB hub: 0x00 -> Use no ports

To make this procedure of configuring this env variable less error prone and less painful, this patch also introduces another env variable that is dynamically generated at each USB scan: "usb_port_detected". This variable is similar to "usb_port_use". It has a bit enabled for each port that has been detected. This can be easily used on a new system, where the USB configuration is static in this way:

Run the USB scan (usb start, usb reset) without the "usb_port_use" variable set. This will result in all ports being scanned and the result written into the "usb_port_detected" variable. To configure the USB subsystem to only scan these specific USB ports upon the next scans, you only need to write the value from "usb_port_detected" into the "usb_port_use" variable:

=> setenv usb_port_use ${usb_port_detected}
=> saveenv

Quoted 8 lines

The next scans will only scan those enabled ports.

Its of course also possible to manually "tune" this env variable. If some ports are not needed in U-Boot, they can be disabled this way. This will result in less USB hub ports getting scanned and therefore in a faster USB scan time. Here an example:

With all USB devices enabled (usb_port_use not set):

This will fall apart when you get multiple root hubs,
if you want to do this (I believe there is much more low hanging fruit see my previous mails), you somehow need to describe the entire path to the hub in the env variable, currently if one hub gets removed, other hubs which are children of the same parent will get a different number in your hub-numbering scheme and things go awry, also what about hubs
on a second host controller, do those number on from the hub-numbering of the first hcd ? That seems vary fragile and will break when we add (semi) parallel scanning.

So NACK because this disallows later implementing parallel scanning without regressing this feature.