[Bug]: HAL_SD_IRQHandler never reaches its error branch on data error in IT mode (IRQ storm)

[mrjimenez]

Bug Summary

HAL_SD_IRQHandler never reaches its error branch when a data error occurs
during an interrupt-mode transfer, causing the SD interrupt to refire forever
and lock up the system.

Detailed Description

In stm32h7xx_hal_sd.c, HAL_SD_IRQHandler() tests its conditions in a single
if / else if chain, and the FIFO branches come before the error branch:

void HAL_SD_IRQHandler(SD_HandleTypeDef *hsd)
{
  /* Check for SDMMC interrupt flags */
  if ((__HAL_SD_GET_FLAG(hsd, SDMMC_FLAG_RXFIFOHF) != RESET) && ((context & SD_CONTEXT_IT) != 0U))
  {
    SD_Read_IT(hsd);
  }
  ...
  else if ((__HAL_SD_GET_FLAG(hsd, SDMMC_FLAG_TXFIFOHE) != RESET) && ((context & SD_CONTEXT_IT) != 0U))
  {
    SD_Write_IT(hsd);
  }
  ...
  else if (__HAL_SD_GET_FLAG(hsd, SDMMC_FLAG_DCRCFAIL | SDMMC_FLAG_DTIMEOUT |
                                  SDMMC_FLAG_RXOVERR  | SDMMC_FLAG_TXUNDERR) != RESET)
  {
    /* error handling: never reached in the scenario below */
  }
}

TXFIFOHE and RXFIFOHF are level-triggered status flags that cannot be
cleared by software — they only change when the FIFO is drained or filled. If a
data error (TXUNDERR, RXOVERR, DCRCFAIL, DTIMEOUT) occurs while such a
flag is asserted, the handler keeps taking the FIFO branch, the error branch is
never evaluated, the error flags are never cleared, and the interrupt refires
immediately and indefinitely.

A second, related problem: when the error branch does run, its
__HAL_SD_DISABLE_IT() mask does not include SDMMC_IT_TXFIFOHE /
SDMMC_IT_RXFIFOHF, so the IRQ line can remain asserted even after the error is
handled.

How it was observed. An SD card stopped accepting data mid-write (a physical
card fault; only a power cycle recovers it). The resulting TXUNDERR left
TXFIFOHE asserted and the board became completely unresponsive: the ISR
starved every thread, so networking and the console died while interrupts kept
firing. Diagnosed via SWD — the CPU was permanently inside HAL_SD_IRQHandler.

Suggested fix, which has been running in our firmware since 2026-07-10
(including a 10-minute stress test writing 512 KB blocks continuously while
acquiring sensor data, with no regression):

1. skip the FIFO branches while any data error flag is pending, so the error
   branch becomes reachable;
2. add SDMMC_IT_TXFIFOHE / SDMMC_IT_RXFIFOHF to the disable mask on the
   error path.

The patch is proposed to the Zephyr HAL mirror as a temporary fix in
zephyrproject-rtos/hal_stm32#395, whose maintainer asked that the issue be
reported here first.

Note on other series. The same if / else if ordering is present in the
HAL_SD_IRQHandler of the STM32F7, H5, H7RS, L4, L5, MP13, MP2, N6, U3 and U5
drivers. I could only test on STM32H7.

Expected Behavior

A data error during an interrupt-mode transfer is handled: the error flags are cleared, the interrupts are disabled, and the transfer is aborted with the error reported to the application.

Actual Behavior

The error branch is never reached; the interrupt refires forever and starves the entire system, which requires a reset to recover.

Environment

STM32H747I-DISCO (STM32H747XI, Cortex-M7), STM32CubeH7 v1.12.1, SDMMC1 in interrupt mode (not IDMA), GCC (Zephyr SDK), Zephyr RTOS 4.4.1.

Severity

Critical

[ALABSTM]

Hello @mrjimenez,

Thank you for this thorough and detailed report, and for the proposed fix. A question though: would it not also be an alternative to move the error flags check before the FIFO checks? This would:

• keep the FIFO checks simpler, with fewer conditions,
• avoid disabling the FIFO interrupts as in your proposal, which would change the current driver's behaviour, otherwise.

With regards,

[mrjimenez]

Hello @ALABSTM,

Thanks for looking into this.

