[U-Boot] [PATCH V2 0/8] Fixes to EHCI cache handling & smsc95xx

[U-Boot] [PATCH V2 0/8] Fixes to EHCI cache handling & smsc95xx

[Ilya Yanok]

This is my second version of long-dicussed series initially posted
by Tom, then by me and then by Marek ;)

This inlcudes proper alignment handling inside EHCI HCD driver,
a little bit more careful error handling and cacheline alignment
fix for smsc95xx USB Ethernet driver.

I noticed no problems with mcx board but with TI beagle xM I get
"EHCI timed out on TD" errors from time to time. Probably that's
because I'm testing with different set of devices. Anyway I can
see the same errors on beagle xM without these series applied
(with D-Cache completely disabled) so I don't think the problem
was introduced by these patches.

Ilya Yanok (5):
  ehci-hcd: fix external buffer cache handling
  usb: pass cache-aligned buffer to usb_get_descriptor()
  usb: check return value of submit_{control,bulk}_msg
  ehci-hcd: change debug() to printf() in case of errors
  smsc95xx: align buffers to cache line size

Marek Vasut (1):
  common.h: Introduce DEFINE_CACHE_ALIGN_BUFFER

Tom Rini (2):
  ehci-omap: Do not call dcache_off from omap_ehci_hcd_init
  ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache
    alignment

 common/usb.c                 |   12 ++++--
 drivers/usb/eth/smsc95xx.c   |   13 ++++--
 drivers/usb/host/ehci-hcd.c  |   93 +++++++++++++++++++++++-------------------
 drivers/usb/host/ehci-omap.c |    1 -
 drivers/usb/musb/musb_core.h |    2 +-
 include/common.h             |   21 ++++++++--
 include/usb.h                |   10 +++++
 7 files changed, 97 insertions(+), 55 deletions(-)

-- 
1.7.9.5

[U-Boot] [PATCH V2 1/8] ehci-omap: Do not call dcache_off from omap_ehci_hcd_init

[Ilya Yanok]

From: Tom Rini <trini@ti.com>

This has never been completely sufficient and now happens too late to
paper over the cache coherency problems with the current USB stack.

Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Tom Rini <trini@ti.com>
Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
No changes from initial Tom's version.

 drivers/usb/host/ehci-omap.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 1ed7710..292673b 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -246,7 +246,6 @@ int omap_ehci_hcd_init(struct omap_usbhs_board_data *usbhs_pdata)
 		if (is_ehci_phy_mode(usbhs_pdata->port_mode[i]))
 			omap_ehci_soft_phy_reset(i);
 
-	dcache_disable();
 	hccr = (struct ehci_hccr *)(OMAP_EHCI_BASE);
 	hcor = (struct ehci_hcor *)(OMAP_EHCI_BASE + 0x10);
 
-- 
1.7.9.5

[U-Boot] [PATCH V2 2/8] common.h: Introduce DEFINE_CACHE_ALIGN_BUFFER

[Ilya Yanok]

From: Marek Vasut <marex@denx.de>

This is the out-of-function-scope counterpart of
ALLOC_CACHE_ALIGN_BUFFER.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Tom Rini <trini@ti.com>
[ilya.yanok]: added missing <linux/compiler.h> include and
{DEFINE,ALLOC}_ALIGN_BUFFER macros allowing explicit alignment
specification.
Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
Changes from Marek's version:
 - added #include <linux/compiler.h> (needed for __aligned())
 - added macros with additional argument to allow explicit alignment
   specification

 include/common.h |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/include/common.h b/include/common.h
index d1dd65a..be9c278 100644
--- a/include/common.h
+++ b/include/common.h
@@ -39,6 +39,7 @@ typedef volatile unsigned char	vu_char;
 #include <linux/bitops.h>
 #include <linux/types.h>
 #include <linux/string.h>
+#include <linux/compiler.h>
 #include <asm/ptrace.h>
 #include <stdarg.h>
 #if defined(CONFIG_PCI) && (defined(CONFIG_4xx) && !defined(CONFIG_AP1000))
@@ -944,11 +945,25 @@ int cpu_release(int nr, int argc, char * const argv[]);
  * of a function scoped static buffer.  It can not be used to create a cache
  * line aligned global buffer.
  */
+#define ALLOC_ALIGN_BUFFER(type, name, size, align)			\
+	char __##name[ROUND(size * sizeof(type), align) + (align - 1)];	\
+									\
+	type *name = (type *) ALIGN((uintptr_t)__##name, align)
 #define ALLOC_CACHE_ALIGN_BUFFER(type, name, size)			\
-	char __##name[ROUND(size * sizeof(type), ARCH_DMA_MINALIGN) +	\
-		      ARCH_DMA_MINALIGN - 1];				\
+	ALLOC_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN)
+
+/*
+ * DEFINE_CACHE_ALIGN_BUFFER() is similar to ALLOC_CACHE_ALIGN_BUFFER, but it's
+ * purpose is to allow allocating aligned buffers outside of function scope.
+ * Usage of this macro shall be avoided or used with extreme care!
+ */
+#define DEFINE_ALIGN_BUFFER(type, name, size, align)			\
+	static char __##name[roundup(size * sizeof(type), align)]	\
+			__aligned(align);				\
 									\
-	type *name = (type *) ALIGN((uintptr_t)__##name, ARCH_DMA_MINALIGN)
+	static type *name = (type *)__##name
+#define DEFINE_CACHE_ALIGN_BUFFER(type, name, size)			\
+	DEFINE_ALIGN_BUFFER(type, name, size, ARCH_DMA_MINALIGN)
 
 /* Pull in stuff for the build system */
 #ifdef DO_DEPS_ONLY
-- 
1.7.9.5

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Ilya Yanok]

