Fixes to avoid some hang conditions using STM32 CAN

1 files changed, 85 insertions, 55 deletions

diff --git a/nuttx/drivers/can.c b/nuttx/drivers/can.c
index 231b099f3..278acbd4f 100644
--- a/nuttx/drivers/can.c
+++ b/nuttx/drivers/can.c
@@ -1,7 +1,7 @@
 /****************************************************************************
  * drivers/can.c
  *
- *   Copyright (C) 2008-2009, 2011-2012 Gregory Nutt. All rights reserved.
+ *   Copyright (C) 2008-2009, 2011-2012, 2014 Gregory Nutt. All rights reserved.
  *   Author: Gregory Nutt <gnutt@nuttx.org>
  *
  * Redistribution and use in source and binary forms, with or without
@@ -91,11 +91,15 @@
 
 static int            can_open(FAR struct file *filep);
 static int            can_close(FAR struct file *filep);
-static ssize_t        can_read(FAR struct file *filep, FAR char *buffer, size_t buflen);
+static ssize_t        can_read(FAR struct file *filep, FAR char *buffer,
+                         size_t buflen);
 static int            can_xmit(FAR struct can_dev_s *dev);
-static ssize_t        can_write(FAR struct file *filep, FAR const char *buffer, size_t buflen);
-static inline ssize_t can_rtrread(FAR struct can_dev_s *dev, FAR struct canioctl_rtr_s *rtr);
-static int            can_ioctl(FAR struct file *filep, int cmd, unsigned long arg);
+static ssize_t        can_write(FAR struct file *filep,
+                         FAR const char *buffer, size_t buflen);
+static inline ssize_t can_rtrread(FAR struct can_dev_s *dev,
+                         FAR struct canioctl_rtr_s *rtr);
+static int            can_ioctl(FAR struct file *filep, int cmd,
+                         unsigned long arg);
 
 /****************************************************************************
  * Private Data
@@ -118,13 +122,13 @@ static const struct file_operations g_canops =
  * Private Functions
  ****************************************************************************/
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_open
  *
  * Description:
  *   This function is called whenever the CAN device is opened.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
 static int can_open(FAR struct file *filep)
 {
@@ -188,17 +192,18 @@ static int can_open(FAR struct file *filep)
         }
       sem_post(&dev->cd_closesem);
     }
+
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_close
  *
  * Description:
  *   This routine is called when the CAN device is closed.
  *   It waits for the last remaining data to be sent.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
 static int can_close(FAR struct file *filep)
 {
@@ -265,18 +270,20 @@ static int can_close(FAR struct file *filep)
           sem_post(&dev->cd_closesem);
         }
     }
+
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_read
  *
  * Description:
  *   Read standard CAN messages
  *
- ************************************************************************************/
+ ****************************************************************************/
 
-static ssize_t can_read(FAR struct file *filep, FAR char *buffer, size_t buflen)
+static ssize_t can_read(FAR struct file *filep, FAR char *buffer,
+                        size_t buflen)
 {
   FAR struct inode     *inode = filep->f_inode;
   FAR struct can_dev_s *dev   = inode->i_private;
@@ -286,8 +293,9 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, size_t buflen)
 
   canvdbg("buflen: %d\n", buflen);
 
-  /* The caller must provide enough memory to catch the smallest possible message
-   * This is not a system error condition, but we won't permit it,  Hence we return 0.
+  /* The caller must provide enough memory to catch the smallest possible
+   * message.  This is not a system error condition, but we won't permit
+   * it,  Hence we return 0.
    */
 
   if (buflen >= CAN_MSGLEN(0))
@@ -355,10 +363,11 @@ static ssize_t can_read(FAR struct file *filep, FAR char *buffer, size_t buflen)
 return_with_irqdisabled:
       irqrestore(flags);
     }
+
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_xmit
  *
  * Description:
@@ -367,7 +376,7 @@ return_with_irqdisabled:
  * Assumptions:
  *   Called with interrupts disabled
  *
