From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Thu, 10 Mar 2016 20:12:17 +0100 Subject: [U-Boot] [PATCH 4/6] usb: usb_hub_power_on(): Use 100ms power-on delay instead of 1 sec (optionally) In-Reply-To: <1457625012-1268-5-git-send-email-sr@denx.de> References: <1457625012-1268-1-git-send-email-sr@denx.de> <1457625012-1268-5-git-send-email-sr@denx.de> Message-ID: <56E1C711.3080204@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi, On 10-03-16 16:50, Stefan Roese wrote: > 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. 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 :) Regards, Hans > > Signed-off-by: Stefan Roese > Cc: Simon Glass > Cc: Hans de Goede > Cc: Stephen Warren > Cc: Marek Vasut > --- > > common/usb_hub.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/usb_hub.c b/common/usb_hub.c > index 780291f..d5fcd27 100644 > --- a/common/usb_hub.c > +++ b/common/usb_hub.c > @@ -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) >