[PATCH v3 0/2] wifi: rtw88: sdio: Fix interrupt storm on 3081 chips and split RX handling

[PATCH v3 0/2] wifi: rtw88: sdio: Fix interrupt storm on 3081 chips and split RX handling

[Alastair D'Silva]

This series resolves an unhandled RX request interrupt storm on 3081-based
SDIO chips (RTL8821CS, RTL8822CS) and cleans up the RX interrupt handling
path.

On 3081 SDIO chips, REG_SDIO_HISR_RX_REQUEST is not cleared automatically
by hardware when the RX FIFO is empty. Masking this bit out in software
before writing back to HISR prevented it from ever being acknowledged in
hardware, trapping the CPU core in an infinite interrupt storm loop.

In v2, HIMR masking and early writeback were introduced. As Luka Gejak
pointed out, this broke the drain loop on 8051 chips (RTL8723BS, RTL8723CS,
RTL8723DS) because 8051 hardware automatically clears the bit when empty
and re-reads HISR across iterations. Clearing it early causes 8051 to abort
draining after a single packet. In addition, RTL8723BS in rtw-next requires
RTW_SDIO_HISR_CLEAR_MASK to prevent undefined bits from causing resume
storms.

As the number of architecture-specific special cases has grown (16-bit vs
32-bit register widths, differing HISR writeback timing, synthetic loop
flags, and RTL8723BS resume masking), attempting to accommodate both
architectures within a single monolithic handler has become fragile and
prone to cross-architecture regressions.

To cleanly address both architectures without compromises:
- Patch 1 makes rtw_sdio_handle_interrupt() a dispatcher with separate
  8051 and 3081 handlers. 8051 retains its original unmasked writeback
  and RTW_SDIO_HISR_CLEAR_MASK, while 3081 adopts the HIMR masking and
  W1C acknowledgment pattern. Spurious IRQ check (hisr == 0) and early exit
  are moved into the dispatcher.
- Patch 2 splits rtw_sdio_rx_isr() into separate 8051 and 3081 variants,
  eliminating per-packet branches in the RX hot path and removing the
  artificial hisr assignment on 3081.

Hardware Verification & Empirical Testing (Fly-C5 / RTL8821CS / Linux 7.2):
===========================================================================
Testing was performed on an Allwinner H618 (Mellow Fly-C5) with an onboard
RTL8821CS SDIO Wi-Fi module running Linux 7.2.8 over wlan0.
Instrumentation was added to measure ISR durations, HIMR toggle overhead,
and packet arrivals occurring while interrupts were masked.

1. Baseline Performance (0 us delay, wlan0):
   - TCP Saturation (iperf3 -P4): 35.1 Mbps, 15,988 RX IRQs, 0 RCU stalls.
   - UDP Saturation (iperf3 -u -b 40M): 36.7 Mbps, 32,271 datagrams
     received, 0 lost (0.0% packet loss), 1,342 masked arrivals (7.5%).
   - HIMR toggle overhead (disable + enable): 34.2 us (10.3% of ISR time).

2. Injected Critical Section Delays (Race-Window & Masking Verification):
   To verify that packet arrivals during interrupt servicing are reliably
   latched and re-asserted upon unmasking, delays were injected between W1C
   acknowledgment and unmasking:

   Delay (us) | Traffic | Total IRQs | Masked Arrv | % IRQs | Loss
   -----------+---------+------------+-------------+--------+------
   0 us       | UDP 40M | 17,786     | 1,342       | 7.5%   | 0.0%
   500 us     | UDP 40M | 9,036      | 1,364       | 15.1%  | 0.0%
   2,000 us   | UDP 40M | 3,892      | 458         | 11.8%  | 0.0%
   10,000 us  | UDP 40M | 1,317      | 217         | 16.5%  | 0.0%
   10,000 us  | TCP sat | 1,048      | 171         | 16.5%  | 24 retr

   Explanation of Masked Arrival Dynamics:
   A counterintuitive observation is that the raw count of masked arrivals
   decreases at longer delays (1,342 -> 217 at 10 ms). This occurs because:
   a) The metric counts *interrupt invocations* where at least one packet
      arrived while masked, not individual packets.
   b) With a 10 ms delay per ISR, the system can only execute ~1,300
      total interrupts during a 10-second test (down from ~17,800).
      The proportion of interrupts with masked arrivals more than
      doubled (7.5% -> 16.5%).
   c) RTL8821CS has a 16 KB RX FIFO. At 40 Mbps, the FIFO fills in ~3.2 ms.
      Once full, 802.11 block ACKs pause and over-the-air transmission
      stops until the host drains the FIFO. Each interrupt then drains
      the accumulated data in a single burst (RX drain time grew from
      234 us to 912 us).
   d) Zero packet loss (0.0%) was maintained across all delay
      configurations.

