[U-Boot] [PATCH v3 1/2] usb: align buffers at cacheline

[Marek Vasut <marex@denx.de>]

> 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 V2:
>     - Use "ARCH_DMA_MINALIGN" directly
>     - Use "ALIGN" to align size as cacheline
>     - Removed headers from usb.h
>     - Send 8 bytes of device descriptor size to read
>       Max packet size
>     scsi.h header is needed to avoid extra memcpy from local buffer
>     to global buffer.
> 
> Changes for V3:
>     - Removed local descriptor elements copy to global descriptor elements
>     - Removed "Signed-off-by: Jim Lin <jilin@nvidia.com>" from commit
> message
> 
>  common/cmd_usb.c            |    3 +-
>  common/usb.c                |   57
> ++++++++++++++++++++++------------------- common/usb_storage.c        |  
> 59 ++++++++++++++++++++---------------------- disk/part_dos.c            
> |    2 +-
>  drivers/usb/host/ehci-hcd.c |    8 ++++++
>  include/scsi.h              |    4 ++-
>  6 files changed, 73 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 63a11c8..191bc5b 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;
> 
> @@ -694,7 +695,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;
> @@ -794,7 +795,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;
> @@ -842,7 +843,10 @@ int usb_new_device(struct usb_device *dev)
>  	dev->epmaxpacketin[0] = 64;
>  	dev->epmaxpacketout[0] = 64;
> 
> -	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc, 64);
> +	desc->bMaxPacketSize0 = 0;
> +	/*8 bytes of the descriptor to read Max packet size*/
> +	err = usb_get_descriptor(dev, USB_DT_DEVICE, 0, desc,
> +			8);

Did some unrelated addition/change creep in here?

>  	if (err < 0) {
>  		USB_PRINTF("usb_new_device: usb_get_descriptor() failed\n");
>  		return 1;
> @@ -905,7 +909,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));
> +				 desc, sizeof(dev->descriptor));

Won't this change (desc) break anything?

>  	if (err < tmp) {
>  		if (err < 0)
>  			printf("unable to get device descriptor (error=%d)\n",

The rest seems fine, from now on it seems to be only matter of trivial fix. 
Thanks for your effort so far!

M