Re: [PATCH rtw-next v5 3/4] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

["Luka Gejak" <luka.gejak@linux.dev>]

Hi Alastair,

On Sun Oct 11, 2026 at 10:07 PM UTC, Alastair D'Silva wrote:
> On 3081-based SDIO chips (e.g. RTL8821CS), hardware does not automatically
> clear REG_SDIO_HISR_RX_REQUEST when the RX buffer is empty. Masking this
> bit out before writing back to HISR prevents it from being acknowledged,
> causing an infinite interrupt storm loop that locks up the system.
>
> 8051-based chips instead rely on hardware to clear this bit automatically
> once the buffer is empty, and software must not clear it.
>
> Split rtw_sdio_handle_interrupt() into separate 8051 and 3081 handlers.
> For 3081, disable interrupts via HIMR, acknowledge pending status bits
> via W1C writeback, service pending events, and re-enable interrupts under
> irq_lock if still running. For 8051 chips, preserve the existing behavior.
>
> Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  drivers/net/wireless/realtek/rtw88/sdio.c | 62 +++++++++++++++++------
>  1 file changed, 46 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index 7d4f17e96648..9fbd7654ada4 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
[...]
> +     rtw_sdio_disable_interrupt(rtwdev);
> +     rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
> +
> +     if (hisr & REG_SDIO_HISR_TXERR)
> +             rtw_sdio_tx_err_isr(rtwdev);
> +     if (hisr & REG_SDIO_HISR_RX_REQUEST)
> +             rtw_sdio_rx_isr(rtwdev);
> +
> +     /* Unmasking HIMR re-asserts the IRQ line if new packets arrived */
> +     scoped_guard(mutex, &rtwsdio->irq_lock) {
> +             if (rtwsdio->running)
> +                     rtw_sdio_enable_interrupt(rtwdev);
> +     }

The ack goes out before the drain, so RX_REQUEST is cleared on the card
before rtw_sdio_rx_isr() has read anything. That is fine while the drain ends
with RX0_REQ_LEN reading zero. It does not end that way in two cases, when
total_rx_bytes reaches SZ_64K and when rtw_sdio_rxfifo_recv() returns an
error from 1/4. There the data is left in the FIFO with the bit already
cleared, and the interface depends on the card setting RX_REQUEST again for
data that is already sitting in the buffer. The comment only claims that for
newly arrived packets.

Your RX test probably covers this, 65 MB in 15 s would have hit the 64K
budget many times, and a stalled interface would have shown up. I would still
want either the datasheet sentence that the bit is set while the RX buffer is
not empty, or a second look after the unmask:

        if (rtw_read32(rtwdev, REG_SDIO_HISR) & REG_SDIO_HISR_RX_REQUEST)
                rtw_sdio_rx_isr(rtwdev);

> +     rtw_write32(rtwdev, REG_SDIO_HISR, hisr);

Here the whole snapshot goes back. On 8051 the write back is masked to
RTW_SDIO_HISR_CLEAR_MASK, because undefined bits written back raise the
interrupt again after resume, and that mask leaves out RX_REQUEST and also
AVAL, bits 8 to 15, BCNERLY_INT, ATIMEND, ATIMEND_E and CTWEND. RX_REQUEST is
the only bit that has to go on 3081, so would

        rtw_write32(rtwdev, REG_SDIO_HISR,
                          hisr & (RTW_SDIO_HISR_CLEAR_MASK |
                                REG_SDIO_HISR_RX_REQUEST));

be enough? If the full write back is deliberate, I would say why.

>       hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
> +     if (!hisr)
> +             goto out;

include/linux/cleanup.h:148 asks that goto and the cleanup helpers are not
mixed in one function, and this function now has both. Wrapping the two
handler calls in if (hisr) { ... } would remove the label.

Two questions on the cover. The storm is described for 8821CS and 8822CS
while the validation lists RTL8821CS only. Was 8822CS observed, or is it
inferred from the same 3081 behaviour? And the board tested is sunxi-mmc,
which runs the handler from ksdioirqd. sdhci, dw_mmc, meson-gx-mmc, mtk-sd
and mmci set MMC_CAP2_SDIO_IRQ_NOTHREAD, so there the handler runs from
sdio_irq_work and the host has its own way of rearming the card IRQ. A line
in the cover saying which of the two the mask and unmask timing was tested on
would help.

Best regards,
Luka Gejak