Skip to content

Commit

Permalink
Fix clearing of TIMx_SR register
Browse files Browse the repository at this point in the history
To clear specific bits in TIMx_SR registers, libmaple code
used to do

  TIMx_SR &= ~(events_handled);  // wrong

This is wrong. If a new event occurs between the read from TIMx_SR
and the write to TIMx_SR, that new event will be cleared immediately
without ever being noticed or handled.

A better way to clear specific bits is

  TIMx_SR = ~(events_handled);  // good

This writes '0' to the bits to be cleared, and '1' to all other bits.
The hardware will not allow software to change any SR bits from 0 to 1;
only hardware events can do that. So bits written as '1' are effectively
ignored and bits written as '0' are cleared. Exactly what we need.
  • Loading branch information
jorisvr committed Aug 20, 2018
1 parent 6a9c795 commit 73e268d
Showing 1 changed file with 6 additions and 6 deletions.
12 changes: 6 additions & 6 deletions STM32F1/system/libmaple/timer_private.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ static inline __always_inline void dispatch_single_irq(timer_dev *dev,
void (*handler)(void) = dev->handlers[iid];
if (handler) {
handler();
regs->SR &= ~irq_mask;
regs->SR = ~irq_mask;
}
}
}
Expand Down Expand Up @@ -165,7 +165,7 @@ static inline __always_inline void dispatch_adv_trg_com(timer_dev *dev) {
handle_irq(dsr, TIMER_SR_TIF, hs, TIMER_TRG_INTERRUPT, handled);
handle_irq(dsr, TIMER_SR_COMIF, hs, TIMER_COM_INTERRUPT, handled);

regs->SR &= ~handled;
regs->SR = ~handled;
}

static inline __always_inline void dispatch_adv_cc(timer_dev *dev) {
Expand All @@ -179,7 +179,7 @@ static inline __always_inline void dispatch_adv_cc(timer_dev *dev) {
handle_irq(dsr, TIMER_SR_CC2IF, hs, TIMER_CC2_INTERRUPT, handled);
handle_irq(dsr, TIMER_SR_CC1IF, hs, TIMER_CC1_INTERRUPT, handled);

regs->SR &= ~handled;
regs->SR = ~handled;
}

static inline __always_inline void dispatch_general(timer_dev *dev) {
Expand All @@ -195,7 +195,7 @@ static inline __always_inline void dispatch_general(timer_dev *dev) {
handle_irq(dsr, TIMER_SR_CC1IF, hs, TIMER_CC1_INTERRUPT, handled);
handle_irq(dsr, TIMER_SR_UIF, hs, TIMER_UPDATE_INTERRUPT, handled);

regs->SR &= ~handled;
regs->SR = ~handled;
}

/* On F1 (XL-density), F2, and F4, TIM9 and TIM12 are restricted
Expand All @@ -211,7 +211,7 @@ static inline __always_inline void dispatch_tim_9_12(timer_dev *dev) {
handle_irq(dsr, TIMER_SR_CC1IF, hs, TIMER_CC1_INTERRUPT, handled);
handle_irq(dsr, TIMER_SR_UIF, hs, TIMER_UPDATE_INTERRUPT, handled);

regs->SR &= ~handled;
regs->SR = ~handled;
}

/* On F1 (XL-density), F2, and F4, timers 10, 11, 13, and 14 are
Expand All @@ -225,7 +225,7 @@ static inline __always_inline void dispatch_tim_10_11_13_14(timer_dev *dev) {
handle_irq(dsr, TIMER_SR_CC1IF, hs, TIMER_CC1_INTERRUPT, handled);
handle_irq(dsr, TIMER_SR_UIF, hs, TIMER_UPDATE_INTERRUPT, handled);

regs->SR &= ~handled;
regs->SR = ~handled;
}

static inline __always_inline void dispatch_basic(timer_dev *dev) {
Expand Down

0 comments on commit 73e268d

Please sign in to comment.