[Core] Unite half-duplex and full-duplex serial drivers (#13081)

* Unite half-duplex and full-duplex serial driver.

* Add full duplex operation mode to the interrupt based driver
* Delete DMA UART based full duplex driver
* The new driver targets #11930

* Fix freezes with failing transactions in half-duplex

* Increase default serial TX/RX buffer size to 128 bytes

* Correctly use bool instead of size_t

Co-authored-by: Nick Brassel <nick@tzarc.org>

1 files changed, 229 insertions, 108 deletions

diff --git a/drivers/chibios/serial_usart.c b/drivers/chibios/serial_usart.c
index 9f180d2d74..ea4473791c 100644
--- a/drivers/chibios/serial_usart.c
+++ b/drivers/chibios/serial_usart.c
@@ -16,179 +16,300 @@
 
 #include "serial_usart.h"
 
-#ifndef USE_GPIOV1
-// The default PAL alternate modes are used to signal that the pins are used for USART
-#    ifndef SERIAL_USART_TX_PAL_MODE
-#        define SERIAL_USART_TX_PAL_MODE 7
+#if defined(SERIAL_USART_CONFIG)
+static SerialConfig serial_config = SERIAL_USART_CONFIG;
+#else
+static SerialConfig serial_config = {
+    .speed = (SERIAL_USART_SPEED), /* speed - mandatory */
+    .cr1   = (SERIAL_USART_CR1),
+    .cr2   = (SERIAL_USART_CR2),
+#    if !defined(SERIAL_USART_FULL_DUPLEX)
+    .cr3   = ((SERIAL_USART_CR3) | USART_CR3_HDSEL) /* activate half-duplex mode */
+#    else
+    .cr3 = (SERIAL_USART_CR3)
 #    endif
+};
 #endif
 
-#ifndef SERIAL_USART_DRIVER
-#    define SERIAL_USART_DRIVER SD1
-#endif
-
-#ifdef SOFT_SERIAL_PIN
-#    define SERIAL_USART_TX_PIN SOFT_SERIAL_PIN
-#endif
-
-static inline msg_t sdWriteHalfDuplex(SerialDriver* driver, uint8_t* data, uint8_t size) {
-    msg_t ret = sdWrite(driver, data, size);
+static SerialDriver* serial_driver = &SERIAL_USART_DRIVER;
 
-    // Half duplex requires us to read back the data we just wrote - just throw it away
-    uint8_t dump[size];
-    sdRead(driver, dump, size);
+static inline bool react_to_transactions(void);
+static inline bool __attribute__((nonnull)) receive(uint8_t* destination, const size_t size);
+static inline bool __attribute__((nonnull)) send(const uint8_t* source, const size_t size);
+static inline int  initiate_transaction(uint8_t sstd_index);
+static inline void usart_clear(void);
 
-    return ret;
+/**
+ * @brief Clear the receive input queue.
+ */
+static inline void usart_clear(void) {
+    osalSysLock();
+    bool volatile queue_not_empty = !iqIsEmptyI(&serial_driver->iqueue);
+    osalSysUnlock();
+
+    while (queue_not_empty) {
+        osalSysLock();
+        /* Hard reset the input queue. */
+        iqResetI(&serial_driver->iqueue);
+        osalSysUnlock();
+        /* Allow pending interrupts to preempt.
+         * Do not merge the lock/unlock blocks into one
+         * or the code will not work properly.
+         * The empty read adds a tiny amount of delay. */
+        (void)queue_not_empty;
+        osalSysLock();
+        queue_not_empty = !iqIsEmptyI(&serial_driver->iqueue);
+        osalSysUnlock();
+    }
 }
-#undef sdWrite
-#define sdWrite sdWriteHalfDuplex
-
-static inline msg_t sdWriteTimeoutHalfDuplex(SerialDriver* driver, uint8_t* data, uint8_t size, uint32_t timeout) {
-    msg_t ret = sdWriteTimeout(driver, data, size, timeout);
 
-    // Half duplex requires us to read back the data we just wrote - just throw it away
-    uint8_t dump[size];
-    sdReadTimeout(driver, dump, size, timeout);
+/**
+ * @brief Blocking send of buffer with timeout.
+ *
+ * @return true Send success.
+ * @return false Send failed.
+ */
+static inline bool send(const uint8_t* source, const size_t size) {
+    bool success = (size_t)sdWriteTimeout(serial_driver, source, size, TIME_MS2I(SERIAL_USART_TIMEOUT)) == size;
+
+#if !defined(SERIAL_USART_FULL_DUPLEX)
+    if (success) {
+        /* Half duplex fills the input queue with the data we wrote - just throw it away.
+           Under the right circumstances (e.g. bad cables paired with high baud rates)
+           less bytes can be present in the input queue, therefore a timeout is needed. */
+        uint8_t dump[size];
+        return receive(dump, size);
+    }
+#endif
 
-    return ret;
+    return success;
 }
-#undef sdWriteTimeout
-#define sdWriteTimeout sdWriteTimeoutHalfDuplex
 
-static inline void sdClear(SerialDriver* driver) {
-    while (sdGetTimeout(driver, TIME_IMMEDIATE) != MSG_TIMEOUT) {
-        // Do nothing with the data
-    }
+/**
+ * @brief  Blocking receive of size * bytes with timeout.
+ *
+ * @return true Receive success.
+ * @return false Receive failed.
+ */
+static inline bool receive(uint8_t* destination, const size_t size) {
+    bool success = (size_t)sdReadTimeout(serial_driver, destination, size, TIME_MS2I(SERIAL_USART_TIMEOUT)) == size;
+    return success;
 }
 
-static SerialConfig sdcfg = {
-    (SERIAL_USART_SPEED),  // speed - mandatory
-    (SERIAL_USART_CR1),    // CR1
-    (SERIAL_USART_CR2),    // CR2
-    (SERIAL_USART_CR3)     // CR3
-};
-
-void handle_soft_serial_slave(void);
+#if !defined(SERIAL_USART_FULL_DUPLEX)
 
-/*
- * This thread runs on the slave and responds to transactions initiated
- * by the master
+/**
+ * @brief Initiate pins for USART peripheral. Half-duplex configuration.
  */
-static THD_WORKING_AREA(waSlaveThread, 2048);
-static THD_FUNCTION(SlaveThread, arg) {
-    (void)arg;
-    chRegSetThreadName("slave_transport");
+__attribute__((weak)) void usart_init(void) {
+#    if defined(MCU_STM32)
+#        if defined(USE_GPIOV1)
+    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_STM32_ALTERNATE_OPENDRAIN);
+#        else
+    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_ALTERNATE(SERIAL_USART_TX_PAL_MODE) | PAL_STM32_OTYPE_OPENDRAIN);
+#        endif
 
-    while (true) {
-        handle_soft_serial_slave();
-    }
+#        if defined(USART_REMAP)
+    USART_REMAP;
+#        endif
+#    else
+#        pragma message "usart_init: MCU Familiy not supported by default, please supply your own init code by implementing usart_init() in your keyboard files."
+#    endif
 }
 
-__attribute__((weak)) void usart_init(void) {
-#if defined(USE_GPIOV1)
-    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_STM32_ALTERNATE_OPENDRAIN);
 #else
-    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_ALTERNATE(SERIAL_USART_TX_PAL_MODE) | PAL_STM32_OTYPE_OPENDRAIN);
-#endif
 
-#if defined(USART_REMAP)
+/**
+ * @brief Initiate pins for USART peripheral. Full-duplex configuration.
+ */
+__attribute__((weak)) void usart_init(void) {
+#    if defined(MCU_STM32)
+#        if defined(USE_GPIOV1)
+    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_STM32_ALTERNATE_PUSHPULL);
+    palSetLineMode(SERIAL_USART_RX_PIN, PAL_MODE_INPUT);
+#        else
+    palSetLineMode(SERIAL_USART_TX_PIN, PAL_MODE_ALTERNATE(SERIAL_USART_TX_PAL_MODE) | PAL_STM32_OTYPE_PUSHPULL | PAL_STM32_OSPEED_HIGHEST);
+    palSetLineMode(SERIAL_USART_RX_PIN, PAL_MODE_ALTERNATE(SERIAL_USART_RX_PAL_MODE) | PAL_STM32_OTYPE_PUSHPULL | PAL_STM32_OSPEED_HIGHEST);
+#        endif
+
+#        if defined(USART_REMAP)
     USART_REMAP;
+#        endif
+#    else
+#        pragma message "usart_init: MCU Familiy not supported by default, please supply your own init code by implementing usart_init() in your keyboard files."
+#    endif
+}
+
 #endif
+
+/**
+ * @brief Overridable master specific initializations.
+ */
+__attribute__((weak, nonnull)) void usart_master_init(SerialDriver** driver) {
+    (void)driver;
+    usart_init();
 }
 
-void usart_master_init(void) {
+/**
+ * @brief Overridable slave specific initializations.
+ */
+__attribute__((weak, nonnull)) void usart_slave_init(SerialDriver** driver) {
+    (void)driver;
     usart_init();
+}
+
+/**
+ * @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("usart_tx_rx");
 
-    sdcfg.cr3 |= USART_CR3_HDSEL;
-    sdStart(&SERIAL_USART_DRIVER, &sdcfg);
+    while (true) {
+        if (!react_to_transactions()) {
+            /* Clear the receive queue, to start with a clean slate.
+             * Parts of failed transactions or spurious bytes could still be in it. */
+            usart_clear();
+        }
+    }
 }
 
