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

[Simon Glass]

Hi Marek,

On Mon, Feb 27, 2012 at 8:49 AM, Marek Vasut <marex at denx.de> 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 at nvidia.com>
>> Signed-off-by: Jim Lin <jilin at nvidia.com>
>> ---
>>
>
> First of all, thanks for this patch, it really helps. But, there are a few
> things that concern me.
>
>> 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.
>>
>>  common/cmd_usb.c            |    3 +-
>>  common/usb.c                |   61
>> ++++++++++++++++++++++++------------------ common/usb_storage.c        |
>> 59 +++++++++++++++++++---------------------- disk/part_dos.c             |
>>    2 +-
>>  drivers/usb/host/ehci-hcd.c |    8 +++++
>>  include/scsi.h              |    4 ++-
>>  6 files changed, 77 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);
>
> Why not memalign(), do you want to allocate on stack so badly ?

This issue came up with MMC and there was a strong request not to
sprinkle the code with malloc. In fact the macro above was devised at
some cost just to solve this problem.

[snip]
>>  {
>>       int tries;
>> -     struct usb_port_status portsts;
>> +     ALLOC_CACHE_ALIGN_BUFFER(struct usb_port_status, portsts, 1);
>
> Looking at all this stuff, it's already allocated on stack. Ok, I already
> outlined first option -- memalign(), but that'd really put quite a strain on the
> memory allocator -- the other option (and maybe even better) is to use
> __attribute__((aligned)), which will allow the compiler to reorder data
> allocated on the stack so the array you create is aligned and not much space
> around on the stack is wasted. Apparently, ALLOC_CACHE_ALIGN_BUFFER() wastes a
> lot of space on the stack.

There was quite a bit of discussion about this related to MMC. I think
the current solution is the result of that discussion...

Regards,
Simon