| Message ID | 1314873018-20946-1-git-send-email-l.majewski@samsung.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Sep 1, 2011 at 3:30 AM, Lukasz Majewski <l.majewski@samsung.com> wrote: > > ALLOC_CACHE_ALIGN_BUFFER shall be used in functions, which are using > stack allocated buffers for DMA transfers. > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > CC: Albert ARIBAUD <albert.u.boot@aribaud.net> > --- > Changes for v2: > - ./include/cache.h has been removed and replaced with > simpler macro added to ./include/common.h > Changes for v3: > - change char * to char > - defined table size definition > Changes for v4: > - (type*) added for compiler warning fix > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com> > --- > include/common.h | 5 +++++ > 1 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/include/common.h b/include/common.h > index 12a1074..a74c6e8 100644 > --- a/include/common.h > +++ b/include/common.h > @@ -767,6 +767,11 @@ int cpu_release(int nr, int argc, char * const argv[]); > #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1) > #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask)) > > +#define ALLOC_CACHE_ALIGN_BUFFER(type, name, size) \ > + char __##name[size + CONFIG_SYS_CACHELINE_SIZE - 1]; \ It was pointed out to me that we need to make sure that both ends of the resulting buffer are cache line aligned. Or put another way, that the __##name array has enough padding at the beginning and end that an invalidate will be both aligned to the cache line and not effect anything defined after the array on the stack. So the above definition needs to change to something like: char __##name[ROUND(size, CONFIG_SYS_CACHELINE_SIZE) + CONFIG_SYS_CACHELINE_SIZE - 1]; Another thing that concerns me is that the macro takes a type, but the size parameter is specified in bytes, not units of the size of the type. Would it make sense to specify the size in units of the type? It would make almost no sense to specify a size that wasn't a multiple of the size of the type anyway. If we want to do that the the array definition becomes: char __##name[ROUND(size * sizeof(type), CONFIG_SYS_CACHELINE_SIZE) + CONFIG_SYS_CACHELINE_SIZE - 1]; And finally, the ROUND macro is written such that it will always return a value that is larger than it's first parameter. Thus ROUND(CONFIG_SYS_CACHELINE_SIZE, CONFIG_SYS_CACHELINE_SIZE) withh not equal CONFIG_SYS_CACHELINE_SIZE, but actually 2 * CONFIG_SYS_CACHELINE_SIZE. I'm not sure if this is intentional. In fact, the only use of ROUND that is not to round the value of CONFIG_SYS_MALLOC_LEN to a multiple of 4096 is in the common/cmd_sf.c implementation. And there it looks like the author worked around the behavior of ROUND by passing "len_arg - 1", instead of len_arg. So, it looks like a patch to fix ROUND might be in order as well. I'll try and send one today. -Anton > > + type *name = (type *) ALIGN(((typeof(CONFIG_SYS_CACHELINE_SIZE))\ > + (__##name)), (CONFIG_SYS_CACHELINE_SIZE)); > + > /* Pull in stuff for the build system */ > #ifdef DO_DEPS_ONLY > # include <environment.h> > -- > 1.7.2.3 >
diff --git a/include/common.h b/include/common.h index 12a1074..a74c6e8 100644 --- a/include/common.h +++ b/include/common.h @@ -767,6 +767,11 @@ int cpu_release(int nr, int argc, char * const argv[]); #define ALIGN(x,a) __ALIGN_MASK((x),(typeof(x))(a)-1) #define __ALIGN_MASK(x,mask) (((x)+(mask))&~(mask)) +#define ALLOC_CACHE_ALIGN_BUFFER(type, name, size) \ + char __##name[size + CONFIG_SYS_CACHELINE_SIZE - 1]; \ + type *name = (type *) ALIGN(((typeof(CONFIG_SYS_CACHELINE_SIZE))\ + (__##name)), (CONFIG_SYS_CACHELINE_SIZE)); + /* Pull in stuff for the build system */ #ifdef DO_DEPS_ONLY # include <environment.h>