diff mbox

[PATCHv5,11/19] linux-generic: buffers: add strong typing support

Message ID 1422938699-12877-12-git-send-email-bill.fischofer@linaro.org
State Accepted
Commit 433ee41111260311119620e9098d6be229e0c54c
Headers show

Commit Message

Bill Fischofer Feb. 3, 2015, 4:44 a.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 .../linux-generic/include/odp/plat/buffer_types.h  | 15 ++++++++---
 .../linux-generic/include/odp_buffer_inlines.h     | 12 ++++-----
 .../linux-generic/include/odp_buffer_internal.h    | 31 +++++++++++-----------
 platform/linux-generic/odp_packet.c                | 13 ++++-----
 4 files changed, 40 insertions(+), 31 deletions(-)

Comments

Anders Roxell Feb. 3, 2015, 9:25 p.m. UTC | #1
On 2015-02-02 22:44, Bill Fischofer wrote:
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>  .../linux-generic/include/odp/plat/buffer_types.h  | 15 ++++++++---
>  .../linux-generic/include/odp_buffer_inlines.h     | 12 ++++-----
>  .../linux-generic/include/odp_buffer_internal.h    | 31 +++++++++++-----------
>  platform/linux-generic/odp_packet.c                | 13 ++++-----
>  4 files changed, 40 insertions(+), 31 deletions(-)
> 
> diff --git a/platform/linux-generic/include/odp/plat/buffer_types.h b/platform/linux-generic/include/odp/plat/buffer_types.h
> index 8601e61..3e7070e 100644
> --- a/platform/linux-generic/include/odp/plat/buffer_types.h
> +++ b/platform/linux-generic/include/odp/plat/buffer_types.h
> @@ -18,6 +18,7 @@ extern "C" {
>  #endif
>  
>  #include <odp/std_types.h>
> +#include <odp/plat/strong_types.h>
>  
>  /** @addtogroup odp_buffer ODP BUFFER
>   *  Operations on a buffer.
> @@ -25,16 +26,22 @@ extern "C" {
>   */
>  
>  /** ODP buffer */
> -typedef uint32_t odp_buffer_t;
> +typedef odp_handle_t odp_buffer_t;
>  
>  /** Invalid buffer */
> -#define ODP_BUFFER_INVALID (0xffffffff)
> +#define ODP_BUFFER_INVALID _odp_cast_scalar(odp_buffer_t, 0xffffffff)
>  
>  /** ODP buffer segment */
> -typedef odp_buffer_t odp_buffer_seg_t;
> +typedef odp_handle_t odp_buffer_seg_t;
>  
>  /** Invalid segment */
> -#define ODP_SEGMENT_INVALID ODP_BUFFER_INVALID
> +#define ODP_SEGMENT_INVALID ((odp_buffer_seg_t)ODP_BUFFER_INVALID)
> +
> +/** Get printable format of odp_buffer_t */
> +static inline uint64_t odp_buffer_to_u64(odp_buffer_t hdl)
> +{
> +	return _odp_pri(hdl);
> +}

The re-org has make it easier for the implementors of ODP.
However, its a bit harder to follow the structure now maybe.

"static inline functions" should go into:
platform/linux-generic/include/odp/*.h

and typedefs, defines, enums, and structs that are platform specific
should go into:
platform/linux-generic/include/odp/plat/*_types.h

doxygen clean API stuff should be in:
include/odp/api/*.h


So the "static inline functions" in this patch set are currently in the wrong
place.

Taras, have I stated this correctly?

Cheers,
Anders
Taras Kondratiuk Feb. 3, 2015, 9:46 p.m. UTC | #2
On 02/03/2015 11:25 PM, Anders Roxell wrote:
> On 2015-02-02 22:44, Bill Fischofer wrote:
>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>> ---
>>  .../linux-generic/include/odp/plat/buffer_types.h  | 15 ++++++++---
>>  .../linux-generic/include/odp_buffer_inlines.h     | 12 ++++-----
>>  .../linux-generic/include/odp_buffer_internal.h    | 31 +++++++++++-----------
>>  platform/linux-generic/odp_packet.c                | 13 ++++-----
>>  4 files changed, 40 insertions(+), 31 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp/plat/buffer_types.h b/platform/linux-generic/include/odp/plat/buffer_types.h
>> index 8601e61..3e7070e 100644
>> --- a/platform/linux-generic/include/odp/plat/buffer_types.h
>> +++ b/platform/linux-generic/include/odp/plat/buffer_types.h
>> @@ -18,6 +18,7 @@ extern "C" {
>>  #endif
>>  
>>  #include <odp/std_types.h>
>> +#include <odp/plat/strong_types.h>
>>  
>>  /** @addtogroup odp_buffer ODP BUFFER
>>   *  Operations on a buffer.
>> @@ -25,16 +26,22 @@ extern "C" {
>>   */
>>  
>>  /** ODP buffer */
>> -typedef uint32_t odp_buffer_t;
>> +typedef odp_handle_t odp_buffer_t;
>>  
>>  /** Invalid buffer */
>> -#define ODP_BUFFER_INVALID (0xffffffff)
>> +#define ODP_BUFFER_INVALID _odp_cast_scalar(odp_buffer_t, 0xffffffff)
>>  
>>  /** ODP buffer segment */
>> -typedef odp_buffer_t odp_buffer_seg_t;
>> +typedef odp_handle_t odp_buffer_seg_t;
>>  
>>  /** Invalid segment */
>> -#define ODP_SEGMENT_INVALID ODP_BUFFER_INVALID
>> +#define ODP_SEGMENT_INVALID ((odp_buffer_seg_t)ODP_BUFFER_INVALID)
>> +
>> +/** Get printable format of odp_buffer_t */
>> +static inline uint64_t odp_buffer_to_u64(odp_buffer_t hdl)
>> +{
>> +	return _odp_pri(hdl);
>> +}
> 
> The re-org has make it easier for the implementors of ODP.
> However, its a bit harder to follow the structure now maybe.
> 
> "static inline functions" should go into:
> platform/linux-generic/include/odp/*.h
> 
> and typedefs, defines, enums, and structs that are platform specific
> should go into:
> platform/linux-generic/include/odp/plat/*_types.h
> 
> doxygen clean API stuff should be in:
> include/odp/api/*.h
> 
> 
> So the "static inline functions" in this patch set are currently in the wrong
> place.
> 
> Taras, have I stated this correctly?

Yes, the idea was to have only types definitions and related macros in
*_types.h. So if another module needs odp_buffer_t handle definition
but not all buffer API functions it can include odp/plat/buffer_types.h
and be sure that this won't cause any cross dependencies.

But these specific functions are just type conversion, so IMO placing
them in *_types.h as an exception is fine.
Bill Fischofer Feb. 3, 2015, 9:50 p.m. UTC | #3
The changes here are to
platform/linux-generic/include/odp/plat/<type>_types.h.  This is where the
strong typing is actually implemented for each ODP type in linux-generic.
The converter functions are part of that implementation, and are also
inlined for performance, which is why they are here.

All other changes within each patch part are the modifications to other ODP
files needed as a result of switching to strong typing for this type.  They
are not separable since once strong typing is enabled for that type the
code won't compile without these changes.

The doxygen stuff is in the APIs which are in include/odp/api/*.h as you
mention.  The main difference between v4 and v5 was to break out the API
definition changes separately, which is why this has gone from 10 to 19
parts.

So it sounds like this is doing exactly what you suggest, no?

On Tue, Feb 3, 2015 at 3:46 PM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 02/03/2015 11:25 PM, Anders Roxell wrote:
> > On 2015-02-02 22:44, Bill Fischofer wrote:
> >> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> >> ---
> >>  .../linux-generic/include/odp/plat/buffer_types.h  | 15 ++++++++---
> >>  .../linux-generic/include/odp_buffer_inlines.h     | 12 ++++-----
> >>  .../linux-generic/include/odp_buffer_internal.h    | 31
> +++++++++++-----------
> >>  platform/linux-generic/odp_packet.c                | 13 ++++-----
> >>  4 files changed, 40 insertions(+), 31 deletions(-)
> >>
> >> diff --git a/platform/linux-generic/include/odp/plat/buffer_types.h
> b/platform/linux-generic/include/odp/plat/buffer_types.h
> >> index 8601e61..3e7070e 100644
> >> --- a/platform/linux-generic/include/odp/plat/buffer_types.h
> >> +++ b/platform/linux-generic/include/odp/plat/buffer_types.h
> >> @@ -18,6 +18,7 @@ extern "C" {
> >>  #endif
> >>
> >>  #include <odp/std_types.h>
> >> +#include <odp/plat/strong_types.h>
> >>
> >>  /** @addtogroup odp_buffer ODP BUFFER
> >>   *  Operations on a buffer.
> >> @@ -25,16 +26,22 @@ extern "C" {
> >>   */
> >>
> >>  /** ODP buffer */
> >> -typedef uint32_t odp_buffer_t;
> >> +typedef odp_handle_t odp_buffer_t;
> >>
> >>  /** Invalid buffer */
> >> -#define ODP_BUFFER_INVALID (0xffffffff)
> >> +#define ODP_BUFFER_INVALID _odp_cast_scalar(odp_buffer_t, 0xffffffff)
> >>
> >>  /** ODP buffer segment */
> >> -typedef odp_buffer_t odp_buffer_seg_t;
> >> +typedef odp_handle_t odp_buffer_seg_t;
> >>
> >>  /** Invalid segment */
> >> -#define ODP_SEGMENT_INVALID ODP_BUFFER_INVALID
> >> +#define ODP_SEGMENT_INVALID ((odp_buffer_seg_t)ODP_BUFFER_INVALID)
> >> +
> >> +/** Get printable format of odp_buffer_t */
> >> +static inline uint64_t odp_buffer_to_u64(odp_buffer_t hdl)
> >> +{
> >> +    return _odp_pri(hdl);
> >> +}
> >
> > The re-org has make it easier for the implementors of ODP.
> > However, its a bit harder to follow the structure now maybe.
> >
> > "static inline functions" should go into:
> > platform/linux-generic/include/odp/*.h
> >
> > and typedefs, defines, enums, and structs that are platform specific
> > should go into:
> > platform/linux-generic/include/odp/plat/*_types.h
> >
> > doxygen clean API stuff should be in:
> > include/odp/api/*.h
> >
> >
> > So the "static inline functions" in this patch set are currently in the
> wrong
> > place.
> >
> > Taras, have I stated this correctly?
>
> Yes, the idea was to have only types definitions and related macros in
> *_types.h. So if another module needs odp_buffer_t handle definition
> but not all buffer API functions it can include odp/plat/buffer_types.h
> and be sure that this won't cause any cross dependencies.
>
> But these specific functions are just type conversion, so IMO placing
> them in *_types.h as an exception is fine.
>
> --
> Taras Kondratiuk
>
Ola Liljedahl Feb. 3, 2015, 9:57 p.m. UTC | #4
On 3 February 2015 at 22:25, Anders Roxell <anders.roxell@linaro.org> wrote:
> On 2015-02-02 22:44, Bill Fischofer wrote:
>> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
>> ---
>>  .../linux-generic/include/odp/plat/buffer_types.h  | 15 ++++++++---
>>  .../linux-generic/include/odp_buffer_inlines.h     | 12 ++++-----
>>  .../linux-generic/include/odp_buffer_internal.h    | 31 +++++++++++-----------
>>  platform/linux-generic/odp_packet.c                | 13 ++++-----
>>  4 files changed, 40 insertions(+), 31 deletions(-)
>>
>> diff --git a/platform/linux-generic/include/odp/plat/buffer_types.h b/platform/linux-generic/include/odp/plat/buffer_types.h
>> index 8601e61..3e7070e 100644
>> --- a/platform/linux-generic/include/odp/plat/buffer_types.h
>> +++ b/platform/linux-generic/include/odp/plat/buffer_types.h
>> @@ -18,6 +18,7 @@ extern "C" {
>>  #endif
>>
>>  #include <odp/std_types.h>
>> +#include <odp/plat/strong_types.h>
>>
>>  /** @addtogroup odp_buffer ODP BUFFER
>>   *  Operations on a buffer.
>> @@ -25,16 +26,22 @@ extern "C" {
>>   */
>>
>>  /** ODP buffer */
>> -typedef uint32_t odp_buffer_t;
>> +typedef odp_handle_t odp_buffer_t;
>>
>>  /** Invalid buffer */
>> -#define ODP_BUFFER_INVALID (0xffffffff)
>> +#define ODP_BUFFER_INVALID _odp_cast_scalar(odp_buffer_t, 0xffffffff)
>>
>>  /** ODP buffer segment */
>> -typedef odp_buffer_t odp_buffer_seg_t;
>> +typedef odp_handle_t odp_buffer_seg_t;
>>
>>  /** Invalid segment */
>> -#define ODP_SEGMENT_INVALID ODP_BUFFER_INVALID
>> +#define ODP_SEGMENT_INVALID ((odp_buffer_seg_t)ODP_BUFFER_INVALID)
>> +
>> +/** Get printable format of odp_buffer_t */
>> +static inline uint64_t odp_buffer_to_u64(odp_buffer_t hdl)
>> +{
>> +     return _odp_pri(hdl);
>> +}
>
> The re-org has make it easier for the implementors of ODP.
> However, its a bit harder to follow the structure now maybe.
>
> "static inline functions" should go into:
> platform/linux-generic/include/odp/*.h
>
> and typedefs, defines, enums, and structs that are platform specific
> should go into:
> platform/linux-generic/include/odp/plat/*_types.h
Me thinks there should be a couple more directory levels here. Why not
repeat plat and platform a couple of times?

>
> doxygen clean API stuff should be in:
> include/odp/api/*.h
>
>
> So the "static inline functions" in this patch set are currently in the wrong
> place.
>
> Taras, have I stated this correctly?
>
> Cheers,
> Anders
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
Taras Kondratiuk Feb. 3, 2015, 10:16 p.m. UTC | #5
On 02/03/2015 11:50 PM, Bill Fischofer wrote:
> The changes here are to
> platform/linux-generic/include/odp/plat/<type>_types.h.  This is where
> the strong typing is actually implemented for each ODP type in
> linux-generic.  The converter functions are part of that implementation,
> and are also inlined for performance, which is why they are here.
> 
> All other changes within each patch part are the modifications to other
> ODP files needed as a result of switching to strong typing for this
> type.  They are not separable since once strong typing is enabled for
> that type the code won't compile without these changes.
> 
> The doxygen stuff is in the APIs which are in include/odp/api/*.h as you
> mention.  The main difference between v4 and v5 was to break out the API
> definition changes separately, which is why this has gone from 10 to 19
> parts.
> 
> So it sounds like this is doing exactly what you suggest, no?

As I've commented in the previous message it is OK, because these API
are kind of exceptional. All other 'static inline' implementations
should not be in *_types.h. That is not related to your series. Just a
reminder.
Taras Kondratiuk Feb. 3, 2015, 10:24 p.m. UTC | #6
On 02/03/2015 11:57 PM, Ola Liljedahl wrote:
>> The re-org has make it easier for the implementors of ODP.
>> However, its a bit harder to follow the structure now maybe.
>>
>> "static inline functions" should go into:
>> platform/linux-generic/include/odp/*.h
>>
>> and typedefs, defines, enums, and structs that are platform specific
>> should go into:
>> platform/linux-generic/include/odp/plat/*_types.h
> Me thinks there should be a couple more directory levels here. Why not
> repeat plat and platform a couple of times?

'plat' is a header namespace to separate files after they are installed
and keep destination directory organized.
If you don't like the name please send a patch to rename it ;)
Ola Liljedahl Feb. 3, 2015, 10:28 p.m. UTC | #7
On 3 February 2015 at 23:24, Taras Kondratiuk
<taras.kondratiuk@linaro.org> wrote:
> On 02/03/2015 11:57 PM, Ola Liljedahl wrote:
>>> The re-org has make it easier for the implementors of ODP.
>>> However, its a bit harder to follow the structure now maybe.
>>>
>>> "static inline functions" should go into:
>>> platform/linux-generic/include/odp/*.h
>>>
>>> and typedefs, defines, enums, and structs that are platform specific
>>> should go into:
>>> platform/linux-generic/include/odp/plat/*_types.h
>> Me thinks there should be a couple more directory levels here. Why not
>> repeat plat and platform a couple of times?
>
> 'plat' is a header namespace to separate files after they are installed
> and keep destination directory organized.
> If you don't like the name please send a patch to rename it ;)
Taras if there was a description of the complete ODP directory
structure somewhere it would probably make sense.

>
>
Mike Holmes Feb. 3, 2015, 10:42 p.m. UTC | #8
On 3 February 2015 at 17:16, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> On 02/03/2015 11:50 PM, Bill Fischofer wrote:
> > The changes here are to
> > platform/linux-generic/include/odp/plat/<type>_types.h.  This is where
> > the strong typing is actually implemented for each ODP type in
> > linux-generic.  The converter functions are part of that implementation,
> > and are also inlined for performance, which is why they are here.
> >
> > All other changes within each patch part are the modifications to other
> > ODP files needed as a result of switching to strong typing for this
> > type.  They are not separable since once strong typing is enabled for
> > that type the code won't compile without these changes.
> >
> > The doxygen stuff is in the APIs which are in include/odp/api/*.h as you
> > mention.  The main difference between v4 and v5 was to break out the API
> > definition changes separately, which is why this has gone from 10 to 19
> > parts.
> >
> > So it sounds like this is doing exactly what you suggest, no?
>
> As I've commented in the previous message it is OK, because these API
> are kind of exceptional.


I have no issue making an exception, but I don't yet grasp the reason why
we make it, what do we gain by doing so ?




> All other 'static inline' implementations
> should not be in *_types.h. That is not related to your series. Just a
> reminder.
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Mike Holmes Feb. 3, 2015, 10:43 p.m. UTC | #9
On 3 February 2015 at 17:28, Ola Liljedahl <ola.liljedahl@linaro.org> wrote:

> On 3 February 2015 at 23:24, Taras Kondratiuk
> <taras.kondratiuk@linaro.org> wrote:
> > On 02/03/2015 11:57 PM, Ola Liljedahl wrote:
> >>> The re-org has make it easier for the implementors of ODP.
> >>> However, its a bit harder to follow the structure now maybe.
> >>>
> >>> "static inline functions" should go into:
> >>> platform/linux-generic/include/odp/*.h
> >>>
> >>> and typedefs, defines, enums, and structs that are platform specific
> >>> should go into:
> >>> platform/linux-generic/include/odp/plat/*_types.h
> >> Me thinks there should be a couple more directory levels here. Why not
> >> repeat plat and platform a couple of times?
> >
> > 'plat' is a header namespace to separate files after they are installed
> > and keep destination directory organized.
> > If you don't like the name please send a patch to rename it ;)
> Taras if there was a description of the complete ODP directory
> structure somewhere it would probably make sense.
>

+1 been trying to work it out so that at Connect we add such docs to the
correct place.


>
> >
> >
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
Taras Kondratiuk Feb. 3, 2015, 11:47 p.m. UTC | #10
On 02/04/2015 12:42 AM, Mike Holmes wrote:
> 
> 
> On 3 February 2015 at 17:16, Taras Kondratiuk
> <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>> wrote:
> 
>     On 02/03/2015 11:50 PM, Bill Fischofer wrote:
>     > The changes here are to
>     > platform/linux-generic/include/odp/plat/<type>_types.h.  This is where
>     > the strong typing is actually implemented for each ODP type in
>     > linux-generic.  The converter functions are part of that implementation,
>     > and are also inlined for performance, which is why they are here.
>     >
>     > All other changes within each patch part are the modifications to other
>     > ODP files needed as a result of switching to strong typing for this
>     > type.  They are not separable since once strong typing is enabled for
>     > that type the code won't compile without these changes.
>     >
>     > The doxygen stuff is in the APIs which are in include/odp/api/*.h as you
>     > mention.  The main difference between v4 and v5 was to break out the API
>     > definition changes separately, which is why this has gone from 10 to 19
>     > parts.
>     >
>     > So it sounds like this is doing exactly what you suggest, no?
> 
>     As I've commented in the previous message it is OK, because these API
>     are kind of exceptional. 
> 
> 
> I have no issue making an exception, but I don't yet grasp the reason
> why we make it, what do we gain by doing so ?

If some module need to use odp_buffer_t it is going to include
odp/plat/buffer_types.h. This module may need to print odp_buffer_t for
debug
purposes, so odp_buffer_to_u64() should be accessible.
Mike Holmes Feb. 4, 2015, midnight UTC | #11
On 3 February 2015 at 18:47, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> On 02/04/2015 12:42 AM, Mike Holmes wrote:
> >
> >
> > On 3 February 2015 at 17:16, Taras Kondratiuk
> > <taras.kondratiuk@linaro.org <mailto:taras.kondratiuk@linaro.org>>
> wrote:
> >
> >     On 02/03/2015 11:50 PM, Bill Fischofer wrote:
> >     > The changes here are to
> >     > platform/linux-generic/include/odp/plat/<type>_types.h.  This is
> where
> >     > the strong typing is actually implemented for each ODP type in
> >     > linux-generic.  The converter functions are part of that
> implementation,
> >     > and are also inlined for performance, which is why they are here.
> >     >
> >     > All other changes within each patch part are the modifications to
> other
> >     > ODP files needed as a result of switching to strong typing for this
> >     > type.  They are not separable since once strong typing is enabled
> for
> >     > that type the code won't compile without these changes.
> >     >
> >     > The doxygen stuff is in the APIs which are in include/odp/api/*.h
> as you
> >     > mention.  The main difference between v4 and v5 was to break out
> the API
> >     > definition changes separately, which is why this has gone from 10
> to 19
> >     > parts.
> >     >
> >     > So it sounds like this is doing exactly what you suggest, no?
> >
> >     As I've commented in the previous message it is OK, because these API
> >     are kind of exceptional.
> >
> >
> > I have no issue making an exception, but I don't yet grasp the reason
> > why we make it, what do we gain by doing so ?
>
> If some module need to use odp_buffer_t it is going to include
> odp/plat/buffer_types.h. This module may need to print odp_buffer_t for
> debug
> purposes, so odp_buffer_to_u64() should be accessible.
>

Thank you
Taras Kondratiuk Feb. 4, 2015, 9:14 a.m. UTC | #12
On 02/04/2015 12:28 AM, Ola Liljedahl wrote:
> On 3 February 2015 at 23:24, Taras Kondratiuk
> <taras.kondratiuk@linaro.org> wrote:
>> On 02/03/2015 11:57 PM, Ola Liljedahl wrote:
>>>> The re-org has make it easier for the implementors of ODP.
>>>> However, its a bit harder to follow the structure now maybe.
>>>>
>>>> "static inline functions" should go into:
>>>> platform/linux-generic/include/odp/*.h
>>>>
>>>> and typedefs, defines, enums, and structs that are platform specific
>>>> should go into:
>>>> platform/linux-generic/include/odp/plat/*_types.h
>>> Me thinks there should be a couple more directory levels here. Why not
>>> repeat plat and platform a couple of times?
>>
>> 'plat' is a header namespace to separate files after they are installed
>> and keep destination directory organized.
>> If you don't like the name please send a patch to rename it ;)
> Taras if there was a description of the complete ODP directory
> structure somewhere it would probably make sense.

Yeah that's a missing piece. Should a part Implementer's guide.
Current structure:

/usr/share/include/
├── odp.h                   - The only header included by applications (for now).
└── odp                     - Structure of this directory is up to implementation.
    │                         This is a linux-generic's structure which can be used as a reference.
    ├── <odp_module>.h      - One header per module. That is not mandatory, but recommended (in case we drop odp.h at some point).
    │                         Each header will normally include corresponding header from 'api' directory plus 'static inline' APIs.
    ├── api                 - A copy of <odp_src>/include/odp/api directory. Contains clean API headers.
    │   └── <odp_module>.h
    └── plat                - Contains types definitions and other platform specific stuff.
        ├── <odp_module>_types.h
        └── <some_sdk>.h
Mike Holmes Feb. 4, 2015, 1:33 p.m. UTC | #13
At HKG15, lets move the arch doc etc and create and split the information
we have into the right docs, we need to get this information recorded
because much of it evaporates on the list.

On 4 February 2015 at 04:14, Taras Kondratiuk <taras.kondratiuk@linaro.org>
wrote:

> On 02/04/2015 12:28 AM, Ola Liljedahl wrote:
> > On 3 February 2015 at 23:24, Taras Kondratiuk
> > <taras.kondratiuk@linaro.org> wrote:
> >> On 02/03/2015 11:57 PM, Ola Liljedahl wrote:
> >>>> The re-org has make it easier for the implementors of ODP.
> >>>> However, its a bit harder to follow the structure now maybe.
> >>>>
> >>>> "static inline functions" should go into:
> >>>> platform/linux-generic/include/odp/*.h
> >>>>
> >>>> and typedefs, defines, enums, and structs that are platform specific
> >>>> should go into:
> >>>> platform/linux-generic/include/odp/plat/*_types.h
> >>> Me thinks there should be a couple more directory levels here. Why not
> >>> repeat plat and platform a couple of times?
> >>
> >> 'plat' is a header namespace to separate files after they are installed
> >> and keep destination directory organized.
> >> If you don't like the name please send a patch to rename it ;)
> > Taras if there was a description of the complete ODP directory
> > structure somewhere it would probably make sense.
>
> Yeah that's a missing piece. Should a part Implementer's guide.
> Current structure:
>
> /usr/share/include/
> ├── odp.h                   - The only header included by applications
> (for now).
> └── odp                     - Structure of this directory is up to
> implementation.
>     │                         This is a linux-generic's structure which
> can be used as a reference.
>     ├── <odp_module>.h      - One header per module. That is not
> mandatory, but recommended (in case we drop odp.h at some point).
>     │                         Each header will normally include
> corresponding header from 'api' directory plus 'static inline' APIs.
>     ├── api                 - A copy of <odp_src>/include/odp/api
> directory. Contains clean API headers.
>     │   └── <odp_module>.h
>     └── plat                - Contains types definitions and other
> platform specific stuff.
>         ├── <odp_module>_types.h
>         └── <some_sdk>.h
>
>
>
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/lng-odp
>
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp/plat/buffer_types.h b/platform/linux-generic/include/odp/plat/buffer_types.h
index 8601e61..3e7070e 100644
--- a/platform/linux-generic/include/odp/plat/buffer_types.h
+++ b/platform/linux-generic/include/odp/plat/buffer_types.h
@@ -18,6 +18,7 @@  extern "C" {
 #endif
 
 #include <odp/std_types.h>
+#include <odp/plat/strong_types.h>
 
 /** @addtogroup odp_buffer ODP BUFFER
  *  Operations on a buffer.
@@ -25,16 +26,22 @@  extern "C" {
  */
 
 /** ODP buffer */
-typedef uint32_t odp_buffer_t;
+typedef odp_handle_t odp_buffer_t;
 
 /** Invalid buffer */
-#define ODP_BUFFER_INVALID (0xffffffff)
+#define ODP_BUFFER_INVALID _odp_cast_scalar(odp_buffer_t, 0xffffffff)
 
 /** ODP buffer segment */
-typedef odp_buffer_t odp_buffer_seg_t;
+typedef odp_handle_t odp_buffer_seg_t;
 
 /** Invalid segment */
-#define ODP_SEGMENT_INVALID ODP_BUFFER_INVALID
+#define ODP_SEGMENT_INVALID ((odp_buffer_seg_t)ODP_BUFFER_INVALID)
+
+/** Get printable format of odp_buffer_t */
+static inline uint64_t odp_buffer_to_u64(odp_buffer_t hdl)
+{
+	return _odp_pri(hdl);
+}
 
 /**
  * @}
diff --git a/platform/linux-generic/include/odp_buffer_inlines.h b/platform/linux-generic/include/odp_buffer_inlines.h
index 6a30a07..ba358f1 100644
--- a/platform/linux-generic/include/odp_buffer_inlines.h
+++ b/platform/linux-generic/include/odp_buffer_inlines.h
@@ -31,7 +31,7 @@  static inline odp_buffer_t odp_buffer_encode_handle(odp_buffer_hdr_t *hdr)
 		ODP_CACHE_LINE_SIZE;
 	handle.seg = 0;
 
-	return handle.u32;
+	return handle.handle;
 }
 
 static inline odp_buffer_t odp_hdr_to_buf(odp_buffer_hdr_t *hdr)
@@ -46,7 +46,7 @@  static inline odp_buffer_hdr_t *odp_buf_to_hdr(odp_buffer_t buf)
 	uint32_t index;
 	struct pool_entry_s *pool;
 
-	handle.u32 = buf;
+	handle.handle = buf;
 	pool_id    = handle.pool_id;
 	index      = handle.index;
 
@@ -100,7 +100,7 @@  static inline odp_buffer_hdr_t *validate_buf(odp_buffer_t buf)
 {
 	odp_buffer_bits_t handle;
 	odp_buffer_hdr_t *buf_hdr;
-	handle.u32 = buf;
+	handle.handle = buf;
 
 	/* For buffer handles, segment index must be 0 and pool id in range */
 	if (handle.seg != 0 || handle.pool_id >= ODP_CONFIG_POOLS)
@@ -150,7 +150,7 @@  static inline odp_buffer_seg_t segment_next(odp_buffer_hdr_t *buf,
 					    odp_buffer_seg_t seg)
 {
 	odp_buffer_bits_t seghandle;
-	seghandle.u32 = seg;
+	seghandle.handle = (odp_buffer_t)seg;
 
 	if (seg == ODP_SEGMENT_INVALID ||
 	    seghandle.prefix != buf->handle.prefix ||
@@ -158,7 +158,7 @@  static inline odp_buffer_seg_t segment_next(odp_buffer_hdr_t *buf,
 		return ODP_SEGMENT_INVALID;
 	else {
 		seghandle.seg++;
-		return (odp_buffer_seg_t)seghandle.u32;
+		return (odp_buffer_seg_t)seghandle.handle;
 	}
 }
 
@@ -171,7 +171,7 @@  static inline void *segment_map(odp_buffer_hdr_t *buf,
 	uint32_t seg_offset, buf_left;
 	odp_buffer_bits_t seghandle;
 	uint8_t *seg_addr;
-	seghandle.u32 = seg;
+	seghandle.handle = (odp_buffer_t)seg;
 
 	if (seghandle.prefix != buf->handle.prefix ||
 	    seghandle.seg >= buf->segcount)
diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
index b3137a7..f199e2e 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -77,29 +77,30 @@  _ODP_STATIC_ASSERT((ODP_CONFIG_PACKET_BUF_LEN_MAX %
 #define ODP_BUFFER_INVALID_INDEX (ODP_BUFFER_MAX_BUFFERS - 1)
 
 typedef union odp_buffer_bits_t {
-	uint32_t     u32;
 	odp_buffer_t handle;
-
-	struct {
+	union {
+		uint32_t     u32;
+		struct {
 #if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
-		uint32_t pool_id:ODP_BUFFER_POOL_BITS;
-		uint32_t index:ODP_BUFFER_INDEX_BITS;
-		uint32_t seg:ODP_BUFFER_SEG_BITS;
+			uint32_t pool_id:ODP_BUFFER_POOL_BITS;
+			uint32_t index:ODP_BUFFER_INDEX_BITS;
+			uint32_t seg:ODP_BUFFER_SEG_BITS;
 #else
-		uint32_t seg:ODP_BUFFER_SEG_BITS;
-		uint32_t index:ODP_BUFFER_INDEX_BITS;
-		uint32_t pool_id:ODP_BUFFER_POOL_BITS;
+			uint32_t seg:ODP_BUFFER_SEG_BITS;
+			uint32_t index:ODP_BUFFER_INDEX_BITS;
+			uint32_t pool_id:ODP_BUFFER_POOL_BITS;
 #endif
-	};
+		};
 
-	struct {
+		struct {
 #if ODP_BYTE_ORDER == ODP_BIG_ENDIAN
-		uint32_t prefix:ODP_BUFFER_PREFIX_BITS;
-		uint32_t pfxseg:ODP_BUFFER_SEG_BITS;
+			uint32_t prefix:ODP_BUFFER_PREFIX_BITS;
+			uint32_t pfxseg:ODP_BUFFER_SEG_BITS;
 #else
-		uint32_t pfxseg:ODP_BUFFER_SEG_BITS;
-		uint32_t prefix:ODP_BUFFER_PREFIX_BITS;
+			uint32_t pfxseg:ODP_BUFFER_SEG_BITS;
+			uint32_t prefix:ODP_BUFFER_PREFIX_BITS;
 #endif
+		};
 	};
 } odp_buffer_bits_t;
 
diff --git a/platform/linux-generic/odp_packet.c b/platform/linux-generic/odp_packet.c
index 968ad3e..a031f13 100644
--- a/platform/linux-generic/odp_packet.c
+++ b/platform/linux-generic/odp_packet.c
@@ -193,7 +193,7 @@  void *odp_packet_offset(odp_packet_t pkt, uint32_t offset, uint32_t *len,
 
 	if (addr != NULL && seg != NULL) {
 		odp_buffer_bits_t seghandle;
-		seghandle.u32 = (uint32_t)pkt;
+		seghandle.handle = pkt;
 		seghandle.seg = (pkt_hdr->headroom + offset) /
 			pkt_hdr->buf_hdr.segsize;
 		*seg = seghandle.handle;
@@ -325,7 +325,7 @@  odp_packet_seg_t odp_packet_last_seg(odp_packet_t pkt)
 	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
 	odp_buffer_bits_t seghandle;
 
-	seghandle.u32 = (uint32_t)pkt;
+	seghandle.handle = pkt;
 	seghandle.seg = pkt_hdr->buf_hdr.segcount - 1;
 	return seghandle.handle;
 }
@@ -334,7 +334,8 @@  odp_packet_seg_t odp_packet_next_seg(odp_packet_t pkt, odp_packet_seg_t seg)
 {
 	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
 
-	return segment_next(&pkt_hdr->buf_hdr, seg);
+	return (odp_packet_seg_t)segment_next(&pkt_hdr->buf_hdr,
+					      (odp_buffer_seg_t)seg);
 }
 
 /*
@@ -348,7 +349,7 @@  void *odp_packet_seg_buf_addr(odp_packet_t pkt, odp_packet_seg_t seg)
 {
 	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
 
-	return segment_map(&pkt_hdr->buf_hdr, seg, NULL,
+	return segment_map(&pkt_hdr->buf_hdr, (odp_buffer_seg_t)seg, NULL,
 			   pkt_hdr->headroom + pkt_hdr->frame_len, 0);
 }
 
@@ -362,7 +363,7 @@  void *odp_packet_seg_data(odp_packet_t pkt, odp_packet_seg_t seg)
 {
 	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
 
-	return segment_map(&pkt_hdr->buf_hdr, seg, NULL,
+	return segment_map(&pkt_hdr->buf_hdr, (odp_buffer_seg_t)seg, NULL,
 			   pkt_hdr->frame_len, pkt_hdr->headroom);
 }
 
@@ -371,7 +372,7 @@  uint32_t odp_packet_seg_data_len(odp_packet_t pkt, odp_packet_seg_t seg)
 	odp_packet_hdr_t *pkt_hdr = odp_packet_hdr(pkt);
 	uint32_t seglen = 0;
 
-	segment_map(&pkt_hdr->buf_hdr, seg, &seglen,
+	segment_map(&pkt_hdr->buf_hdr, (odp_buffer_seg_t)seg, &seglen,
 		    pkt_hdr->frame_len, pkt_hdr->headroom);
 
 	return seglen;