[Core] Split ChibiOS usart split driver in protocol and hardware driver part

builddefs/common_features.mk

@@ -583,6 +583,14 @@ ifneq ($(strip $(DEBOUNCE_TYPE)), custom)
     QUANTUM_SRC += $(QUANTUM_DIR)/debounce/$(strip $(DEBOUNCE_TYPE)).c
 endif
 
+
+VALID_SERIAL_DRIVER_TYPES := bitbang usart
+
+SERIAL_DRIVER ?= bitbang
+ifeq ($(filter $(SERIAL_DRIVER),$(VALID_SERIAL_DRIVER_TYPES)),)
+    $(call CATASTROPHIC_ERROR,Invalid SERIAL_DRIVER,SERIAL_DRIVER="$(SERIAL_DRIVER)" is not a valid SERIAL driver)
+endif
+
 ifeq ($(strip $(SPLIT_KEYBOARD)), yes)
     POST_CONFIG_H += $(QUANTUM_DIR)/split_common/post_config.h
     OPT_DEFS += -DSPLIT_KEYBOARD
@@ -607,11 +615,11 @@ ifeq ($(strip $(SPLIT_KEYBOARD)), yes)
             endif
         endif
 
-        SERIAL_DRIVER ?= bitbang
         OPT_DEFS += -DSERIAL_DRIVER_$(strip $(shell echo $(SERIAL_DRIVER) | tr '[:lower:]' '[:upper:]'))
         ifeq ($(strip $(SERIAL_DRIVER)), bitbang)
             QUANTUM_LIB_SRC += serial.c
         else
+            QUANTUM_LIB_SRC += serial_protocol.c
             QUANTUM_LIB_SRC += serial_$(strip $(SERIAL_DRIVER)).c
         endif
     endif

docs/serial_driver.md

@@ -254,6 +254,16 @@ This is the default time window in milliseconds in which a successful communicat
 
 <hr>
 
+## Troubleshooting
+
+If you're having issues withe serial communication, you can enable debug messages that will give you insights which part of the communication failed. The enable these messages add to your keyboards `config.h` file:
+
+```c
+#define SERIAL_DEBUG
+```
+
+?> The messages will be printed out to the `CONSOLE` output. For additional information, refer to [Debugging/Troubleshooting QMK](faq_debug.md).
+
 ## Alternate Functions for selected STM32 MCUs
 
 Pins for USART Peripherals with 

drivers/serial.h

@@ -27,3 +27,13 @@ void soft_serial_initiator_init(void);
 void soft_serial_target_init(void);
 
 bool soft_serial_transaction(int sstd_index);
+
+#ifdef SERIAL_DEBUG
+#    include <debug.h>
+#    include <print.h>
+#    define serial_dprintf(...) dprintf(__VA_ARGS__)
+#else
+#    define serial_dprintf(...) \
+        do {                    \
+        } while (0)
+#endif

platforms/chibios/drivers/serial.c

