usb: do not reset if a low-speed device timed out

usb: do not reset if a low-speed device timed out

[Maxim Moseychuk]

В Чт, 04/01/2018 в 11:57 -0500, Alan Stern пишет:
> On Thu, 4 Jan 2018, Maxim Moseychuk wrote:
> 
> > Some low-speed devices (for example, bluetooth) do not have
> > time to initialize. For them, ETIMEDOUT is a valid error.
> > We need to give them another try. Otherwise, they will
> > never be initialized correctly and in dmesg will be messages
> > "Bluetooth: hci0 command 0x1002 tx timeout" or similars.
> 
> Your patch description talks about low-speed devices...
> 
> > Fixes: 264904ccc33c ("usb: retry reset if a device times out")
> > Signed-off-by: Maxim Moseychuk <franchesko.salias.hudro.pedros@gmai
> l.com>
> > ---
> >  drivers/usb/core/hub.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> > index cf7bbcb9a63c..46366c73f0df 100644
> > --- a/drivers/usb/core/hub.c
> > +++ b/drivers/usb/core/hub.c
> > @@ -4524,7 +4524,10 @@ hub_port_init(struct usb_hub *hub, struct
> usb_device *udev, int port1,
> >                                * reset. But only on the first
> attempt,
> >                                * lest we get into a time out/reset
> loop
> >                                */
> > -                             if (r == 0  || (r == -ETIMEDOUT &&
> retries == 0))
> > +                             if (r == 0 ||
> > +                                             r == -ETIMEDOUT
> > +                                             && retries == 0
> > +                                             && udev->speed >
> USB_SPEED_FULL)
> 
> but the patch itself affects both low-speed and full-speed devices.
> 
> Also, I would keep the parens around the && expressions, and there's
> no
> need for the first line break you added.  (For that matter, it looks
> strange to break the first line _after_ the || operator and then to
> break the later lines _before_ the && operators -- try to copy the 
> style used elsewhere in that source file.)
> 
> Alan Stern
> 

I probably chose the wrong term. We are talking about devices running
at speeds of usb 1.1 (Low-speed and Full-speed).
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usb: do not reset if a low-speed device timed out

[Alan Stern]

On Thu, 4 Jan 2018, Maxim Moseychuk wrote:

> Some low-speed devices (for example, bluetooth) do not have
> time to initialize. For them, ETIMEDOUT is a valid error.
> We need to give them another try. Otherwise, they will
> never be initialized correctly and in dmesg will be messages
> "Bluetooth: hci0 command 0x1002 tx timeout" or similars.

Your patch description talks about low-speed devices...

> Fixes: 264904ccc33c ("usb: retry reset if a device times out")
> Signed-off-by: Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com>
> ---
>  drivers/usb/core/hub.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index cf7bbcb9a63c..46366c73f0df 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4524,7 +4524,10 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
>  				 * reset. But only on the first attempt,
>  				 * lest we get into a time out/reset loop
>  				 */
> -				if (r == 0  || (r == -ETIMEDOUT && retries == 0))
> +				if (r == 0 ||
> +						r == -ETIMEDOUT
> +						&& retries == 0
> +						&& udev->speed > USB_SPEED_FULL)

but the patch itself affects both low-speed and full-speed devices.

Also, I would keep the parens around the && expressions, and there's no
need for the first line break you added.  (For that matter, it looks
strange to break the first line _after_ the || operator and then to
break the later lines _before_ the && operators -- try to copy the 
style used elsewhere in that source file.)

Alan Stern
---
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

usb: do not reset if a low-speed device timed out

[Maxim Moseychuk]

Some low-speed devices (for example, bluetooth) do not have
time to initialize. For them, ETIMEDOUT is a valid error.
We need to give them another try. Otherwise, they will
never be initialized correctly and in dmesg will be messages
"Bluetooth: hci0 command 0x1002 tx timeout" or similars.

Fixes: 264904ccc33c ("usb: retry reset if a device times out")
Signed-off-by: Maxim Moseychuk <franchesko.salias.hudro.pedros@gmail.com>
---
 drivers/usb/core/hub.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cf7bbcb9a63c..46366c73f0df 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4524,7 +4524,10 @@ hub_port_init(struct usb_hub *hub, struct usb_device *udev, int port1,
 				 * reset. But only on the first attempt,
 				 * lest we get into a time out/reset loop
 				 */
-				if (r == 0  || (r == -ETIMEDOUT && retries == 0))
+				if (r == 0 ||
+						r == -ETIMEDOUT
+						&& retries == 0
+						&& udev->speed > USB_SPEED_FULL)
 					break;
 			}
 			udev->descriptor.bMaxPacketSize0 =