Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753619AbcLBTf5 (ORCPT ); Fri, 2 Dec 2016 14:35:57 -0500 Received: from iolanthe.rowland.org ([192.131.102.54]:44378 "HELO iolanthe.rowland.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750900AbcLBTfz (ORCPT ); Fri, 2 Dec 2016 14:35:55 -0500 Date: Fri, 2 Dec 2016 14:35:22 -0500 (EST) From: Alan Stern X-X-Sender: stern@iolanthe.rowland.org To: Guenter Roeck cc: Greg Kroah-Hartman , , , Douglas Anderson Subject: Re: [PATCH] usb: hub: Wait for connection to be reestablished after port reset In-Reply-To: <1480628999-4042-1-git-send-email-linux@roeck-us.net> Message-ID: MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2489 Lines: 60 On Thu, 1 Dec 2016, Guenter Roeck wrote: > On a system with a defective USB device connected to an USB hub, > an endless sequence of port connect events was observed. The sequence > of events as observed is as follows: > > - Port reports connected event (port status=USB_PORT_STAT_CONNECTION). > - Event handler debounces port and resets it by calling hub_port_reset(). > - hub_port_reset() calls hub_port_wait_reset() to wait for the reset > to complete. > - The reset completes, but USB_PORT_STAT_CONNECTION is not immediately > set in the port status register. > - hub_port_wait_reset() returns -ENOTCONN. > - Port initialization sequence is aborted. > - A few milliseconds later, the port again reports a connected event, > and the sequence repeats. > > This continues either forever or, randomly, stops if the connection > is already re-established when the port status is read. It results in > a high rate of udev events. This in turn destabilizes userspace since > the above sequence holds the device mutex pretty much continuously > and prevents userspace from actually reading the device status. > > To prevent the problem from happening, let's wait for the connection > to be re-established after a port reset. If the device was actually > disconnected, the code will still return an error, but it will do so > only after the long reset timeout. > > Cc: Douglas Anderson > Signed-off-by: Guenter Roeck Acked-by: Alan Stern > --- > drivers/usb/core/hub.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c > index cbb146736f57..9bcb649e0e8c 100644 > --- a/drivers/usb/core/hub.c > +++ b/drivers/usb/core/hub.c > @@ -2731,8 +2731,15 @@ static int hub_port_wait_reset(struct usb_hub *hub, int port1, > if (ret < 0) > return ret; > > - /* The port state is unknown until the reset completes. */ > - if (!(portstatus & USB_PORT_STAT_RESET)) > + /* > + * The port state is unknown until the reset completes. > + * > + * On top of that, some chips may require additional time > + * to re-establish a connection after the reset is complete, > + * so also wait for the connection to be re-established. > + */ > + if (!(portstatus & USB_PORT_STAT_RESET) && > + (portstatus & USB_PORT_STAT_CONNECTION)) > break; > > /* switch to the long delay after two short delay failures */ >