On Mon, Dec 14, 2015 at 6:40 AM, Maxim Uvarov <maxim.uva...@linaro.org> wrote:
> On 12/14/2015 14:39, Bill Fischofer wrote: > >> >> >> On Mon, Dec 14, 2015 at 3:24 AM, Maxim Uvarov <maxim.uva...@linaro.org >> <mailto:maxim.uva...@linaro.org>> wrote: >> >> On 12/11/2015 16:22, Bill Fischofer wrote: >> >> The linux-generic implementation of odp_time_t makes use of POSIX >> APIs that are sensitive to the _POSIX_C_SOURCE level. Use an >> indirection >> mechanism so that these dependencies do not "bleed through" >> the ODP API. >> This means that ODP applications can be independent of >> _POSIX_C_SOURCE >> level. >> >> Signed-off-by: Bill Fischofer <bill.fischo...@linaro.org >> <mailto:bill.fischo...@linaro.org>> >> --- >> .../linux-generic/include/odp/plat/time_types.h | 9 ++++--- >> platform/linux-generic/odp_time.c | 30 >> +++++++++++----------- >> 2 files changed, 20 insertions(+), 19 deletions(-) >> >> diff --git >> a/platform/linux-generic/include/odp/plat/time_types.h >> b/platform/linux-generic/include/odp/plat/time_types.h >> index e5765ec..e999beb 100644 >> --- a/platform/linux-generic/include/odp/plat/time_types.h >> +++ b/platform/linux-generic/include/odp/plat/time_types.h >> @@ -21,11 +21,12 @@ extern "C" { >> * @{ >> **/ >> -typedef struct timespec odp_time_t; >> +typedef struct { >> + int64_t tv_sec; >> + int64_t tv_nsec; >> +} odp_time_t; >> >> >> Looks like that struct should be moved to ./include/odp/api/time.h >> Overwise apps will relay on different time structures which >> platforms can redifine. >> >> >> odp_time_t, like other ODP types, expects a platform-dependent >> definition. There's no need to define the type as part of the ODP time >> APIs. I'll add this topic to today's arch call discussions. >> > > In that case you need to add accessors and getters to time fields. I.e. > apps can not directly relay on struct members. > odp_time_sec(odp_time_t time) > odp_time_sec_set(odp_time_t time) > odp_time_nsec(odp_time_t time) > odp_time_nsec_set(odp_time_t time) > > which looks ugly for me. It's better to define some common struct which > apps can use directly. > That might not be a bad extension, but they are not currently part of the API. The only manipulations on odp_time_t currently defined are the ability to convert an odp_time_t to and from nanoseconds, the ability to get the current time, and the ability to add, subtract, and compare two odp_time_t values. > > Maxim. > > >> Also doxygen notes are missing: >> /opt/Linaro/check-odp.git/build/odp-apply/include/odp/api/time.h:32: >> warning: documented symbol `odp_time_t' was not declared or defined. >> >> /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:25: >> warning: Member tv_sec (variable) of class odp_time_t is not >> documented. >> >> /opt/Linaro/check-odp.git/build/odp-apply/platform/linux-generic/include/odp/plat/time_types.h:26: >> warning: Member tv_nsec (variable) of class odp_time_t is not >> documented. >> >> Maxim. >> >> >> -odp_time_t odp_time_null(void); >> - >> -#define ODP_TIME_NULL odp_time_null() >> +#define ODP_TIME_NULL ((odp_time_t){0, 0}) >> /** >> * @} >> diff --git a/platform/linux-generic/odp_time.c >> b/platform/linux-generic/odp_time.c >> index 1c7c214..9b64b37 100644 >> --- a/platform/linux-generic/odp_time.c >> +++ b/platform/linux-generic/odp_time.c >> @@ -11,7 +11,12 @@ >> #include <odp/hints.h> >> #include <odp_debug_internal.h> >> -static struct timespec start_time; >> +typedef union { >> + odp_time_t ex; >> + struct timespec in; >> +} _odp_time_t; >> + >> +static odp_time_t start_time; >> static inline >> uint64_t time_to_ns(odp_time_t time) >> @@ -27,7 +32,7 @@ uint64_t time_to_ns(odp_time_t time) >> static inline >> odp_time_t time_diff(odp_time_t t2, odp_time_t t1) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec - t1.tv_sec; >> time.tv_nsec = t2.tv_nsec - t1.tv_nsec; >> @@ -43,13 +48,13 @@ odp_time_t time_diff(odp_time_t t2, >> odp_time_t t1) >> odp_time_t odp_time_local(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in >> <http://time.in>); >> >> if (odp_unlikely(ret != 0)) >> ODP_ABORT("clock_gettime failed\n"); >> - return time_diff(time, start_time); >> + return time_diff(time.ex, start_time); >> } >> odp_time_t odp_time_diff(odp_time_t t2, odp_time_t t1) >> @@ -64,7 +69,7 @@ uint64_t odp_time_to_ns(odp_time_t time) >> odp_time_t odp_time_local_from_ns(uint64_t ns) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = ns / ODP_TIME_SEC_IN_NS; >> time.tv_nsec = ns - time.tv_sec * ODP_TIME_SEC_IN_NS; >> @@ -85,7 +90,7 @@ int odp_time_cmp(odp_time_t t2, odp_time_t t1) >> odp_time_t odp_time_sum(odp_time_t t1, odp_time_t t2) >> { >> - struct timespec time; >> + odp_time_t time; >> time.tv_sec = t2.tv_sec + t1.tv_sec; >> time.tv_nsec = t2.tv_nsec + t1.tv_nsec; >> @@ -113,18 +118,13 @@ uint64_t odp_time_to_u64(odp_time_t time) >> return time_to_ns(time) / resolution; >> } >> -odp_time_t odp_time_null(void) >> -{ >> - return (struct timespec) {0, 0}; >> -} >> - >> int odp_time_global_init(void) >> { >> int ret; >> - struct timespec time; >> + _odp_time_t time; >> - ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time); >> - start_time = ret ? odp_time_null() : time; >> + ret = clock_gettime(CLOCK_MONOTONIC_RAW, &time.in >> <http://time.in>); >> + start_time = ret ? ODP_TIME_NULL : time.ex; >> return ret; >> } >> >> >> _______________________________________________ >> lng-odp mailing list >> lng-odp@lists.linaro.org <mailto:lng-odp@lists.linaro.org> >> https://lists.linaro.org/mailman/listinfo/lng-odp >> >> >> >
_______________________________________________ lng-odp mailing list lng-odp@lists.linaro.org https://lists.linaro.org/mailman/listinfo/lng-odp
