From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefan Roese Date: Fri, 11 Mar 2016 11:42:24 +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: <56E29EB3.2060401@redhat.com> References: <1457625012-1268-1-git-send-email-sr@denx.de> <1457625012-1268-5-git-send-email-sr@denx.de> <56E1C711.3080204@redhat.com> <56E29A39.6000802@denx.de> <56E29EB3.2060401@redhat.com> Message-ID: <56E2A110.4090907@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Hans, On 11.03.2016 11:32, Hans de Goede wrote: > Hi, > > On 11-03-16 11:13, Stefan Roese wrote: >> Hi Hans, >> >> On 10.03.2016 20:12, Hans de Goede wrote: >>> 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 >> >> 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. Okay. I'll change it to include this initial delay here as well. >> 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). Right. > Nice improvement in scan time from this simple patch btw :) Yep! I was also pretty impressed by this. :) Thanks, Stefan