[U-Boot] [PATCH v6] usb: align buffers at cacheline

Marek Vasut
2012-03-05T12:03:19Z

Dear Puneet Saxena,

> As DMA expects the buffers to be equal and larger then
> cache lines, This aligns buffers at cacheline.
>
> Signed-off-by: Puneet Saxena <puneets@nvidia.com>
> ---
>
> Changes for V5:
> - Aligned "usb_device_descriptor" using ARCH_DMA_MINALIGN
>
> Changes for V6:
> - Cosmetic changes.
>
> 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 | 4 ++-
> 7 files changed, 75 insertions(+), 59 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));
> 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));
> +

Why is this memcpy() here? Didn't we finally decide to align the global
descriptor so this is unnecessary?

The rest of the patch is perfect, I'm looking forward to testing it. :-)

Best regards,
Marek Vasut