v3:
- Rebased on rtw-next.
- Preserved 8051 interrupt handling behavior and RTW_SDIO_HISR_CLEAR_MASK
  by splitting rtw_sdio_handle_interrupt() into separate 8051 and 3081
  dispatch routines (thanks Luka Gejak for the review and analysis).
- Moved hisr reading and early exit on spurious IRQ into the dispatcher.
- Split rtw_sdio_rx_isr() into 8051 and 3081 implementations (patch 2).
- Added hardware benchmark metrics and delay-injected test results under
  the cut line.
- Added Assisted-by tag per Documentation/process/coding-assistants.rst.

v2:
- Reworked interrupt acknowledgment to use HIMR masking
  (rtw_sdio_disable_interrupt / rtw_sdio_enable_interrupt) to avoid races
  between reading RX0_REQ_LEN and clearing HISR.

v1:
- https://lore.kernel.org/linux-wireless/20260921085502.123456-1-alastair@d-silva.org/

Alastair D'Silva (2):
  wifi: rtw88: sdio: Fix unhandled RX request interrupt storm
  wifi: rtw88: sdio: Split rtw_sdio_rx_isr into 8051 and 3081 variants

 drivers/net/wireless/realtek/rtw88/sdio.c | 95 +++++++++++++++--------
 1 file changed, 62 insertions(+), 33 deletions(-)

-- 
2.53.0

[PATCH v3 1/2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Alastair D'Silva]

8051 and 3081 SDIO chipsets handle the REG_SDIO_HISR_RX_REQUEST status bit
differently:

- 8051-based chips (e.g. RTL8723BS, RTL8723CS, RTL8723DS):
  The hardware automatically clears REG_SDIO_HISR_RX_REQUEST once the RX
  buffer is empty. Software must not clear this bit, because the drain
  loop in rtw_sdio_rx_isr() re-reads REG_SDIO_HISR across iterations to
  decide whether more requests are pending. Clearing it in software
  terminates the loop after a single request, stranding the remainder of
  the FIFO. Additionally, RTL8723BS requires RTW_SDIO_HISR_CLEAR_MASK to
  avoid undefined bits causing resume storms.

- 3081-based chips (e.g. RTL8821CS, RTL8822CS):
  The hardware does not automatically clear REG_SDIO_HISR_RX_REQUEST when
  the RX buffer is empty. Masking this bit out in software before writing
  back to HISR prevented it from ever being acknowledged in hardware,
  trapping the CPU core in an infinite interrupt storm loop that starved
  RCU and locked up the system. Furthermore, on 3081 chips the physical
  RX FIFO capacity is at most 24 KB (16 KB on RTL8821CS, 24 KB on
  RTL8822CS), which is well within the 64 KB loop budget.

As the number of architecture-specific special cases has grown (16-bit vs
32-bit register widths, differing HISR writeback timing, synthetic loop
flags, and RTL8723BS resume masking), attempting to accommodate both
architectures within a single monolithic handler has become fragile and
prone to cross-architecture regressions.

Resolve this by making rtw_sdio_handle_interrupt() a dispatcher with
separate paths for 8051 and 3081:

1. 8051 chips preserve the existing unmasked writeback behavior, leaving
   REG_SDIO_HISR_RX_REQUEST for hardware to drop and respecting
   RTW_SDIO_HISR_CLEAR_MASK on RTL8723BS.
2. 3081 chips adopt the interrupt masking pattern: disable HIMR,
   acknowledge pending status bits in HISR via W1C, service the pending
   events, and re-enable HIMR. Any packet arriving during servicing
   latches REG_SDIO_HISR_RX_REQUEST in hardware and re-asserts the IRQ line
   once unmasked.

Splitting into separate handlers isolates these quirks cleanly and
simplifies future maintenance.

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>
---
v3:
- Rebased on rtw-next.
- Preserved 8051 interrupt handling behavior and RTW_SDIO_HISR_CLEAR_MASK
  by splitting rtw_sdio_handle_interrupt() into separate 8051 and 3081
  dispatch routines (thanks Luka Gejak for the review and analysis).
- Moved hisr reading and early exit on spurious IRQ into the dispatcher.
- Verified on Allwinner H618 (Mellow Fly-C5) with RTL8821CS under Linux 7.2:
  * Baseline (0 us delay): 36.7 Mbps UDP (0.0% loss), 35.1 Mbps TCP, 0 RCU stalls.
    HIMR toggle overhead is 34.2 us (10.3% of ISR duration).
  * Injected delays (500 us, 2 ms, 10 ms): zero packet loss (0.0%), masked
    arrivals verified latched and re-asserted upon unmasking (7.5% -> 16.5%
    of IRQs catching masked packet arrivals).
- Added Assisted-by tag per Documentation/process/coding-assistants.rst.

v2:
- Reworked interrupt acknowledgment to use HIMR masking
  (rtw_sdio_disable_interrupt / rtw_sdio_enable_interrupt) to avoid races
  between reading RX0_REQ_LEN and clearing HISR.

v1:
- https://lore.kernel.org/linux-wireless/20260921085502.123456-1-alastair@d-silva.org/

 drivers/net/wireless/realtek/rtw88/sdio.c | 51 +++++++++++++++++------
 1 file changed, 38 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index dc2fd0f8f9ff..63d62242ac1e 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1455,20 +1455,8 @@ static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 	} while (total_rx_bytes < SZ_64K && hisr & REG_SDIO_HISR_RX_REQUEST);
 }
 
