Fix atomicity issues in SPI::beginTransaction and SPI::endTransaction (Andrew Kroll)

cmaglie · cmaglie · commit a9735bf91fda · 2014-11-25T15:56:11.000+01:00
Previously, it could happen that SPI::beginTransaction was
interrupted by an ISR, while it is changing the SPI_AVR_EIMSK
register or interruptSave variable (it seems that there is
a small window after changing SPI_AVR_EIMSK where an interrupt
might still occur). If this happens, interruptSave is overwritten
with an invalid value, permanently disabling the pin interrupts.

To prevent this, disable interrupts globally while changing
these values.
diff --git a/hardware/arduino/avr/libraries/SPI/SPI.h b/hardware/arduino/avr/libraries/SPI/SPI.h
@@ -23,6 +23,13 @@
 // SPI_HAS_NOTUSINGINTERRUPT means that SPI has notUsingInterrupt() method
 #define SPI_HAS_NOTUSINGINTERRUPT 1
 
+// SPI_ATOMIC_VERSION means that SPI has atomicity fixes and what version.
+// This way when there is a bug fix you can check this define to alert users
+// of your code if it uses better version of this library.
+// This also implies everything that SPI_HAS_TRANSACTION as documented above is
+// available too.
+#define SPI_ATOMIC_VERSION 1
+
 // Uncomment this line to add detection of mismatched begin/end transactions.
 // A mismatch occurs if other libraries fail to use SPI.endTransaction() for
 // each SPI.beginTransaction().  Connect an LED to this pin.  The LED will turn
@@ -170,24 +177,29 @@ class SPIClass {
   // and configure the correct settings.
   inline static void beginTransaction(SPISettings settings) {
     if (interruptMode > 0) {
+      uint8_t sreg = SREG;
+      noInterrupts();
+
       #ifdef SPI_AVR_EIMSK
       if (interruptMode == 1) {
         interruptSave = SPI_AVR_EIMSK;
         SPI_AVR_EIMSK &= ~interruptMask;
+        SREG = sreg;
       } else
       #endif
       {
-        interruptSave = SREG;
-        cli();
+        interruptSave = sreg;
       }
     }
+
     #ifdef SPI_TRANSACTION_MISMATCH_LED
     if (inTransactionFlag) {
       pinMode(SPI_TRANSACTION_MISMATCH_LED, OUTPUT);
       digitalWrite(SPI_TRANSACTION_MISMATCH_LED, HIGH);
     }
     inTransactionFlag = 1;
     #endif
+
     SPCR = settings.spcr;
     SPSR = settings.spsr;
   }
@@ -253,10 +265,16 @@ class SPIClass {
     }
     inTransactionFlag = 0;
     #endif
+
     if (interruptMode > 0) {
+      #ifdef SPI_AVR_EIMSK
+      uint8_t sreg = SREG;
+      #endif
+      noInterrupts();
       #ifdef SPI_AVR_EIMSK
       if (interruptMode == 1) {
         SPI_AVR_EIMSK = interruptSave;
+        SREG = sreg;
       } else
       #endif
       {
