[nrf fromlist] drivers: timer: nrf_grtc_timer: Optimize to reduce register access

Speed up execution of the interrupt handler and sys_clock_set_timeout().
Sys_clock_set_timeout() can be called in two scenarios: from previous
timeout expiration handler or freely. If the former case fast path
can be used since CC value in the GRTC register just expired and it
can be used as a reference for CCADD setting. This is only a single
register write so it's much faster. In the latter a longer procedure
is applied which also happens in two variants. If value which is
set in CC is further in the future (e.g. K_FOREVER was set before) then
CC can be safely overwritten with a new value without a risk of
triggering unexpected COMPARE event. If value in CC is earlier than
the new CC value (if earlier timeout was aborted) then there is a
risk of COMPARE event happening while it is being overwritten.
That case requires long and safer procedure of setting CC.

Update hal_nordic with changes in the nrfx_grtc driver which are
needed for nrf_grtc_timer changes.

Upstream PR #: 87944

Signed-off-by: Krzysztof Chruściński <[email protected]>

Author: nordic-krch

drivers/timer/nrf_grtc_timer.c

@@ -49,21 +49,27 @@
 #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))
-
-#define MAX_CYCLES (MAX_TICKS * CYC_PER_TICK)
+/* To allow use of CCADD we need to limit max cycles to 31 bits. */
+#define MAX_CYCLES BIT_MASK(31)
+#define MAX_TICKS (MAX_CYCLES / CYC_PER_TICK)
 
 #define LFCLK_FREQUENCY_HZ DT_PROP(LFCLK_NODE, clock_frequency)
 
+/* Threshold used to determine if there is a risk of unexpected GRTC COMPARE event coming
+ * from previous CC value.
+ */
+#define LATENCY_THR_TICKS 200
+
 #if defined(CONFIG_TEST)
 const int32_t z_sys_timer_irq_for_test = DT_IRQN(GRTC_NODE);
 #endif
 
 static void sys_clock_timeout_handler(int32_t id, uint64_t cc_val, void *p_context);
 
-static struct k_spinlock lock;
 static uint64_t last_count; /* Time (SYSCOUNTER value) @last sys_clock_announce() */
+static uint32_t last_elapsed;
+static uint64_t cc_value; /* Value that is expected to be in CC register. */
+static uint64_t expired_cc; /* Value that is expected to be in CC register. */
 static atomic_t int_mask;
 static uint8_t ext_channels_allocated;
 static nrfx_grtc_channel_t system_clock_channel_data = {
@@ -145,17 +151,13 @@ static void compare_int_unlock(int32_t chan, bool key)
 static void sys_clock_timeout_handler(int32_t id, uint64_t cc_val, void *p_context)
 {
 	ARG_UNUSED(id);
+	ARG_UNUSED(cc_val);
 	ARG_UNUSED(p_context);
-	uint64_t dticks;
-	uint64_t now = counter();
-
-	if (unlikely(now < cc_val)) {
-		return;
-	}
+	uint32_t dticks;
 
 	dticks = counter_sub(cc_val, last_count) / CYC_PER_TICK;
-
-	last_count += dticks * CYC_PER_TICK;
+	last_count += (dticks * CYC_PER_TICK);
+	expired_cc = cc_val;
 
 	if (!IS_ENABLED(CONFIG_TICKLESS_KERNEL)) {
 		/* protection is not needed because we are in the GRTC interrupt
@@ -164,6 +166,7 @@ static void sys_clock_timeout_handler(int32_t id, uint64_t cc_val, void *p_conte
 		system_timeout_set_abs(last_count + CYC_PER_TICK);
 	}
 
+	last_elapsed = 0;
 	sys_clock_announce((int32_t)dticks);
 }
 
@@ -368,6 +371,7 @@ int z_nrf_grtc_wakeup_prepare(uint64_t wake_time_us)
 	/* Minimum time that ensures valid execution of system-off procedure. */
 	uint32_t minimum_latency_us;
 	uint32_t chan;
+	struct k_spinlock lock;
 	int ret;
 
 	nrfx_grtc_sleep_configuration_get(&sleep_cfg);
@@ -424,20 +428,12 @@ int z_nrf_grtc_wakeup_prepare(uint64_t wake_time_us)
 
 uint32_t sys_clock_cycle_get_32(void)
 {
-	k_spinlock_key_t key = k_spin_lock(&lock);
-	uint32_t ret = (uint32_t)counter();
-
-	k_spin_unlock(&lock, key);
-	return ret;
+	return (uint32_t)counter();
 }
 
 uint64_t sys_clock_cycle_get_64(void)
 {
-	k_spinlock_key_t key = k_spin_lock(&lock);
-	uint64_t ret = counter();
-
-	k_spin_unlock(&lock, key);
-	return ret;
+	return counter();
 }
 
 uint32_t sys_clock_elapsed(void)
@@ -446,7 +442,9 @@ uint32_t sys_clock_elapsed(void)
 		return 0;
 	}
 
-	return (uint32_t)(counter_sub(counter(), last_count) / CYC_PER_TICK);
+	last_elapsed = (uint32_t)counter_sub(counter(), last_count);
+
+	return last_elapsed / CYC_PER_TICK;
 }
 
 static int sys_clock_driver_init(void)
@@ -485,6 +483,9 @@ static int sys_clock_driver_init(void)
 	}
 #endif /* CONFIG_NRF_GRTC_START_SYSCOUNTER */
 
