[rtems commit] leon, grcan: semaphore reset count required after flushing

[Daniel Hellstrom]

Module:    rtems
Branch:    master
Commit:    9154155110c97b3899aa9825153732f49c11ee53
Changeset: http://git.rtems.org/rtems/commit/?id=9154155110c97b3899aa9825153732f49c11ee53

Author:    Daniel Hellstrom <daniel at gaisler.com>
Date:      Tue Apr 11 12:30:02 2017 +0200

leon, grcan: semaphore reset count required after flushing

It is also required to use semaphore release instead of flush when stopping
or on BUSOFF/AHBERR condition. Otherwise a task just about to wait
(taking the semaphore) could end up locked because the semaphore count is
still the same.

There was previously a scenario where the semaphore flush would not always make
semaphore obtain to return in case of BUSOFF, AHBERROR or grcan_stop. It has to
do with the rtems_semaphore_flush() not releasing the semaphore but just aborts
any _current_ waiter.

---

 c/src/lib/libbsp/sparc/shared/can/grcan.c | 62 ++++++++++++++++---------------
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/c/src/lib/libbsp/sparc/shared/can/grcan.c b/c/src/lib/libbsp/sparc/shared/can/grcan.c
index 1050934..ac1c718 100644
--- a/c/src/lib/libbsp/sparc/shared/can/grcan.c
+++ b/c/src/lib/libbsp/sparc/shared/can/grcan.c
@@ -492,15 +492,19 @@ static void grcan_hw_stop(struct grcan_priv *pDev)
 
 static void grcan_sw_stop(struct grcan_priv *pDev)
 {
-	/* Reset semaphores to the initial state and wakeing
-	 * all threads waiting for an IRQ. The threads that
-	 * get woken up must check for RTEMS_UNSATISFIED in
+	/*
+	 * Release semaphores to wake all threads waiting for an IRQ.
+	 * The threads that
+	 * get woken up must check started state in
 	 * order to determine that they should return to
 	 * user space with error status.
+	 *
+	 * Entering into started mode again will reset the
+	 * semaphore count.
 	 */
-	rtems_semaphore_flush(pDev->rx_sem);
-	rtems_semaphore_flush(pDev->tx_sem);
-	rtems_semaphore_flush(pDev->txempty_sem);
+	rtems_semaphore_release(pDev->rx_sem);
+	rtems_semaphore_release(pDev->tx_sem);
+	rtems_semaphore_release(pDev->txempty_sem);
 }
 
 static void grcan_hw_config(struct grcan_regs *regs, struct grcan_config *conf)
@@ -962,17 +966,14 @@ static int grcan_wait_rxdata(struct grcan_priv *pDev, int min)
 
 	/* Wait for IRQ to fire only if has been triggered */
 	if (wait) {
-		if (
-			rtems_semaphore_obtain(
-				pDev->rx_sem,
-				RTEMS_WAIT,
-				RTEMS_NO_TIMEOUT
-			) == RTEMS_UNSATISFIED
-		) {
-			DBGC(DBG_STATE, "UNSATISFIED\n");
-			/* Device driver has been closed or stopped, return with error status */
-			return state2err[pDev->started];
-		}
+		rtems_semaphore_obtain(pDev->rx_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
+		/*
+		 * The semaphore is released either due to the expected IRQ
+		 * condition or by BUSOFF, AHBERROR or another thread calling
+		 * grcan_stop(). In either case, state2err[] has the correnct
+		 * return value.
+		 */
+		return state2err[pDev->started];
 	}
 
 	return 0;
@@ -1054,13 +1055,8 @@ static int grcan_wait_txspace(struct grcan_priv *pDev, int min)
 
 	/* Wait for IRQ to fire only if it has been triggered */
 	if (wait) {
-		if (rtems_semaphore_obtain(pDev->tx_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT) ==
-		    RTEMS_UNSATISFIED) {
-			/* Device driver has flushed us, this may be due to another thread has
-			 * closed the device, this is to avoid deadlock */
-			DBGC(DBG_STATE, "UNSATISFIED\n");
-			return state2err[pDev->started];
-		}
+		rtems_semaphore_obtain(pDev->tx_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
+		return state2err[pDev->started];
 	}
 
 	/* At this point the TxIRQ has been masked, we ned not to mask it */
@@ -1117,11 +1113,10 @@ static int grcan_tx_flush(struct grcan_priv *pDev)
 			break;
 
 		/* Wait for IRQ to wake us */
-		if (rtems_semaphore_obtain
-		    (pDev->txempty_sem, RTEMS_WAIT,
-		     RTEMS_NO_TIMEOUT) == RTEMS_UNSATISFIED) {
-			DBGC(DBG_STATE, "UNSATISFIED\n");
-			return state2err[pDev->started];
+		rtems_semaphore_obtain(pDev->txempty_sem, RTEMS_WAIT, RTEMS_NO_TIMEOUT);
+		state = pDev->started;
+		if (state != STATE_STARTED) {
+			return state2err[state];
 		}
 	}
 	return 0;
@@ -1565,6 +1560,13 @@ int grcan_start(void *d)
 		return -2;
 	}
 
+	/* Clear semaphore state. This is to avoid effects from previous
+	 * bus-off/stop where semahpores where flushed() but the count remained.
+	 */
+	rtems_semaphore_obtain(pDev->rx_sem, RTEMS_NO_WAIT, 0);
+	rtems_semaphore_obtain(pDev->tx_sem, RTEMS_NO_WAIT, 0);
+	rtems_semaphore_obtain(pDev->txempty_sem, RTEMS_NO_WAIT, 0);
+
 	/* Read and write are now open... */
 	pDev->started = STATE_STARTED;
 	DBGC(DBG_STATE, "STOPPED|BUSOFF|AHBERR->STARTED\n");
@@ -1942,7 +1944,7 @@ static void grcan_interrupt(void *arg)
 		 */
 		SPIN_UNLOCK(&pDev->devlock, irqflags);
 
-		/* flush semaphores to wake blocked threads */
+		/* Release semaphores to wake blocked threads. */
 		grcan_sw_stop(pDev);
 
 		/*