~ruther/qmk_firmware

7712a286dccea029785976311433cf8673594f6f — Stefan Kerkmann 3 years ago 176ab14
[Core] Use a mutex guard for split shared memory (#16647)

M platforms/chibios/drivers/serial.c => platforms/chibios/drivers/serial.c +20 -9
@@ 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;
}

M platforms/chibios/drivers/serial_usart.c => platforms/chibios/drivers/serial_usart.c +9 -1
@@ 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;
}

/**

M platforms/chibios/platform.mk => platforms/chibios/platform.mk +5 -1
@@ 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


A platforms/chibios/synchronization_util.c => platforms/chibios/synchronization_util.c +26 -0
@@ 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

A platforms/synchronization_util.h => platforms/synchronization_util.h +14 -0
@@ 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

M quantum/split_common/transactions.c => quantum/split_common/transactions.c +8 -9
@@ 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) {