[U-Boot] [PATCH] mmc:dcache: Cache line size aligned internal MMC buffers

[Anton Staaf]

On Mon, Aug 29, 2011 at 1:47 PM, Scott Wood <scottwood at freescale.com> wrote:
> On 08/29/2011 03:12 PM, Anton Staaf wrote:
>> 1) Mikes's macro
>>
>> #define DMA_ALIGN_SIZE(size) \
>>        (((size) + CONFIG_SYS_CACHELINE_SIZE - 1)
>>
>> #define DMA_DECLARE_BUFFER(type, name, size) \
>>        void __##name[DMA_ALIGN_SIZE(size * sizeof(type))]; \
>>        type * name = __##name & ~(CONFIG_SYS_CACHELINE_SIZE - 1));
>>
>> DMA_DECLARE_BUFFER(int, buffer, 100);
>
> This doesn't compile, and it tries to round the buffer down below its
> starting point.

You are correct.  I wrote that one as a modification of mikes initial
proposal.  I should have caught the incorrect rounding when I did.
The patch that Lukasz sent titled "dcache: Dcache line size aligned
stack buffer allocation" has a correct implementation.

> After fixing the more obvious issues, I get "error: initializer element
> is not constant".

I think this requires the use of -std=c99 or GCC extensions.  More
specifics can be found here:
http://gcc.gnu.org/onlinedocs/gcc/Variable-Length.html

> There might be no way to express and-by-constant as a relocation.
>
> You could set the pointer at runtime, though, and remove some of the
> macrification:
>
> #define DMA_ALIGN_SIZE(size) \
>        ((size) + CONFIG_SYS_CACHELINE_SIZE - 1)
> #define DMA_ALIGN_ADDR(addr) \
>        (DMA_ALIGN_SIZE(addr) & (CONFIG_SYS_CACHELINE_SIZE - 1))
>
> int buffer_unaligned[DMA_ALIGN_SIZE(100)];
> int *buffer;
>
> some_init_func()
> {
>        buffer = (int *)(DMA_ALIGN_ADDR((uintptr_t)buffer_unaligned));
> }

:) This was one of my suggestions earlier on a different thread.  It
was rejected there, I believe because it makes things less clear.

>> 3) Use GCC specific alignment attribute:
>>
>> #define CACHLINE_ALIGNED __attribute__ ((aligned (CONFIG_SYS_CACHELINE_SIZE)))
>>
>> int buffer[100] CACHELINE_ALIGNED;
>>
>> Pros: The declaration of the buffer is even simpler and more obvious,
>> no use of alloca at all.
>>
>> Cons: This doesn't work in any version of GCC before October 2010.
>> Meaning that it probably doesn't work in whatever compiler you're
>> using.
>>
>>
>> It's really too bad that this isn't a usable solution.  I suppose that
>> we could switch to it at some point when we expect U-Boot to only be
>> compiled by versions of GCC that support this.  By the way, the
>> failure mode here is pretty bad.  If you compile the above code with
>> an older GCC it will silently fail to align the variable.  :(
>
> If the decision is made to depend on newer compilers, U-Boot could check
> for it and #error out if the compiler is too old (possibly just in the
> files that depend on this feature).

Yup, I think whatever macro we come up with we can simplify it
eventually using the GCC attribute solution and we can make the macro
conditional on compiler version.

Thanks,
    Anton

> -Scott
>
>