[U-Boot] usb EHCI failing with 'EHCI fail timeout STD_ASS reset'

[U-Boot] usb EHCI failing with 'EHCI fail timeout STD_ASS reset'

[Dan Lykowski]

Hi everyone,

I've seen this question posted a few times in the past with no answer.
Since I was having the same problem, I thought I would share the solution I
came up with.
I'll try to provide a patch if I can get my mess cleaned up.

After some digging, I was able to see the EHCI HC getting a Master Abort
over the PCI bus.
It turns out that some.. all??.. PCI EHCI controllers have 64-bits for the
buffer registers in the qTD structure.
See Appendix B in the EHCI specification.

This is what I changed the qTD structure to.

/* Queue Element Transfer Descriptor (qTD). */
struct qTD {
    uint32_t qt_next;
    uint32_t qt_altnext;
    uint32_t qt_token;
    uint32_t qt_buffer[5];
    uint32_t qt_buffer_hi[5];
    uint32_t unused[3];
};

Adding the qt_buffer_hi array, realigning the buffers on 32-byte boundries,
and filling qt_buffer_hi with zeros when qt_buffer is being filled fixes the
problem.

Thanks,
Dan Lykowski

[U-Boot] [PATCH] USB: sync Queue Element Transfer Descriptor against EHCI spec

[Wolfgang Denk]

Appendix B "EHCI 64-Bit Data Structures" of the "Enhanced Host
Controller Interface Specification for Universal Serial Bus" (Rev.
1.0, March 12, 2002) defines additional fields which were missing in
U-Boot's struct qTD; as these are also present in recent versions of
struct ehci_qtd in the Linux kernel, we add them here, too.

This fixes some nasty memory corruption problems.

Reported-by: Dan Lykowski <lykowdk@gmail.com>
See http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/76942

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Remy Bohmer <linux@bohmer.net>
Cc: Dan Lykowski <lykowdk@gmail.com>
Cc: Stefano Babic <sbabic@denx.de>
Tested-by: Stefano Babic <sbabic@denx.de>
---

Dear Remy,

this patch goes back to an old posting by Dan Lykowski.

Unfortunately Dan never submitted a patch, so this went unfixed until
now, when the bug bit Stefano...

Best regards,

Wolfgang


 drivers/usb/host/ehci-hcd.c |    1 +
 drivers/usb/host/ehci.h     |   14 +++++++++-----
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 37d056e..f44fc4e 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -288,6 +288,7 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 	idx = 0;
 	while (idx < 5) {
 		td->qt_buffer[idx] = cpu_to_hc32(addr);
+		td->qt_buffer_hi[idx] = 0;
 		next = (addr + 4096) & ~4095;
 		delta = next - addr;
 		if (delta >= sz)
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 6fae8ba..d3aa55b 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -166,12 +166,16 @@ struct usb_linux_config_descriptor {
 
 /* Queue Element Transfer Descriptor (qTD). */
 struct qTD {
-	uint32_t qt_next;
+	/* this part defined by EHCI spec */
+	uint32_t qt_next;		/* see EHCI 3.5.1 */
 #define	QT_NEXT_TERMINATE	1
-	uint32_t qt_altnext;
-	uint32_t qt_token;
-	uint32_t qt_buffer[5];
-};
+	uint32_t qt_altnext;		/* see EHCI 3.5.2 */
+	uint32_t qt_token;		/* see EHCI 3.5.3 */
+	uint32_t qt_buffer[5];		/* see EHCI 3.5.4 */
+	uint32_t qt_buffer_hi[5];	/* Appendix B */
+	/* pad struct for 32 byte alignment */
+	uint32_t unused[3];
+} __attribute__ ((aligned (32)));
 
 /* Queue Head (QH). */
 struct QH {
-- 
1.7.2.3

[U-Boot] [PATCH] USB: sync Queue Element Transfer Descriptor against EHCI spec

[Wolfgang Denk]

In message <1287497595-22758-1-git-send-email-wd@denx.de> I wrote:
> Appendix B "EHCI 64-Bit Data Structures" of the "Enhanced Host
> Controller Interface Specification for Universal Serial Bus" (Rev.
> 1.0, March 12, 2002) defines additional fields which were missing in
> U-Boot's struct qTD; as these are also present in recent versions of
> struct ehci_qtd in the Linux kernel, we add them here, too.
> 
> This fixes some nasty memory corruption problems.
> 
> Reported-by: Dan Lykowski <lykowdk@gmail.com>
> See http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/76942
> 
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Dan Lykowski <lykowdk@gmail.com>
> Cc: Stefano Babic <sbabic@denx.de>
> Tested-by: Stefano Babic <sbabic@denx.de>
> ---
> 
> Dear Remy,
> 
> this patch goes back to an old posting by Dan Lykowski.
> 
> Unfortunately Dan never submitted a patch, so this went unfixed until
> now, when the bug bit Stefano...
> 
> Best regards,
> 
> Wolfgang
> 
> 
>  drivers/usb/host/ehci-hcd.c |    1 +
>  drivers/usb/host/ehci.h     |   14 +++++++++-----
>  2 files changed, 10 insertions(+), 5 deletions(-)

Applied directly.

Remy, I hope this is OK with you.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I must follow the people.  Am I not their leader? - Benjamin Disraeli

[U-Boot] Data bus error

[sywang]

Did you have the data bus error with CN5010-based board at porting u-boot? 

How to debug? Any suggestions on this? 

Thanks!
Shuyou  

[U-Boot] [PATCH] USB: sync Queue Element Transfer Descriptor against EHCI spec

[Remy Bohmer]

Hi,

2010/10/20 Wolfgang Denk <wd@denx.de>:
>
> In message <1287497595-22758-1-git-send-email-wd@denx.de> I wrote:
>> Appendix B "EHCI 64-Bit Data Structures" of the "Enhanced Host
>> Controller Interface Specification for Universal Serial Bus" (Rev.
>> 1.0, March 12, 2002) defines additional fields which were missing in
>> U-Boot's struct qTD; as these are also present in recent versions of
>> struct ehci_qtd in the Linux kernel, we add them here, too.
>>
>> This fixes some nasty memory corruption problems.
>>
>> Reported-by: Dan Lykowski <lykowdk@gmail.com>
>> See http://permalink.gmane.org/gmane.comp.boot-loaders.u-boot/76942
>>
>> Signed-off-by: Wolfgang Denk <wd@denx.de>
>> Cc: Remy Bohmer <linux@bohmer.net>
>> Cc: Dan Lykowski <lykowdk@gmail.com>
>> Cc: Stefano Babic <sbabic@denx.de>
>> Tested-by: Stefano Babic <sbabic@denx.de>
>
> Applied directly.
>
> Remy, I hope this is OK with you.

