Skip to content

cpu/stm32/periph/timer: fix clobered IRQ flag #19385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 13, 2023

Conversation

Enoch247
Copy link
Contributor

@Enoch247 Enoch247 commented Mar 13, 2023

Contribution description

From the git commit:

The STM32 periph_timer driver reads the timer's status flags, then clears them all. It is possible that a timer interrupt could occur between reading the flag and clearing it. This would lead to a lost interrupt.

The timer's status flags can be cleared by software, but can only be set by the hardware. This patch takes advantage of this by only clearing the flags it knows are set. The rest of the flags are set, which doesn't actually change their state.

I had trouble finding anything in ST's datasheet saying that software could not set the timer's status flags, but testing showed that this is how it works in practice. Further, ST's own HAL confirms this. If the hardware didn't work this way, it would be impossible to atomically read-modify-write the flags.

Testing procedure

I tested by doing the following:

  1. make -C tests/periph_timer BOARD=nucleo-f767zi all flash term
  2. press s
  3. press [ENTER]
  4. observe test passes
  5. make -C tests/periph_timer_periodic BOARD=nucleo-f767zi all flash term
  6. press s
  7. press [ENTER]
  8. observe test passes
  9. make -C tests/periph_timer_short_relative_set BOARD=nucleo-f767zi all flash term
  10. press s
  11. press [ENTER]
  12. observe test passes

Issues/PRs references

  • none known

The STM32 periph_timer driver reads the timer's status flags, then
clears them all. It is possible that a timer interrupt could occur
between reading the flag and clearing it. This would lead to a lost
interrupt.

The timer's status flags can be cleared by software, but can only be set
by the hardware. This patch takes advantage of this by only clearing the
flags it knows are set. The rest of the flags are set, which doesn't
actually change their state.
@github-actions github-actions bot added Area: cpu Area: CPU/MCU ports Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Mar 13, 2023
@benpicco benpicco added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Mar 13, 2023
@riot-ci
Copy link

riot-ci commented Mar 13, 2023

Murdock results

✔️ PASSED

dea2543 cpu/stm32/periph/timer: fix clobered IRQ flag

Success Failures Total Runtime
6882 0 6882 09m:30s

Artifacts

@benpicco
Copy link
Contributor

bors merge

@bors
Copy link
Contributor

bors bot commented Mar 13, 2023

Build succeeded:

@bors bors bot merged commit 9142d9c into RIOT-OS:master Mar 13, 2023
@Enoch247 Enoch247 deleted the fix-stm32-timer-driver-isr branch March 14, 2023 13:20
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants