Hi Marek, On Mon, Feb 27, 2012 at 9:11 AM, Marek Vasut <ma...@denx.de> wrote: >> Hi Marek, >> >> On Mon, Feb 27, 2012 at 8:49 AM, Marek Vasut <ma...@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 <pune...@nvidia.com> >> >> Signed-off-by: Jim Lin <ji...@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 :-(
It's not pretty under the hood but it solves the problem well for MMC. Apart from the alignment of buffers, it is as efficient as the original code. >> >> [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? Start here perhaps and look at the whole thread: http://lists.denx.de/pipermail/u-boot/2011-August/099152.html > > Thanks! Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot