| Message ID | 1403027720-9738-4-git-send-email-taras.kondratiuk@linaro.org |
|---|---|
| State | RFC |
| Headers | show |
Hi, On Tue, Jun 17, 2014 at 8:55 PM, Taras Kondratiuk < taras.kondratiuk@linaro.org> wrote: > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> > --- > include/odp_align.h | 32 +----------- > platform/linux-generic/include/plat/odp_align.h | 61 > +++++++++++++++++++++++ > 2 files changed, 63 insertions(+), 30 deletions(-) > create mode 100644 platform/linux-generic/include/plat/odp_align.h > > diff --git a/include/odp_align.h b/include/odp_align.h > index 83495a8..241b58a 100644 > --- a/include/odp_align.h > +++ b/include/odp_align.h > @@ -14,6 +14,8 @@ > #ifndef ODP_ALIGN_H_ > #define ODP_ALIGN_H_ > > +#include <plat/odp_align.h> > + > #ifdef __cplusplus > extern "C" { > #endif > @@ -43,38 +45,10 @@ extern "C" { > */ > #define ODP_FIELD_SIZEOF(type, member) sizeof(((type *)0)->member) > > -#if defined __x86_64__ || defined __i386__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > - > -#elif defined __arm__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > - > -#elif defined __OCTEON__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 128 > - > -#elif defined __powerpc__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > What's the rule of thumb with renaming ODP_* #defines to PLAT_ODP_*, like in patch 9/18? It would be easier for developers to see what's available in the API, rather then going to a specific platform and look there. Or is it that the defines above are not expected to show up on all platforms? > - > -#else > -#error GCC target not found > -#endif > - > #else > #error Non-gcc compatible compiler > #endif > > -/** Page size */ > -#define ODP_PAGE_SIZE 4096 > - > - > /* > * Round up > */ > @@ -155,7 +129,6 @@ extern "C" { > #define ODP_ALIGNED_PAGE ODP_ALIGNED(ODP_PAGE_SIZE) > > > - > /* > * Check align > */ > @@ -174,7 +147,6 @@ extern "C" { > #define ODP_VAL_IS_POWER_2(x) ((((x)-1) & (x)) == 0) > > > - > #ifdef __cplusplus > } > #endif > diff --git a/platform/linux-generic/include/plat/odp_align.h > b/platform/linux-generic/include/plat/odp_align.h > new file mode 100644 > index 0000000..78f3b38 > --- /dev/null > +++ b/platform/linux-generic/include/plat/odp_align.h > @@ -0,0 +1,61 @@ > +/* Copyright (c) 2013, Linaro Limited > + * All rights reserved. > + * > + * SPDX-License-Identifier: BSD-3-Clause > + */ > + > + > +/** > + * @file > + * > + * ODP alignments > + */ > + > +#ifndef ODP_ALIGN_H_ > +#error This file should be included only into corresponding top level > header > +#else > + > +#ifdef __cplusplus > +extern "C" { > +#endif > + > + > +#ifdef __GNUC__ > + > +#if defined __x86_64__ || defined __i386__ > + > +/** Cache line size */ > +#define ODP_CACHE_LINE_SIZE 64 > + > +#elif defined __arm__ > + > +/** Cache line size */ > +#define ODP_CACHE_LINE_SIZE 64 > + > +#elif defined __OCTEON__ > + > +/** Cache line size */ > +#define ODP_CACHE_LINE_SIZE 128 > + > +#elif defined __powerpc__ > + > +/** Cache line size */ > +#define ODP_CACHE_LINE_SIZE 64 > + > +#else > +#error GCC target not found > +#endif > + > +#else > +#error Non-gcc compatible compiler > +#endif > + > +/** Page size */ > +#define ODP_PAGE_SIZE 4096 > + > + > +#ifdef __cplusplus > +} > +#endif > + > +#endif > -- > 1.7.9.5 > > > _______________________________________________ > lng-odp mailing list > lng-odp@lists.linaro.org > http://lists.linaro.org/mailman/listinfo/lng-odp >
On 06/19/2014 02:10 PM, Ciprian Barbu wrote: > Hi, > > > On Tue, Jun 17, 2014 at 8:55 PM, Taras Kondratiuk > <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>> wrote: > > Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org > <mailto:taras.kondratiuk@linaro.org>> > --- > include/odp_align.h | 32 +----------- > platform/linux-generic/include/plat/odp_align.h | 61 > +++++++++++++++++++++++ > 2 files changed, 63 insertions(+), 30 deletions(-) > create mode 100644 platform/linux-generic/include/plat/odp_align.h > > diff --git a/include/odp_align.h b/include/odp_align.h > index 83495a8..241b58a 100644 > --- a/include/odp_align.h > +++ b/include/odp_align.h > @@ -14,6 +14,8 @@ > #ifndef ODP_ALIGN_H_ > #define ODP_ALIGN_H_ > > +#include <plat/odp_align.h> > + > #ifdef __cplusplus > extern "C" { > #endif > @@ -43,38 +45,10 @@ extern "C" { > */ > #define ODP_FIELD_SIZEOF(type, member) sizeof(((type *)0)->member) > > -#if defined __x86_64__ || defined __i386__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > - > -#elif defined __arm__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > - > -#elif defined __OCTEON__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 128 > - > -#elif defined __powerpc__ > - > -/** Cache line size */ > -#define ODP_CACHE_LINE_SIZE 64 > > > What's the rule of thumb with renaming ODP_* #defines to PLAT_ODP_*, > like in patch 9/18? It would be easier for developers to see what's > available in the API, rather then going to a specific platform and look > there. Or is it that the defines above are not expected to show up on > all platforms? You are right. To be consistent here should be something like #define ODP_CACHE_LINE_SIZE PLAT_ODP_CACHE_LINE_SIZE I have concerns about the whole 'indirection' approach now. Code looks very weird when plat_odp_* is mixed with odp_* types and macros. We should either stick only with plat_odp_* inside of implementation and leave odp_* for applications, or drop plat_odp_* completely and define types in platform headers directly. I prefer the second option.
diff --git a/include/odp_align.h b/include/odp_align.h index 83495a8..241b58a 100644 --- a/include/odp_align.h +++ b/include/odp_align.h @@ -14,6 +14,8 @@ #ifndef ODP_ALIGN_H_ #define ODP_ALIGN_H_ +#include <plat/odp_align.h> + #ifdef __cplusplus extern "C" { #endif @@ -43,38 +45,10 @@ extern "C" { */ #define ODP_FIELD_SIZEOF(type, member) sizeof(((type *)0)->member) -#if defined __x86_64__ || defined __i386__ - -/** Cache line size */ -#define ODP_CACHE_LINE_SIZE 64 - -#elif defined __arm__ - -/** Cache line size */ -#define ODP_CACHE_LINE_SIZE 64 - -#elif defined __OCTEON__ - -/** Cache line size */ -#define ODP_CACHE_LINE_SIZE 128 - -#elif defined __powerpc__ - -/** Cache line size */ -#define ODP_CACHE_LINE_SIZE 64 - -#else -#error GCC target not found -#endif - #else #error Non-gcc compatible compiler #endif -/** Page size */ -#define ODP_PAGE_SIZE 4096 - - /* * Round up */ @@ -155,7 +129,6 @@ extern "C" { #define ODP_ALIGNED_PAGE ODP_ALIGNED(ODP_PAGE_SIZE) - /* * Check align */ @@ -174,7 +147,6 @@ extern "C" { #define ODP_VAL_IS_POWER_2(x) ((((x)-1) & (x)) == 0) - #ifdef __cplusplus } #endif diff --git a/platform/linux-generic/include/plat/odp_align.h b/platform/linux-generic/include/plat/odp_align.h new file mode 100644 index 0000000..78f3b38 --- /dev/null +++ b/platform/linux-generic/include/plat/odp_align.h @@ -0,0 +1,61 @@ +/* Copyright (c) 2013, Linaro Limited + * All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + + +/** + * @file + * + * ODP alignments + */ + +#ifndef ODP_ALIGN_H_ +#error This file should be included only into corresponding top level header +#else + +#ifdef __cplusplus +extern "C" { +#endif + + +#ifdef __GNUC__ + +#if defined __x86_64__ || defined __i386__ + +/** Cache line size */ +#define ODP_CACHE_LINE_SIZE 64 + +#elif defined __arm__ + +/** Cache line size */ +#define ODP_CACHE_LINE_SIZE 64 + +#elif defined __OCTEON__ + +/** Cache line size */ +#define ODP_CACHE_LINE_SIZE 128 + +#elif defined __powerpc__ + +/** Cache line size */ +#define ODP_CACHE_LINE_SIZE 64 + +#else +#error GCC target not found +#endif + +#else +#error Non-gcc compatible compiler +#endif + +/** Page size */ +#define ODP_PAGE_SIZE 4096 + + +#ifdef __cplusplus +} +#endif + +#endif
Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org> --- include/odp_align.h | 32 +----------- platform/linux-generic/include/plat/odp_align.h | 61 +++++++++++++++++++++++ 2 files changed, 63 insertions(+), 30 deletions(-) create mode 100644 platform/linux-generic/include/plat/odp_align.h