Some GPIO manipulations in matrix.c change to atomic. (#10491)

* Changed the processing of select_xxx()/unselect_xxx() in quantum/matrix.c to be atomic. * Changed the processing of select_xxx()/unselect_xxx() in quantum/split_common/matrix.c to be atomic. * update matrix.c * add ATOMIC_BLOCK_FORCEON macro to quantum/quantum_atomic_extend.h * quantum_atomic_extend.h's contents move into quantum.h * update ATOMIC_BLOCK_xxx for unknown platform * ATOMIC_BLOCK macro support PROTOCOL_ARM_ATSAM * Add Atomic Operation section in docs/internals_gpio_control.md
qmk · Oct 26, 2020 · 8337fcc · 8337fcc
1 parent 0fde6c3
commit 8337fcc
Show file tree

Hide file tree

Showing 4 changed files with 124 additions and 20 deletions.
diff --git a/docs/internals_gpio_control.md b/docs/internals_gpio_control.md
@@ -21,3 +21,22 @@ The following functions can provide basic control of GPIOs and are found in `qua
 ## Advanced Settings :id=advanced-settings
 
 Each microcontroller can have multiple advanced settings regarding its GPIO. This abstraction layer does not limit the use of architecture-specific functions. Advanced users should consult the datasheet of their desired device and include any needed libraries. For AVR, the standard avr/io.h library is used; for STM32, the ChibiOS [PAL library](http://chibios.sourceforge.net/docs3/hal/group___p_a_l.html) is used.
+
+## Atomic Operation
+
+The above functions are not always guaranteed to work atomically. Therefore, if you want to prevent interruptions in the middle of operations when using multiple combinations of the above functions, use the following `ATOMIC_BLOCK_FORCEON` macro.
+
+eg.
+```c
+void some_function() {
+     // some process
+     ATOMIC_BLOCK_FORCEON {
+        // Atomic Processing
+     }
+     // some process
+}
+```
+
+`ATOMIC_BLOCK_FORCEON` forces interrupts to be disabled before the block is executed, without regard to whether they are enabled or disabled. Then, after the block is executed, the interrupt is enabled.
+
+Note that `ATOMIC_BLOCK_FORCEON` can therefore be used if you know that interrupts are enabled before the execution of the block, or if you know that it is OK to enable interrupts at the completion of the block.
diff --git a/quantum/matrix.c b/quantum/matrix.c
@@ -32,6 +32,19 @@ static const pin_t col_pins[MATRIX_COLS] = MATRIX_COL_PINS;
 extern matrix_row_t raw_matrix[MATRIX_ROWS];  // raw values
 extern matrix_row_t matrix[MATRIX_ROWS];      // debounced values
 
+static inline void setPinOutput_writeLow(pin_t pin) {
+    ATOMIC_BLOCK_FORCEON {
+        setPinOutput(pin);
+        writePinLow(pin);
+    }
+}
+
+static inline void setPinInputHigh_atomic(pin_t pin) {
+    ATOMIC_BLOCK_FORCEON {
+        setPinInputHigh(pin);
+    }
+}
+
 // matrix code
 
 #ifdef DIRECT_PINS
@@ -70,22 +83,23 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
 #    if (DIODE_DIRECTION == COL2ROW)
 
 static void select_row(uint8_t row) {
-    setPinOutput(row_pins[row]);
-    writePinLow(row_pins[row]);
+    setPinOutput_writeLow(row_pins[row]);
 }
 
-static void unselect_row(uint8_t row) { setPinInputHigh(row_pins[row]); }
+static void unselect_row(uint8_t row) {
+    setPinInputHigh_atomic(row_pins[row]);
+}
 
 static void unselect_rows(void) {
     for (uint8_t x = 0; x < MATRIX_ROWS; x++) {
-        setPinInputHigh(row_pins[x]);
+        setPinInputHigh_atomic(row_pins[x]);
     }
 }
 
 static void init_pins(void) {
     unselect_rows();
     for (uint8_t x = 0; x < MATRIX_COLS; x++) {
-        setPinInputHigh(col_pins[x]);
+        setPinInputHigh_atomic(col_pins[x]);
     }
 }
 
@@ -120,22 +134,23 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
 #    elif (DIODE_DIRECTION == ROW2COL)
 
 static void select_col(uint8_t col) {
-    setPinOutput(col_pins[col]);
-    writePinLow(col_pins[col]);
+    setPinOutput_writeLow(col_pins[col]);
 }
 
-static void unselect_col(uint8_t col) { setPinInputHigh(col_pins[col]); }
+static void unselect_col(uint8_t col) {
+    setPinInputHigh_atomic(col_pins[col]);
+}
 
 static void unselect_cols(void) {
     for (uint8_t x = 0; x < MATRIX_COLS; x++) {
-        setPinInputHigh(col_pins[x]);
+        setPinInputHigh_atomic(col_pins[x]);
     }
 }
 
 static void init_pins(void) {
     unselect_cols();
     for (uint8_t x = 0; x < MATRIX_ROWS; x++) {
-        setPinInputHigh(row_pins[x]);
+        setPinInputHigh_atomic(row_pins[x]);
     }
 }
 

diff --git a/quantum/quantum.h b/quantum/quantum.h
@@ -220,6 +220,61 @@ typedef ioline_t pin_t;
 #    define togglePin(pin) palToggleLine(pin)
 #endif
 
+// Atomic macro to help make GPIO and other controls atomic.
+#ifdef IGNORE_ATOMIC_BLOCK
+/* do nothing atomic macro */
+#    define ATOMIC_BLOCK for (uint8_t __ToDo = 1; __ToDo; __ToDo = 0)
+#    define ATOMIC_BLOCK_RESTORESTATE ATOMIC_BLOCK
+#    define ATOMIC_BLOCK_FORCEON ATOMIC_BLOCK
+
+#elif defined(__AVR__)
+/* atomic macro for AVR */
+#    include <util/atomic.h>
+
+#    define ATOMIC_BLOCK_RESTORESTATE ATOMIC_BLOCK(ATOMIC_RESTORESTATE)
+#    define ATOMIC_BLOCK_FORCEON ATOMIC_BLOCK(ATOMIC_FORCEON)
+
+#elif defined(PROTOCOL_CHIBIOS) || defined(PROTOCOL_ARM_ATSAM)
+/* atomic macro for ChibiOS / ARM ATSAM */
+#    if defined(PROTOCOL_ARM_ATSAM)
+#        include "arm_atsam_protocol.h"
+#    endif
+
+static __inline__ uint8_t __interrupt_disable__(void) {
+#    if defined(PROTOCOL_CHIBIOS)
+    chSysLock();
+#    endif
+#    if defined(PROTOCOL_ARM_ATSAM)
+    __disable_irq();
+#    endif
+    return 1;
+}
+
+static __inline__ void __interrupt_enable__(const uint8_t *__s) {
+#    if defined(PROTOCOL_CHIBIOS)
+    chSysUnlock();
+#    endif
+#    if defined(PROTOCOL_ARM_ATSAM)
+    __enable_irq();
+#    endif
+    __asm__ volatile("" ::: "memory");
+    (void)__s;
+}
+
+#    define ATOMIC_BLOCK(type) for (type, __ToDo = __interrupt_disable__(); __ToDo; __ToDo = 0)
+#    define ATOMIC_FORCEON uint8_t sreg_save __attribute__((__cleanup__(__interrupt_enable__))) = 0
+
+#    define ATOMIC_BLOCK_RESTORESTATE _Static_assert(0, "ATOMIC_BLOCK_RESTORESTATE dose not implement")
+#    define ATOMIC_BLOCK_FORCEON ATOMIC_BLOCK(ATOMIC_FORCEON)
+
+/* Other platform */
+#else
+
+#    define ATOMIC_BLOCK_RESTORESTATE _Static_assert(0, "ATOMIC_BLOCK_RESTORESTATE dose not implement")
+#    define ATOMIC_BLOCK_FORCEON _Static_assert(0, "ATOMIC_BLOCK_FORCEON dose not implement")
+
+#endif
+
 #define SEND_STRING(string) send_string_P(PSTR(string))
 #define SEND_STRING_DELAY(string, interval) send_string_with_delay_P(PSTR(string), interval)
 

diff --git a/quantum/split_common/matrix.c b/quantum/split_common/matrix.c
@@ -45,6 +45,19 @@ uint8_t thisHand, thatHand;
 // user-defined overridable functions
 __attribute__((weak)) void matrix_slave_scan_user(void) {}
 
+static inline void setPinOutput_writeLow(pin_t pin) {
+    ATOMIC_BLOCK_FORCEON {
+        setPinOutput(pin);
+        writePinLow(pin);
+    }
+}
+
+static inline void setPinInputHigh_atomic(pin_t pin) {
+    ATOMIC_BLOCK_FORCEON {
+        setPinInputHigh(pin);
+    }
+}
+
 // matrix code
 
 #ifdef DIRECT_PINS
@@ -83,22 +96,23 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
 #    if (DIODE_DIRECTION == COL2ROW)
 
 static void select_row(uint8_t row) {
-    setPinOutput(row_pins[row]);
-    writePinLow(row_pins[row]);
+    setPinOutput_writeLow(row_pins[row]);
 }
 
-static void unselect_row(uint8_t row) { setPinInputHigh(row_pins[row]); }
+static void unselect_row(uint8_t row) {
+    setPinInputHigh_atomic(row_pins[row]);
+}
 
 static void unselect_rows(void) {
     for (uint8_t x = 0; x < ROWS_PER_HAND; x++) {
-        setPinInputHigh(row_pins[x]);
+        setPinInputHigh_atomic(row_pins[x]);
     }
 }
 
 static void init_pins(void) {
     unselect_rows();
     for (uint8_t x = 0; x < MATRIX_COLS; x++) {
-        setPinInputHigh(col_pins[x]);
+        setPinInputHigh_atomic(col_pins[x]);
     }
 }
 
@@ -133,22 +147,23 @@ static bool read_cols_on_row(matrix_row_t current_matrix[], uint8_t current_row)
 #    elif (DIODE_DIRECTION == ROW2COL)
 
 static void select_col(uint8_t col) {
-    setPinOutput(col_pins[col]);
-    writePinLow(col_pins[col]);
+    setPinOutput_writeLow(col_pins[col]);
 }
 
-static void unselect_col(uint8_t col) { setPinInputHigh(col_pins[col]); }
+static void unselect_col(uint8_t col) {
+    setPinInputHigh_atomic(col_pins[col]);
+}
 
 static void unselect_cols(void) {
     for (uint8_t x = 0; x < MATRIX_COLS; x++) {
-        setPinInputHigh(col_pins[x]);
+        setPinInputHigh_atomic(col_pins[x]);
     }
 }
 
 static void init_pins(void) {
     unselect_cols();
     for (uint8_t x = 0; x < ROWS_PER_HAND; x++) {
-        setPinInputHigh(row_pins[x]);
+        setPinInputHigh_atomic(row_pins[x]);
     }
 }