The SeaBIOS USB drivers all have different timeouts for USB commands. The USB specification does specify maximum command times (section 9.2.6). In particular, it states that all commands are expected to complete within 5 seconds.
This series unifies the command timeouts across the drivers. Generally, the timeout is increased to 5 seconds for all commands. This fixes an issue reported by some users of USB flash drives on XHCI controllers (the xhci controller had a 1 second bulk timeout).
The control command timeouts are also increased to 5 seconds except for the set_address command. This is reduced to 150ms (spec caps device to 50ms) to prevent 5 second hangs should a port report a device that isn't actually there. Once the driver starts talking to a device it would be very unusual to timeout on a command, so there shouldn't be any problem with a limit of five seconds.
-Kevin
Kevin O'Connor (4): xhci: Update the times for usb command timeouts. ehci: Update usb command timeouts to use usb_xfer_time() uhci: Update usb command timeouts to use usb_xfer_time() ohci: Update usb command timeouts to use usb_xfer_time()
src/hw/usb-ehci.c | 11 ++++++----- src/hw/usb-ohci.c | 7 +++---- src/hw/usb-uhci.c | 13 ++++++------- src/hw/usb-xhci.c | 4 ++-- src/hw/usb.c | 13 +++++++++++++ src/hw/usb.h | 5 +++++ 6 files changed, 35 insertions(+), 18 deletions(-)
The xhci controller had a hardcoded 1 second timeout for both bulk and control transfers. The 1 second bulk timeout is too small for some real devices.
Increase both times to 5.1 seconds - according to the USB spec, the maximum time a command should take is 5 seconds. However, have the set_address command only wait for 150ms (spec says set_address should take no more than 50ms).
Introduce usb_xfer_time() to calculate maximum command time.
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-xhci.c | 4 ++-- src/hw/usb.c | 13 +++++++++++++ src/hw/usb.h | 5 +++++ 3 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/src/hw/usb-xhci.c b/src/hw/usb-xhci.c index ca1fe25..e3052a5 100644 --- a/src/hw/usb-xhci.c +++ b/src/hw/usb-xhci.c @@ -1042,7 +1042,7 @@ xhci_control(struct usb_pipe *p, int dir, const void *cmd, int cmdsize xhci_xfer_data(pipe, dir, data, datalen); xhci_xfer_status(pipe, dir, datalen);
- int cc = xhci_event_wait(xhci, &pipe->reqs, 1000); + int cc = xhci_event_wait(xhci, &pipe->reqs, usb_xfer_time(p, datalen)); if (cc != CC_SUCCESS) { dprintf(1, "%s: control xfer failed (cc %d)\n", __func__, cc); return -1; @@ -1062,7 +1062,7 @@ xhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datalen) GET_LOWFLAT(pipe->pipe.cntl), struct usb_xhci_s, usb);
xhci_xfer_normal(pipe, data, datalen); - int cc = xhci_event_wait(xhci, &pipe->reqs, 1000); + int cc = xhci_event_wait(xhci, &pipe->reqs, usb_xfer_time(p, datalen)); if (cc != CC_SUCCESS) { dprintf(1, "%s: bulk xfer failed (cc %d)\n", __func__, cc); return -1; diff --git a/src/hw/usb.c b/src/hw/usb.c index 7b8a9f5..46b46be 100644 --- a/src/hw/usb.c +++ b/src/hw/usb.c @@ -187,6 +187,19 @@ usb_getFrameExp(struct usbdevice_s *usbdev return (period <= 4) ? 0 : period - 4; }
+// Maximum time (in ms) a data transfer should take +int +usb_xfer_time(struct usb_pipe *pipe, int datalen) +{ + // Use the maximum command time (5 seconds), except for + // set_address commands where we don't want to stall the boot if + // the device doesn't actually exist. Add 100ms to account for + // any controller delays. + if (!pipe->devaddr) + return USB_TIME_STATUS + 100; + return USB_TIME_COMMAND + 100; +} + // Find the first endpoing of a given type in an interface description. struct usb_endpoint_descriptor * findEndPointDesc(struct usbdevice_s *usbdev, int type, int dir) diff --git a/src/hw/usb.h b/src/hw/usb.h index 6196296..3a663ce 100644 --- a/src/hw/usb.h +++ b/src/hw/usb.h @@ -84,6 +84,10 @@ struct usbhub_op_s { #define USB_TIME_DRSTR 50 #define USB_TIME_RSTRCY 10
+#define USB_TIME_STATUS 50 +#define USB_TIME_DATAIN 500 +#define USB_TIME_COMMAND 5000 + #define USB_TIME_SETADDR_RECOVERY 2
#define USB_PID_OUT 0xe1 @@ -237,6 +241,7 @@ void usb_desc2pipe(struct usb_pipe *pipe, struct usbdevice_s *usbdev , struct usb_endpoint_descriptor *epdesc); int usb_getFrameExp(struct usbdevice_s *usbdev , struct usb_endpoint_descriptor *epdesc); +int usb_xfer_time(struct usb_pipe *pipe, int datalen); struct usb_endpoint_descriptor *findEndPointDesc(struct usbdevice_s *usbdev , int type, int dir); void usb_enumerate(struct usbhub_s *hub);
Kevin,
I still have issues with various USB sticks not being detected at boot due to the xhci_event_wait() call in xhci_cmd_submit() which still has a timeout of 1000ms (vs 5000 for the ones affected by this patch). Manually setting it to 5000 fixes the issues with all the USB flash devices I've tested (which had a problem at 1000ms)
-Matt
On Sat, Jun 21, 2014 at 12:20:21PM -0500, Matt DeVillier wrote:
Kevin,
I still have issues with various USB sticks not being detected at boot due to the xhci_event_wait() call in xhci_cmd_submit() which still has a timeout of 1000ms (vs 5000 for the ones affected by this patch). Manually setting it to 5000 fixes the issues with all the USB flash devices I've tested (which had a problem at 1000ms)
Thanks Matt. Can you post the SeaBIOS log with one of these devices?
-Kevin
On 6/21/2014 1:08 PM, Kevin O'Connor wrote:
On Sat, Jun 21, 2014 at 12:20:21PM -0500, Matt DeVillier wrote:
Kevin,
I still have issues with various USB sticks not being detected at boot due to the xhci_event_wait() call in xhci_cmd_submit() which still has a timeout of 1000ms (vs 5000 for the ones affected by this patch). Manually setting it to 5000 fixes the issues with all the USB flash devices I've tested (which had a problem at 1000ms)
Thanks Matt. Can you post the SeaBIOS log with one of these devices?
-Kevin
Kevin,
attached are outputs of for a USB3 flash drive which needed the extra wait time, as well as an older Logitech wired keyboard which never successfully initializes regardless of timeout setting.
USB3 flash drive:
Bus 002 Device 002: ID 13fe:5500 Kingston Technology Company Inc. Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 3.00 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 9 idVendor 0x13fe Kingston Technology Company Inc. idProduct 0x5500 bcdDevice 1.00 iManufacturer 1 UFD 3.0 iProduct 2 Silicon-Power16G iSerial 3 14008120135E60044176F2DFD23 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 44 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 126mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk-Only iInterface 0 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 3 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x02 EP 2 OUT bmAttributes 2 Transfer Type Bulk Synch Type None Usage Type Data wMaxPacketSize 0x0400 1x 1024 bytes bInterval 0 bMaxBurst 3 Binary Object Store Descriptor: bLength 5 bDescriptorType 15 wTotalLength 22 bNumDeviceCaps 2 USB 2.0 Extension Device Capability: bLength 7 bDescriptorType 16 bDevCapabilityType 2 bmAttributes 0x00000002 Link Power Management (LPM) Supported SuperSpeed USB Device Capability: bLength 10 bDescriptorType 16 bDevCapabilityType 3 bmAttributes 0x00 wSpeedsSupported 0x000e Device can operate at Full Speed (12Mbps) Device can operate at High Speed (480Mbps) Device can operate at SuperSpeed (5Gbps) bFunctionalitySupport 2 Lowest fully-functional device speed is High Speed (480Mbps) bU1DevExitLat 10 micro seconds bU2DevExitLat 2047 micro seconds Device Status: 0x000c (Bus Powered) U1 Enabled U2 Enabled
/: Bus 02.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/4p, 5000M |__ Port 2: Dev 2, If 0, Class=Mass Storage, Driver=usb-storage, 5000M
keyboard:
Bus 001 Device 005: ID 046d:c30b Logitech, Inc. NetPlay Keyboard Device Descriptor: bLength 18 bDescriptorType 1 bcdUSB 1.10 bDeviceClass 0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize0 8 idVendor 0x046d Logitech, Inc. idProduct 0xc30b NetPlay Keyboard bcdDevice 1.33 iManufacturer 1 Logitech iProduct 2 NetPlay Keyboard iSerial 0 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 34 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 100mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber 0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 1 Boot Interface Subclass bInterfaceProtocol 1 Keyboard iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType 33 bcdHID 1.10 bCountryCode 0 Not supported bNumDescriptors 1 bDescriptorType 34 Report wDescriptorLength 65 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes 3 Transfer Type Interrupt Synch Type None Usage Type Data wMaxPacketSize 0x0008 1x 8 bytes bInterval 24 Device Status: 0x0000 (Bus Powered)
/: Bus 01.Port 1: Dev 1, Class=root_hub, Driver=xhci_hcd/9p, 480M |__ Port 3: Dev 5, If 0, Class=Human Interface Device, Driver=usbhid, 1.5M
regards, Matt
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ehci.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/src/hw/usb-ehci.c b/src/hw/usb-ehci.c index 9d9427b..76c76e0 100644 --- a/src/hw/usb-ehci.c +++ b/src/hw/usb-ehci.c @@ -503,9 +503,8 @@ ehci_reset_pipe(struct ehci_pipe *pipe) }
static int -ehci_wait_td(struct ehci_pipe *pipe, struct ehci_qtd *td, int timeout) +ehci_wait_td(struct ehci_pipe *pipe, struct ehci_qtd *td, u32 end) { - u32 end = timer_calc(timeout); u32 status; for (;;) { status = td->token; @@ -604,12 +603,13 @@ ehci_control(struct usb_pipe *p, int dir, const void *cmd, int cmdsize | (dir ? QTD_PID_OUT : QTD_PID_IN) | ehci_maxerr(3));
// Transfer data + u32 end = timer_calc(usb_xfer_time(p, datasize)); barrier(); pipe->qh.qtd_next = (u32)tds; int i, ret=0; for (i=0; i<3; i++) { struct ehci_qtd *td = &tds[i]; - ret = ehci_wait_td(pipe, td, 500); + ret = ehci_wait_td(pipe, td, end); if (ret) break; } @@ -629,6 +629,7 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize) , &pipe->qh, dir, data, datasize);
// Allocate 4 tds on stack (with required alignment) + u32 end = timer_calc(usb_xfer_time(p, datasize)); u8 tdsbuf[sizeof(struct ehci_qtd) * STACKQTDS + EHCI_QTD_ALIGN - 1]; struct ehci_qtd *tds = (void*)ALIGN((u32)tdsbuf, EHCI_QTD_ALIGN); memset(tds, 0, sizeof(*tds) * STACKQTDS); @@ -639,7 +640,7 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize) int tdpos = 0; while (datasize) { struct ehci_qtd *td = &tds[tdpos++ % STACKQTDS]; - int ret = ehci_wait_td(pipe, td, 5000); + int ret = ehci_wait_td(pipe, td, end); if (ret) return -1;
@@ -659,7 +660,7 @@ ehci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize) int i; for (i=0; i<STACKQTDS; i++) { struct ehci_qtd *td = &tds[tdpos++ % STACKQTDS]; - int ret = ehci_wait_td(pipe, td, 5000); + int ret = ehci_wait_td(pipe, td, end); if (ret) return -1; }
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-uhci.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/hw/usb-uhci.c b/src/hw/usb-uhci.c index 2321e21..9d7ef6a 100644 --- a/src/hw/usb-uhci.c +++ b/src/hw/usb-uhci.c @@ -398,9 +398,8 @@ uhci_alloc_pipe(struct usbdevice_s *usbdev }
static int -wait_pipe(struct uhci_pipe *pipe, int timeout) +wait_pipe(struct uhci_pipe *pipe, u32 end) { - u32 end = timer_calc(timeout); for (;;) { u32 el_link = GET_LOWFLAT(pipe->qh.element); if (el_link & UHCI_PTR_TERM) @@ -422,9 +421,8 @@ wait_pipe(struct uhci_pipe *pipe, int timeout) }
static int -wait_td(struct uhci_td *td) +wait_td(struct uhci_td *td, u32 end) { - u32 end = timer_calc(5000); // XXX - lookup real time. u32 status; for (;;) { status = td->status; @@ -495,7 +493,7 @@ uhci_control(struct usb_pipe *p, int dir, const void *cmd, int cmdsize // Transfer data barrier(); pipe->qh.element = (u32)&tds[0]; - int ret = wait_pipe(pipe, 500); + int ret = wait_pipe(pipe, timer_calc(usb_xfer_time(p, datasize))); free(tds); return ret; } @@ -523,13 +521,14 @@ uhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize) memset(tds, 0, sizeof(*tds) * STACKTDS);
// Enable tds + u32 end = timer_calc(usb_xfer_time(p, datasize)); barrier(); SET_LOWFLAT(pipe->qh.element, (u32)MAKE_FLATPTR(GET_SEG(SS), tds));
int tdpos = 0; while (datasize) { struct uhci_td *td = &tds[tdpos++ % STACKTDS]; - int ret = wait_td(td); + int ret = wait_td(td, end); if (ret) goto fail;
@@ -552,7 +551,7 @@ uhci_send_bulk(struct usb_pipe *p, int dir, void *data, int datasize) datasize -= transfer; } SET_LOWFLAT(pipe->toggle, !!toggle); - return wait_pipe(pipe, 5000); + return wait_pipe(pipe, end); fail: dprintf(1, "uhci_send_bulk failed\n"); SET_LOWFLAT(pipe->qh.element, UHCI_PTR_TERM);
Signed-off-by: Kevin O'Connor kevin@koconnor.net --- src/hw/usb-ohci.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/hw/usb-ohci.c b/src/hw/usb-ohci.c index d55b64a..f566c4b 100644 --- a/src/hw/usb-ohci.c +++ b/src/hw/usb-ohci.c @@ -434,10 +434,9 @@ ohci_alloc_pipe(struct usbdevice_s *usbdev }
static int -wait_ed(struct ohci_ed *ed) +wait_ed(struct ohci_ed *ed, int timeout) { - // XXX - 500ms just a guess - u32 end = timer_calc(500); + u32 end = timer_calc(timeout); for (;;) { if (ed->hwHeadP == ed->hwTailP) return 0; @@ -497,7 +496,7 @@ ohci_control(struct usb_pipe *p, int dir, const void *cmd, int cmdsize pipe->ed.hwINFO &= ~ED_SKIP; writel(&cntl->regs->cmdstatus, OHCI_CLF);
- int ret = wait_ed(&pipe->ed); + int ret = wait_ed(&pipe->ed, usb_xfer_time(p, datasize)); pipe->ed.hwINFO |= ED_SKIP; if (ret) ohci_waittick(cntl);
On Sat, Jun 14, 2014 at 02:08:49PM -0400, Kevin O'Connor wrote:
The SeaBIOS USB drivers all have different timeouts for USB commands. The USB specification does specify maximum command times (section 9.2.6). In particular, it states that all commands are expected to complete within 5 seconds.
This series unifies the command timeouts across the drivers. Generally, the timeout is increased to 5 seconds for all commands. This fixes an issue reported by some users of USB flash drives on XHCI controllers (the xhci controller had a 1 second bulk timeout).
The control command timeouts are also increased to 5 seconds except for the set_address command. This is reduced to 150ms (spec caps device to 50ms) to prevent 5 second hangs should a port report a device that isn't actually there. Once the driver starts talking to a device it would be very unusual to timeout on a command, so there shouldn't be any problem with a limit of five seconds.
FYI, I have pushed this series.
-Kevin
-
Kevin O'Connor -
Matt DeVillier