Re: [PATCH] usb: retry reset if a device times out

[Peter Chen <hzpeterchen@gmail.com>]

On Thu, Feb 04, 2016 at 09:53:10AM +0100, Oliver Neukum wrote:
> On Thu, 2016-02-04 at 14:53 +0800, Peter Chen wrote:
> > On Wed, Feb 03, 2016 at 11:24:57AM -0500, Alan Stern wrote:
> > > On Wed, 3 Feb 2016, Oliver Neukum wrote:
> > > 
> > > > Some devices I got show an inability to operate right after
> > > > power on if they are already connected. They are beyond recovery
> > > > if the descriptors are requested multiple times. So in case of
> > > > a timeout we rather bail early and reset again.
> > > > 
> > > > This patch is a rework of a patch that fell through the cracks.
> > > > http://www.spinics.net/lists/linux-usb/msg103263.html
> > > > 
> > > > Signed-off-by: Oliver Neukum <oneukum@suse.com>
> > > > CC: stable@vger.kernel.org
> > > > ---
> > > >  drivers/usb/core/hub.c | 7 ++++++-
> > > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > > > index f912fe6..2124c4e 100644
> > > > --- a/drivers/usb/core/hub.c
> > > > +++ b/drivers/usb/core/hub.c
> > > > @@ -4496,7 +4496,12 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
> > > >  						r = -EPROTO;
> > > >  					break;
> > > >  				}
> > > > -				if (r == 0)
> > > > +				/*
> > > > +				 * Some devices time out if they are powered on
> > > > +				 * when already connected. They need a second
> > > > +				 * reset.
> > > > +				 */
> > > > +				if (r == 0  || r == -ETIMEDOUT)
> > > >  					break;
> > > >  			}
> > > >  			udev->descriptor.bMaxPacketSize0 =
> > > > 
> > > 
> > > Hmmm.  Your device fails completely if there are multiple attempts 
> > > without a reset in between, right?
> > > 
> > > What about devices which always time out the first control request
> > > after a reset?  I can't be certain any such devices exist, but it
> > > wouldn't be surprising given the range of hardware bugs in USB devices.
> > > 
> > > Would it be safer to do this instead?
> > > 
> > > 				if (r == 0 || (r == -ETIMEDOUT &&
> > > 						j = 0))
> > > 					break;
> > > 
> > 
> > Alan, you may want to skip the first timeout, but quit at the second
> > timeout, right? If it is, the code may need to change like below:
> 
> No. If it times out immediately you want a second reset. If it times
> out any time after that you want a retry of reading the descriptors.
> 

But from what I see, Alan wants to cover the device which the first control
request has failed due to timeout, but the second retry may succeed.

-- 

Best Regards,
Peter Chen