@@ -233,7 +233,7 @@ static inline bool initiate_transaction(uint8_t sstd_index) {
     // check if the slave is present
     if (serial_read_pin()) {
         // slave failed to pull the line low, assume not present
-        dprintf("serial::NO_RESPONSE\n");
+        serial_dprintf("serial::NO_RESPONSE\n");
         chSysUnlock();
         return false;
     }
@@ -269,7 +269,7 @@ static inline bool initiate_transaction(uint8_t sstd_index) {
     serial_delay();
 
     if ((checksum_computed) != (checksum_received)) {
-        dprintf("serial::FAIL[%u,%u,%u]\n", checksum_computed, checksum_received, sstd_index);
+        serial_dprintf("serial::FAIL[%u,%u,%u]\n", checksum_computed, checksum_received, sstd_index);
         serial_output();
         serial_high();
 

platforms/chibios/drivers/serial_protocol.c

@@ -0,0 +1,164 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <ch.h>
+
+#include "quantum.h"
+#include "serial.h"
+#include "serial_protocol.h"
+#include "printf.h"
+#include "synchronization_util.h"
+
+static inline bool initiate_transaction(uint8_t transaction_id);
+static inline bool react_to_transaction(void);
+
+/**
+ * @brief This thread runs on the slave and responds to transactions initiated
+ * by the master.
+ */
+static THD_WORKING_AREA(waSlaveThread, 1024);
+static THD_FUNCTION(SlaveThread, arg) {
+    (void)arg;
+    chRegSetThreadName("split_protocol_tx_rx");
+
+    while (true) {
+        split_shared_memory_lock();
+        if (unlikely(!react_to_transaction())) {
+            /* Clear the receive queue, to start with a clean slate.
+             * Parts of failed transactions or spurious bytes could still be in it. */
+            serial_transport_driver_clear();
+        }
+        split_shared_memory_unlock();
+    }
+}
+
+/**
+ * @brief Slave specific initializations.
+ */
+void soft_serial_target_init(void) {
+    serial_transport_driver_slave_init();
+
+    /* Start transport thread. */
+    chThdCreateStatic(waSlaveThread, sizeof(waSlaveThread), HIGHPRIO, SlaveThread, NULL);
+}
+
+/**
+ * @brief Master specific initializations.
+ */
+void soft_serial_initiator_init(void) {
+    serial_transport_driver_master_init();
+}
+
+/**
+ * @brief React to transactions started by the master.
+ */
+static inline bool react_to_transaction(void) {
+    uint8_t transaction_id = 0;
+    /* Wait until there is a transaction for us. */
+    if (unlikely(!serial_transport_receive_blocking(&transaction_id, sizeof(transaction_id)))) {
+        return false;
+    }
+
+    /* Sanity check that we are actually responding to a valid transaction. */
+    if (unlikely(transaction_id >= NUM_TOTAL_TRANSACTIONS)) {
+        return false;
+    }
+
+    split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
+
+    /* Send back the handshake which is XORed as a simple checksum,
+     to signal that the slave is ready to receive possible transaction buffers  */
+    transaction_id ^= NUM_TOTAL_TRANSACTIONS;
+    if (unlikely(!serial_transport_send(&transaction_id, sizeof(transaction_id)))) {
+        return false;
+    }
+
+    /* Receive transaction buffer from the master. If this transaction requires it.*/
+    if (transaction->initiator2target_buffer_size) {
+        if (unlikely(!serial_transport_receive(split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size))) {
+            return false;
+        }
+    }
+
+    /* Allow any slave processing to occur. */
+    if (transaction->slave_callback) {
+        transaction->slave_callback(transaction->initiator2target_buffer_size, split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size, split_trans_target2initiator_buffer(transaction));
+    }
+
+    /* Send transaction buffer to the master. If this transaction requires it. */
+    if (transaction->target2initiator_buffer_size) {
+        if (unlikely(!serial_transport_send(split_trans_target2initiator_buffer(transaction), transaction->target2initiator_buffer_size))) {
+            return false;
+        }
+    }
+
+    return true;
+}
+
+/**
+ * @brief Start transaction from the master half to the slave half.
+ *
+ * @param index Transaction Table index of the transaction to start.
+ * @return bool Indicates success of transaction.
+ */
+bool soft_serial_transaction(int index) {
+    split_shared_memory_lock();
+    bool result = initiate_transaction((uint8_t)index);
+    split_shared_memory_unlock();
+
+    if (unlikely(!result)) {
+        /* Clear the receive queue, to start with a clean slate.
+         * Parts of failed transactions or spurious bytes could still be in it. */
+        serial_transport_driver_clear();
+    }
+
+    return result;
+}
+
+/**
+ * @brief Initiate transaction to slave half.
+ */
+static inline bool initiate_transaction(uint8_t transaction_id) {
+    /* Sanity check that we are actually starting a valid transaction. */
+    if (unlikely(transaction_id >= NUM_TOTAL_TRANSACTIONS)) {
+        serial_dprintf("SPLIT: illegal transaction id\n");
+        return false;
+    }
+
+    split_transaction_desc_t* transaction = &split_transaction_table[transaction_id];
+
+    /* Send transaction table index to the slave, which doubles as basic handshake token. */
+    if (unlikely(!serial_transport_send(&transaction_id, sizeof(transaction_id)))) {
+        serial_dprintf("SPLIT: sending handshake failed\n");
+        return false;
+    }
+
+    uint8_t transaction_id_shake = 0xFF;
+
+    /* Which we always read back first so that we can error out correctly.
+     *   - due to the half duplex limitations on return codes, we always have to read *something*.
+     *   - without the read, write only transactions *always* succeed, even during the boot process where the slave is not ready.
+     */
+    if (unlikely(!serial_transport_receive(&transaction_id_shake, sizeof(transaction_id_shake)) || (transaction_id_shake != (transaction_id ^ NUM_TOTAL_TRANSACTIONS)))) {
+        serial_dprintf("SPLIT: receiving handshake failed\n");
+        return false;
+    }
+
+    /* Send transaction buffer to the slave. If this transaction requires it. */
+    if (transaction->initiator2target_buffer_size) {
+        if (unlikely(!serial_transport_send(split_trans_initiator2target_buffer(transaction), transaction->initiator2target_buffer_size))) {
+            serial_dprintf("SPLIT: sending buffer failed\n");
+            return false;
+        }
+    }
+
+    /* Receive transaction buffer from the slave. If this transaction requires it. */
+    if (transaction->target2initiator_buffer_size) {
+        if (unlikely(!serial_transport_receive(split_trans_target2initiator_buffer(transaction), transaction->target2initiator_buffer_size))) {
+            serial_dprintf("SPLIT: receiving buffer failed\n");
+            return false;
+        }
+    }
+
+    return true;
+}

platforms/chibios/drivers/serial_protocol.h

@@ -0,0 +1,49 @@
+// Copyright 2022 Stefan Kerkmann
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <stddef.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#pragma once
+
+/**
+ * @brief Clears any intermediate sending or receiving state of the driver to a known good
+ * state. This happens after errors in the middle of transactions, to start with
+ * a clean slate.
+ */
+void serial_transport_driver_clear(void);
+
+/**
+ * @brief Driver specific initialization on the slave half.
+ */
+void serial_transport_driver_slave_init(void);
+
+/**
+ * @brief Driver specific specific initialization on the master half.
+ */
+void serial_transport_driver_master_init(void);
+
+/**
+ * @brief  Blocking receive of size * bytes.
+ *
+ * @return true Receive success.
+ * @return false Receive failed, e.g. by bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_receive(uint8_t* destination, const size_t size);
+
+/**
+ * @brief Blocking receive of size * bytes with an implicitly defined timeout.
+ *
+ * @return true Receive success.
+ * @return false Receive failed, e.g. by timeout or bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_receive_blocking(uint8_t* destination, const size_t size);
+
+/**
+ * @brief Blocking send of buffer with timeout.
+ *
+ * @return true Send success.
+ * @return false Send failed, e.g. by timeout or bit errors.
+ */
+bool __attribute__((nonnull, hot)) serial_transport_send(const uint8_t* source, const size_t size);