From 75a18e69f9d3b6dfa470d0a7dbd78408d6a1c496 Mon Sep 17 00:00:00 2001 From: Takeshi ISHII <2170248+mtei@users.noreply.github.com> Date: Mon, 26 Oct 2020 12:11:14 +0900 Subject: [PATCH] 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 --- docs/internals_gpio_control.md | 19 ++++++++++++ quantum/matrix.c | 35 +++++++++++++++------- quantum/quantum.h | 55 ++++++++++++++++++++++++++++++++++ quantum/split_common/matrix.c | 35 +++++++++++++++------- 4 files changed, 124 insertions(+), 20 deletions(-) diff --git a/docs/internals_gpio_control.md b/docs/internals_gpio_control.md index 48eaf8875b..7c2228949b 100644 --- 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 index c68c56cac2..cab0d2ddca 100644 --- 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 index a2c0ec9a28..42e8c00091 100644 --- 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 + +# 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 index 5bad9db08f..cd5a024c3d 100644 --- 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]); } }