Bug 6661 - usb ehci-hcd, ehci_intr() function is using same variable for different things, which make if-cases fail.
Summary: usb ehci-hcd, ehci_intr() function is using same variable for different thing...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: USB (show other bugs)
Hardware: i386 Linux
: P2 normal
Assignee: David Brownell
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-06-07 04:19 UTC by Per Hallsmark
Modified: 2006-06-13 12:46 UTC (History)
0 users

See Also:
Kernel Version: 2.6.15.4
Subsystem:
Regression: ---
Bisected commit-id:


Attachments
the official patch (1.05 KB, patch)
2006-06-13 09:35 UTC, David Brownell
Details | Diff

Description Per Hallsmark 2006-06-07 04:19:09 UTC
Most recent kernel where this bug did not occur:
Haven't seen such :-)

Distribution:
kernel.org

Hardware Environment:
Intel XScale ixp465 (for the bug it's more of interest that it is
a EHCI USB 2.0 controller)

Software Environment:
Linux kernel 2.6.15.4

Problem Description:

In ehci-hcd.c there is a existing, even checked latest available 2.6.16.20,
bug that can show up sometimes. Problem is that in the IRQ handler, the
controller status is accuired in the beginning and stored in a variable
called "status". This variable is later used for another controller status
accuiring on another register, then yet later used as if it was the
first variable.
Solution is as simple as using another variable for the second status
accuiring. A patch could be like:

@@ -633,10 +637,11 @@
                        writel (status | CMD_RUN, &ehci->regs->command);

                while (i--) {
-                       status = readl (&ehci->regs->port_status [i]);
-                       if (status & PORT_OWNER)
+                       int pstatus;
+                       pstatus = readl (&ehci->regs->port_status [i]);
+                       if (pstatus & PORT_OWNER)
                                continue;
-                       if (!(status & PORT_RESUME)
+                       if (!(pstatus & PORT_RESUME)
                                        || ehci->reset_done [i] != 0)
                                continue;

Steps to reproduce:
Bug is intermittent, one can see that code has been added trying to cope
with problem caused by using same variable for different things.
Comment 1 Per Hallsmark 2006-06-11 23:59:02 UTC
Actually code should work. An extra variable also named status is
declared inside a new scope at that case flow.
Could problem be xscale/gcc 3.4.4?? strange though, seems like
basic functionality.
Comment 2 David Brownell 2006-06-13 09:35:51 UTC
Created attachment 8295 [details]
the official patch

This would be a very rare bug, and nobody appears to have been using
the sorts of configurations that would trigger it.
Comment 3 David Brownell 2006-06-13 09:38:18 UTC
to original submitter:  no, there was never any code added to 
try working around that issue. 
 
And I have a **really** hard time understanding how this could 
block anything, in the absence of hardware bugs that affect basic 
functionality of that USB controller. 

Note You need to log in before you can comment on or make changes to this bug.