You seem to be in a hurry...
Patch is OK...

Remy

[U-Boot] [PATCH] USB: sync Queue Element Transfer Descriptor against EHCI spec

[Wolfgang Denk]

Dear Remy Bohmer,

In message <AANLkTi=a6bfXBNxo-3A31cOgZAN0j+VfEAovb31r3529@mail.gmail.com> you wrote:
> 
> > Remy, I hope this is OK with you.
> 
> You seem to be in a hurry...

I thought we could be close to a -rc1. Alas, I was wrong once more.

> Patch is OK...

Unfortunately it seems it ain't so :-(  Patch follows...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Free markets select for winning solutions."        - Eric S. Raymond

[U-Boot] [PATCH] USB: fix Queue Element Transfer Descriptor changes

[Wolfgang Denk]

Commit 3ed1607 "USB: sync Queue Element Transfer Descriptor against
EHCI spec" added an "__attribute__ ((aligned (32)))" to the
declaration of struct qTD, as used for example in the Linux kernel as
well.

However, it turns out that this attribute causes errors in "usb start"
(like "ERROR: NOT USB_CONFIG_DESC 7b" and similar). Drop the attribute
again.

Signed-off-by: Wolfgang Denk <wd@denx.de>
Cc: Dan Lykowski <lykowdk@gmail.com>
Cc: Remy Bohmer <linux@bohmer.net>
Cc: Stefano Babic <sbabic@denx.de>
---
 drivers/usb/host/ehci.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index d3aa55b..945ab64 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -175,7 +175,7 @@ struct qTD {
 	uint32_t qt_buffer_hi[5];	/* Appendix B */
 	/* pad struct for 32 byte alignment */
 	uint32_t unused[3];
-} __attribute__ ((aligned (32)));
+};
 
 /* Queue Head (QH). */
 struct QH {
-- 
1.7.2.3

[U-Boot] [PATCH] USB: fix Queue Element Transfer Descriptor changes

[Remy Bohmer]

Hi,

2010/10/20 Wolfgang Denk <wd@denx.de>:
> Commit 3ed1607 "USB: sync Queue Element Transfer Descriptor against
> EHCI spec" added an "__attribute__ ((aligned (32)))" to the
> declaration of struct qTD, as used for example in the Linux kernel as
> well.
>
> However, it turns out that this attribute causes errors in "usb start"
> (like "ERROR: NOT USB_CONFIG_DESC 7b" and similar). Drop the attribute
> again.
>
> Signed-off-by: Wolfgang Denk <wd@denx.de>
> Cc: Dan Lykowski <lykowdk@gmail.com>
> Cc: Remy Bohmer <linux@bohmer.net>
> Cc: Stefano Babic <sbabic@denx.de>
> ---

Applied to u-boot-usb
Thanks.

Remy