-void usart_slave_init(void) {
-    usart_init();
+/**
+ * @brief Slave specific initializations.
+ */
+void soft_serial_target_init(void) {
+    usart_slave_init(&serial_driver);
 
-    sdcfg.cr3 |= USART_CR3_HDSEL;
-    sdStart(&SERIAL_USART_DRIVER, &sdcfg);
+    sdStart(serial_driver, &serial_config);
 
-    // Start transport thread
+    /* Start transport thread. */
     chThdCreateStatic(waSlaveThread, sizeof(waSlaveThread), HIGHPRIO, SlaveThread, NULL);
 }
 
-void soft_serial_initiator_init(void) { usart_master_init(); }
+/**
+ * @brief React to transactions started by the master.
+ */
+static inline bool react_to_transactions(void) {
+    /* Wait until there is a transaction for us. */
+    uint8_t sstd_index = (uint8_t)sdGet(serial_driver);
 
-void soft_serial_target_init(void) { usart_slave_init(); }
+    /* Sanity check that we are actually responding to a valid transaction. */
+    if (sstd_index >= NUM_TOTAL_TRANSACTIONS) {
+        return false;
+    }
 
-void handle_soft_serial_slave(void) {
-    uint8_t                   sstd_index = sdGet(&SERIAL_USART_DRIVER);  // first chunk is always transaction id
-    split_transaction_desc_t* trans      = &split_transaction_table[sstd_index];
+    split_transaction_desc_t* trans = &split_transaction_table[sstd_index];
 
-    // Always write back the sstd_index as part of a basic handshake
+    /* Send back the handshake which is XORed as a simple checksum,
+     to signal that the slave is ready to receive possible transaction buffers  */
     sstd_index ^= HANDSHAKE_MAGIC;
-    sdWrite(&SERIAL_USART_DRIVER, &sstd_index, sizeof(sstd_index));
+    if (!send(&sstd_index, sizeof(sstd_index))) {
+        *trans->status = TRANSACTION_DATA_ERROR;
+        return false;
+    }
 
+    /* Receive transaction buffer from the master. If this transaction requires it.*/
     if (trans->initiator2target_buffer_size) {
-        sdRead(&SERIAL_USART_DRIVER, split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size);
+        if (!receive(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size)) {
+            *trans->status = TRANSACTION_DATA_ERROR;
+            return false;
+        }
     }
 
-    // Allow any slave processing to occur
+    /* Allow any slave processing to occur. */
     if (trans->slave_callback) {
-        trans->slave_callback(trans->initiator2target_buffer_size, split_trans_initiator2target_buffer(trans), trans->target2initiator_buffer_size, split_trans_target2initiator_buffer(trans));
+        trans->slave_callback(trans->initiator2target_buffer_size, split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size, split_trans_target2initiator_buffer(trans));
     }
 
+    /* Send transaction buffer to the master. If this transaction requires it. */
     if (trans->target2initiator_buffer_size) {
-        sdWrite(&SERIAL_USART_DRIVER, split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size);
+        if (!send(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size)) {
+            *trans->status = TRANSACTION_DATA_ERROR;
+            return false;
+        }
     }
 
-    if (trans->status) {
-        *trans->status = TRANSACTION_ACCEPTED;
-    }
+    *trans->status = TRANSACTION_ACCEPTED;
+    return true;
+}
+
+/**
+ * @brief Master specific initializations.
+ */
+void soft_serial_initiator_init(void) {
+    usart_master_init(&serial_driver);
+
+#if defined(MCU_STM32) && defined(SERIAL_USART_PIN_SWAP)
+    serial_config.cr2 |= USART_CR2_SWAP;  // master has swapped TX/RX pins
+#endif
+
+    sdStart(serial_driver, &serial_config);
 }
 
-/////////
-//  start transaction by initiator
-//
-// int  soft_serial_transaction(int sstd_index)
-//
-// Returns:
-//    TRANSACTION_END
-//    TRANSACTION_NO_RESPONSE
-//    TRANSACTION_DATA_ERROR
+/**
+ * @brief Start transaction from the master half to the slave half.
+ *
+ * @param index Transaction Table index of the transaction to start.
+ * @return int TRANSACTION_NO_RESPONSE in case of Timeout.
+ *             TRANSACTION_TYPE_ERROR in case of invalid transaction index.
+ *             TRANSACTION_END in case of success.
+ */
 int soft_serial_transaction(int index) {
-    uint8_t sstd_index = 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);
+}
 
-    if (sstd_index > NUM_TOTAL_TRANSACTIONS) return TRANSACTION_TYPE_ERROR;
-    split_transaction_desc_t* trans = &split_transaction_table[sstd_index];
-    msg_t                     res   = 0;
+/**
+ * @brief Initiate transaction to slave half.
+ */
+static inline int initiate_transaction(uint8_t sstd_index) {
+    /* Sanity check that we are actually starting a valid transaction. */
+    if (sstd_index >= NUM_TOTAL_TRANSACTIONS) {
+        dprintln("USART: Illegal transaction Id.");
+        return TRANSACTION_TYPE_ERROR;
+    }
 
-    if (!trans->status) return TRANSACTION_TYPE_ERROR;  // not registered
+    split_transaction_desc_t* trans = &split_transaction_table[sstd_index];
 
-    sdClear(&SERIAL_USART_DRIVER);
+    /* Transaction is not registered. Abort. */
+    if (!trans->status) {
+        dprintln("USART: Transaction not registered.");
+        return TRANSACTION_TYPE_ERROR;
+    }
 
-    // First chunk is always transaction id
-    sdWriteTimeout(&SERIAL_USART_DRIVER, &sstd_index, sizeof(sstd_index), TIME_MS2I(SERIAL_USART_TIMEOUT));
+    /* Send transaction table index to the slave, which doubles as basic handshake token. */
+    if (!send(&sstd_index, sizeof(sstd_index))) {
+        dprintln("USART: Send Handshake failed.");
+        return TRANSACTION_TYPE_ERROR;
+    }
 
     uint8_t sstd_index_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
-    res = sdReadTimeout(&SERIAL_USART_DRIVER, &sstd_index_shake, sizeof(sstd_index_shake), TIME_MS2I(SERIAL_USART_TIMEOUT));
-    if (res < 0 || (sstd_index_shake != (sstd_index ^ HANDSHAKE_MAGIC))) {
-        dprintf("serial::usart_shake NO_RESPONSE\n");
+    /* 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 (!receive(&sstd_index_shake, sizeof(sstd_index_shake)) || (sstd_index_shake != (sstd_index ^ HANDSHAKE_MAGIC))) {
+        dprintln("USART: Handshake failed.");
         return TRANSACTION_NO_RESPONSE;
     }
 
+    /* Send transaction buffer to the slave. If this transaction requires it. */
     if (trans->initiator2target_buffer_size) {
-        res = sdWriteTimeout(&SERIAL_USART_DRIVER, split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size, TIME_MS2I(SERIAL_USART_TIMEOUT));
-        if (res < 0) {
-            dprintf("serial::usart_transmit NO_RESPONSE\n");
+        if (!send(split_trans_initiator2target_buffer(trans), trans->initiator2target_buffer_size)) {
+            dprintln("USART: Send failed.");
             return TRANSACTION_NO_RESPONSE;
         }
     }
 
+    /* Receive transaction buffer from the slave. If this transaction requires it. */
     if (trans->target2initiator_buffer_size) {
-        res = sdReadTimeout(&SERIAL_USART_DRIVER, split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size, TIME_MS2I(SERIAL_USART_TIMEOUT));
-        if (res < 0) {
-            dprintf("serial::usart_receive NO_RESPONSE\n");
+        if (!receive(split_trans_target2initiator_buffer(trans), trans->target2initiator_buffer_size)) {
+            dprintln("USART: Receive failed.");
             return TRANSACTION_NO_RESPONSE;
         }
     }