From acc27b9005fcef1f51c0dd7226faa5c101aa96df Mon Sep 17 00:00:00 2001 From: Michael Brown Date: Wed, 18 Mar 2015 16:43:18 +0000 Subject: [PATCH] [usb] Fix USB timeouts to match specification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Tested-by: Robin Smidsrød Signed-off-by: Michael Brown --- src/drivers/bus/usb.c | 6 ++++++ src/drivers/usb/xhci.h | 6 ++++-- src/include/ipxe/usb.h | 32 +++++++++++++++++++++++++++----- 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/drivers/bus/usb.c b/src/drivers/bus/usb.c index 548aa7b0..54a115ef 100644 --- 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 30320807..5abb5caa 100644 --- 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 e961f748..a9c185f6 100644 --- 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 100 +#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_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 {