Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fixes race condition in event wait logic of SDMMC driver. #2989

Merged
merged 1 commit into from
Apr 6, 2021
Merged

Fixes race condition in event wait logic of SDMMC driver. #2989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 30 additions & 30 deletions arch/arm/src/cxd56xx/cxd56_sdhci.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,9 +398,8 @@ static int cxd56_sdio_recvshort(FAR struct sdio_dev_s *dev, uint32_t cmd,
/* EVENT handler */

static void cxd56_sdio_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static sdio_eventset_t cxd56_sdio_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout);
sdio_eventset_t eventset, uint32_t timeout);
static sdio_eventset_t cxd56_sdio_eventwait(FAR struct sdio_dev_s *dev);
static void cxd56_sdio_callbackenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static int cxd56_sdio_registercallback(FAR struct sdio_dev_s *dev,
Expand Down Expand Up @@ -2494,7 +2493,7 @@ static int cxd56_sdio_recvshort(FAR struct sdio_dev_s *dev, uint32_t cmd,
****************************************************************************/

static void cxd56_sdio_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset)
sdio_eventset_t eventset, uint32_t timeout)
{
struct cxd56_sdiodev_s *priv = (struct cxd56_sdiodev_s *)dev;
uint32_t waitints;
Expand Down Expand Up @@ -2523,6 +2522,32 @@ static void cxd56_sdio_waitenable(FAR struct sdio_dev_s *dev,
/* Enable event-related interrupts */

cxd56_configwaitints(priv, waitints, eventset, 0);

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;
int ret;

/* Yes.. Handle a corner case */

if (!timeout)
{
priv->wkupevent = SDIOWAIT_TIMEOUT;
return;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
cxd56_eventtimeout, (wdparm_t)priv);
if (ret != OK)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}
}

/****************************************************************************
Expand All @@ -2546,8 +2571,7 @@ static void cxd56_sdio_waitenable(FAR struct sdio_dev_s *dev,
*
****************************************************************************/

static sdio_eventset_t cxd56_sdio_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout)
static sdio_eventset_t cxd56_sdio_eventwait(FAR struct sdio_dev_s *dev)
{
struct cxd56_sdiodev_s *priv = (struct cxd56_sdiodev_s *)dev;
sdio_eventset_t wkupevent = 0;
Expand All @@ -2561,30 +2585,6 @@ static sdio_eventset_t cxd56_sdio_eventwait(FAR struct sdio_dev_s *dev,
DEBUGASSERT((priv->waitevents != 0 && priv->wkupevent == 0) ||
(priv->waitevents == 0 && priv->wkupevent != 0));

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;

/* Yes.. Handle a corner case */

if (!timeout)
{
return SDIOWAIT_TIMEOUT;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
cxd56_eventtimeout, (wdparm_t)priv);
if (ret != OK)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}

/* Loop until the event (or the timeout occurs). Race conditions are
* avoided by calling cxd56_waitenable prior to triggering the logic that
* will cause the wait to terminate. Under certain race conditions, the
Expand Down
62 changes: 31 additions & 31 deletions arch/arm/src/imxrt/imxrt_usdhc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,8 @@ static int imxrt_recvshort(FAR struct sdio_dev_s *dev, uint32_t cmd,
/* EVENT handler */

static void imxrt_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static sdio_eventset_t imxrt_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout);
sdio_eventset_t eventset, uint32_t timeout);
static sdio_eventset_t imxrt_eventwait(FAR struct sdio_dev_s *dev);
static void imxrt_callbackenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static int imxrt_registercallback(FAR struct sdio_dev_s *dev,
Expand Down Expand Up @@ -2627,7 +2626,7 @@ static int imxrt_recvshort(FAR struct sdio_dev_s *dev, uint32_t cmd,
****************************************************************************/

static void imxrt_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset)
sdio_eventset_t eventset, uint32_t timeout)
{
struct imxrt_dev_s *priv = (struct imxrt_dev_s *)dev;
uint32_t waitints;
Expand Down Expand Up @@ -2660,6 +2659,33 @@ static void imxrt_waitenable(FAR struct sdio_dev_s *dev,
/* Enable event-related interrupts */

imxrt_configwaitints(priv, waitints, eventset, 0);

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;
int ret;

/* Yes.. Handle a corner case */

if (!timeout)
{
priv->wkupevent = SDIOWAIT_TIMEOUT;
return;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
imxrt_eventtimeout, (wdparm_t)priv);

if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}
}

