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

[Scott Wood <scottwood@freescale.com>]

On 08/29/2011 03:58 PM, Anton Staaf wrote:
> On Mon, Aug 29, 2011 at 1:47 PM, Scott Wood <scottwood@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.

With the version in that patch I get the slightly different "error:
initializer element is not computable at load time".  Seems like whether
you cast the address to (type *) or (void *) determines which error you
get.  This is with GCC 4.5.1 (powerpc) and 4.6.0 (x86).  Maybe it's
arch-dependent, based on available relocation types.

Also, shouldn't the array be of type "char" rather than "char *"?

How do you make the declaration static?

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

-std=c99 doesn't help.

The problem isn't the array itself, it's the pointer initializer.

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

So, the complex macro is bad because it obscures things, and this
version is bad because it doesn't? :-)

-Scott