michaelh/zephyr
1
0
Fork 0
Code Releases Activity

drivers: timer: Optimized the ambiq stimer driver

Browse source 
The original driver has two defects: 1. When setting the next timeout
value the original implementation simply sets a delta value equal to
ticks * CYC_PER_TICK. This operation is reckless and may incorrectly
"reset" the fractional tick, causing clock skew. 2. The original
implementation doesn't handle the counter overflow situation. When the
counter overflows from 0xffffffff to 0x0, the uptimer counter becomes
incorrect. We have fixed above issue by rewriting most of the functions in
this driver and verified it by running all tests under
tests/kernel/timer folder.

Signed-off-by: Zhengwei Wang <zwang@ambiq.com>
This commit is contained in:
Zhengwei Wang 2024-02-07 15:02:15 +08:00 • committed by Anas Nashif
commit 96ff0f1e04
1 changed files with 88 additions and 27 deletions
Download patch file Download diff file Expand all files Collapse all files

115
drivers/timer/ambiq_stimer.c
View file

@ -35,12 +35,41 @@
const int32_t z_sys_timer_irq_for_test = TIMER_IRQ;
#endif
/* Value of STIMER counter when the previous kernel tick was announced */
static atomic_t g_last_count;
/* Elapsed ticks since the previous kernel tick was announced, It will get accumulated every time
* stimer_isr is triggered, or sys_clock_set_timeout/sys_clock_elapsed API is called.
* It will be cleared after sys_clock_announce is called,.
*/
static uint32_t g_tick_elapsed;
/* Value of STIMER counter when the previous timer API is called, this value is
* aligned to tick boundary. It is updated along with the g_tick_elapsed value.
*/
static uint32_t g_last_time_stamp;
/* Spinlock to sync between Compare ISR and update of Compare register */
static struct k_spinlock g_lock;
static void update_tick_counter(void)
{
/* Read current cycle count. */
uint32_t now = am_hal_stimer_counter_get();
/* If current cycle count is smaller than the last time stamp, a counter overflow happened.
* We need to extend the current counter value to 64 bits and add it with 0xFFFFFFFF
* to get the correct elapsed cycles.
*/
uint64_t now_64 = (g_last_time_stamp <= now) ? (uint64_t)now : (uint64_t)now + COUNTER_MAX;
/* Get elapsed cycles */
uint32_t elapsed_cycle = (now_64 - g_last_time_stamp);
/* Get elapsed ticks. */
uint32_t dticks = elapsed_cycle / CYC_PER_TICK;
g_last_time_stamp += dticks * CYC_PER_TICK;
g_tick_elapsed += dticks;
}
static void stimer_isr(const void *arg)
{
ARG_UNUSED(arg);
@ -52,22 +81,34 @@ static void stimer_isr(const void *arg)
k_spinlock_key_t key = k_spin_lock(&g_lock);
uint32_t now = am_hal_stimer_counter_get();
uint32_t dticks = (uint32_t)((now - g_last_count) / CYC_PER_TICK);
g_last_count += dticks * CYC_PER_TICK;
/*Calculate the elapsed ticks based on the current cycle count*/
update_tick_counter();
if (!IS_ENABLED(CONFIG_TICKLESS_KERNEL)) {
uint32_t next = g_last_count + CYC_PER_TICK;
if ((int32_t)(next - now) < MIN_DELAY) {
next += CYC_PER_TICK;
}
am_hal_stimer_compare_delta_set(0, next - g_last_count);
/* Get the counter value to trigger the next tick interrupt. */
uint64_t next = (uint64_t)g_last_time_stamp + CYC_PER_TICK;
/* Read current cycle count. */
uint32_t now = am_hal_stimer_counter_get();
/* If current cycle count is smaller than the last time stamp, a counter
* overflow happened. We need to extend the current counter value to 64 bits
* and add 0xFFFFFFFF to get the correct elapsed cycles.
*/
uint64_t now_64 = (g_last_time_stamp <= now) ? (uint64_t)now
: (uint64_t)now + COUNTER_MAX;
uint32_t delta = (now_64 + MIN_DELAY < next) ? (next - now_64) : MIN_DELAY;
/* Set delta. */
am_hal_stimer_compare_delta_set(0, delta);
}
k_spin_unlock(&g_lock, key);
sys_clock_announce(dticks);
sys_clock_announce(g_tick_elapsed);
g_tick_elapsed = 0;
}
}
@ -79,17 +120,40 @@ void sys_clock_set_timeout(int32_t ticks, bool idle)
return;
}
if (ticks == K_TICKS_FOREVER) {
return;
}
ticks = MIN(MAX_TICKS, ticks);
/* If tick is 0, set delta cyc to MIN_DELAY to trigger tick isr asap */
uint32_t cyc = MAX(ticks * CYC_PER_TICK, MIN_DELAY);
/* Adjust the ticks to the range of [1, MAX_TICKS]. */
ticks = (ticks == K_TICKS_FOREVER) ? MAX_TICKS : ticks;
ticks = CLAMP(ticks, 1, (int32_t)MAX_TICKS);
k_spinlock_key_t key = k_spin_lock(&g_lock);
am_hal_stimer_compare_delta_set(0, cyc);
/* Update the internal tick counter*/
update_tick_counter();
/* Get current hardware counter value.*/
uint32_t now = am_hal_stimer_counter_get();
/* last: the last recorded counter value.
* now_64: current counter value. Extended to uint64_t to easy the handing of hardware
* counter overflow.
* next: counter values where to trigger the scheduled timeout.
* last < now_64 < next
*/
uint64_t last = (uint64_t)g_last_time_stamp;
uint64_t now_64 = (g_last_time_stamp <= now) ? (uint64_t)now : (uint64_t)now + COUNTER_MAX;
uint64_t next = now_64 + ticks * CYC_PER_TICK;
uint32_t gap = next - last;
uint32_t gap_aligned = (gap / CYC_PER_TICK) * CYC_PER_TICK;
uint64_t next_aligned = last + gap_aligned;
uint32_t delta = next_aligned - now_64;
if (delta <= MIN_DELAY) {
/*If the delta value is smaller than MIN_DELAY, trigger a interrupt immediately*/
am_hal_stimer_int_set(AM_HAL_STIMER_INT_COMPAREA);
} else {
am_hal_stimer_compare_delta_set(0, delta);
}
k_spin_unlock(&g_lock, key);
}
@ -101,10 +165,10 @@ uint32_t sys_clock_elapsed(void)
}
k_spinlock_key_t key = k_spin_lock(&g_lock);
uint32_t ret = (am_hal_stimer_counter_get() - g_last_count) / CYC_PER_TICK;
update_tick_counter();
k_spin_unlock(&g_lock, key);
return ret;
return g_tick_elapsed;
}
uint32_t sys_clock_cycle_get_32(void)
@ -115,7 +179,6 @@ uint32_t sys_clock_cycle_get_32(void)
static int stimer_init(void)
{
uint32_t oldCfg;
k_spinlock_key_t key = k_spin_lock(&g_lock);
oldCfg = am_hal_stimer_config(AM_HAL_STIMER_CFG_FREEZE);
@ -126,9 +189,7 @@ static int stimer_init(void)
am_hal_stimer_config((oldCfg & ~(AM_HAL_STIMER_CFG_FREEZE | STIMER_STCFG_CLKSEL_Msk)) |
AM_HAL_STIMER_XTAL_32KHZ | AM_HAL_STIMER_CFG_COMPARE_A_ENABLE);
#endif
g_last_count = am_hal_stimer_counter_get();
k_spin_unlock(&g_lock, key);
g_last_time_stamp = am_hal_stimer_counter_get();
NVIC_ClearPendingIRQ(TIMER_IRQ);
IRQ_CONNECT(TIMER_IRQ, 0, stimer_isr, 0, 0);