RE: [PATCH] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Ping-Ke Shih]

Alastair D'Silva <alastair@xxxxxxxxxxx> wrote:
> On Thu, 2026-09-17 at 09:09 +0000, Ping-Ke Shih wrote:
> > Ping-Ke Shih wrote:
> > >
> > > Hi Martin,
> > >
> > > Alastair D'Silva <alastair@xxxxxxxxxxx> wrote:
> > > > In rtw_sdio_handle_interrupt(), the HISR status register is
> > > > cleared using
> > > > Write-1-to-Clear (W1C) semantics. However, the driver masks out
> > > > the
> > > > REG_SDIO_HISR_RX_REQUEST bit in the local 'hisr' variable before
> > > > writing
> > > > it back, causing a 0 to be written to that bit.
> > >
> > > I feel this patch makes sense.
> > >
> > > Did you remember why you clear the bit locally? Can you share
> > > vendor
> > > driver you referenced?
> > >
> >
> > I asked internal expert who said this bit will be cleared
> > automatically
> > if all packets in RX buffer are received. If we clear this bit here,
> > the interrupt will not raise again if no newly incoming packet even
> > there are remaining packets in RX buffer.
> >
> > Checking rtw_sdio_rx_isr(), we can see it reads hisr for each
> > iteration,
> > which this is also the evidence that the bit will be cleared
> > automatically.
> >
> > Therefore, we need to dig further why the RX buffer can't be empty
> > and get stuck.
> >
> > Ping-Ke
> >
> 
> Hi Ping-Ke and Martin,
> 
> Thank you for following up and consulting with the internal hardware
> team. We have extensive test telemetry and logs from our bring-up of
> the Mellow Fly-C5 board in Armbian that provide full context on this
> issue.
> 
> 1. Test Platform & Failure Telemetry:
> -------------------------------------
> - Hardware: Mellow Fly-C5 (Allwinner H618 SoC, sun50i-h618).
> 
> - Wi-Fi Chip: Onboard Realtek RTL8821CS connected over SDIO (mmc1 /
> sunxi-mmc).
> 
> - Kernels: Tested on mainline Linux 6.18 and 7.x branches using
> rtw88_8821cs.
> 
> - Reproduction: On stock mainline kernels, immediately upon interface
> bring-up and association (wlan0: associated), the CPU core servicing
> the SDIO IRQ became 100% pegged in an interrupt storm, triggering RCU
> stalls:
>     rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
>             0-...0: (1 GPs behind) idle=... softirq=...
>     rcu: rcu_preempt kthread starved for 5126 jiffies!

Does it mean even if RX buffer is empty, interrupt is still triggered?

> 
> - Verification: With this patch applied (leaving
> REG_SDIO_HISR_RX_REQUEST set so W1C acknowledges it), the board
> completed 100/100 consecutive reboots under active network traffic
> (continuous ping, SSH, Moonraker HTTP) with zero RCU stalls and zero
> packet loss.
> 
> 
> 2. Why the bit was cleared locally (Vendor driver reference):
> -------------------------------------------------------------
> 
> In Realtek's vendor drivers (such as rtl8822bs and rtl8723ds):
> - In hal/rtl8822b/sdio/rtl8822bs_io.c:
>     if (phal->sdio_hisr & BIT_RX_REQUEST_8822B) {
>         /* No need to write 1 clear for RX_REQUEST */
>         phal->sdio_hisr ^= BIT_RX_REQUEST_8822B;
> - In include/hal_com_reg.h:
>     MASK_SDIO_HISR_CLEAR explicitly excludes SDIO_HISR_RX_REQUEST.
> 
> Martin appears to have ported this comment and software-clearing
> convention directly into rtw88_sdio_handle_interrupt().

Thanks for the info. It looks like every SDIO chip does the same thing.

> 
> 
> 3. Difference between 8051 and 3081 chips in rtw_sdio_rx_isr():
> ---------------------------------------------------------------
> 
> Regarding the observation that rtw_sdio_rx_isr() re-reads HISR in each
> iteration:
> 
> Notice that in rtw_sdio_rx_isr() (sdio.c:1057-1073):
>     if (rtw_chip_wcpu_8051(rtwdev)) {
>         hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
>     } else {
>         /* RTW_WCPU_3081 chips have improved hardware or
>          * firmware and can use rx_len unconditionally.
>          */
>         hisr = REG_SDIO_HISR_RX_REQUEST;
>     }
> 
> RTL8821CS (and RTL8822C/B) has wlan_cpu == RTW_WCPU_3081. For 3081
> chips, rtw_sdio_rx_isr() never actually reads REG_SDIO_HISR! It sets
> hisr to REG_SDIO_HISR_RX_REQUEST unconditionally and loops purely based
> on REG_SDIO_RX0_REQ_LEN.

Indeed. I also found that after sending previous mail...

> 
> 
> 4. The RX buffer IS empty when the storm occurs:
> ------------------------------------------------
> 
> The buffer is not getting stuck with unread data. In our testing:
> 
> - rtw_sdio_rx_isr() drains all available packets until
> REG_SDIO_RX0_REQ_LEN reads as 0, and then breaks out of the loop.
> 
> - rtw_sdio_handle_interrupt() then writes back to REG_SDIO_HISR with
>   REG_SDIO_HISR_RX_REQUEST masked out (writing 0).
> 
> - Because the bit was not cleared in hardware via W1C (and was not
> automatically de-asserted by hardware when rx_len reached 0), the SDIO
> host controller sees the IRQ line still asserted and immediately re-
> invokes the handler.
> 
> - On re-entry, REG_SDIO_HISR_RX_REQUEST is still 1, but
> REG_SDIO_RX0_REQ_LEN is 0. rtw_sdio_rx_isr() immediately breaks out,
> hisr writes 0 again, and the CPU is trapped in a 100% spin loop.

This answers my question above.

> 
> 
> 5. Effect on future interrupts:
> -------------------------------
> 
> Regarding the internal expert's concern that clearing the bit prevents
> future interrupts: in our testing, writing 1 to clear
> REG_SDIO_HISR_RX_REQUEST after the FIFO is drained did NOT prevent
> subsequent RX interrupts. When new packets arrived over the air, the
> hardware asserted REG_SDIO_HISR_RX_REQUEST again normally.
> 
> If there is concern about edge cases (such as hitting the 64KB
> total_rx_bytes limit before the FIFO is completely empty), would it be
> acceptable to only clear REG_SDIO_HISR_RX_REQUEST if
> REG_SDIO_RX0_REQ_LEN reads 0, or re-read REG_SDIO_HISR at the end of
> rtw_sdio_rx_isr()?

I guess there is a racing between W1C REG_SDIO_HISR_RX_REQUEST and
REG_SDIO_RX0_REQ_LEN == 0.

With a suggestion from internal, if we want to disable the RX request,
the better way is to disable/enable it by IMR. The corresponding 
functions are:

   rtw_sdio_enable_interrupt()
   rtw_sdio_disable_interrupt()

To avoid interrupt storm, I personally suggest to combine NAPI, which
disable interrupt when it processes RX budget (I think we can W1C
REG_SDIO_HISR_RX_REQUEST by the way). If (RX) budget is full, it can
poll again by estimated time. Until budget is not full, it re-enable
interrupt. 

Ping-Ke