Skip to content

Commit

Permalink
arch/arm64/src/imx9/imx9_usdhc.c: Simplify eventwait logic and remove…
Browse files Browse the repository at this point in the history
… 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 <[email protected]>
  • Loading branch information
jlaitine committed Dec 4, 2024
1 parent 035940f commit e93724a
Showing 1 changed file with 29 additions and 60 deletions.
89 changes: 29 additions & 60 deletions arch/arm64/src/imx9/imx9_usdhc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
}
}
}

Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/****************************************************************************
Expand Down

0 comments on commit e93724a

Please sign in to comment.