From 74c0d756dea14855bec47f380b6ccb557be5db7c Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Thu, 7 Aug 2014 17:07:59 -0600 Subject: [PATCH] usb: hub: don't check CONNECTION in hub_port_reset() One specific USB 3.0 device behaves strangely when reset by usb_new_device()'s call to hub_port_reset(). For some reason, the device appears to briefly drop off the bus when this second bus reset is executed, yet if we retry this loop, it'll eventually come back after another two resets. If USB bus reset is executed over and over within usb_new_device()'s call to hub_port_reset(), I see the following sequence of results, which repeats as long as you want: 1) STAT_C_CONNECTION = 1 STAT_CONNECTION = 0 USB_PORT_STAT_ENABLE 0 2) STAT_C_CONNECTION = 1 STAT_CONNECTION = 1 USB_PORT_STAT_ENABLE 0 3) STAT_C_CONNECTION = 1 STAT_CONNECTION = 1 USB_PORT_STAT_ENABLE 1 The device in question is a SanDisk Ultra USB 3.0 16GB memory stick with USB VID/PID 0x0781/0x5581. In order to allow this device to work with U-Boot, ignore the {C_,}CONNECTION bits in the status/change registers, and only use the ENABLE bit to determine if the reset was successful. To be honest, extensive investigation has failed to determine why this problem occurs. I'd love to know! I don't know if it's caused by: * A HW bug in the device * A HW bug in the Tegra USB controller * A SW bug in the U-Boot Tegra USB driver * A SW bug in the U-Boot USB core This issue only occurs when the device's USB3 pins are attached to the host; if only the USB2 pins are connected the issue does not occur. The USB3 controller on Tegra is in reset, so is not actively communicating with the device at all - a USB3 analyzer confirms this. Slightly unplugging the device (so the USB3 pins don't contact) or using a USB2 cable or hub as an intermediary avoids the problem. For some reason, the Linux kernel (either on the same Tegra board, or on an x86 host) has no issue with the device, and I observe no disconnections during reset. This change won't affect any USB device that already works, since such devices could not currently be triggering the error return this patch removes, or they wouldn't be working currently. However, this patch is quite reliable in practice, hence I hope it's acceptable to solve the problem. The only potential fallout I can see from this patch is: * A broken device that triggers C_CONNECTION/!CONNECTION now causes the loop in hub_port_reset() to run multiple times. If it never succeeds, this will cause "usb start" to take roughly 1s extra to execute. * If the user unplugs a device while hub_port_reset() is executing, and very quickly swaps in a new device, hub_port_reset() might succeed on the new device. This would mean that any information cached about the original device (from the descriptor read in usb_new_device(), which simply caches the max packet size) might be invalid, which would cause problems talking to the new device. However, without this change, the new device wouldn't work anyway, so this is probably not much of a loss. Signed-off-by: Stephen Warren --- common/usb_hub.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/common/usb_hub.c b/common/usb_hub.c index 2add4b9792..c416e5e0b3 100644 --- a/common/usb_hub.c +++ b/common/usb_hub.c @@ -209,9 +209,22 @@ int hub_port_reset(struct usb_device *dev, int port, (portstatus & USB_PORT_STAT_CONNECTION) ? 1 : 0, (portstatus & USB_PORT_STAT_ENABLE) ? 1 : 0); - if ((portchange & USB_PORT_STAT_C_CONNECTION) || - !(portstatus & USB_PORT_STAT_CONNECTION)) - return -1; + /* + * Perhaps we should check for the following here: + * - C_CONNECTION hasn't been set. + * - CONNECTION is still set. + * + * Doing so would ensure that the device is still connected + * to the bus, and hasn't been unplugged or replaced while the + * USB bus reset was going on. + * + * However, if we do that, then (at least) a San Disk Ultra + * USB 3.0 16GB device fails to reset on (at least) an NVIDIA + * Tegra Jetson TK1 board. For some reason, the device appears + * to briefly drop off the bus when this second bus reset is + * executed, yet if we retry this loop, it'll eventually come + * back after another reset or two. + */ if (portstatus & USB_PORT_STAT_ENABLE) break; -- 2.39.5