From c2595870e502e41aeac8c98fcaaf339060965426 Mon Sep 17 00:00:00 2001 From: Michal Lenc Date: Tue, 22 Sep 2026 16:20:37 +0200 Subject: [PATCH] arch/arm/src/samv7/sam_mcan.c: fix potential false debug assertions MCAN controller keeps track of empty TX HW FIFO slots in priv->txfsem semaphore. The semaphore is incremented from TX complete interrupt and taken before new frame is inserted to the HW FIFO. There may be a situation when TX HW FIFO is not full but the semaphore is not yet incremented because the driver didn't handle the interrupt. I managed to reproduce this issue when sending large data chunks over CAN bus and keeping the buffers full for most of the transmission process. This situation leads to the debug assertion although technically it's not a big issue -> the sending function waits on the semaphore until it's posted by the interrupt handler. Moreover, the sanity checks should not be necessary because mcan_buffer_reserve function will take care of fixing the semaphore value if it doesn't match with the FIFO. The entire semaphore logic is a bit weird and probably not necessary. All we need to do is to check SAM_MCAN_TXFQS register if there is at least one free slot in the queue. But this would require a bigger SAMv7 MCAN rewrite, this is rather a hot fix. Signed-off-by: Michal Lenc --- arch/arm/src/samv7/sam_mcan.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/arch/arm/src/samv7/sam_mcan.c b/arch/arm/src/samv7/sam_mcan.c index 164bba5d0a4..62a78090933 100644 --- a/arch/arm/src/samv7/sam_mcan.c +++ b/arch/arm/src/samv7/sam_mcan.c @@ -3222,12 +3222,21 @@ static bool mcan_txready(struct can_dev_s *dev) #ifdef CONFIG_DEBUG_FEATURES /* As a sanity check, the txfsem should also track the number of elements - * the TX FIFO/queue. Make sure that they are consistent. + * the TX FIFO/queue. Check only if the value doesn't exceed number of + * FIFO queue members. Sanity check comparing semaphore value with notfull + * flag may not always work, because SAM_MCAN_TXFQS register may signalize + * not full queue before we process the interrupt and increment the + * semaphore. The sanity checks should not be necessary because + * mcan_buffer_reserve function will take care of fixing the semaphore + * value if it doesn't match with the FIFO. + * + * REVISIT: The entire semaphore logic is a bit weird and probably not + * necessary. All we need to do is to check SAM_MCAN_TXFQS register + * if there is at least one free slot in the queue. */ nxsem_get_value(&priv->txfsem, &sval); - DEBUGASSERT(((notfull && sval > 0) || (!notfull && sval <= 0)) && - (sval <= priv->config->ntxfifoq)); + DEBUGASSERT((sval <= priv->config->ntxfifoq)); #endif nxmutex_unlock(&priv->lock);