/****************************************************************************
Expand All @@ -2683,8 +2709,7 @@ static void imxrt_waitenable(FAR struct sdio_dev_s *dev,
*
****************************************************************************/

static sdio_eventset_t imxrt_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout)
static sdio_eventset_t imxrt_eventwait(FAR struct sdio_dev_s *dev)
{
struct imxrt_dev_s *priv = (struct imxrt_dev_s *)dev;
sdio_eventset_t wkupevent = 0; int ret;
Expand All @@ -2698,31 +2723,6 @@ static sdio_eventset_t imxrt_eventwait(FAR struct sdio_dev_s *dev,
DEBUGASSERT((priv->waitevents != 0 && priv->wkupevent == 0) ||
(priv->waitevents == 0 && priv->wkupevent != 0));

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;

/* Yes.. Handle a corner case */

if (!timeout)
{
return SDIOWAIT_TIMEOUT;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
imxrt_eventtimeout, (wdparm_t)priv);

if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}

/* Loop until the event (or the timeout occurs). Race conditions are
* avoided by calling imxrt_waitenable prior to triggering the logic
* that will cause the wait to terminate. Under certain race
Expand Down
60 changes: 30 additions & 30 deletions arch/arm/src/kinetis/kinetis_sdhc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,8 @@ static int kinetis_recvnotimpl(FAR struct sdio_dev_s *dev, uint32_t cmd,
/* EVENT handler */

static void kinetis_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static sdio_eventset_t
kinetis_eventwait(FAR struct sdio_dev_s *dev, uint32_t timeout);
sdio_eventset_t eventset, uint32_t timeout);
static sdio_eventset_t kinetis_eventwait(FAR struct sdio_dev_s *dev);
static void kinetis_callbackenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static int kinetis_registercallback(FAR struct sdio_dev_s *dev,
Expand Down Expand Up @@ -2430,7 +2429,7 @@ static int kinetis_recvnotimpl(FAR struct sdio_dev_s *dev, uint32_t cmd,
****************************************************************************/

static void kinetis_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset)
sdio_eventset_t eventset, uint32_t timeout)
{
struct kinetis_dev_s *priv = (struct kinetis_dev_s *)dev;
uint32_t waitints;
Expand Down Expand Up @@ -2459,6 +2458,32 @@ static void kinetis_waitenable(FAR struct sdio_dev_s *dev,
/* Enable event-related interrupts */

kinetis_configwaitints(priv, waitints, eventset, 0);

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;
int ret;

/* Yes.. Handle a corner case */

if (!timeout)
{
priv->wkupevent = SDIOWAIT_TIMEOUT;
return;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
kinetis_eventtimeout, (wdparm_t)priv);
if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}
}

/****************************************************************************
Expand All @@ -2482,8 +2507,7 @@ static void kinetis_waitenable(FAR struct sdio_dev_s *dev,
*
****************************************************************************/