From: Tom Rini <trini@ti.com>

The USB spec says that 32 bytes is the minimum required alignment.
However on some platforms we have a larger minimum requirement for cache
coherency.  In those cases, use that value rather than the USB spec
minimum.  We add a cpp check to <usb.h> to define USB_DMA_MINALIGN and
make use of it in ehci-hcd.c and musb_core.h.  We cannot use MAX() here
as we are not allowed to have tests inside of align(...).

Signed-off-by: Tom Rini <trini@ti.com>
[marek.vasut]: introduce some crazy macro voodoo
Signed-off-by: Marek Vasut <marex@denx.de>
[ilya.yanok]: moved external buffer fixes to separate patch,
we use {ALLOC,DEFINE}_ALIGN_BUFFER macros with alignment of USB_DMA_MINALIGN
for qh_list, qh and qtd structures to make sure they are proper aligned
for both controller and cache operations.
For some unclear reason qh structure should remain static.
Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
Changes from Marek's version:
 - external buffer related things moved to separate patch
 - use static arrays for qh structure as it seems that current code
   relies on it's address being constant.

 drivers/usb/host/ehci-hcd.c  |   64 ++++++++++++++++++++++--------------------
 drivers/usb/musb/musb_core.h |    2 +-
 include/usb.h                |   10 +++++++
 3 files changed, 45 insertions(+), 31 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 04300be..59039f4 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -34,7 +34,10 @@ struct ehci_hccr *hccr;	/* R/O registers, not need for volatile */
 volatile struct ehci_hcor *hcor;
 
 static uint16_t portreset;
-static struct QH qh_list __attribute__((aligned(32)));
+DEFINE_ALIGN_BUFFER(struct QH, qh_list, 1, USB_DMA_MINALIGN);
+
+#define ALIGN_END_ADDR(type, ptr, size)			\
+	((uint32_t)(ptr) + roundup((size) * sizeof(type), USB_DMA_MINALIGN))
 
 static struct descriptor {
 	struct usb_hub_descriptor hub;
@@ -207,8 +210,9 @@ static int
 ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		   int length, struct devrequest *req)
 {
-	static struct QH qh __attribute__((aligned(32)));
-	static struct qTD qtd[3] __attribute__((aligned (32)));
+	/* for some reason this doesn't work with non-static qh */
+	DEFINE_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);
+	ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
 	int qtd_counter = 0;
 
 	volatile struct qTD *vtd;
@@ -229,8 +233,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		      le16_to_cpu(req->value), le16_to_cpu(req->value),
 		      le16_to_cpu(req->index));
 
-	memset(&qh, 0, sizeof(struct QH));
-	memset(qtd, 0, sizeof(qtd));
+	memset(qh, 0, sizeof(struct QH));
+	memset(qtd, 0, 3 * sizeof(*qtd));
 
 	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
 
@@ -244,7 +248,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	 *   qh_overlay.qt_next ...... 13-10 H
 	 * - qh_overlay.qt_altnext
 	 */
-	qh.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
+	qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
 	c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
 	     usb_pipeendpoint(pipe) == 0) ? 1 : 0;
 	endpt = (8 << 28) |
@@ -255,14 +259,14 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	    (usb_pipespeed(pipe) << 12) |
 	    (usb_pipeendpoint(pipe) << 8) |
 	    (0 << 7) | (usb_pipedevice(pipe) << 0);
-	qh.qh_endpt1 = cpu_to_hc32(endpt);
+	qh->qh_endpt1 = cpu_to_hc32(endpt);
 	endpt = (1 << 30) |
 	    (dev->portnr << 23) |
 	    (dev->parent->devnum << 16) | (0 << 8) | (0 << 0);
-	qh.qh_endpt2 = cpu_to_hc32(endpt);
-	qh.qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh->qh_endpt2 = cpu_to_hc32(endpt);
+	qh->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 
-	tdp = &qh.qh_overlay.qt_next;
+	tdp = &qh->qh_overlay.qt_next;
 
 	if (req != NULL) {
 		/*
@@ -340,13 +344,13 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		tdp = &qtd[qtd_counter++].qt_next;
 	}
 
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh | QH_LINK_TYPE_QH);
+	qh_list->qh_link = cpu_to_hc32((uint32_t)qh | QH_LINK_TYPE_QH);
 
 	/* Flush dcache */
-	flush_dcache_range((uint32_t)&qh_list,
-		(uint32_t)&qh_list + sizeof(struct QH));
-	flush_dcache_range((uint32_t)&qh, (uint32_t)&qh + sizeof(struct QH));
-	flush_dcache_range((uint32_t)qtd, (uint32_t)qtd + sizeof(qtd));
+	flush_dcache_range((uint32_t)qh_list,
+		ALIGN_END_ADDR(struct QH, qh_list, 1));
+	flush_dcache_range((uint32_t)qh, ALIGN_END_ADDR(struct QH, qh, 1));
+	flush_dcache_range((uint32_t)qtd, ALIGN_END_ADDR(struct qTD, qtd, 3));
 
 	usbsts = ehci_readl(&hcor->or_usbsts);
 	ehci_writel(&hcor->or_usbsts, (usbsts & 0x3f));
@@ -369,12 +373,12 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	timeout = USB_TIMEOUT_MS(pipe);
 	do {
 		/* Invalidate dcache */
-		invalidate_dcache_range((uint32_t)&qh_list,
-			(uint32_t)&qh_list + sizeof(struct QH));
-		invalidate_dcache_range((uint32_t)&qh,
-			(uint32_t)&qh + sizeof(struct QH));
+		invalidate_dcache_range((uint32_t)qh_list,
+			ALIGN_END_ADDR(struct QH, qh_list, 1));
+		invalidate_dcache_range((uint32_t)qh,
+			ALIGN_END_ADDR(struct QH, qh, 1));
 		invalidate_dcache_range((uint32_t)qtd,
-			(uint32_t)qtd + sizeof(qtd));
+			ALIGN_END_ADDR(struct qTD, qtd, 3));
 
 		token = hc32_to_cpu(vtd->qt_token);
 		if (!(token & 0x80))
@@ -403,9 +407,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		goto fail;
 	}
 
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
+	qh_list->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
 
