diff mbox

[PATCHv2] Factor ODP types into a common include file

Message ID 1415931525-18390-3-git-send-email-bill.fischofer@linaro.org
State New
Headers show

Commit Message

Bill Fischofer Nov. 14, 2014, 2:18 a.m. UTC
Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
---
 platform/linux-generic/Makefile.am                 |  1 +
 platform/linux-generic/include/api/odp_buffer.h    |  9 +--
 .../linux-generic/include/api/odp_buffer_pool.h    |  5 +-
 .../linux-generic/include/api/odp_impl_types.h     | 72 ++++++++++++++++++++++
 platform/linux-generic/include/api/odp_packet.h    | 30 ---------
 platform/linux-generic/include/api/odp_packet_io.h | 12 +---
 6 files changed, 76 insertions(+), 53 deletions(-)
 create mode 100644 platform/linux-generic/include/api/odp_impl_types.h

Comments

Anders Roxell Nov. 14, 2014, 5:42 p.m. UTC | #1
On 2014-11-13 20:18, Bill Fischofer wrote:
> Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> ---
>  platform/linux-generic/Makefile.am                 |  1 +
>  platform/linux-generic/include/api/odp_buffer.h    |  9 +--
>  .../linux-generic/include/api/odp_buffer_pool.h    |  5 +-
>  .../linux-generic/include/api/odp_impl_types.h     | 72 ++++++++++++++++++++++

Instead of of odp_impl_types.h change it to odp_platform_types.h

I think it's more clear what we mean.