-static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
+static void rtw_sdio_handle_interrupt_8051(struct rtw_dev *rtwdev, u32 hisr)
 {
-	struct ieee80211_hw *hw = sdio_get_drvdata(sdio_func);
-	struct rtw_sdio *rtwsdio;
-	struct rtw_dev *rtwdev;
-	u32 hisr;
-
-	rtwdev = hw->priv;
-	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
-
-	rtwsdio->irq_thread = current;
-
-	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
-
 	if (hisr & REG_SDIO_HISR_TXERR)
 		rtw_sdio_tx_err_isr(rtwdev);
 	if (hisr & REG_SDIO_HISR_RX_REQUEST) {
@@ -1485,7 +1473,44 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 		hisr &= RTW_SDIO_HISR_CLEAR_MASK;
 
 	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
+}
+
+static void rtw_sdio_handle_interrupt_3081(struct rtw_dev *rtwdev, u32 hisr)
+{
+	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 */
+	rtw_sdio_enable_interrupt(rtwdev);
+}
+
+static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
+{
+	struct ieee80211_hw *hw = sdio_get_drvdata(sdio_func);
+	struct rtw_sdio *rtwsdio;
+	struct rtw_dev *rtwdev;
+	u32 hisr;
+
+	rtwdev = hw->priv;
+	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	rtwsdio->irq_thread = current;
+
+	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
+	if (!hisr)
+		goto out;
+
+	if (rtw_chip_wcpu_8051(rtwdev))
+		rtw_sdio_handle_interrupt_8051(rtwdev, hisr);
+	else
+		rtw_sdio_handle_interrupt_3081(rtwdev, hisr);
 
+out:
 	rtwsdio->irq_thread = NULL;
 }
 
-- 
2.53.0

[PATCH v3 2/2] wifi: rtw88: sdio: Split rtw_sdio_rx_isr into 8051 and 3081 variants

[Alastair D'Silva]

Following the split of rtw_sdio_handle_interrupt(), the receive FIFO drain
loop in rtw_sdio_rx_isr() still contained a growing number of special
cases between 8051 and 3081 chips, evaluated twice per packet in the RX
hot path:

1. Register width: 8051 uses a 16-bit read of REG_SDIO_RX0_REQ_LEN,
   while 3081 uses a 32-bit read.
2. Loop termination: 8051 must re-read REG_SDIO_HISR on each iteration
   because the RX buffer may contain data while HW or FW is still filling
   it. Conversely, 3081 has improved HW/FW that can use rx_len
   unconditionally, previously requiring a synthetic assignment of
   hisr = REG_SDIO_HISR_RX_REQUEST to trick the loop condition into
   continuing.

To avoid accumulating further special cases and eliminate per-packet
branching in the RX hot path, split rtw_sdio_rx_isr() into separate
rtw_sdio_rx_isr_8051() and rtw_sdio_rx_isr_3081() functions.

This removes the artificial hisr assignment on 3081 and keeps the RX
processing logic cleanly separated by architecture.

Assisted-by: LLM
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
v3:
- New patch in v3.
- Split rtw_sdio_rx_isr into 8051 and 3081 variants following the dispatcher split.
- Eliminates per-packet wcpu branching in the RX hot path and removes artificial
  hisr assignments on 3081.
- Added Assisted-by tag per Documentation/process/coding-assistants.rst.

 drivers/net/wireless/realtek/rtw88/sdio.c | 54 ++++++++++++-----------
 1 file changed, 29 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 63d62242ac1e..fbf67427b450 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1418,16 +1418,12 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 	}
 }
 
-static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
+static void rtw_sdio_rx_isr_8051(struct rtw_dev *rtwdev)
 {
 	u32 rx_len, hisr, total_rx_bytes = 0;
 
 	do {
-		if (rtw_chip_wcpu_8051(rtwdev))
-			rx_len = rtw_read16(rtwdev, REG_SDIO_RX0_REQ_LEN);
-		else
-			rx_len = rtw_read32(rtwdev, REG_SDIO_RX0_REQ_LEN);
-
+		rx_len = rtw_read16(rtwdev, REG_SDIO_RX0_REQ_LEN);
 		if (!rx_len)
 			break;
 
@@ -1435,33 +1431,41 @@ static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 
 		total_rx_bytes += rx_len;
 
-		if (rtw_chip_wcpu_8051(rtwdev)) {
-			/* Stop if no more RX requests are pending, even if
-			 * rx_len could be greater than zero in the next
-			 * iteration. This is needed because the RX buffer may
-			 * already contain data while either HW or FW are not
-			 * done filling that buffer yet. Still reading the
-			 * buffer can result in packets where
-			 * rtw_rx_pkt_stat.pkt_len is zero or points beyond the
-			 * end of the buffer.
-			 */
-			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;
-		}
+		/* Stop if no more RX requests are pending, even if
+		 * rx_len could be greater than zero in the next
+		 * iteration. This is needed because the RX buffer may
+		 * already contain data while either HW or FW are not
+		 * done filling that buffer yet. Still reading the
+		 * buffer can result in packets where
+		 * rtw_rx_pkt_stat.pkt_len is zero or points beyond the
+		 * end of the buffer.
+		 */
+		hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
 	} while (total_rx_bytes < SZ_64K && hisr & REG_SDIO_HISR_RX_REQUEST);
 }
 
+static void rtw_sdio_rx_isr_3081(struct rtw_dev *rtwdev)
+{
+	u32 rx_len, total_rx_bytes = 0;
+
+	do {
+		rx_len = rtw_read32(rtwdev, REG_SDIO_RX0_REQ_LEN);
+		if (!rx_len)
+			break;
+
+		rtw_sdio_rxfifo_recv(rtwdev, rx_len);
+
+		total_rx_bytes += rx_len;
+	} while (total_rx_bytes < SZ_64K);
+}
+
 static void rtw_sdio_handle_interrupt_8051(struct rtw_dev *rtwdev, u32 hisr)
 {
 	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);
+		rtw_sdio_rx_isr_8051(rtwdev);
 	}
 
 	/*
@@ -1483,7 +1487,7 @@ static void rtw_sdio_handle_interrupt_3081(struct rtw_dev *rtwdev, u32 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);
+		rtw_sdio_rx_isr_3081(rtwdev);
 
 	/* Unmasking HIMR re-asserts the IRQ line if new packets arrived */
 	rtw_sdio_enable_interrupt(rtwdev);
-- 
2.53.0

Re: [PATCH v3 1/2] wifi: rtw88: sdio: Fix unhandled RX request interrupt storm

[Luka Gejak]

Hi Alastair,

The split looks good to me.

One issue with acknowledging the request up front:

> +	rtw_sdio_disable_interrupt(rtwdev);
> +	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);

When dev_alloc_skb() fails, rtw_sdio_rxfifo_recv() returns without
reading the port:

	skb = dev_alloc_skb(bufsz);
	if (!skb)
	  return;

but the loop counts the bytes anyway:

		rtw_sdio_rxfifo_recv(rtwdev, rx_len);

		total_rx_bytes += rx_len;

so a few failed reads burn the 64K budget and the loop exits with the
FIFO still full. The request is already cleared by then, and only a new
packet sets it again, which may never come if the FIFO is full and the
sender is paused. 8051 does not have this problem because it leaves the
bit to hardware. Can rtw_sdio_rxfifo_recv() report the failure, so the
loop can stop and retry instead? The read error just below the allocation
leaves the FIFO in the same state.

Same function, the enable at the end:

> +	/* Unmasking HIMR re-asserts the IRQ line if new packets arrived */
> +	rtw_sdio_enable_interrupt(rtwdev);

It also runs when the device has been stopped. ksdioirqd can still run a
handler that was signalled before rtw_sdio_stop() released the host, and
stop only writes HIMR as zero:

	static void rtw_sdio_stop(struct rtw_dev *rtwdev)
	{
		rtw_sdio_disable_interrupt(rtwdev);
	}

rtwsdio->irq_mask still has RX_REQUEST and CPWM1, so this puts the mask
back and arms the stopped device again. PCI clears rtwpci->running in its
stop path and only re-enables while it is still set:

	if (rtwpci->running)
		rtw_pci_enable_interrupt(rtwdev, rtwpci, rx);

Can you add something like that here?

Best regards,
Luka Gejak