-	token = hc32_to_cpu(qh.qh_overlay.qt_token);
+	token = hc32_to_cpu(qh->qh_overlay.qt_token);
 	if (!(token & 0x80)) {
 		debug("TOKEN=%#x\n", token);
 		switch (token & 0xfc) {
@@ -733,16 +737,16 @@ int usb_lowlevel_init(void)
 #endif
 
 	/* Set head of reclaim list */
-	memset(&qh_list, 0, sizeof(qh_list));
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
-	qh_list.qh_endpt1 = cpu_to_hc32((1 << 15) | (USB_SPEED_HIGH << 12));
-	qh_list.qh_curtd = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_token = cpu_to_hc32(0x40);
+	memset(qh_list, 0, sizeof(*qh_list));
+	qh_list->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
+	qh_list->qh_endpt1 = cpu_to_hc32((1 << 15) | (USB_SPEED_HIGH << 12));
+	qh_list->qh_curtd = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_token = cpu_to_hc32(0x40);
 
 	/* Set async. queue head pointer. */
-	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)&qh_list);
+	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)qh_list);
 
 	reg = ehci_readl(&hccr->cr_hcsparams);
 	descriptor.hub.bNbrPorts = HCS_N_PORTS(reg);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index a8adcce..e914369 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -145,7 +145,7 @@ struct musb_regs {
 		struct musb_epN_regs epN;
 	} ep[16];
 
-} __attribute__((packed, aligned(32)));
+} __attribute__((packed, aligned(USB_DMA_MINALIGN)));
 #endif
 
 /*
diff --git a/include/usb.h b/include/usb.h
index 6da91e7..ba3d169 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -29,6 +29,16 @@
 #include <usb_defs.h>
 #include <usbdescriptors.h>
 
+/*
+ * The EHCI spec says that we must align to at least 32 bytes.  However,
+ * some platforms require larger alignment.
+ */
+#if ARCH_DMA_MINALIGN > 32
+#define USB_DMA_MINALIGN	ARCH_DMA_MINALIGN
+#else
+#define USB_DMA_MINALIGN	32
+#endif
+
 /* Everything is aribtrary */
 #define USB_ALTSETTINGALLOC		4
 #define USB_MAXALTSETTING		128	/* Hard limit */
-- 
1.7.9.5

[U-Boot] [PATCH V2 4/8] ehci-hcd: fix external buffer cache handling

[Ilya Yanok]

Buffer coming from upper layers should be cacheline aligned/padded
to perform safe cache operations. For now we don't do bounce
buffering so getting unaligned buffer is an upper layer error.
We can't check if the buffer is properly padded with current
interface so just assume it is (consider changing with in the
future). The following changes are done:

1. Remove useless length alignment check. We get actual transfer
length not the size of the underlying buffer so it's perfectly
valid for it to be unaligned.
2. Move flush_dcache_range() out of while loop or it will
flush too much.
3. Don't try to fix buffer address before calling invalidate:
if it's unaligned it's an error anyway so let cache subsystem
cry about that.
4. Fix end buffer address to be cacheline aligned assuming upper
layer reserved enough space. This is potentially dangerous
operation so upper layers should be careful about that.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 drivers/usb/host/ehci-hcd.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 59039f4..a6cd5e3 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -175,18 +175,15 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 {
 	uint32_t delta, next;
 	uint32_t addr = (uint32_t)buf;
-	size_t rsz = roundup(sz, 32);
 	int idx;
 
-	if (sz != rsz)
-		debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz);
-
-	if (addr & 31)
+	if (addr != ALIGN(addr, ARCH_DMA_MINALIGN))
 		debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf);
 
+	flush_dcache_range(addr, ALIGN(addr + sz, ARCH_DMA_MINALIGN));
+
 	idx = 0;
 	while (idx < 5) {
-		flush_dcache_range(addr, addr + rsz);
 		td->qt_buffer[idx] = cpu_to_hc32(addr);
 		td->qt_buffer_hi[idx] = 0;
 		next = (addr + 4096) & ~4095;
@@ -386,9 +383,17 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		WATCHDOG_RESET();
 	} while (get_timer(ts) < timeout);
 
-	/* Invalidate the memory area occupied by buffer */
-	invalidate_dcache_range(((uint32_t)buffer & ~31),
-		((uint32_t)buffer & ~31) + roundup(length, 32));
+	/*
+	 * Invalidate the memory area occupied by buffer
+	 * Don't try to fix the buffer alignment, if it isn't properly
+	 * aligned it's upper layer's fault so let invalidate_dcache_range()
+	 * vow about it. But we have to fix the length as it's actual
+	 * transfer length and can be unaligned. This is potentially
+	 * dangerous operation, it's responsibility of the calling
+	 * code to make sure enough space is reserved.
+	 */
+	invalidate_dcache_range((uint32_t)buffer,
+		ALIGN((uint32_t)buffer + length, ARCH_DMA_MINALIGN));
 
 	/* Check that the TD processing happened */
 	if (token & 0x80) {
-- 
1.7.9.5

[U-Boot] [PATCH V2 5/8] usb: pass cache-aligned buffer to usb_get_descriptor()

[Ilya Yanok]

usb_get_descriptor passes it's buffer argument directly to
usb_control_msg() so it has to be properly aligned/padded.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 common/usb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index c80155c..46f4741 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -799,12 +799,13 @@ int usb_new_device(struct usb_device *dev)
 	dev->epmaxpacketin[0] = 8;
 	dev->epmaxpacketout[0] = 8;
 
-	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, &dev->descriptor, 8);
+	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, tmpbuf, 8);
 	if (err < 8) {
 		printf("\n      USB device not responding, " \
 		       "giving up (status=%lX)\n", dev->status);
 		return 1;
 	}
