[01/10] Move odp_buffer.h to linux-generic

To be able to make platform specific changes in odp_buffer.h
move it to platform directory.

Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
---
 include/odp_buffer.h                            |  107 -----------------------
 platform/linux-generic/include/api/odp_buffer.h |  107 +++++++++++++++++++++++
 2 files changed, 107 insertions(+), 107 deletions(-)
 delete mode 100644 include/odp_buffer.h
 create mode 100644 platform/linux-generic/include/api/odp_buffer.h

On 2014-04-14 13:55, Taras Kondratiuk wrote:
> To be able to make platform specific changes in odp_buffer.h
> move it to platform directory.
>
> Signed-off-by: Taras Kondratiuk <taras.kondratiuk@linaro.org>
> ---
>   include/odp_buffer.h                            |  107 -----------------------
>   platform/linux-generic/include/api/odp_buffer.h |  107 +++++++++++++++++++++++
>   2 files changed, 107 insertions(+), 107 deletions(-)
>   delete mode 100644 include/odp_buffer.h
>   create mode 100644 platform/linux-generic/include/api/odp_buffer.h
>
> diff --git a/include/odp_buffer.h b/include/odp_buffer.h
> deleted file mode 100644
> index 2d2c25a..0000000
> --- a/include/odp_buffer.h
> +++ /dev/null
> @@ -1,107 +0,0 @@
> -/* Copyright (c) 2013, Linaro Limited
> - * All rights reserved.
> - *
> - * SPDX-License-Identifier:     BSD-3-Clause
> - */
> -
> -
> -/**
> - * @file
> - *
> - * ODP buffer descriptor
> - */
> -
> -#ifndef ODP_BUFFER_H_
> -#define ODP_BUFFER_H_
> -
> -#ifdef __cplusplus
> -extern "C" {
> -#endif
> -
> -
> -
> -#include <odp_std_types.h>
> -
> -
> -
> -
> -
> -/**
> - * ODP buffer
> - */
> -typedef uint32_t odp_buffer_t;
> -
> -#define ODP_BUFFER_INVALID (0xffffffff) /**< Invalid buffer */
> -
> -
> -/**
> - * Buffer start address
> - *
> - * @param buf      Buffer handle
> - *
> - * @return Buffer start address
> - */
> -void *odp_buffer_addr(odp_buffer_t buf);
> -
> -/**
> - * Buffer maximum data size
> - *
> - * @param buf      Buffer handle
> - *
> - * @return Buffer maximum data size
> - */
> -size_t odp_buffer_size(odp_buffer_t buf);
> -
> -/**
> - * Buffer type
> - *
> - * @param buf      Buffer handle
> - *
> - * @return Buffer type
> - */
> -int odp_buffer_type(odp_buffer_t buf);
> -
> -#define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
> -#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
> -#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
> -#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */

Perhaps it would be good to
#define ODP_BUFFER_TYPE_INVALID ARCH_ODP_BUFFER_TYPE_INVALID

where ARCH_* is defined in the ARCH specific .h file.

Same goes for other primitives here.

> -
> -/**
> - * Tests if buffer is part of a scatter/gather list
> - *
> - * @param buf      Buffer handle
> - *
> - * @return 1 if belongs to a scatter list, otherwise 0
> - */
> -int odp_buffer_is_scatter(odp_buffer_t buf);
> -
> -/**
> - * Tests if buffer is valid
> - *
> - * @param buf      Buffer handle
> - *
> - * @return 1 if valid, otherwise 0
> - */
> -int odp_buffer_is_valid(odp_buffer_t buf);
> -
> -/**
> - * Print buffer metadata to STDOUT
> - *
> - * @param buf      Buffer handle
> - *
> - */
> -void odp_buffer_print(odp_buffer_t buf);
> -
> -
> -#ifdef __cplusplus
> -}
> -#endif
> -
> -#endif
> -
> -
> -
> -
> -
> -
> -
> diff --git a/platform/linux-generic/include/api/odp_buffer.h b/platform/linux-generic/include/api/odp_buffer.h
> new file mode 100644
> index 0000000..2d2c25a
> --- /dev/null
> +++ b/platform/linux-generic/include/api/odp_buffer.h
> @@ -0,0 +1,107 @@
> +/* Copyright (c) 2013, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + *
> + * ODP buffer descriptor
> + */
> +
> +#ifndef ODP_BUFFER_H_
> +#define ODP_BUFFER_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +
> +
> +#include <odp_std_types.h>
> +
> +
> +
> +
> +
> +/**
> + * ODP buffer
> + */
> +typedef uint32_t odp_buffer_t;
> +
> +#define ODP_BUFFER_INVALID (0xffffffff) /**< Invalid buffer */
> +
> +
> +/**
> + * Buffer start address
> + *
> + * @param buf      Buffer handle
> + *
> + * @return Buffer start address
> + */
> +void *odp_buffer_addr(odp_buffer_t buf);
> +
> +/**
> + * Buffer maximum data size
> + *
> + * @param buf      Buffer handle
> + *
> + * @return Buffer maximum data size
> + */
> +size_t odp_buffer_size(odp_buffer_t buf);
> +
> +/**
> + * Buffer type
> + *
> + * @param buf      Buffer handle
> + *
> + * @return Buffer type
> + */
> +int odp_buffer_type(odp_buffer_t buf);
> +
> +#define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
> +#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
> +#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
> +#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */
> +
> +/**
> + * Tests if buffer is part of a scatter/gather list
> + *
> + * @param buf      Buffer handle
> + *
> + * @return 1 if belongs to a scatter list, otherwise 0
> + */
> +int odp_buffer_is_scatter(odp_buffer_t buf);
> +
> +/**
> + * Tests if buffer is valid
> + *
> + * @param buf      Buffer handle
> + *
> + * @return 1 if valid, otherwise 0
> + */
> +int odp_buffer_is_valid(odp_buffer_t buf);
> +
> +/**
> + * Print buffer metadata to STDOUT
> + *
> + * @param buf      Buffer handle
> + *
> + */
> +void odp_buffer_print(odp_buffer_t buf);
> +
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> +
> +
> +
> +
> +
> +
> +
>

