Skip to content

drivers/periph/timer: document timer_init re-initialization precondition - #22385

Open
andrewlihhh wants to merge 3 commits into
RIOT-OS:masterfrom
andrewlihhh:fix/lpc23xx-timer-isr-ctx-atomic
Open

drivers/periph/timer: document timer_init re-initialization precondition#22385
andrewlihhh wants to merge 3 commits into
RIOT-OS:masterfrom
andrewlihhh:fix/lpc23xx-timer-isr-ctx-atomic

Conversation

@andrewlihhh

@andrewlihhh andrewlihhh commented Jun 16, 2026

Copy link
Copy Markdown

Contribution description

This PR documents the usage model of timer_init() for the same timer device.

The discussion in #22384 pointed out that the periph_timer API did not clearly
state whether calling timer_init() again on an already active timer is
supported.

This PR clarifies that callers must not call timer_init() again on the same
timer while it is active. A timer must either not have been initialized before,
or it must be stopped with timer_stop() before calling timer_init() again.

The change is documentation-only. It does not change any timer implementation.

Testing procedure

Documentation-only change.

To review:

  1. Check the new @pre entry in drivers/include/periph/timer.h.
  2. Check the updated periph_timer entry in
    drivers/include/periph/doc.txt.
  3. Verify that no driver implementation is changed.

Issues/PRs references

Refs #22384

Declaration of AI-Tools / LLMs usage:

AI-Tools / LLMs that were used are:

  • OpenAI Codex for drafting the documentation wording and pull request text, with user review

@github-actions

Copy link
Copy Markdown

Hey @andrewlihhh, thank you for your first contribution! We really appreciate it! If you haven't already, please take a look at our contributing guidelines before the review process starts. Also, due to how the GitHub review system works, please avoid force-pushing or squashing your commits unless asked to by a maintainer (or unless your commit is still in "draft commit" stage). Lastly, make sure to comply with our AI Policy when using AI. Your pull request will be reviewed as soon as possible.

@github-actions github-actions Bot added Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jun 16, 2026
@AnnsAnns AnnsAnns added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Jun 16, 2026
@riot-ci

riot-ci commented Jun 16, 2026

Copy link
Copy Markdown

Murdock results

✔️ PASSED

3de6585 Update drivers/include/periph/timer.h

Success Failures Total Runtime
11124 0 11124 15m:17s

Artifacts

@crasbe

crasbe commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Thank you for the Pull Request. Can you provide some testing traces of the board before and after your PR to show the changes?

Also, you don't have to open a separate issue if you intend to open a PR too, that's redundant.

@andrewlihhh

Copy link
Copy Markdown
Author

I do not have the original lpc23xx/mcb2388 board locally, so I prepared a small supplemental QEMU + RIOT reproduction on a RIOT-supported Cortex-M target (microbit / nrf51) to show the before/after timing difference.

This is not meant as exact board-level validation for lpc23xx. I am only using it as supplemental evidence that the change affects when a pending timer IRQ can observe the callback context during re-initialization.

What changes is the delivery point of the same pending IRQ relative to the re-init window.

Before, the pending IRQ can be handled during the re-init window:

[drv] timer_init before NVIC_EnableIRQ phase=1
[drv] inject pending compare0 IRQ phase=1
[drv] irq_handler enter num=0 phase=1 flags=0x01 evt0=0 inten=0x00010000
[drv] timer_init after NVIC_EnableIRQ phase=1
[app] CB_NEW phase=1 chan=0 hits_old=0 hits_new=1 cc0=32 evt0=0

After, the same pending IRQ is deferred until the end of the interrupt-masked region:

[drv] timer_init before NVIC_EnableIRQ phase=1
[drv] inject pending compare0 IRQ phase=1
[drv] timer_init after NVIC_EnableIRQ phase=1
[drv] timer_init after TASKS_START phase=1
[drv] irq_handler enter num=0 phase=1 flags=0x01 evt0=1 inten=0x00010000
[app] CB_NEW phase=1 chan=0 hits_old=0 hits_new=1 cc0=32 evt0=0
[drv] timer_init after irq_restore phase=1

So while this is not a direct lpc23xx hardware trace, it does show the intended effect of the change: without protection, a pending IRQ may run inside the callback-context update / re-enable window; with the protected version, delivery is pushed to the end of that region.

@github-actions github-actions Bot added Area: doc Area: Documentation Area: drivers Area: Device drivers and removed Platform: ARM Platform: This PR/issue effects ARM-based platforms Area: cpu Area: CPU/MCU ports labels Jun 24, 2026
@andrewlihhh andrewlihhh changed the title cpu/lpc23xx/periph/timer: protect ISR callback context update drivers/periph/timer: document timer_init re-initialization precondition Jun 24, 2026
@andrewlihhh

Copy link
Copy Markdown
Author

I reworked this PR into a documentation-only change as suggested. It now documents the timer_init() re-initialization precondition instead of changing the lpc23xx driver implementation.

@AnnsAnns AnnsAnns added AI: Co-Authored PR/Issue relies on AI. Documentation / Code was partly generated by AI. AI: Vibed PR/Issue appears to be more AI than Human. and removed AI: Co-Authored PR/Issue relies on AI. Documentation / Code was partly generated by AI. labels Jul 6, 2026
@crasbe crasbe added the Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation label Jul 11, 2026
Comment thread drivers/include/periph/timer.h Outdated
Co-authored-by: crasbe <crasbe@gmail.com>

@crasbe crasbe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for addressing the changes! Please squash!

@crasbe crasbe added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs labels Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Vibed PR/Issue appears to be more AI than Human. Area: doc Area: Documentation Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR CI: skip compile test If set, CI server will run only non-compile jobs, but no compile jobs or their dependent jobs Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: enhancement The issue suggests enhanceable parts / The PR enhances parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: cpu/lpc23xx/periph/timer: old timer IRQ may call new callback after re-init

4 participants