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

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

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

- 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
lng-odp@lists.linaro.org
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to