On 04/28/17 22:09, Bill Fischofer wrote:
> 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?
>
I think odp_x_to_u64() makes sense only for handles which somehow
related to odp event. I.e. it's reasonable to have it for timer, but not
for time. odp_time_t is more data structure then handle of something.
Maxim.
> On Fri, Apr 28, 2017 at 7:09 AM, Petri Savolainen <
> petri.savolai...@linaro.org> 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 <petri.savolai...@linaro.org>
>> ---
>> 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
>>
>>
