Skip to content

drivers/sdmmc_sdhc: add timeout to wait for ISR mutex unlock #21432

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+20 −11
Open

drivers/sdmmc_sdhc: add timeout to wait for ISR mutex unlock #21432

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
31 changes: 20 additions & 11 deletions drivers/sdmmc/sdmmc_sdhc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,24 +79,29 @@

/* millisecond timer definitions dependent on active ztimer backend */
#if IS_USED(MODULE_ZTIMER_MSEC)
#define _ZTIMER_ACQUIRE() ztimer_acquire(ZTIMER_MSEC)
#define _ZTIMER_RELEASE() ztimer_release(ZTIMER_MSEC)
#define _ZTIMER_NOW() ztimer_now(ZTIMER_MSEC)
#define _ZTIMER_SLEEP_MS(n) ztimer_sleep(ZTIMER_MSEC, n)
#define _ZTIMER_CLOCK ZTIMER_MSEC
#define _ZTIMER_TICKS_PER_MS 1
Comment on lines +82 to +83
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define _ZTIMER_CLOCK ZTIMER_MSEC
#define _ZTIMER_TICKS_PER_MS 1
# define _ZTIMER_CLOCK ZTIMER_MSEC
# define _ZTIMER_TICKS_PER_MS 1


#elif IS_USED(MODULE_ZTIMER_USEC)
#define _ZTIMER_ACQUIRE() ztimer_acquire(ZTIMER_USEC)
#define _ZTIMER_RELEASE() ztimer_release(ZTIMER_USEC)
#define _ZTIMER_NOW() ztimer_now(ZTIMER_USEC) / US_PER_MS
#define _ZTIMER_SLEEP_MS(n) ztimer_sleep(ZTIMER_USEC, n * US_PER_MS)
#define _ZTIMER_CLOCK ZTIMER_USEC
#define _ZTIMER_TICKS_PER_MS US_PER_MS
Comment on lines +86 to +87
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#define _ZTIMER_CLOCK ZTIMER_USEC
#define _ZTIMER_TICKS_PER_MS US_PER_MS
# define _ZTIMER_CLOCK ZTIMER_USEC
# define _ZTIMER_TICKS_PER_MS US_PER_MS

I'm not sure if it makes sense to partially apply the indentation style now?
OR you could use the opportunity to apply it to the other #if-#else statements as well.

Ping @maribu?

#else
#error "Either module ztimer_msec or ztimer_usec is needed"
#endif

#define _ZTIMER_ACQUIRE() ztimer_acquire(_ZTIMER_CLOCK)
#define _ZTIMER_RELEASE() ztimer_release(_ZTIMER_CLOCK)
#define _ZTIMER_NOW() (ztimer_now(_ZTIMER_CLOCK) / _ZTIMER_TICKS_PER_MS)
#define _ZTIMER_SLEEP_MS(n) ztimer_sleep(_ZTIMER_CLOCK, n * _ZTIMER_TICKS_PER_MS)

/* Monitor card insertion and removal */
#define SDHC_NISTR_CARD_DETECT (SDHC_NISTR_CREM | SDHC_NISTR_CINS)
#define SDHC_NISTER_CARD_DETECT (SDHC_NISTER_CREM | SDHC_NISTER_CINS)
#define SDHC_NISIER_CARD_DETECT (SDHC_NISIER_CREM | SDHC_NISIER_CINS)

/* 2s timeout for IRQ wait */
#define SDHC_IRQ_TIMEOUT_MS (2000 * _ZTIMER_TICKS_PER_MS)

#include "board.h"

/* forward declaration of _driver */
Expand Down Expand Up @@ -743,16 +748,20 @@ static bool _wait_for_event(sdhc_dev_t *sdhc_dev,
#if defined(CPU_SAMD5X) || defined(CPU_SAME5X)
pm_block(SAM0_PM_IDLE);
#endif
mutex_lock(&sdhc_dev->irq_wait);
bool timeout = ztimer_mutex_lock_timeout(_ZTIMER_CLOCK, &sdhc_dev->irq_wait,
SDHC_IRQ_TIMEOUT_MS) < 0;
#if defined(CPU_SAMD5X) || defined(CPU_SAME5X)
pm_unblock(SAM0_PM_IDLE);
#endif

sdhc->NISIER.reg &= ~event;
sdhc->EISIER.reg &= ~error_mask;

if (sdhc_dev->error & error_mask) {
if (timeout || (sdhc_dev->error & error_mask)) {
if (IS_USED(ENABLE_DEBUG)) {
if (timeout) {
DEBUG("[sdmmc] IRQ wait timeout\n");
}
DEBUG("[sdmmc] SDHC error: EISTR=%04x, ", sdhc_dev->error);
switch (reset) {
case SDHC_SRR_SWRSTCMD:
Expand Down Expand Up @@ -832,7 +841,7 @@ static int _sdhc_to_sdmmc_err_code(uint16_t code)
static void _isr(sdhc_dev_t *sdhc_dev)
{
sdhc_t *sdhc = sdhc_dev->conf->sdhc;

DEBUG_PUTS("[sdmmc] IRQ");
if (sdhc->NISTR.reg & SDHC_NISTR_CARD_DETECT) {
DEBUG_PUTS("[sdmmc] card presence changed");

Expand Down