From 1ed69cd53515a37490f0a3dec7751879c78e6186 Mon Sep 17 00:00:00 2001
From: Gregory Nutt <gnutt@nuttx.org>
Date: Tue, 10 Nov 2015 07:41:40 -0600
Subject: [PATCH] Fix another corner case in the upper half CAN driver

---
 drivers/can.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/drivers/can.c b/drivers/can.c
index 998e1c7b97..f8a6be2e12 100644
--- a/drivers/can.c
+++ b/drivers/can.c
@@ -1044,6 +1044,12 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
  * Returned Value:
  *   OK on success; a negated errno on failure.
  *
+ * Assumptions:
+ *   Interrupts are disabled.  This is required by can_xmit() which is called
+ *   by this function.  Interrupts are explicitly disabled when called
+ *   through can_write().  Interrupts are expected be disabled when called
+ *   from the CAN interrupt handler.
+ *
  ****************************************************************************/
 
 int can_txdone(FAR struct can_dev_s *dev)
@@ -1143,6 +1149,10 @@ int can_txdone(FAR struct can_dev_s *dev)
  * Returned Value:
  *   OK on success; a negated errno on failure.
  *
+ * Assumptions:
+ *   Interrupts are disabled.  This is required by can_xmit() which is called
+ *   by this function.
+ *
  ****************************************************************************/
 
 #ifdef CONFIG_CAN_TXREADY
@@ -1154,26 +1164,20 @@ int can_txready(FAR struct can_dev_s *dev)
             dev->cd_xmit.tx_head, dev->cd_xmit.tx_queue, dev->cd_xmit.tx_tail,
             dev->cd_ntxwaiters);
 
-  /* Are there any threads waiting for space in the xmit FIFO? */
+  /* Verify that the xmit FIFO is not empty.  This is safe because interrupts
+   * are always disabled when calling into can_xmit(); this cannot collide
+   * with ongoing activity from can_write().
+   */
 
-  if (dev->cd_ntxwaiters > 0)
+  if (dev->cd_xmit.tx_head != dev->cd_xmit.tx_tail)
     {
-      /* Verify that the xmit FIFO is not empty.
-       *
-       * REVISIT: This probably should be an assertion since we should only
-       * be waiting for space in the xmit FIFO if the xmit FIFO is full.
+      /* Send the next message in the S/W FIFO.  In the case where the
+       * H/W TX FIFO is not empty, this should add one more CAN message
+       * to the H/W TX FIFO and can_txdone() should be called, making
+       * space in the S/W FIFO
        */
 
-      if (dev->cd_xmit.tx_head != dev->cd_xmit.tx_tail)
-        {
-          /* Send the next message in the S/W FIFO.  In the case where the
-           * H/W TX FIFO is not empty, this should add one more CAN message
-           * to the H/W TX FIFO and can_txdone() should be called, making
-           * space in the S/W FIFO
-           */
-
-          (void)can_xmit(dev);
-        }
+      (void)can_xmit(dev);
 
       /* Inform one waiter that new xmit space is available in the S/W FIFO.
        * NOTE that is can_txdone() is, indeed, called twice that the tx_sem
@@ -1181,7 +1185,12 @@ int can_txready(FAR struct can_dev_s *dev)
        * harmful.
        */
 
-      ret = sem_post(&dev->cd_xmit.tx_sem);
+      /* Are there any threads waiting for space in the xmit FIFO? */
+
+      if (dev->cd_ntxwaiters > 0)
+        {
+          ret = sem_post(&dev->cd_xmit.tx_sem);
+        }
     }
 
   return ret;