drivers: nrf_rtc_timer: Rework set_absolute_alarm()

Eliminate waiting for a potential COMPARE event when setting a CC
value close to the previously set one and rely instead on checking
target time when processing channel events in the ISR.

Signed-off-by: Andrzej Głąbek <andrzej.glabek@nordicsemi.no>

drivers/timer/nrf_rtc_timer.c

@@ -71,6 +71,11 @@ static uint32_t get_comparator(int32_t chan)
 	return nrf_rtc_cc_get(RTC, chan);
 }
 
+static bool event_check(int32_t chan)
+{
+	return nrf_rtc_event_check(RTC, RTC_CHANNEL_EVENT_ADDR(chan));
+}
+
 static void event_clear(int32_t chan)
 {
 	nrf_rtc_event_clear(RTC, RTC_CHANNEL_EVENT_ADDR(chan));
@@ -225,63 +230,56 @@ uint64_t z_nrf_rtc_timer_get_ticks(k_timeout_t t)
  * @param[in] chan A channel for which a new CC value is to be set.
  *
  * @param[in] abs_val An absolute value of CC register to be set.
- *
- * @returns CC value that was actually set. It is equal to @p abs_val or
- *  shifted ahead if @p abs_val was too near in the future (+1 case).
  */
-static uint32_t set_absolute_alarm(int32_t chan, uint32_t abs_val)
+static void set_absolute_alarm(int32_t chan, uint32_t abs_val)
 {
-	uint32_t now;
-	uint32_t now2;
 	uint32_t cc_val = abs_val & COUNTER_MAX;
-	uint32_t prev_cc = get_comparator(chan);
+	uint32_t cc_inc = 2;
-	uint32_t tick_inc = 2;
+
+	/* Disable event routing for the channel to avoid getting a COMPARE
+	 * event for the previous CC value before the new one takes effect
+	 * (however, even if such spurious event was generated, it would be
+	 * properly filtered out in process_channel(), where the target time
+	 * is checked).
+	 * Clear also the event as it may already be generated at this point.
+	 */
+	event_disable(chan);
+	event_clear(chan);
+
+	for (;;) {
+		uint32_t now;
+
+		set_comparator(chan, cc_val);
+		/* Enable event routing after the required CC value was set.
+		 * Even though the above operation may get repeated (see below),
+		 * there is no need to disable event routing in every iteration
+		 * of the loop, as the COMPARE event resulting from any attempt
+		 * of setting the CC register is acceptable (as mentioned above,
+		 * process_channel() does the proper filtering).
+		 */
+		event_enable(chan);
 
-	do {
 		now = counter();
 
-		/* Handle case when previous event may generate an event.
+		/* RTC may not generate a COMPARE event if its COUNTER value
-		 * It is handled by setting CC to now (far in the future),
+		 * is N and a given CC register is set to N or N+1. If it turns
-		 * in case previous event was set for next tick wait for half
+		 * out that the above configuration of the comparator resulted
-		 * LF tick and clear event that may have been generated.
+		 * in such CC value or even in a value that is considered to be
+		 * from the past, repeat the operation using a CC value that is
+		 * guaranteed to generate the event. Start with 2 RTC ticks from
+		 * now and if that fails (because the operation gets delayed),
+		 * go even futher in the next attempt.
+		 * But if the COMPARE event turns out to be already generated,
+		 * there is obviously no need to continue the loop.
 		 */
-		set_comparator(chan, now);
+		if ((counter_sub(cc_val, now + 2) > COUNTER_HALF_SPAN) &&
-		if (counter_sub(prev_cc, now) == 1) {
+		    !event_check(chan)) {
-			/* It should wait for half of RTC tick 15.26us. As
+			cc_val = now + cc_inc;
-			 * busy wait runs from different clock source thus
+			cc_inc++;
-			 * wait longer to cover for discrepancy.
+		} else {
-			 */
+			break;
-			k_busy_wait(19);
 		}
-
-		/* RTC may not generate event if CC is set for 1 tick from now.
-		 * Because of that if requested cc_val is in the past or next tick,
-		 * set CC to further in future. Start with 2 ticks from now but
-		 * if that fails go even futher. It may fail if operation got
-		 * interrupted and RTC counter progressed or if optimization is
-		 * turned off.
-		 */
-		if (counter_sub(cc_val, now + 2) > COUNTER_HALF_SPAN) {
-			cc_val = now + tick_inc;
-			tick_inc++;
 	}
-
-		event_clear(chan);
-		event_enable(chan);
-		set_comparator(chan, cc_val);
-		now2 = counter();
-		prev_cc = cc_val;
-		/* Rerun the algorithm if counter progressed during execution
-		 * and cc_val is in the past or one tick from now. In such
-		 * scenario, it is possible that event will not be generated.
-		 * Rerunning the algorithm will delay the alarm but ensure that
-		 * event will be generated at the moment indicated by value in
-		 * CC register.
-		 */
-	} while ((now2 != now) &&
-		 (counter_sub(cc_val, now2 + 2) > COUNTER_HALF_SPAN));
-
-	return cc_val;
 }
 
 static int compare_set_nolocks(int32_t chan, uint64_t target_time,
@@ -302,9 +300,7 @@ static int compare_set_nolocks(int32_t chan, uint64_t target_time,
 			/* Target time is valid and is different than currently set.
 			 * Set CC value.
 			 */
-			uint32_t cc_set = set_absolute_alarm(chan, cc_value);
+			set_absolute_alarm(chan, cc_value);
-
-			target_time += counter_sub(cc_set, cc_value);
 		}
 	} else {
 		/* Force ISR handling when exiting from critical section. */
@@ -454,7 +450,7 @@ static bool channel_processing_check_and_clear(int32_t chan)
 		 * or be forced.
 		 */
 		result = atomic_and(&force_isr_mask, ~BIT(chan)) ||
-			 nrf_rtc_event_check(RTC, RTC_CHANNEL_EVENT_ADDR(chan));
+			 event_check(chan);
 
 		if (result) {
 			event_clear(chan);
@@ -493,6 +489,13 @@ static void process_channel(int32_t chan)
 			cc_data[chan].callback = NULL;
 			cc_data[chan].target_time = TARGET_TIME_INVALID;
 			event_disable(chan);
+			/* Because of the way set_absolute_alarm() sets the CC
+			 * register, it may turn out that another COMPARE event
+			 * has been generated for the same alarm. Make sure the
+			 * event is cleared, so that the ISR is not executed
+			 * again unnecessarily.
+			 */
+			event_clear(chan);
 		}
 
 		full_int_unlock(mcu_critical_state);