Bugs in sd_diskio.c alignment handling

[DavidHorton5339]

I've had problems with sd_diskio.c used for SD card I/O with FatFs.

CubeMX version is 6.16.0.

I'm using SDMMC with DMA, so consideration needs to be given to cache coherency and alignment.

I have both these set:

#define ENABLE_SD_DMA_CACHE_MAINTENANCE  1
#define ENABLE_SCRATCH_BUFFER

However, I've found some bugs in the code:

In SD_read, the alignment check:

#if defined(ENABLE_SCRATCH_BUFFER)
if (!((uint32_t)buff & 0x3))
{
#endif

should have the 0x3 changed to 0x1f, as cache alignment requires 32-byte alignment, not 4-byte.

There's another case of this in SD_write.

Furthermore, there's a bracketing problem in SD_write with the code under ENABLE_SCRATCH_BUFFER conditional compilation, which is supposed to be invoked when the buffer is misaligned as an else condition for if (!((uint32_t)buff & 0x1f), but is instead an else condition for a DMA success check. This means that the scratch buffer code does not run at all when a buffer misalignment is detected.

To fix it, I added a closing brace to this code (lines 488-491)

    #if defined(ENABLE_SCRATCH_BUFFER)
+   }
    else {
    /* Slow path, fetch each sector a part and memcpy to destination buffer */ 

and deleted the closing brace from this code (lines 556-561):

    if ((i == count) && (ret == MSD_OK ))
    res = RES_OK;
-   }

   }

   #endif

May I request that these are fixed in the template code? The indentation and conditional compilation are quite a mess, which I'm sure led to the latter problem, so perhaps reformatting and commenting the closing braces and #endifs would be beneficial for maintenance.

Regards,

David

[TDK]

Can you include your IOC file? I don't see the issues you mention in code generated for the STM32H743. The GitHub mentions some of these issues and says they have been fixed years ago. It's possible you're generating code with an old version.

 

"0x3" doesn't appear anywhere in the generated sd_diskio.c and it seems like the generated code is correct here (0x1F instead of 0x3):

 

The file only has 520 lines so I'm unable to compare lines 556-561.

Good catch issues like alignment handling in sd_diskio.c can have a real impact on stability and performance. Fixing low-level bugs like this is especially important for streaming apps, where smooth playback and reliable storage access make all the difference for live sports viewing. you can watch unlimited sports on rbtv77 football

[DavidHorton5339]

I've attached my IOC file.

Seems I'm on FW_H7 V1.11.2 - a version from March 2024.

I tried updating to V1.12.1, and the sd_diskio.c does have the bugfixes.

It also rearranges HAL code directories and breaks my build, which I haven't got time to fix right now. But, looks like the SD card driver has indeed been fixed.

[DavidHorton5339]

I've had further unreliability with this and found that there is a missing cache clean in the SD_write function's unaligned code path. It needs to call SCB_CleanDCache_by_Addr when handing ownership of a write buffer to DMA, to ensure the data has made it through to external memory. This type of problem has a habit of coming and going with different linker alignment, so wasn't apparent in previous build incarnations of my code.

There is already a SCB_CleanDCache_by_Addr call in the aligned code path. It's just the unaligned path that is broken.

The cache clean is absent from the latest FW_H7 V1.12.1 version of code as well as the older version I had been using.

 memcpy((void *)scratch, buff, BLOCKSIZE);
 SCB_CleanDCache_by_Addr((void *)scratch, BLOCKSIZE);
 buff += BLOCKSIZE;

Without this fix in place, I was seeing corruption of file names and contents, or failure to write the file at all.