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

[Marek Vasut]

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

This is really weird solution :-(
> 
> [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...

Can you please point me to such discussion?

Thanks!