+	memcpy(&dev->descriptor, tmpbuf, 8);
 #else
 	/* This is a Windows scheme of initialization sequence, with double
 	 * reset of the device (Linux uses the same sequence)
@@ -893,7 +894,7 @@ int usb_new_device(struct usb_device *dev)
 	tmp = sizeof(dev->descriptor);
 
 	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0,
-				 &dev->descriptor, sizeof(dev->descriptor));
+				 tmpbuf, sizeof(dev->descriptor));
 	if (err < tmp) {
 		if (err < 0)
 			printf("unable to get device descriptor (error=%d)\n",
@@ -903,6 +904,7 @@ int usb_new_device(struct usb_device *dev)
 				"(expected %i, got %i)\n", tmp, err);
 		return 1;
 	}
+	memcpy(&dev->descriptor, tmpbuf, sizeof(dev->descriptor));
 	/* correct le values */
 	le16_to_cpus(&dev->descriptor.bcdUSB);
 	le16_to_cpus(&dev->descriptor.idVendor);
-- 
1.7.9.5

[U-Boot] [PATCH V2 6/8] usb: check return value of submit_{control, bulk}_msg

[Ilya Yanok]

Return values of submit_{control,bulk}_msg() functions
should be checked to detect possible error.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 common/usb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/common/usb.c b/common/usb.c
index 46f4741..1b40228 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -188,7 +188,8 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 		   request, requesttype, value, index, size);
 	dev->status = USB_ST_NOT_PROC; /*not yet processed */
 
-	submit_control_msg(dev, pipe, data, size, setup_packet);
+	if (submit_control_msg(dev, pipe, data, size, setup_packet) < 0)
+		return -1;
 	if (timeout == 0)
 		return (int)size;
 
