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

[puneets <puneets@nvidia.com>]

On Friday 02 March 2012 08:11 PM, Marek Vasut wrote:
>> 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?
That's what I did in original patch where I am aligning it by adding the 
line

+        /* Device Descriptor */
+#ifdef ARCH_DMA_MINALIGN
+       struct usb_device_descriptor descriptor
+               __attribute__((aligned(ARCH_DMA_MINALIGN)));
+#else
+       struct usb_device_descriptor descriptor;
+#endif

in usb.h Line:112

> M
Thanx,
Puneet

-----------------------------------------------------------------------------------
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.
-----------------------------------------------------------------------------------