kaspar/RIOT
1
0
Fork 0
mirror of https://github.com/RIOT-OS/RIOT.git synced 2025-12-23 21:43:51 +01:00
Code Releases Activity

cpu/stm32/periph/timer: fix execution flow

Browse Source 
The implmentation of `timer_set_absolute()` has The following problems.
First, it attempts to restore the auto reload register (ARR) to it's
default if the ARR was previosly set by `timer_set_periodic()` by
comparing it to the channel's capture compare (CC) register _after_ it
has already set the CC register. Secondly, it clears spurious IRQs
_after_ the CC register has been set. If the value being set is equal to
the timer's current count (or the two become equal before the supurios
IRQ clearing happens), this could cause a legitimate IRQ to be cleared.

The implmentation of `timer_set()` has the same error in handling the
ARR as described above.

This patch reorders the operations of both functions to do:

1. handle ARR
2. clear spurious IRQs
3. set channel's CC
4. enable IRQ

Additionally, the calulation of `value` in `timer_set()` is moved
earlier in the function's exec path as a pedantic measure.
This commit is contained in:
Joshua DeWeese 2023-03-07 11:29:17 -05:00
parent 6488fe7cb3
commit eeb359e80c
1 changed files with 12 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
cpu/stm32/periph/timer.c
View File

@ -128,8 +128,6 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
unsigned irqstate = irq_disable(); unsigned irqstate = irq_disable();
set_oneshot(tim, channel); set_oneshot(tim, channel);
TIM_CHAN(tim, channel) = (value & timer_config[tim].max);
#ifdef MODULE_PERIPH_TIMER_PERIODIC #ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) { if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max; dev(tim)->ARR = timer_config[tim].max;
@ -139,6 +137,8 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
/* clear spurious IRQs */ /* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel); dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);
TIM_CHAN(tim, channel) = (value & timer_config[tim].max);
/* enable IRQ */ /* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel); dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);
irq_restore(irqstate); irq_restore(irqstate);
@ -148,6 +148,8 @@ int timer_set_absolute(tim_t tim, int channel, unsigned int value)
int timer_set(tim_t tim, int channel, unsigned int timeout) int timer_set(tim_t tim, int channel, unsigned int timeout)
{ {
unsigned value = (dev(tim)->CNT + timeout) & timer_config[tim].max;
if (channel >= (int)TIMER_CHANNEL_NUMOF) { if (channel >= (int)TIMER_CHANNEL_NUMOF) {
return -1; return -1;
} }
@ -155,21 +157,20 @@ int timer_set(tim_t tim, int channel, unsigned int timeout)
unsigned irqstate = irq_disable(); unsigned irqstate = irq_disable();
set_oneshot(tim, channel); set_oneshot(tim, channel);
/* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);
unsigned value = (dev(tim)->CNT + timeout) & timer_config[tim].max;
TIM_CHAN(tim, channel) = value;
/* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);
#ifdef MODULE_PERIPH_TIMER_PERIODIC #ifdef MODULE_PERIPH_TIMER_PERIODIC
if (dev(tim)->ARR == TIM_CHAN(tim, channel)) { if (dev(tim)->ARR == TIM_CHAN(tim, channel)) {
dev(tim)->ARR = timer_config[tim].max; dev(tim)->ARR = timer_config[tim].max;
} }
#endif #endif
/* clear spurious IRQs */
dev(tim)->SR &= ~(TIM_SR_CC1IF << channel);
TIM_CHAN(tim, channel) = value;
/* enable IRQ */
dev(tim)->DIER |= (TIM_DIER_CC1IE << channel);
/* calculate time till timeout */ /* calculate time till timeout */
value = (value - dev(tim)->CNT) & timer_config[tim].max; value = (value - dev(tim)->CNT) & timer_config[tim].max;