Aside from the deprecation question, for this series:
Reviewed-and-tested-by: Bill Fischofer <bill.fischo...@linaro.org>
Also, Parts 9 and 11 are fine but both cause checkpatch to hiccup:
bill@Ubuntu64:~/linaro/hubodp$ ./scripts/checkpatch.pl
0009-linux-gen-time-use-hw-time-counter-when-available.patch
Unescaped left brace in regex is deprecated, passed through in regex;
marked by <-- HERE in m/\((?^x:
(?^x:
(?:(?^:(?:(?^x:
const|
__percpu|
__nocast|
__safe|
__bitwise__|
__packed__|
__packed2__|
__naked|
__maybe_unused|
__always_unused|
__noreturn|
__used|
__cold|
__pure|
__noclone|
__deprecated|
__read_mostly|
__kprobes|
(?^:(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initdata\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initconst\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:init\b)))|
____cacheline_aligned|
____cacheline_aligned_in_smp|
____cacheline_internodealigned_in_smp|
__weak
)|(?^x:
__user|
__kernel|
__force|
__iomem|
__must_check|
__init_refok|
__kprobes|
__ref|
__rcu
)|(?x:
(?^:fastcall)
)))\s+|const\s+)*
(?:
(?:typeof|__typeof__)\s*\([^\)]*\)|
(?:(?^:(?x:
u_(?:char|short|int|long) | # bsd
u(?:nchar|short|int|long) # sysv
))\b)|
(?:(?^:(?x:
(?:__)?(?:u|s|be|le)(?:8|16|32|64)|
atomic_t
))\b)|
(?:(?x:
(?^:void)|
(?^:(?:(?:un)?signed\s+)?char)|
(?^:(?:(?:un)?signed\s+)?short\s+int)|
(?^:(?:(?:un)?signed\s+)?short)|
(?^:(?:(?:un)?signed\s+)?int)|
(?^:(?:(?:un)?signed\s+)?long\s+int)|
(?^:(?:(?:un)?signed\s+)?long\s+long\s+int)|
(?^:(?:(?:un)?signed\s+)?long\s+long)|
(?^:(?:(?:un)?signed\s+)?long)|
(?^:(?:un)?signed)|
(?^:float)|
(?^:double)|
(?^:bool)|
(?^:struct\s+(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
))|
(?^:union\s+(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
))|
(?^:enum\s+(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
))|
(?^:(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)_t)|
(?^:(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)_handler)|
(?^:(?^x:
[A-Za-z_][A-Za-z\d_]*
(?:\s*\#\#\s*[A-Za-z_][A-Za-z\d_]*)*
)_handler_fn)|
(?^:char\s+(?:un)?signed)|
(?^:int\s+(?:(?:un)?signed\s+)?short\s)|
(?^:int\s+short(?:\s+(?:un)?signed))|
(?^:short\s+int(?:\s+(?:un)?signed))|
(?^:(?:un)?signed\s+int\s+short)|
(?^:short\s+(?:un)?signed)|
(?^:long\s+int\s+(?:un)?signed)|
(?^:int\s+long\s+(?:un)?signed)|
(?^:long\s+(?:un)?signed\s+int)|
(?^:int\s+(?:un)?signed\s+long)|
(?^:int\s+(?:un)?signed)|
(?^:int\s+long\s+long\s+(?:un)?signed)|
(?^:long\s+long\s+int\s+(?:un)?signed)|
(?^:long\s+long\s+(?:un)?signed\s+int)|
(?^:long\s+long\s+(?:un)?signed)|
(?^:long\s+(?:un)?signed)
)\b)
)
(?:\s+(?^:(?:(?^x:
const|
__percpu|
__nocast|
__safe|
__bitwise__|
__packed__|
__packed2__|
__naked|
__maybe_unused|
__always_unused|
__noreturn|
__used|
__cold|
__pure|
__noclone|
__deprecated|
__read_mostly|
__kprobes|
(?^:(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initdata\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initconst\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:init\b)))|
____cacheline_aligned|
____cacheline_aligned_in_smp|
____cacheline_internodealigned_in_smp|
__weak
)|(?^x:
__user|
__kernel|
__force|
__iomem|
__must_check|
__init_refok|
__kprobes|
__ref|
__rcu
)|(?x:
(?^:fastcall)
)))|\s+const)*
)
(?:(?:\s|\*|\[\])+\s*const|(?:\s|\*\s*(?:const\s*)?|\[\])+|(?:\s*\[\s*\])+)?
(?:\s+(?^:inline|__always_inline|noinline|__inline|__inline__)|\s+(?^:(?:(?^x:
const|
__percpu|
__nocast|
__safe|
__bitwise__|
__packed__|
__packed2__|
__naked|
__maybe_unused|
__always_unused|
__noreturn|
__used|
__cold|
__pure|
__noclone|
__deprecated|
__read_mostly|
__kprobes|
(?^:(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initdata\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:initconst\b))|(?^:(?^:__(?:mem|cpu|dev|net_|))(?:init\b)))|
____cacheline_aligned|
____cacheline_aligned_in_smp|
____cacheline_internodealigned_in_smp|
__weak
)|(?^x:
__user|
__kernel|
__force|
__iomem|
__must_check|
__init_refok|
__kprobes|
__ref|
__rcu
)|(?x:
(?^:fastcall)
))))*
)\){ <-- HERE / at ./scripts/checkpatch.pl line 3926.
total: 0 errors, 0 warnings, 0 checks, 645 lines checked
NOTE: Ignored message types: BIT_MACRO COMPARISON_TO_NULL
DEPRECATED_VARIABLE NEW_TYPEDEFS SPLIT_STRING SSCANF_TO_KSTRTO
0009-linux-gen-time-use-hw-time-counter-when-available.patch has no obvious
style problems and is ready for submission.
bill@Ubuntu64:~/linaro/hubodp$
On Fri, Apr 28, 2017 at 2:28 PM, Maxim Uvarov <maxim.uva...@linaro.org>
wrote:
> 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
> >>
> >>
>
>
