[nrf fromtree] Fix nrfx_grtc test and optimize z_nrf_grtc_timer_get_ticks

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)

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);
 	}

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
 

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 {

kernel/sched.c

@@ -1064,27 +1064,26 @@ static inline void z_vrfy_k_yield(void)
 #include <zephyr/syscalls/k_yield_mrsh.c>
 #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);
 

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);
 		}
 	}
 

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);

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)),