@@ -220,7 +221,8 @@ int usb_bulk_msg(struct usb_device *dev, unsigned int pipe,
 	if (len < 0)
 		return -1;
 	dev->status = USB_ST_NOT_PROC; /*not yet processed */
-	submit_bulk_msg(dev, pipe, data, len);
+	if (submit_bulk_msg(dev, pipe, data, len) < 0)
+		return -1;
 	while (timeout--) {
 		if (!((volatile unsigned long)dev->status & USB_ST_NOT_PROC))
 			break;
-- 
1.7.9.5

[U-Boot] [PATCH V2 7/8] ehci-hcd: change debug() to printf() in case of errors

[Ilya Yanok]

Printing message could be useful if something goes really wrong.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 drivers/usb/host/ehci-hcd.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index a6cd5e3..52df7fa 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -196,7 +196,7 @@ static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 	}
 
 	if (idx == 5) {
-		debug("out of buffer pointers (%u bytes left)\n", sz);
+		printf("out of buffer pointers (%u bytes left)\n", sz);
 		return -1;
 	}
 
@@ -282,7 +282,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		    (0 << 15) | (0 << 12) | (3 << 10) | (2 << 8) | (0x80 << 0);
 		qtd[qtd_counter].qt_token = cpu_to_hc32(token);
 		if (ehci_td_buffer(&qtd[qtd_counter], req, sizeof(*req)) != 0) {
-			debug("unable construct SETUP td\n");
+			printf("unable construct SETUP td\n");
 			goto fail;
 		}
 		/* Update previous qTD! */
@@ -311,7 +311,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		    ((usb_pipein(pipe) ? 1 : 0) << 8) | (0x80 << 0);
 		qtd[qtd_counter].qt_token = cpu_to_hc32(token);
 		if (ehci_td_buffer(&qtd[qtd_counter], buffer, length) != 0) {
-			debug("unable construct DATA td\n");
+			printf("unable construct DATA td\n");
 			goto fail;
 		}
 		/* Update previous qTD! */
-- 
1.7.9.5

[U-Boot] [PATCH V2 8/8] smsc95xx: align buffers to cache line size

[Ilya Yanok]

Align buffers passed to the USB code to cache line size so
they can be DMAed safely.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 drivers/usb/eth/smsc95xx.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/eth/smsc95xx.c b/drivers/usb/eth/smsc95xx.c
index c7aebea..c62a8c1 100644
--- a/drivers/usb/eth/smsc95xx.c
+++ b/drivers/usb/eth/smsc95xx.c
@@ -153,13 +153,15 @@ static int curr_eth_dev; /* index for name of next device detected */
 static int smsc95xx_write_reg(struct ueth_data *dev, u32 index, u32 data)
 {
 	int len;
+	ALLOC_CACHE_ALIGN_BUFFER(u32, tmpbuf, 1);
 
 	cpu_to_le32s(&data);
+	tmpbuf[0] = data;
 
 	len = usb_control_msg(dev->pusb_dev, usb_sndctrlpipe(dev->pusb_dev, 0),
 		USB_VENDOR_REQUEST_WRITE_REGISTER,
 		USB_DIR_OUT | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, &data, sizeof(data), USB_CTRL_SET_TIMEOUT);
+		00, index, tmpbuf, sizeof(data), USB_CTRL_SET_TIMEOUT);
 	if (len != sizeof(data)) {
 		debug("smsc95xx_write_reg failed: index=%d, data=%d, len=%d",
 		      index, data, len);
@@ -171,11 +173,13 @@ static int smsc95xx_write_reg(struct ueth_data *dev, u32 index, u32 data)
 static int smsc95xx_read_reg(struct ueth_data *dev, u32 index, u32 *data)
 {
 	int len;
+	ALLOC_CACHE_ALIGN_BUFFER(u32, tmpbuf, 1);
 
 	len = usb_control_msg(dev->pusb_dev, usb_rcvctrlpipe(dev->pusb_dev, 0),
 		USB_VENDOR_REQUEST_READ_REGISTER,
 		USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
-		00, index, data, sizeof(data), USB_CTRL_GET_TIMEOUT);
+		00, index, tmpbuf, sizeof(data), USB_CTRL_GET_TIMEOUT);
+	*data = tmpbuf[0];
 	if (len != sizeof(data)) {
 		debug("smsc95xx_read_reg failed: index=%d, len=%d",
 		      index, len);
@@ -664,7 +668,8 @@ static int smsc95xx_send(struct eth_device *eth, void* packet, int length)
 	int actual_len;
 	u32 tx_cmd_a;
 	u32 tx_cmd_b;
-	unsigned char msg[PKTSIZE + sizeof(tx_cmd_a) + sizeof(tx_cmd_b)];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, msg,
+				 PKTSIZE + sizeof(tx_cmd_a) + sizeof(tx_cmd_b));
 
 	debug("** %s(), len %d, buf %#x\n", __func__, length, (int)msg);
 	if (length > PKTSIZE)
@@ -695,7 +700,7 @@ static int smsc95xx_send(struct eth_device *eth, void* packet, int length)
 static int smsc95xx_recv(struct eth_device *eth)
 {
 	struct ueth_data *dev = (struct ueth_data *)eth->priv;
-	static unsigned char  recv_buf[AX_RX_URB_SIZE];
+	DEFINE_CACHE_ALIGN_BUFFER(unsigned char, recv_buf, AX_RX_URB_SIZE);
 	unsigned char *buf_ptr;
 	int err;
 	int actual_len;
-- 
1.7.9.5

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Marek Vasut]

Dear Ilya Yanok,

[...]

> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 04300be..59039f4 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -34,7 +34,10 @@ struct ehci_hccr *hccr;	/* R/O registers, not need for
> volatile */ volatile struct ehci_hcor *hcor;
> 
>  static uint16_t portreset;
> -static struct QH qh_list __attribute__((aligned(32)));
> +DEFINE_ALIGN_BUFFER(struct QH, qh_list, 1, USB_DMA_MINALIGN);
> +
> +#define ALIGN_END_ADDR(type, ptr, size)			\
> +	((uint32_t)(ptr) + roundup((size) * sizeof(type), USB_DMA_MINALIGN))
> 
>  static struct descriptor {
>  	struct usb_hub_descriptor hub;
> @@ -207,8 +210,9 @@ static int
>  ehci_submit_async(struct usb_device *dev, unsigned long pipe, void
> *buffer, int length, struct devrequest *req)
>  {
> -	static struct QH qh __attribute__((aligned(32)));
> -	static struct qTD qtd[3] __attribute__((aligned (32)));
> +	/* for some reason this doesn't work with non-static qh */
> +	DEFINE_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);

For some reason this doesn't work, but for obvious reason this is NAK. The rest 
is OK, but this is not. Let's investigate.

> +	ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
>  	int qtd_counter = 0;
> 
>  	volatile struct qTD *vtd;
[...]

I'll poke into this hopefully later today, but if you can try finding something 
until then, that'd be great :)

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Ilya Yanok]

Dear Marek,

On Sun, Jul 15, 2012 at 6:59 PM, Marek Vasut <marex@denx.de> wrote:

> > index 04300be..59039f4 100644
> > --- a/drivers/usb/host/ehci-hcd.c
> > +++ b/drivers/usb/host/ehci-hcd.c
> > @@ -34,7 +34,10 @@ struct ehci_hccr *hccr;    /* R/O registers, not need
> for
> > volatile */ volatile struct ehci_hcor *hcor;
> >
> >  static uint16_t portreset;
> > -static struct QH qh_list __attribute__((aligned(32)));
> > +DEFINE_ALIGN_BUFFER(struct QH, qh_list, 1, USB_DMA_MINALIGN);
> > +
> > +#define ALIGN_END_ADDR(type, ptr, size)                      \
> > +     ((uint32_t)(ptr) + roundup((size) * sizeof(type),
> USB_DMA_MINALIGN))
> >
> >  static struct descriptor {
> >       struct usb_hub_descriptor hub;
> > @@ -207,8 +210,9 @@ static int
> >  ehci_submit_async(struct usb_device *dev, unsigned long pipe, void
> > *buffer, int length, struct devrequest *req)
> >  {
> > -     static struct QH qh __attribute__((aligned(32)));
> > -     static struct qTD qtd[3] __attribute__((aligned (32)));
> > +     /* for some reason this doesn't work with non-static qh */
> > +     DEFINE_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);
>
> > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> For some reason this doesn't work, but for obvious reason this is NAK. The
> rest
> is OK, but this is not. Let's investigate.
>

But this was static since the initial EHCI support introduction... the
patch just preserves the previous behavior.
I agree we need to find the reason it doesn't work with on-stack qh and fix
it but it's another problem, not the one these series try to address.


>
> > +     ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
> >       int qtd_counter = 0;
> >
> >       volatile struct qTD *vtd;
> [...]
>
> I'll poke into this hopefully later today, but if you can try finding
> something
> until then, that'd be great :)
>

Sorry, I've already lost too much time with this and I won't return to this
in the near future.

Regards, Ilya.

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Marek Vasut]

Dear Ilya Yanok,

> Dear Marek,
> 
> On Sun, Jul 15, 2012 at 6:59 PM, Marek Vasut <marex@denx.de> wrote:
> > > index 04300be..59039f4 100644
> > > --- a/drivers/usb/host/ehci-hcd.c
> > > +++ b/drivers/usb/host/ehci-hcd.c
> > > @@ -34,7 +34,10 @@ struct ehci_hccr *hccr;    /* R/O registers, not
> > > need
> > 
> > for
> > 
> > > volatile */ volatile struct ehci_hcor *hcor;
> > > 
> > >  static uint16_t portreset;
> > > 
> > > -static struct QH qh_list __attribute__((aligned(32)));
> > > +DEFINE_ALIGN_BUFFER(struct QH, qh_list, 1, USB_DMA_MINALIGN);
> > > +
> > > +#define ALIGN_END_ADDR(type, ptr, size)                      \
> > > +     ((uint32_t)(ptr) + roundup((size) * sizeof(type),
> > 
> > USB_DMA_MINALIGN))
> > 
> > >  static struct descriptor {
> > >  
> > >       struct usb_hub_descriptor hub;
> > > 
> > > @@ -207,8 +210,9 @@ static int
> > > 
> > >  ehci_submit_async(struct usb_device *dev, unsigned long pipe, void
> > > 
> > > *buffer, int length, struct devrequest *req)
> > > 
> > >  {
> > > 
> > > -     static struct QH qh __attribute__((aligned(32)));
> > > -     static struct qTD qtd[3] __attribute__((aligned (32)));
> > > +     /* for some reason this doesn't work with non-static qh */
> > > +     DEFINE_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);
> > > 
> > > diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> > 
> > For some reason this doesn't work, but for obvious reason this is NAK.
> > The rest
> > is OK, but this is not. Let's investigate.
> 
> But this was static since the initial EHCI support introduction... the
> patch just preserves the previous behavior.
> I agree we need to find the reason it doesn't work with on-stack qh and fix
> it but it's another problem, not the one these series try to address.

But this is like covering bugs ... the rest of the patches are OK, it's just 
this little detail that annoys me.

> > > +     ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
> > > 
> > >       int qtd_counter = 0;
> > >       
> > >       volatile struct qTD *vtd;
> > 
> > [...]
> > 
> > I'll poke into this hopefully later today, but if you can try finding
> > something
> > until then, that'd be great :)
> 
> Sorry, I've already lost too much time with this and I won't return to this
> in the near future.

Dunno how much this might motivate you, but this is about my thought now:

http://farm8.staticflickr.com/7138/7456661744_331a4f3535.jpg

I'll try taking over from here though. Thanks for the set and for all your 
investment into this!

> Regards, Ilya.

Best regards,
Marek Vasut

[U-Boot] [PATCH V2 4/8] ehci-hcd: fix external buffer cache handling

[Marek Vasut]

Dear Ilya Yanok,

> Buffer coming from upper layers should be cacheline aligned/padded
> to perform safe cache operations. For now we don't do bounce
> buffering so getting unaligned buffer is an upper layer error.
> We can't check if the buffer is properly padded with current
> interface so just assume it is (consider changing with in the
> future). The following changes are done:
> 
> 1. Remove useless length alignment check. We get actual transfer
> length not the size of the underlying buffer so it's perfectly
> valid for it to be unaligned.
> 2. Move flush_dcache_range() out of while loop or it will
> flush too much.
> 3. Don't try to fix buffer address before calling invalidate:
> if it's unaligned it's an error anyway so let cache subsystem
> cry about that.
> 4. Fix end buffer address to be cacheline aligned assuming upper
> layer reserved enough space. This is potentially dangerous
> operation so upper layers should be careful about that.
> 
> Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
> ---
>  drivers/usb/host/ehci-hcd.c |   23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 59039f4..a6cd5e3 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -175,18 +175,15 @@ static int ehci_td_buffer(struct qTD *td, void *buf,
> size_t sz) {
>  	uint32_t delta, next;
>  	uint32_t addr = (uint32_t)buf;
> -	size_t rsz = roundup(sz, 32);
>  	int idx;
> 
> -	if (sz != rsz)
> -		debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz);

Shall we not also test if addr + sz is aligned?

> -	if (addr & 31)
> +	if (addr != ALIGN(addr, ARCH_DMA_MINALIGN))
>  		debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf);
[...]

Best regards,
Marek Vasut

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Ilya Yanok]

Dear Marek,

ok, I finally managed to fix it. Will post the patches in a few seconds.

Regards, Ilya.

[U-Boot] [PATCH] ehci-hcd: program asynclistaddr before every transfer

[Ilya Yanok]

Move or_asynclistaddr programming to ehci_submit_async()
function to make sure queue head is properly programmed
before every transfer. This solves the problem with changing
qh address.

Also remove unneeded qh_list->qh_link reprogramming at the
end of transfer.

Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
 drivers/usb/host/ehci-hcd.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 04300be..3be2a77 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -348,6 +348,9 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	flush_dcache_range((uint32_t)&qh, (uint32_t)&qh + sizeof(struct QH));
 	flush_dcache_range((uint32_t)qtd, (uint32_t)qtd + sizeof(qtd));
 
+	/* Set async. queue head pointer. */
+	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)&qh_list);
+
 	usbsts = ehci_readl(&hcor->or_usbsts);
 	ehci_writel(&hcor->or_usbsts, (usbsts & 0x3f));
 
