Fixes to EHCI cache handling & smsc95xx

PATCH New Ilya Yanok 3 participants 19 messages Jul 15, 2012 – Jul 18, 2012

Subsystems (4)

USB USB ehci driver MUSB multipoint high speed dual-role controller OMAP USB support

lore

Share & tools

lore

Ilya Yanok

author

2012-07-15 14:43 UTC #0 lore raw

v2 cover Fixes to EHCI cache handling & smsc95xx

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(-)

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #1 lore raw

PATCH v2 1/8 ehci-omap: Do not call dcache_off from omap_ehci_hcd_init

From: Tom Rini <[email protected]>

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 <[email protected]>

Signed-off-by Tom Rini <[email protected]>

Signed-off-by Ilya Yanok <[email protected]>

---
No changes from initial Tom's version.

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

drivers/usb/host/ehci-omap.c +0 -1

index 1ed7710..292673b 100644
@@ -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);

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #2 lore raw

PATCH v2 2/8 common.h: Introduce DEFINE_CACHE_ALIGN_BUFFER

In reply to Ilya Yanok

From: Marek Vasut <[email protected]>

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

Signed-off-by Marek Vasut <[email protected]>

Cc Tom Rini <[email protected]>

[ilya.yanok]: added missing <linux/compiler.h> include and {DEFINE,ALLOC}_ALIGN_BUFFER macros allowing explicit alignment specification.

Signed-off-by Ilya Yanok <[email protected]>

---
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(-)

include/common.h +18 -3

index d1dd65a..be9c278 100644
@@ -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

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #3 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

In reply to Ilya Yanok

From: Tom Rini <[email protected]>

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 <[email protected]>

[marek.vasut]: introduce some crazy macro voodoo

Signed-off-by Marek Vasut <[email protected]>

[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 <[email protected]>

---
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(-)

drivers/usb/host/ehci-hcd.c +34 ~~-30~~

index 04300be..59039f4 100644
@@ -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);

drivers/usb/musb/musb_core.h +1 -1

index a8adcce..e914369 100644
@@ -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
 
 /*

include/usb.h +10 -0

index 6da91e7..ba3d169 100644
@@ -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 */

Signature · 1 line

1.7.9.5

Marek Vasut

2012-07-15 14:59 UTC #4 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

Dear Ilya Yanok,

[...]

Quoted 17 lines

drivers/usb/host/ehci-hcd.c +4 -1

index 04300be..59039f4 100644
@@ -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;

drivers/usb/host/ehci-hcd.c static int · line 214

Quoted 7 lines

drivers/usb/host/ehci-hcd.c +2 -2

@@ -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.

drivers/usb/host/ehci-hcd.c static int · line 218

Quoted 4 lines

drivers/usb/host/ehci-hcd.c +1 -0

+	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 :)

Ilya Yanok

author

2012-07-15 15:51 UTC #5 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

Dear Marek,

Quoted 26 lines · Marek Vasut

drivers/usb/host/ehci-hcd.c struct ehci_hccr *hccr; /* R/O registers, not need

Quoted 4 lines

index 04300be..59039f4 100644
drivers/usb/host/ehci-hcd.c +0 -0
@@ -34,7 +34,10 @@ struct ehci_hccr *hccr;    /* R/O registers, not need

for

drivers/usb/host/ehci-hcd.c struct ehci_hccr *hccr; /* R/O registers, not need for volatile */

Quoted 8 lines

drivers/usb/host/ehci-hcd.c +4 -1

 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))

Quoted 11 lines

drivers/usb/host/ehci-hcd.c +2 -2

 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);

Quoted 5 lines

drivers/usb/host/ehci-hcd.c
Quoted 1 line
drivers/usb/host/ehci-hcd.c +0 -0
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.

Quoted 11 lines

Quoted 5 lines
+     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.

Marek Vasut

2012-07-15 17:12 UTC #6 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

Dear Ilya Yanok,

Quoted 46 lines

Dear Marek,
Quoted 37 lines · Marek Vasut
Quoted 5 lines
index 04300be..59039f4 100644
drivers/usb/host/ehci-hcd.c +0 -0
@@ -34,7 +34,10 @@ struct ehci_hccr *hccr;    /* R/O registers, not
need
for
drivers/usb/host/ehci-hcd.c struct ehci_hccr *hccr; /* R/O registers, not need for volatile */
Quoted 9 lines
volatile */ volatile struct ehci_hcor *hcor;
static uint16_t portreset;
drivers/usb/host/ehci-hcd.c +4 -1
-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))
Quoted 17 lines
static struct descriptor {
      struct usb_hub_descriptor hub;
drivers/usb/host/ehci-hcd.c +2 -2
@@ -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);
Quoted 5 lines
drivers/usb/host/ehci-hcd.c
Quoted 1 line
drivers/usb/host/ehci-hcd.c +0 -0
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.

Quoted 4 lines

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.

Quoted 6 lines

Quoted 6 lines
Quoted 5 lines
+ ALLOC_ALIGN_BUFFER(struct qTD, qtd, 3, USB_DMA_MINALIGN);
      int qtd_counter = 0;
      
      volatile struct qTD *vtd;

