From 205e684958ebea4d7f2a15d75b94aac880fb2390 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andrzej=20G=C5=82=C4=85bek?= Date: Mon, 2 Jan 2023 13:33:41 +0100 Subject: [PATCH] drivers: nrf_rtc_timer: Rework set_absolute_alarm() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- drivers/timer/nrf_rtc_timer.c | 109 +++++++++++++++++----------------- 1 file changed, 56 insertions(+), 53 deletions(-) diff --git a/drivers/timer/nrf_rtc_timer.c b/drivers/timer/nrf_rtc_timer.c index 98f5b3874bb..4488a058f93 100644 --- a/drivers/timer/nrf_rtc_timer.c +++ b/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 tick_inc = 2; + uint32_t cc_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. - * It is handled by setting CC to now (far in the future), - * in case previous event was set for next tick wait for half - * LF tick and clear event that may have been generated. + /* RTC may not generate a COMPARE event if its COUNTER value + * is N and a given CC register is set to N or N+1. If it turns + * out that the above configuration of the comparator resulted + * 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(prev_cc, now) == 1) { - /* It should wait for half of RTC tick 15.26us. As - * busy wait runs from different clock source thus - * wait longer to cover for discrepancy. - */ - k_busy_wait(19); + if ((counter_sub(cc_val, now + 2) > COUNTER_HALF_SPAN) && + !event_check(chan)) { + cc_val = now + cc_inc; + cc_inc++; + } else { + break; } - - /* 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); - - target_time += counter_sub(cc_set, cc_value); + set_absolute_alarm(chan, 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);