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

[puneets <puneets@nvidia.com>]

Hi Mike,
On Tuesday 06 March 2012 08:37 AM, Mike Frysinger wrote:
> * PGP Signed by an unknown key
>
> On Monday 05 March 2012 09:46:21 Puneet Saxena wrote:
>> As DMA expects the buffers to be equal and larger then
>> cache lines, This aligns buffers at cacheline.
> i don't think this statement is true.  DMA doesn't care about alignment (well,
> some do, but it's not related to cache lines but rather some other restriction
> in the peripheral DMA itself).  what does matter is that cache operations
> operate on cache lines and not individual bytes.  hence the core arm code was
> updated to warn when someone told it to invalidate X bytes but the hardware
> literally could not, so it had to invalidate X + Y bytes.
>
Agreed, Will update the commit message in next patchset.
>> --- a/drivers/usb/host/ehci-hcd.c
>> +++ b/drivers/usb/host/ehci-hcd.c
>>
>>   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
> i think this is wrong and merely hides the errors from higher up instead of
> fixing them.  the point of the warning was to tell you that the code was
> invalidating *too many* bytes.  this code still invalidates too many bytes
> without any justification as for why it's OK to do here.  further, this code
> path only matters to the invalidation logic, not the flush logic.
> -mike
>
The sole purpose of this patch to remove the warnings as start/stop 
address sent for invalidating
is unaligned. Without this patch code works fine but with lots of 
spew...Which we don't want and discussed
in earlier thread which Simon posted. Please have a look on following link.

As I understood, you agree that we need to align start/stop buffer 
address and also agree that
to align stop address we need to align size as start address is already 
aligned.
Now, "why its OK to do here"?
We could have aligned the size in two places, cache_qtd() and cache_qh() 
but then we need to place alignment check
at all the places where size is passed. So I thought better Aligning at 
flush_invalidate() and "ALIGN" macro does not
increase the size if size is already aligned.


> * Unknown Key
> * 0xE837F581
Thanx & Regards,
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.
-----------------------------------------------------------------------------------