On 04/14/2014 03:46 PM, David Nyström wrote:
> On 2014-04-14 13:55, Taras Kondratiuk wrote:
>> -/**
>> - * Buffer type
>> - *
>> - * @param buf      Buffer handle
>> - *
>> - * @return Buffer type
>> - */
>> -int odp_buffer_type(odp_buffer_t buf);
>> -
>> -#define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
>> -#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
>> -#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
>> -#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */
>
> Perhaps it would be good to
> #define ODP_BUFFER_TYPE_INVALID ARCH_ODP_BUFFER_TYPE_INVALID
>
> where ARCH_* is defined in the ARCH specific .h file.
>
> Same goes for other primitives here.

That's possible option, but this file will most probably contain
arch specific inline functions as we start performance optimizations.

I tend to agree with Petri that it worth to move all headers to platform
specific place. The biggest concern here was: how to be sure that API
won't diverge in different implementations? We can (or even should) have 
some compatibility test application which will basically call each and 
every API function.

Ensuring API consistency across implementations is one of the reasons for
having a set of API conformance tests that can be run as regressions.

When we say that ODP has an API this means that ODP *documents* the API
externals.  How those externals are implemented may mean that there is a
"generic" include that sub-includes things as needed or it may mean that
platform-specific include files implement these externals.  A lot of this
is a function of the tools technology.  It's unfortunate that we're saddled
with some legacy GCC limitations on this but that's the reality.  We
absolutely want most of the accessor/abstract functions to be inlined for
performance and with GCC in many cases that means these have to be in
platform-specific files rather than in common files that are they
overridden by their implementation counterparts.

Bill


On Mon, Apr 14, 2014 at 11:24 AM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 04/14/2014 03:46 PM, David Nyström wrote:
>
>> On 2014-04-14 13:55, Taras Kondratiuk wrote:
>>
>>> -/**
>>> - * Buffer type
>>> - *
>>> - * @param buf      Buffer handle
>>> - *
>>> - * @return Buffer type
>>> - */
>>> -int odp_buffer_type(odp_buffer_t buf);
>>> -
>>> -#define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
>>> -#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
>>> -#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
>>> -#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */
>>>
>>
>> Perhaps it would be good to
>> #define ODP_BUFFER_TYPE_INVALID ARCH_ODP_BUFFER_TYPE_INVALID
>>
>> where ARCH_* is defined in the ARCH specific .h file.
>>
>> Same goes for other primitives here.
>>
>
> That's possible option, but this file will most probably contain
> arch specific inline functions as we start performance optimizations.
>
> I tend to agree with Petri that it worth to move all headers to platform
> specific place. The biggest concern here was: how to be sure that API
> won't diverge in different implementations? We can (or even should) have
> some compatibility test application which will basically call each and
> every API function.
>
> --
> Taras Kondratiuk
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>

> -----Original Message-----
> From: ext Taras Kondratiuk [mailto:taras.kondratiuk@linaro.org]
> Sent: Monday, April 14, 2014 7:25 PM
> To: David Nyström; lng-odp@lists.linaro.org
> Cc: Filip Moerman; Petri Savolainen
> Subject: Re: [lng-odp] [PATCH 01/10] Move odp_buffer.h to linux-generic
> 
> On 04/14/2014 03:46 PM, David Nyström wrote:
> > On 2014-04-14 13:55, Taras Kondratiuk wrote:
> >> -/**
> >> - * Buffer type
> >> - *
> >> - * @param buf      Buffer handle
> >> - *
> >> - * @return Buffer type
> >> - */
> >> -int odp_buffer_type(odp_buffer_t buf);
> >> -
> >> -#define ODP_BUFFER_TYPE_INVALID (-1) /**< Buffer type invalid */
> >> -#define ODP_BUFFER_TYPE_RAW       0  /**< Raw buffer */
> >> -#define ODP_BUFFER_TYPE_PACKET    1  /**< Packet buffer */
> >> -#define ODP_BUFFER_TYPE_TIMER     2  /**< Timer buffer */
> >
> > Perhaps it would be good to
> > #define ODP_BUFFER_TYPE_INVALID ARCH_ODP_BUFFER_TYPE_INVALID
> >
> > where ARCH_* is defined in the ARCH specific .h file.
> >
> > Same goes for other primitives here.
> 
> That's possible option, but this file will most probably contain
> arch specific inline functions as we start performance optimizations.
> 
> I tend to agree with Petri that it worth to move all headers to
> platform
> specific place. The biggest concern here was: how to be sure that API
> won't diverge in different implementations? We can (or even should)
> have
> some compatibility test application which will basically call each and
> every API function.
> 
> --
> Taras Kondratiuk

Agree with Taras. Thorough API compatibility testing is needed in any case (whether all header files are shared or platform specific). It's trivial to produce an implementation that compiles (e.g. full of dummy functions), but it's harder to get functionality match 100% the API specification. Also error conditions, side-effects or dependencies between functions should be tested (function B behaves differently, if function A was called before it). That's a lot of work.

API conformance testing is almost non-existent right now...

 
-Petri