@@ -403,8 +406,6 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		goto fail;
 	}
 
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
-
 	token = hc32_to_cpu(qh.qh_overlay.qt_token);
 	if (!(token & 0x80)) {
 		debug("TOKEN=%#x\n", token);
@@ -741,9 +742,6 @@ int usb_lowlevel_init(void)
 	qh_list.qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 	qh_list.qh_overlay.qt_token = cpu_to_hc32(0x40);
 
-	/* Set async. queue head pointer. */
-	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)&qh_list);
-
 	reg = ehci_readl(&hccr->cr_hcsparams);
 	descriptor.hub.bNbrPorts = HCS_N_PORTS(reg);
 	printf("Register %x NbrPorts %d\n", reg, descriptor.hub.bNbrPorts);
-- 
1.7.9.5

[U-Boot] [PATCH V2a 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Ilya Yanok]

From: Tom Rini <trini@ti.com>

The USB spec says that 32 bytes is the minimum required alignment.
However on some platforms we have a larger minimum requirement for cache
coherency.  In those cases, use that value rather than the USB spec
minimum.  We add a cpp check to <usb.h> to define USB_DMA_MINALIGN and
make use of it in ehci-hcd.c and musb_core.h.  We cannot use MAX() here
as we are not allowed to have tests inside of align(...).

