All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled
@ 2012-04-09  3:09 Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline Marek Vasut
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:09 UTC (permalink / raw)
  To: u-boot

This patchset reworks the USB core and EHCI-HCD to work with data caches.

Marek Vasut (3):
  USB: Drop ehci_alloc/ehci_free in ehci-hcd
  USB: Drop cache flush bloat in EHCI-HCD
  USB: Document the QH and qTD antics in EHCI-HCD

Puneet Saxena (1):
  USB: Align buffers at cacheline

 common/cmd_usb.c            |    3 +-
 common/usb.c                |   22 ++--
 common/usb_hub.c            |   27 ++--
 common/usb_storage.c        |   59 +++++-----
 disk/part_dos.c             |    2 +-
 drivers/usb/host/ehci-hcd.c |  284 ++++++++++++++-----------------------------
 include/scsi.h              |    4 +-
 include/usb.h               |    4 +-
 8 files changed, 155 insertions(+), 250 deletions(-)

Cc: Puneet Saxena <puneets@nvidia.com>

-- 
1.7.9.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline
  2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
@ 2012-04-09  3:09 ` Marek Vasut
  2012-04-09  4:49   ` [U-Boot] [PATCH 1/4 V2] " Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 2/4] USB: Drop ehci_alloc/ehci_free in ehci-hcd Marek Vasut
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:09 UTC (permalink / raw)
  To: u-boot

From: Puneet Saxena <puneets@nvidia.com>

This avoids cache-alignment warnings shown in console
when a usb command is entered.

Whenever X bytes of unaligned buffer is invalidated, arm core
invalidates X + Y bytes as per the cache line size and throws
these warnings.

