[U-Boot] usb EHCI failing with 'EHCI fail timeout STD_ASS reset'
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
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 {
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
Did you have the data bus error with CN5010-based board at porting u-boot?
How to debug? Any suggestions on this?
Thanks! Shuyou
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
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
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 {
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
participants (4)
-
Dan Lykowski -
Remy Bohmer -
sywang -
Wolfgang Denk