I like it too. It was a bit confusing in the beginning, because now there is include/odp/api for the public API, whereas the platform includes are include//api/odp. But that sort of makes easier to depict which definition is part of the API and which is the platform (potentially) inline declaration.
Maybe there could be an include file with all the inline declaration to make the platform/xxx/include/api/odp/xxx.h files look better, because as Bill points out you now need to interleave static inline declaration between lines of #include. The first obvious downside is that it's harder to find documentation for platform defines, take ODP_SCHED_WAIT for example. The programmer must always remember to look for the documentation in the public API, if he doesn't want to use the generated html docs. I found the extra plat/ directory a bit confusing too, right now it only has the platform types in it but I guess the argument for having it is that it contains extra files that don't have a correspondent in the public API. So maybe we just need to document that really well. I gave it a shot at reorganizing the structure at some point, though I didn't get many eyes looking at it, but I really think this is the way to go. It's not all perfect, but it's the best option I've seen so far, too little to argue against. On Fri, Jan 16, 2015 at 3:33 AM, Bill Fischofer <[email protected]> wrote: > This looks good. It also seems to provide a relatively straightforward path > for platform-specific inlines. I tested this with a simple test to > platform/linux-generic/include/api/odp/packet.h: > > #include <odp/std_types.h> > #include <odp/plat/packet_io_types.h> > #include <odp/plat/packet_types.h> > #include <odp/plat/buffer_types.h> > #include <odp/plat/buffer_pool_types.h> > > /* Prototypes for inlined functions must precede the public API */ > static inline odp_buffer_t odp_packet_to_buffer(odp_packet_t pkt); > > /* The platform-independent public API */ > #include <odp/api/packet.h> > > /* Now OK to define inlined implementations of overridden public APIs */ > static inline odp_buffer_t odp_packet_to_buffer(odp_packet_t pkt) > { > return (odp_buffer_t)pkt; > } > > It's a bit awkward, but this allows odp_packet_to_buffer() to be inlined > without having to modify the platform-independent definition of the API. > The static inline prototype has to precede the #include <odp/api/packet.h> > but it does the job. If the platform prototype or inlined version has a > different signature (except for ODP types, which will persist until we > implement strong typing) than the public API then the compiler will flag the > difference, so that's covered as well. > > Since this is a major change it's too late to get this into the 0.8 release, > however we should try to merge this early next week since everything pending > will have to rebase after this merge. But I don't see any reason why we > can't have this be one of the main features of 0.9. > > For this series: > > Reviewed-and-tested-by: Bill Fischofer <[email protected]> > > On Thu, Jan 15, 2015 at 8:23 AM, Anders Roxell <[email protected]> > wrote: >> >> Hi, >> >> This patchset: >> 1. Will make it easier for platform implementors to maintain new ODP >> versions. >> 2. Platform independent stuff is moved out from linux-generic >> 3. Tries to increase structure to the installed header files >> >> The following changes since commit >> 475705c3728d8b7b9073f065798a6b3ca9880b5c: >> >> validation: synchronizer tests (2015-01-14 18:01:48 +0300) >> >> are available in the git repository at: >> >> git://git.linaro.org/people/anders.roxell/odp.git master >> >> for you to fetch changes up to aa9739a65b5caec49a17d3f30bd28105f5c6dfd0: >> >> api: schedule: move defines (2015-01-15 12:35:55 +0100) >> >> ---------------------------------------------------------------- >> Anders Roxell (15): >> api: move generic API into the odp namespace >> api: align: move internal macros internal >> api: align: move platform defines out of public API >> api: buffer_pool: move typedef and define >> api: buffer: move typedefs and defines >> api: packet: move typedefs and defines >> api: packet_io: move typedef and defines >> api: shared_memory: move typedef and defines >> api: std_types: move typedef >> api: plat: move platform type headers into plat directory >> helper: move headers into namespace the odp/helper >> api: remove platform includes >> api: queue: move types and defines >> api: crypto: move type, define and enums >> api: schedule: move defines >> >> Taras Kondratiuk (5): >> api: move ODP headers to a directory >> doc: cfg: disable TYPEDEF_HIDES_STRUCT >> doc: cfg: fix structure attributes parsing >> api: atomic: move types documentation out of linux-generic >> api: byteorder: move types documentation out of linux-generic >> >> aminclude.am | 5 +- >> doc/doxygen.cfg | 13 +- >> example/Makefile.inc | 2 +- >> example/generator/odp_generator.c | 10 +- >> example/ipsec/odp_ipsec.c | 10 +- >> example/ipsec/odp_ipsec_cache.c | 2 +- >> example/ipsec/odp_ipsec_cache.h | 2 +- >> example/ipsec/odp_ipsec_fwd_db.h | 2 +- >> example/ipsec/odp_ipsec_loop_db.h | 2 +- >> example/ipsec/odp_ipsec_misc.h | 6 +- >> example/ipsec/odp_ipsec_stream.c | 6 +- >> example/l2fwd/odp_l2fwd.c | 6 +- >> example/packet/odp_pktio.c | 6 +- >> example/timer/odp_timer_test.c | 2 +- >> .../include/{odph_chksum.h => odp/helper/chksum.h} | 2 +- >> helper/include/{odph_eth.h => odp/helper/eth.h} | 8 +- >> helper/include/{odph_icmp.h => odp/helper/icmp.h} | 6 +- >> helper/include/{odph_ip.h => odp/helper/ip.h} | 8 +- >> .../include/{odph_ipsec.h => odp/helper/ipsec.h} | 8 +- >> .../include/{odph_linux.h => odp/helper/linux.h} | 0 >> helper/include/{odph_ring.h => odp/helper/ring.h} | 6 +- >> helper/include/{odph_tcp.h => odp/helper/tcp.h} | 6 +- >> helper/include/{odph_udp.h => odp/helper/udp.h} | 6 +- >> include/odp.h | 56 +++ >> include/odp/api/align.h | 77 ++++ >> include/odp/api/atomic.h | 260 +++++++++++++ >> .../api/odp_barrier.h => include/odp/api/barrier.h | 3 - >> .../api/odp_buffer.h => include/odp/api/buffer.h | 23 +- >> .../odp/api/buffer_pool.h | 15 +- >> include/odp/api/byteorder.h | 180 +++++++++ >> .../odp/api/classification.h | 6 - >> .../odp_compiler.h => include/odp/api/compiler.h | 0 >> .../api/odp_config.h => include/odp/api/config.h | 0 >> .../odp_coremask.h => include/odp/api/coremask.h | 58 +-- >> .../api/odp_crypto.h => include/odp/api/crypto.h | 115 +++--- >> .../api/odp_debug.h => include/odp/api/debug.h | 2 +- >> .../api/odp_hints.h => include/odp/api/hints.h | 0 >> .../api/odp_init.h => include/odp/api/init.h | 2 +- >> .../api/odp_packet.h => include/odp/api/packet.h | 28 +- >> .../odp/api/packet_flags.h | 4 +- >> .../odp_packet_io.h => include/odp/api/packet_io.h | 20 +- >> .../api/odp_queue.h => include/odp/api/queue.h | 110 ++++-- >> .../api/odp_rwlock.h => include/odp/api/rwlock.h | 2 +- >> .../odp_schedule.h => include/odp/api/schedule.h | 16 +- >> .../odp/api/shared_memory.h | 18 +- >> .../odp_spinlock.h => include/odp/api/spinlock.h | 2 +- >> include/odp/api/std_types.h | 42 +++ >> include/odp/api/sync.h | 42 +++ >> .../odp/api/system_info.h | 2 - >> .../api/odp_thread.h => include/odp/api/thread.h | 0 >> .../odp/api/ticketlock.h | 4 +- >> .../api/odp_time.h => include/odp/api/time.h | 2 - >> .../api/odp_timer.h => include/odp/api/timer.h | 13 +- >> .../api/odp_version.h => include/odp/api/version.h | 0 >> platform/linux-generic/Makefile.am | 134 ++++--- >> platform/linux-generic/include/api/odp.h | 56 --- >> .../include/api/{odp_align.h => odp/align.h} | 33 +- >> platform/linux-generic/include/api/odp/atomic.h | 194 ++++++++++ >> platform/linux-generic/include/api/odp/barrier.h | 23 ++ >> platform/linux-generic/include/api/odp/buffer.h | 25 ++ >> .../linux-generic/include/api/odp/buffer_pool.h | 25 ++ >> platform/linux-generic/include/api/odp/byteorder.h | 137 +++++++ >> .../linux-generic/include/api/odp/classification.h | 27 ++ >> platform/linux-generic/include/api/odp/compiler.h | 22 ++ >> platform/linux-generic/include/api/odp/config.h | 22 ++ >> platform/linux-generic/include/api/odp/coremask.h | 53 +++ >> platform/linux-generic/include/api/odp/crypto.h | 28 ++ >> platform/linux-generic/include/api/odp/debug.h | 22 ++ >> platform/linux-generic/include/api/odp/hints.h | 22 ++ >> platform/linux-generic/include/api/odp/init.h | 22 ++ >> platform/linux-generic/include/api/odp/packet.h | 27 ++ >> .../linux-generic/include/api/odp/packet_flags.h | 21 ++ >> platform/linux-generic/include/api/odp/packet_io.h | 27 ++ >> .../include/api/odp/plat/atomic_types.h | 81 ++++ >> .../include/api/odp/plat/buffer_pool_types.h | 37 ++ >> .../include/api/odp/plat/buffer_types.h | 45 +++ >> .../include/api/odp/plat/byteorder_types.h | 91 +++++ >> .../include/api/odp/plat/coremask_types.h | 49 +++ >> .../include/api/odp/plat/crypto_types.h | 82 ++++ >> .../include/api/odp/plat/packet_io_types.h | 40 ++ >> .../include/api/odp/plat/packet_types.h | 47 +++ >> .../include/api/odp/plat/queue_types.h | 75 ++++ >> .../include/api/odp/plat/schedule_types.h | 39 ++ >> .../include/api/odp/plat/shared_memory_types.h | 39 ++ >> platform/linux-generic/include/api/odp/queue.h | 26 ++ >> platform/linux-generic/include/api/odp/rwlock.h | 21 ++ >> platform/linux-generic/include/api/odp/schedule.h | 23 ++ >> .../linux-generic/include/api/odp/shared_memory.h | 24 ++ >> platform/linux-generic/include/api/odp/spinlock.h | 22 ++ >> .../api/{odp_std_types.h => odp/std_types.h} | 22 +- >> .../include/api/{odp_sync.h => odp/sync.h} | 25 +- >> .../linux-generic/include/api/odp/system_info.h | 23 ++ >> platform/linux-generic/include/api/odp/thread.h | 22 ++ >> .../linux-generic/include/api/odp/ticketlock.h | 22 ++ >> platform/linux-generic/include/api/odp/time.h | 22 ++ >> platform/linux-generic/include/api/odp/timer.h | 27 ++ >> platform/linux-generic/include/api/odp/version.h | 22 ++ >> platform/linux-generic/include/api/odp_atomic.h | 419 >> --------------------- >> platform/linux-generic/include/api/odp_byteorder.h | 287 -------------- >> .../linux-generic/include/api/odp_platform_types.h | 81 ---- >> .../linux-generic/include/odp_align_internal.h | 23 +- >> .../linux-generic/include/odp_atomic_internal.h | 8 +- >> .../linux-generic/include/odp_buffer_internal.h | 18 +- >> .../include/odp_buffer_pool_internal.h | 20 +- >> .../include/odp_classification_datamodel.h | 4 +- >> .../include/odp_classification_inlines.h | 10 +- >> .../include/odp_classification_internal.h | 6 +- >> .../linux-generic/include/odp_debug_internal.h | 2 +- >> .../linux-generic/include/odp_packet_internal.h | 8 +- >> .../linux-generic/include/odp_packet_io_internal.h | 6 +- >> platform/linux-generic/include/odp_packet_socket.h | 10 +- >> .../linux-generic/include/odp_queue_internal.h | 10 +- >> .../linux-generic/include/odp_schedule_internal.h | 4 +- >> .../linux-generic/include/odp_timer_internal.h | 6 +- >> platform/linux-generic/odp_barrier.c | 4 +- >> platform/linux-generic/odp_buffer.c | 2 +- >> platform/linux-generic/odp_buffer_pool.c | 12 +- >> platform/linux-generic/odp_classification.c | 16 +- >> platform/linux-generic/odp_coremask.c | 2 +- >> platform/linux-generic/odp_crypto.c | 16 +- >> platform/linux-generic/odp_impl.c | 2 +- >> platform/linux-generic/odp_init.c | 4 +- >> platform/linux-generic/odp_linux.c | 8 +- >> platform/linux-generic/odp_packet.c | 14 +- >> platform/linux-generic/odp_packet_flags.c | 2 +- >> platform/linux-generic/odp_packet_io.c | 10 +- >> platform/linux-generic/odp_packet_socket.c | 6 +- >> platform/linux-generic/odp_queue.c | 20 +- >> platform/linux-generic/odp_ring.c | 10 +- >> platform/linux-generic/odp_rwlock.c | 4 +- >> platform/linux-generic/odp_schedule.c | 22 +- >> platform/linux-generic/odp_shared_memory.c | 10 +- >> platform/linux-generic/odp_spinlock.c | 2 +- >> platform/linux-generic/odp_system_info.c | 4 +- >> platform/linux-generic/odp_thread.c | 10 +- >> platform/linux-generic/odp_ticketlock.c | 6 +- >> platform/linux-generic/odp_time.c | 6 +- >> platform/linux-generic/odp_timer.c | 26 +- >> platform/linux-generic/odp_weak.c | 4 +- >> scripts/odp_version.sh | 2 +- >> test/Makefile.inc | 2 +- >> test/api_test/odp_atomic_test.h | 2 +- >> test/api_test/odp_common.c | 2 +- >> test/api_test/odp_ring_test.c | 2 +- >> test/performance/odp_scheduling.c | 2 +- >> test/validation/common/odp_cunit_common.c | 2 +- >> test/validation/odp_pktio.c | 6 +- >> 147 files changed, 2878 insertions(+), 1358 deletions(-) >> rename helper/include/{odph_chksum.h => odp/helper/chksum.h} (96%) >> rename helper/include/{odph_eth.h => odp/helper/eth.h} (96%) >> rename helper/include/{odph_icmp.h => odp/helper/icmp.h} (97%) >> rename helper/include/{odph_ip.h => odp/helper/ip.h} (97%) >> rename helper/include/{odph_ipsec.h => odp/helper/ipsec.h} (94%) >> rename helper/include/{odph_linux.h => odp/helper/linux.h} (100%) >> rename helper/include/{odph_ring.h => odp/helper/ring.h} (99%) >> rename helper/include/{odph_tcp.h => odp/helper/tcp.h} (95%) >> rename helper/include/{odph_udp.h => odp/helper/udp.h} (96%) >> create mode 100644 include/odp.h >> create mode 100644 include/odp/api/align.h >> create mode 100644 include/odp/api/atomic.h >> rename platform/linux-generic/include/api/odp_barrier.h => >> include/odp/api/barrier.h (95%) >> rename platform/linux-generic/include/api/odp_buffer.h => >> include/odp/api/buffer.h (88%) >> rename platform/linux-generic/include/api/odp_buffer_pool.h => >> include/odp/api/buffer_pool.h (97%) >> create mode 100644 include/odp/api/byteorder.h >> rename platform/linux-generic/include/api/odp_classification.h => >> include/odp/api/classification.h (99%) >> rename platform/linux-generic/include/api/odp_compiler.h => >> include/odp/api/compiler.h (100%) >> rename platform/linux-generic/include/api/odp_config.h => >> include/odp/api/config.h (100%) >> rename platform/linux-generic/include/api/odp_coremask.h => >> include/odp/api/coremask.h (74%) >> rename platform/linux-generic/include/api/odp_crypto.h => >> include/odp/api/crypto.h (79%) >> rename platform/linux-generic/include/api/odp_debug.h => >> include/odp/api/debug.h (95%) >> rename platform/linux-generic/include/api/odp_hints.h => >> include/odp/api/hints.h (100%) >> rename platform/linux-generic/include/api/odp_init.h => >> include/odp/api/init.h (99%) >> rename platform/linux-generic/include/api/odp_packet.h => >> include/odp/api/packet.h (98%) >> rename platform/linux-generic/include/api/odp_packet_flags.h => >> include/odp/api/packet_flags.h (99%) >> rename platform/linux-generic/include/api/odp_packet_io.h => >> include/odp/api/packet_io.h (95%) >> rename platform/linux-generic/include/api/odp_queue.h => >> include/odp/api/queue.h (75%) >> rename platform/linux-generic/include/api/odp_rwlock.h => >> include/odp/api/rwlock.h (98%) >> rename platform/linux-generic/include/api/odp_schedule.h => >> include/odp/api/schedule.h (94%) >> rename platform/linux-generic/include/api/odp_shared_memory.h => >> include/odp/api/shared_memory.h (93%) >> rename platform/linux-generic/include/api/odp_spinlock.h => >> include/odp/api/spinlock.h (97%) >> create mode 100644 include/odp/api/std_types.h >> create mode 100644 include/odp/api/sync.h >> rename platform/linux-generic/include/api/odp_system_info.h => >> include/odp/api/system_info.h (97%) >> rename platform/linux-generic/include/api/odp_thread.h => >> include/odp/api/thread.h (100%) >> rename platform/linux-generic/include/api/odp_ticketlock.h => >> include/odp/api/ticketlock.h (97%) >> rename platform/linux-generic/include/api/odp_time.h => >> include/odp/api/time.h (97%) >> rename platform/linux-generic/include/api/odp_timer.h => >> include/odp/api/timer.h (98%) >> rename platform/linux-generic/include/api/odp_version.h => >> include/odp/api/version.h (100%) >> delete mode 100644 platform/linux-generic/include/api/odp.h >> rename platform/linux-generic/include/api/{odp_align.h => odp/align.h} >> (58%) >> create mode 100644 platform/linux-generic/include/api/odp/atomic.h >> create mode 100644 platform/linux-generic/include/api/odp/barrier.h >> create mode 100644 platform/linux-generic/include/api/odp/buffer.h >> create mode 100644 platform/linux-generic/include/api/odp/buffer_pool.h >> create mode 100644 platform/linux-generic/include/api/odp/byteorder.h >> create mode 100644 >> platform/linux-generic/include/api/odp/classification.h >> create mode 100644 platform/linux-generic/include/api/odp/compiler.h >> create mode 100644 platform/linux-generic/include/api/odp/config.h >> create mode 100644 platform/linux-generic/include/api/odp/coremask.h >> create mode 100644 platform/linux-generic/include/api/odp/crypto.h >> create mode 100644 platform/linux-generic/include/api/odp/debug.h >> create mode 100644 platform/linux-generic/include/api/odp/hints.h >> create mode 100644 platform/linux-generic/include/api/odp/init.h >> create mode 100644 platform/linux-generic/include/api/odp/packet.h >> create mode 100644 platform/linux-generic/include/api/odp/packet_flags.h >> create mode 100644 platform/linux-generic/include/api/odp/packet_io.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/atomic_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/buffer_pool_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/buffer_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/byteorder_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/coremask_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/crypto_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/packet_io_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/packet_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/queue_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/schedule_types.h >> create mode 100644 >> platform/linux-generic/include/api/odp/plat/shared_memory_types.h >> create mode 100644 platform/linux-generic/include/api/odp/queue.h >> create mode 100644 platform/linux-generic/include/api/odp/rwlock.h >> create mode 100644 platform/linux-generic/include/api/odp/schedule.h >> create mode 100644 platform/linux-generic/include/api/odp/shared_memory.h >> create mode 100644 platform/linux-generic/include/api/odp/spinlock.h >> rename platform/linux-generic/include/api/{odp_std_types.h => >> odp/std_types.h} (57%) >> rename platform/linux-generic/include/api/{odp_sync.h => odp/sync.h} >> (69%) >> create mode 100644 platform/linux-generic/include/api/odp/system_info.h >> create mode 100644 platform/linux-generic/include/api/odp/thread.h >> create mode 100644 platform/linux-generic/include/api/odp/ticketlock.h >> create mode 100644 platform/linux-generic/include/api/odp/time.h >> create mode 100644 platform/linux-generic/include/api/odp/timer.h >> create mode 100644 platform/linux-generic/include/api/odp/version.h >> delete mode 100644 platform/linux-generic/include/api/odp_atomic.h >> delete mode 100644 platform/linux-generic/include/api/odp_byteorder.h >> delete mode 100644 >> platform/linux-generic/include/api/odp_platform_types.h >> >> _______________________________________________ >> lng-odp mailing list >> [email protected] >> http://lists.linaro.org/mailman/listinfo/lng-odp > > > > _______________________________________________ > lng-odp mailing list > [email protected] > http://lists.linaro.org/mailman/listinfo/lng-odp > _______________________________________________ lng-odp mailing list [email protected] http://lists.linaro.org/mailman/listinfo/lng-odp
