[v2] remove helper dependence on ODP internals

Remove the helpers dependence on internal implementation details,
implementations should only have to implement the public API.

Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
---

v2
Put string on one line

Other checkpatch warnings accepted by author
checkpatch macro handling bug
accepted line length for readability
kernel specific warning on ARCH

 helper/linux.c                     | 32 ++++++-------
 helper/odph_debug.h                | 93 ++++++++++++++++++++++++++++++++++++++
 helper/odph_pause.h                | 54 ++++++++++++++++++++++
 helper/ring.c                      | 42 ++++++++---------
 platform/linux-generic/Makefile.am |  4 +-
 5 files changed, 186 insertions(+), 39 deletions(-)
 create mode 100644 helper/odph_debug.h
 create mode 100644 helper/odph_pause.h

Merged,

Thanks,
Maxim.

On 07/01/15 19:51, Mike Holmes wrote:
> Remove the helpers dependence on internal implementation details,
> implementations should only have to implement the public API.
>
> Signed-off-by: Mike Holmes <mike.holmes@linaro.org>
> ---
>
> v2
> Put string on one line
>
> Other checkpatch warnings accepted by author
> checkpatch macro handling bug
> accepted line length for readability
> kernel specific warning on ARCH
>
>   helper/linux.c                     | 32 ++++++-------
>   helper/odph_debug.h                | 93 ++++++++++++++++++++++++++++++++++++++
>   helper/odph_pause.h                | 54 ++++++++++++++++++++++
>   helper/ring.c                      | 42 ++++++++---------
>   platform/linux-generic/Makefile.am |  4 +-
>   5 files changed, 186 insertions(+), 39 deletions(-)
>   create mode 100644 helper/odph_debug.h
>   create mode 100644 helper/odph_pause.h
>
> diff --git a/helper/linux.c b/helper/linux.c
> index be1ea13..3659be0 100644
> --- a/helper/linux.c
> +++ b/helper/linux.c
> @@ -17,12 +17,10 @@
>   #include <stdio.h>
>   
>   #include <odp/helper/linux.h>
> -#include <odp_internal.h>
>   #include <odp/thread.h>
>   #include <odp/init.h>
>   #include <odp/system_info.h>
> -#include <odp_debug_internal.h>
> -
> +#include "odph_debug.h"
>   
>   int odph_linux_cpumask_default(odp_cpumask_t *mask, int num)
>   {
> @@ -32,7 +30,7 @@ int odph_linux_cpumask_default(odp_cpumask_t *mask, int num)
>   	ret = pthread_getaffinity_np(pthread_self(),
>   				     sizeof(cpu_set_t), &cpuset);
>   	if (ret != 0)
> -		ODP_ABORT("failed to read CPU affinity value\n");
> +		ODPH_ABORT("failed to read CPU affinity value\n");
>   
>   	odp_cpumask_zero(mask);
>   
> @@ -61,16 +59,16 @@ static void *odp_run_start_routine(void *arg)
>   
>   	/* ODP thread local init */
>   	if (odp_init_local()) {
> -		ODP_ERR("Local init failed\n");
> +		ODPH_ERR("Local init failed\n");
>   		return NULL;
>   	}
>   
>   	void *ret_ptr = start_args->start_routine(start_args->arg);
>   	int ret = odp_term_local();
>   	if (ret < 0)
> -		ODP_ERR("Local term failed\n");
> +		ODPH_ERR("Local term failed\n");
>   	else if (ret == 0 && odp_term_global())
> -		ODP_ERR("Global term failed\n");
> +		ODPH_ERR("Global term failed\n");
>   
>   	return ret_ptr;
>   }
> @@ -95,8 +93,8 @@ int odph_linux_pthread_create(odph_linux_pthread_t *thread_tbl,
>   	cpu_count = odp_cpu_count();
>   
>   	if (num < 1 || num > cpu_count) {
> -		ODP_ERR("Invalid number of threads: %d (%d cores available)\n",
> -			num, cpu_count);
> +		ODPH_ERR("Invalid number of threads: %d (%d cores available)\n",
> +			 num, cpu_count);
>   		return 0;
>   	}
>   
> @@ -116,7 +114,7 @@ int odph_linux_pthread_create(odph_linux_pthread_t *thread_tbl,
>   
>   		thread_tbl[i].start_args = malloc(sizeof(odp_start_args_t));
>   		if (thread_tbl[i].start_args == NULL)
> -			ODP_ABORT("Malloc failed");
> +			ODPH_ABORT("Malloc failed");
>   
>   		thread_tbl[i].start_args->start_routine = start_routine;
>   		thread_tbl[i].start_args->arg           = arg;
> @@ -124,7 +122,7 @@ int odph_linux_pthread_create(odph_linux_pthread_t *thread_tbl,
>   		ret = pthread_create(&thread_tbl[i].thread, &thread_tbl[i].attr,
>   			       odp_run_start_routine, thread_tbl[i].start_args);
>   		if (ret != 0) {
> -			ODP_ERR("Failed to start thread on cpu #%d\n", cpu);
> +			ODPH_ERR("Failed to start thread on cpu #%d\n", cpu);
>   			free(thread_tbl[i].start_args);
>   			break;
>   		}
> @@ -167,7 +165,7 @@ int odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>   	cpu_count = odp_cpu_count();
>   
>   	if (num < 1 || num > cpu_count) {
> -		ODP_ERR("Bad num\n");
> +		ODPH_ERR("Bad num\n");
>   		return -1;
>   	}
>   
> @@ -181,7 +179,7 @@ int odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>   		pid = fork();
>   
>   		if (pid < 0) {
> -			ODP_ERR("fork() failed\n");
> +			ODPH_ERR("fork() failed\n");
>   			return -1;
>   		}
>   
> @@ -196,12 +194,12 @@ int odph_linux_process_fork_n(odph_linux_process_t *proc_tbl,
>   
>   		/* Child process */
>   		if (sched_setaffinity(0, sizeof(cpu_set_t), &proc_mask.set)) {
> -			ODP_ERR("sched_setaffinity() failed\n");
> +			ODPH_ERR("sched_setaffinity() failed\n");
>   			return -2;
>   		}
>   
>   		if (odp_init_local()) {
> -			ODP_ERR("Local init failed\n");
> +			ODPH_ERR("Local init failed\n");
>   			return -2;
>   		}
>   
> @@ -232,7 +230,7 @@ int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
>   		pid = wait(&status);
>   
>   		if (pid < 0) {
> -			ODP_ERR("wait() failed\n");
> +			ODPH_ERR("wait() failed\n");
>   			return -1;
>   		}
>   
> @@ -244,7 +242,7 @@ int odph_linux_process_wait_n(odph_linux_process_t *proc_tbl, int num)
>   		}
>   
>   		if (j == num) {
> -			ODP_ERR("Bad pid\n");
> +			ODPH_ERR("Bad pid\n");
>   			return -1;
>   		}
>   	}
> diff --git a/helper/odph_debug.h b/helper/odph_debug.h
> new file mode 100644
> index 0000000..9c216cd
> --- /dev/null
> +++ b/helper/odph_debug.h
> @@ -0,0 +1,93 @@
> +/* Copyright (c) 2015, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +/**
> + * @file
> + *
> + * HELPER debug
> + */
> +
> +#ifndef HELPER_DEBUG_H_
> +#define HELPER_DEBUG_H_
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#ifndef ODPH_DEBUG_PRINT
> +#define ODPH_DEBUG_PRINT 1
> +#endif
> +
> +/**
> + * log level.
> + */
> +typedef enum HELPER_log_level {
> +	ODPH_LOG_DBG,
> +	ODPH_LOG_ERR,
> +	ODPH_LOG_ABORT
> +} HELPER_log_level_e;
> +
> +/**
> + * default LOG macro.
> + */
> +#define ODPH_LOG(level, fmt, ...) \
> +do { \
> +	switch (level) { \
> +	case ODPH_LOG_ERR: \
> +		fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
> +		__LINE__, __func__, ##__VA_ARGS__); \
> +		break; \
> +	case ODPH_LOG_DBG: \
> +		if (ODPH_DEBUG_PRINT == 1) \
> +			fprintf(stderr, "%s:%d:%s():" fmt, __FILE__, \
> +			__LINE__, __func__, ##__VA_ARGS__); \
> +		break; \
> +	case ODPH_LOG_ABORT: \
> +		fprintf(stderr, "%s:%d:%s(): " fmt, __FILE__, \
> +		__LINE__, __func__, ##__VA_ARGS__); \
> +		abort(); \
> +		break; \
> +	default: \
> +		fprintf(stderr, "Unknown LOG level"); \
> +		break;\
> +	} \
> +} while (0)
> +
> +/**
> + * Debug printing macro, which prints output when DEBUG flag is set.
> + */
> +#define ODPH_DBG(fmt, ...) \
> +		ODPH_LOG(ODPH_LOG_DBG, fmt, ##__VA_ARGS__)
> +
> +/**
> + * Print output to stderr (file, line and function).
> + */
> +#define ODPH_ERR(fmt, ...) \
> +		ODPH_LOG(ODPH_LOG_ERR, fmt, ##__VA_ARGS__)
> +
> +/**
> + * Print output to stderr (file, line and function),
> + * then abort.
> + */
> +#define ODPH_ABORT(fmt, ...) \
> +		ODPH_LOG(ODPH_LOG_ABORT, fmt, ##__VA_ARGS__)
> +
> +/**
> + * @}
> + */
> +
> +/**
> + * Mark intentionally unused argument for functions
> + */
> +#define ODPH_UNUSED     __attribute__((__unused__))
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/helper/odph_pause.h b/helper/odph_pause.h
> new file mode 100644
> index 0000000..5618f1f
> --- /dev/null
> +++ b/helper/odph_pause.h
> @@ -0,0 +1,54 @@
> +/* Copyright (c) 2015, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier:     BSD-3-Clause
> + */
> +
> +#ifndef ODPH_PAUSE_H_
> +#define ODPH_PAUSE_H_
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +/**
> + * Spin loop for helper internal use
> + */
> +static inline void odph_pause(void)
> +{
> +#if defined __x86_64__ || defined __i386__
> +
> +#ifdef __SSE2__
> +	__asm__ __volatile__ ("pause");
> +#else
> +	__asm__ __volatile__ ("rep; nop");
> +#endif
> +
> +#elif defined __arm__
> +
> +#if __ARM_ARCH == 7
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +#endif
> +
> +#elif defined __OCTEON__
> +
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +	__asm__ __volatile__ ("nop");
> +
> +#endif
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif
> diff --git a/helper/ring.c b/helper/ring.c
> index 67fece9..3122173 100644
> --- a/helper/ring.c
> +++ b/helper/ring.c
> @@ -70,20 +70,20 @@
>    ***************************************************************************/
>   
>   #include <odp/shared_memory.h>
> -#include <odp_internal.h>
> -#include <odp_spin_internal.h>
> -#include <odp_align_internal.h>
>   #include <odp/spinlock.h>
> +#include "odph_pause.h"
>   #include <odp/align.h>
>   #include <fcntl.h>
>   #include <stdio.h>
>   #include <string.h>
> -#include <odp_debug_internal.h>
> +#include "odph_debug.h"
>   #include <odp/rwlock.h>
>   #include <odp/helper/ring.h>
>   
>   static TAILQ_HEAD(, odph_ring) odp_ring_list;
>   
> +#define RING_VAL_IS_POWER_2(x) ((((x) - 1) & (x)) == 0)
> +
>   /*
>    * the enqueue of pointers on the ring.
>    */
> @@ -161,9 +161,9 @@ odph_ring_create(const char *name, unsigned count, unsigned flags)
>   	odp_shm_t shm;
>   
>   	/* count must be a power of 2 */
> -	if (!ODP_VAL_IS_POWER_2(count) || (count > ODPH_RING_SZ_MASK)) {
> -		ODP_ERR("Requested size is invalid, must be power of 2, and  do not exceed the size limit %u\n",
> -			ODPH_RING_SZ_MASK);
> +	if (!RING_VAL_IS_POWER_2(count) || (count > ODPH_RING_SZ_MASK)) {
> +		ODPH_ERR("Requested size is invalid, must be power of 2, and do not exceed the size limit %u\n",
> +			 ODPH_RING_SZ_MASK);
>   		return NULL;
>   	}
>   
> @@ -194,7 +194,7 @@ odph_ring_create(const char *name, unsigned count, unsigned flags)
>   
>   		TAILQ_INSERT_TAIL(&odp_ring_list, r, next);
>   	} else {
> -		ODP_ERR("Cannot reserve memory\n");
> +		ODPH_ERR("Cannot reserve memory\n");
>   	}
>   
>   	odp_rwlock_write_unlock(&qlock);
> @@ -283,7 +283,7 @@ int __odph_ring_mp_do_enqueue(odph_ring_t *r, void * const *obj_table,
>   	 * we need to wait for them to complete
>   	 */
>   	while (odp_unlikely(r->prod.tail != prod_head))
> -		odp_spin();
> +		odph_pause();
>   
>   	/* Release our entries and the memory they refer to */
>   	__atomic_thread_fence(__ATOMIC_RELEASE);
> @@ -400,7 +400,7 @@ int __odph_ring_mc_do_dequeue(odph_ring_t *r, void **obj_table,
>   	 * we need to wait for them to complete
>   	 */
>   	while (odp_unlikely(r->cons.tail != cons_head))
> -		odp_spin();
> +		odph_pause();
>   
>   	/* Release our entries and the memory they refer to */
>   	__atomic_thread_fence(__ATOMIC_RELEASE);
> @@ -532,19 +532,19 @@ unsigned odph_ring_free_count(const odph_ring_t *r)
>   /* dump the status of the ring on the console */
>   void odph_ring_dump(const odph_ring_t *r)
>   {
> -	ODP_DBG("ring <%s>@%p\n", r->name, r);
> -	ODP_DBG("  flags=%x\n", r->flags);
> -	ODP_DBG("  size=%"PRIu32"\n", r->prod.size);
> -	ODP_DBG("  ct=%"PRIu32"\n", r->cons.tail);
> -	ODP_DBG("  ch=%"PRIu32"\n", r->cons.head);
> -	ODP_DBG("  pt=%"PRIu32"\n", r->prod.tail);
> -	ODP_DBG("  ph=%"PRIu32"\n", r->prod.head);
> -	ODP_DBG("  used=%u\n", odph_ring_count(r));
> -	ODP_DBG("  avail=%u\n", odph_ring_free_count(r));
> +	ODPH_DBG("ring <%s>@%p\n", r->name, r);
> +	ODPH_DBG("  flags=%x\n", r->flags);
> +	ODPH_DBG("  size=%" PRIu32 "\n", r->prod.size);
> +	ODPH_DBG("  ct=%" PRIu32 "\n", r->cons.tail);
> +	ODPH_DBG("  ch=%" PRIu32 "\n", r->cons.head);
> +	ODPH_DBG("  pt=%" PRIu32 "\n", r->prod.tail);
> +	ODPH_DBG("  ph=%" PRIu32 "\n", r->prod.head);
> +	ODPH_DBG("  used=%u\n", odph_ring_count(r));
> +	ODPH_DBG("  avail=%u\n", odph_ring_free_count(r));
>   	if (r->prod.watermark == r->prod.size)
> -		ODP_DBG("  watermark=0\n");
> +		ODPH_DBG("  watermark=0\n");
>   	else
> -		ODP_DBG("  watermark=%"PRIu32"\n", r->prod.watermark);
> +		ODPH_DBG("  watermark=%" PRIu32 "\n", r->prod.watermark);
>   }
>   
>   /* dump the status of all rings on the console */
> diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am
> index b8f93c7..4f2063f 100644
> --- a/platform/linux-generic/Makefile.am
> +++ b/platform/linux-generic/Makefile.am
> @@ -137,7 +137,9 @@ subdirheaders_HEADERS = \
>   			$(top_srcdir)/helper/include/odp/helper/linux.h \
>   			$(top_srcdir)/helper/include/odp/helper/ring.h \
>   			$(top_srcdir)/helper/include/odp/helper/tcp.h \
> -			$(top_srcdir)/helper/include/odp/helper/udp.h
> +			$(top_srcdir)/helper/include/odp/helper/udp.h \
> +			$(top_srcdir)/helper/odph_debug.h \
> +			$(top_srcdir)/helper/odph_pause.h
>   
>   __LIB__libodp_la_SOURCES = \
>   			   odp_barrier.c \