[U-Boot] [PATCH 6/6] usb: Implement static USB port configuration to speed up USB scanning

Hans de Goede
2016-03-10T19:18:00Z

Hi,

On 10-03-16 16:50, Stefan Roese wrote:
> 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):

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.

Regards,

Hans

>
> => 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 <sr@denx.de>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Stephen Warren <swarren@nvidia.com>
> Cc: Marek Vasut <marex@denx.de>
>
> ---
>
> common/usb_hub.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 44 insertions(+)
>
> diff --git a/common/usb_hub.c b/common/usb_hub.c
> index d5fcd27..b600cfa 100644
> --- a/common/usb_hub.c
> +++ b/common/usb_hub.c
> @@ -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;
> }
>
>