Signed-off-by: Tom Rini <trini@ti.com>
[marek.vasut]: introduce some crazy macro voodoo
Signed-off-by: Marek Vasut <marex@denx.de>
[ilya.yanok]: moved external buffer fixes to separate patch,
we use {ALLOC,DEFINE}_ALIGN_BUFFER macros with alignment of USB_DMA_MINALIGN
for qh_list, qh and qtd structures to make sure they are proper aligned
for both controller and cache operations.
Signed-off-by: Ilya Yanok <ilya.yanok@cogentembedded.com>
---
Changes from V2:
 - qh struct made non-static

 drivers/usb/host/ehci-hcd.c  |   61 ++++++++++++++++++++++--------------------
 drivers/usb/musb/musb_core.h |    2 +-
 include/usb.h                |   10 +++++++
 3 files changed, 43 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 3be2a77..2b46662 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -34,7 +34,10 @@ struct ehci_hccr *hccr;	/* R/O registers, not need for volatile */
 volatile struct ehci_hcor *hcor;
 
 static uint16_t portreset;
-static struct QH qh_list __attribute__((aligned(32)));
+DEFINE_ALIGN_BUFFER(struct QH, qh_list, 1, USB_DMA_MINALIGN);
+
+#define ALIGN_END_ADDR(type, ptr, size)			\
+	((uint32_t)(ptr) + roundup((size) * sizeof(type), USB_DMA_MINALIGN))
 
 static struct descriptor {
 	struct usb_hub_descriptor hub;
@@ -207,8 +210,8 @@ static int
 ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		   int length, struct devrequest *req)
 {
-	static struct QH qh __attribute__((aligned(32)));
-	static struct qTD qtd[3] __attribute__((aligned (32)));
+	ALLOC_ALIGN_BUFFER(struct QH, qh, 1, USB_DMA_MINALIGN);
+	ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
 	int qtd_counter = 0;
 
 	volatile struct qTD *vtd;
@@ -229,8 +232,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		      le16_to_cpu(req->value), le16_to_cpu(req->value),
 		      le16_to_cpu(req->index));
 
-	memset(&qh, 0, sizeof(struct QH));
-	memset(qtd, 0, sizeof(qtd));
+	memset(qh, 0, sizeof(struct QH));
+	memset(qtd, 0, 3 * sizeof(*qtd));
 
 	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
 
@@ -244,7 +247,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	 *   qh_overlay.qt_next ...... 13-10 H
 	 * - qh_overlay.qt_altnext
 	 */
-	qh.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
+	qh->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
 	c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
 	     usb_pipeendpoint(pipe) == 0) ? 1 : 0;
 	endpt = (8 << 28) |
@@ -255,14 +258,14 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	    (usb_pipespeed(pipe) << 12) |
 	    (usb_pipeendpoint(pipe) << 8) |
 	    (0 << 7) | (usb_pipedevice(pipe) << 0);
-	qh.qh_endpt1 = cpu_to_hc32(endpt);
+	qh->qh_endpt1 = cpu_to_hc32(endpt);
 	endpt = (1 << 30) |
 	    (dev->portnr << 23) |
 	    (dev->parent->devnum << 16) | (0 << 8) | (0 << 0);
-	qh.qh_endpt2 = cpu_to_hc32(endpt);
-	qh.qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh->qh_endpt2 = cpu_to_hc32(endpt);
+	qh->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 
-	tdp = &qh.qh_overlay.qt_next;
+	tdp = &qh->qh_overlay.qt_next;
 
 	if (req != NULL) {
 		/*
@@ -340,16 +343,16 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		tdp = &qtd[qtd_counter++].qt_next;
 	}
 
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh | QH_LINK_TYPE_QH);
+	qh_list->qh_link = cpu_to_hc32((uint32_t)qh | QH_LINK_TYPE_QH);
 
 	/* Flush dcache */
-	flush_dcache_range((uint32_t)&qh_list,
-		(uint32_t)&qh_list + sizeof(struct QH));
-	flush_dcache_range((uint32_t)&qh, (uint32_t)&qh + sizeof(struct QH));
-	flush_dcache_range((uint32_t)qtd, (uint32_t)qtd + sizeof(qtd));
+	flush_dcache_range((uint32_t)qh_list,
+		ALIGN_END_ADDR(struct QH, qh_list, 1));
+	flush_dcache_range((uint32_t)qh, ALIGN_END_ADDR(struct QH, qh, 1));
+	flush_dcache_range((uint32_t)qtd, ALIGN_END_ADDR(struct qTD, qtd, 3));
 
 	/* Set async. queue head pointer. */
-	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)&qh_list);
+	ehci_writel(&hcor->or_asynclistaddr, (uint32_t)qh_list);
 
 	usbsts = ehci_readl(&hcor->or_usbsts);
 	ehci_writel(&hcor->or_usbsts, (usbsts & 0x3f));
