From bf8120d51d77ed7bdab646227ceb165d10581d74 Mon Sep 17 00:00:00 2001
From: Gregory Nutt <gnutt@nuttx.org>
Date: Wed, 10 Feb 2016 09:50:28 -0600
Subject: [PATCH] Fix some odd logic in CC3000 semaphore handling.  Noted by
 Vladimir Komendantskiy

---
 drivers/wireless/cc3000/cc3000.c | 92 ++++++++------------------------
 sched/semaphore/spinlock.c       |  5 +-
 2 files changed, 25 insertions(+), 72 deletions(-)

diff --git a/drivers/wireless/cc3000/cc3000.c b/drivers/wireless/cc3000/cc3000.c
index 29b6ad84b1..68924193f2 100644
--- a/drivers/wireless/cc3000/cc3000.c
+++ b/drivers/wireless/cc3000/cc3000.c
@@ -211,27 +211,23 @@ uint8_t spi_readCommand[] = READ_COMMAND;
  *
  ****************************************************************************/
 
-static int cc3000_devtake(FAR struct cc3000_dev_s *priv)
+static inline void cc3000_devtake(FAR struct cc3000_dev_s *priv)
 {
-  int rv;
-
   /* Take the semaphore (perhaps waiting) */
 
-  while ((rv = sem_wait(&priv->devsem)) != 0)
+  while (sem_wait(&priv->devsem) < 0)
     {
-      /* The only case that an error should occur here is if the wait was awakened
-       * by a signal.
+      /* The only case that an error should occur here is if the wait was
+       * awakened by a signal.
        */
 
-      DEBUGASSERT(rv == OK || errno == EINTR);
+      DEBUGASSERT(errno == EINTR);
     }
-
-  return rv;
 }
 
-static inline int cc3000_devgive(FAR struct cc3000_dev_s *priv)
+static inline void cc3000_devgive(FAR struct cc3000_dev_s *priv)
 {
-  return sem_post(&priv->devsem);
+  (void)sem_post(&priv->devsem);
 }
 
 /****************************************************************************
@@ -354,27 +350,16 @@ static int cc3000_wait(FAR struct cc3000_dev_s *priv, sem_t *psem)
   /* Wait on first psem to become signaled */
 
   ret = sem_wait(psem);
-  if (ret >= 0)
-    {
-      /* Yes... then retake the mutual exclusion semaphore */
-
-      ret = cc3000_devtake(priv);
-    }
-
-  sched_unlock();
-
-  /* Was the semaphore wait successful? Did we successful re-take the
-   * mutual exclusion semaphore?
-   */
-
   if (ret < 0)
     {
-      /* No.. One of the two sem_wait's failed. */
-
-      ret = -errno;
+      return -errno;
     }
 
-  return ret;
+  /* Then retake the mutual exclusion semaphore */
+
+  cc3000_devtake(priv);
+  sched_unlock();
+  return OK;
 }
 
 /****************************************************************************
@@ -820,11 +805,7 @@ static int cc3000_open(FAR struct file *filep)
 
   /* Get exclusive access to the driver data structure */
 
-  ret = cc3000_devtake(priv);
-  if (ret < 0)
-    {
-      return -ret;
-    }
+  cc3000_devtake(priv);
 
   /* Increment the reference count */
 
@@ -986,9 +967,8 @@ out_with_sem:
 
 static int cc3000_close(FAR struct file *filep)
 {
-  FAR struct inode         *inode;
+  FAR struct inode *inode;
   FAR struct cc3000_dev_s *priv;
-  int                       ret;
 #ifdef CONFIG_CC3000_MT
   int s;
 #endif
@@ -1005,11 +985,7 @@ static int cc3000_close(FAR struct file *filep)
 
   /* Get exclusive access to the driver data structure */
 
-  ret = cc3000_devtake(priv);
-  if (ret < 0)
-    {
-      return -EINTR;
-    }
+  cc3000_devtake(priv);
 
   /* Decrement the reference count unless it would decrement a negative
    * value.  When the count decrements to zero, there are no further
@@ -1086,13 +1062,7 @@ static ssize_t cc3000_read(FAR struct file *filep, FAR char *buffer, size_t len)
 
   /* Get exclusive access to the driver data structure */
 
-  ret = cc3000_devtake(priv);
-
-  if (ret < 0)
-    {
-      nread = -errno;
-      goto errout_without_sem;
-    }
+  cc3000_devtake(priv);
 
   /* Verify that the caller has provided a buffer large enough to receive
    * the maximum data.
@@ -1152,7 +1122,7 @@ static ssize_t cc3000_read(FAR struct file *filep, FAR char *buffer, size_t len)
         {
           /* Yes... then retake the mutual exclusion semaphore */
 
-          ret = cc3000_devtake(priv);
+          cc3000_devtake(priv);
         }
 
       /* Was the semaphore wait successful? Did we successful re-take the
@@ -1239,15 +1209,7 @@ static ssize_t cc3000_write(FAR struct file *filep, FAR const char *usrbuffer, s
 
   /* Get exclusive access to the driver data structure */
 
-  ret = cc3000_devtake(priv);
-  if (ret < 0)
-    {
-      /* This should only happen if the wait was canceled by an signal */
-
-      ndbg("sem_wait: %d\n", errno);
-      nwritten = -errno;
-      goto errout_without_sem;
-    }
+  cc3000_devtake(priv);
 
   /* Figure out the total length of the packet in order to figure out if there is padding or not */
 
@@ -1352,13 +1314,7 @@ static int cc3000_ioctl(FAR struct file *filep, int cmd, unsigned long arg)
 
   /* Get exclusive access to the driver data structure */
 
-  ret = cc3000_devtake(priv);
-  if (ret < 0)
-    {
-      /* This should only happen if the wait was canceled by an signal */
-
-      return -errno;
-    }
+  cc3000_devtake(priv);
 
   /* Process the IOCTL by command */
 
@@ -1463,13 +1419,7 @@ static int cc3000_poll(FAR struct file *filep, FAR struct pollfd *fds,
 
   /* Are we setting up the poll?  Or tearing it down? */
 
-  ret = cc3000_devtake(priv);
-  if (ret < 0)
-    {
-      /* This should only happen if the wait was canceled by an signal */
-
-      return -errno;
-    }
+  cc3000_devtake(priv);
 
   if (setup)
     {
diff --git a/sched/semaphore/spinlock.c b/sched/semaphore/spinlock.c
index c251b5f747..b39cfe4d9a 100644
--- a/sched/semaphore/spinlock.c
+++ b/sched/semaphore/spinlock.c
@@ -139,7 +139,10 @@ void spinlock(FAR struct spinlock_s *lock)
 
 #  warning Missing logic
 #endif
-      /* Take the lock */
+      /* Take the lock.  REVISIT:  We should set an indication in the TCB
+       * that the thread is spinning.  This might be useful in determining
+       * some scheduling actions?
+       */
 
       while (up_testset(&lock->sp_lock) == SP_LOCKED)
         {