cancel
Showing results for 
Search instead for 
Did you mean: 

[BUG] SDIO + RTOS (Stat = STA_NOINIT)

valentin
Senior
Posted on December 02, 2017 at 03:42

Hi,

I just took two days to trace a bug that only appeared when I was doing concurrent reads & writes on my SD card from separate threads in FREERTOS.

The bug is in sd_diskio.c and needs fixing intwo functions. The issue was that one thread doing a read could get interrupted during SD_status after it had set Stat = STA_NOINIT.

This would then cause the other thread callinga FATFS function that checks for Stat == STA_NOINIT to fail erroneously.

This is how to fix the first bug:

/**
 * @brief Initializes a Drive
 * @param lun : not used
 * @retval DSTATUS: Operation status
 */
DSTATUS SD_initialize(BYTE lun) {
 // Stat = STA_NOINIT; // FIX: DON'T modify this as not reentrant!
 /* Configure the uSD device */
 if (BSP_SD_Init() == MSD_OK) {
 Stat &= ~STA_NOINIT;
 } else {
 Stat = STA_NOINIT; // FIX: instead change here in case of failure
 }
 return Stat;
}
/**
 * @brief Gets Disk Status
 * @param lun : not used
 * @retval DSTATUS: Operation status
 */
DSTATUS SD_status(BYTE lun) {
 // Stat = STA_NOINIT; // FIX: DON'T modify this as not reentrant!
 if (BSP_SD_GetStatus() == MSD_OK) {
 Stat &= ~STA_NOINIT;
 } else {
 Stat = STA_NOINIT; // FIX: instead change here in case of failure
 }
 return Stat;
}�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

I omitted the mutex locks in hereto highlight theSTA_NOINIT issue, which is separate from the mutex locks.

Part 2:

It seems like the whole set of ST's sdio drivers is not reentrant!

In order to make it work, one needs to enclose all non-ISR functions in sd_diskio.c, bsp_driver_sd.c and stm32f2xx_hal_sd.c with a Mutex lock to a single mutex.

An example code snippet here:

/*
 * hal_sd_driver_mutex.h
 *
 * Created on: 2/12/2017
 * Author: Valentin
 */
#ifndef HAL_MODS_INC_HAL_SD_DRIVER_MUTEX_H_
#define HAL_MODS_INC_HAL_SD_DRIVER_MUTEX_H_
#include 'FreeRTOS.h'
#include 'semphr.h'
#ifdef __cplusplus
 extern 'C' {
#endif
#define DEBUG_BlockSD_Mutex 1
#if DEBUG_BlockSD_Mutex == 1
extern SemaphoreHandle_t mutex;
extern volatile uint8_t taken;
void takeMutex();
void giveMutex();
#endif
#ifdef __cplusplus
 }
#endif
#endif /* HAL_MODS_INC_HAL_SD_DRIVER_MUTEX_H_ */
�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

in hal_sd_driver_mutex.c:

#if DEBUG_BlockSD_Mutex == 1
#include 'hal_sd_driver_mutex.h'
SemaphoreHandle_t mutex;
void takeMutex() {
 if (!mutex) {
 mutex = xSemaphoreCreateRecursiveMutex();
 assert_param(mutex);
 }
 uint8_t res = (uint8_t) xSemaphoreTakeRecursive(mutex, 480);
 assert_param(res);
}
void giveMutex() {
 uint8_t res = (uint8_t) xSemaphoreGiveRecursive(mutex);
 assert_param(res);
}
#endif�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

(changed

stm32f2xx_hal_sd.c tostm32f2xx_hal_sd.cpp to use RAII mutex lock/unlock):

#if DEBUG_BlockSD_Mutex == 1
#include 'hal_sd_driver_mutex.h'
class Mutexer {
 Mutexer() {
 takeMutex();
 }
 ~Mutexer() {
 giveMutex();
 }
};
#endif
/**
 * @brief Returns the SD current state.
 * @param hsd: SD handle
 * @retval SD card current state
 */
static HAL_SD_CardStateTypedef SD_GetState(SD_HandleTypeDef *hsd) {
#if DEBUG_BlockSD_Mutex == 1
 const Mutexer mtx();
#endif
 uint32_t resp1 = 0U;
 if (SD_SendStatus(hsd, &resp1) != SD_OK) {
 return SD_CARD_ERROR;
 } else {
 return (HAL_SD_CardStateTypedef) ((resp1 >> 9U) & 0x0FU);
 }
}�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

-> do this for all non-ISR functions in this file!

and in

sd_diskio

.c

[...]
#if DEBUG_BlockSD_Mutex == 1
#include 'hal_sd_driver_mutex.h'
#endif
[...]
/**
 * @brief Reads Sector(s)
 * @param lun : not used
 * @param *buff: Data buffer to store read data
 * @param sector: Sector address (LBA)
 * @param count: Number of sectors to read (1..128)
 * @retval DRESULT: Operation result
 */
DRESULT SD_read(BYTE lun, BYTE *buff, DWORD sector, UINT count) {
 DRESULT res = RES_OK;
#if DEBUG_BlockSD_Mutex == 1
 takeMutex();
#endif
 if (BSP_SD_ReadBlocks_DMA((uint32_t*) buff, (uint64_t) (sector * BLOCK_SIZE),
 BLOCK_SIZE, count) != MSD_OK) {
 res = RES_ERROR;
 }
#if DEBUG_BlockSD_Mutex == 1
 giveMutex();
#endif
 return res;
}�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?�?

Before these fixes I was getting crashes on concurrent SD read/writes, either because of Stat ==STA_NOINIT or of lacking sync in the low level driver calls to the sd card.

Opinions, feedback welcome.

@ST please fixSTA_NOINIT part and maybe add a note somewhere about sd driver mutex requirements for FreeRTOS or even better: implement a lock in case FreeRTOS is active.

I was actually under the impression that the SD drivers are completely thread-safe, given that they come via Cube with FreeRTOS enabled, FATFS having a FS_REENTRANT switch and no notes about SD reentrancy being an issue anywhere.

Anyway, here's the solution.

Thanks!

Edit:

Actually now when I look at it again, the

Stat ==STA_NOINIT part is also just part of the synchronisation issue and would be fixed by adding the lock. Still doesn't hurt to do it. One operation less.

#reentrancy #freertos+fat #driver #sdio #bug #hal
0 REPLIES 0