| Message ID | 1457625012-1268-3-git-send-email-sr@denx.de |
|---|---|
| State | Superseded |
| Delegated to: | Marek Vasut |
| Headers | show |
Hi, On 10-03-16 16:50, Stefan Roese 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.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. 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 <hdegoede@redhat.com> Regards, Hans > > 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 | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/common/usb_hub.c b/common/usb_hub.c > index 10fdd3c..660f4f4 100644 > --- a/common/usb_hub.c > +++ b/common/usb_hub.c > @@ -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: >
On 03/10/2016 08:50 AM, Stefan Roese 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.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. This patch, Tested-by: Stephen Warren <swarren@nvidia.com>
On 10.03.2016 19:55, Hans de Goede wrote: > Hi, > > On 10-03-16 16:50, Stefan Roese 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.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. > > 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 <hdegoede@redhat.com> Thanks. I'll remove the #if !defined(CONFIG_USB_FAST_SCAN) to make this change default in the next patchset version. Thanks, Stefan
On 10.03.2016 19:55, Hans de Goede wrote: > Hi, > > On 10-03-16 16:50, Stefan Roese 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.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. > > 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 <hdegoede@redhat.com> Thanks. Again, I'll make this change unconditionally in the next patchset version. Thanks, Stefan
diff --git a/common/usb_hub.c b/common/usb_hub.c index 10fdd3c..660f4f4 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -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:
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 <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 | 4 ++++ 1 file changed, 4 insertions(+)