From 3a700a5f62f7b8b0be698de2a16490d0a8cf6c2b Mon Sep 17 00:00:00 2001 From: Kevin O'Connor Date: Mon, 24 Nov 2025 17:09:41 -0500 Subject: [PATCH] timer_irq: Remove TIMER_IDLE_REPEAT_TICKS special case The TIMER_IDLE_REPEAT_TICKS was intended to reduce the number of cases where timers would defer to tasks when tasks are mostly idle. However, with commit ea546c78 this became less important because timers now only defer to tasks if tasks are consistently busy for two consecutive calls to sched_check_set_tasks_busy(). The TIMER_IDLE_REPEAT_TICKS mechanism could result in extended task delays if timers do become busy. Timers can become busy in normal operation if timers are scheduled to run more than 500,000 times a second (every 2us or faster). This can occur on stepper movement when using high microstep settings. If timers become busy, it could take up to 1ms for tasks to next be given an opportunity to run (two calls to sched_check_set_tasks_busy() at 500us per call). This wouldn't typically be an issue if tasks are also busy, but in some loads tasks may need to run periodically in such a way that the task status alternates between idle and busy. In this case, the TIMER_IDLE_REPEAT_TICKS mechanism could effectively limit the number of task wakeups to only ~1000 per second. The "USB to canbus bridge" code uses tasks to transfer data to/from USB and canbus. If timers become busy, the limiting of task wakeups could lead to a situation where the effective bandwidth becomes severely constrained. In particular, this can be an issue on USB implementations that do not support "double buffering" (such as the stm32 usbotg code). There are reports of "Timer too close" errors on "USB to canbus bridge" mode as a result of this issue. Fix by removing the TIMER_IDLE_REPEAT_TICKS check. Check for busy tasks every TIMER_REPEAT_TICKS instead (100us). Signed-off-by: Kevin O'Connor --- src/avr/timer.c | 3 +-- src/generic/armcm_timer.c | 3 +-- src/generic/timer_irq.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/src/avr/timer.c b/src/avr/timer.c index d306b7214..ef6269f06 100644 --- a/src/avr/timer.c +++ b/src/avr/timer.c @@ -154,7 +154,6 @@ static struct timer wrap_timer = { .waketime = 0x8000, }; -#define TIMER_IDLE_REPEAT_TICKS 8000 #define TIMER_REPEAT_TICKS 3000 #define TIMER_MIN_ENTRY_TICKS 44 @@ -202,7 +201,7 @@ ISR(TIMER1_COMPA_vect) next = now + TIMER_DEFER_REPEAT_TICKS; goto done; } - timer_repeat_set(now + TIMER_IDLE_REPEAT_TICKS); + timer_repeat_set(now + TIMER_REPEAT_TICKS); timer_set(now); } } diff --git a/src/generic/armcm_timer.c b/src/generic/armcm_timer.c index e77081804..b7f572685 100644 --- a/src/generic/armcm_timer.c +++ b/src/generic/armcm_timer.c @@ -112,7 +112,6 @@ timer_init(void) DECL_INIT(timer_init); static uint32_t timer_repeat_until; -#define TIMER_IDLE_REPEAT_TICKS timer_from_us(500) #define TIMER_REPEAT_TICKS timer_from_us(100) #define TIMER_MIN_TRY_TICKS timer_from_us(2) @@ -141,7 +140,7 @@ timer_dispatch_many(void) timer_repeat_until = now + TIMER_REPEAT_TICKS; return TIMER_DEFER_REPEAT_TICKS; } - timer_repeat_until = tru = now + TIMER_IDLE_REPEAT_TICKS; + timer_repeat_until = tru = now + TIMER_REPEAT_TICKS; } // Next timer in the past or near future - wait for it to be ready diff --git a/src/generic/timer_irq.c b/src/generic/timer_irq.c index 7c2e871bd..9f6b59aa3 100644 --- a/src/generic/timer_irq.c +++ b/src/generic/timer_irq.c @@ -30,7 +30,6 @@ timer_is_before(uint32_t time1, uint32_t time2) } static uint32_t timer_repeat_until; -#define TIMER_IDLE_REPEAT_TICKS timer_from_us(500) #define TIMER_REPEAT_TICKS timer_from_us(100) #define TIMER_MIN_TRY_TICKS timer_from_us(2) @@ -59,7 +58,7 @@ timer_dispatch_many(void) timer_repeat_until = now + TIMER_REPEAT_TICKS; return now + TIMER_DEFER_REPEAT_TICKS; } - timer_repeat_until = tru = now + TIMER_IDLE_REPEAT_TICKS; + timer_repeat_until = tru = now + TIMER_REPEAT_TICKS; } // Next timer in the past or near future - wait for it to be ready