On Mon, 16 Oct 2017, Mathias Nyman wrote: > If the connect status change is set during reset signaling, but > the status remains connected just retry port reset. > > This solves an issue with connecting a 90W HP Thunderbolt 3 dock > with a Lenovo Carbon x1 (5th generation) which causes a 30min loop > of a high speed device being re-discovererd before usb ports starts > working. > > [...] > [ 389.023845] usb 3-1: new high-speed USB device number 55 using xhci_hcd > [ 389.491841] usb 3-1: new high-speed USB device number 56 using xhci_hcd > [ 389.959928] usb 3-1: new high-speed USB device number 57 using xhci_hcd > [...] > > This is caused by a high speed device that doesn't successfully go to the > enabled state after the second port reset. Instead the connection bounces > (connected, with connect status change), bailing out completely from > enumeration just to restart from scratch. > > Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1716332 > > Cc: Stable <[email protected]> > Signed-off-by: Mathias Nyman <[email protected]> > --- > drivers/usb/core/hub.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index 41eaf0b..3461347 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2710,13 +2710,13 @@ static int hub_port_wait_reset(struct usb_hub *hub, > int port1, > if (!(portstatus & USB_PORT_STAT_CONNECTION)) > return -ENOTCONN; > > - /* bomb out completely if the connection bounced. A USB 3.0 > - * connection may bounce if multiple warm resets were issued, > + /* Retry if connect change is set but status is still connected. > + * A USB 3.0 connection may bounce if multiple warm resets were issued, > * but the device may have successfully re-connected. Ignore it. > */ > if (!hub_is_superspeed(hub->hdev) && > (portchange & USB_PORT_STAT_C_CONNECTION)) > - return -ENOTCONN; > + return -EAGAIN; > > if (!(portstatus & USB_PORT_STAT_ENABLE)) > return -EBUSY; > @@ -2789,6 +2789,10 @@ static int hub_port_reset(struct usb_hub *hub, int > port1, > dev_dbg(hub->intfdev, > "port_wait_reset: err = %d\n", > status); > + /* USB2 connection bounced, but remains connected */ > + if (status == -EAGAIN) > + usb_clear_port_feature(hub->hdev, port1, > + USB_PORT_FEAT_C_CONNECTION);
There's something very awkward about issuing this call here. For one thing, we don't really know at this point that the C_CONNECTION port status bit is set; we only know that the reset needs to be retried. For another, about 14 lines lower down the routine already clears that status bit anyway (although not for the USB-2.0 case). In fact, it's awkward that we issue a bunch of unconditional Clear-Port-Feature calls just below this point, without knowing whether the corresponding status bits are set. Ideally, hub_port_wait_reset() would return its portstatus and portchange values. Or it would clear the extra status bits by itself. Maybe that can all be cleaned up in a later patch. Also, does this fully solve the problem? I can see that it would prevent the lengthy repeated rediscoveries, but wouldn't it also prevent the device from working at all after the first few resets failed? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
