SDMMC - [STM32CubeF7][1.1x.0][stm32f7xx_hal_sd] SDMMC / DMA IRQ handlers data race

[phlf]

Usecase:

Using the SDMMC module with DMA in HS mode (clock bypass @48MHz following CMD6 switch CMD), the SDMMC_IT_DTIMEOUT interrupt is raised when waiting few dozen of seconds after last successful data transfer (FATFS f_read in this case).

The HAL_SD_IRQHandler then calls the SD_DMARxAbort callbacks which sets the hsd->hdmatx to NULL.

Thus the following data transfer are locked as calls to HAL_SD_GetState(&sSdHdl) never return HAL_SD_STATE_READY.

Cause:

The SD_DMAReceiveCplt which is triggered before HAL_SD_IRQHandler clears the SDMMC_FLAG_DATAEND. Then the HAL_SD_IRQHandler checks for SDMMC_FLAG_DATAEND fails and the call to

__HAL_SD_DISABLE_IT(hsd, SDMMC_IT_DATAEND | SDMMC_IT_DCRCFAIL | SDMMC_IT_DTIMEOUT | SDMMC_IT_TXUNDERR | SDMMC_IT_RXOVERR);

never happens, thus allowing for SDMMC_IT_DTIMEOUT to be raised even if the transfer was successful...

Fix (?) :

Change

