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

Marek Vasut
2012-02-27T17:11:31Z

> Hi Marek,
>
> On Mon, Feb 27, 2012 at 8:49 AM, Marek Vasut <marex@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@nvidia.com>
> >> Signed-off-by: Jim Lin <jilin@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!