From dfa342b5f8548fc0d5ac0fbc0cdc70ff335a44c4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Joakim=20Nohlg=C3=A5rd?= <joakim.nohlgard@eistec.se>
Date: Wed, 21 Mar 2018 13:19:11 +0100
Subject: [PATCH] cpu/samd21: Avoid clearing interrupt bits unintentionally
The INTENSET, INTENCLR, INTFLAG registers are write-1-to-confirm
registers, so writing zeroes will not affect anything, on the other
hand, a compiler generated read-modify-write cycle may unintentionally
affect more bits than the one being set. Avoid by using direct
assignment instead of or-assignment (|=) or bitfield writes (.bit.xxx=).
---
cpu/samd21/periph/timer.c | 52 +++++++++++++++++++--------------------
1 file changed, 26 insertions(+), 26 deletions(-)
diff --git a/cpu/samd21/periph/timer.c b/cpu/samd21/periph/timer.c
index d4d530bcba..689d9bc733 100644
--- a/cpu/samd21/periph/timer.c
+++ b/cpu/samd21/periph/timer.c
@@ -142,14 +142,14 @@ int timer_set_absolute(tim_t dev, int channel, unsigned int value)
/* set timeout value */
switch (channel) {
case 0:
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
TIMER_0_DEV.CC[0].reg = value;
- TIMER_0_DEV.INTENSET.bit.MC0 = 1;
+ TIMER_0_DEV.INTENSET.reg = TC_INTENSET_MC0;
break;
case 1:
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
TIMER_0_DEV.CC[1].reg = value;
- TIMER_0_DEV.INTENSET.bit.MC1 = 1;
+ TIMER_0_DEV.INTENSET.reg = TC_INTENSET_MC1;
break;
default:
return -1;
@@ -161,14 +161,14 @@ int timer_set_absolute(tim_t dev, int channel, unsigned int value)
/* set timeout value */
switch (channel) {
case 0:
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
TIMER_1_DEV.CC[0].reg = value;
- TIMER_1_DEV.INTENSET.bit.MC0 = 1;
+ TIMER_1_DEV.INTENSET.reg = TC_INTENSET_MC0;
break;
case 1:
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
TIMER_1_DEV.CC[1].reg = value;
- TIMER_1_DEV.INTENSET.bit.MC1 = 1;
+ TIMER_1_DEV.INTENSET.reg = TC_INTENSET_MC1;
break;
default:
return -1;
@@ -191,12 +191,12 @@ int timer_clear(tim_t dev, int channel)
case TIMER_0:
switch (channel) {
case 0:
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
- TIMER_0_DEV.INTENCLR.bit.MC0 = 1;
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
+ TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
break;
case 1:
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
- TIMER_0_DEV.INTENCLR.bit.MC1 = 1;
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
+ TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
break;
default:
return -1;
@@ -207,12 +207,12 @@ int timer_clear(tim_t dev, int channel)
case TIMER_1:
switch (channel) {
case 0:
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
- TIMER_1_DEV.INTENCLR.bit.MC0 = 1;
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
+ TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
break;
case 1:
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
- TIMER_1_DEV.INTENCLR.bit.MC1 = 1;
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
+ TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
break;
default:
return -1;
@@ -309,16 +309,16 @@ static inline void _irq_enable(tim_t dev)
void TIMER_0_ISR(void)
{
if (TIMER_0_DEV.INTFLAG.bit.MC0 && TIMER_0_DEV.INTENSET.bit.MC0) {
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
+ TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
if(config[TIMER_0].cb) {
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
- TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
config[TIMER_0].cb(config[TIMER_0].arg, 0);
}
}
- else if (TIMER_0_DEV.INTFLAG.bit.MC1 && TIMER_0_DEV.INTENSET.bit.MC1) {
+ if (TIMER_0_DEV.INTFLAG.bit.MC1 && TIMER_0_DEV.INTENSET.bit.MC1) {
+ TIMER_0_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
+ TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
if(config[TIMER_0].cb) {
- TIMER_0_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
- TIMER_0_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
config[TIMER_0].cb(config[TIMER_0].arg, 1);
}
}
@@ -332,16 +332,16 @@ void TIMER_0_ISR(void)
void TIMER_1_ISR(void)
{
if (TIMER_1_DEV.INTFLAG.bit.MC0 && TIMER_1_DEV.INTENSET.bit.MC0) {
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC0;
+ TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
if (config[TIMER_1].cb) {
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC0;
- TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC0;
config[TIMER_1].cb(config[TIMER_1].arg, 0);
}
}
- else if (TIMER_1_DEV.INTFLAG.bit.MC1 && TIMER_1_DEV.INTENSET.bit.MC1) {
+ if (TIMER_1_DEV.INTFLAG.bit.MC1 && TIMER_1_DEV.INTENSET.bit.MC1) {
+ TIMER_1_DEV.INTFLAG.reg = TC_INTFLAG_MC1;
+ TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
if(config[TIMER_1].cb) {
- TIMER_1_DEV.INTFLAG.reg |= TC_INTFLAG_MC1;
- TIMER_1_DEV.INTENCLR.reg = TC_INTENCLR_MC1;
config[TIMER_1].cb(config[TIMER_1].arg, 1);
}
}
--
GitLab