On 01/18/2013 08:35 PM, Alan Stern wrote:
> On Fri, 18 Jan 2013, Roger Quadros wrote:
>
>> Thanks. I used the below code (also attached) and could reproduce
>> corruption on the first byte at ehci->priv. This is from the the kernel log.
>>
>>> [ 30.381774] ehci-omap ehci-omap.0: ehci_hcd_omap_probe a 0x1234abcd, b 0x5679efba
>>
>>> [ 122.523468] ehci-omap ehci-omap.0: ehci_hcd_omap_remove a 0x1234abe0, b 0x5679efba
>>
>> NOTE: EHCI needs to be running and you need to enumerate some device for
>> the corruption to happen.
>>
>> If I disable CONFIG_USB_DEBUG, then there is no corruption.
>
> I tried doing something similar with (using ehci-pci rather than
> ehci-omap) and didn't get any corruption. My test patch is below.
> Unforunately you won't be able to utilize the hw_breakpoint mechanism
> in your testing because it hasn't been set up for OMAP. Here's the log
> for a test with two EHCI controllers and one device plugged in. (Note:
> I did have CONFIG_USB_DEBUG enabled, but this log doesn't include any
> of the debug-level messages.)
>
Alan,
It seems that EHCI_STATS is locally defined in ehci_hcd.c thus causing
different versions of struct ehci_hcd to be visible to other users
like ehci-omap.c
The attached patch solves the problem for me and must go in with your
other patches.
>From 70a3621d583c0c76d0bf8239b27eb65d01d918b2 Mon Sep 17 00:00:00 2001
From: Roger Quadros <rogerq@xxxxxx>
Date: Tue, 22 Jan 2013 14:37:57 +0200
Subject: [PATCH] USB: EHCI: Move defination of EHCI_STATS to ehci.h
Without this, platform drivers e.g. ehci-omap.c will see a
different version of struct ehci_hcd than ehci-hcd.c and
break reference to 'debug_dir' and 'priv' members when
CONFIG_USB_DEBUG is enabled.
Signed-off-by: Roger Quadros <rogerq@xxxxxx>
---
drivers/usb/host/ehci-hcd.c | 4 ----
drivers/usb/host/ehci.h | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 97b829e..5a35246 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -74,10 +74,6 @@ static const char hcd_name [] = "ehci_hcd";
#undef VERBOSE_DEBUG
#undef EHCI_URB_TRACE
-#ifdef DEBUG
-#define EHCI_STATS
-#endif
-
/* magic numbers that can affect system performance */
#define EHCI_TUNE_CERR 3 /* 0-3 qtd retries; 0 == don't stop */
#define EHCI_TUNE_RL_HS 4 /* nak throttle; see 4.9 */
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index e9f8c0e..31a935d 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -38,6 +38,10 @@ typedef __u16 __bitwise __hc16;
#endif
/* statistics can be kept for tuning/monitoring */
+#ifdef DEBUG
+#define EHCI_STATS
+#endif
+
struct ehci_stats {
/* irq usage */
unsigned long normal;
-- 1.7.4.1
>From 70a3621d583c0c76d0bf8239b27eb65d01d918b2 Mon Sep 17 00:00:00 2001
From: Roger Quadros <rogerq@xxxxxx>
Date: Tue, 22 Jan 2013 14:37:57 +0200
Subject: [PATCH] USB: EHCI: Move defination of EHCI_STATS to ehci.h
Without this, platform drivers e.g. ehci-omap.c will see a
different version of struct ehci_hcd than ehci-hcd.c and
break reference to 'debug_dir' and 'priv' members when
CONFIG_USB_DEBUG is enabled.
Signed-off-by: Roger Quadros <rogerq@xxxxxx>
---
drivers/usb/host/ehci-hcd.c | 4 ----
drivers/usb/host/ehci.h | 4 ++++
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 97b829e..5a35246 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -74,10 +74,6 @@ static const char hcd_name [] = "ehci_hcd";
#undef VERBOSE_DEBUG
#undef EHCI_URB_TRACE
-#ifdef DEBUG
-#define EHCI_STATS
-#endif
-
/* magic numbers that can affect system performance */
#define EHCI_TUNE_CERR 3 /* 0-3 qtd retries; 0 == don't stop */
#define EHCI_TUNE_RL_HS 4 /* nak throttle; see 4.9 */
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index e9f8c0e..31a935d 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -38,6 +38,10 @@ typedef __u16 __bitwise __hc16;
#endif
/* statistics can be kept for tuning/monitoring */
+#ifdef DEBUG
+#define EHCI_STATS
+#endif
+
struct ehci_stats {
/* irq usage */
unsigned long normal;
--
1.7.4.1
