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

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

[Oliver Neukum]

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 =
-- 
2.1.4

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

[Alan Stern]

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 Stern

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

[Oliver Neukum]

On Wed, 2016-02-03 at 11:24 -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;
Indeed. Greg, I'll make an improved patch.

	Regards
		Oliver

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

[Peter Chen]

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:

 				if (r == 0 || (r == -ETIMEDOUT &&
 						j != 0))

-- 

Best Regards,
Peter Chen

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

[Oliver Neukum]

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.

	Regards
		Oliver

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

[Peter Chen]

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

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

[Oliver Neukum]

On Thu, 2016-02-04 at 17:08 +0800, Peter Chen wrote:
> 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.

Exactly. On the other hand we have devices which are "dazed" upon
being powered and need a reset after the first time out.
I think I see the problem now. The test is incorrect and should be
r == -ETIMEDOUT && i == 0
as we need to check the outer loop

	Regards
		Oliver

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

[Alan Stern]

On Thu, 4 Feb 2016, Oliver Neukum wrote:

> On Thu, 2016-02-04 at 17:08 +0800, Peter Chen wrote:
> > 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.
> 
> Exactly. On the other hand we have devices which are "dazed" upon
> being powered and need a reset after the first time out.
> I think I see the problem now. The test is incorrect and should be
> r == -ETIMEDOUT && i == 0
> as we need to check the outer loop

Right; I carelessly used the wrong loop variable.  We should break out 
of the inner loop early if a timeout occurs during the first iteration 
of the outer loop.

Alan Stern

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

[Peter Chen]

On Thu, Feb 04, 2016 at 10:41:45AM -0500, Alan Stern wrote:
> On Thu, 4 Feb 2016, Oliver Neukum wrote:
> 
> > On Thu, 2016-02-04 at 17:08 +0800, Peter Chen wrote:
> > > 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.
> > 
> > Exactly. On the other hand we have devices which are "dazed" upon
> > being powered and need a reset after the first time out.
> > I think I see the problem now. The test is incorrect and should be
> > r == -ETIMEDOUT && i == 0
> > as we need to check the outer loop
> 
> Right; I carelessly used the wrong loop variable.  We should break out 
> of the inner loop early if a timeout occurs during the first iteration 
> of the outer loop.
> 

I agree, that can reduce the waiting time from 15s to 5s for broken
device or buggy controller if bus reset can recover.

-- 

Best Regards,
Peter Chen

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

[Oliver Neukum]

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. But it must be
done only on the first loop lest we get into a reset/time out
spiral that can be overcome with a retry.

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 | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index f912fe6..e4e46f6 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4496,7 +4496,13 @@ 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. But only on the first attempt,
+				 * lest we get into a time out/reset loop
+				 */
+				if (r == 0  || (r == -ETIMEDOUT && j == 0))
 					break;
 			}
 			udev->descriptor.bMaxPacketSize0 =
-- 
2.1.4

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

[Peter Chen]

On Wed, Feb 10, 2016 at 11:33:18AM +0100, 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. But it must be
> done only on the first loop lest we get into a reset/time out
> spiral that can be overcome with a retry.
> 
> 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 | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index f912fe6..e4e46f6 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4496,7 +4496,13 @@ 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. But only on the first attempt,
> +				 * lest we get into a time out/reset loop
> +				 */
> +				if (r == 0  || (r == -ETIMEDOUT && j == 0))
>  					break;
>  			}
>  			udev->descriptor.bMaxPacketSize0 =
> -- 
> 2.1.4
> 

I remembered you agreed with using outer loop counter (i) as the quit
condition.

-- 

Best Regards,
Peter Chen

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

[Oliver Neukum]

On Wed, 2016-02-17 at 11:38 +0800, Peter Chen wrote:
> I remembered you agreed with using outer loop counter (i) as the quit
> condition.

Hi,

thank you very much. A typo that is almost impossible to catch.
I'll submit a patch to fix this.

	Thanks
		Oliver