Hi
Looks like a bug?
Thanks
Guennadi
---------------------------------
Guennadi Liakhovetski, Ph.D.
DSA Daten- und Systemtechnik GmbH
Pascalstr. 28
D-52076 Aachen
Germany
Signed-off-by Guennadi Liakhovetski <[email protected]>
--- a/drivers/usb/host/usb-uhci.c Fri Jan 20 09:27:50 2006
+++ b/drivers/usb/host/usb-uhci.c Fri Jan 20 09:28:05 2006
@@ -2505,7 +2505,7 @@
((urb_priv_t*)urb->hcpriv)->flags=0;
}
- if ((urb->status != -ECONNABORTED) && (urb->status != ECONNRESET) &&
+ if ((urb->status != -ECONNABORTED) && (urb->status != -ECONNRESET) &&
(urb->status != -ENOENT)) {
urb->status = -EINPROGRESS;
On Fri, Jan 20, 2006 at 09:33:26AM +0100, Guennadi Liakhovetski wrote:
> Hi
>
> Looks like a bug?
Looks like you're right.
Marcelo, I've merged it into -upstream.
> Thanks
> Guennadi
Thanks,
Willy
> ---------------------------------
> Guennadi Liakhovetski, Ph.D.
> DSA Daten- und Systemtechnik GmbH
> Pascalstr. 28
> D-52076 Aachen
> Germany
>
> Signed-off-by Guennadi Liakhovetski <[email protected]>
>
> --- a/drivers/usb/host/usb-uhci.c Fri Jan 20 09:27:50 2006
> +++ b/drivers/usb/host/usb-uhci.c Fri Jan 20 09:28:05 2006
> @@ -2505,7 +2505,7 @@
> ((urb_priv_t*)urb->hcpriv)->flags=0;
> }
>
> - if ((urb->status != -ECONNABORTED) && (urb->status !=
> ECONNRESET) &&
> + if ((urb->status != -ECONNABORTED) && (urb->status !=
> -ECONNRESET) &&
> (urb->status != -ENOENT)) {
>
> urb->status = -EINPROGRESS;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
On Fri, 20 Jan 2006 09:33:26 +0100 (CET), Guennadi Liakhovetski <[email protected]> wrote:
> Looks like a bug?
> --- a/drivers/usb/host/usb-uhci.c Fri Jan 20 09:27:50 2006
> +++ b/drivers/usb/host/usb-uhci.c Fri Jan 20 09:28:05 2006
> @@ -2505,7 +2505,7 @@
> ((urb_priv_t*)urb->hcpriv)->flags=0;
> }
>
> - if ((urb->status != -ECONNABORTED) && (urb->status != ECONNRESET) &&
> + if ((urb->status != -ECONNABORTED) && (urb->status != -ECONNRESET) &&
> (urb->status != -ENOENT)) {
This is not what the author intended, obviously. But I am not quite sure
what happens because of it. Seems like we unlink some things which are
about to return anyway... and then return -104 instead of -84. This
may be relatively harmless. At worst, the driver resubmits and gets
its -84 that way.
I vote to apply this and see what happens. We are early in 2.4.33 cycle,
so it should be safe.
-- Pete
On Fri, 20 Jan 2006, Pete Zaitcev wrote:
> On Fri, 20 Jan 2006 09:33:26 +0100 (CET), Guennadi Liakhovetski <[email protected]> wrote:
>
> > Looks like a bug?
>
> > --- a/drivers/usb/host/usb-uhci.c Fri Jan 20 09:27:50 2006
> > +++ b/drivers/usb/host/usb-uhci.c Fri Jan 20 09:28:05 2006
> > @@ -2505,7 +2505,7 @@
> > ((urb_priv_t*)urb->hcpriv)->flags=0;
> > }
> >
> > - if ((urb->status != -ECONNABORTED) && (urb->status != ECONNRESET) &&
> > + if ((urb->status != -ECONNABORTED) && (urb->status != -ECONNRESET) &&
> > (urb->status != -ENOENT)) {
>
> This is not what the author intended, obviously. But I am not quite sure
> what happens because of it. Seems like we unlink some things which are
This is my concirn too. The current behaviour is in fact just
> > - if ((urb->status != -ECONNABORTED) && (urb->status != ECONNRESET) &&
> > + if ((urb->status != -ECONNABORTED) &&
> > (urb->status != -ENOENT)) {
and nobody complains...:-) So, maybe this would be the right fix? At least
safe in that it cannot break anything:-) But I don't understand that code
very well. E.g., I don't understand why about 15 lines above the code in
question
if (urb->complete) {
//dbg("process_interrupt: calling completion, status %i",status);
urb->status = status;
i.e., if (!urb->completion) urb->status is not set, so, depending on
whether the urb has ->completion either the old or the new status will be
tested. Is this really correct? And a couple lines above that "goto
recycle;" is superfluous...
Thanks
Guennadi
> about to return anyway... and then return -104 instead of -84. This
> may be relatively harmless. At worst, the driver resubmits and gets
> its -84 that way.
>
> I vote to apply this and see what happens. We are early in 2.4.33 cycle,
> so it should be safe.
>
> -- Pete
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
---
Guennadi Liakhovetski
On Sun, 22 Jan 2006 01:15:23 +0100 (CET), Guennadi Liakhovetski <[email protected]> wrote:
> i.e., if (!urb->completion) urb->status is not set, so, depending on
> whether the urb has ->completion either the old or the new status will be
> tested. Is this really correct? And a couple lines above that "goto
> recycle;" is superfluous...
I would not recommend to get too excercised over uhci-hcd in 2.4.32.
I do not with to touch it, because it mostly works, and the risk of
regressions is too great.
-- Pete