@@ -372,12 +375,12 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	timeout = USB_TIMEOUT_MS(pipe);
 	do {
 		/* Invalidate dcache */
-		invalidate_dcache_range((uint32_t)&qh_list,
-			(uint32_t)&qh_list + sizeof(struct QH));
-		invalidate_dcache_range((uint32_t)&qh,
-			(uint32_t)&qh + sizeof(struct QH));
+		invalidate_dcache_range((uint32_t)qh_list,
+			ALIGN_END_ADDR(struct QH, qh_list, 1));
+		invalidate_dcache_range((uint32_t)qh,
+			ALIGN_END_ADDR(struct QH, qh, 1));
 		invalidate_dcache_range((uint32_t)qtd,
-			(uint32_t)qtd + sizeof(qtd));
+			ALIGN_END_ADDR(struct qTD, qtd, 3));
 
 		token = hc32_to_cpu(vtd->qt_token);
 		if (!(token & 0x80))
@@ -406,7 +409,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		goto fail;
 	}
 
-	token = hc32_to_cpu(qh.qh_overlay.qt_token);
+	token = hc32_to_cpu(qh->qh_overlay.qt_token);
 	if (!(token & 0x80)) {
 		debug("TOKEN=%#x\n", token);
 		switch (token & 0xfc) {
@@ -734,13 +737,13 @@ int usb_lowlevel_init(void)
 #endif
 
 	/* Set head of reclaim list */
-	memset(&qh_list, 0, sizeof(qh_list));
-	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
-	qh_list.qh_endpt1 = cpu_to_hc32((1 << 15) | (USB_SPEED_HIGH << 12));
-	qh_list.qh_curtd = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
-	qh_list.qh_overlay.qt_token = cpu_to_hc32(0x40);
+	memset(qh_list, 0, sizeof(*qh_list));
+	qh_list->qh_link = cpu_to_hc32((uint32_t)qh_list | QH_LINK_TYPE_QH);
+	qh_list->qh_endpt1 = cpu_to_hc32((1 << 15) | (USB_SPEED_HIGH << 12));
+	qh_list->qh_curtd = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
+	qh_list->qh_overlay.qt_token = cpu_to_hc32(0x40);
 
 	reg = ehci_readl(&hccr->cr_hcsparams);
 	descriptor.hub.bNbrPorts = HCS_N_PORTS(reg);
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index a8adcce..e914369 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -145,7 +145,7 @@ struct musb_regs {
 		struct musb_epN_regs epN;
 	} ep[16];
 
-} __attribute__((packed, aligned(32)));
+} __attribute__((packed, aligned(USB_DMA_MINALIGN)));
 #endif
 
 /*
diff --git a/include/usb.h b/include/usb.h
index 6da91e7..ba3d169 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -29,6 +29,16 @@
 #include <usb_defs.h>
 #include <usbdescriptors.h>
 
+/*
+ * The EHCI spec says that we must align to at least 32 bytes.  However,
+ * some platforms require larger alignment.
+ */
+#if ARCH_DMA_MINALIGN > 32
+#define USB_DMA_MINALIGN	ARCH_DMA_MINALIGN
+#else
+#define USB_DMA_MINALIGN	32
+#endif
+
 /* Everything is aribtrary */
 #define USB_ALTSETTINGALLOC		4
 #define USB_MAXALTSETTING		128	/* Hard limit */
-- 
1.7.9.5

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Marek Vasut]

Dear Ilya Yanok,

> Dear Marek,
> 
> ok, I finally managed to fix it. Will post the patches in a few seconds.

So the link I sent you was true afterall ? :)

What was the problem? I'll pick the patches for -next though, unless we're 100% 
definitelly sure they won't break anything else.

> Regards, Ilya.

Best regards,
Marek Vasut

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Ilya Yanok]

Dear Marek,

On Mon, Jul 16, 2012 at 2:47 PM, Marek Vasut <marex@denx.de> wrote:

>
> > ok, I finally managed to fix it. Will post the patches in a few seconds.
>
>
> So the link I sent you was true afterall ? :)
>

Not exactly ;) Actually I've already switched to another task but when I
went to bed one idea came to my mind and I decided to try it.

What was the problem? I'll pick the patches for -next though, unless we're
> 100%
> definitelly sure they won't break anything else.
>

It looks like the controller could stop at the second QH structure (the one
we allocated on stack) on the end of transfer. So on the next function call
it started with the incorrect QH if address of qh was changed.

Regards, Ilya.

[U-Boot] [PATCH V2 3/8] ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

[Marek Vasut]

Dear Ilya Yanok,

> Dear Marek,
> 
> ok, I finally managed to fix it. Will post the patches in a few seconds.
> 
> Regards, Ilya.

I applied and pushed it to u-boot-usb. Can you please give it a proper test now 
and check if nothing is missing?

Thanks Ilya, Tom, for working on this!

Best regards,
Marek Vasut