void HAL_SD_IRQHandler(SD_HandleTypeDef* hsd)
{
    uint32_t errorstate = HAL_SD_ERROR_NONE;
 
    /* Check for SDMMC interrupt flags */
    if (__HAL_SD_GET_FLAG(hsd, SDMMC_IT_DATAEND) != RESET) {
        __HAL_SD_CLEAR_FLAG(hsd, SDMMC_FLAG_DATAEND);
 
        __HAL_SD_DISABLE_IT(
            hsd, SDMMC_IT_DATAEND | SDMMC_IT_DCRCFAIL | SDMMC_IT_DTIMEOUT | SDMMC_IT_TXUNDERR | SDMMC_IT_RXOVERR);
 
        if ((hsd->Context & SD_CONTEXT_IT) != RESET) {
            if (((hsd->Context & SD_CONTEXT_READ_MULTIPLE_BLOCK) != RESET)
                || ((hsd->Context & SD_CONTEXT_WRITE_MULTIPLE_BLOCK) != RESET)) {
                errorstate = SDMMC_CmdStopTransfer(hsd->Instance);
                if (errorstate != HAL_SD_ERROR_NONE) {
                    hsd->ErrorCode |= errorstate;
                    HAL_SD_ErrorCallback(hsd);
                }
            }
 
            /* Clear all the static flags */
            __HAL_SD_CLEAR_FLAG(hsd, SDMMC_STATIC_FLAGS);
 
             hsd->State = HAL_SD_STATE_READY;
            if (((hsd->Context & SD_CONTEXT_READ_SINGLE_BLOCK) != RESET)
                || ((hsd->Context & SD_CONTEXT_READ_MULTIPLE_BLOCK) != RESET)) {
                HAL_SD_RxCpltCallback(hsd);
            } else {
                HAL_SD_TxCpltCallback(hsd);
            }
        } else if ((hsd->Context & SD_CONTEXT_DMA) != RESET) {
            if ((hsd->Context & (SD_CONTEXT_WRITE_MULTIPLE_BLOCK | SD_CONTEXT_READ_MULTIPLE_BLOCK)) != RESET) {
                errorstate = SDMMC_CmdStopTransfer(hsd->Instance);
                if (errorstate != HAL_SD_ERROR_NONE) {
                    hsd->ErrorCode |= errorstate;
                    HAL_SD_ErrorCallback(hsd);
                }
            }
            if (((hsd->Context & SD_CONTEXT_READ_SINGLE_BLOCK) == RESET)
                && ((hsd->Context & SD_CONTEXT_READ_MULTIPLE_BLOCK) == RESET)) {
                /* Disable the DMA transfer for transmit request by setting the DMAEN bit
        in the SD DCTRL register */
                hsd->Instance->DCTRL &= (uint32_t) ~((uint32_t)SDMMC_DCTRL_DMAEN);
 
                hsd->State = HAL_SD_STATE_READY;
 
                HAL_SD_TxCpltCallback(hsd);
            }
        }
 
[...]
 
}

to

void HAL_SD_IRQHandler(SD_HandleTypeDef* hsd)
{
    uint32_t errorstate = HAL_SD_ERROR_NONE;
 
    /* Check for SDMMC interrupt flags */
    if (__HAL_SD_GET_FLAG(hsd, SDMMC_IT_DATAEND) != RESET) {
        __HAL_SD_CLEAR_FLAG(hsd, SDMMC_FLAG_DATAEND);
 
        __HAL_SD_DISABLE_IT(
            hsd, SDMMC_IT_DATAEND | SDMMC_IT_DCRCFAIL | SDMMC_IT_DTIMEOUT | SDMMC_IT_TXUNDERR | SDMMC_IT_RXOVERR);
 
        if ((hsd->Context & (SD_CONTEXT_IT|SD_CONTEXT_DMA)) != RESET) {
            if (((hsd->Context & SD_CONTEXT_READ_MULTIPLE_BLOCK) != RESET)
                || ((hsd->Context & SD_CONTEXT_WRITE_MULTIPLE_BLOCK) != RESET)) {
                errorstate = SDMMC_CmdStopTransfer(hsd->Instance);
                if (errorstate != HAL_SD_ERROR_NONE) {
                    hsd->ErrorCode |= errorstate;
                    HAL_SD_ErrorCallback(hsd);
                }
            }
 
            /* Clear all the static flags */
            __HAL_SD_CLEAR_FLAG(hsd, SDMMC_STATIC_FLAGS);
 
            if (((hsd->Context & SD_CONTEXT_READ_SINGLE_BLOCK) != RESET)
                || ((hsd->Context & SD_CONTEXT_READ_MULTIPLE_BLOCK) != RESET)) {
                HAL_SD_RxCpltCallback(hsd);
            } else {
                HAL_SD_TxCpltCallback(hsd);
            }
            if((hsd->Context & SD_CONTEXT_DMA) != RESET){
                hsd->Instance->DCTRL &= (uint32_t) ~((uint32_t)SDMMC_DCTRL_DMAEN);
            }
            hsd->State = HAL_SD_STATE_READY;
            hsd->Context = SD_CONTEXT_NONE;
        }
 
[...]
 
}

and

static void SD_DMAReceiveCplt(DMA_HandleTypeDef* hdma)
{
    SD_HandleTypeDef* hsd        = (SD_HandleTypeDef*)(hdma->Parent);
    uint32_t          errorstate = HAL_SD_ERROR_NONE;
 
    /* Send stop command in multiblock write */
    if (hsd->Context == (SD_CONTEXT_READ_MULTIPLE_BLOCK | SD_CONTEXT_DMA)) {
        errorstate = SDMMC_CmdStopTransfer(hsd->Instance);
        if (errorstate != HAL_SD_ERROR_NONE) {
            hsd->ErrorCode |= errorstate;
            HAL_SD_ErrorCallback(hsd);
        }
    }
 
    /* Disable the DMA transfer for transmit request by setting the DMAEN bit
  in the SD DCTRL register */
    hsd->Instance->DCTRL &= (uint32_t)~((uint32_t)SDMMC_DCTRL_DMAEN);
 
    /* Clear all the static flags */
    __HAL_SD_CLEAR_FLAG(hsd, SDMMC_STATIC_FLAGS);
    
 
    hsd->State = HAL_SD_STATE_READY;
 
    HAL_SD_RxCpltCallback(hsd);
}

to

static void SD_DMAReceiveCplt(DMA_HandleTypeDef* hdma)
{
    SD_HandleTypeDef* hsd        = (SD_HandleTypeDef*)(hdma->Parent);
 
    /* Enable DATAEND Interrupt */
    __HAL_SD_ENABLE_IT(hsd, (SDMMC_IT_DATAEND));
}

Can someone confirm this ?

[TSche.17 · Accepted solution]

Yes, confirmed (32-bit DTIMER overflow @48MHz = 90sec). The F7 SD HAL using DMA is a mess, I had to rewrite many more parts of it. Also, all the expensive calls to SDMMC_CmdStopTransfer (or even HAL_SD_GetCardState) inside interrupt do look crazy. Further, SD HAL code makes naive assumptions on execution order of the two (SD+DMA) interrupt handlers.