Quoted 8 lines

Quoted 5 lines
[...]
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!

Quoted 1 line

Regards, Ilya.

Best regards,
Marek Vasut

Ilya Yanok

author

2012-07-16 08:09 UTC #7 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

Dear Marek,

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

Regards, Ilya.

Ilya Yanok

author

2012-07-16 08:12 UTC #8 lore raw

PATCH 1/1 ehci-hcd: program asynclistaddr before every transfer

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 <[email protected]>

---
 drivers/usb/host/ehci-hcd.c |    8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

drivers/usb/host/ehci-hcd.c +3 -5

index 04300be..3be2a77 100644
@@ -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);

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-16 08:14 UTC #9 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

In reply to Ilya Yanok

From: Tom Rini <[email protected]>

Signed-off-by Tom Rini <[email protected]>

[marek.vasut]: introduce some crazy macro voodoo

Signed-off-by Marek Vasut <[email protected]>

Signed-off-by Ilya Yanok <[email protected]>

---
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(-)

drivers/usb/host/ehci-hcd.c +32 ~~-29~~

index 3be2a77..2b46662 100644
@@ -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);

drivers/usb/musb/musb_core.h +1 -1

index a8adcce..e914369 100644
@@ -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
 
 /*

include/usb.h +10 -0

index 6da91e7..ba3d169 100644
@@ -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 */

Signature · 1 line

1.7.9.5

Marek Vasut

2012-07-16 10:47 UTC #10 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

In reply to Ilya Yanok

Dear Ilya Yanok,

Quoted 3 lines

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.

Quoted 1 line

Regards, Ilya.

Best regards,
Marek Vasut

Ilya Yanok

author

2012-07-17 17:58 UTC #11 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

Dear Marek,

Quoted 6 lines · Marek Vasut

Quoted 2 lines
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

Quoted 3 lines

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.

Marek Vasut

2012-07-18 12:52 UTC #12 lore raw

PATCH v2 3/8 ehci-hcd.c, musb_core, usb.h: Add USB_DMA_MINALIGN define for cache alignment

In reply to Ilya Yanok

Dear Ilya Yanok,

Quoted 5 lines

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

Ilya Yanok

author

2012-07-15 14:43 UTC #13 lore raw

PATCH v2 4/8 ehci-hcd: fix external buffer cache handling

In reply to 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 <[email protected]>

---
 drivers/usb/host/ehci-hcd.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

drivers/usb/host/ehci-hcd.c +14 -9

index 59039f4..a6cd5e3 100644
@@ -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) {

Signature · 1 line

1.7.9.5

Marek Vasut

2012-07-16 00:29 UTC #14 lore raw

PATCH v2 4/8 ehci-hcd: fix external buffer cache handling

Dear Ilya Yanok,

Quoted 29 lines

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 <[email protected]>
---
 drivers/usb/host/ehci-hcd.c |   23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
drivers/usb/host/ehci-hcd.c +0 -0
index 59039f4..a6cd5e3 100644

drivers/usb/host/ehci-hcd.c static int ehci_td_buffer(struct qTD *td, void *buf, · line 182

Quoted 8 lines

drivers/usb/host/ehci-hcd.c +0 -3

@@ -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?

drivers/usb/host/ehci-hcd.c static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz) · line 181

Quoted 3 lines

drivers/usb/host/ehci-hcd.c +1 -1

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

[...]

Best regards,
Marek Vasut

Ilya Yanok

author

2012-07-15 14:43 UTC #15 lore raw

PATCH v2 5/8 usb: pass cache-aligned buffer to usb_get_descriptor()

In reply to 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 <[email protected]>

---
 common/usb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

common/usb.c +4 -2

index c80155c..46f4741 100644
@@ -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);

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #16 lore raw

PATCH v2 6/8 usb: check return value of submit_{control, bulk}_msg

In reply to Ilya Yanok

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

Signed-off-by Ilya Yanok <[email protected]>

---
 common/usb.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

common/usb.c +4 -2

index 46f4741..1b40228 100644
@@ -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;

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #17 lore raw

PATCH v2 7/8 ehci-hcd: change debug() to printf() in case of errors

In reply to Ilya Yanok

Printing message could be useful if something goes really wrong.

Signed-off-by Ilya Yanok <[email protected]>

---
 drivers/usb/host/ehci-hcd.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

drivers/usb/host/ehci-hcd.c +3 -3

index a6cd5e3..52df7fa 100644
@@ -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! */

Signature · 1 line

1.7.9.5

Ilya Yanok

author

2012-07-15 14:43 UTC #18 lore raw

PATCH v2 8/8 smsc95xx: align buffers to cache line size

In reply to Ilya Yanok

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

Signed-off-by Ilya Yanok <[email protected]>

---
 drivers/usb/eth/smsc95xx.c |   13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

drivers/usb/eth/smsc95xx.c +9 -4

index c7aebea..c62a8c1 100644
@@ -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;

Signature · 1 line

1.7.9.5