I agree that the validation test for odp_time_to_u64() is suspect and
should be fixed, but why remove this API? We've established that various
abstract types have odp_xxx_to_u64() routines designed for debugging use
and this makes odp_time_t an exception to that rule.

Also, shouldn't we deprecate it if we want to remove it? What's the point
of having the deprecation framework in place if we're not going to bother
with it?

On Fri, Apr 28, 2017 at 7:09 AM, Petri Savolainen <
[email protected]> wrote:

> Debug function that converts odp_time_t to u64 is unnecessary
> since odp_time_to_ns() returns time as a u64 (nsec) value.
> Application can always use that as the 64 bit representation
> of an odp_time_t value. Also validation tests for odp_time_to_u64()
> were erroneous since those compared returned u64 values and
> expected greater/lesser than relation.
>
> Signed-off-by: Petri Savolainen <[email protected]>
> ---
>  include/odp/api/spec/time.h                 | 13 ----------
>  platform/linux-generic/odp_time.c           | 15 ------------
>  test/common_plat/validation/api/time/time.c | 37
> -----------------------------
>  test/common_plat/validation/api/time/time.h |  2 --
>  4 files changed, 67 deletions(-)
>
> diff --git a/include/odp/api/spec/time.h b/include/odp/api/spec/time.h
> index fcc94c98..29175eb5 100644
> --- a/include/odp/api/spec/time.h
> +++ b/include/odp/api/spec/time.h
> @@ -158,19 +158,6 @@ void odp_time_wait_until(odp_time_t time);
>  void odp_time_wait_ns(uint64_t ns);
>
>  /**
> - * Get printable value for an odp_time_t
> - *
> - * @param time time to be printed
> - *
> - * @return uint64_t value that can be used to print/display this time
> - *
> - * @note This routine is intended to be used for diagnostic purposes
> - * to enable applications to generate a printable value that represents
> - * an odp_time_t time.
> - */
> -uint64_t odp_time_to_u64(odp_time_t time);
> -
> -/**
>   * @}
>   */
>
> diff --git a/platform/linux-generic/odp_time.c
> b/platform/linux-generic/odp_time.c
> index 81e05224..0e5966c0 100644
> --- a/platform/linux-generic/odp_time.c
> +++ b/platform/linux-generic/odp_time.c
> @@ -176,21 +176,6 @@ void odp_time_wait_until(odp_time_t time)
>         return time_wait_until(time);
>  }
>
> -uint64_t odp_time_to_u64(odp_time_t time)
> -{
> -       int ret;
> -       struct timespec tres;
> -       uint64_t resolution;
> -
> -       ret = clock_getres(CLOCK_MONOTONIC_RAW, &tres);
> -       if (odp_unlikely(ret != 0))
> -               ODP_ABORT("clock_getres failed\n");
> -
> -       resolution = (uint64_t)tres.tv_nsec;
> -
> -       return time_to_ns(time) / resolution;
> -}
> -
>  int odp_time_init_global(void)
>  {
>         int ret;
> diff --git a/test/common_plat/validation/api/time/time.c
> b/test/common_plat/validation/api/time/time.c
> index 530d5c07..df65c719 100644
> --- a/test/common_plat/validation/api/time/time.c
> +++ b/test/common_plat/validation/api/time/time.c
> @@ -398,41 +398,6 @@ void time_test_wait_ns(void)
>         }
>  }
>
> -static void time_test_to_u64(time_cb time)
> -{
> -       volatile int count = 0;
> -       uint64_t val1, val2;
> -       odp_time_t t1, t2;
> -
> -       t1 = time();
> -
> -       val1 = odp_time_to_u64(t1);
> -       CU_ASSERT(val1 > 0);
> -
> -       while (count < BUSY_LOOP_CNT) {
> -               count++;
> -       };
> -
> -       t2 = time();
> -       val2 = odp_time_to_u64(t2);
> -       CU_ASSERT(val2 > 0);
> -
> -       CU_ASSERT(val2 > val1);
> -
> -       val1 = odp_time_to_u64(ODP_TIME_NULL);
> -       CU_ASSERT(val1 == 0);
> -}
> -
> -void time_test_local_to_u64(void)
> -{
> -       time_test_to_u64(odp_time_local);
> -}
> -
> -void time_test_global_to_u64(void)
> -{
> -       time_test_to_u64(odp_time_global);
> -}
> -
>  odp_testinfo_t time_suite_time[] = {
>         ODP_TEST_INFO(time_test_constants),
>         ODP_TEST_INFO(time_test_local_res),
> @@ -443,14 +408,12 @@ odp_testinfo_t time_suite_time[] = {
>         ODP_TEST_INFO(time_test_local_sum),
>         ODP_TEST_INFO(time_test_local_wait_until),
>         ODP_TEST_INFO(time_test_wait_ns),
> -       ODP_TEST_INFO(time_test_local_to_u64),
>         ODP_TEST_INFO(time_test_global_res),
>         ODP_TEST_INFO(time_test_global_conversion),
>         ODP_TEST_INFO(time_test_global_cmp),
>         ODP_TEST_INFO(time_test_global_diff),
>         ODP_TEST_INFO(time_test_global_sum),
>         ODP_TEST_INFO(time_test_global_wait_until),
> -       ODP_TEST_INFO(time_test_global_to_u64),
>         ODP_TEST_INFO_NULL
>  };
>
> diff --git a/test/common_plat/validation/api/time/time.h
> b/test/common_plat/validation/api/time/time.h
> index e5132a49..10956294 100644
> --- a/test/common_plat/validation/api/time/time.h
> +++ b/test/common_plat/validation/api/time/time.h
> @@ -24,8 +24,6 @@ void time_test_global_sum(void);
>  void time_test_local_wait_until(void);
>  void time_test_global_wait_until(void);
>  void time_test_wait_ns(void);
> -void time_test_local_to_u64(void);
> -void time_test_global_to_u64(void);
>  void time_test_monotony(void);
>
>  /* test arrays: */
> --
> 2.11.0
>
>

Reply via email to