You are right about the ordering, and I think your version is better than
mine: moving the error check ahead of the FIFO checks fixes the
reachability problem with one condition less in each FIFO branch, instead
of repeating the error mask in two places. I can rework the patch that way.

However, I believe the second part, disabling the FIFO interrupts on the
error path, is still needed, because reordering alone does not deassert
the IRQ line. My reasoning, reading the v1.12.1 sources:

1. HAL_SD_WriteBlocks_IT() enables SDMMC_FLAG_TXFIFOHE (and
   HAL_SD_ReadBlocks_IT() enables SDMMC_FLAG_RXFIFOHF).
2. The error branch disables DATAEND | DCRCFAIL | DTIMEOUT | TXUNDERR | RXOVERR,
   but not the FIFO interrupts.
3. The same branch performs DCTRL |= SDMMC_DCTRL_FIFORST. With the
   transmit FIFO emptied, TXFIFOHE ("half empty") is asserted, and
   being a level-triggered status flag, the software cannot clear it.
4. The branch then sets Context = SD_CONTEXT_NONE.

So on the next entry to the handler, with your ordering: the error branch
is not taken (the flags were cleared), and the FIFO branch is not taken
either, because (context & SD_CONTEXT_IT) is now false. The handler
returns having done nothing, while the interrupt is still asserted. The
storm becomes a loop that spins without doing any work, rather than one
that keeps servicing the FIFO.

On the behaviour change you mention: the FIFO interrupts would only be
disabled on a transfer that is being aborted, and HAL_SD_ReadBlocks_IT()
/ HAL_SD_WriteBlocks_IT() re-enable them for the next one. I could not
find a legitimate behaviour that this would break, but you know the
driver far better than I do. If there is one, please let me know.

My suggestion would be to combine both: your reordering plus the FIFO
interrupt disable on the error path. That ends up smaller than my original
patch.

Just one thing I would like to add: the analysis above is read from the
code, not measured. The original failure depended on a physical card fault
that I cannot reproduce on demand, since the card that caused it now behaves
normally, so I have not been able to test the reorder-only variant on my
hardware.

That said, if you agree, I can prepare the combined patch.

Best regards,
Marcelo.

[ALABSTM]

Hello @mrjimenez,

I also thought of an even simpler fix consisting in getting rid of the nesting by changing the if ... else if ... else if structure into sequential if ... if ... if blocks, without changing the checks order.

Anyway, I would prefer to wait for @KRASTM and our development team to have a closer look at your proposal and our discussion so far before proceeding any further.

With regards,

[mrjimenez]

Hello @ALABSTM,

Understood, I will wait for @KRASTM and the team. Just leaving two notes
that may be useful for that review.

On de-nesting. I think it is the most robust of the three options. It
fixes reachability without depending on the order at all, so a flag added
later in any position still gets evaluated, whereas both my version and
your reordering stay sensitive to whoever edits the chain next. It is also
what was done for the same class of bug in HAL_DFSDM_IRQHandler, where
only the first flag encountered was being handled, your internal report
#HAL1-27454, which reached us as commit d3a20294 in the Zephyr HAL mirror.

One thing worth checking with de-nesting: several branches would now be
able to run in the same invocation. The pair I would look at is
DATAEND together with an error flag, since both the completion callback
and the error callback would then fire for the same transfer.

On the behaviour concern you raised earlier. I went back to the code,
and the DATAEND branch already does what I proposed for the error path:

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

The normal completion path masks TXFIFOHE/RXFIFOHF; only the error path
omits them. So adding them there would not change the driver's behaviour so
much as make the error path consistent with the normal one.

This part is orthogonal to the three options: none of them, on its own,
deasserts the IRQ line when an error occurs without DATAEND.

With regards,
Marcelo.

[KRASTM]

Hello All,

Thank you, @mrjimenez for bringing this to our attention, and @ALABSTM, for your assistance.

Regarding the issue, I believe both solutions are valid, either moving the error flags check before the FIFO checks or check both conditions in the same statement (errors Flags + FIFO Flag). In addition, the change made after adding the masks TXFIFOHE/RXFIFOHF to __HAL_SD_DISABLE_IT() should be kept.

For the second point, concerning the De-nesting option, I believe it is not applicable to SDMMC since we cannot have both a DATAEND callback and an error callback for the same transfer. However, I will share all proposals with our team for review and keep you updated.

With regards,