On Tue, Aug 26, 2014 at 05:03:45PM +0100, Mike Holmes wrote:
> 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
> 
>  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      | 26 ++++++--
>  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, 68 insertions(+), 138 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/platform/linux-generic/include/api/odp_init.h 
> b/platform/linux-generic/include/api/odp_init.h
> index 490324a..4acaea4 100644
> --- a/platform/linux-generic/include/api/odp_init.h
> +++ b/platform/linux-generic/include/api/odp_init.h
> @@ -23,23 +23,41 @@ extern "C" {
>  #include <odp_std_types.h>
> 
> 
> +/**
> + * ODP  initialisation
> + * Data that is required to initialize the ODP
> + * application, such as specifying a logging callback, the log level etc.
> + */

Line wrapping is odd.

Isn't this initialising ODP with application specific data, rather than
initialising the application?

> +typedef struct odp_global_init {
> +} odp_global_init_t;
> 
> +/**
> + * ODP  platform initialization
> + * Platform specific data. ODP does nothing with
> + * this data.

The data will be used by ODP, it'll just be a platform specific part of
ODP. I am thinking from an application perspective where ODP is one blob
of functionality rather than some layer in between the application and
an SDK.

It's worth noting somewhere, probably here, that use of this structure
is not portable and what an application should do to cope with that. Is
the application expected to just know which platform it's running on?
or do we need an API that returns a platform identifier?

> + */
> +typedef struct odp_global_platform_init {
> +} odp_global_platform_init_t;
> 
>  /**
> - * Perform global ODP initalisation.
> + * 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 ODP
> + * @param[in] platform_params Those parameters that are passed
> + * without interpretation to the implementation.

Isn't the implementation itself part of ODP? Also I think it's better to
stick to using the term platform rather than mixing the terms platform
and implementation (they mean the same thing in this context.. right?)

How about..

params          ODP initialisation parameters, common across all platforms
platform_params Platform specific initialisation parameters

-- 
Stuart.


_______________________________________________
lng-odp mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/lng-odp

Reply via email to