From e9bc195bf48417fce2c9de5b404ad4250c90f54a Mon Sep 17 00:00:00 2001 From: Grant Ramsay Date: Thu, 3 Aug 2023 14:08:00 +1200 Subject: [PATCH] drivers: can: mcan: manually track available TX buffers The MCAN driver operates in TX queue mode (TXBC.TFQM = 1). In this mode TXFQS.TFQPI returns the first available buffer (usually buffer zero). Hardware is free to re-use a buffer as soon as TX completes, it does not have to wait for the matching TX event to be processed. If a TX completes and that TX buffer is re-used before processing the TX event, two TX events for the same buffer occur. The first event calls the second events TX callback, and the second event results in a NULL pointer exception. In a "normal" configuration, the TX event ISR will always preempt the queuing of a TX frame to the same TX buffer. However, this issue could occur if: * Sending a message with ISRs temporarily disabled. * The ISR is processed on a different core to the TX call. The fix is to manually track which TX buffers are available, only freeing a buffer after the TX event has been processed. The MCAN user manual states that this is allowed: "The application may use register TXBRP instead of the Put Index and may place messages to any Tx Buffer without pending transmission request" Signed-off-by: Grant Ramsay --- drivers/can/can_mcan.c | 23 +++++++++++------------ 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/drivers/can/can_mcan.c b/drivers/can/can_mcan.c index 6626429e910..dff4a0ca51d 100644 --- a/drivers/can/can_mcan.c +++ b/drivers/can/can_mcan.c @@ -822,7 +822,7 @@ int can_mcan_send(const struct device *dev, const struct can_frame *frame, k_tim #endif /* !CONFIG_CAN_FD_MODE */ .efc = 1U, }; - uint32_t put_idx; + uint32_t put_idx = -1; uint32_t reg; int err; @@ -888,16 +888,16 @@ int can_mcan_send(const struct device *dev, const struct can_frame *frame, k_tim return -EAGAIN; } - err = can_mcan_read_reg(dev, CAN_MCAN_TXFQS, ®); - if (err != 0) { - return err; - } - - __ASSERT_NO_MSG((reg & CAN_MCAN_TXFQS_TFQF) != CAN_MCAN_TXFQS_TFQF); - k_mutex_lock(&data->tx_mtx, K_FOREVER); - put_idx = FIELD_GET(CAN_MCAN_TXFQS_TFQPI, reg); + /* Acquire a free TX buffer */ + for (int i = 0; i < cbs->num_tx; i++) { + if (cbs->tx[i].function == NULL) { + put_idx = i; + break; + } + } + tx_hdr.mm = put_idx; if ((frame->flags & CAN_FRAME_IDE) != 0U) { @@ -930,14 +930,13 @@ int can_mcan_send(const struct device *dev, const struct can_frame *frame, k_tim err = can_mcan_write_reg(dev, CAN_MCAN_TXBAR, BIT(put_idx)); if (err != 0) { - goto err_free_tx_cb; + cbs->tx[put_idx].function = NULL; + goto err_unlock; } k_mutex_unlock(&data->tx_mtx); return 0; -err_free_tx_cb: - cbs->tx[put_idx].function = NULL; err_unlock: k_mutex_unlock(&data->tx_mtx); k_sem_give(&data->tx_sem);