* [PATCH] usb: retry reset if a device times out
@ 2016-02-03 10:47 Oliver Neukum
2016-02-03 16:24 ` Alan Stern
0 siblings, 1 reply; 12+ messages in thread
From: Oliver Neukum @ 2016-02-03 10:47 UTC (permalink / raw)
To: gregkh, stern, linux-usb; +Cc: Oliver Neukum, stable
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-03 10:47 [PATCH] usb: retry reset if a device times out Oliver Neukum
@ 2016-02-03 16:24 ` Alan Stern
2016-02-03 20:52 ` Oliver Neukum
2016-02-04 6:53 ` Peter Chen
0 siblings, 2 replies; 12+ messages in thread
From: Alan Stern @ 2016-02-03 16:24 UTC (permalink / raw)
To: Oliver Neukum; +Cc: gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-03 16:24 ` Alan Stern
@ 2016-02-03 20:52 ` Oliver Neukum
2016-02-04 6:53 ` Peter Chen
1 sibling, 0 replies; 12+ messages in thread
From: Oliver Neukum @ 2016-02-03 20:52 UTC (permalink / raw)
To: Alan Stern; +Cc: gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-03 16:24 ` Alan Stern
2016-02-03 20:52 ` Oliver Neukum
@ 2016-02-04 6:53 ` Peter Chen
2016-02-04 8:53 ` Oliver Neukum
1 sibling, 1 reply; 12+ messages in thread
From: Peter Chen @ 2016-02-04 6:53 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-04 6:53 ` Peter Chen
@ 2016-02-04 8:53 ` Oliver Neukum
2016-02-04 9:08 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Oliver Neukum @ 2016-02-04 8:53 UTC (permalink / raw)
To: Peter Chen; +Cc: Alan Stern, gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-04 8:53 ` Oliver Neukum
@ 2016-02-04 9:08 ` Peter Chen
2016-02-04 9:30 ` Oliver Neukum
0 siblings, 1 reply; 12+ messages in thread
From: Peter Chen @ 2016-02-04 9:08 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Alan Stern, gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-04 9:08 ` Peter Chen
@ 2016-02-04 9:30 ` Oliver Neukum
2016-02-04 15:41 ` Alan Stern
0 siblings, 1 reply; 12+ messages in thread
From: Oliver Neukum @ 2016-02-04 9:30 UTC (permalink / raw)
To: Peter Chen; +Cc: gregkh, Alan Stern, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-04 9:30 ` Oliver Neukum
@ 2016-02-04 15:41 ` Alan Stern
2016-02-05 2:16 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Alan Stern @ 2016-02-04 15:41 UTC (permalink / raw)
To: Oliver Neukum; +Cc: Peter Chen, gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-04 15:41 ` Alan Stern
@ 2016-02-05 2:16 ` Peter Chen
0 siblings, 0 replies; 12+ messages in thread
From: Peter Chen @ 2016-02-05 2:16 UTC (permalink / raw)
To: Alan Stern; +Cc: Oliver Neukum, gregkh, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-17 3:38 ` Peter Chen
@ 2016-02-17 9:15 ` Oliver Neukum
0 siblings, 0 replies; 12+ messages in thread
From: Oliver Neukum @ 2016-02-17 9:15 UTC (permalink / raw)
To: Peter Chen; +Cc: gregkh, stern, linux-usb, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] usb: retry reset if a device times out
2016-02-10 10:33 Oliver Neukum
@ 2016-02-17 3:38 ` Peter Chen
2016-02-17 9:15 ` Oliver Neukum
0 siblings, 1 reply; 12+ messages in thread
From: Peter Chen @ 2016-02-17 3:38 UTC (permalink / raw)
To: Oliver Neukum; +Cc: linux-usb, gregkh, stern, stable
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] usb: retry reset if a device times out
@ 2016-02-10 10:33 Oliver Neukum
2016-02-17 3:38 ` Peter Chen
0 siblings, 1 reply; 12+ messages in thread
From: Oliver Neukum @ 2016-02-10 10:33 UTC (permalink / raw)
To: linux-usb, gregkh, stern, hzpeterchen; +Cc: Oliver Neukum, stable
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-02-17 9:18 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-03 10:47 [PATCH] usb: retry reset if a device times out Oliver Neukum
2016-02-03 16:24 ` Alan Stern
2016-02-03 20:52 ` Oliver Neukum
2016-02-04 6:53 ` Peter Chen
2016-02-04 8:53 ` Oliver Neukum
2016-02-04 9:08 ` Peter Chen
2016-02-04 9:30 ` Oliver Neukum
2016-02-04 15:41 ` Alan Stern
2016-02-05 2:16 ` Peter Chen
2016-02-10 10:33 Oliver Neukum
2016-02-17 3:38 ` Peter Chen
2016-02-17 9:15 ` Oliver Neukum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.