[usb] Fix USB timeouts to match specification

Several of the USB timeouts were chosen on the principle of "pick an
arbitrary but ridiculously large value, just to be safe".  It turns
out that some of the timeouts permitted by the USB specification are
even larger: for example, control transactions are allowed to take up
to five seconds to complete.

Fix up these USB timeout values to match those found in the USB2
specification.

Debugged-by: Robin Smidsrød <robin@smidsrod.no>
Tested-by: Robin Smidsrød <robin@smidsrod.no>
Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c
index 548aa7b08ef50099c34c418be45ca526239bf84c..54a115efc7e8cf7c8f7369f4fe317e7ca6eef1d0 100644 (file)
--- a/src/drivers/bus/usb.c
+++ b/src/drivers/bus/usb.c
@@ -1281,6 +1281,9 @@ static int register_usb ( struct usb_device *usb ) {
                goto err_enable;
        }
 
+       /* Allow recovery interval since port may have been reset */
+       mdelay ( USB_RESET_RECOVER_DELAY_MS );
+
        /* Get device speed */
        if ( ( rc = hub->driver->speed ( hub, port ) ) != 0 ) {
                DBGC ( hub, "USB hub %s port %d could not get speed: %s\n",
@@ -1316,6 +1319,9 @@ static int register_usb ( struct usb_device *usb ) {
        }
        DBGC2 ( usb, "USB %s assigned address %d\n", usb->name, usb->address );
 
+       /* Allow recovery interval after Set Address command */
+       mdelay ( USB_SET_ADDRESS_RECOVER_DELAY_MS );
+
        /* Read first part of device descriptor to get EP0 MTU */
        if ( ( rc = usb_get_mtu ( usb, &usb->device ) ) != 0 ) {
                DBGC ( usb, "USB %s could not get MTU: %s\n",

diff --git a/src/drivers/usb/xhci.h b/src/drivers/usb/xhci.h
index 3032080715bac680757202c1412556f44f652f95..5abb5caaf59a057f18703e8ff53380905773fffa 100644 (file)
--- a/src/drivers/usb/xhci.h
+++ b/src/drivers/usb/xhci.h
@@ -991,9 +991,11 @@ xhci_ring_consumed ( struct xhci_trb_ring *ring ) {
 
 /** Maximum time to wait for a command to complete
  *
- * This is a policy decision.
+ * The "address device" command involves waiting for a response to a
+ * USB control transaction, and so we must wait for up to the 5000ms
+ * that USB allows for devices to respond to control transactions.
  */
-#define XHCI_COMMAND_MAX_WAIT_MS 500
+#define XHCI_COMMAND_MAX_WAIT_MS USB_CONTROL_MAX_WAIT_MS
 
 /** Time to delay after aborting a command
  *

diff --git a/src/include/ipxe/usb.h b/src/include/ipxe/usb.h
index e961f74802312f6b5a0e8b484f2bdb683696f1ad..a9c185f6c82e3221d42e0ee12fd642a68e734365 100644 (file)
--- a/src/include/ipxe/usb.h
+++ b/src/include/ipxe/usb.h
@@ -1178,20 +1178,42 @@ extern unsigned int usb_route_string ( struct usb_device *usb );
 extern unsigned int usb_depth ( struct usb_device *usb );
 extern struct usb_port * usb_root_hub_port ( struct usb_device *usb );
 
-/** Minimum reset time */
+/** Minimum reset time
+ *
+ * Section 7.1.7.5 of the USB2 specification states that root hub
+ * ports should assert reset signalling for at least 50ms.
+ */
 #define USB_RESET_DELAY_MS 50
 
+/** Reset recovery time
+ *
+ * Section 9.2.6.2 of the USB2 specification states that the
+ * "recovery" interval after a port reset is 10ms.
+ */
+#define USB_RESET_RECOVER_DELAY_MS 10
+
 /** Maximum time to wait for a control transaction to complete
  *
- * This is a policy decision.
+ * Section 9.2.6.1 of the USB2 specification states that the upper
+ * limit for commands to be processed is 5 seconds.
+ */
+#define USB_CONTROL_MAX_WAIT_MS 5000
+
+/** Set address recovery time
+ *
+ * Section 9.2.6.3 of the USB2 specification states that devices are
+ * allowed a 2ms recovery interval after receiving a new address.
  */
-#define USB_CONTROL_MAX_WAIT_MS 100
+#define USB_SET_ADDRESS_RECOVER_DELAY_MS 2
 
 /** Time to wait for ports to stabilise
  *
- * This is a policy decision.
+ * Section 7.1.7.3 of the USB specification states that we must allow
+ * 100ms for devices to signal attachment, and an additional 100ms for
+ * connection debouncing.  (This delay is parallelised across all
+ * ports on a hub; we do not delay separately for each port.)
  */
-#define USB_PORT_DELAY_MS 100
+#define USB_PORT_DELAY_MS 200
 
 /** A USB device ID */
 struct usb_device_id {