On 29 August 2014 03:59, Savolainen, Petri (NSN - FI/Espoo) < petri.savolai...@nsn.com> wrote:
> > > > -----Original Message----- > > From: lng-odp-boun...@lists.linaro.org [mailto:lng-odp- > > boun...@lists.linaro.org] On Behalf Of ext Mike Holmes > > Sent: Thursday, August 28, 2014 9:26 PM > > To: lng-odp@lists.linaro.org > > Subject: [lng-odp] [PATCH v4] Add-global_init-paramiters > > > > Signed-off-by: Mike Holmes <mike.hol...@linaro.org> > > --- > > > > v2: > > Factor out odp_platform_init so that odp_global_init is only defined > > for linux_generic and resued. > > > > v3: > > Use -M to simplify the diff > > Fix missing doxygen comments > > > > v4: > > Improve the documentation. > > > > example/generator/odp_generator.c | 2 +- > > example/l2fwd/odp_l2fwd.c | 2 +- > > example/odp_example/odp_example.c | 2 +- > > example/packet/odp_pktio.c | 2 +- > > example/timer/odp_timer_test.c | 2 +- > > platform/linux-dpdk/Makefile.am | 3 +- > > platform/linux-dpdk/{odp_init.c => odp_platform.c} | 63 > +---------------- > > -- > > platform/linux-generic/Makefile.am | 1 + > > platform/linux-generic/include/api/odp_init.h | 34 +++++++--- > > platform/linux-generic/include/odp_internal.h | 3 + > > platform/linux-generic/odp_init.c | 9 ++- > > platform/linux-generic/odp_platform.c | 14 +++++ > > platform/linux-keystone2/Makefile.am | 3 +- > > .../linux-keystone2/{odp_init.c => odp_platform.c} | 72 > +++-------------- > > ----- > > test/api_test/odp_common.c | 2 +- > > 15 files changed, 72 insertions(+), 142 deletions(-) > > rename platform/linux-dpdk/{odp_init.c => odp_platform.c} (50%) > > create mode 100644 platform/linux-generic/odp_platform.c > > rename platform/linux-keystone2/{odp_init.c => odp_platform.c} (72%) > > > > diff --git a/example/generator/odp_generator.c > > b/example/generator/odp_generator.c > > index 9fa9b37..ba7aa68 100644 > > --- a/example/generator/odp_generator.c > > +++ b/example/generator/odp_generator.c > > @@ -525,7 +525,7 @@ int main(int argc, char *argv[]) > > int core_count; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/l2fwd/odp_l2fwd.c b/example/l2fwd/odp_l2fwd.c > > index d74449a..f8d6729 100644 > > --- a/example/l2fwd/odp_l2fwd.c > > +++ b/example/l2fwd/odp_l2fwd.c > > @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) > > odp_pktio_t pktio; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/odp_example/odp_example.c > > b/example/odp_example/odp_example.c > > index f0bdf29..3d38ac7 100644 > > --- a/example/odp_example/odp_example.c > > +++ b/example/odp_example/odp_example.c > > @@ -951,7 +951,7 @@ int main(int argc, char *argv[]) > > > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > printf("ODP global init failed.\n"); > > return -1; > > } > > diff --git a/example/packet/odp_pktio.c b/example/packet/odp_pktio.c > > index f247bd0..f3543a0 100644 > > --- a/example/packet/odp_pktio.c > > +++ b/example/packet/odp_pktio.c > > @@ -311,7 +311,7 @@ int main(int argc, char *argv[]) > > int core_count; > > > > /* Init ODP before calling anything else */ > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > ODP_ERR("Error: ODP global init failed.\n"); > > exit(EXIT_FAILURE); > > } > > diff --git a/example/timer/odp_timer_test.c > > b/example/timer/odp_timer_test.c > > index bf1d7df..abf90aa 100644 > > --- a/example/timer/odp_timer_test.c > > +++ b/example/timer/odp_timer_test.c > > @@ -260,7 +260,7 @@ int main(int argc, char *argv[]) > > > > memset(thread_tbl, 0, sizeof(thread_tbl)); > > > > - if (odp_init_global()) { > > + if (odp_init_global(NULL, NULL)) { > > printf("ODP global init failed.\n"); > > return -1; > > } > > diff --git a/platform/linux-dpdk/Makefile.am b/platform/linux- > > dpdk/Makefile.am > > index ff49b7d..70fa549 100644 > > --- a/platform/linux-dpdk/Makefile.am > > +++ b/platform/linux-dpdk/Makefile.am > > @@ -67,13 +67,14 @@ __LIB__libodp_la_SOURCES = \ > > odp_buffer.c \ > > odp_buffer_pool.c \ > > ../linux-generic/odp_coremask.c \ > > - odp_init.c \ > > + ../linux-generic/odp_init.c \ > > odp_linux.c \ > > odp_packet.c \ > > odp_packet_dpdk.c \ > > ../linux-generic/odp_packet_flags.c \ > > odp_packet_io.c \ > > ../linux-generic/odp_packet_socket.c \ > > + odp_platform.c \ > > odp_queue.c \ > > ../linux-generic/odp_ring.c \ > > ../linux-generic/odp_rwlock.c \ > > diff --git a/platform/linux-dpdk/odp_init.c b/platform/linux- > > dpdk/odp_platform.c > > similarity index 50% > > rename from platform/linux-dpdk/odp_init.c > > rename to platform/linux-dpdk/odp_platform.c > > index ecc2066..5a054fd 100644 > > --- a/platform/linux-dpdk/odp_init.c > > +++ b/platform/linux-dpdk/odp_platform.c > > @@ -1,4 +1,4 @@ > > -/* Copyright (c) 2013, Linaro Limited > > +/* Copyright (c) 2014, Linaro Limited > > * All rights reserved. > > * > > * SPDX-License-Identifier: BSD-3-Clause > > @@ -9,7 +9,7 @@ > > #include <odp_debug.h> > > #include <odp_packet_dpdk.h> > > > > -int odp_init_dpdk(void) > > +int odp_init_platform(odp_global_platform_init_t *platform_params > > ODP_UNUSED) > > { > > int test_argc = 5; > > char *test_argv[6]; > > @@ -50,64 +50,7 @@ int odp_init_dpdk(void) > > return 0; > > } > > > > -int odp_init_global(void) > > +int odp_crypto_init_global(void) > > Is crypto init now in odp_platform.c? It should be in odp_crypto.c or > similar... > no problem I can make a dummy odp_crypto.c for ks2 and point DPDK at the linux generic. I was trying to avoid connecting internals. > > > { > > - odp_thread_init_global(); > > - > > - odp_system_info_init(); > > - > > - if (odp_init_dpdk()) { > > - ODP_ERR("ODP dpdk init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_shm_init_global()) { > > - ODP_ERR("ODP shm init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_buffer_pool_init_global()) { > > - ODP_ERR("ODP buffer pool init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_queue_init_global()) { > > - ODP_ERR("ODP queue init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_schedule_init_global()) { > > - ODP_ERR("ODP schedule init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_pktio_init_global()) { > > - ODP_ERR("ODP packet io init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_timer_init_global()) { > > - ODP_ERR("ODP timer init failed.\n"); > > - return -1; > > - } > > - > > - return 0; > > -} > > - > > - > > -int odp_init_local(int thr_id) > > -{ > > - odp_thread_init_local(thr_id); > > - > > - if (odp_pktio_init_local()) { > > - ODP_ERR("ODP packet io local init failed.\n"); > > - return -1; > > - } > > - > > - if (odp_schedule_init_local()) { > > - ODP_ERR("ODP schedule local init failed.\n"); > > - return -1; > > - } > > - > > return 0; > > } > > diff --git a/platform/linux-generic/Makefile.am b/platform/linux- > > generic/Makefile.am > > index f4dfdc1..c4aae9d 100644 > > --- a/platform/linux-generic/Makefile.am > > +++ b/platform/linux-generic/Makefile.am > > @@ -62,6 +62,7 @@ __LIB__libodp_la_SOURCES = \ > > odp_packet_flags.c \ > > odp_packet_io.c \ > > odp_packet_socket.c \ > > + odp_platform.c \ > > odp_queue.c \ > > odp_ring.c \ > > odp_rwlock.c \ > > diff --git a/platform/linux-generic/include/api/odp_init.h > > b/platform/linux-generic/include/api/odp_init.h > > index 490324a..ece4547 100644 > > --- a/platform/linux-generic/include/api/odp_init.h > > +++ b/platform/linux-generic/include/api/odp_init.h > > @@ -22,24 +22,42 @@ extern "C" { > > > > #include <odp_std_types.h> > > > > - > > - > > +/** ODP initialisation data. > > + * Data that is required to initialize the ODP API with the > > + * application specific data such as specifying a logging callback, the > > log > > + * level etc. > > + */ > > +typedef struct odp_global_init { > > The struct name should be "struct odp_global_init_t", "struct > odp_global_init_s" or "struct odp_global_init_". > > The name without postfix (odp_global_init) should be reserved for variable > names. > no probelm > I will go with odp_init_s > > +} odp_global_init_t; > > In short, this could be named odp_init_t. It's the main init struct there > will be. > If there will be a local init struct it could be then odp_init_local_t... > > > + > > +/** ODP platform initialization data. > > + * @note ODP API does nothing with this data. It is the underlying > > + * implementation requires it and any data passed here is not portable. > > + * It is required that the application takes care of identifying and > > + * passing any required platform specific data. > > + */ > > +typedef struct odp_global_platform_init { > > Same thing, rename struct. > no problem odp_init_platform_s > > > +} odp_platform_init_t; > > Same thing, odp_platform_init_t ?? > > > > > > /** > > - * Perform global ODP initalisation. > > - * > > - * This function must be called once before calling > > - * any other ODP API functions. > > + * Perform global ODP initialisation. > > * > > + * This function must be called once before calling any other ODP API > > + * functions. > > + * @param[in] params Those parameters that are interpreted by the ODP > API > > "Those parameters that are common to all ODP implementations. Specified in > this API" > > API does not interpret, implementation does... > Ok, I am struggling with names how about "platform SDK" for the underlying code such as DPDK, so that the platform has an API implementation - which is actually implemented in terms of a platform SDK We need to distinguish that implementations should not use the platform parameters unless the underlying framework requires it, not that any implementation is free to just add local stuff - or thats how it feels to me at least. > > > + * @param[in] platform_params Those parameters that are passed without > > + * interpretation by the ODP API to the implementation. > > " ... implementation specific parameter. Specified outside of ODP API" > > > * @return 0 if successful > > */ > > -int odp_init_global(void); > > +int odp_init_global(odp_global_init_t *params, > > + odp_global_platform_init_t *platform_params); > > > > > > /** > > - * Perform thread local ODP initalisation. > > + * Perform thread local ODP initialisation. > > * > > * All threads must call this function before calling > > + * > > Extra line feed? > Will fix > > > * any other ODP API functions. > > * @param thr_id Thread id > > * @return 0 if successful > > diff --git a/platform/linux-generic/include/odp_internal.h > > b/platform/linux-generic/include/odp_internal.h > > index aa79493..6c71fc9 100644 > > --- a/platform/linux-generic/include/odp_internal.h > > +++ b/platform/linux-generic/include/odp_internal.h > > @@ -4,6 +4,7 @@ > > * SPDX-License-Identifier: BSD-3-Clause > > */ > > > > +#include <odp_init.h> > > > > /** > > * @file > > @@ -42,6 +43,8 @@ int odp_schedule_init_local(void); > > int odp_timer_init_global(void); > > int odp_timer_disarm_all(void); > > > > +int odp_init_platform(odp_global_platform_init_t *platform_params); > > + > > #ifdef __cplusplus > > } > > #endif > > diff --git a/platform/linux-generic/odp_init.c b/platform/linux- > > generic/odp_init.c > > index 5b7e192..f595def 100644 > > --- a/platform/linux-generic/odp_init.c > > +++ b/platform/linux-generic/odp_init.c > > @@ -8,13 +8,18 @@ > > #include <odp_internal.h> > > #include <odp_debug.h> > > > > - > > -int odp_init_global(void) > > +int odp_init_global(odp_global_init_t *params ODP_UNUSED, > > + odp_global_platform_init_t *platform_params ODP_UNUSED) > > { > > odp_thread_init_global(); > > > > odp_system_info_init(); > > > > + if (odp_init_platform(platform_params)) { > > platform_params is used here, so remove ODP_UNUSED from argument list. > > On the other hand, it would be better to remove odp_init_platform() > altogether since does not do anything. It's better to add it when actually > used. > The order of init calls is very important, e.g. now you could not allocate > shared > memory in odp_init_platform() because it's executed before shm init... > odp_init_platform() does do something for DPDK / KS2 and we need a place holder because we want this to be shared between implementations don't we ? This is the stuff that must happen before any ODP init starts to satisfy the platform SDK > > - Petri > > > + ODP_ERR("ODP platform init failed.\n"); > > + return -1; > > + } > > + > > if (odp_shm_init_global()) { > > ODP_ERR("ODP shm init failed.\n"); > > return -1; > > diff --git a/platform/linux-generic/odp_platform.c b/platform/linux- > > generic/odp_platform.c > > new file mode 100644 > > index 0000000..a9efa7f > > --- /dev/null > > +++ b/platform/linux-generic/odp_platform.c > > @@ -0,0 +1,14 @@ > > +/* Copyright (c) 2014, Linaro Limited > > + * All rights reserved. > > + * > > + * SPDX-License-Identifier: BSD-3-Clause > > + */ > > + > > +#include <odp_init.h> > > +#include <odp_internal.h> > > +#include <odp_debug.h> > > + > > +int odp_init_platform(odp_global_platform_init_t *platform_params > > ODP_UNUSED) > > +{ > > + return 0; > > +} > > > > > -- *Mike Holmes* Linaro Technical Manager / Lead LNG - ODP
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org http://lists.linaro.org/mailman/listinfo/lng-odp
