diff --git a/drivers/timer/nrf_grtc_timer.c b/drivers/timer/nrf_grtc_timer.c index 609a32b406a..bec83e1a088 100644 --- a/drivers/timer/nrf_grtc_timer.c +++ b/drivers/timer/nrf_grtc_timer.c @@ -47,6 +47,8 @@ ((uint64_t)sys_clock_hw_cycles_per_sec() / (uint64_t)CONFIG_SYS_CLOCK_TICKS_PER_SEC) #define COUNTER_SPAN (GRTC_SYSCOUNTERL_VALUE_Msk | ((uint64_t)GRTC_SYSCOUNTERH_VALUE_Msk << 32)) +#define MAX_ABS_TICKS (COUNTER_SPAN / CYC_PER_TICK) + #define MAX_TICKS \ (((COUNTER_SPAN / CYC_PER_TICK) > INT_MAX) ? INT_MAX : (COUNTER_SPAN / CYC_PER_TICK)) @@ -289,31 +291,17 @@ void z_nrf_grtc_timer_abort(int32_t chan) uint64_t z_nrf_grtc_timer_get_ticks(k_timeout_t t) { - uint64_t curr_time; - int64_t curr_tick; - int64_t result; - int64_t abs_ticks; - int64_t grtc_ticks; - - curr_time = counter(); - curr_tick = sys_clock_tick_get(); - - grtc_ticks = t.ticks * CYC_PER_TICK; - abs_ticks = Z_TICK_ABS(t.ticks); - if (abs_ticks < 0) { - /* relative timeout */ - return (grtc_ticks > (int64_t)COUNTER_SPAN) ? - -EINVAL : (curr_time + grtc_ticks); - } + int64_t abs_ticks = Z_TICK_ABS(t.ticks); - /* absolute timeout */ - result = (abs_ticks - curr_tick) * CYC_PER_TICK; + if (Z_IS_TIMEOUT_RELATIVE(t)) { + int64_t grtc_ticks = t.ticks * CYC_PER_TICK; - if (result > (int64_t)COUNTER_SPAN) { - return -EINVAL; + return (grtc_ticks > (int64_t)COUNTER_SPAN) ? + -EINVAL : (counter() + grtc_ticks); } - return curr_time + result; + /* absolute timeout */ + return (abs_ticks > MAX_ABS_TICKS) ? -EINVAL : (abs_ticks * CYC_PER_TICK); } int z_nrf_grtc_timer_capture_prepare(int32_t chan) diff --git a/drivers/timer/nrf_rtc_timer.c b/drivers/timer/nrf_rtc_timer.c index d45d9229d31..0fdf951f925 100644 --- a/drivers/timer/nrf_rtc_timer.c +++ b/drivers/timer/nrf_rtc_timer.c @@ -208,8 +208,7 @@ uint64_t z_nrf_rtc_timer_get_ticks(k_timeout_t t) } while (curr_time != z_nrf_rtc_timer_read()); abs_ticks = Z_TICK_ABS(t.ticks); - if (abs_ticks < 0) { - /* relative timeout */ + if (Z_IS_TIMEOUT_RELATIVE(t)) { return (t.ticks > COUNTER_SPAN) ? -EINVAL : (curr_time + t.ticks); } diff --git a/include/zephyr/sys_clock.h b/include/zephyr/sys_clock.h index 36abcff9f87..8d24f0317eb 100644 --- a/include/zephyr/sys_clock.h +++ b/include/zephyr/sys_clock.h @@ -147,6 +147,13 @@ typedef struct { */ #define Z_TICK_ABS(t) (K_TICKS_FOREVER - 1 - (t)) +/* Test for relative timeout */ +#if CONFIG_TIMEOUT_64BIT +#define Z_IS_TIMEOUT_RELATIVE(timeout) (Z_TICK_ABS((timeout).ticks) < 0) +#else +#define Z_IS_TIMEOUT_RELATIVE(timeout) true +#endif + /* added tick needed to account for tick in progress */ #define _TICK_ALIGN 1 diff --git a/kernel/nothread.c b/kernel/nothread.c index 3e590e714b2..09bf4545961 100644 --- a/kernel/nothread.c +++ b/kernel/nothread.c @@ -36,7 +36,7 @@ int32_t z_impl_k_sleep(k_timeout_t timeout) } ticks = timeout.ticks; - if (Z_TICK_ABS(ticks) <= 0) { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { /* ticks is delta timeout */ ticks_to_wait = ticks; } else { diff --git a/kernel/sched.c b/kernel/sched.c index c126792bcbb..e0b4f35938c 100644 --- a/kernel/sched.c +++ b/kernel/sched.c @@ -1064,27 +1064,26 @@ static inline void z_vrfy_k_yield(void) #include #endif /* CONFIG_USERSPACE */ -static int32_t z_tick_sleep(k_ticks_t ticks) +static int32_t z_tick_sleep(k_timeout_t timeout) { uint32_t expected_wakeup_ticks; __ASSERT(!arch_is_in_isr(), ""); - LOG_DBG("thread %p for %lu ticks", _current, (unsigned long)ticks); + LOG_DBG("thread %p for %lu ticks", _current, (unsigned long)timeout.ticks); - /* wait of 0 ms is treated as a 'yield' */ - if (ticks == 0) { + /* K_NO_WAIT is treated as a 'yield' */ + if (K_TIMEOUT_EQ(timeout, K_NO_WAIT)) { k_yield(); return 0; } - if (Z_TICK_ABS(ticks) <= 0) { - expected_wakeup_ticks = ticks + sys_clock_tick_get_32(); + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { + expected_wakeup_ticks = timeout.ticks + sys_clock_tick_get_32(); } else { - expected_wakeup_ticks = Z_TICK_ABS(ticks); + expected_wakeup_ticks = Z_TICK_ABS(timeout.ticks); } - k_timeout_t timeout = Z_TIMEOUT_TICKS(ticks); k_spinlock_key_t key = k_spin_lock(&_sched_spinlock); #if defined(CONFIG_TIMESLICING) && defined(CONFIG_SWAP_NONATOMIC) @@ -1100,7 +1099,8 @@ static int32_t z_tick_sleep(k_ticks_t ticks) uint32_t left_ticks = expected_wakeup_ticks - sys_clock_tick_get_32(); /* To handle a negative value correctly, once type-cast it to signed 32 bit */ - ticks = (k_ticks_t)(int32_t)left_ticks; + k_ticks_t ticks = (k_ticks_t)(int32_t)left_ticks; + if (ticks > 0) { return ticks; } @@ -1116,9 +1116,7 @@ int32_t z_impl_k_sleep(k_timeout_t timeout) SYS_PORT_TRACING_FUNC_ENTER(k_thread, sleep, timeout); - ticks = timeout.ticks; - - ticks = z_tick_sleep(ticks); + ticks = z_tick_sleep(timeout); int32_t ret = K_TIMEOUT_EQ(timeout, K_FOREVER) ? K_TICKS_FOREVER : k_ticks_to_ms_ceil64(ticks); @@ -1143,7 +1141,7 @@ int32_t z_impl_k_usleep(int32_t us) SYS_PORT_TRACING_FUNC_ENTER(k_thread, usleep, us); ticks = k_us_to_ticks_ceil64(us); - ticks = z_tick_sleep(ticks); + ticks = z_tick_sleep(Z_TIMEOUT_TICKS(ticks)); int32_t ret = k_ticks_to_us_ceil64(ticks); diff --git a/kernel/timeout.c b/kernel/timeout.c index c18f2e41d06..4afe2fd5f10 100644 --- a/kernel/timeout.c +++ b/kernel/timeout.c @@ -113,13 +113,12 @@ void z_add_timeout(struct _timeout *to, _timeout_func_t fn, K_SPINLOCK(&timeout_lock) { struct _timeout *t; - if (IS_ENABLED(CONFIG_TIMEOUT_64BIT) && - (Z_TICK_ABS(timeout.ticks) >= 0)) { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { + to->dticks = timeout.ticks + 1 + elapsed(); + } else { k_ticks_t ticks = Z_TICK_ABS(timeout.ticks) - curr_tick; to->dticks = MAX(1, ticks); - } else { - to->dticks = timeout.ticks + 1 + elapsed(); } for (t = first(); t != NULL; t = next(t)) { @@ -306,10 +305,10 @@ k_timepoint_t sys_timepoint_calc(k_timeout_t timeout) } else { k_ticks_t dt = timeout.ticks; - if (IS_ENABLED(CONFIG_TIMEOUT_64BIT) && Z_TICK_ABS(dt) >= 0) { - timepoint.tick = Z_TICK_ABS(dt); - } else { + if (Z_IS_TIMEOUT_RELATIVE(timeout)) { timepoint.tick = sys_clock_tick_get() + MAX(1, dt); + } else { + timepoint.tick = Z_TICK_ABS(dt); } } diff --git a/kernel/timer.c b/kernel/timer.c index 466cab6c484..513d676bf0d 100644 --- a/kernel/timer.c +++ b/kernel/timer.c @@ -166,8 +166,13 @@ void z_impl_k_timer_start(struct k_timer *timer, k_timeout_t duration, * argument the same way k_sleep() does), but historical. The * timer_api test relies on this behavior. */ - if (Z_TICK_ABS(duration.ticks) < 0) { - duration.ticks = MAX(duration.ticks - 1, 0); + if (Z_IS_TIMEOUT_RELATIVE(duration)) { + /* For the duration == K_NO_WAIT case, ensure that behaviour + * is consistent for both 32-bit k_ticks_t which are unsigned + * and 64-bit k_ticks_t which are signed. + */ + duration.ticks = MAX(1, duration.ticks); + duration.ticks = duration.ticks - 1; } (void)z_abort_timeout(&timer->timeout); diff --git a/tests/drivers/timer/nrf_grtc_timer/src/main.c b/tests/drivers/timer/nrf_grtc_timer/src/main.c index 2fff9802d49..f53188841bd 100644 --- a/tests/drivers/timer/nrf_grtc_timer/src/main.c +++ b/tests/drivers/timer/nrf_grtc_timer/src/main.c @@ -40,18 +40,37 @@ ZTEST(nrf_grtc_timer, test_get_ticks) for (uint32_t i = 0; i < NUMBER_OF_TRIES; i++) { /* Absolute timeout 1ms in the past */ - t = Z_TIMEOUT_TICKS(Z_TICK_ABS(sys_clock_tick_get() - K_MSEC(1).ticks)); + uint64_t curr_tick; + uint64_t curr_grtc_tick; + uint64_t curr_tick2; - exp_ticks = z_nrf_grtc_timer_read() - K_MSEC(1).ticks * CYC_PER_TICK; + do { + /* GRTC and system tick must be read during single system tick. */ + curr_tick = sys_clock_tick_get(); + curr_grtc_tick = z_nrf_grtc_timer_read(); + curr_tick2 = sys_clock_tick_get(); + } while (curr_tick != curr_tick2); + + t = Z_TIMEOUT_TICKS(Z_TICK_ABS(curr_tick - K_MSEC(1).ticks)); + + exp_ticks = curr_grtc_tick - K_MSEC(1).ticks * CYC_PER_TICK; ticks = z_nrf_grtc_timer_get_ticks(t); + zassert_true((ticks >= (exp_ticks - CYC_PER_TICK + 1)) && (ticks <= (exp_ticks + GRTC_SLEW_TICKS)), "Unexpected result %" PRId64 " (expected: %" PRId64 ")", ticks, exp_ticks); /* Absolute timeout 10ms in the future */ - t = Z_TIMEOUT_TICKS(Z_TICK_ABS(sys_clock_tick_get() + K_MSEC(10).ticks)); - exp_ticks = z_nrf_grtc_timer_read() + K_MSEC(10).ticks * CYC_PER_TICK; + do { + /* GRTC and system tick must be read during single system tick. */ + curr_tick = sys_clock_tick_get(); + curr_grtc_tick = z_nrf_grtc_timer_read(); + curr_tick2 = sys_clock_tick_get(); + } while (curr_tick != curr_tick2); + + t = Z_TIMEOUT_TICKS(Z_TICK_ABS(curr_tick + K_MSEC(10).ticks)); + exp_ticks = curr_grtc_tick + K_MSEC(10).ticks * CYC_PER_TICK; ticks = z_nrf_grtc_timer_get_ticks(t); zassert_true((ticks >= (exp_ticks - CYC_PER_TICK + 1)) && (ticks <= (exp_ticks + GRTC_SLEW_TICKS)),