On Aug 29, 2014, at 2:59 AM, Savolainen, Petri (NSN - FI/Espoo) <[email protected]> wrote:
> > >> -----Original Message----- >> From: [email protected] [mailto:lng-odp- >> [email protected]] On Behalf Of ext Mike Holmes >> Sent: Thursday, August 28, 2014 9:26 PM >> To: [email protected] >> Subject: [lng-odp] [PATCH v4] Add-global_init-paramiters >> >> Signed-off-by: Mike Holmes <[email protected]> >> --- >> >> 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... > >> { >> - 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. I would use the 'struct odp_init_s {‘ adding the _s postfix and shorting the name to the suggestions below. > >> +} 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. > >> +} odp_global_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... > >> + * @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? > >> * 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… I disagree here, we should keep the odp_init_platform() as we do not know what will happen in the future and to make the platforms follow the same pattern. > > - 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; >> +} > > > > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp Keith Wiles, Principal Technologist with CTO office, Wind River mobile 972-213-5533 _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
