cpu/sam0_common/../rtt: correcly clear flags.

[jcarrano]

Contribution description

The INTFLAGS register is cleared by writing a 1 to the corresponding interrupt flag bit. From the samr21's manual:

Writing a zero to this bit has no effect.
Writing a one to this bit clears the Compare 0 interrupt flag.

This is a common pattern in flag registers.

This RTT driver is using or-equal to clear the flags, which means it can possibly clear other interrupts. There's a small chance that one event is missed if it happens very close to another event.

Testing procedure

I did not test this, I just read the manual and did what it says. I'm opening this PR because I think it does not make sense to have an open issue whose fix is only two lines.

Issues/PRs references

Fixes #10351.

[benpicco]

Good catch!
But there is more:

diff --git a/cpu/sam0_common/periph/rtt.c b/cpu/sam0_common/periph/rtt.c
index 3f7de9049..34f05e52f 100644
--- a/cpu/sam0_common/periph/rtt.c
+++ b/cpu/sam0_common/periph/rtt.c
@@ -98,7 +98,7 @@ void rtt_init(void)
     _wait_syncbusy();
 
     /* initially clear flag */
-    RTC->MODE0.INTFLAG.reg |= RTC_MODE0_INTFLAG_CMP0
+    RTC->MODE0.INTFLAG.reg = RTC_MODE0_INTFLAG_CMP0
                            |  RTC_MODE0_INTFLAG_OVF;
 
     NVIC_EnableIRQ(RTC_IRQn);
@@ -116,12 +116,12 @@ void rtt_set_overflow_cb(rtt_cb_t cb, void *arg)
     _overflow_arg = arg;
 
     /* enable overflow interrupt */
-    RTC->MODE0.INTENSET.bit.OVF = 1;
+    RTC->MODE0.INTENSET.reg = RTC_MODE0_INTENSET_OVF;
 }
 void rtt_clear_overflow_cb(void)
 {
     /* disable overflow interrupt */
-    RTC->MODE0.INTENCLR.bit.OVF = 1;
+    RTC->MODE0.INTENCLR.reg = RTC_MODE0_INTENCLR_OVF;
 }
 
 uint32_t rtt_get_counter(void)
@@ -158,14 +158,14 @@ void rtt_set_alarm(uint32_t alarm, rtt_cb_t cb, void *arg)
     _wait_syncbusy();
 
     /* enable compare interrupt and clear flag */
-    RTC->MODE0.INTFLAG.reg |= RTC_MODE0_INTFLAG_CMP0;
-    RTC->MODE0.INTENSET.reg |= RTC_MODE0_INTENSET_CMP0;
+    RTC->MODE0.INTFLAG.reg  = RTC_MODE0_INTFLAG_CMP0;
+    RTC->MODE0.INTENSET.reg = RTC_MODE0_INTENSET_CMP0;
 }
 
 void rtt_clear_alarm(void)
 {
     /* clear compare interrupt */
-    RTC->MODE0.INTENCLR.bit.CMP0 = 1;
+    RTC->MODE0.INTENCLR.reg = RTC_MODE0_INTENCLR_CMP0;
 }
 
 void rtt_poweron(void)

Tested on samr21-xpro and same54-xpro.

[benpicco]

Thank you! You can squash already.

And better call the commit cpu/sam0_common: rtt: correcly clear flags, I don't know what those ../ are supposed to mean.

[benpicco]

Remedies the misguided use of those registers.

Tested both on samr21-xpro and same54-xpro with tests/periph_rtt.

[dylad]

Nice cleanup !
One last change remaining and I will test on SAML21 and SAML1X tonight or tomorrow.

[dylad]

ACK
Tested against SAML10/SAML11/SAML21/SAME54 + arduino-zero (SAMD21) with tests/periph_rtt
@jcarrano thanks for the fixup. Squash & go !