I am sending this to the list looking for comments. Testing has been
confined to just a few USB sticks - no USB hard drives, USB card
readers, etc. But there are reports on the mailing list of problems
with U-Boot's detection of USB mass storage devices.
We have had a lot of problems with different USB sticks, specifically
the faster bulk storage devices. Two of these sticks have
a problem where they do not respond to a USB descriptor submission
within the allowed U-Boot timeout. This seems to only happen when a
REQUEST SENSE is submitted immediately after a TEST UNIT READY fails.
There is also some oddness with reset, where some devices need more
than one reset. This appears to be due to faulty reset code in U-Boot.
USB sticks where this this problem has been noticed are:
ID 13fe:3800 Kingston Technology Company Inc.
ID 0930:6545 Toshiba Corp. Kingston DataTraveler 2.0 Stick (4GB)
/ PNY Attache 4GB Stick
This problem has been reproduced on the Tegra 20 Seaboard, but it is
TBD whether it happens on other ARM platforms with HS USB also.
This patch:
1. Increases the USB descriptor submission timeout to 3 seconds since the
original 1 second timeout seems arbitrary
2. Replaces the delay-based reset logic with logic which waits until it
sees the reset is successful, rather than blindly continuing
3. Resets and retries a USB mass storage device after it fails once, in
case even 3 seconds is not enough
BUG=chromiumos-6780
TEST=tried with four different USB sticks, including two which previously
failed
Change-Id: I5bd8891bff67a74b758e349f2a3b3575806eed59
Signed-off-by: Simon Glass <sjg(a)chromium.org>
---
common/usb.c | 176 ++++++++++++++++++++++++++++++++++++-------
common/usb_storage.c | 55 +++++++++++---
drivers/usb/host/ehci-hcd.c | 20 ++++-
include/usb.h | 11 +++
4 files changed, 217 insertions(+), 45 deletions(-)
diff --git a/common/usb.c b/common/usb.c
index 9896f46..3265d5d 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -222,10 +222,14 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
int usb_bulk_msg(struct usb_device *dev, unsigned int pipe,
void *data, int len, int *actual_length, int timeout)
{
+ int err;
+
if (len < 0)
return -1;
dev->status = USB_ST_NOT_PROC; /*not yet processed */
- submit_bulk_msg(dev, pipe, data, len);
+ err = submit_bulk_msg(dev, pipe, data, len);
+ if (err < 0)
+ return err;
while (timeout--) {
if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC))
break;
@@ -758,6 +762,39 @@ struct usb_device *usb_alloc_new_device(void)
return &usb_dev[dev_index - 1];
}
+/* find a device's parent hub, and reset this device's port on that hub */
+
+int usb_reset_device_on_hub(struct usb_device *dev)
+{
+ struct usb_device *parent = dev->parent;
+ unsigned short portstatus;
+ int port = -1;
+ int err;
+
+ /* find the port number we're at */
+ if (parent) {
+ int j;
+
+ for (j = 0; j < parent->maxchild; j++) {
+ if (parent->children[j] == dev) {
+ port = j;
+ break;
+ }
+ }
+ if (port < 0) {
+ printf("usb_new_device:cannot locate device's port.\n");
+ return 1;
+ }
+
+ err = hub_port_reset(dev->parent, port, &portstatus);
+ if (err < 0) {
+ printf("\n Couldn't reset port %i\n", port);
+ return 1;
+ }
+ }
+ return 0;
+}
+
/*
* Free the newly created device node.
* Called in error cases where configuring a newly attached
@@ -941,6 +978,14 @@ int usb_new_device(struct usb_device *dev)
return 0;
}
+/* reset and restart a device that is misbehaving */
+
+int usb_restart_device(struct usb_device *dev)
+{
+ usb_reset_device_on_hub(dev); /* ignore return value */
+ return usb_new_device(dev);
+}
+
/* build device Tree */
void usb_scan_devices(void)
{
@@ -1069,45 +1114,123 @@ static inline char *portspeed(int portstatus)
return "12 Mb/s";
}
-static int hub_port_reset(struct usb_device *dev, int port,
- unsigned short *portstat)
+/* brought this in from kernel 2.6.36 as it is a problem area. Some USB
+sticks do not operate properly with the previous reset code */
+#define PORT_RESET_TRIES 5
+#define SET_ADDRESS_TRIES 2
+#define GET_DESCRIPTOR_TRIES 2
+#define SET_CONFIG_TRIES (2 * (use_both_schemes + 1))
+#define USE_NEW_SCHEME(i) ((i) / 2 == old_scheme_first)
+
+#define HUB_ROOT_RESET_TIME 50 /* times are in msec */
+#define HUB_SHORT_RESET_TIME 10
+#define HUB_LONG_RESET_TIME 200
+#define HUB_RESET_TIMEOUT 500
+
+#define ENOTCONN 107
+
+static int hub_port_wait_reset(struct usb_device *dev, int port,
+ unsigned int delay, unsigned short *portstatus_ret)
{
- int tries;
+ int delay_time, ret;
struct usb_port_status portsts;
unsigned short portstatus, portchange;
- USB_HUB_PRINTF("hub_port_reset: resetting port %d...\n", port);
- for (tries = 0; tries < MAX_TRIES; tries++) {
-
- usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET);
- wait_ms(200);
+ for (delay_time = 0;
+ delay_time < HUB_RESET_TIMEOUT;
+ delay_time += delay) {
+ /* wait to give the device a chance to reset */
+ wait_ms(delay);
- if (usb_get_port_status(dev, port + 1, &portsts) < 0) {
+ /* read and decode port status */
+ ret = usb_get_port_status(dev, port + 1, &portsts);
+ if (ret < 0) {
USB_HUB_PRINTF("get_port_status failed status %lX\n",
dev->status);
- return -1;
+ return ret;
}
portstatus = le16_to_cpu(portsts.wPortStatus);
portchange = le16_to_cpu(portsts.wPortChange);
- USB_HUB_PRINTF("portstatus %x, change %x, %s\n",
- portstatus, portchange,
- portspeed(portstatus));
+ /* Device went away? */
+ if (!(portstatus & USB_PORT_STAT_CONNECTION))
+ return -ENOTCONN;
- USB_HUB_PRINTF("STAT_C_CONNECTION = %d STAT_CONNECTION = %d" \
- " USB_PORT_STAT_ENABLE %d\n",
- (portchange & USB_PORT_STAT_C_CONNECTION) ? 1 : 0,
- (portstatus & USB_PORT_STAT_CONNECTION) ? 1 : 0,
- (portstatus & USB_PORT_STAT_ENABLE) ? 1 : 0);
+ /* bomb out completely if the connection bounced */
+ if ((portchange & USB_PORT_STAT_C_CONNECTION))
+ return -ENOTCONN;
- if ((portchange & USB_PORT_STAT_C_CONNECTION) ||
- !(portstatus & USB_PORT_STAT_CONNECTION))
- return -1;
+ /* if we`ve finished resetting, then break out of the loop */
+ if (!(portstatus & USB_PORT_STAT_RESET) &&
+ (portstatus & USB_PORT_STAT_ENABLE)) {
+ *portstatus_ret = portstatus;
+ return 0;
+ }
- if (portstatus & USB_PORT_STAT_ENABLE)
- break;
+ /* switch to the long delay after two short delay failures */
+ if (delay_time >= 2 * HUB_SHORT_RESET_TIME)
+ delay = HUB_LONG_RESET_TIME;
+
+ USB_HUB_PRINTF(
+ "port %d not reset yet, waiting %dms\n",
+ port, delay);
+ }
+
+ return -1;
+}
+
+
+static int hub_port_reset(struct usb_device *dev, int port,
+ unsigned short *portstat)
+{
+ int tries, status;
+ unsigned delay = HUB_SHORT_RESET_TIME;
+ int oldspeed = dev->speed;
+
+ /* root hub ports have a slightly longer reset period
+ * (from USB 2.0 spec, section 7.1.7.5)
+ */
+ if (!dev->parent) {
+ delay = HUB_ROOT_RESET_TIME;
+ }
- wait_ms(200);
+ /* Some low speed devices have problems with the quick delay, so */
+ /* be a bit pessimistic with those devices. RHbug #23670 */
+ if (oldspeed == USB_SPEED_LOW)
+ delay = HUB_LONG_RESET_TIME;
+
+ USB_HUB_PRINTF("hub_port_reset: resetting port %d...\n", port);
+ for (tries = 0; tries < MAX_TRIES; tries++) {
+
+ status = usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET);
+ if (status)
+ USB_HUB_PRINTF("cannot reset port %d (err = %d)\n",
+ port, status);
+ else {
+ status = hub_port_wait_reset(dev, port, delay,
+ portstat);
+ if (status && status != -ENOTCONN)
+ USB_HUB_PRINTF("port_wait_reset: err = %d\n",
+ status);
+ }
+
+ /* return on disconnect or reset */
+ switch (status) {
+ case 0:
+ /* TRSTRCY = 10 ms; plus some extra */
+ wait_ms(10 + 40);
+ /* FALL THROUGH */
+ case -1:
+ /* we have finished trying to reset, so return */
+ usb_clear_port_feature(dev,
+ port + 1, USB_PORT_FEAT_C_RESET);
+ return 0;
+ }
+
+ USB_HUB_PRINTF (
+ "port %d not enabled, trying reset again...\n",
+ port);
+ delay = HUB_LONG_RESET_TIME;
}
if (tries == MAX_TRIES) {
@@ -1116,9 +1239,6 @@ static int hub_port_reset(struct usb_device *dev, int port,
USB_HUB_PRINTF("Maybe the USB cable is bad?\n");
return -1;
}
-
- usb_clear_port_feature(dev, port + 1, USB_PORT_FEAT_C_RESET);
- *portstat = portstatus;
return 0;
}
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 76949b8..0fb9c1b 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -158,9 +158,10 @@ struct us_data {
static struct us_data usb_stor[USB_MAX_STOR_DEV];
+/* start our error numbers after the USB ones */
#define USB_STOR_TRANSPORT_GOOD 0
-#define USB_STOR_TRANSPORT_FAILED -1
-#define USB_STOR_TRANSPORT_ERROR -2
+#define USB_STOR_TRANSPORT_FAILED (USB_ENEXTFREE)
+#define USB_STOR_TRANSPORT_ERROR (USB_ENEXTFREE-1)
int usb_stor_get_info(struct usb_device *dev, struct us_data *us,
block_dev_desc_t *dev_desc);
@@ -213,6 +214,7 @@ int usb_stor_scan(int mode)
{
unsigned char i;
struct usb_device *dev;
+ int result;
/* GJ */
memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
@@ -244,8 +246,21 @@ int usb_stor_scan(int mode)
/* ok, it is a storage devices
* get info and fill it in
*/
- if (usb_stor_get_info(dev, &usb_stor[usb_max_devs],
- &usb_dev_desc[usb_max_devs]) == 1)
+ result = usb_stor_get_info(dev, &usb_stor[usb_max_devs],
+ &usb_dev_desc[usb_max_devs]);
+ if (result == USB_EDEVCRITICAL) {
+ /*
+ * Something there, but failed badly.
+ * Retry one more time. This happens
+ * sometimes with some USB sticks,
+ * e.g. Patriot Rage ID 13fe:3800
+ */
+ printf (".");
+ usb_restart_device(dev); /* ignore return value */
+ result = usb_stor_get_info(dev, &usb_stor[usb_max_devs],
+ &usb_dev_desc[usb_max_devs]);
+ }
+ if (result == 1)
usb_max_devs++;
}
/* if storage device */
@@ -690,10 +705,13 @@ int usb_stor_BBB_transport(ccb *srb, struct us_data *us)
goto st;
}
if (result < 0) {
- USB_STOR_PRINTF("usb_bulk_msg error status %ld\n",
+ USB_STOR_PRINTF("usb_bulk_msg error status %#lx\n",
us->pusb_dev->status);
usb_stor_BBB_reset(us);
- return USB_STOR_TRANSPORT_FAILED;
+
+ /* if we got a critical device error, report it specially */
+ return result == USB_EDEVCRITICAL ? result
+ : USB_STOR_TRANSPORT_FAILED;
}
#ifdef BBB_XPORT_TRACE
for (index = 0; index < data_actlen; index++)
@@ -900,6 +918,7 @@ static int usb_inquiry(ccb *srb, struct us_data *ss)
static int usb_request_sense(ccb *srb, struct us_data *ss)
{
+ int result;
char *ptr;
ptr = (char *)srb->pdata;
@@ -909,7 +928,12 @@ static int usb_request_sense(ccb *srb, struct us_data *ss)
srb->datalen = 18;
srb->pdata = &srb->sense_buf[0];
srb->cmdlen = 12;
- ss->transport(srb, ss);
+ result = ss->transport(srb, ss);
+ if (result < 0) {
+ if (result != USB_EDEVCRITICAL)
+ USB_STOR_PRINTF("Request Sense failed\n");
+ return result;
+ }
USB_STOR_PRINTF("Request Sense returned %02X %02X %02X\n",
srb->sense_buf[2], srb->sense_buf[12],
srb->sense_buf[13]);
@@ -920,6 +944,7 @@ static int usb_request_sense(ccb *srb, struct us_data *ss)
static int usb_test_unit_ready(ccb *srb, struct us_data *ss)
{
int retries = 10;
+ int result;
do {
memset(&srb->cmd[0], 0, 12);
@@ -928,7 +953,9 @@ static int usb_test_unit_ready(ccb *srb, struct us_data *ss)
srb->cmdlen = 12;
if (ss->transport(srb, ss) == USB_STOR_TRANSPORT_GOOD)
return 0;
- usb_request_sense(srb, ss);
+ result = usb_request_sense(srb, ss);
+ if (result == USB_EDEVCRITICAL)
+ return result;
wait_ms(100);
} while (retries--);
@@ -1314,6 +1341,7 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
unsigned long cap[2];
unsigned long *capacity, *blksz;
ccb *pccb = &usb_ccb;
+ int result;
/* for some reasons a couple of devices would not survive this reset */
if (
@@ -1372,11 +1400,14 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
#endif /* CONFIG_USB_BIN_FIXUP */
USB_STOR_PRINTF("ISO Vers %X, Response Data %X\n", usb_stor_buf[2],
usb_stor_buf[3]);
- if (usb_test_unit_ready(pccb, ss)) {
+ result = usb_test_unit_ready(pccb, ss);
+ if (result) {
+ if (result == USB_EDEVCRITICAL)
+ return result;
printf("Device NOT ready\n"
- " Request Sense returned %02X %02X %02X\n",
- pccb->sense_buf[2], pccb->sense_buf[12],
- pccb->sense_buf[13]);
+ " Request Sense returned %02X %02X %02X\n",
+ pccb->sense_buf[2], pccb->sense_buf[12],
+ pccb->sense_buf[13]);
if (dev_desc->removable == 1) {
dev_desc->type = perq;
return 1;
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 13cd84a..1143cd6 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -330,6 +330,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
uint32_t c, toggle;
uint32_t cmd;
int ret = 0;
+ int result = USB_EFAIL;
#ifdef CONFIG_USB_EHCI_DATA_ALIGN
/* In case ehci host requires alignment for buffers */
@@ -342,8 +343,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
if (!align_buf)
return -1;
if ((int)align_buf & (CONFIG_USB_EHCI_DATA_ALIGN - 1))
- buffer = (void *)((int)align_buf +
- CONFIG_USB_EHCI_DATA_ALIGN -
+ buffer = (void *)((int)align_buf +
+ CONFIG_USB_EHCI_DATA_ALIGN -
((int)align_buf & (CONFIG_USB_EHCI_DATA_ALIGN - 1)));
else
buffer = align_buf;
@@ -475,7 +476,10 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
goto fail;
}
- /* Wait for TDs to be processed. */
+ /*
+ * Wait for TDs to be processed. We wait 3s since some USB
+ * sticks can take a long time immediately after system reset
+ */
ts = get_timer(0);
vtd = td;
do {
@@ -484,7 +488,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
token = hc32_to_cpu(vtd->qt_token);
if (!(token & 0x80))
break;
- } while (get_timer(ts) < CONFIG_SYS_HZ);
+ } while (get_timer(ts) < CONFIG_SYS_HZ * 3);
/* Disable async schedule. */
cmd = ehci_readl(&hcor->or_usbcmd);
@@ -497,6 +501,12 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
printf("EHCI fail timeout STD_ASS reset\n");
goto fail;
}
+ /* check that the TD processing happened */
+ if (token & 0x80) {
+ printf("EHCI timed out on TD - token=%#x\n", token);
+ result = USB_EDEVCRITICAL;
+ goto fail;
+ }
qh_list.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
@@ -559,7 +569,7 @@ fail:
td = (void *)hc32_to_cpu(qh->qh_overlay.qt_next);
}
ehci_free(qh, sizeof(*qh));
- return -1;
+ return result;
}
static inline int min3(int a, int b, int c)
diff --git a/include/usb.h b/include/usb.h
index afd65e3..91aa441 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -42,6 +42,16 @@
#define USB_CNTL_TIMEOUT 100 /* 100ms timeout */
+/*
+ * Errors we can report, e.g. return USB_EDEVCRITICAL
+ * Use -ve numbers to fit in with usb_storage
+ * U-Boot needs some unified numbers
+ */
+#define USB_EOK 0 /* ok, no error */
+#define USB_EFAIL -1 /* general failure(!) */
+#define USB_EDEVCRITICAL -2 /* must reset device on hub */
+#define USB_ENEXTFREE -3 /* next free error number */
+
/* device request (setup) */
struct devrequest {
unsigned char requesttype;
@@ -198,6 +208,7 @@ int usb_get_class_descriptor(struct usb_device *dev, int ifnum,
int usb_clear_halt(struct usb_device *dev, int pipe);
int usb_string(struct usb_device *dev, int index, char *buf, size_t size);
int usb_set_interface(struct usb_device *dev, int interface, int alternate);
+int usb_restart_device(struct usb_device *dev);
/* big endian -> little endian conversion */
/* some CPUs are already little endian e.g. the ARM920T */
--
1.7.3.1