- ************************************************************************************/
+ ****************************************************************************/
 
 static int can_xmit(FAR struct can_dev_s *dev)
 {
@@ -430,11 +439,12 @@ static int can_xmit(FAR struct can_dev_s *dev)
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_write
- ************************************************************************************/
+ ****************************************************************************/
 
-static ssize_t can_write(FAR struct file *filep, FAR const char *buffer, size_t buflen)
+static ssize_t can_write(FAR struct file *filep, FAR const char *buffer,
+                         size_t buflen)
 {
   FAR struct inode        *inode = filep->f_inode;
   FAR struct can_dev_s    *dev   = inode->i_private;
@@ -453,9 +463,10 @@ static ssize_t can_write(FAR struct file *filep, FAR const char *buffer, size_t
 
   flags = irqsave();
 
-  /* Check if the TX is inactive when we started. In certain race conditions, there
-   * may be a pending interrupt to kick things back off, but we will be sure here that
-   * there is not.  That the hardware is IDLE and will need to be kick-started.
+  /* Check if the TX is inactive when we started. In certain race conditions,
+   * there may be a pending interrupt to kick things back off, but we will
+   * be sure here that there is not.  That the hardware is IDLE and will
+   * need to be kick-started.
    */
 
   inactive = dev_txempty(dev);
@@ -466,8 +477,8 @@ static ssize_t can_write(FAR struct file *filep, FAR const char *buffer, size_t
 
   while ((buflen - nsent) >= CAN_MSGLEN(0))
     {
-      /* Check if adding this new message would over-run the drivers ability to enqueue
-       * xmit data.
+      /* Check if adding this new message would over-run the drivers ability
+       * to enqueue xmit data.
        */
 
       nexttail = fifo->tx_tail + 1;
@@ -492,11 +503,12 @@ static ssize_t can_write(FAR struct file *filep, FAR const char *buffer, size_t
                 {
                   ret = nsent;
                 }
+
               goto return_with_irqdisabled;
             }
 
           /* If the TX hardware was inactive when we started, then we will have
-           * start the XMIT sequence generate the TX done interrrupts needed
+           * start the XMIT sequence generate the TX done interrupts needed
            * to clear the FIFO.
            */
 
@@ -562,17 +574,19 @@ return_with_irqdisabled:
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_rtrread
  *
  * Description:
- *   Read RTR messages.  The RTR message is a special message -- it is an outgoing
- *   message that says "Please re-transmit the message with the same identifier as
- *   this message.  So the RTR read is really a send-wait-receive operation.
+ *   Read RTR messages.  The RTR message is a special message -- it is an
+ *   outgoing message that says "Please re-transmit the message with the
+ *   same identifier as this message.  So the RTR read is really a
+ *   send-wait-receive operation.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
-static inline ssize_t can_rtrread(FAR struct can_dev_s *dev, FAR struct canioctl_rtr_s *rtr)
+static inline ssize_t can_rtrread(FAR struct can_dev_s *dev,
+                                  FAR struct canioctl_rtr_s *rtr)
 {
   FAR struct can_rtrwait_s *wait = NULL;
   irqstate_t                flags;
@@ -583,7 +597,7 @@ static inline ssize_t can_rtrread(FAR struct can_dev_s *dev, FAR struct canioctl
 
   flags = irqsave();
 
-  /* Find an avaiable slot in the pending RTR list */
+  /* Find an available slot in the pending RTR list */
 
   for (i = 0; i < CONFIG_CAN_NPENDINGRTR; i++)
     {
@@ -610,13 +624,14 @@ static inline ssize_t can_rtrread(FAR struct can_dev_s *dev, FAR struct canioctl
           ret = sem_wait(&wait->cr_sem);
         }
     }
+
   irqrestore(flags);
   return ret;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_ioctl
- ************************************************************************************/
+ ****************************************************************************/
 
 static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
 {
@@ -630,22 +645,26 @@ static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
 
   switch (cmd)
     {
-      /* CANIOCTL_RTR: Send the remote transmission request and wait for the response.
-       * Argument is a reference to struct canioctl_rtr_s (casting to uintptr_t first
-       * eliminates complaints on some architectures where the sizeof long is different
-       * from the size of a pointer).
+      /* CANIOCTL_RTR: Send the remote transmission request and wait for the
+       * response.  Argument is a reference to struct canioctl_rtr_s
+       * (casting to uintptr_t first eliminates complaints on some
+       * architectures where the sizeof long is different from the size of
+       * a pointer).
        */
 
       case CANIOCTL_RTR:
         ret = can_rtrread(dev, (struct canioctl_rtr_s*)((uintptr_t)arg));
         break;
 
-      /* Not a "built-in" ioctl command.. perhaps it is unique to this device driver */
+      /* Not a "built-in" ioctl command.. perhaps it is unique to this
+       * device driver.
+       */
 
       default:
         ret = dev_ioctl(dev, cmd, arg);
         break;
     }
+
   return ret;
 }
 
@@ -653,13 +672,13 @@ static int can_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
  * Public Functions
  ****************************************************************************/
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_register
  *
  * Description:
  *   Register serial console and serial ports.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
 int can_register(FAR const char *path, FAR struct can_dev_s *dev)
 {
@@ -690,7 +709,7 @@ int can_register(FAR const char *path, FAR struct can_dev_s *dev)
   return register_driver(path, &g_canops, 0666, dev);
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_receive
  *
  * Description:
@@ -704,9 +723,10 @@ int can_register(FAR const char *path, FAR struct can_dev_s *dev)
  * Assumptions:
  *   CAN interrupts are disabled.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
-int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_t *data)
+int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr,
+                FAR uint8_t *data)
 {
   FAR struct can_rxfifo_s *fifo = &dev->cd_recv;
   FAR uint8_t             *dest;
@@ -716,8 +736,8 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_
 
   canllvdbg("ID: %d DLC: %d\n", hdr->ch_id, hdr->ch_dlc);
 
-  /* Check if adding this new message would over-run the drivers ability to enqueue
-   * read data.
+  /* Check if adding this new message would over-run the drivers ability to
+   * enqueue read data.
    */
 
   nexttail = fifo->rx_tail + 1;
@@ -726,7 +746,9 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_
       nexttail = 0;
     }
 
-  /* First, check if this response matches any RTR response that we may be waiting for */
+  /* First, check if this response matches any RTR response that we may be
+   * waiting for.
+   */
 
   if (dev->cd_npendrtr > 0)
     {
@@ -739,8 +761,8 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_
           FAR struct can_rtrwait_s *rtr = &dev->cd_rtr[i];
           FAR struct can_msg_s     *msg = rtr->cr_msg;
 
-          /* Check if the entry is valid and if the ID matches.  A valid entry has
-           * a non-NULL receiving address
+          /* Check if the entry is valid and if the ID matches.  A valid
+           * entry has a non-NULL receiving address
            */
 
           if (msg && hdr->ch_id == rtr->cr_id)
@@ -781,20 +803,23 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_
       fifo->rx_tail = nexttail;
 
       /* The increment the counting semaphore. The maximum value should be
-       * CONFIG_CAN_FIFOSIZE -- one possible count for each allocated message buffer.
+       * CONFIG_CAN_FIFOSIZE -- one possible count for each allocated
+       * message buffer.
        */
 
       sem_post(&fifo->rx_sem);
       err = OK;
     }
+
   return err;
 }
 
-/************************************************************************************
+/****************************************************************************
  * Name: can_txdone
  *
  * Description:
- *   Called from the CAN interrupt handler at the completion of a send operation.
+ *   Called from the CAN interrupt handler at the completion of a send
+ *   operation.
  *
  * Parameters:
  *   dev  - The specific CAN device
@@ -804,7 +829,7 @@ int can_receive(FAR struct can_dev_s *dev, FAR struct can_hdr_s *hdr, FAR uint8_
  * Return:
  *   OK on success; a negated errno on failure.
  *
- ************************************************************************************/
+ ****************************************************************************/
 
 int can_txdone(FAR struct can_dev_s *dev)
 {
@@ -828,17 +853,22 @@ int can_txdone(FAR struct can_dev_s *dev)
 
       /* Send the next message in the FIFO */
 
-      ret = can_xmit(dev);
+      (void)can_xmit(dev);
 
       /* Are there any threads waiting for space in the TX FIFO? */
 
-      if (ret == OK && dev->cd_ntxwaiters > 0)
+      if (dev->cd_ntxwaiters > 0)
         {
           /* Yes.. Inform them that new xmit space is available */
 
           ret = sem_post(&dev->cd_xmit.tx_sem);
         }
+      else
+        {
+          ret = OK;
+        }
     }
+
   return ret;
 }