+	nrfx_grtc_channel_callback_set(system_clock_channel_data.channel,
+				       sys_clock_timeout_handler, NULL);
+
 	int_mask = NRFX_GRTC_CONFIG_ALLOWED_CC_CHANNELS_MASK;
 	if (!IS_ENABLED(CONFIG_TICKLESS_KERNEL)) {
 		system_timeout_set_relative(CYC_PER_TICK);
@@ -539,22 +540,47 @@ void sys_clock_set_timeout(int32_t ticks, bool idle)
 {
 	ARG_UNUSED(idle);
 
+	if (ticks == 0) {
+		return;
+	}
+
 	if (!IS_ENABLED(CONFIG_TICKLESS_KERNEL)) {
 		return;
 	}
 
-	ticks = (ticks == K_TICKS_FOREVER) ? MAX_TICKS : MIN(MAX_TICKS, MAX(ticks, 0));
+	uint32_t cyc;
+	uint32_t ch = system_clock_channel_data.channel;
 
-	uint64_t delta_time = ticks * CYC_PER_TICK;
+	if ((uint32_t)ticks > MAX_TICKS) {
+		cyc = MAX_CYCLES;
+	} else {
+		cyc = ticks * CYC_PER_TICK;
+	}
 
-	uint64_t target_time = counter() + delta_time;
+	if (cc_value == expired_cc) {
+		cc_value += cyc;
+		nrfx_grtc_syscounter_cc_rel_set(ch, cyc, NRFX_GRTC_CC_RELATIVE_COMPARE);
+	} else {
+		bool safe_setting = false;
+		int64_t prev_cc_val = cc_value;
 
-	/* Rounded down target_time to the tick boundary
-	 * (but not less than one tick after the last)
-	 */
-	target_time = MAX((target_time - last_count)/CYC_PER_TICK, 1)*CYC_PER_TICK + last_count;
+		cc_value = last_count + last_elapsed + cyc;
+
+		/* In case of timeout abort it may happen that CC is being set to a value
+		 * that later than previous CC. If previous CC value is not far in the
+		 * future, there is a risk that COMPARE event will be triggered for that
+		 * previous CC value. If there is such risk safe procedure must be applied
+		 * which is more time consuming but ensures that there will be no spurious
+		 * event.
+		 */
+		if (prev_cc_val < cc_value) {
+			int64_t now = last_count + last_elapsed;
 
-	system_timeout_set_abs(target_time);
+			safe_setting = (prev_cc_val - now) < LATENCY_THR_TICKS;
+		}
+
+		nrfx_grtc_syscounter_cc_abs_set(ch, cc_value, safe_setting);
+	}
 }
 
 #if defined(CONFIG_NRF_GRTC_TIMER_APP_DEFINED_INIT)

west.yml

@@ -198,7 +198,7 @@ manifest:
       groups:
         - hal
     - name: hal_nordic
-      revision: 243c6708278364516c6ca1dcccc00c5f721095fa
+      revision: pull/288/head
       path: modules/hal/nordic
       groups:
         - hal