From c27c07c5bd3c1e116b12f1c546c3b11ca3aa9ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Mon, 31 Mar 2025 13:41:18 +0200 Subject: [PATCH 1/2] [nrf fromlist] drivers: timer: nrf_grtc_timer: Optimize to reduce register access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- drivers/timer/nrf_grtc_timer.c | 89 ++++++++++++++++++++++------------ west.yml | 2 +- 2 files changed, 59 insertions(+), 32 deletions(-) diff --git a/drivers/timer/nrf_grtc_timer.c b/drivers/timer/nrf_grtc_timer.c index 285e48a57fd..f8831d9c429 100644 --- a/drivers/timer/nrf_grtc_timer.c +++ b/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_REL_CYCLES BIT_MASK(31) +#define MAX_REL_TICKS (MAX_REL_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); } @@ -362,6 +365,7 @@ int z_nrf_grtc_timer_capture_read(int32_t chan, uint64_t *captured_time) int z_nrf_grtc_wakeup_prepare(uint64_t wake_time_us) { nrfx_err_t err_code; + static struct k_spinlock lock; static uint8_t systemoff_channel; uint64_t now = counter(); nrfx_grtc_sleep_config_t sleep_cfg; @@ -422,22 +426,15 @@ int z_nrf_grtc_wakeup_prepare(uint64_t wake_time_us) } #endif /* CONFIG_POWEROFF */ + 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 +443,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 +484,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 +541,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 ch = system_clock_channel_data.channel; - uint64_t delta_time = ticks * CYC_PER_TICK; + if ((cc_value == expired_cc) && (ticks < MAX_REL_TICKS)) { + uint32_t cyc = ticks * CYC_PER_TICK; - uint64_t target_time = counter() + delta_time; + /* If it's the first timeout setting after previous expiration and timeout + * is short so fast method can be used which utilizes relative CC configuration. + */ + cc_value += cyc; + nrfx_grtc_syscounter_cc_rel_set(ch, cyc, NRFX_GRTC_CC_RELATIVE_COMPARE); + return; + } + + uint64_t cyc = (uint64_t)ticks * CYC_PER_TICK; + 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) + 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. */ - target_time = MAX((target_time - last_count)/CYC_PER_TICK, 1)*CYC_PER_TICK + last_count; + if (prev_cc_val < cc_value) { + int64_t now = last_count + last_elapsed; + + safe_setting = (prev_cc_val - now) < LATENCY_THR_TICKS; + } - system_timeout_set_abs(target_time); + nrfx_grtc_syscounter_cc_abs_set(ch, cc_value, safe_setting); } #if defined(CONFIG_NRF_GRTC_TIMER_APP_DEFINED_INIT) diff --git a/west.yml b/west.yml index 5c06ac61f0c..b37484a29f0 100644 --- a/west.yml +++ b/west.yml @@ -198,7 +198,7 @@ manifest: groups: - hal - name: hal_nordic - revision: 243c6708278364516c6ca1dcccc00c5f721095fa + revision: pull/288/head path: modules/hal/nordic groups: - hal From 3ca0b860c23ee54295975ea44bc2a48fc381ffee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Krzysztof=20Chru=C5=9Bci=C5=84ski?= Date: Mon, 19 May 2025 15:42:20 +0200 Subject: [PATCH 2/2] [nrf fromlist] tests: drivers: timer: nrf_grtc_timer: Add stress test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add stress test that randomly starts and aborts multiple timers from various contexts. Test checks if timers do not expire prematurely. Upstream PR #: 87944 Signed-off-by: Krzysztof Chruściński --- .../boards/nrf54h20dk_nrf54h20_cpuapp.overlay | 17 ++ .../boards/nrf54l15dk_nrf54l15_cpuapp.overlay | 32 +++ .../nrf54l15dk_nrf54l15_cpuflpr.overlay | 32 +++ tests/drivers/timer/nrf_grtc_timer/prj.conf | 6 + tests/drivers/timer/nrf_grtc_timer/src/main.c | 247 ++++++++++++++++++ .../timer/nrf_grtc_timer/testcase.yaml | 37 +-- 6 files changed, 354 insertions(+), 17 deletions(-) create mode 100644 tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuapp.overlay create mode 100644 tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuflpr.overlay diff --git a/tests/drivers/timer/nrf_grtc_timer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay index 04580b71480..7ae95926703 100644 --- a/tests/drivers/timer/nrf_grtc_timer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay +++ b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54h20dk_nrf54h20_cpuapp.overlay @@ -2,4 +2,21 @@ &grtc { /delete-property/ child-owned-channels; + interrupts = <109 2>; +}; + +test_timer: &timer131 { + status = "okay"; + interrupts = <419 1>; +}; + +&timer130 { + status = "okay"; + prescaler = <0>; +}; + +/ { + chosen { + zephyr,cpu-load-counter = &timer130; + }; }; diff --git a/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuapp.overlay b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuapp.overlay new file mode 100644 index 00000000000..7bcede529b0 --- /dev/null +++ b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuapp.overlay @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +&grtc { + interrupts = <228 2>; +}; + +test_timer: &timer21 { + status = "okay"; + interrupts = <203 1>; +}; + +&timer20 { + status = "okay"; + interrupts = <202 0>; +}; + +&timer00 { + status = "okay"; + prescaler = <0>; +}; + +/ { + chosen { + zephyr,cpu-load-counter = &timer00; + }; + + busy-sim { + compatible = "vnd,busy-sim"; + status = "okay"; + counter = <&timer20>; + }; +}; diff --git a/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuflpr.overlay b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuflpr.overlay new file mode 100644 index 00000000000..cfa72ed0273 --- /dev/null +++ b/tests/drivers/timer/nrf_grtc_timer/boards/nrf54l15dk_nrf54l15_cpuflpr.overlay @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +&grtc { + /*interrupts = <226 2>;*/ +}; + +test_timer: &timer21 { + status = "okay"; + /*interrupts = <203 2>;*/ +}; + +&timer20 { + status = "okay"; + interrupts = <202 0>; +}; + +&timer00 { + status = "okay"; + prescaler = <0>; +}; + +/ { + chosen { + zephyr,cpu-load-counter = &timer00; + }; + + busy-sim { + compatible = "vnd,busy-sim"; + status = "okay"; + counter = <&timer20>; + }; +}; diff --git a/tests/drivers/timer/nrf_grtc_timer/prj.conf b/tests/drivers/timer/nrf_grtc_timer/prj.conf index dea03477519..93926f090b7 100644 --- a/tests/drivers/timer/nrf_grtc_timer/prj.conf +++ b/tests/drivers/timer/nrf_grtc_timer/prj.conf @@ -1,2 +1,8 @@ CONFIG_ZTEST=y CONFIG_NRF_GRTC_TIMER=y +CONFIG_COUNTER=y +CONFIG_TEST_RANDOM_GENERATOR=y +CONFIG_XOSHIRO_RANDOM_GENERATOR=y +CONFIG_LOG_PRINTK=y +CONFIG_CPU_LOAD=y +CONFIG_CPU_LOAD_USE_COUNTER=y diff --git a/tests/drivers/timer/nrf_grtc_timer/src/main.c b/tests/drivers/timer/nrf_grtc_timer/src/main.c index f53188841bd..323b1d0685d 100644 --- a/tests/drivers/timer/nrf_grtc_timer/src/main.c +++ b/tests/drivers/timer/nrf_grtc_timer/src/main.c @@ -5,7 +5,15 @@ */ #include #include +#include +#include +#include +#include +#include +#include +#include #include +LOG_MODULE_REGISTER(test, 1); #define GRTC_SLEW_TICKS 10 #define NUMBER_OF_TRIES 2000 @@ -150,4 +158,243 @@ ZTEST(nrf_grtc_timer, test_timer_abort_in_compare_mode) z_nrf_grtc_timer_chan_free(channel); } +enum test_timer_state { + TIMER_IDLE, + TIMER_PREPARE, + TIMER_ACTIVE +}; + +enum test_ctx { + TEST_HIGH_PRI, + TEST_TIMER_CB, + TEST_THREAD +}; + +struct test_grtc_timer { + struct k_timer timer; + uint32_t ticks; + uint32_t expire; + uint32_t start_cnt; + uint32_t expire_cnt; + uint32_t abort_cnt; + uint32_t exp_expire; + int max_late; + int min_late; + int avg_late; + uint32_t early_cnt; + enum test_timer_state state; +}; + +static atomic_t test_active_cnt; +static struct test_grtc_timer timers[8]; +static uint32_t test_end; +static k_tid_t test_tid; +static volatile bool test_run; +static uint32_t ctx_cnt[3]; +static const char *const ctx_name[] = { "HIGH PRIO ISR", "TIMER CALLBACK", "THREAD" }; + +static bool stress_test_action(int ctx, int id) +{ + struct test_grtc_timer *timer = &timers[id]; + + ctx_cnt[ctx]++; + if (timer->state == TIMER_ACTIVE) { + /* Aborting soon to expire timers from higher interrupt priority may lead + * to test failures. + */ + if (ctx == 0 && (k_timer_remaining_get(&timer->timer) < 5)) { + return true; + } + + if (timer->abort_cnt < timer->expire_cnt / 2) { + bool any_active; + + timer->state = TIMER_PREPARE; + k_timer_stop(&timer->timer); + timer->abort_cnt++; + any_active = atomic_dec(&test_active_cnt) > 1; + timer->state = TIMER_IDLE; + + return any_active; + } + } else if (timer->state == TIMER_IDLE) { + int ticks = 10 + (sys_rand32_get() & 0x3F); + k_timeout_t t = K_TICKS(ticks); + + timer->exp_expire = k_ticks_to_cyc_floor32(sys_clock_tick_get_32() + ticks); + timer->state = TIMER_PREPARE; + timer->ticks = ticks; + k_timer_start(&timer->timer, t, K_NO_WAIT); + atomic_inc(&test_active_cnt); + timer->start_cnt++; + timer->state = TIMER_ACTIVE; + } + + return true; +} + +static void stress_test_actions(int ctx) +{ + uint32_t r = sys_rand32_get(); + int action_cnt = Z_MAX(r & 0x3, 1); + int tmr_id = (r >> 8) % ARRAY_SIZE(timers); + + /* Occasionally wake thread context from which timer actions are also executed. */ + if ((((r >> 2) & 0x3) == 0) || test_active_cnt < 2) { + LOG_DBG("ctx:%d thread wakeup", ctx); + k_wakeup(test_tid); + } + + for (int i = 0; i < action_cnt; i++) { + if (stress_test_action(ctx, tmr_id) == false) { + stress_test_action(ctx, tmr_id); + } + } +} + +static void timer_cb(struct k_timer *timer) +{ + struct test_grtc_timer *test_timer = CONTAINER_OF(timer, struct test_grtc_timer, timer); + uint32_t now = k_cycle_get_32(); + int diff = now - test_timer->exp_expire; + + atomic_dec(&test_active_cnt); + zassert_true(diff >= 0); + test_timer->max_late = MAX(diff, test_timer->max_late); + test_timer->min_late = MIN(diff, test_timer->min_late); + + if (test_timer->expire_cnt == 0) { + test_timer->avg_late = diff; + } else { + test_timer->avg_late = (test_timer->avg_late * test_timer->expire_cnt + diff) / + (test_timer->expire_cnt + 1); + } + + test_timer->expire_cnt++; + test_timer->state = TIMER_IDLE; + + if (test_run) { + stress_test_actions(TEST_TIMER_CB); + } +} + +static void counter_set(const struct device *dev, struct counter_alarm_cfg *cfg) +{ + int err; + uint32_t us = 150 + (sys_rand32_get() & 0x3F); + + cfg->ticks = counter_us_to_ticks(dev, us); + err = counter_set_channel_alarm(dev, 0, cfg); + zassert_equal(err, 0); +} + +static void counter_cb(const struct device *dev, uint8_t chan_id, uint32_t ticks, void *user_data) +{ + struct counter_alarm_cfg *config = user_data; + + if (test_run) { + stress_test_actions(TEST_HIGH_PRI); + counter_set(dev, config); + } +} + +static void report_progress(uint32_t start, uint32_t end) +{ + static uint32_t next_report; + static uint32_t step; + static uint32_t progress; + + if (next_report == 0) { + step = (end - start) / 10; + next_report = start + step; + } + + if (k_uptime_get_32() > next_report) { + next_report += step; + progress += 10; + printk("%d%%\r", progress); + } +} + +static void grtc_stress_test(bool busy_sim_en) +{ + static struct counter_alarm_cfg alarm_cfg; +#if DT_NODE_EXISTS(DT_NODELABEL(test_timer)) && DT_NODE_HAS_STATUS(DT_NODELABEL(test_timer), okay) + const struct device *const counter_dev = DEVICE_DT_GET(DT_NODELABEL(test_timer)); +#else + const struct device *const counter_dev = NULL; +#endif + uint32_t test_ms = 5000; + uint32_t test_start = k_uptime_get_32(); + uint32_t load; + + test_end = k_cycle_get_32() + k_ms_to_cyc_floor32(test_ms); + test_tid = k_current_get(); + + for (size_t i = 0; i < ARRAY_SIZE(timers); i++) { + k_timer_init(&timers[i].timer, timer_cb, NULL); + } + + if (IS_ENABLED(CONFIG_CPU_LOAD)) { + (void)cpu_load_get(true); + } + + if (counter_dev) { + counter_start(counter_dev); + } + + alarm_cfg.callback = counter_cb; + alarm_cfg.user_data = &alarm_cfg; + test_run = true; + + if (counter_dev) { + counter_set(counter_dev, &alarm_cfg); + } + + if (busy_sim_en) { + busy_sim_start(500, 200, 1000, 400, NULL); + } + + LOG_DBG("Starting test, will end at %d", test_end); + while (k_cycle_get_32() < test_end) { + report_progress(test_start, test_start + test_ms); + stress_test_actions(TEST_THREAD); + k_sleep(K_MSEC(test_ms)); + } + + load = IS_ENABLED(CONFIG_CPU_LOAD) ? cpu_load_get(true) : 0; + + test_run = false; + k_msleep(50); + + for (size_t i = 0; i < ARRAY_SIZE(timers); i++) { + zassert_equal(timers[i].state, TIMER_IDLE, "Unexpected timer %d state:%d", + i, timers[i].state); + TC_PRINT("Timer%d (%p)\r\n\tstart_cnt:%d abort_cnt:%d expire_cnt:%d\n", + i, &timers[i], timers[i].start_cnt, timers[i].abort_cnt, + timers[i].expire_cnt); + TC_PRINT("\tavarage late:%d ticks, max late:%d, min late:%d early:%d\n", + timers[i].avg_late, timers[i].max_late, timers[i].min_late, + timers[i].early_cnt); + } + + for (size_t i = 0; i < ARRAY_SIZE(ctx_cnt); i++) { + TC_PRINT("Context: %s executed %d times\n", ctx_name[i], ctx_cnt[i]); + } + TC_PRINT("CPU load during test:%d.%d\n", load / 10, load % 10); + + if (busy_sim_en) { + busy_sim_stop(); + } + + if (counter_dev) { + counter_stop(counter_dev); + } +} + +ZTEST(nrf_grtc_timer, test_stress) +{ + grtc_stress_test(false); +} + ZTEST_SUITE(nrf_grtc_timer, NULL, NULL, NULL, NULL, NULL); diff --git a/tests/drivers/timer/nrf_grtc_timer/testcase.yaml b/tests/drivers/timer/nrf_grtc_timer/testcase.yaml index ece1358741a..526590e1c28 100644 --- a/tests/drivers/timer/nrf_grtc_timer/testcase.yaml +++ b/tests/drivers/timer/nrf_grtc_timer/testcase.yaml @@ -1,18 +1,21 @@ +common: + tags: + - drivers + platform_allow: + - nrf54l15dk/nrf54l15/cpuapp + - nrf54l15dk/nrf54l15/cpuflpr + - nrf54l15bsim/nrf54l15/cpuapp + - nrf54h20dk/nrf54h20/cpuapp + - nrf54h20dk/nrf54h20/cpurad + - nrf54h20dk/nrf54h20/cpuppr + - nrf54l20pdk/nrf54l20/cpuapp + - nrf54l20pdk/nrf54l20/cpuflpr + - ophelia4ev/nrf54l15/cpuapp + - ophelia4ev/nrf54l15/cpuflpr + integration_platforms: + - nrf54l15dk/nrf54l15/cpuapp tests: - drivers.timer.nrf_grtc_timer: - tags: drivers - platform_allow: - - nrf54l09pdk/nrf54l09/cpuapp - - nrf54l09pdk/nrf54l09/cpuflpr - - nrf54l15dk/nrf54l15/cpuapp - - nrf54l15dk/nrf54l15/cpuflpr - - nrf54l15bsim/nrf54l15/cpuapp - - nrf54h20dk/nrf54h20/cpuapp - - nrf54h20dk/nrf54h20/cpurad - - nrf54h20dk/nrf54h20/cpuppr - - nrf54l20pdk/nrf54l20/cpuapp - - nrf54l20pdk/nrf54l20/cpuflpr - - ophelia4ev/nrf54l15/cpuapp - - ophelia4ev/nrf54l15/cpuflpr - integration_platforms: - - nrf54l20pdk/nrf54l20/cpuapp + drivers.timer.nrf_grtc_timer: {} + drivers.timer.nrf_grtc_timer.no_assert: + extra_configs: + - CONFIG_ASSERT=n