This is an automated email from the ASF dual-hosted git repository.

xiaoxiang781216 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git

commit c2595870e502e41aeac8c98fcaaf339060965426
Author: Michal Lenc <[email protected]>
AuthorDate: Tue Sep 22 16:20:37 2026 +0200

    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 <[email protected]>
---
 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);

Reply via email to