Skip to content

Conversation

@CyberDuck79
Copy link

Description

Problem

When SD card DMA operations fail (e.g., due to buffer memory placement issues), the SD_read() and SD_write() functions in sd_diskio.c wait in a loop until SD_TIMEOUT expires. This can cause the application to hang for several seconds before returning an error.

The HAL provides HAL_SD_ErrorCallback() for this purpose, but it was not implemented, so DMA errors were silently ignored.

Solution

This PR adds proper error callback handling:

HAL_SD_ErrorCallback() in bsp_sd_diskio.c - receives HAL errors and forwards to BSP layer
BSP_SD_ErrorCallback() in sd_diskio.c - sets an error flag
Error flag checking in SD_read()/SD_write() wait loops - exits immediately on error

Benefit

DMA errors now return immediately with RES_ERROR instead of waiting for timeout.
This makes error detection faster, helps developers diagnose SD card issues more quickly.
Also it follows the same pattern as HAL_SD_TxCpltCallback and HAL_SD_RxCpltCallback.

Testing

Tested on Daisy Patch with FatFS file operations.

Without this callback, SD card DMA errors cause the wait loops in
SD_read() and SD_write() to spin until timeout (SD_TIMEOUT), which
can take several seconds.

This adds:
- HAL_SD_ErrorCallback() in bsp_sd_diskio.c that calls BSP_SD_ErrorCallback()
- BSP_SD_ErrorCallback() in sd_diskio.c that sets an error flag
- Error flag checking in SD_read() and SD_write() wait loops

Now when a DMA error occurs (e.g., buffer in wrong memory region),
the functions return immediately with an error instead of hanging.
@github-actions
Copy link

Test Results

150 tests  ±0   150 ✅ ±0   0s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 0efc790. ± Comparison against base commit f044cdc.

@stephenhensley
Copy link
Collaborator

Thanks for the contribution! This looks solid to me.

I'll give this a quick test sometime before the end of the week, but based on the changes I don't expect any issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants