Re: [PATCH] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm
From: Alastair D'Silva
Date: Mon Sep 21 2026 - 05:24:46 EST
On Mon, 2026-09-21 at 02:44 +0000, Ping-Ke Shih wrote:
> 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
Thanks for the feedback.
Regarding using NAPI and IMR: While NAPI and IMR masking is the standard approach for PCIe,
implementing true NAPI for the SDIO interface is problematic. napi_poll runs in NET_RX_SOFTIRQ
context (which cannot sleep), but reading from the SDIO bus requires sdio_claim_host(), which takes
a mutex and must be able to sleep.
Fortunately, the kernel's MMC core already runs sdio_irq_thread in process context specifically to
handle this. If we simply leave the REG_SDIO_HISR_RX_REQUEST bit asserted in hardware (by
conditionally skipping the W1C), the sdio_irq_thread acts exactly like a NAPI polling loop. It will
immediately re-invoke our handler in the next cycle, yielding to the scheduler as needed, but safely
in process context.
This achieves the budget-limited polling you suggested, but avoids the heavy overhead of extra SDIO
bus transactions to toggle the IMR on and off.
Before I spin a V2 and run through my testing, can you please confirm this aligns with what you were
expecting?
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..933e51a980a8 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1037,7 +1037,16 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
}
}
-static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
+/**
+ * rtw_sdio_rx_isr() - Process RX packets from SDIO hardware buffer
+ * @rtwdev: The rtw88 device context
+ *
+ * Reads packets from the hardware RX FIFO until the buffer is empty or the
+ * 64K packet budget is reached.
+ *
+ * Return: true if packets are still pending (budget reached), false otherwise.
+ */
+static bool rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
{
u32 rx_len, hisr, total_rx_bytes = 0;
@@ -1072,6 +1081,8 @@ static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
hisr = REG_SDIO_HISR_RX_REQUEST;
}
} while (total_rx_bytes < SZ_64K && hisr & REG_SDIO_HISR_RX_REQUEST);
+
+ return rx_len > 0;
}
static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
@@ -1091,8 +1102,14 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
if (hisr & REG_SDIO_HISR_TXERR)
rtw_sdio_tx_err_isr(rtwdev);
if (hisr & REG_SDIO_HISR_RX_REQUEST) {
- hisr &= ~REG_SDIO_HISR_RX_REQUEST;
- rtw_sdio_rx_isr(rtwdev);
+ bool rx_pending = rtw_sdio_rx_isr(rtwdev);
+
+ /* If budget was reached and packets are still pending, do not W1C
+ * the RX_REQUEST bit. This leaves the hardware IRQ asserted,
+ * causing sdio_irq_thread to poll again like a NAPI loop.
+ */
+ if (rx_pending)
+ hisr &= ~REG_SDIO_HISR_RX_REQUEST;
}
rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
--
2.53.0
--
Alastair D'Silva