>  platform/linux-generic/include/api/odp_packet.h    | 30 ---------
>  platform/linux-generic/include/api/odp_packet_io.h | 12 +---
>  6 files changed, 76 insertions(+), 53 deletions(-)
>  create mode 100644 platform/linux-generic/include/api/odp_impl_types.h
> 
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am
> index 0153a22..66ba7fb 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -36,6 +36,7 @@ include_HEADERS = \
>  		  $(top_srcdir)/platform/linux-generic/include/api/odp_ticketlock.h \
>  		  $(top_srcdir)/platform/linux-generic/include/api/odp_time.h \
>  		  $(top_srcdir)/platform/linux-generic/include/api/odp_timer.h \
> +		  $(top_srcdir)/platform/linux-generic/include/api/odp_impl_types.h \
>  		  $(top_srcdir)/platform/linux-generic/include/api/odp_version.h
>  
>  subdirheadersdir = $(includedir)
> diff --git a/platform/linux-generic/include/api/odp_buffer.h b/platform/linux-generic/include/api/odp_buffer.h
> index 289e0eb..141a16b 100644
> --- a/platform/linux-generic/include/api/odp_buffer.h
> +++ b/platform/linux-generic/include/api/odp_buffer.h
> @@ -20,20 +20,13 @@ extern "C" {
>  
>  
>  #include <odp_std_types.h>
> -
> +#include <odp_impl_types.h>
>  
>  /** @defgroup odp_buffer ODP BUFFER
>   *  Operations on a buffer.
>   *  @{
>   */
>  
> -/**
> - * ODP buffer
> - */
> -typedef uint32_t odp_buffer_t;
> -
> -#define ODP_BUFFER_INVALID (0xffffffff) /**< Invalid buffer */
> -
>  
>  /**
>   * Buffer start address
> diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h b/platform/linux-generic/include/api/odp_buffer_pool.h
> index d04abf0..c46044d 100644
> --- a/platform/linux-generic/include/api/odp_buffer_pool.h
> +++ b/platform/linux-generic/include/api/odp_buffer_pool.h
> @@ -21,6 +21,7 @@ extern "C" {
>  
>  
>  #include <odp_std_types.h>
> +#include <odp_impl_types.h>
>  #include <odp_buffer.h>
>  
>  /** @addtogroup odp_buffer
> @@ -34,10 +35,6 @@ extern "C" {
>  /** Invalid buffer pool */
>  #define ODP_BUFFER_POOL_INVALID   0
>  
> -/** ODP buffer pool */
> -typedef uint32_t odp_buffer_pool_t;
> -
> -
>  /**
>   * Create a buffer pool
>   *
> diff --git a/platform/linux-generic/include/api/odp_impl_types.h b/platform/linux-generic/include/api/odp_impl_types.h
> new file mode 100644
> index 0000000..f4deedb
> --- /dev/null
> +++ b/platform/linux-generic/include/api/odp_impl_types.h
> @@ -0,0 +1,72 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + * ODP implementation types
> + * This file contains all of the implementation-defined types for ODP
> + * abstract definitions. Having this in one file means that other ODP
> + * API files are implementation-independent and avoids circular
> + * dependencies for files that refer to types managed by other
> + * components. Included here are typedefs and related typed constants
> + * that are referenced by other ODP API files.

Again, why are you only using 70 characters per row and not 80?

> + */
> +
> +#ifndef ODP_IMPL_TYPES_H_
> +#define ODP_IMPL_TYPES_H_
> +
> +/** @defgroup odp_types ODP TYPES

ODP PLATFORM TYPES

> + *  Implementation definitions for ODP abstract types.

Implementation specific definitions of ODP abstract types.


Cheers,
Anders
Bill Fischofer Nov. 14, 2014, 6:27 p.m. UTC | #2
OK, will make these changes in v3.  The line fill is done automagically by
Emacs.  I just need to change the default wrap column from 66 to 80 if that
works better for you.

Bill

On Fri, Nov 14, 2014 at 11:42 AM, Anders Roxell <anders.roxell@linaro.org>
wrote:

> On 2014-11-13 20:18, Bill Fischofer wrote:
> > Signed-off-by: Bill Fischofer <bill.fischofer@linaro.org>
> > ---
> >  platform/linux-generic/Makefile.am                 |  1 +
> >  platform/linux-generic/include/api/odp_buffer.h    |  9 +--
> >  .../linux-generic/include/api/odp_buffer_pool.h    |  5 +-
> >  .../linux-generic/include/api/odp_impl_types.h     | 72
> ++++++++++++++++++++++
>
> Instead of of odp_impl_types.h change it to odp_platform_types.h
>
> I think it's more clear what we mean.
>
> >  platform/linux-generic/include/api/odp_packet.h    | 30 ---------
> >  platform/linux-generic/include/api/odp_packet_io.h | 12 +---
> >  6 files changed, 76 insertions(+), 53 deletions(-)
> >  create mode 100644 platform/linux-generic/include/api/odp_impl_types.h
> >
> > diff --git a/platform/linux-generic/Makefile.am
> b/platform/linux-generic/Makefile.am
> > index 0153a22..66ba7fb 100644
> > --- a/platform/linux-generic/Makefile.am
> > +++ b/platform/linux-generic/Makefile.am
> > @@ -36,6 +36,7 @@ include_HEADERS = \
> >
>  $(top_srcdir)/platform/linux-generic/include/api/odp_ticketlock.h \
> >
>  $(top_srcdir)/platform/linux-generic/include/api/odp_time.h \
> >
>  $(top_srcdir)/platform/linux-generic/include/api/odp_timer.h \
> > +
>  $(top_srcdir)/platform/linux-generic/include/api/odp_impl_types.h \
> >
>  $(top_srcdir)/platform/linux-generic/include/api/odp_version.h
> >
> >  subdirheadersdir = $(includedir)
> > diff --git a/platform/linux-generic/include/api/odp_buffer.h
> b/platform/linux-generic/include/api/odp_buffer.h
> > index 289e0eb..141a16b 100644
> > --- a/platform/linux-generic/include/api/odp_buffer.h
> > +++ b/platform/linux-generic/include/api/odp_buffer.h
> > @@ -20,20 +20,13 @@ extern "C" {
> >
> >
> >  #include <odp_std_types.h>
> > -
> > +#include <odp_impl_types.h>
> >
> >  /** @defgroup odp_buffer ODP BUFFER
> >   *  Operations on a buffer.
> >   *  @{
> >   */
> >
> > -/**
> > - * ODP buffer
> > - */
> > -typedef uint32_t odp_buffer_t;
> > -
> > -#define ODP_BUFFER_INVALID (0xffffffff) /**< Invalid buffer */
> > -
> >
> >  /**
> >   * Buffer start address
> > diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h
> b/platform/linux-generic/include/api/odp_buffer_pool.h
> > index d04abf0..c46044d 100644
> > --- a/platform/linux-generic/include/api/odp_buffer_pool.h
> > +++ b/platform/linux-generic/include/api/odp_buffer_pool.h
> > @@ -21,6 +21,7 @@ extern "C" {
> >
> >
> >  #include <odp_std_types.h>
> > +#include <odp_impl_types.h>
> >  #include <odp_buffer.h>
> >
> >  /** @addtogroup odp_buffer
> > @@ -34,10 +35,6 @@ extern "C" {
> >  /** Invalid buffer pool */
> >  #define ODP_BUFFER_POOL_INVALID   0
> >
> > -/** ODP buffer pool */
> > -typedef uint32_t odp_buffer_pool_t;
> > -
> > -
> >  /**
> >   * Create a buffer pool
> >   *
> > diff --git a/platform/linux-generic/include/api/odp_impl_types.h
> b/platform/linux-generic/include/api/odp_impl_types.h
> > new file mode 100644
> > index 0000000..f4deedb
> > --- /dev/null
> > +++ b/platform/linux-generic/include/api/odp_impl_types.h
> > @@ -0,0 +1,72 @@
> > +/* Copyright (c) 2014, Linaro Limited
> > + * All rights reserved.
> > + *
> > + * SPDX-License-Identifier:     BSD-3-Clause
> > + */
> > +
> > +
> > +/**
> > + * @file
> > + * ODP implementation types
> > + * This file contains all of the implementation-defined types for ODP
> > + * abstract definitions. Having this in one file means that other ODP
> > + * API files are implementation-independent and avoids circular
> > + * dependencies for files that refer to types managed by other
> > + * components. Included here are typedefs and related typed constants
> > + * that are referenced by other ODP API files.
>
> Again, why are you only using 70 characters per row and not 80?
>
> > + */
> > +
> > +#ifndef ODP_IMPL_TYPES_H_
> > +#define ODP_IMPL_TYPES_H_
> > +
> > +/** @defgroup odp_types ODP TYPES
>
> ODP PLATFORM TYPES
>
> > + *  Implementation definitions for ODP abstract types.
>
> Implementation specific definitions of ODP abstract types.
>
>
> Cheers,
> Anders
>
Taras Kondratiuk Nov. 18, 2014, 3:16 p.m. UTC | #3
On 11/14/2014 04:18 AM, Bill Fischofer wrote:
> diff --git a/platform/linux-generic/include/api/odp_impl_types.h b/platform/linux-generic/include/api/odp_impl_types.h
> new file mode 100644
> index 0000000..f4deedb
> --- /dev/null
> +++ b/platform/linux-generic/include/api/odp_impl_types.h
> @@ -0,0 +1,72 @@
> +/* Copyright (c) 2014, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + * ODP implementation types
> + * This file contains all of the implementation-defined types for ODP
> + * abstract definitions. Having this in one file means that other ODP
> + * API files are implementation-independent and avoids circular
> + * dependencies for files that refer to types managed by other
> + * components. Included here are typedefs and related typed constants
> + * that are referenced by other ODP API files.
> + */
> +
> +#ifndef ODP_IMPL_TYPES_H_
> +#define ODP_IMPL_TYPES_H_
> +
> +/** @defgroup odp_types ODP TYPES
> + *  Implementation definitions for ODP abstract types.
> + *  @{
> + */
> +
> +/** ODP Buffer pool */
> +typedef uint32_t odp_buffer_pool_t;
> +
> +/** ODP buffer */
> +typedef uint32_t odp_buffer_t;
> +
> +/** Invalid buffer */
> +#define ODP_BUFFER_INVALID (0xffffffff)
> +
> +/** ODP packet */
> +typedef odp_buffer_t odp_packet_t;
> +
> +/** Invalid packet */
> +#define ODP_PACKET_INVALID ODP_BUFFER_INVALID
> +
> +/** Invalid offset */
> +#define ODP_PACKET_OFFSET_INVALID ((uint32_t)-1)
> +
> +/** ODP packet segment */
> +typedef int odp_packet_seg_t;
> +
> +/** Invalid packet segment */
> +#define ODP_PACKET_SEG_INVALID -1
> +
> +/** ODP packet segment info */
> +typedef struct odp_packet_seg_info_t {
> +	void   *addr;      /**< Segment start address */
> +	size_t  size;      /**< Segment maximum data size */
> +	void   *data;      /**< Segment data address */
> +	size_t  data_len;  /**< Segment data length */
> +} odp_packet_seg_info_t;

Sorry for a late comment.
odp_packet_seg_info_t is a public structure visible to a user.
It shouldn't be changed by implementation.
Bill Fischofer Nov. 18, 2014, 4:31 p.m. UTC | #4
This patch is against the tip, not the spec.  It would be reorganized as
part of implementing the spec.

For v1.0 there may well be changes needed here as well, but I'd prefer to
get this in now so that we can delta off it as needed rather than keep more
patches in limbo.

Bill

On Tue, Nov 18, 2014 at 9:16 AM, Taras Kondratiuk <
taras.kondratiuk@linaro.org> wrote:

> On 11/14/2014 04:18 AM, Bill Fischofer wrote:
>
>> diff --git a/platform/linux-generic/include/api/odp_impl_types.h
>> b/platform/linux-generic/include/api/odp_impl_types.h
>> new file mode 100644
>> index 0000000..f4deedb
>> --- /dev/null
>> +++ b/platform/linux-generic/include/api/odp_impl_types.h
>> @@ -0,0 +1,72 @@
>> +/* Copyright (c) 2014, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier:     BSD-3-Clause
>> + */
>> +
>> +
>> +/**
>> + * @file
>> + * ODP implementation types
>> + * This file contains all of the implementation-defined types for ODP
>> + * abstract definitions. Having this in one file means that other ODP
>> + * API files are implementation-independent and avoids circular
>> + * dependencies for files that refer to types managed by other
>> + * components. Included here are typedefs and related typed constants
>> + * that are referenced by other ODP API files.
>> + */
>> +
>> +#ifndef ODP_IMPL_TYPES_H_
>> +#define ODP_IMPL_TYPES_H_
>> +
>> +/** @defgroup odp_types ODP TYPES
>> + *  Implementation definitions for ODP abstract types.
>> + *  @{
>> + */
>> +
>> +/** ODP Buffer pool */
>> +typedef uint32_t odp_buffer_pool_t;
>> +
>> +/** ODP buffer */
>> +typedef uint32_t odp_buffer_t;
>> +
>> +/** Invalid buffer */
>> +#define ODP_BUFFER_INVALID (0xffffffff)
>> +
>> +/** ODP packet */
>> +typedef odp_buffer_t odp_packet_t;
>> +
>> +/** Invalid packet */
>> +#define ODP_PACKET_INVALID ODP_BUFFER_INVALID
>> +
>> +/** Invalid offset */
>> +#define ODP_PACKET_OFFSET_INVALID ((uint32_t)-1)
>> +
>> +/** ODP packet segment */
>> +typedef int odp_packet_seg_t;
>> +
>> +/** Invalid packet segment */
>> +#define ODP_PACKET_SEG_INVALID -1
>> +
>> +/** ODP packet segment info */
>> +typedef struct odp_packet_seg_info_t {
>> +       void   *addr;      /**< Segment start address */
>> +       size_t  size;      /**< Segment maximum data size */
>> +       void   *data;      /**< Segment data address */
>> +       size_t  data_len;  /**< Segment data length */
>> +} odp_packet_seg_info_t;
>>
>
> Sorry for a late comment.
> odp_packet_seg_info_t is a public structure visible to a user.
> It shouldn't be changed by implementation.
>
diff mbox

Patch

diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am
index 0153a22..66ba7fb 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -36,6 +36,7 @@  include_HEADERS = \
 		  $(top_srcdir)/platform/linux-generic/include/api/odp_ticketlock.h \
 		  $(top_srcdir)/platform/linux-generic/include/api/odp_time.h \
 		  $(top_srcdir)/platform/linux-generic/include/api/odp_timer.h \
+		  $(top_srcdir)/platform/linux-generic/include/api/odp_impl_types.h \
 		  $(top_srcdir)/platform/linux-generic/include/api/odp_version.h
 
 subdirheadersdir = $(includedir)
diff --git a/platform/linux-generic/include/api/odp_buffer.h b/platform/linux-generic/include/api/odp_buffer.h
index 289e0eb..141a16b 100644
--- a/platform/linux-generic/include/api/odp_buffer.h
+++ b/platform/linux-generic/include/api/odp_buffer.h
@@ -20,20 +20,13 @@  extern "C" {
 
 
 #include <odp_std_types.h>
-
+#include <odp_impl_types.h>
 
 /** @defgroup odp_buffer ODP BUFFER
  *  Operations on a buffer.
  *  @{
  */
 
-/**
- * ODP buffer
- */
-typedef uint32_t odp_buffer_t;
-
-#define ODP_BUFFER_INVALID (0xffffffff) /**< Invalid buffer */
-
 
 /**
  * Buffer start address
diff --git a/platform/linux-generic/include/api/odp_buffer_pool.h b/platform/linux-generic/include/api/odp_buffer_pool.h
index d04abf0..c46044d 100644
--- a/platform/linux-generic/include/api/odp_buffer_pool.h
+++ b/platform/linux-generic/include/api/odp_buffer_pool.h
@@ -21,6 +21,7 @@  extern "C" {
 
 
 #include <odp_std_types.h>
+#include <odp_impl_types.h>
 #include <odp_buffer.h>
 
 /** @addtogroup odp_buffer
@@ -34,10 +35,6 @@  extern "C" {
 /** Invalid buffer pool */
 #define ODP_BUFFER_POOL_INVALID   0
 
-/** ODP buffer pool */
-typedef uint32_t odp_buffer_pool_t;
-
-
 /**
  * Create a buffer pool
  *
diff --git a/platform/linux-generic/include/api/odp_impl_types.h b/platform/linux-generic/include/api/odp_impl_types.h
new file mode 100644
index 0000000..f4deedb
--- /dev/null
+++ b/platform/linux-generic/include/api/odp_impl_types.h
@@ -0,0 +1,72 @@ 
+/* Copyright (c) 2014, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier:     BSD-3-Clause
+ */
+
+
+/**
+ * @file
+ * ODP implementation types
+ * This file contains all of the implementation-defined types for ODP
+ * abstract definitions. Having this in one file means that other ODP
+ * API files are implementation-independent and avoids circular
+ * dependencies for files that refer to types managed by other
+ * components. Included here are typedefs and related typed constants
+ * that are referenced by other ODP API files.
+ */
+
+#ifndef ODP_IMPL_TYPES_H_
+#define ODP_IMPL_TYPES_H_
+
+/** @defgroup odp_types ODP TYPES
+ *  Implementation definitions for ODP abstract types.
+ *  @{
+ */
+
+/** ODP Buffer pool */
+typedef uint32_t odp_buffer_pool_t;
+
+/** ODP buffer */
+typedef uint32_t odp_buffer_t;
+
+/** Invalid buffer */
+#define ODP_BUFFER_INVALID (0xffffffff)
+
+/** ODP packet */
+typedef odp_buffer_t odp_packet_t;
+
+/** Invalid packet */
+#define ODP_PACKET_INVALID ODP_BUFFER_INVALID
+
+/** Invalid offset */
+#define ODP_PACKET_OFFSET_INVALID ((uint32_t)-1)
+
+/** ODP packet segment */
+typedef int odp_packet_seg_t;
+
+/** Invalid packet segment */
+#define ODP_PACKET_SEG_INVALID -1
+
+/** ODP packet segment info */
+typedef struct odp_packet_seg_info_t {
+	void   *addr;      /**< Segment start address */
+	size_t  size;      /**< Segment maximum data size */
+	void   *data;      /**< Segment data address */
+	size_t  data_len;  /**< Segment data length */
+} odp_packet_seg_info_t;
+
+/** ODP packet IO handle */
+typedef uint32_t odp_pktio_t;
+
+/** Invalid packet IO handle */
+#define ODP_PKTIO_INVALID 0
+
+/** odp_pktio_t value to indicate any port */
+#define ODP_PKTIO_ANY ((odp_pktio_t)~0)
+
+/**
+ * @}
+ */
+
+#endif
diff --git a/platform/linux-generic/include/api/odp_packet.h b/platform/linux-generic/include/api/odp_packet.h
index 688e047..5298fa0 100644
--- a/platform/linux-generic/include/api/odp_packet.h
+++ b/platform/linux-generic/include/api/odp_packet.h
@@ -25,36 +25,6 @@  extern "C" {
  *  @{
  */
 
-/**
- * ODP packet descriptor
- */
-typedef odp_buffer_t odp_packet_t;
-
-/** Invalid packet */
-#define ODP_PACKET_INVALID ODP_BUFFER_INVALID
-
-/** Invalid offset */
-#define ODP_PACKET_OFFSET_INVALID ((uint32_t)-1)
-
-
-/**
- * ODP packet segment handle
- */
-typedef int odp_packet_seg_t;
-
-/** Invalid packet segment */
-#define ODP_PACKET_SEG_INVALID -1
-
-/**
- * ODP packet segment info
- */
-typedef struct odp_packet_seg_info_t {
-	void   *addr;      /**< Segment start address */
-	size_t  size;      /**< Segment maximum data size */
-	void   *data;      /**< Segment data address */
-	size_t  data_len;  /**< Segment data length */
-} odp_packet_seg_info_t;
-
 
 /**
  * Initialize the packet
diff --git a/platform/linux-generic/include/api/odp_packet_io.h b/platform/linux-generic/include/api/odp_packet_io.h
index 360636d..45666aa 100644
--- a/platform/linux-generic/include/api/odp_packet_io.h
+++ b/platform/linux-generic/include/api/odp_packet_io.h
@@ -19,6 +19,7 @@  extern "C" {
 #endif
 
 #include <odp_std_types.h>
+#include <odp_impl_types.h>
 #include <odp_buffer_pool.h>
 #include <odp_packet.h>
 #include <odp_queue.h>
@@ -28,17 +29,6 @@  extern "C" {
  *  @{
  */
 
-/** ODP packet IO handle */
-typedef uint32_t odp_pktio_t;
-
-/** Invalid packet IO handle */
-#define ODP_PKTIO_INVALID 0
-
-/**
- * odp_pktio_t value to indicate any port
- */
-#define ODP_PKTIO_ANY ((odp_pktio_t)~0)
-
 /**
  * Open an ODP packet IO instance
  *