Signed-off-by: Puneet Saxena <puneets@nvidia.com>
---
 common/cmd_usb.c     |    3 +-
 common/usb.c         |   22 +++++++++---------
 common/usb_hub.c     |   27 +++++++++++-----------
 common/usb_storage.c |   59 +++++++++++++++++++++++--------------------------
 disk/part_dos.c      |    2 +-
 include/scsi.h       |    4 ++-
 include/usb.h        |    4 ++-
 7 files changed, 62 insertions(+), 59 deletions(-)

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index 9eba271..a8e3ae5 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -150,7 +150,8 @@ void usb_display_class_sub(unsigned char dclass, unsigned char subclass,
 
 void usb_display_string(struct usb_device *dev, int index)
 {
-	char buffer[256];
+	ALLOC_CACHE_ALIGN_BUFFER(char, buffer, 256);
+
 	if (index != 0) {
 		if (usb_string(dev, index, &buffer[0], 256) > 0)
 			printf("String: \"%s\"", buffer);
diff --git a/common/usb.c b/common/usb.c
index 1ec30bc..87ffb79 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -169,7 +169,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 			unsigned short value, unsigned short index,
 			void *data, unsigned short size, int timeout)
 {
-	struct devrequest setup_packet;
+	ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, setup_packet, 1);
 
 	if ((timeout == 0) && (!asynch_allowed)) {
 		/* request for a asynch control pipe is not allowed */
@@ -177,17 +177,17 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 	}
 
 	/* set setup command */
-	setup_packet.requesttype = requesttype;
-	setup_packet.request = request;
-	setup_packet.value = cpu_to_le16(value);
-	setup_packet.index = cpu_to_le16(index);
-	setup_packet.length = cpu_to_le16(size);
+	setup_packet->requesttype = requesttype;
+	setup_packet->request = request;
+	setup_packet->value = cpu_to_le16(value);
+	setup_packet->index = cpu_to_le16(index);
+	setup_packet->length = cpu_to_le16(size);
 	USB_PRINTF("usb_control_msg: request: 0x%X, requesttype: 0x%X, " \
 		   "value 0x%X index 0x%X length 0x%X\n",
 		   request, requesttype, value, index, size);
 	dev->status = USB_ST_NOT_PROC; /*not yet processed */
 
-	submit_control_msg(dev, pipe, data, size, &setup_packet);
+	submit_control_msg(dev, pipe, data, size, setup_packet);
 	if (timeout == 0)
 		return (int)size;
 
@@ -681,7 +681,7 @@ static int usb_string_sub(struct usb_device *dev, unsigned int langid,
  */
 int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 {
-	unsigned char mybuf[USB_BUFSIZ];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, mybuf, USB_BUFSIZ);
 	unsigned char *tbuf;
 	int err;
 	unsigned int u, idx;
@@ -781,7 +781,7 @@ int usb_new_device(struct usb_device *dev)
 {
 	int addr, err;
 	int tmp;
-	unsigned char tmpbuf[USB_BUFSIZ];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
 
 	/* We still haven't set the Address yet */
 	addr = dev->devnum;
@@ -908,8 +908,8 @@ int usb_new_device(struct usb_device *dev)
 	le16_to_cpus(&dev->descriptor.idProduct);
 	le16_to_cpus(&dev->descriptor.bcdDevice);
 	/* only support for one config for now */
-	usb_get_configuration_no(dev, &tmpbuf[0], 0);
-	usb_parse_config(dev, &tmpbuf[0], 0);
+	usb_get_configuration_no(dev, tmpbuf, 0);
+	usb_parse_config(dev, tmpbuf, 0);
 	usb_set_maxpacket(dev);
 	/* we set the default configuration here */
 	if (usb_set_configuration(dev, dev->config.desc.bConfigurationValue)) {
diff --git a/common/usb_hub.c b/common/usb_hub.c
index e0edaad..f35ad95 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -153,7 +153,7 @@ int hub_port_reset(struct usb_device *dev, int port,
 			unsigned short *portstat)
 {
 	int tries;
-	struct usb_port_status portsts;
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 	unsigned short portstatus, portchange;
 
 	USB_HUB_PRINTF("hub_port_reset: resetting port %d...\n", port);
@@ -162,13 +162,13 @@ int hub_port_reset(struct usb_device *dev, int port,
 		usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET);
 		mdelay(200);
 
-		if (usb_get_port_status(dev, port + 1, &portsts) < 0) {
+		if (usb_get_port_status(dev, port + 1, portsts) < 0) {
 			USB_HUB_PRINTF("get_port_status failed status %lX\n",
 					dev->status);
 			return -1;
 		}
-		portstatus = le16_to_cpu(portsts.wPortStatus);
-		portchange = le16_to_cpu(portsts.wPortChange);
+		portstatus = le16_to_cpu(portsts->wPortStatus);
+		portchange = le16_to_cpu(portsts->wPortChange);
 
 		USB_HUB_PRINTF("portstatus %x, change %x, %s\n",
 				portstatus, portchange,
@@ -206,19 +206,19 @@ int hub_port_reset(struct usb_device *dev, int port,
 void usb_hub_port_connect_change(struct usb_device *dev, int port)
 {
 	struct usb_device *usb;
-	struct usb_port_status portsts;
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 	unsigned short portstatus;
 
 	/* Check status */
-	if (usb_get_port_status(dev, port + 1, &portsts) < 0) {
+	if (usb_get_port_status(dev, port + 1, portsts) < 0) {
 		USB_HUB_PRINTF("get_port_status failed\n");
 		return;
 	}
 
-	portstatus = le16_to_cpu(portsts.wPortStatus);
+	portstatus = le16_to_cpu(portsts->wPortStatus);
 	USB_HUB_PRINTF("portstatus %x, change %x, %s\n",
 			portstatus,
-			le16_to_cpu(portsts.wPortChange),
+			le16_to_cpu(portsts->wPortChange),
 			portspeed(portstatus));
 
 	/* Clear the connection change status */
@@ -267,7 +267,8 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
 static int usb_hub_configure(struct usb_device *dev)
 {
 	int i;
-	unsigned char buffer[USB_BUFSIZ], *bitmap;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);
+	unsigned char *bitmap;
 	struct usb_hub_descriptor *descriptor;
 	struct usb_hub_device *hub;
 #ifdef USB_HUB_DEBUG
@@ -389,16 +390,16 @@ static int usb_hub_configure(struct usb_device *dev)
 	usb_hub_power_on(hub);
 
 	for (i = 0; i < dev->maxchild; i++) {
-		struct usb_port_status portsts;
+		ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 		unsigned short portstatus, portchange;
 
-		if (usb_get_port_status(dev, i + 1, &portsts) < 0) {
+		if (usb_get_port_status(dev, i + 1, portsts) < 0) {
 			USB_HUB_PRINTF("get_port_status failed\n");
 			continue;
 		}
 
-		portstatus = le16_to_cpu(portsts.wPortStatus);
-		portchange = le16_to_cpu(portsts.wPortChange);
+		portstatus = le16_to_cpu(portsts->wPortStatus);
+		portchange = le16_to_cpu(portsts->wPortChange);
 		USB_HUB_PRINTF("Port %d Status %X Change %X\n",
 				i + 1, portstatus, portchange);
 
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 1208333..faad237 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -79,8 +79,7 @@ static const unsigned char us_direction[256/8] = {
 };
 #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
 
-static unsigned char usb_stor_buf[512];
-static ccb usb_ccb;
+static ccb usb_ccb __attribute__((aligned(ARCH_DMA_MINALIGN)));
 
 /*
  * CBI style
@@ -210,17 +209,17 @@ int usb_stor_info(void)
 static unsigned int usb_get_max_lun(struct us_data *us)
 {
 	int len;
-	unsigned char result;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, result, 1);
 	len = usb_control_msg(us->pusb_dev,
 			      usb_rcvctrlpipe(us->pusb_dev, 0),
 			      US_BBB_GET_MAX_LUN,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
 			      0, us->ifnum,
-			      &result, sizeof(result),
+			      result, sizeof(char),
 			      USB_CNTL_TIMEOUT * 5);
 	USB_STOR_PRINTF("Get Max LUN -> len = %i, result = %i\n",
-			len, (int) result);
-	return (len > 0) ? result : 0;
+			len, (int) *result);
+	return (len > 0) ? *result : 0;
 }
 
 /*******************************************************************************
@@ -233,9 +232,6 @@ int usb_stor_scan(int mode)
 	unsigned char i;
 	struct usb_device *dev;
 
-	/* GJ */
-	memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
-
 	if (mode == 1)
 		printf("       scanning bus for storage devices... ");
 
@@ -499,7 +495,7 @@ int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 	int actlen;
 	int dir_in;
 	unsigned int pipe;
-	umass_bbb_cbw_t cbw;
+	ALLOC_CACHE_ALIGN_BUFFER(umass_bbb_cbw_t, cbw, 1);
 
 	dir_in = US_DIRECTION(srb->cmd[0]);
 
@@ -522,16 +518,16 @@ int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 	/* always OUT to the ep */
 	pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out);
 
-	cbw.dCBWSignature = cpu_to_le32(CBWSIGNATURE);
-	cbw.dCBWTag = cpu_to_le32(CBWTag++);
-	cbw.dCBWDataTransferLength = cpu_to_le32(srb->datalen);
-	cbw.bCBWFlags = (dir_in ? CBWFLAGS_IN : CBWFLAGS_OUT);
-	cbw.bCBWLUN = srb->lun;
-	cbw.bCDBLength = srb->cmdlen;
+	cbw->dCBWSignature = cpu_to_le32(CBWSIGNATURE);
+	cbw->dCBWTag = cpu_to_le32(CBWTag++);
+	cbw->dCBWDataTransferLength = cpu_to_le32(srb->datalen);
+	cbw->bCBWFlags = (dir_in ? CBWFLAGS_IN : CBWFLAGS_OUT);
+	cbw->bCBWLUN = srb->lun;
+	cbw->bCDBLength = srb->cmdlen;
 	/* copy the command data into the CBW command data buffer */
 	/* DST SRC LEN!!! */
-	memcpy(cbw.CBWCDB, srb->cmd, srb->cmdlen);
-	result = usb_bulk_msg(us->pusb_dev, pipe, &cbw, UMASS_BBB_CBW_SIZE,
+	memcpy(cbw->CBWCDB, srb->cmd, srb->cmdlen);
+	result = usb_bulk_msg(us->pusb_dev, pipe, cbw, UMASS_BBB_CBW_SIZE,
 			      &actlen, USB_CNTL_TIMEOUT * 5);
 	if (result < 0)
 		USB_STOR_PRINTF("usb_stor_BBB_comdat:usb_bulk_msg error\n");
@@ -675,7 +671,7 @@ int usb_stor_BBB_transport(ccb *srb, struct us_data *us)
 	int dir_in;
 	int actlen, data_actlen;
 	unsigned int pipe, pipein, pipeout;
-	umass_bbb_csw_t csw;
+	ALLOC_CACHE_ALIGN_BUFFER(umass_bbb_csw_t, csw, 1);
 #ifdef BBB_XPORT_TRACE
 	unsigned char *ptr;
 	int index;
@@ -733,7 +729,7 @@ st:
 	retry = 0;
 again:
 	USB_STOR_PRINTF("STATUS phase\n");
-	result = usb_bulk_msg(us->pusb_dev, pipein, &csw, UMASS_BBB_CSW_SIZE,
+	result = usb_bulk_msg(us->pusb_dev, pipein, csw, UMASS_BBB_CSW_SIZE,
 				&actlen, USB_CNTL_TIMEOUT*5);
 
 	/* special handling of STALL in STATUS phase */
@@ -753,28 +749,28 @@ again:
 		return USB_STOR_TRANSPORT_FAILED;
 	}
 #ifdef BBB_XPORT_TRACE
-	ptr = (unsigned char *)&csw;
+	ptr = (unsigned char *)csw;
 	for (index = 0; index < UMASS_BBB_CSW_SIZE; index++)
 		printf("ptr[%d] %#x ", index, ptr[index]);
 	printf("\n");
 #endif
 	/* misuse pipe to get the residue */
-	pipe = le32_to_cpu(csw.dCSWDataResidue);
+	pipe = le32_to_cpu(csw->dCSWDataResidue);
 	if (pipe == 0 && srb->datalen != 0 && srb->datalen - data_actlen != 0)
 		pipe = srb->datalen - data_actlen;
-	if (CSWSIGNATURE != le32_to_cpu(csw.dCSWSignature)) {
+	if (CSWSIGNATURE != le32_to_cpu(csw->dCSWSignature)) {
 		USB_STOR_PRINTF("!CSWSIGNATURE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if ((CBWTag - 1) != le32_to_cpu(csw.dCSWTag)) {
+	} else if ((CBWTag - 1) != le32_to_cpu(csw->dCSWTag)) {
 		USB_STOR_PRINTF("!Tag\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus > CSWSTATUS_PHASE) {
+	} else if (csw->bCSWStatus > CSWSTATUS_PHASE) {
 		USB_STOR_PRINTF(">PHASE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus == CSWSTATUS_PHASE) {
+	} else if (csw->bCSWStatus == CSWSTATUS_PHASE) {
 		USB_STOR_PRINTF("=PHASE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
@@ -782,7 +778,7 @@ again:
 		USB_STOR_PRINTF("transferred %dB instead of %ldB\n",
 			data_actlen, srb->datalen);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus == CSWSTATUS_FAILED) {
+	} else if (csw->bCSWStatus == CSWSTATUS_FAILED) {
 		USB_STOR_PRINTF("FAILED\n");
 		return USB_STOR_TRANSPORT_FAILED;
 	}
@@ -1343,7 +1339,8 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		      block_dev_desc_t *dev_desc)
 {
 	unsigned char perq, modi;
-	unsigned long cap[2];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned long, cap, 2);
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, usb_stor_buf, 36);
 	unsigned long *capacity, *blksz;
 	ccb *pccb = &usb_ccb;
 
@@ -1367,9 +1364,9 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		/* drive is removable */
 		dev_desc->removable = 1;
 	}
-	memcpy(&dev_desc->vendor[0], &usb_stor_buf[8], 8);
-	memcpy(&dev_desc->product[0], &usb_stor_buf[16], 16);
-	memcpy(&dev_desc->revision[0], &usb_stor_buf[32], 4);
+	memcpy(&dev_desc->vendor[0], (const void *) &usb_stor_buf[8], 8);
+	memcpy(&dev_desc->product[0], (const void *) &usb_stor_buf[16], 16);
+	memcpy(&dev_desc->revision[0], (const void *) &usb_stor_buf[32], 4);
 	dev_desc->vendor[8] = 0;
 	dev_desc->product[16] = 0;
 	dev_desc->revision[4] = 0;
diff --git a/disk/part_dos.c b/disk/part_dos.c
index b5bcb37..70211ee 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -87,7 +87,7 @@ static int test_block_type(unsigned char *buffer)
 
 int test_part_dos (block_dev_desc_t *dev_desc)
 {
-	unsigned char buffer[dev_desc->blksz];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 
 	if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) ||
 	    (buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) ||
diff --git a/include/scsi.h b/include/scsi.h
index c52759c..89ae45f 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -26,7 +26,9 @@
 
 typedef struct SCSI_cmd_block{
 	unsigned char		cmd[16];					/* command				   */
-	unsigned char		sense_buf[64];		/* for request sense */
+	/* for request sense */
+	unsigned char		sense_buf[64]
+		__attribute__((aligned(ARCH_DMA_MINALIGN)));
 	unsigned char		status;						/* SCSI Status			 */
 	unsigned char		target;						/* Target ID				 */
 	unsigned char		lun;							/* Target LUN        */
diff --git a/include/usb.h b/include/usb.h
index 48e4bcd..6da91e7 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -109,7 +109,9 @@ struct usb_device {
 	int epmaxpacketout[16];		/* OUTput endpoint specific maximums */
 
 	int configno;			/* selected config number */
-	struct usb_device_descriptor descriptor; /* Device Descriptor */
+	/* Device Descriptor */
+	struct usb_device_descriptor descriptor
+		__attribute__((aligned(ARCH_DMA_MINALIGN)));
 	struct usb_config config; /* config descriptor */
 
 	int have_langid;		/* whether string_langid is valid yet */
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 2/4] USB: Drop ehci_alloc/ehci_free in ehci-hcd
  2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline Marek Vasut
@ 2012-04-09  3:09 ` Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 3/4] USB: Drop cache flush bloat in EHCI-HCD Marek Vasut
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:09 UTC (permalink / raw)
  To: u-boot

These two functions were called only from ehci_submit_async(), therefore
dissolve them as part of ehci_submit_async() to get rid of all those static
variables.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Puneet Saxena <puneets@nvidia.com>
---
 drivers/usb/host/ehci-hcd.c |  123 +++++++++++-------------------------------
 1 files changed, 32 insertions(+), 91 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index b6422d7..8ac0b44 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -225,11 +225,6 @@ static int handshake(uint32_t *ptr, uint32_t mask, uint32_t done, int usec)
 	return -1;
 }
 
-static void ehci_free(void *p, size_t sz)
-{
-
-}
-
 static int ehci_reset(void)
 {
 	uint32_t cmd;
@@ -266,35 +261,6 @@ out:
 	return ret;
 }
 
-static void *ehci_alloc(size_t sz, size_t align)
-{
-	static struct QH qh __attribute__((aligned(32)));
-	static struct qTD td[3] __attribute__((aligned (32)));
-	static int ntds;
-	void *p;
-
-	switch (sz) {
-	case sizeof(struct QH):
-		p = &qh;
-		ntds = 0;
-		break;
-	case sizeof(struct qTD):
-		if (ntds == 3) {
-			debug("out of TDs\n");
-			return NULL;
-		}
-		p = &td[ntds];
-		ntds++;
-		break;
-	default:
-		debug("unknown allocation size\n");
-		return NULL;
-	}
-
-	memset(p, 0, sz);
-	return p;
-}
-
 static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 {
 	uint32_t addr, delta, next;
@@ -326,8 +292,10 @@ static int
 ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		   int length, struct devrequest *req)
 {
-	struct QH *qh;
-	struct qTD *td;
+	static struct QH qh __attribute__((aligned(32)));
+	static struct qTD qtd[3] __attribute__((aligned (32)));
+	int qtd_counter = 0;
+
 	volatile struct qTD *vtd;
 	unsigned long ts;
 	uint32_t *tdp;
@@ -346,12 +314,10 @@ 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));
 
-	qh = ehci_alloc(sizeof(struct QH), 32);
-	if (qh == NULL) {
-		debug("unable to allocate QH\n");
-		return -1;
-	}
-	qh->qh_link = cpu_to_hc32((uint32_t)&qh_list | QH_LINK_TYPE_QH);
+	memset(&qh, 0, sizeof(struct QH));
+	memset(qtd, 0, sizeof(qtd));
+
+	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) |
@@ -362,85 +328,67 @@ 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);
 
-	td = NULL;
-	tdp = &qh->qh_overlay.qt_next;
+	tdp = &qh.qh_overlay.qt_next;
 
 	toggle =
 	    usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
 
 	if (req != NULL) {
-		td = ehci_alloc(sizeof(struct qTD), 32);
-		if (td == NULL) {
-			debug("unable to allocate SETUP td\n");
-			goto fail;
-		}
-		td->qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-		td->qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (0 << 31) |
 		    (sizeof(*req) << 16) |
 		    (0 << 15) | (0 << 12) | (3 << 10) | (2 << 8) | (0x80 << 0);
-		td->qt_token = cpu_to_hc32(token);
-		if (ehci_td_buffer(td, req, sizeof(*req)) != 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");
-			ehci_free(td, sizeof(*td));
 			goto fail;
 		}
-		*tdp = cpu_to_hc32((uint32_t) td);
-		tdp = &td->qt_next;
+		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
+		tdp = &qtd[qtd_counter++].qt_next;
 		toggle = 1;
 	}
 
 	if (length > 0 || req == NULL) {
-		td = ehci_alloc(sizeof(struct qTD), 32);
-		if (td == NULL) {
-			debug("unable to allocate DATA td\n");
-			goto fail;
-		}
-		td->qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-		td->qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (toggle << 31) |
 		    (length << 16) |
 		    ((req == NULL ? 1 : 0) << 15) |
 		    (0 << 12) |
 		    (3 << 10) |
 		    ((usb_pipein(pipe) ? 1 : 0) << 8) | (0x80 << 0);
-		td->qt_token = cpu_to_hc32(token);
-		if (ehci_td_buffer(td, buffer, length) != 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");
-			ehci_free(td, sizeof(*td));
 			goto fail;
 		}
-		*tdp = cpu_to_hc32((uint32_t) td);
-		tdp = &td->qt_next;
+		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
+		tdp = &qtd[qtd_counter++].qt_next;
 	}
 
 	if (req != NULL) {
-		td = ehci_alloc(sizeof(struct qTD), 32);
-		if (td == NULL) {
-			debug("unable to allocate ACK td\n");
-			goto fail;
-		}
-		td->qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
-		td->qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
+		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (toggle << 31) |
 		    (0 << 16) |
 		    (1 << 15) |
 		    (0 << 12) |
 		    (3 << 10) |
 		    ((usb_pipein(pipe) ? 0 : 1) << 8) | (0x80 << 0);
-		td->qt_token = cpu_to_hc32(token);
-		*tdp = cpu_to_hc32((uint32_t) td);
-		tdp = &td->qt_next;
+		qtd[qtd_counter].qt_token = cpu_to_hc32(token);
+		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
+		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 */
 	ehci_flush_dcache(&qh_list);
@@ -462,7 +410,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	/* Wait for TDs to be processed. */
 	ts = get_timer(0);
-	vtd = td;
+	vtd = &qtd[qtd_counter - 1];
 	timeout = USB_TIMEOUT_MS(pipe);
 	do {
 		/* Invalidate dcache */
@@ -492,7 +440,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	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) {
@@ -531,13 +479,6 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	return (dev->status != USB_ST_NOT_PROC) ? 0 : -1;
 
 fail:
-	td = (void *)hc32_to_cpu(qh->qh_overlay.qt_next);
-	while (td != (void *)QT_NEXT_TERMINATE) {
-		qh->qh_overlay.qt_next = td->qt_next;
-		ehci_free(td, sizeof(*td));
-		td = (void *)hc32_to_cpu(qh->qh_overlay.qt_next);
-	}
-	ehci_free(qh, sizeof(*qh));
 	return -1;
 }
 
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4] USB: Drop cache flush bloat in EHCI-HCD
  2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 2/4] USB: Drop ehci_alloc/ehci_free in ehci-hcd Marek Vasut
@ 2012-04-09  3:09 ` Marek Vasut
  2012-04-09  4:47   ` [U-Boot] [PATCH 3/4 V2] " Marek Vasut
  2012-04-09  3:09 ` [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics " Marek Vasut
  2012-04-09  3:12 ` [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
  4 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:09 UTC (permalink / raw)
  To: u-boot

Discard the creepy cache flushing mechanisms in ehci-hcd.c and replace them with
more straightforward flushing. In the new approach, the flushing takes place
directly in ehci_submit_async() call instead of going through the QH list and
flushing all members and buffers. This discards a lot of weird bit operations
on the members of QH and qTD structures.

NOTE: Certainly, this flushes even qTDs which are possibly unused in some
transactions, though the overhead of the previous code was much higher than is
the overhead of flushing two more cache lines (which most probably aren't even
cached).

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Puneet Saxena <puneets@nvidia.com>
---
 drivers/usb/host/ehci-hcd.c |  123 ++++++++-----------------------------------
 1 files changed, 23 insertions(+), 100 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8ac0b44..55fe067 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -108,99 +108,6 @@ static struct descriptor {
 #define ehci_is_TDI()	(0)
 #endif
 
-#if defined(CONFIG_EHCI_DCACHE)
-/*
- * Routines to handle (flush/invalidate) the dcache for the QH and qTD
- * structures and data buffers. This is needed on platforms using this
- * EHCI support with dcache enabled.
- */
-static void flush_invalidate(u32 addr, int size, int flush)
-{
-	if (flush)
-		flush_dcache_range(addr, addr + size);
-	else
-		invalidate_dcache_range(addr, addr + size);
-}
-
-static void cache_qtd(struct qTD *qtd, int flush)
-{
-	u32 *ptr = (u32 *)qtd->qt_buffer[0];
-	int len = (qtd->qt_token & 0x7fff0000) >> 16;
-
-	flush_invalidate((u32)qtd, sizeof(struct qTD), flush);
-	if (ptr && len)
-		flush_invalidate((u32)ptr, len, flush);
-}
-
-
-static inline struct QH *qh_addr(struct QH *qh)
-{
-	return (struct QH *)((u32)qh & 0xffffffe0);
-}
-
-static void cache_qh(struct QH *qh, int flush)
-{
-	struct qTD *qtd;
-	struct qTD *next;
-	static struct qTD *first_qtd;
-
-	/*
-	 * Walk the QH list and flush/invalidate all entries
-	 */
-	while (1) {
-		flush_invalidate((u32)qh_addr(qh), sizeof(struct QH), flush);
-		if ((u32)qh & QH_LINK_TYPE_QH)
-			break;
-		qh = qh_addr(qh);
-		qh = (struct QH *)qh->qh_link;
-	}
-	qh = qh_addr(qh);
-
-	/*
-	 * Save first qTD pointer, needed for invalidating pass on this QH
-	 */
-	if (flush)
-		first_qtd = qtd = (struct qTD *)(*(u32 *)&qh->qh_overlay &
-						 0xffffffe0);
-	else
-		qtd = first_qtd;
-
-	/*
-	 * Walk the qTD list and flush/invalidate all entries
-	 */
-	while (1) {
-		if (qtd == NULL)
-			break;
-		cache_qtd(qtd, flush);
-		next = (struct qTD *)((u32)qtd->qt_next & 0xffffffe0);
-		if (next == qtd)
-			break;
-		qtd = next;
-	}
-}
-
-static inline void ehci_flush_dcache(struct QH *qh)
-{
-	cache_qh(qh, 1);
-}
-
-static inline void ehci_invalidate_dcache(struct QH *qh)
-{
-	cache_qh(qh, 0);
-}
-#else /* CONFIG_EHCI_DCACHE */
-/*
- *
- */
-static inline void ehci_flush_dcache(struct QH *qh)
-{
-}
-
-static inline void ehci_invalidate_dcache(struct QH *qh)
-{
-}
-#endif /* CONFIG_EHCI_DCACHE */
-
 void __ehci_powerup_fixup(uint32_t *status_reg, uint32_t *reg)
 {
 	mdelay(50);
@@ -263,12 +170,20 @@ out:
 
 static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 {
-	uint32_t addr, delta, next;
+	uint32_t delta, next;
+	uint32_t addr = (uint32_t)buf;
+	size_t rsz = roundup(sz, 32);
 	int idx;
 
-	addr = (uint32_t) buf;
+	if (sz != rsz)
+		debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz);
+
+	if (addr & ~31)
+		debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf);
+
 	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;
@@ -317,6 +232,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	memset(&qh, 0, sizeof(struct QH));
 	memset(qtd, 0, sizeof(qtd));
 
+	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
+
 	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;
@@ -337,9 +254,6 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	tdp = &qh.qh_overlay.qt_next;
 
-	toggle =
-	    usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
-
 	if (req != NULL) {
 		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
@@ -391,7 +305,10 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh | QH_LINK_TYPE_QH);
 
 	/* Flush dcache */
-	ehci_flush_dcache(&qh_list);
+	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));
 
 	usbsts = ehci_readl(&hcor->or_usbsts);
 	ehci_writel(&hcor->or_usbsts, (usbsts & 0x3f));
@@ -414,7 +331,13 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	timeout = USB_TIMEOUT_MS(pipe);
 	do {
 		/* Invalidate dcache */
-		ehci_invalidate_dcache(&qh_list);
+		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)qtd,
+			(uint32_t)qtd + sizeof(qtd));
+
 		token = hc32_to_cpu(vtd->qt_token);
 		if (!(token & 0x80))
 			break;
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics in EHCI-HCD
  2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
                   ` (2 preceding siblings ...)
  2012-04-09  3:09 ` [U-Boot] [PATCH 3/4] USB: Drop cache flush bloat in EHCI-HCD Marek Vasut
@ 2012-04-09  3:09 ` Marek Vasut
  2012-04-27  5:31   ` Mike Frysinger
  2012-04-09  3:12 ` [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
  4 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:09 UTC (permalink / raw)
  To: u-boot

The construction of QH and qTD lists in ehci_submit_async() call is cryptic
business, add at least a bit of comments so if someone is reading it, he can at
least reference the intel ehci manual (ehci-r10.pdf).

Signed-off-by: Marek Vasut <marex@denx.de>
---
 drivers/usb/host/ehci-hcd.c |   38 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 55fe067..524bc41 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -234,6 +234,16 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
 
+	/*
+	 * Setup QH (3.6 in ehci-r10.pdf)
+	 *
+	 *   qh_link ................. 03-00 H
+	 *   qh_endpt1 ............... 07-04 H
+	 *   qh_endpt2 ............... 0B-08 H
+	 * - qh_curtd
+	 *   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);
 	c = (usb_pipespeed(pipe) != USB_SPEED_HIGH &&
 	     usb_pipeendpoint(pipe) == 0) ? 1 : 0;
@@ -255,6 +265,15 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	tdp = &qh.qh_overlay.qt_next;
 
 	if (req != NULL) {
+		/*
+		 * Setup request qTD (3.5 in ehci-r10.pdf)
+		 *
+		 *   qt_next ................ 03-00 H
+		 *   qt_altnext ............. 07-04 H
+		 *   qt_token ............... 0B-08 H
+		 *
+		 *   [ buffer, buffer_hi ] loaded with "req".
+		 */
 		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (0 << 31) |
@@ -265,12 +284,22 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 			debug("unable construct SETUP td\n");
 			goto fail;
 		}
+		/* Update previous qTD! */
 		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
 		tdp = &qtd[qtd_counter++].qt_next;
 		toggle = 1;
 	}
 
 	if (length > 0 || req == NULL) {
+		/*
+		 * Setup request qTD (3.5 in ehci-r10.pdf)
+		 *
+		 *   qt_next ................ 03-00 H
+		 *   qt_altnext ............. 07-04 H
+		 *   qt_token ............... 0B-08 H
+		 *
+		 *   [ buffer, buffer_hi ] loaded with "buffer".
+		 */
 		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (toggle << 31) |
@@ -284,11 +313,19 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 			debug("unable construct DATA td\n");
 			goto fail;
 		}
+		/* Update previous qTD! */
 		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
 		tdp = &qtd[qtd_counter++].qt_next;
 	}
 
 	if (req != NULL) {
+		/*
+		 * Setup request qTD (3.5 in ehci-r10.pdf)
+		 *
+		 *   qt_next ................ 03-00 H
+		 *   qt_altnext ............. 07-04 H
+		 *   qt_token ............... 0B-08 H
+		 */
 		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
 		token = (toggle << 31) |
@@ -298,6 +335,7 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 		    (3 << 10) |
 		    ((usb_pipein(pipe) ? 0 : 1) << 8) | (0x80 << 0);
 		qtd[qtd_counter].qt_token = cpu_to_hc32(token);
+		/* Update previous qTD! */
 		*tdp = cpu_to_hc32((uint32_t)&qtd[qtd_counter]);
 		tdp = &qtd[qtd_counter++].qt_next;
 	}
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled
  2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
                   ` (3 preceding siblings ...)
  2012-04-09  3:09 ` [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics " Marek Vasut
@ 2012-04-09  3:12 ` Marek Vasut
  2012-04-09  7:33   ` puneets
  4 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  3:12 UTC (permalink / raw)
  To: u-boot

Dear Marek Vasut,

> This patchset reworks the USB core and EHCI-HCD to work with data caches.
> 
> Marek Vasut (3):
>   USB: Drop ehci_alloc/ehci_free in ehci-hcd
>   USB: Drop cache flush bloat in EHCI-HCD
>   USB: Document the QH and qTD antics in EHCI-HCD
> 
> Puneet Saxena (1):
>   USB: Align buffers at cacheline
> 
>  common/cmd_usb.c            |    3 +-
>  common/usb.c                |   22 ++--
>  common/usb_hub.c            |   27 ++--
>  common/usb_storage.c        |   59 +++++-----
>  disk/part_dos.c             |    2 +-
>  drivers/usb/host/ehci-hcd.c |  284
> ++++++++++++++----------------------------- include/scsi.h              | 
>   4 +-
>  include/usb.h               |    4 +-
>  8 files changed, 155 insertions(+), 250 deletions(-)
> 
> Cc: Puneet Saxena <puneets@nvidia.com>

Puneet, can you please test this stuff? It should fix your cache misalignment 
issues. Mine are now gone and USB works with caches on too.

I'd like to queue this for -next.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4 V2] USB: Drop cache flush bloat in EHCI-HCD
  2012-04-09  3:09 ` [U-Boot] [PATCH 3/4] USB: Drop cache flush bloat in EHCI-HCD Marek Vasut
@ 2012-04-09  4:47   ` Marek Vasut
  2012-05-24  6:57     ` Anatolij Gustschin
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  4:47 UTC (permalink / raw)
  To: u-boot

Discard the creepy cache flushing mechanisms in ehci-hcd.c and replace them with
more straightforward flushing. In the new approach, the flushing takes place
directly in ehci_submit_async() call instead of going through the QH list and
flushing all members and buffers. This discards a lot of weird bit operations
on the members of QH and qTD structures.

NOTE: Certainly, this flushes even qTDs which are possibly unused in some
transactions, though the overhead of the previous code was much higher than is
the overhead of flushing two more cache lines (which most probably aren't even
cached).

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Puneet Saxena <puneets@nvidia.com>
---
 drivers/usb/host/ehci-hcd.c |  127 +++++++++----------------------------------
 1 files changed, 27 insertions(+), 100 deletions(-)

V2: Also flush the data buffer.
    Fix typo in address alignment checking in ehci_td_buffer()

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 8ac0b44..9f0991c 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -108,99 +108,6 @@ static struct descriptor {
 #define ehci_is_TDI()	(0)
 #endif
 
-#if defined(CONFIG_EHCI_DCACHE)
-/*
- * Routines to handle (flush/invalidate) the dcache for the QH and qTD
- * structures and data buffers. This is needed on platforms using this
- * EHCI support with dcache enabled.
- */
-static void flush_invalidate(u32 addr, int size, int flush)
-{
-	if (flush)
-		flush_dcache_range(addr, addr + size);
-	else
-		invalidate_dcache_range(addr, addr + size);
-}
-
-static void cache_qtd(struct qTD *qtd, int flush)
-{
-	u32 *ptr = (u32 *)qtd->qt_buffer[0];
-	int len = (qtd->qt_token & 0x7fff0000) >> 16;
-
-	flush_invalidate((u32)qtd, sizeof(struct qTD), flush);
-	if (ptr && len)
-		flush_invalidate((u32)ptr, len, flush);
-}
-
-
-static inline struct QH *qh_addr(struct QH *qh)
-{
-	return (struct QH *)((u32)qh & 0xffffffe0);
-}
-
-static void cache_qh(struct QH *qh, int flush)
-{
-	struct qTD *qtd;
-	struct qTD *next;
-	static struct qTD *first_qtd;
-
-	/*
-	 * Walk the QH list and flush/invalidate all entries
-	 */
-	while (1) {
-		flush_invalidate((u32)qh_addr(qh), sizeof(struct QH), flush);
-		if ((u32)qh & QH_LINK_TYPE_QH)
-			break;
-		qh = qh_addr(qh);
-		qh = (struct QH *)qh->qh_link;
-	}
-	qh = qh_addr(qh);
-
-	/*
-	 * Save first qTD pointer, needed for invalidating pass on this QH
-	 */
-	if (flush)
-		first_qtd = qtd = (struct qTD *)(*(u32 *)&qh->qh_overlay &
-						 0xffffffe0);
-	else
-		qtd = first_qtd;
-
-	/*
-	 * Walk the qTD list and flush/invalidate all entries
-	 */
-	while (1) {
-		if (qtd == NULL)
-			break;
-		cache_qtd(qtd, flush);
-		next = (struct qTD *)((u32)qtd->qt_next & 0xffffffe0);
-		if (next == qtd)
-			break;
-		qtd = next;
-	}
-}
-
-static inline void ehci_flush_dcache(struct QH *qh)
-{
-	cache_qh(qh, 1);
-}
-
-static inline void ehci_invalidate_dcache(struct QH *qh)
-{
-	cache_qh(qh, 0);
-}
-#else /* CONFIG_EHCI_DCACHE */
-/*
- *
- */
-static inline void ehci_flush_dcache(struct QH *qh)
-{
-}
-
-static inline void ehci_invalidate_dcache(struct QH *qh)
-{
-}
-#endif /* CONFIG_EHCI_DCACHE */
-
 void __ehci_powerup_fixup(uint32_t *status_reg, uint32_t *reg)
 {
 	mdelay(50);
@@ -263,12 +170,20 @@ out:
 
 static int ehci_td_buffer(struct qTD *td, void *buf, size_t sz)
 {
-	uint32_t addr, delta, next;
+	uint32_t delta, next;
+	uint32_t addr = (uint32_t)buf;
+	size_t rsz = roundup(sz, 32);
 	int idx;
 
-	addr = (uint32_t) buf;
+	if (sz != rsz)
+		debug("EHCI-HCD: Misaligned buffer size (%08x)\n", sz);
+
+	if (addr & 31)
+		debug("EHCI-HCD: Misaligned buffer address (%p)\n", buf);
+
 	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;
@@ -317,6 +232,8 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	memset(&qh, 0, sizeof(struct QH));
 	memset(qtd, 0, sizeof(qtd));
 
+	toggle = usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
+
 	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;
@@ -337,9 +254,6 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 
 	tdp = &qh.qh_overlay.qt_next;
 
-	toggle =
-	    usb_gettoggle(dev, usb_pipeendpoint(pipe), usb_pipeout(pipe));
-
 	if (req != NULL) {
 		qtd[qtd_counter].qt_next = cpu_to_hc32(QT_NEXT_TERMINATE);
 		qtd[qtd_counter].qt_altnext = cpu_to_hc32(QT_NEXT_TERMINATE);
@@ -391,7 +305,10 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	qh_list.qh_link = cpu_to_hc32((uint32_t)&qh | QH_LINK_TYPE_QH);
 
 	/* Flush dcache */
-	ehci_flush_dcache(&qh_list);
+	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));
 
 	usbsts = ehci_readl(&hcor->or_usbsts);
 	ehci_writel(&hcor->or_usbsts, (usbsts & 0x3f));
@@ -414,13 +331,23 @@ ehci_submit_async(struct usb_device *dev, unsigned long pipe, void *buffer,
 	timeout = USB_TIMEOUT_MS(pipe);
 	do {
 		/* Invalidate dcache */
-		ehci_invalidate_dcache(&qh_list);
+		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)qtd,
+			(uint32_t)qtd + sizeof(qtd));
+
 		token = hc32_to_cpu(vtd->qt_token);
 		if (!(token & 0x80))
 			break;
 		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));
+
 	/* Check that the TD processing happened */
 	if (token & 0x80) {
 		printf("EHCI timed out on TD - token=%#x\n", token);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 1/4 V2] USB: Align buffers at cacheline
  2012-04-09  3:09 ` [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline Marek Vasut
@ 2012-04-09  4:49   ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  4:49 UTC (permalink / raw)
  To: u-boot

From: Puneet Saxena <puneets@nvidia.com>

This avoids cache-alignment warnings shown in console
when a usb command is entered.

Whenever X bytes of unaligned buffer is invalidated, arm core
invalidates X + Y bytes as per the cache line size and throws
these warnings.

Signed-off-by: Puneet Saxena <puneets@nvidia.com>
Signed-off-by: Marek Vasut <marex@denx.de>
---
 common/cmd_usb.c     |    3 +-
 common/usb.c         |   22 +++++++++---------
 common/usb_hub.c     |   27 +++++++++++-----------
 common/usb_storage.c |   59 +++++++++++++++++++++++--------------------------
 disk/part_dos.c      |    6 ++--
 include/scsi.h       |    4 ++-
 include/usb.h        |    4 ++-
 7 files changed, 64 insertions(+), 61 deletions(-)

V2: Fixup part_dos.c, properly align all buffers!

diff --git a/common/cmd_usb.c b/common/cmd_usb.c
index 9eba271..a8e3ae5 100644
--- a/common/cmd_usb.c
+++ b/common/cmd_usb.c
@@ -150,7 +150,8 @@ void usb_display_class_sub(unsigned char dclass, unsigned char subclass,
 
 void usb_display_string(struct usb_device *dev, int index)
 {
-	char buffer[256];
+	ALLOC_CACHE_ALIGN_BUFFER(char, buffer, 256);
+
 	if (index != 0) {
 		if (usb_string(dev, index, &buffer[0], 256) > 0)
 			printf("String: \"%s\"", buffer);
diff --git a/common/usb.c b/common/usb.c
index 1ec30bc..87ffb79 100644
--- a/common/usb.c
+++ b/common/usb.c
@@ -169,7 +169,7 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 			unsigned short value, unsigned short index,
 			void *data, unsigned short size, int timeout)
 {
-	struct devrequest setup_packet;
+	ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, setup_packet, 1);
 
 	if ((timeout == 0) && (!asynch_allowed)) {
 		/* request for a asynch control pipe is not allowed */
@@ -177,17 +177,17 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
 	}
 
 	/* set setup command */
-	setup_packet.requesttype = requesttype;
-	setup_packet.request = request;
-	setup_packet.value = cpu_to_le16(value);
-	setup_packet.index = cpu_to_le16(index);
-	setup_packet.length = cpu_to_le16(size);
+	setup_packet->requesttype = requesttype;
+	setup_packet->request = request;
+	setup_packet->value = cpu_to_le16(value);
+	setup_packet->index = cpu_to_le16(index);
+	setup_packet->length = cpu_to_le16(size);
 	USB_PRINTF("usb_control_msg: request: 0x%X, requesttype: 0x%X, " \
 		   "value 0x%X index 0x%X length 0x%X\n",
 		   request, requesttype, value, index, size);
 	dev->status = USB_ST_NOT_PROC; /*not yet processed */
 
-	submit_control_msg(dev, pipe, data, size, &setup_packet);
+	submit_control_msg(dev, pipe, data, size, setup_packet);
 	if (timeout == 0)
 		return (int)size;
 
@@ -681,7 +681,7 @@ static int usb_string_sub(struct usb_device *dev, unsigned int langid,
  */
 int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
 {
-	unsigned char mybuf[USB_BUFSIZ];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, mybuf, USB_BUFSIZ);
 	unsigned char *tbuf;
 	int err;
 	unsigned int u, idx;
@@ -781,7 +781,7 @@ int usb_new_device(struct usb_device *dev)
 {
 	int addr, err;
 	int tmp;
-	unsigned char tmpbuf[USB_BUFSIZ];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, tmpbuf, USB_BUFSIZ);
 
 	/* We still haven't set the Address yet */
 	addr = dev->devnum;
@@ -908,8 +908,8 @@ int usb_new_device(struct usb_device *dev)
 	le16_to_cpus(&dev->descriptor.idProduct);
 	le16_to_cpus(&dev->descriptor.bcdDevice);
 	/* only support for one config for now */
-	usb_get_configuration_no(dev, &tmpbuf[0], 0);
-	usb_parse_config(dev, &tmpbuf[0], 0);
+	usb_get_configuration_no(dev, tmpbuf, 0);
+	usb_parse_config(dev, tmpbuf, 0);
 	usb_set_maxpacket(dev);
 	/* we set the default configuration here */
 	if (usb_set_configuration(dev, dev->config.desc.bConfigurationValue)) {
diff --git a/common/usb_hub.c b/common/usb_hub.c
index e0edaad..f35ad95 100644
--- a/common/usb_hub.c
+++ b/common/usb_hub.c
@@ -153,7 +153,7 @@ int hub_port_reset(struct usb_device *dev, int port,
 			unsigned short *portstat)
 {
 	int tries;
-	struct usb_port_status portsts;
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 	unsigned short portstatus, portchange;
 
 	USB_HUB_PRINTF("hub_port_reset: resetting port %d...\n", port);
@@ -162,13 +162,13 @@ int hub_port_reset(struct usb_device *dev, int port,
 		usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET);
 		mdelay(200);
 
-		if (usb_get_port_status(dev, port + 1, &portsts) < 0) {
+		if (usb_get_port_status(dev, port + 1, portsts) < 0) {
 			USB_HUB_PRINTF("get_port_status failed status %lX\n",
 					dev->status);
 			return -1;
 		}
-		portstatus = le16_to_cpu(portsts.wPortStatus);
-		portchange = le16_to_cpu(portsts.wPortChange);
+		portstatus = le16_to_cpu(portsts->wPortStatus);
+		portchange = le16_to_cpu(portsts->wPortChange);
 
 		USB_HUB_PRINTF("portstatus %x, change %x, %s\n",
 				portstatus, portchange,
@@ -206,19 +206,19 @@ int hub_port_reset(struct usb_device *dev, int port,
 void usb_hub_port_connect_change(struct usb_device *dev, int port)
 {
 	struct usb_device *usb;
-	struct usb_port_status portsts;
+	ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 	unsigned short portstatus;
 
 	/* Check status */
-	if (usb_get_port_status(dev, port + 1, &portsts) < 0) {
+	if (usb_get_port_status(dev, port + 1, portsts) < 0) {
 		USB_HUB_PRINTF("get_port_status failed\n");
 		return;
 	}
 
-	portstatus = le16_to_cpu(portsts.wPortStatus);
+	portstatus = le16_to_cpu(portsts->wPortStatus);
 	USB_HUB_PRINTF("portstatus %x, change %x, %s\n",
 			portstatus,
-			le16_to_cpu(portsts.wPortChange),
+			le16_to_cpu(portsts->wPortChange),
 			portspeed(portstatus));
 
 	/* Clear the connection change status */
@@ -267,7 +267,8 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
 static int usb_hub_configure(struct usb_device *dev)
 {
 	int i;
-	unsigned char buffer[USB_BUFSIZ], *bitmap;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);
+	unsigned char *bitmap;
 	struct usb_hub_descriptor *descriptor;
 	struct usb_hub_device *hub;
 #ifdef USB_HUB_DEBUG
@@ -389,16 +390,16 @@ static int usb_hub_configure(struct usb_device *dev)
 	usb_hub_power_on(hub);
 
 	for (i = 0; i < dev->maxchild; i++) {
-		struct usb_port_status portsts;
+		ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
 		unsigned short portstatus, portchange;
 
-		if (usb_get_port_status(dev, i + 1, &portsts) < 0) {
+		if (usb_get_port_status(dev, i + 1, portsts) < 0) {
 			USB_HUB_PRINTF("get_port_status failed\n");
 			continue;
 		}
 
-		portstatus = le16_to_cpu(portsts.wPortStatus);
-		portchange = le16_to_cpu(portsts.wPortChange);
+		portstatus = le16_to_cpu(portsts->wPortStatus);
+		portchange = le16_to_cpu(portsts->wPortChange);
 		USB_HUB_PRINTF("Port %d Status %X Change %X\n",
 				i + 1, portstatus, portchange);
 
diff --git a/common/usb_storage.c b/common/usb_storage.c
index 1208333..faad237 100644
--- a/common/usb_storage.c
+++ b/common/usb_storage.c
@@ -79,8 +79,7 @@ static const unsigned char us_direction[256/8] = {
 };
 #define US_DIRECTION(x) ((us_direction[x>>3] >> (x & 7)) & 1)
 
-static unsigned char usb_stor_buf[512];
-static ccb usb_ccb;
+static ccb usb_ccb __attribute__((aligned(ARCH_DMA_MINALIGN)));
 
 /*
  * CBI style
@@ -210,17 +209,17 @@ int usb_stor_info(void)
 static unsigned int usb_get_max_lun(struct us_data *us)
 {
 	int len;
-	unsigned char result;
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, result, 1);
 	len = usb_control_msg(us->pusb_dev,
 			      usb_rcvctrlpipe(us->pusb_dev, 0),
 			      US_BBB_GET_MAX_LUN,
 			      USB_TYPE_CLASS | USB_RECIP_INTERFACE | USB_DIR_IN,
 			      0, us->ifnum,
-			      &result, sizeof(result),
+			      result, sizeof(char),
 			      USB_CNTL_TIMEOUT * 5);
 	USB_STOR_PRINTF("Get Max LUN -> len = %i, result = %i\n",
-			len, (int) result);
-	return (len > 0) ? result : 0;
+			len, (int) *result);
+	return (len > 0) ? *result : 0;
 }
 
 /*******************************************************************************
@@ -233,9 +232,6 @@ int usb_stor_scan(int mode)
 	unsigned char i;
 	struct usb_device *dev;
 
-	/* GJ */
-	memset(usb_stor_buf, 0, sizeof(usb_stor_buf));
-
 	if (mode == 1)
 		printf("       scanning bus for storage devices... ");
 
@@ -499,7 +495,7 @@ int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 	int actlen;
 	int dir_in;
 	unsigned int pipe;
-	umass_bbb_cbw_t cbw;
+	ALLOC_CACHE_ALIGN_BUFFER(umass_bbb_cbw_t, cbw, 1);
 
 	dir_in = US_DIRECTION(srb->cmd[0]);
 
@@ -522,16 +518,16 @@ int usb_stor_BBB_comdat(ccb *srb, struct us_data *us)
 	/* always OUT to the ep */
 	pipe = usb_sndbulkpipe(us->pusb_dev, us->ep_out);
 
-	cbw.dCBWSignature = cpu_to_le32(CBWSIGNATURE);
-	cbw.dCBWTag = cpu_to_le32(CBWTag++);
-	cbw.dCBWDataTransferLength = cpu_to_le32(srb->datalen);
-	cbw.bCBWFlags = (dir_in ? CBWFLAGS_IN : CBWFLAGS_OUT);
-	cbw.bCBWLUN = srb->lun;
-	cbw.bCDBLength = srb->cmdlen;
+	cbw->dCBWSignature = cpu_to_le32(CBWSIGNATURE);
+	cbw->dCBWTag = cpu_to_le32(CBWTag++);
+	cbw->dCBWDataTransferLength = cpu_to_le32(srb->datalen);
+	cbw->bCBWFlags = (dir_in ? CBWFLAGS_IN : CBWFLAGS_OUT);
+	cbw->bCBWLUN = srb->lun;
+	cbw->bCDBLength = srb->cmdlen;
 	/* copy the command data into the CBW command data buffer */
 	/* DST SRC LEN!!! */
-	memcpy(cbw.CBWCDB, srb->cmd, srb->cmdlen);
-	result = usb_bulk_msg(us->pusb_dev, pipe, &cbw, UMASS_BBB_CBW_SIZE,
+	memcpy(cbw->CBWCDB, srb->cmd, srb->cmdlen);
+	result = usb_bulk_msg(us->pusb_dev, pipe, cbw, UMASS_BBB_CBW_SIZE,
 			      &actlen, USB_CNTL_TIMEOUT * 5);
 	if (result < 0)
 		USB_STOR_PRINTF("usb_stor_BBB_comdat:usb_bulk_msg error\n");
@@ -675,7 +671,7 @@ int usb_stor_BBB_transport(ccb *srb, struct us_data *us)
 	int dir_in;
 	int actlen, data_actlen;
 	unsigned int pipe, pipein, pipeout;
-	umass_bbb_csw_t csw;
+	ALLOC_CACHE_ALIGN_BUFFER(umass_bbb_csw_t, csw, 1);
 #ifdef BBB_XPORT_TRACE
 	unsigned char *ptr;
 	int index;
@@ -733,7 +729,7 @@ st:
 	retry = 0;
 again:
 	USB_STOR_PRINTF("STATUS phase\n");
-	result = usb_bulk_msg(us->pusb_dev, pipein, &csw, UMASS_BBB_CSW_SIZE,
+	result = usb_bulk_msg(us->pusb_dev, pipein, csw, UMASS_BBB_CSW_SIZE,
 				&actlen, USB_CNTL_TIMEOUT*5);
 
 	/* special handling of STALL in STATUS phase */
@@ -753,28 +749,28 @@ again:
 		return USB_STOR_TRANSPORT_FAILED;
 	}
 #ifdef BBB_XPORT_TRACE
-	ptr = (unsigned char *)&csw;
+	ptr = (unsigned char *)csw;
 	for (index = 0; index < UMASS_BBB_CSW_SIZE; index++)
 		printf("ptr[%d] %#x ", index, ptr[index]);
 	printf("\n");
 #endif
 	/* misuse pipe to get the residue */
-	pipe = le32_to_cpu(csw.dCSWDataResidue);
+	pipe = le32_to_cpu(csw->dCSWDataResidue);
 	if (pipe == 0 && srb->datalen != 0 && srb->datalen - data_actlen != 0)
 		pipe = srb->datalen - data_actlen;
-	if (CSWSIGNATURE != le32_to_cpu(csw.dCSWSignature)) {
+	if (CSWSIGNATURE != le32_to_cpu(csw->dCSWSignature)) {
 		USB_STOR_PRINTF("!CSWSIGNATURE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if ((CBWTag - 1) != le32_to_cpu(csw.dCSWTag)) {
+	} else if ((CBWTag - 1) != le32_to_cpu(csw->dCSWTag)) {
 		USB_STOR_PRINTF("!Tag\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus > CSWSTATUS_PHASE) {
+	} else if (csw->bCSWStatus > CSWSTATUS_PHASE) {
 		USB_STOR_PRINTF(">PHASE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus == CSWSTATUS_PHASE) {
+	} else if (csw->bCSWStatus == CSWSTATUS_PHASE) {
 		USB_STOR_PRINTF("=PHASE\n");
 		usb_stor_BBB_reset(us);
 		return USB_STOR_TRANSPORT_FAILED;
@@ -782,7 +778,7 @@ again:
 		USB_STOR_PRINTF("transferred %dB instead of %ldB\n",
 			data_actlen, srb->datalen);
 		return USB_STOR_TRANSPORT_FAILED;
-	} else if (csw.bCSWStatus == CSWSTATUS_FAILED) {
+	} else if (csw->bCSWStatus == CSWSTATUS_FAILED) {
 		USB_STOR_PRINTF("FAILED\n");
 		return USB_STOR_TRANSPORT_FAILED;
 	}
@@ -1343,7 +1339,8 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		      block_dev_desc_t *dev_desc)
 {
 	unsigned char perq, modi;
-	unsigned long cap[2];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned long, cap, 2);
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, usb_stor_buf, 36);
 	unsigned long *capacity, *blksz;
 	ccb *pccb = &usb_ccb;
 
@@ -1367,9 +1364,9 @@ int usb_stor_get_info(struct usb_device *dev, struct us_data *ss,
 		/* drive is removable */
 		dev_desc->removable = 1;
 	}
-	memcpy(&dev_desc->vendor[0], &usb_stor_buf[8], 8);
-	memcpy(&dev_desc->product[0], &usb_stor_buf[16], 16);
-	memcpy(&dev_desc->revision[0], &usb_stor_buf[32], 4);
+	memcpy(&dev_desc->vendor[0], (const void *) &usb_stor_buf[8], 8);
+	memcpy(&dev_desc->product[0], (const void *) &usb_stor_buf[16], 16);
+	memcpy(&dev_desc->revision[0], (const void *) &usb_stor_buf[32], 4);
 	dev_desc->vendor[8] = 0;
 	dev_desc->product[16] = 0;
 	dev_desc->revision[4] = 0;
diff --git a/disk/part_dos.c b/disk/part_dos.c
index b5bcb37..c028aaf 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -87,7 +87,7 @@ static int test_block_type(unsigned char *buffer)
 
 int test_part_dos (block_dev_desc_t *dev_desc)
 {
-	unsigned char buffer[dev_desc->blksz];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 
 	if ((dev_desc->block_read(dev_desc->dev, 0, 1, (ulong *) buffer) != 1) ||
 	    (buffer[DOS_PART_MAGIC_OFFSET + 0] != 0x55) ||
@@ -102,7 +102,7 @@ int test_part_dos (block_dev_desc_t *dev_desc)
 static void print_partition_extended (block_dev_desc_t *dev_desc, int ext_part_sector, int relative,
 							   int part_num)
 {
-	unsigned char buffer[dev_desc->blksz];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 	dos_partition_t *pt;
 	int i;
 
@@ -166,7 +166,7 @@ static int get_partition_info_extended (block_dev_desc_t *dev_desc, int ext_part
 				 int relative, int part_num,
 				 int which_part, disk_partition_t *info)
 {
-	unsigned char buffer[dev_desc->blksz];
+	ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, dev_desc->blksz);
 	dos_partition_t *pt;
 	int i;
 
diff --git a/include/scsi.h b/include/scsi.h
index c52759c..89ae45f 100644
--- a/include/scsi.h
+++ b/include/scsi.h
@@ -26,7 +26,9 @@
 
 typedef struct SCSI_cmd_block{
 	unsigned char		cmd[16];					/* command				   */
-	unsigned char		sense_buf[64];		/* for request sense */
+	/* for request sense */
+	unsigned char		sense_buf[64]
+		__attribute__((aligned(ARCH_DMA_MINALIGN)));
 	unsigned char		status;						/* SCSI Status			 */
 	unsigned char		target;						/* Target ID				 */
 	unsigned char		lun;							/* Target LUN        */
diff --git a/include/usb.h b/include/usb.h
index 48e4bcd..6da91e7 100644
--- a/include/usb.h
+++ b/include/usb.h
@@ -109,7 +109,9 @@ struct usb_device {
 	int epmaxpacketout[16];		/* OUTput endpoint specific maximums */
 
 	int configno;			/* selected config number */
-	struct usb_device_descriptor descriptor; /* Device Descriptor */
+	/* Device Descriptor */
+	struct usb_device_descriptor descriptor
+		__attribute__((aligned(ARCH_DMA_MINALIGN)));
 	struct usb_config config; /* config descriptor */
 
 	int have_langid;		/* whether string_langid is valid yet */
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled
  2012-04-09  3:12 ` [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
@ 2012-04-09  7:33   ` puneets
  2012-04-09  7:41     ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: puneets @ 2012-04-09  7:33 UTC (permalink / raw)
  To: u-boot

Dear Marek,
After applying these 4 patches I don't see any alignment spew and it 
detects the
mass storage device perfectly though I see a warning "EHCI timed out on 
TD - token=0x80008c80"
consistently in each "usb reset".

Thanx & Regards,
Puneet


On Monday 09 April 2012 08:42 AM, Marek Vasut wrote:
> Dear Marek Vasut,
>
>> This patchset reworks the USB core and EHCI-HCD to work with data caches.
>>
>> Marek Vasut (3):
>>    USB: Drop ehci_alloc/ehci_free in ehci-hcd
>>    USB: Drop cache flush bloat in EHCI-HCD
>>    USB: Document the QH and qTD antics in EHCI-HCD
>>
>> Puneet Saxena (1):
>>    USB: Align buffers at cacheline
>>
>>   common/cmd_usb.c            |    3 +-
>>   common/usb.c                |   22 ++--
>>   common/usb_hub.c            |   27 ++--
>>   common/usb_storage.c        |   59 +++++-----
>>   disk/part_dos.c             |    2 +-
>>   drivers/usb/host/ehci-hcd.c |  284
>> ++++++++++++++----------------------------- include/scsi.h              |
>>    4 +-
>>   include/usb.h               |    4 +-
>>   8 files changed, 155 insertions(+), 250 deletions(-)
>>
>> Cc: Puneet Saxena<puneets@nvidia.com>
> Puneet, can you please test this stuff? It should fix your cache misalignment
> issues. Mine are now gone and USB works with caches on too.
>
> I'd like to queue this for -next.
>
> Best regards,
> Marek Vasut


-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled
  2012-04-09  7:33   ` puneets
@ 2012-04-09  7:41     ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2012-04-09  7:41 UTC (permalink / raw)
  To: u-boot

Dear puneets,

> Dear Marek,
> After applying these 4 patches I don't see any alignment spew and it
> detects the
> mass storage device perfectly though I see a warning "EHCI timed out on
> TD - token=0x80008c80"
> consistently in each "usb reset".

That's something else (unrelated to this patchset), I saw it in your backlog 
too. Can you try debuging it on top of these patches? Because these patches fix 
a lot of other issues, so let's take them as a base for now. Also, did you 
really test dos partition table and vfat? I had to fix them to work with 
patches, you change to part_dos.c wasn't enough.

> 
> Thanx & Regards,
> Puneet
> 
> On Monday 09 April 2012 08:42 AM, Marek Vasut wrote:
> > Dear Marek Vasut,
> > 
> >> This patchset reworks the USB core and EHCI-HCD to work with data
> >> caches.
> >> 
> >> Marek Vasut (3):
> >>    USB: Drop ehci_alloc/ehci_free in ehci-hcd
> >>    USB: Drop cache flush bloat in EHCI-HCD
> >>    USB: Document the QH and qTD antics in EHCI-HCD
> >> 
> >> Puneet Saxena (1):
> >>    USB: Align buffers at cacheline
> >>   
> >>   common/cmd_usb.c            |    3 +-
> >>   common/usb.c                |   22 ++--
> >>   common/usb_hub.c            |   27 ++--
> >>   common/usb_storage.c        |   59 +++++-----
> >>   disk/part_dos.c             |    2 +-
> >>   drivers/usb/host/ehci-hcd.c |  284
> >> 
> >> ++++++++++++++----------------------------- include/scsi.h             
> >> |
> >> 
> >>    4 +-
> >>   
> >>   include/usb.h               |    4 +-
> >>   8 files changed, 155 insertions(+), 250 deletions(-)
> >> 
> >> Cc: Puneet Saxena<puneets@nvidia.com>
> > 
> > Puneet, can you please test this stuff? It should fix your cache
> > misalignment issues. Mine are now gone and USB works with caches on too.
> > 
> > I'd like to queue this for -next.
> > 
> > Best regards,
> > Marek Vasut
> 
> ---------------------------------------------------------------------------
> -------- This email message is for the sole use of the intended
> recipient(s) and may contain confidential information.  Any unauthorized
> review, use, disclosure or distribution is prohibited.  If you are not the
> intended recipient, please contact the sender by reply email and destroy
> all copies of the original message.
> ---------------------------------------------------------------------------
> --------

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics in EHCI-HCD
  2012-04-09  3:09 ` [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics " Marek Vasut
@ 2012-04-27  5:31   ` Mike Frysinger
  0 siblings, 0 replies; 15+ messages in thread
From: Mike Frysinger @ 2012-04-27  5:31 UTC (permalink / raw)
  To: u-boot

On Sunday 08 April 2012 23:09:52 Marek Vasut wrote:
> The construction of QH and qTD lists in ehci_submit_async() call is cryptic
> business, add at least a bit of comments so if someone is reading it, he
> can at least reference the intel ehci manual (ehci-r10.pdf).

nice
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120427/8a5711a1/attachment.pgp>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4 V2] USB: Drop cache flush bloat in EHCI-HCD
  2012-04-09  4:47   ` [U-Boot] [PATCH 3/4 V2] " Marek Vasut
@ 2012-05-24  6:57     ` Anatolij Gustschin
  2012-05-24 13:21       ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Anatolij Gustschin @ 2012-05-24  6:57 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Mon,  9 Apr 2012 06:47:31 +0200
Marek Vasut <marex@denx.de> wrote:

> Discard the creepy cache flushing mechanisms in ehci-hcd.c and replace them with
> more straightforward flushing. In the new approach, the flushing takes place
> directly in ehci_submit_async() call instead of going through the QH list and
> flushing all members and buffers. This discards a lot of weird bit operations
> on the members of QH and qTD structures.
> 
> NOTE: Certainly, this flushes even qTDs which are possibly unused in some
> transactions, though the overhead of the previous code was much higher than is
> the overhead of flushing two more cache lines (which most probably aren't even
> cached).
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Puneet Saxena <puneets@nvidia.com>
> ---
>  drivers/usb/host/ehci-hcd.c |  127 +++++++++----------------------------------
>  1 files changed, 27 insertions(+), 100 deletions(-)

Unfortunately this patch breaks compiling for many powerpc boards, mpc512x,
mpc83xx, mpc85xx and QorIQ Px based with USB support enabled.

Thanks,
Anatolij

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4 V2] USB: Drop cache flush bloat in EHCI-HCD
  2012-05-24  6:57     ` Anatolij Gustschin
@ 2012-05-24 13:21       ` Marek Vasut
  2012-05-29  8:24         ` Liu Gang
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Vasut @ 2012-05-24 13:21 UTC (permalink / raw)
  To: u-boot

Dear Anatolij Gustschin,

> Hi Marek,
> 
> On Mon,  9 Apr 2012 06:47:31 +0200
> 
> Marek Vasut <marex@denx.de> wrote:
> > Discard the creepy cache flushing mechanisms in ehci-hcd.c and replace
> > them with more straightforward flushing. In the new approach, the
> > flushing takes place directly in ehci_submit_async() call instead of
> > going through the QH list and flushing all members and buffers. This
> > discards a lot of weird bit operations on the members of QH and qTD
> > structures.
> > 
> > NOTE: Certainly, this flushes even qTDs which are possibly unused in some
> > transactions, though the overhead of the previous code was much higher
> > than is the overhead of flushing two more cache lines (which most
> > probably aren't even cached).
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Puneet Saxena <puneets@nvidia.com>
> > ---
> > 
> >  drivers/usb/host/ehci-hcd.c |  127
> >  +++++++++---------------------------------- 1 files changed, 27
> >  insertions(+), 100 deletions(-)
> 
> Unfortunately this patch breaks compiling for many powerpc boards, mpc512x,
> mpc83xx, mpc85xx and QorIQ Px based with USB support enabled.

Because they have broken cache implementation, right? I mean, they have their 
own snooping methods, so they don't need the cache flushing at all, but then, if 
they don't, these methods (dcache_flush() etc) should be optimized to empty 
functions. Maybe we should implement them for these CPUs then? btw. I thought 
these compiled before, hm...

> Thanks,
> Anatolij

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4 V2] USB: Drop cache flush bloat in EHCI-HCD
  2012-05-24 13:21       ` Marek Vasut
@ 2012-05-29  8:24         ` Liu Gang
  2012-05-29  8:36           ` Marek Vasut
  0 siblings, 1 reply; 15+ messages in thread
From: Liu Gang @ 2012-05-29  8:24 UTC (permalink / raw)
  To: u-boot

Hi Marek,

On Thu, 2012-05-24 at 15:21 +0200, Marek Vasut wrote:
 
> > >  drivers/usb/host/ehci-hcd.c |  127
> > >  +++++++++---------------------------------- 1 files changed, 27
> > >  insertions(+), 100 deletions(-)
> > 
> > Unfortunately this patch breaks compiling for many powerpc boards, mpc512x,
> > mpc83xx, mpc85xx and QorIQ Px based with USB support enabled.
> 
> Because they have broken cache implementation, right? I mean, they have their 
> own snooping methods, so they don't need the cache flushing at all, but then, if 
> they don't, these methods (dcache_flush() etc) should be optimized to empty 
> functions. Maybe we should implement them for these CPUs then? btw. I thought 
> these compiled before, hm...

Now this patch has been applied at "master" branch, but the building for
powerpc boards as Anatolij mentioned will be failed.
So Which platforms this patch applies to? If they all can not flush
cache like the powerpc snooping methods?

Thanks!

Best Regards!

Liu Gang

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [U-Boot] [PATCH 3/4 V2] USB: Drop cache flush bloat in EHCI-HCD
  2012-05-29  8:24         ` Liu Gang
@ 2012-05-29  8:36           ` Marek Vasut
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Vasut @ 2012-05-29  8:36 UTC (permalink / raw)
  To: u-boot

Dear Liu Gang,

> Hi Marek,
> 
> On Thu, 2012-05-24 at 15:21 +0200, Marek Vasut wrote:
> > > >  drivers/usb/host/ehci-hcd.c |  127
> > > >  +++++++++---------------------------------- 1 files changed, 27
> > > >  insertions(+), 100 deletions(-)
> > > 
> > > Unfortunately this patch breaks compiling for many powerpc boards,
> > > mpc512x, mpc83xx, mpc85xx and QorIQ Px based with USB support enabled.
> > 
> > Because they have broken cache implementation, right? I mean, they have
> > their own snooping methods, so they don't need the cache flushing at
> > all, but then, if they don't, these methods (dcache_flush() etc) should
> > be optimized to empty functions. Maybe we should implement them for
> > these CPUs then? btw. I thought these compiled before, hm...
> 
> Now this patch has been applied at "master" branch, but the building for
> powerpc boards as Anatolij mentioned will be failed.
> So Which platforms this patch applies to? If they all can not flush
> cache like the powerpc snooping methods?

I already submitted a patch for this issue

http://www.mail-archive.com/u-boot at lists.denx.de/msg84629.html

> Thanks!
> 
> Best Regards!
> 
> Liu Gang

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2012-05-29  8:36 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-09  3:09 [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
2012-04-09  3:09 ` [U-Boot] [PATCH 1/4] USB: Align buffers at cacheline Marek Vasut
2012-04-09  4:49   ` [U-Boot] [PATCH 1/4 V2] " Marek Vasut
2012-04-09  3:09 ` [U-Boot] [PATCH 2/4] USB: Drop ehci_alloc/ehci_free in ehci-hcd Marek Vasut
2012-04-09  3:09 ` [U-Boot] [PATCH 3/4] USB: Drop cache flush bloat in EHCI-HCD Marek Vasut
2012-04-09  4:47   ` [U-Boot] [PATCH 3/4 V2] " Marek Vasut
2012-05-24  6:57     ` Anatolij Gustschin
2012-05-24 13:21       ` Marek Vasut
2012-05-29  8:24         ` Liu Gang
2012-05-29  8:36           ` Marek Vasut
2012-04-09  3:09 ` [U-Boot] [PATCH 4/4] USB: Document the QH and qTD antics " Marek Vasut
2012-04-27  5:31   ` Mike Frysinger
2012-04-09  3:12 ` [U-Boot] [PATCH 0/4] USB: Fix EHCI to work with data cache enabled Marek Vasut
2012-04-09  7:33   ` puneets
2012-04-09  7:41     ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.