Feature/zephyr compat/k timer isr dispatch#12
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes Boreas’ k_timer behavior more Zephyr-compatible by optionally dispatching timer expiry callbacks in true ISR context (via ESP_TIMER_ISR) instead of the esp_timer task, and updates docs/examples/tests accordingly.
Changes:
- Add
CONFIG_K_TIMER_DISPATCH_ISR(defaulty) and switchk_timerto useESP_TIMER_ISRwhen enabled. - Update internal users of
k_timer(e.g., watchdog supervisor, delayable work, thread delayed start) to accommodate ISR-dispatch semantics. - Add ISR-dispatch-focused unit tests and update the work-queue demo to defer logging out of ISR context.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
test/sdkconfig.defaults |
Enables ISR-dispatch config for the test app. |
test/main/test_k_timer.c |
Adds ISR-dispatch validation tests and marks callbacks IRAM_ATTR. |
examples/work_queue_demo/main/main.c |
Demonstrates deferring timer ISR work to k_work before logging. |
components/zsys/src/watchdog.c |
Defers watchdog timer expiry work via k_work to avoid ISR-heavy logic. |
components/zkernel/src/k_work.c |
Marks delayable-work timer expiry callback IRAM_ATTR under ISR dispatch. |
components/zkernel/src/k_timer.c |
Switches esp_timer dispatch method to ISR when configured. |
components/zkernel/src/k_thread.c |
Adapts delayed thread start to safely resume from ISR-dispatched timer expiry. |
components/zkernel/Kconfig |
Introduces K_TIMER_DISPATCH_ISR option (default enabled). |
components/zkernel/include/boreas/zephyr/kernel.h |
Updates timer callback-context documentation; inlines user-data helpers. |
Comments suppressed due to low confidence (1)
components/zkernel/src/k_work.c:342
- k_work_delayable_timer_expiry() is marked IRAM_ATTR under CONFIG_K_TIMER_DISPATCH_ISR, but it calls k_work_submit*/k_work_submit_to_queue, which are not IRAM_ATTR and may reside in flash. This undermines the stated “IRAM-only” requirement for ISR-dispatched timer callbacks and can crash when flash cache is disabled. Either ensure the work submission path (and its dependencies) is IRAM-safe under this config, or defer the submission to task context (e.g., via a pended function call).
#ifdef CONFIG_K_TIMER_DISPATCH_ISR
static void IRAM_ATTR k_work_delayable_timer_expiry(struct k_timer *timer)
#else
static void k_work_delayable_timer_expiry(struct k_timer *timer)
#endif
{
struct k_work_delayable *dwork = CONTAINER_OF(timer, struct k_work_delayable, timer);
if (dwork->queue) {
k_work_submit_to_queue(dwork->queue, &dwork->work);
} else {
k_work_submit(&dwork->work);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
components/zkernel/src/k_timer.c:88
- With ISR dispatch enabled,
esp_timer_stop()does not wait for an in-flight callback to finish, butk_timer_start()proceeds to reset/update shared fields (status,first_interval_pending,is_periodic,period_us, etc.). This can race withk_timer_esp_callback()running on another core and, in particular,period_usis a non-atomic 64-bit value that may be torn. To match Zephyr’s restart/stop semantics safely underESP_TIMER_ISR, add proper synchronization (e.g., a per-timer portMUX/spinlock used by both the ISR callback and start/stop paths, and/or anin_callbackflag that start/stop waits out in task context).
/* Stop any in-flight timer. With ESP_TIMER_TASK dispatch,
* esp_timer_stop blocks until the callback finishes; with
* ESP_TIMER_ISR dispatch it only removes the timer from the
* armed list (the ISR callback may still be executing on
* another core). Callers must not restart a timer whose
* callback is still running — same constraint as upstream Zephyr. */
if (__atomic_load_n(&timer->running, __ATOMIC_ACQUIRE)) {
esp_timer_stop(timer->handle);
}
__atomic_store_n(&timer->status, 0, __ATOMIC_RELAXED);
__atomic_store_n(&timer->first_interval_pending, false, __ATOMIC_RELAXED);
__atomic_store_n(&timer->is_periodic, !k_timeout_is_no_wait(period), __ATOMIC_RELEASE);
Comment on lines
116
to
134
| @@ -133,15 +134,15 @@ static int k_work_submit_internal(struct k_work_q *queue, struct k_work *work) | |||
| return 0; | |||
Comment on lines
+49
to
+53
| static void K_ISR_SAFE supervisor_check(struct k_timer *timer) | ||
| { | ||
| (void)timer; | ||
| k_work_submit(&supervisor_work); | ||
| } |
Comment on lines
+419
to
+424
| static inline void k_timer_user_data_set(struct k_timer *timer, void *user_data) | ||
| { | ||
| timer->user_data = user_data; | ||
| } | ||
|
|
||
| static inline void *k_timer_user_data_get(const struct k_timer *timer) |
swoisz
added a commit
that referenced
this pull request
Jul 2, 2026
…-isr-dispatch Feature/zephyr compat/k timer isr dispatch
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Align ISR dispatch ability. Previously all k_timer backend cbs were in the esp_timer task ctx. This is more zephyr-like.