static sdio_eventset_t kinetis_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout)
static sdio_eventset_t kinetis_eventwait(FAR struct sdio_dev_s *dev)
{
struct kinetis_dev_s *priv = (struct kinetis_dev_s *)dev;
sdio_eventset_t wkupevent = 0;
Expand All @@ -2497,30 +2521,6 @@ static sdio_eventset_t kinetis_eventwait(FAR struct sdio_dev_s *dev,
DEBUGASSERT((priv->waitevents != 0 && priv->wkupevent == 0) ||
(priv->waitevents == 0 && priv->wkupevent != 0));

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;

/* Yes.. Handle a corner case */

if (!timeout)
{
return SDIOWAIT_TIMEOUT;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
kinetis_eventtimeout, (wdparm_t)priv);
if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}

/* Loop until the event (or the timeout occurs). Race conditions are
* avoided by calling kinetis_waitenable prior to triggering the logic
* that will cause the wait to terminate. Under certain race conditions,
Expand Down
68 changes: 32 additions & 36 deletions arch/arm/src/lpc17xx_40xx/lpc17_40_sdcard.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,8 @@ static int lpc17_40_recvnotimpl(FAR struct sdio_dev_s *dev, uint32_t cmd,
/* EVENT handler */

static void lpc17_40_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static sdio_eventset_t
lpc17_40_eventwait(FAR struct sdio_dev_s *dev, uint32_t timeout);
sdio_eventset_t eventset, uint32_t timeout);
static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev);
static void lpc17_40_callbackenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset);
static int lpc17_40_registercallback(FAR struct sdio_dev_s *dev,
Expand Down Expand Up @@ -2237,10 +2236,11 @@ static int lpc17_40_recvnotimpl(FAR struct sdio_dev_s *dev, uint32_t cmd,
****************************************************************************/

static void lpc17_40_waitenable(FAR struct sdio_dev_s *dev,
sdio_eventset_t eventset)
sdio_eventset_t eventset, uint32_t timeout)
{
struct lpc17_40_dev_s *priv = (struct lpc17_40_dev_s *)dev;
uint32_t waitmask;
int ret;

DEBUGASSERT(priv != NULL);

Expand Down Expand Up @@ -2272,6 +2272,33 @@ static void lpc17_40_waitenable(FAR struct sdio_dev_s *dev,

putreg32(SDCARD_WAITALL_ICR, LPC17_40_SDCARD_CLEAR);
lpc17_40_configwaitints(priv, waitmask, eventset, 0);

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;

/* Yes.. Handle a cornercase: The user request a timeout event but
* with timeout == 0?
*/

if (!timeout)
{
priv->wkupevent = SDIOWAIT_TIMEOUT;
return;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
lpc17_40_eventtimeout, (wdparm_t)priv);
if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}
}

/****************************************************************************
Expand All @@ -2295,8 +2322,7 @@ static void lpc17_40_waitenable(FAR struct sdio_dev_s *dev,
*
****************************************************************************/

static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev,
uint32_t timeout)
static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev)
{
struct lpc17_40_dev_s *priv = (struct lpc17_40_dev_s *)dev;
sdio_eventset_t wkupevent = 0;
Expand All @@ -2311,35 +2337,6 @@ static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev,
flags = enter_critical_section();
DEBUGASSERT(priv->waitevents != 0 || priv->wkupevent != 0);

/* Check if the timeout event is specified in the event set */

if ((priv->waitevents & SDIOWAIT_TIMEOUT) != 0)
{
int delay;

/* Yes.. Handle a cornercase: The user request a timeout event but
* with timeout == 0?
*/

if (!timeout)
{
/* Then just tell the caller that we already timed out */

wkupevent = SDIOWAIT_TIMEOUT;
goto errout;
}

/* Start the watchdog timer */

delay = MSEC2TICK(timeout);
ret = wd_start(&priv->waitwdog, delay,
lpc17_40_eventtimeout, (wdparm_t)priv);
if (ret < 0)
{
mcerr("ERROR: wd_start failed: %d\n", ret);
}
}

/* Loop until the event (or the timeout occurs). Race conditions are
* avoided by calling lpc17_40_waitenable prior to triggering the logic
* that will cause the wait to terminate. Under certain race conditions,
Expand Down Expand Up @@ -2388,7 +2385,6 @@ static sdio_eventset_t lpc17_40_eventwait(FAR struct sdio_dev_s *dev,
priv->xfrflags = 0;
#endif

errout:
leave_critical_section(flags);
lpc17_40_dumpsamples(priv);
return wkupevent;
Expand Down
Loading