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 <michallenc@seznam.cz>
This commit is contained in:
Michal Lenc 2026-09-22 16:20:37 +02:00 • committed by Xiang Xiao
parent d49d3bff9e
commit c2595870e5

View file

@ -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);