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

[Marek Vasut <marex@denx.de>]

> On Friday 02 March 2012 07:16 PM, Marek Vasut 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@nvidia.com>
> >> ---
> >> 
> >> Changes for V3:
> >>      - Removed local descriptor elements copy to global descriptor
> >>      elements - Removed "Signed-off-by: Jim Lin<jilin@nvidia.com>" from
> >>      commit
> >> 
> >> message
> >> 
> >> 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.
> >> 
> >>   common/cmd_usb.c            |    3 +-
> >>   common/usb.c                |   56
> >> 
> >> ++++++++++++++++++++++------------------ 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(+), 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..42a44e2 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;
> >> 
> >> @@ -909,7 +910,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));
> >> 
> >>   	if (err<  tmp) {
> >>   	
> >>   		if (err<  0)
> >>   		
> >>   			printf("unable to get device descriptor (error=%d)\n",
> >> 
> >> @@ -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));
> > 
> > Hey, it's almost perfect!
> > 
> > Just one last question -- why do you need to copy this stuff?
> 
> We need to copy this stuff as I spoke in previous reply  -
> "memcpy" is needed to configure Usb version, vendor id, prod Id ..etc
> 
> >  of the device. Its also useful to hook actual device driver and detect
> >  no. of configuration supported. The effect can be verified
> 
> with/without using memcpy
> in "usb tree" and "usb info" commands.
> 
> > It's because dev-
> > 
> >> descriptor is unaligned?
> 
> No, As global dev - descriptor is unaligned we are passing aligned local
> dev-desc and memcpy in global.

Can't we just align the global one?
M