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.

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

Conversation

fabian18
Copy link
Contributor

@fabian18 fabian18 commented Apr 24, 2025

Contribution description

I witnessed situations where the sdmmc driver stalls while waiting for an IRQ.
It might be hardware though ...
However even if it is hardware, it keeps going with this changes in this PR.

Testing procedure

Internal app where data is written to an eMMC and I keep pressing ENTER to continuously execute vfs ls.

> 2025-04-24 10:31:28,805 # vfs ls /sd0/data
2025-04-24 10:31:28,821 # RPOS.TXT	4 B
2025-04-24 10:31:28,821 # 0.CNK	5559 B
2025-04-24 10:31:28,821 # 1.CNK	4181 B
2025-04-24 10:31:28,821 # 2.CNK	696 B
2025-04-24 10:31:28,821 # total 4 files
> 2025-04-24 10:31:28,837 # vfs ls /sd0/data
2025-04-24 10:31:30,853 # [ERROR] [sdmmc] Card not present
2025-04-24 10:31:30,853 # vfs_opendir error: -EIO
> 2025-04-24 10:31:30,853 # vfs ls /sd0/data
2025-04-24 10:31:30,853 # vfs_opendir error: -EIO
> 2025-04-24 10:31:30,869 # vfs ls /sd0/data
2025-04-24 10:31:31,077 # RPOS.TXT	4 B
2025-04-24 10:31:31,093 # 0.CNK	5559 B
2025-04-24 10:31:31,093 # 1.CNK	4181 B
2025-04-24 10:31:31,093 # 2.CNK	1385 B
2025-04-24 10:31:31,094 # total 4 files
> 2025-04-24 10:31:31,094 # vfs ls /sd0/data
2025-04-24 10:31:31,094 # RPOS.TXT	4 B
2025-04-24 10:31:31,109 # 0.CNK	5559 B
2025-04-24 10:31:31,109 # 1.CNK	4181 B
2025-04-24 10:31:31,124 # 2.CNK	1385 B
2025-04-24 10:31:31,125 # total 4 files
> 2025-04-24 10:31:31,125 # vfs ls /sd0/dvfs ls /sd0/data


Without the change it was stalling. With this change it keeps working afterwards.

Issues/PRs references

@github-actions github-actions bot added the Area: drivers Area: Device drivers label Apr 24, 2025
Comment on lines +82 to +83
#define _ZTIMER_CLOCK ZTIMER_MSEC
#define _ZTIMER_TICKS_PER_MS 1
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

Comment on lines +86 to +87
#define _ZTIMER_CLOCK ZTIMER_USEC
#define _ZTIMER_TICKS_PER_MS US_PER_MS
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?

@crasbe
Copy link
Contributor

crasbe commented Apr 24, 2025

One small thing: The title and commit message should have a slash instead of colon. So drivers/sdmmc_sdhc: add timeout to wait for ISR mutex unlock.

@fabian18 fabian18 changed the title drivers:sdmmc_sdhc: add timeout to wait for ISR mutex unlock drivers/sdmmc_sdhc: add timeout to wait for ISR mutex unlock Apr 24, 2025
@fabian18 fabian18 force-pushed the fix/sdmmc_sdhc_irq_timeout branch from 48b1ae9 to 7ffe014 Compare April 24, 2025 12:20
@crasbe crasbe added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 24, 2025
@benpicco benpicco requested a review from gschorcht April 24, 2025 13:08
@riot-ci
Copy link

riot-ci commented Apr 24, 2025

Murdock results

✔️ PASSED

7ffe014 drivers/sdmmc_sdhc: add timeout to wait for ISR mutex unlock

Success Failures Total Runtime
10299 0 10299 10m:25s

Artifacts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: drivers Area: Device drivers CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants