[U-Boot] [PATCH v5] usb: align buffers at cacheline
Eric Nelson
eric.nelson at boundarydevices.com
Mon Mar 5 14:24:57 CET 2012
On 03/05/2012 12:16 AM, Puneet Saxena wrote:
> As DMA expects the buffers to be equal and larger then
> cache lines, This aligns buffers at cacheline.
>
> Signed-off-by: Puneet Saxena<puneets at nvidia.com>
> ---
> Changes for V4:
> - Added memcpy to copy local descriptor to global descriptor.
> Without that, USB version, class, vendor, product Id...etc is not configured.
> This information is useful for loading correct device driver and possible
> configuration.
>
> Changes for V5:
> - Aligned "usb_device_descriptor" using ARCH_DMA_MINALIGN
>
> common/cmd_usb.c | 3 +-
> common/usb.c | 54 ++++++++++++++++++++++-----------------
> common/usb_storage.c | 59 ++++++++++++++++++++----------------------
> disk/part_dos.c | 2 +-
> drivers/usb/host/ehci-hcd.c | 8 ++++++
> include/scsi.h | 4 ++-
> include/usb.h | 6 +++-
> 7 files changed, 76 insertions(+), 60 deletions(-)
>
> diff --git a/common/cmd_usb.c b/common/cmd_usb.c
> index 320667f..bca9d94 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 6e21ae2..3005012 100644
> --- a/common/usb.c
> +++ b/common/usb.c
> @@ -73,7 +73,6 @@ static struct usb_device usb_dev[USB_MAX_DEVICE];
> static int dev_index;
> static int running;
> static int asynch_allowed;
> -static struct devrequest setup_packet;
>
> char usb_started; /* flag for the started/stopped USB status */
>
> @@ -185,23 +184,25 @@ int usb_control_msg(struct usb_device *dev, unsigned int pipe,
> unsigned short value, unsigned short index,
> void *data, unsigned short size, int timeout)
> {
> + ALLOC_CACHE_ALIGN_BUFFER(struct devrequest, setup_packet,
> + sizeof(struct devrequest));
sizeof(struct devrequest) should be 1, right?
See comment for usage:
http://git.denx.de/?p=u-boot.git;a=blob;f=include/common.h;h=a2c6b27d43cce33d1a00a033e4b33c895c4e1d8d;hb=HEAD#l904
> if ((timeout == 0)&& (!asynch_allowed)) {
> /* request for a asynch control pipe is not allowed */
> return -1;
> }
>
> /* 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;
>
> @@ -698,7 +699,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;
> @@ -798,7 +799,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;
> @@ -919,14 +920,18 @@ int usb_new_device(struct usb_device *dev)
> "(expected %i, got %i)\n", tmp, err);
> return 1;
> }
> +
> + /* Now copy the local device descriptor to global device descriptor*/
> + memcpy(&dev->descriptor, desc, sizeof(dev->descriptor));
> +
> /* correct le values */
> le16_to_cpus(&dev->descriptor.bcdUSB);
> le16_to_cpus(&dev->descriptor.idVendor);
> 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)) {
> @@ -1080,7 +1085,7 @@ static 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);
> @@ -1089,13 +1094,13 @@ static int hub_port_reset(struct usb_device *dev, int port,
> usb_set_port_feature(dev, port + 1, USB_PORT_FEAT_RESET);
> wait_ms(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,
> @@ -1133,19 +1138,19 @@ static 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 */
> @@ -1194,7 +1199,8 @@ void usb_hub_port_connect_change(struct usb_device *dev, int port)
> 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
> @@ -1316,16 +1322,16 @@ 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 de84c8d..88ca390 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/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index d893b2a..eb5220b 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -120,6 +120,14 @@ static struct descriptor {
> */
> static void flush_invalidate(u32 addr, int size, int flush)
> {
> + /*
> + * Size is the bytes actually moved during transaction,
> + * which may not equal to the cache line. This results
> + * stop address passed for invalidating cache may not be aligned.
> + * Therfore making size as multiple of cache line size.
> + */
> + size = ALIGN(size, ARCH_DMA_MINALIGN);
> +
> if (flush)
> flush_dcache_range(addr, addr + size);
> else
> 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 06170cd..8c082c1 100644
> --- a/include/usb.h
> +++ b/include/usb.h
> @@ -109,9 +109,11 @@ struct usb_device {
> int epmaxpacketout[16]; /* OUTput endpoint specific maximums */
>
> int configno; /* selected config number */
> - struct usb_device_descriptor descriptor; /* Device Descriptor */
> - struct usb_config config; /* config 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 */
> int string_langid; /* language ID for strings */
> int (*irq_handle)(struct usb_device *dev);
More information about the U-Boot
mailing list