From e93724aef4b4c016cf52e231ce5688d8b427fb7a Mon Sep 17 00:00:00 2001
From: Jukka Laitinen <jukkax@ssrc.tii.ae>
Date: Tue, 3 Dec 2024 14:56:03 +0200
Subject: [PATCH] arch/arm64/src/imx9/imx9_usdhc.c: Simplify eventwait logic
 and remove race condition

There is a race condition when timeout and completion interrupts occur at the same time.

Fix this and simplify the eventwait code.

Signed-off-by: Jukka Laitinen <jukkax@ssrc.tii.ae>
---
 arch/arm64/src/imx9/imx9_usdhc.c | 89 +++++++++++---------------------
 1 file changed, 29 insertions(+), 60 deletions(-)

diff --git a/arch/arm64/src/imx9/imx9_usdhc.c b/arch/arm64/src/imx9/imx9_usdhc.c
index 157cc6d52dc65..b6eb5d8430220 100644
--- a/arch/arm64/src/imx9/imx9_usdhc.c
+++ b/arch/arm64/src/imx9/imx9_usdhc.c
@@ -1114,13 +1114,14 @@ static void imx9_recvdma(struct imx9_dev_s *priv)
  *   None
  *
  * Assumptions:
- *   Always called from the interrupt level with interrupts disabled.
+ *   None
  *
  ****************************************************************************/
 
 static void imx9_eventtimeout(wdparm_t arg)
 {
   struct imx9_dev_s *priv = (struct imx9_dev_s *)arg;
+  int sval;
 
   DEBUGASSERT(priv != NULL);
   DEBUGASSERT((priv->waitevents & SDIOWAIT_TIMEOUT) != 0);
@@ -1129,14 +1130,29 @@ static void imx9_eventtimeout(wdparm_t arg)
 
   if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
     {
-      /* Yes.. Sample registers at the time of the timeout */
+      /* Disable further xfrints and waitints */
 
-      imx9_sample(priv, SAMPLENDX_END_TRANSFER);
+      putreg32(priv->cintints,
+               priv->addr + IMX9_USDHC_IRQSIGEN_OFFSET);
 
-      /* Wake up any waiting threads */
+      /* Wake up any waiting threads. Before disabling the
+       * interrupts, the interrupt might have occured as well.
+       * Check the semaphore value to avoid double-post
+       */
+
+      sem_getvalue(&priv->waitsem, &sval);
+      if (sval < 0)
+        {
+          /* Sample registers at the time of the timeout */
 
-      imx9_endwait(priv, SDIOWAIT_TIMEOUT);
-      mcerr("ERROR: Timeout: remaining: %lu\n", priv->remaining);
+          imx9_sample(priv, SAMPLENDX_END_TRANSFER);
+
+          /* End wait with timeout */
+
+          imx9_endwait(priv, SDIOWAIT_TIMEOUT);
+
+          mcerr("ERROR: Timeout: remaining: %lu\n", priv->remaining);
+        }
     }
 }
 
@@ -2813,9 +2829,6 @@ static void imx9_waitenable(struct sdio_dev_s *dev,
  *
  * Input Parameters:
  *   dev     - An instance of the SDIO device interface
- *   timeout - Maximum time in milliseconds to wait.  Zero means immediate
- *             timeout with no wait.  The timeout value is ignored if
- *             SDIOWAIT_TIMEOUT is not included in the waited-for eventset.
  *
  * Returned Value:
  *   Event set containing the event(s) that ended the wait.  Should always
@@ -2826,65 +2839,21 @@ static void imx9_waitenable(struct sdio_dev_s *dev,
 static sdio_eventset_t imx9_eventwait(struct sdio_dev_s *dev)
 {
   struct imx9_dev_s *priv = (struct imx9_dev_s *)dev;
-  sdio_eventset_t wkupevent = 0; int ret;
 
-  /* There is a race condition here... the event may have completed before
-   * we get here.  In this case waitevents will be zero, but wkupevents
-   * will be non-zero (and, hopefully, the semaphore count will also be
-   * non-zero.
+  /* Wait for an event in event set to occur.  If this the event has
+   * already occurred, then the semaphore will already have been
+   * incremented and there will be no wait.
    */
 
-  DEBUGASSERT((priv->waitevents != 0 && priv->wkupevent == 0) ||
-              (priv->waitevents == 0 && priv->wkupevent != 0));
-
-  /* Loop until the event (or the timeout occurs). Race conditions are
-   * avoided by calling imx9_waitenable prior to triggering the logic
-   * that will cause the wait to terminate.  Under certain race
-   * conditions, the waited-for may have already occurred before this
-   * function was called!
-   */
-
-  for (; ; )
-    {
-      /* Wait for an event in event set to occur.  If this the event has
-       * already occurred, then the semaphore will already have been
-       * incremented and there will be no wait.
-       */
-
-      ret = nxsem_wait_uninterruptible(&priv->waitsem);
-      if (ret < 0)
-        {
-          /* Task canceled.  Cancel the wdog (assuming it was started) and
-           * return an SDIO error.
-           */
-
-          wd_cancel(&priv->waitwdog);
-          return SDIOWAIT_ERROR;
-        }
+  nxsem_wait_uninterruptible(&priv->waitsem);
 
-      wkupevent = priv->wkupevent;
-
-      /* Check if the event has occurred.  When the event has occurred, then
-       * evenset will be set to 0 and wkupevent will be set to a non-zero
-       * value.
-       */
-
-      if (wkupevent != 0)
-        {
-          /* Yes... break out of the loop with wkupevent non-zero */
-
-          break;
-        }
-    }
-
-  /* Disable event-related interrupts */
-
-  imx9_configwaitints(priv, 0, 0, 0);
 #ifdef CONFIG_IMX9_USDHC_DMA
   priv->xfrflags = 0;
 #endif
+
   imx9_dumpsamples(priv);
-  return wkupevent;
+
+  return priv->wkupevent;
 }
 
 /****************************************************************************