summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStefan Kerkmann <karlk90@pm.me>2022-04-19 12:56:16 +0200
committerGitHub <noreply@github.com>2022-04-19 20:56:16 +1000
commit7712a286dccea029785976311433cf8673594f6f (patch)
tree775ae193b6fab345cce5e9046a3ff4116e83bc36
parent176ab1464964863637cc5d6a944dbdbe3f567aec (diff)
[Core] Use a mutex guard for split shared memory (#16647)
-rw-r--r--platforms/chibios/drivers/serial.c29
-rw-r--r--platforms/chibios/drivers/serial_usart.c10
-rw-r--r--platforms/chibios/platform.mk6
-rw-r--r--platforms/chibios/synchronization_util.c26
-rw-r--r--platforms/synchronization_util.h14
-rw-r--r--quantum/split_common/transactions.c17
6 files changed, 82 insertions, 20 deletions
diff --git a/platforms/chibios/drivers/serial.c b/platforms/chibios/drivers/serial.c
index bb7b3c0554..0cff057d1d 100644
--- a/platforms/chibios/drivers/serial.c
+++ b/platforms/chibios/drivers/serial.c
@@ -5,6 +5,7 @@
#include "quantum.h"
#include "serial.h"
#include "wait.h"
+#include "synchronization_util.h"
#include <hal.h>
@@ -86,7 +87,10 @@ static THD_FUNCTION(Thread1, arg) {
chRegSetThreadName("blinker");
while (true) {
palWaitLineTimeout(SOFT_SERIAL_PIN, TIME_INFINITE);
+
+ split_shared_memory_lock();
interrupt_handler(NULL);
+ split_shared_memory_unlock();
}
}
@@ -205,14 +209,9 @@ void interrupt_handler(void *arg) {
chSysUnlockFromISR();
}
-/////////
-// start transaction by initiator
-//
-// bool soft_serial_transaction(int sstd_index)
-//
-// this code is very time dependent, so we need to disable interrupts
-bool soft_serial_transaction(int sstd_index) {
+static inline bool initiate_transaction(uint8_t sstd_index) {
if (sstd_index > NUM_TOTAL_TRANSACTIONS) return false;
+
split_transaction_desc_t *trans = &split_transaction_table[sstd_index];
// TODO: remove extra delay between transactions
@@ -239,8 +238,7 @@ bool soft_serial_transaction(int sstd_index) {
return false;
}
- // if the slave is present syncronize with it
-
+ // if the slave is present synchronize with it
uint8_t checksum = 0;
// send data to the slave
serial_write_byte(sstd_index); // first chunk is transaction id
@@ -286,3 +284,16 @@ bool soft_serial_transaction(int sstd_index) {
chSysUnlock();
return true;
}
+
+/////////
+// start transaction by initiator
+//
+// bool soft_serial_transaction(int sstd_index)
+//
+// this code is very time dependent, so we need to disable interrupts
+bool soft_serial_transaction(int sstd_index) {
+ split_shared_memory_lock();
+ bool result = initiate_transaction((uint8_t)sstd_index);
+ split_shared_memory_unlock();
+ return result;
+}
diff --git a/platforms/chibios/drivers/serial_usart.c b/platforms/chibios/drivers/serial_usart.c
index 85c64214d1..e9fa4af7a3 100644
--- a/platforms/chibios/drivers/serial_usart.c
+++ b/platforms/chibios/drivers/serial_usart.c
@@ -15,6 +15,7 @@
*/
#include "serial_usart.h"
+#include "synchronization_util.h"
#if defined(SERIAL_USART_CONFIG)
static SerialConfig serial_config = SERIAL_USART_CONFIG;
@@ -173,6 +174,7 @@ static THD_FUNCTION(SlaveThread, arg) {
* Parts of failed transactions or spurious bytes could still be in it. */
usart_clear();
}
+ split_shared_memory_unlock();
}
}
@@ -200,6 +202,7 @@ static inline bool react_to_transactions(void) {
return false;
}
+ split_shared_memory_lock();
split_transaction_desc_t* trans = &split_transaction_table[sstd_index];
/* Send back the handshake which is XORed as a simple checksum,
@@ -254,7 +257,12 @@ bool soft_serial_transaction(int index) {
/* Clear the receive queue, to start with a clean slate.
* Parts of failed transactions or spurious bytes could still be in it. */
usart_clear();
- return initiate_transaction((uint8_t)index);
+
+ split_shared_memory_lock();
+ bool result = initiate_transaction((uint8_t)index);
+ split_shared_memory_unlock();
+
+ return result;
}
/**
diff --git a/platforms/chibios/platform.mk b/platforms/chibios/platform.mk
index 91dd0479bc..21751f23fd 100644
--- a/platforms/chibios/platform.mk
+++ b/platforms/chibios/platform.mk
@@ -265,7 +265,8 @@ PLATFORM_SRC = \
$(STREAMSSRC) \
$(CHIBIOS)/os/various/syscalls.c \
$(PLATFORM_COMMON_DIR)/syscall-fallbacks.c \
- $(PLATFORM_COMMON_DIR)/wait.c
+ $(PLATFORM_COMMON_DIR)/wait.c \
+ $(PLATFORM_COMMON_DIR)/synchronization_util.c
# Ensure the ASM files are not subjected to LTO -- it'll strip out interrupt handlers otherwise.
QUANTUM_LIB_SRC += $(STARTUPASM) $(PORTASM) $(OSALASM) $(PLATFORMASM)
@@ -420,6 +421,9 @@ LDFLAGS += $(SHARED_LDFLAGS) $(SHARED_LDSYMBOLS) $(TOOLCHAIN_LDFLAGS) $(TOOLCHA
# Tell QMK that we are hosting it on ChibiOS.
OPT_DEFS += -DPROTOCOL_CHIBIOS
+# ChibiOS supports synchronization primitives like a Mutex
+OPT_DEFS += -DPLATFORM_SUPPORTS_SYNCHRONIZATION
+
# Workaround to stop ChibiOS from complaining about new GCC -- it's been fixed for 7/8/9 already
OPT_DEFS += -DPORT_IGNORE_GCC_VERSION_CHECK=1
diff --git a/platforms/chibios/synchronization_util.c b/platforms/chibios/synchronization_util.c
new file mode 100644
index 0000000000..bc4a4e621f
--- /dev/null
+++ b/platforms/chibios/synchronization_util.c
@@ -0,0 +1,26 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include "synchronization_util.h"
+#include "ch.h"
+
+#if defined(SPLIT_KEYBOARD)
+static MUTEX_DECL(SPLIT_SHARED_MEMORY_MUTEX);
+
+/**
+ * @brief Acquire exclusive access to the split keyboard shared memory, by
+ * locking the mutex guarding it. If the mutex is already held, the calling
+ * thread will be suspended until the mutex currently owning thread releases the
+ * mutex again.
+ */
+void split_shared_memory_lock(void) {
+ chMtxLock(&SPLIT_SHARED_MEMORY_MUTEX);
+}
+
+/**
+ * @brief Release the split shared memory mutex that has been acquired before.
+ */
+void split_shared_memory_unlock(void) {
+ chMtxUnlock(&SPLIT_SHARED_MEMORY_MUTEX);
+}
+#endif
diff --git a/platforms/synchronization_util.h b/platforms/synchronization_util.h
new file mode 100644
index 0000000000..3730f271db
--- /dev/null
+++ b/platforms/synchronization_util.h
@@ -0,0 +1,14 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#pragma once
+
+#if defined(PLATFORM_SUPPORTS_SYNCHRONIZATION)
+# if defined(SPLIT_KEYBOARD)
+void split_shared_memory_lock(void);
+void split_shared_memory_unlock(void);
+# endif
+#else
+inline void split_shared_memory_lock(void){};
+inline void split_shared_memory_unlock(void){};
+#endif
diff --git a/quantum/split_common/transactions.c b/quantum/split_common/transactions.c
index 105bf918cb..9e3df534e3 100644
--- a/quantum/split_common/transactions.c
+++ b/quantum/split_common/transactions.c
@@ -23,8 +23,9 @@
#include "quantum.h"
#include "transactions.h"
#include "transport.h"
-#include "split_util.h"
#include "transaction_id_define.h"
+#include "split_util.h"
+#include "synchronization_util.h"
#define SYNC_TIMER_OFFSET 2
@@ -63,9 +64,7 @@ static bool transaction_handler_master(matrix_row_t master_matrix[], matrix_row_
}
}
bool this_okay = true;
- ATOMIC_BLOCK_FORCEON {
- this_okay = handler(master_matrix, slave_matrix);
- };
+ this_okay = handler(master_matrix, slave_matrix);
if (this_okay) return true;
}
dprintf("Failed to execute %s\n", prefix);
@@ -77,11 +76,11 @@ static bool transaction_handler_master(matrix_row_t master_matrix[], matrix_row_
if (!transaction_handler_master(master_matrix, slave_matrix, #prefix, &prefix##_handlers_master)) return false; \
} while (0)
-#define TRANSACTION_HANDLER_SLAVE(prefix) \
- do { \
- ATOMIC_BLOCK_FORCEON { \
- prefix##_handlers_slave(master_matrix, slave_matrix); \
- }; \
+#define TRANSACTION_HANDLER_SLAVE(prefix) \
+ do { \
+ split_shared_memory_lock(); \
+ prefix##_handlers_slave(master_matrix, slave_matrix); \
+ split_shared_memory_unlock(); \
} while (0)
inline static bool read_if_checksum_mismatch(int8_t trans_id_checksum, int8_t trans_id_retrieve, uint32_t *last_update, void *destination, const void *equiv_shmem, size_t length) {