This series reworks the timing of USB detection. Previously, the USB controllers would wait for the port power to stabilize and then immiediately check for device presence. Unfortunately, the USB specification (USB2 spec) allows for devices to wait up to 100ms from the time that power is stable to the time they assert their presence. This series changes the controllers to behave more like the existing usb-hub code - each port will repeatedly re-check for device presence for 100ms after power is stable.
This may also help with USB3 device detection. In particular there were reports of devices that start off identifying as USB2 and then later change to a USB3 type device. The additional polling may catch these cases.
This series could cause a slight increase in overall boot time (due to the additional 100ms polling). However, when SeaBIOS' threads are enabled, I think only a few unusual corner cases would cause higher boot times.
The series is a fairly significant change to the initial USB timing behaviour. I ran tests on a bunch of USB devices and did not see any regressions. More testing would be appreciated.
The series is also at: https://github.com/KevinOConnor/seabios/tree/testing
-Kevin
Kevin O'Connor (7): ehci: Move port power up from ehci_hub_detect() to check_ehci_ports(). usb-hub: Enable power to all ports prior to calling usb_enumerate(). xhci: Change xhci_hub_detect() to use connect status instead of link state. uhci: Repeatedly poll for device detect for 100ms. ohci: Repeatedly poll for device detect for 100ms. ehci: Stall uhci/ohci init only until default port routing is done. usb: Perform device detect polling on all usb controllers.
src/hw/usb-ehci.c | 58 +++++++++++++++++++++++++------------------------------ src/hw/usb-hub.c | 46 +++++++++++++++---------------------------- src/hw/usb-ohci.c | 8 +------- src/hw/usb-uhci.c | 7 +++---- src/hw/usb-xhci.c | 16 +++++---------- src/hw/usb.c | 21 ++++++++++++++------ src/hw/usb.h | 2 +- 7 files changed, 67 insertions(+), 91 deletions(-)
Don't perform port power up in the detect code. Instead do it prior to calling usb_enumerate(). This makes the code more similar to the ohci and xhci drivers. It can also reduce the total boot time when threads are disabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ehci.c | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-)
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index 76c76e0..4e4cc72 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -50,18 +50,6 @@ ehci_hub_detect(struct usbhub_s *hub, u32 port) u32 *portreg = &cntl->regs->portsc[port]; u32 portsc = readl(portreg);
- // Power up port. - if (!(portsc & PORT_POWER)) { - portsc |= PORT_POWER; - writel(portreg, portsc); - msleep(EHCI_TIME_POSTPOWER); - } else { - // Port is already powered up, but we don't know how long it - // has been powered up, so wait the 20ms. - msleep(EHCI_TIME_POSTPOWER); - } - portsc = readl(portreg); - if (!(portsc & PORT_CONNECT)) // No device present goto doneearly; @@ -135,7 +123,18 @@ static struct usbhub_op_s ehci_HubOp = { static int check_ehci_ports(struct usb_ehci_s *cntl) { - ASSERT32FLAT(); + // Power up ports. + int i; + for (i=0; i<cntl->checkports; i++) { + u32 *portreg = &cntl->regs->portsc[i]; + u32 portsc = readl(portreg); + if (!(portsc & PORT_POWER)) { + portsc |= PORT_POWER; + writel(portreg, portsc); + } + } + msleep(EHCI_TIME_POSTPOWER); + struct usbhub_s hub; memset(&hub, 0, sizeof(hub)); hub.cntl = &cntl->usb;
Don't perform port power up in the detect code. Instead do it prior to calling usb_enumerate(). This makes the code more similar to the usb root hub drivers. It can also reduce the total boot time when threads are disabled.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-hub.c | 22 ++++++++++++---------- src/hw/usb.h | 1 - 2 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/src/hw/usb-hub.c b/src/hw/usb-hub.c index 477518b..2a34e57 100644 --- a/src/hw/usb-hub.c +++ b/src/hw/usb-hub.c @@ -72,19 +72,11 @@ get_port_status(struct usbhub_s *hub, int port, struct usb_port_status *sts) static int usb_hub_detect(struct usbhub_s *hub, u32 port) { - // Turn on power to port. - int ret = set_port_feature(hub, port, USB_PORT_FEAT_POWER); - if (ret) - goto fail; - - // Wait for port power to stabilize. - msleep(hub->powerwait); - // Check periodically for a device connect. struct usb_port_status sts; u32 end = timer_calc(USB_TIME_SIGATT); for (;;) { - ret = get_port_status(hub, port, &sts); + int ret = get_port_status(hub, port, &sts); if (ret) goto fail; if (sts.wPortStatus & USB_PORT_STAT_CONNECTION) @@ -175,9 +167,19 @@ usb_hub_setup(struct usbdevice_s *usbdev) memset(&hub, 0, sizeof(hub)); hub.usbdev = usbdev; hub.cntl = usbdev->defpipe->cntl; - hub.powerwait = desc.bPwrOn2PwrGood * 2; hub.portcount = desc.bNbrPorts; hub.op = &HubOp; + + // Turn on power to ports. + int port; + for (port=0; port<desc.bNbrPorts; port++) { + ret = set_port_feature(&hub, port, USB_PORT_FEAT_POWER); + if (ret) + return ret; + } + // Wait for port power to stabilize. + msleep(desc.bPwrOn2PwrGood * 2); + usb_enumerate(&hub);
dprintf(1, "Initialized USB HUB (%d ports used)\n", hub.devcount); diff --git a/src/hw/usb.h b/src/hw/usb.h index 3a663ce..223e4d6 100644 --- a/src/hw/usb.h +++ b/src/hw/usb.h @@ -46,7 +46,6 @@ struct usbhub_s { struct usbdevice_s *usbdev; struct usb_s *cntl; struct mutex_s lock; - u32 powerwait; u32 port; u32 threads; u32 portcount;
Use the connect status bit to determine if a device is connected instead of the port link state state machine status. This makes the driver more similar to the other drivers and may help diagnose devices that take longer to boot up.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-xhci.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index 87e5e64..5f91d89 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -339,15 +339,7 @@ xhci_hub_detect(struct usbhub_s *hub, u32 port) { struct usb_xhci_s *xhci = container_of(hub->cntl, struct usb_xhci_s, usb); u32 portsc = readl(&xhci->pr[port].portsc); - - xhci_print_port_state(3, __func__, port, portsc); - switch (xhci_get_field(portsc, XHCI_PORTSC_PLS)) { - case PLS_U0: - case PLS_POLLING: - return 0; - default: - return -1; - } + return (portsc & XHCI_PORTSC_CCS) ? 0 : -1; }
// Reset device on port
According to the USB2 specification, a device may take up to 100ms (USB_TIME_SIGATT) after port power stabilizes to be detected. So, continually recheck for a device connection event.
Technically, the uhci root hub ports are always powered up, but it's not possible to know how long the machine has been on, so it's better to be safer here.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-uhci.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/hw/usb-uhci.c b/src/hw/usb-uhci.c index 9d7ef6a..406af90 100644 --- a/src/hw/usb-uhci.c +++ b/src/hw/usb-uhci.c @@ -44,10 +44,17 @@ uhci_hub_detect(struct usbhub_s *hub, u32 port) struct usb_uhci_s *cntl = container_of(hub->cntl, struct usb_uhci_s, usb); u16 ioport = cntl->iobase + USBPORTSC1 + port * 2;
- u16 status = inw(ioport); - if (!(status & USBPORTSC_CCS)) - // No device - return -1; + u32 end = timer_calc(USB_TIME_SIGATT); + for (;;) { + u16 status = inw(ioport); + if (status & USBPORTSC_CCS) + // Device connected. + break; + if (timer_check(end)) + // No device found. + return -1; + msleep(5); + }
// XXX - if just powered up, need to wait for USB_TIME_ATTDB?
According to the USB2 specification, a device may take up to 100ms (USB_TIME_SIGATT) after port power stabilizes to be detected. So, continually recheck for a device connection event.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ohci.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/hw/usb-ohci.c b/src/hw/usb-ohci.c index f566c4b..8089e13 100644 --- a/src/hw/usb-ohci.c +++ b/src/hw/usb-ohci.c @@ -42,10 +42,17 @@ static int ohci_hub_detect(struct usbhub_s *hub, u32 port) { struct usb_ohci_s *cntl = container_of(hub->cntl, struct usb_ohci_s, usb); - u32 sts = readl(&cntl->regs->roothub_portstatus[port]); - if (!(sts & RH_PS_CCS)) - // No device. - return -1; + u32 end = timer_calc(USB_TIME_SIGATT); + for (;;) { + u32 sts = readl(&cntl->regs->roothub_portstatus[port]); + if (sts & RH_PS_CCS) + // Device connected. + break; + if (timer_check(end)) + // No device found. + return -1; + msleep(5); + }
// XXX - need to wait for USB_TIME_ATTDB if just powered up?
Now that uhci and ohci will continually poll for a device connect, the ehci controller only needs to ensure that the default routing is setup properly before allowing uhci and ohci to be initialized.
This also fixes two error paths in configure_ehci() that were not properly updating PendingEHCIPorts.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ehci.c | 31 +++++++++++++------------------ 1 file changed, 13 insertions(+), 18 deletions(-)
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index 4e4cc72..86496ce 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -32,7 +32,7 @@ struct ehci_pipe { struct usb_pipe pipe; };
-static int PendingEHCIPorts; +static int PendingEHCI;
/**************************************************************** @@ -52,12 +52,12 @@ ehci_hub_detect(struct usbhub_s *hub, u32 port)
if (!(portsc & PORT_CONNECT)) // No device present - goto doneearly; + return -1;
if ((portsc & PORT_LINESTATUS_MASK) == PORT_LINESTATUS_KSTATE) { // low speed device writel(portreg, portsc | PORT_OWNER); - goto doneearly; + return -1; }
// XXX - if just powered up, need to wait for USB_TIME_ATTDB? @@ -67,10 +67,6 @@ ehci_hub_detect(struct usbhub_s *hub, u32 port) writel(portreg, portsc); msleep(USB_TIME_DRSTR); return 0; - -doneearly: - PendingEHCIPorts--; - return -1; }
// Reset device on port @@ -86,21 +82,17 @@ ehci_hub_reset(struct usbhub_s *hub, u32 port) writel(portreg, portsc); msleep(EHCI_TIME_POSTRESET);
- int rv = -1; portsc = readl(portreg); if (!(portsc & PORT_CONNECT)) // No longer connected - goto resetfail; + return -1; if (!(portsc & PORT_PE)) { // full speed device writel(portreg, portsc | PORT_OWNER); - goto resetfail; + return -1; }
- rv = USB_HIGHSPEED; -resetfail: - PendingEHCIPorts--; - return rv; + return USB_HIGHSPEED; }
// Disable port @@ -230,6 +222,7 @@ configure_ehci(void *data) struct ehci_qh *async_qh = memalign_high(EHCI_QH_ALIGN, sizeof(*async_qh)); if (!fl || !intr_qh || !async_qh) { warn_noalloc(); + PendingEHCI--; goto fail; }
@@ -245,6 +238,7 @@ configure_ehci(void *data) break; if (timer_check(end)) { warn_timeout(); + PendingEHCI--; goto fail; } yield(); @@ -278,6 +272,7 @@ configure_ehci(void *data)
// Set default of high speed for root hub. writel(&cntl->regs->configflag, 1); + PendingEHCI--;
// Find devices int count = check_ehci_ports(cntl); @@ -318,7 +313,7 @@ ehci_controller_setup(struct pci_device *pci) cntl->regs = (void*)caps + readb(&caps->caplength); if (hcc_params & HCC_64BIT_ADDR) cntl->regs->ctrldssegment = 0; - PendingEHCIPorts += cntl->checkports; + PendingEHCI++;
dprintf(1, "EHCI init on dev %02x:%02x.%x (regs=%p)\n" , pci_bdf_to_bus(bdf), pci_bdf_to_dev(bdf) @@ -342,9 +337,9 @@ ehci_setup(void) ehci_controller_setup(pci); }
- // Wait for all EHCI ports to initialize. This forces OHCI/UHCI - // setup to always be after any EHCI ports are set to low speed. - while (PendingEHCIPorts) + // Wait for all EHCI controllers to initialize. This forces OHCI/UHCI + // setup to always be after any EHCI ports are routed to EHCI. + while (PendingEHCI) yield(); }
Move the 100ms (USB_TIME_SIGATT) device detect polling from the ohci/uhci/usb-hub code to the generic usb_hub_port_setup() code. This extends the 100ms polling to ehci and xhci controllers. The code in usb_hub_port_setup() now compares USB_TIME_SIGATT to the start of usb_enumerate(), which may make boots faster when threads are disabled.
This patch also changes the meaning of the return code for hub->op->detect() calls. Now 1 indicates device found, 0 indicates device not found, and -1 indicates permanent failure.
Also, the xhci controller generic delay of 100ms is replaced with a 20ms root hub power stabilize time. This in combination with the 100ms for USB_TIME_SIGATT should be closer to the USB2 spec (the USB3 spec does not seem to declare an equivalent of USB_TIME_SIGATT).
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ehci.c | 4 ++-- src/hw/usb-hub.c | 26 +++++--------------------- src/hw/usb-ohci.c | 17 ++--------------- src/hw/usb-uhci.c | 18 +++++------------- src/hw/usb-xhci.c | 8 +++++--- src/hw/usb.c | 21 +++++++++++++++------ src/hw/usb.h | 1 + 7 files changed, 35 insertions(+), 60 deletions(-)
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index 86496ce..20e387b 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -52,7 +52,7 @@ ehci_hub_detect(struct usbhub_s *hub, u32 port)
if (!(portsc & PORT_CONNECT)) // No device present - return -1; + return 0;
if ((portsc & PORT_LINESTATUS_MASK) == PORT_LINESTATUS_KSTATE) { // low speed device @@ -66,7 +66,7 @@ ehci_hub_detect(struct usbhub_s *hub, u32 port) portsc = (portsc & ~PORT_PE) | PORT_RESET; writel(portreg, portsc); msleep(USB_TIME_DRSTR); - return 0; + return 1; }
// Reset device on port diff --git a/src/hw/usb-hub.c b/src/hw/usb-hub.c index 2a34e57..4731a91 100644 --- a/src/hw/usb-hub.c +++ b/src/hw/usb-hub.c @@ -72,29 +72,13 @@ get_port_status(struct usbhub_s *hub, int port, struct usb_port_status *sts) static int usb_hub_detect(struct usbhub_s *hub, u32 port) { - // Check periodically for a device connect. struct usb_port_status sts; - u32 end = timer_calc(USB_TIME_SIGATT); - for (;;) { - int ret = get_port_status(hub, port, &sts); - if (ret) - goto fail; - if (sts.wPortStatus & USB_PORT_STAT_CONNECTION) - // Device connected. - break; - if (timer_check(end)) - // No device found. - return -1; - msleep(5); + int ret = get_port_status(hub, port, &sts); + if (ret) { + dprintf(1, "Failure on hub port %d detect\n", port); + return -1; } - - // XXX - wait USB_TIME_ATTDB time? - - return 0; - -fail: - dprintf(1, "Failure on hub port %d detect\n", port); - return -1; + return (sts.wPortStatus & USB_PORT_STAT_CONNECTION) ? 1 : 0; }
// Disable port diff --git a/src/hw/usb-ohci.c b/src/hw/usb-ohci.c index 8089e13..7a22057 100644 --- a/src/hw/usb-ohci.c +++ b/src/hw/usb-ohci.c @@ -42,21 +42,8 @@ static int ohci_hub_detect(struct usbhub_s *hub, u32 port) { struct usb_ohci_s *cntl = container_of(hub->cntl, struct usb_ohci_s, usb); - u32 end = timer_calc(USB_TIME_SIGATT); - for (;;) { - u32 sts = readl(&cntl->regs->roothub_portstatus[port]); - if (sts & RH_PS_CCS) - // Device connected. - break; - if (timer_check(end)) - // No device found. - return -1; - msleep(5); - } - - // XXX - need to wait for USB_TIME_ATTDB if just powered up? - - return 0; + u32 sts = readl(&cntl->regs->roothub_portstatus[port]); + return (sts & RH_PS_CCS) ? 1 : 0; }
// Disable port diff --git a/src/hw/usb-uhci.c b/src/hw/usb-uhci.c index 406af90..a34f6db 100644 --- a/src/hw/usb-uhci.c +++ b/src/hw/usb-uhci.c @@ -43,25 +43,17 @@ uhci_hub_detect(struct usbhub_s *hub, u32 port) { struct usb_uhci_s *cntl = container_of(hub->cntl, struct usb_uhci_s, usb); u16 ioport = cntl->iobase + USBPORTSC1 + port * 2; - - u32 end = timer_calc(USB_TIME_SIGATT); - for (;;) { - u16 status = inw(ioport); - if (status & USBPORTSC_CCS) - // Device connected. - break; - if (timer_check(end)) - // No device found. - return -1; - msleep(5); - } + u16 status = inw(ioport); + if (!(status & USBPORTSC_CCS)) + // No device found. + return 0;
// XXX - if just powered up, need to wait for USB_TIME_ATTDB?
// Begin reset on port outw(USBPORTSC_PR, ioport); msleep(USB_TIME_DRSTR); - return 0; + return 1; }
// Reset device on port diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index 5f91d89..eafa4cb 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -320,6 +320,8 @@ static int wait_bit(u32 *reg, u32 mask, int value, u32 timeout) * Root hub ****************************************************************/
+#define XHCI_TIME_POSTPOWER 20 + // Check if device attached to port static void xhci_print_port_state(int loglevel, const char *prefix, u32 port, u32 portsc) @@ -339,7 +341,7 @@ xhci_hub_detect(struct usbhub_s *hub, u32 port) { struct usb_xhci_s *xhci = container_of(hub->cntl, struct usb_xhci_s, usb); u32 portsc = readl(&xhci->pr[port].portsc); - return (portsc & XHCI_PORTSC_CCS) ? 0 : -1; + return (portsc & XHCI_PORTSC_CCS) ? 1 : 0; }
// Reset device on port @@ -388,8 +390,8 @@ static struct usbhub_op_s xhci_hub_ops = { static int xhci_check_ports(struct usb_xhci_s *xhci) { - // FIXME: try find a more elegant way than a fixed delay - msleep(100); + // Wait for port power to stabilize. + msleep(XHCI_TIME_POSTPOWER);
struct usbhub_s hub; memset(&hub, 0, sizeof(hub)); diff --git a/src/hw/usb.c b/src/hw/usb.c index 930b5d9..153e153 100644 --- a/src/hw/usb.c +++ b/src/hw/usb.c @@ -404,15 +404,23 @@ usb_hub_port_setup(void *data) struct usbhub_s *hub = usbdev->hub; u32 port = usbdev->port;
- // Detect if device present (and possibly start reset) - int ret = hub->op->detect(hub, port); - if (ret) - // No device present - goto done; + for (;;) { + // Detect if device present (and possibly start reset) + int ret = hub->op->detect(hub, port); + if (ret > 0) + // Device connected. + break; + if (ret < 0 || timer_check(hub->detectend)) + // No device found. + goto done; + msleep(5); + } + + // XXX - wait USB_TIME_ATTDB time?
// Reset port and determine device speed mutex_lock(&hub->cntl->resetlock); - ret = hub->op->reset(hub, port); + int ret = hub->op->reset(hub, port); if (ret < 0) // Reset failed goto resetfail; @@ -447,6 +455,7 @@ usb_enumerate(struct usbhub_s *hub) { u32 portcount = hub->portcount; hub->threads = portcount; + hub->detectend = timer_calc(USB_TIME_SIGATT);
// Launch a thread for every port. int i; diff --git a/src/hw/usb.h b/src/hw/usb.h index 223e4d6..fe80ea6 100644 --- a/src/hw/usb.h +++ b/src/hw/usb.h @@ -46,6 +46,7 @@ struct usbhub_s { struct usbdevice_s *usbdev; struct usb_s *cntl; struct mutex_s lock; + u32 detectend; u32 port; u32 threads; u32 portcount;
no regressions using your testing branch on panther, with a variety of USB2/USB3 flash devices connected to an xHCI controller.
On 9/10/2014 1:45 PM, Kevin O'Connor wrote:
This series reworks the timing of USB detection. Previously, the USB controllers would wait for the port power to stabilize and then immiediately check for device presence. Unfortunately, the USB specification (USB2 spec) allows for devices to wait up to 100ms from the time that power is stable to the time they assert their presence. This series changes the controllers to behave more like the existing usb-hub code - each port will repeatedly re-check for device presence for 100ms after power is stable.
This may also help with USB3 device detection. In particular there were reports of devices that start off identifying as USB2 and then later change to a USB3 type device. The additional polling may catch these cases.
This series could cause a slight increase in overall boot time (due to the additional 100ms polling). However, when SeaBIOS' threads are enabled, I think only a few unusual corner cases would cause higher boot times.
The series is a fairly significant change to the initial USB timing behaviour. I ran tests on a bunch of USB devices and did not see any regressions. More testing would be appreciated.
The series is also at: https://github.com/KevinOConnor/seabios/tree/testing
-Kevin
Kevin O'Connor (7): ehci: Move port power up from ehci_hub_detect() to check_ehci_ports(). usb-hub: Enable power to all ports prior to calling usb_enumerate(). xhci: Change xhci_hub_detect() to use connect status instead of link state. uhci: Repeatedly poll for device detect for 100ms. ohci: Repeatedly poll for device detect for 100ms. ehci: Stall uhci/ohci init only until default port routing is done. usb: Perform device detect polling on all usb controllers.
src/hw/usb-ehci.c | 58 +++++++++++++++++++++++++------------------------------ src/hw/usb-hub.c | 46 +++++++++++++++---------------------------- src/hw/usb-ohci.c | 8 +------- src/hw/usb-uhci.c | 7 +++---- src/hw/usb-xhci.c | 16 +++++---------- src/hw/usb.c | 21 ++++++++++++++------ src/hw/usb.h | 2 +- 7 files changed, 67 insertions(+), 91 deletions(-)
-
Kevin O'Connor -
Matt DeVillier