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

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

[Alastair D'Silva]

On 3081-based SDIO chips (e.g. RTL8821CS, RTL8822CS), hardware does not
automatically clear the REG_SDIO_HISR_RX_REQUEST status bit 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 that locked up the system upon
receiving network traffic.

This series fixes the interrupt storm on 3081 chips and addresses
related SDIO lifecycle, locking, and error handling issues:

- Patch 1 returns error codes from rtw_sdio_rxfifo_recv() on allocation
  or read failures so the caller stops immediately instead of burning
  through the loop budget.
- Patch 2 tracks operational state via rtwsdio->running protected by an
  irq_lock mutex (using a mutex as SDIO bus operations sleep), serializes
  start/stop state transitions under sdio_claim_host() and irq_lock, and
  cancels the TX worker synchronously on stop.
- Patch 3 fixes the interrupt storm by splitting
  rtw_sdio_handle_interrupt() into 8051 and 3081 paths. On 3081,
  interrupts are masked via HIMR, acknowledged via W1C writeback,
  serviced, and re-enabled under irq_lock if still running.
- Patch 4 refactors rtw_sdio_rx_isr() into separate 8051 and 3081 variants
  to clarify the differences in register width and HISR handling without
  any logic changes.

Hardware Validation:
Validated on an Allwinner H618 board (Mellow Fly-C5) with RTL8821CS SDIO
running Linux 7.2.10, with the Ethernet interface administratively brought
down to ensure all traffic traversed the WiFi link exclusively:
- TX test (iperf3 client upload): 45.7 MB in 10s at 38.3 Mbits/sec
  (0 errors).
- RX test (iperf3 reverse mode flood): 65.1 MB in 15s at 36.4 Mbits/sec
  with zero stalls, zero lockups, and no interrupt storm.
- Bidirectional test (iperf3 --bidir simultaneous TX and RX):
  20.9 MB TX (11.7 Mbits/sec) + 43.6 MB RX (24.4 Mbits/sec),
  aggregate ~36.1 Mbits/sec.
- dmesg verified clean with zero warnings, traces, or hangs.

Differences in v5:
- Patch 1: Added Acked-by from Ping-Ke Shih.
- Patch 2: Updated Fixes: tag to commit 28eaf7971a20 ("wifi: rtw88: sdio:
  add TX back-pressure and retry on page starvation") per discussion
  between Ping-Ke Shih and Luka Gejak.
- Patch 2: Replaced lockless running checks with an irq_lock mutex in
  struct rtw_sdio. Since SDIO I/O sleeps, a mutex is used instead of a
  spinlock. Enforced lock ordering (sdio_claim_host() -> irq_lock) in
  start and stop paths to avoid lock inversion with the MMC IRQ handler.
- Patch 2: Retained running checks in rtw_sdio_tx_handler() and
  rtw_sdio_handle_interrupt() to avoid races during stop.
- Patch 3: Protected the 3081 interrupt re-enable check under irq_lock
  (scoped_guard) per feedback from Ping-Ke Shih.
- Patch 4: Fixed comment formatting style in rtw_sdio_rx_isr_8051() per
  coding style rules.
- Patch 4: Added Acked-by from Ping-Ke Shih.

Differences in v4:
- Split pre-existing lifecycle and error handling issues into separate
  patches (Patches 1 and 2) prior to the storm fix (per Luka Gejak).
- Guard HIMR re-enabling with rtwsdio->running in the 3081 handler.
- Significantly reduced and streamlined commit messages across the series
  (per Ping-Ke Shih).
- Clarified in Patch 4 that it is a refactoring without logic changes,
  highlighting register width and HISR behavior (per Ping-Ke Shih).
- Tagged with rtw-next in subject prefix for NIPA testing.

Alastair D'Silva (4):
  wifi: rtw88: sdio: Handle allocation and read errors in
    rtw_sdio_rxfifo_recv
  wifi: rtw88: sdio: Track running state and cancel TX worker on stop
  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 | 165 ++++++++++++++++------
 drivers/net/wireless/realtek/rtw88/sdio.h |   3 +
 2 files changed, 126 insertions(+), 42 deletions(-)

-- 
2.53.0

[PATCH rtw-next v5 1/4] wifi: rtw88: sdio: Handle allocation and read errors in rtw_sdio_rxfifo_recv

[Alastair D'Silva]

When dev_alloc_skb() or rtw_sdio_read_port() fails, rtw_sdio_rxfifo_recv()
returns without reading data. Because it returned void, the caller still
counted rx_len against the loop budget while leaving data stranded in the
FIFO.

Return an error code from rtw_sdio_rxfifo_recv() on failure and break out
of the receive loop in the caller immediately.

Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index dc2fd0f8f9ff..e39284b71837 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1349,7 +1349,7 @@ static void rtw_sdio_rx_skb(struct rtw_dev *rtwdev, struct sk_buff *skb,
 	ieee80211_rx_irqsafe(rtwdev->hw, skb);
 }
 
-static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
+static int rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 	const struct rtw_chip_info *chip = rtwdev->chip;
@@ -1366,12 +1366,12 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 
 	skb = dev_alloc_skb(bufsz);
 	if (!skb)
-		return;
+		return -ENOMEM;
 
 	ret = rtw_sdio_read_port(rtwdev, skb->data, bufsz);
 	if (ret) {
 		dev_kfree_skb_any(skb);
-		return;
+		return ret;
 	}
 
 	while (true) {
@@ -1380,7 +1380,7 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 					   &pkt_stat, &rx_status);
 		if (ret) {
 			dev_kfree_skb_any(skb);
-			return;
+			return ret;
 		}
 
 		pkt_offset = pkt_desc_sz + pkt_stat.drv_info_sz +
@@ -1416,11 +1416,14 @@ static void rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 		skb_reserve(skb, curr_pkt_len);
 		rx_len -= curr_pkt_len;
 	}
+
+	return 0;
 }
 
 static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 {
 	u32 rx_len, hisr, total_rx_bytes = 0;
+	int ret;
 
 	do {
 		if (rtw_chip_wcpu_8051(rtwdev))
@@ -1431,7 +1434,9 @@ static void rtw_sdio_rx_isr(struct rtw_dev *rtwdev)
 		if (!rx_len)
 			break;
 
-		rtw_sdio_rxfifo_recv(rtwdev, rx_len);
+		ret = rtw_sdio_rxfifo_recv(rtwdev, rx_len);
+		if (ret)
+			break;
 
 		total_rx_bytes += rx_len;
 
-- 
2.53.0

Re: [PATCH rtw-next v5 1/4] wifi: rtw88: sdio: Handle allocation and read errors in rtw_sdio_rxfifo_recv

[Luka Gejak]

On Sun Oct 11, 2026 at 10:07 PM UTC, Alastair D'Silva wrote:
> When dev_alloc_skb() or rtw_sdio_read_port() fails, rtw_sdio_rxfifo_recv()
> returns without reading data. Because it returned void, the caller still
> counted rx_len against the loop budget while leaving data stranded in the
> FIFO.
>
> Return an error code from rtw_sdio_rxfifo_recv() on failure and break out
> of the receive loop in the caller immediately.
>
> Fixes: 65371a3f14e7 ("wifi: rtw88: sdio: Add HCI implementation for SDIO based chipsets")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak

[PATCH rtw-next v5 2/4] wifi: rtw88: sdio: Track running state and cancel TX worker on stop

[Alastair D'Silva]

When rtw_sdio_stop() disables interrupts, in-flight interrupt handlers or
delayed TX work may still run against a powered-down device.

Track the operational state in rtwsdio->running with an irq_lock mutex
(similar to PCI), check it at the entry of rtw_sdio_handle_interrupt()
and rtw_sdio_tx_handler(), serialize state transitions in rtw_sdio_start()
and rtw_sdio_stop() under sdio_claim_host() and irq_lock, and cancel the
TX worker synchronously in rtw_sdio_stop().

Fixes: 28eaf7971a20 ("wifi: rtw88: sdio: add TX back-pressure and retry on page starvation")
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 | 46 +++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/sdio.h |  3 ++
 2 files changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index e39284b71837..7d4f17e96648 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -941,6 +941,8 @@ static void rtw_sdio_init(struct rtw_dev *rtwdev)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
+	mutex_init(&rtwsdio->irq_lock);
+
 	if (rtw_is_8723bs(rtwdev))
 		rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST;
 	else
@@ -948,6 +950,13 @@ static void rtw_sdio_init(struct rtw_dev *rtwdev)
 				    REG_SDIO_HIMR_CPWM1;
 }
 
+static void rtw_sdio_deinit(struct rtw_dev *rtwdev)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	mutex_destroy(&rtwsdio->irq_lock);
+}
+
 static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
 {
 	u8 size, timeout;
@@ -1054,6 +1063,7 @@ static int rtw_sdio_8723bs_check_rqpn(struct rtw_dev *rtwdev)
 
 static int rtw_sdio_start(struct rtw_dev *rtwdev)
 {
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 	u32 clear;
 
 	if (rtw_is_8723bs(rtwdev)) {
@@ -1069,20 +1079,36 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
 
 	rtw_sdio_enable_rx_aggregation(rtwdev);
 
+	sdio_claim_host(rtwsdio->sdio_func);
+
 	if (rtw_is_8723bs(rtwdev)) {
 		clear = rtw_read32(rtwdev, REG_SDIO_HISR) & RTW_SDIO_HISR_CLEAR_MASK;
 		if (clear)
 			rtw_write32(rtwdev, REG_SDIO_HISR, clear);
 	}
 
-	rtw_sdio_enable_interrupt(rtwdev);
+	scoped_guard(mutex, &rtwsdio->irq_lock) {
+		rtwsdio->running = true;
+		rtw_sdio_enable_interrupt(rtwdev);
+	}
+
+	sdio_release_host(rtwsdio->sdio_func);
 
 	return 0;
 }
 
 static void rtw_sdio_stop(struct rtw_dev *rtwdev)
 {
-	rtw_sdio_disable_interrupt(rtwdev);
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	sdio_claim_host(rtwsdio->sdio_func);
+	scoped_guard(mutex, &rtwsdio->irq_lock) {
+		rtwsdio->running = false;
+		rtw_sdio_disable_interrupt(rtwdev);
+	}
+	sdio_release_host(rtwsdio->sdio_func);
+
+	cancel_delayed_work_sync(&rtwsdio->tx_handler_data->work);
 }
 
 static void rtw_sdio_deep_ps_enter(struct rtw_dev *rtwdev)
@@ -1470,6 +1496,11 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 	rtwdev = hw->priv;
 	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
+	scoped_guard(mutex, &rtwsdio->irq_lock) {
+		if (!rtwsdio->running)
+			return;
+	}
+
 	rtwsdio->irq_thread = current;
 
 	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
@@ -1703,6 +1734,11 @@ static void rtw_sdio_tx_handler(struct work_struct *work)
 	rtwdev = work_data->rtwdev;
 	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
+	scoped_guard(mutex, &rtwsdio->irq_lock) {
+		if (!rtwsdio->running)
+			return;
+	}
+
 	if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_TX_WAKE))
 		rtw_sdio_deep_ps_leave(rtwdev);
 
@@ -1818,7 +1854,7 @@ int rtw_sdio_probe(struct sdio_func *sdio_func,
 	ret = rtw_sdio_init_tx(rtwdev);
 	if (ret) {
 		rtw_err(rtwdev, "failed to init SDIO TX queue\n");
-		goto err_sdio_declaim;
+		goto err_deinit;
 	}
 
 	ret = rtw_chip_info_setup(rtwdev);
@@ -1843,7 +1879,8 @@ int rtw_sdio_probe(struct sdio_func *sdio_func,
 	rtw_sdio_free_irq(rtwdev, sdio_func);
 err_destroy_txwq:
 	rtw_sdio_deinit_tx(rtwdev);
-err_sdio_declaim:
+err_deinit:
+	rtw_sdio_deinit(rtwdev);
 	rtw_sdio_declaim(rtwdev, sdio_func);
 err_deinit_core:
 	rtw_core_deinit(rtwdev);
@@ -1869,6 +1906,7 @@ void rtw_sdio_remove(struct sdio_func *sdio_func)
 	rtw_sdio_free_irq(rtwdev, sdio_func);
 	rtw_sdio_declaim(rtwdev, sdio_func);
 	rtw_sdio_deinit_tx(rtwdev);
+	rtw_sdio_deinit(rtwdev);
 	rtw_core_deinit(rtwdev);
 	ieee80211_free_hw(hw);
 }
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 6e7e6009744b..00a61415ba9c 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -168,6 +168,9 @@ struct rtw_sdio {
 	bool sdio3_bus_mode;
 
 	void *irq_thread;
+	bool running;
+	/* protect running state and interrupt mask */
+	struct mutex irq_lock;
 
 	struct workqueue_struct *txwq;
 	struct rtw_sdio_work_data *tx_handler_data;
-- 
2.53.0

Re: [PATCH rtw-next v5 2/4] wifi: rtw88: sdio: Track running state and cancel TX worker on stop

[Luka Gejak]

On Sun Oct 11, 2026 at 10:07 PM UTC, Alastair D'Silva wrote:
> When rtw_sdio_stop() disables interrupts, in-flight interrupt handlers or
> delayed TX work may still run against a powered-down device.
>
> Track the operational state in rtwsdio->running with an irq_lock mutex
> (similar to PCI), check it at the entry of rtw_sdio_handle_interrupt()
> and rtw_sdio_tx_handler(), serialize state transitions in rtw_sdio_start()
> and rtw_sdio_stop() under sdio_claim_host() and irq_lock, and cancel the
> TX worker synchronously in rtw_sdio_stop().
>
> Fixes: 28eaf7971a20 ("wifi: rtw88: sdio: add TX back-pressure and retry on page starvation")
> 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 | 46 +++++++++++++++++++++--
>  drivers/net/wireless/realtek/rtw88/sdio.h |  3 ++
>  2 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index e39284b71837..7d4f17e96648 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
[...]
> +static void rtw_sdio_deinit(struct rtw_dev *rtwdev)
> +{
> +	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> +
> +	mutex_destroy(&rtwsdio->irq_lock);
> +}
> +
[...]
> @@ -1843,7 +1879,8 @@ int rtw_sdio_probe(struct sdio_func *sdio_func,
>  	rtw_sdio_free_irq(rtwdev, sdio_func);
>  err_destroy_txwq:
>  	rtw_sdio_deinit_tx(rtwdev);
> -err_sdio_declaim:
> +err_deinit:
> +	rtw_sdio_deinit(rtwdev);
>  	rtw_sdio_declaim(rtwdev, sdio_func);

One nit: The rename takes every failure that lands on err_destroy_txwq
(rtw_chip_info_setup(), rtw_sdio_request_irq(), and rtw_register_hw() through
err_free_irq) past the new err_deinit, so rtw_sdio_deinit() runs only when
rtw_sdio_init_tx() itself failed. Nothing to fix while the helper is a
mutex_destroy() alone. If it ever grows, that path would miss it, so I would
either let those failures fall through to err_deinit or drop the helper.

Best regards,
Luka Gejak

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

[Alastair D'Silva]

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
@@ -1486,6 +1486,45 @@ 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_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);
+	}
+
+	/*
+	 * RTL8723BS keeps raising the interrupt after resume if undefined
+	 * status bits are written back, so acknowledge only the bits this
+	 * driver defines. Other chips keep the existing behaviour.
+	 */
+	if (rtw_is_8723bs(rtwdev))
+		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)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	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);
+	}
+}
+
 static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 {
 	struct ieee80211_hw *hw = sdio_get_drvdata(sdio_func);
@@ -1504,24 +1543,15 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 	rtwsdio->irq_thread = current;
 
 	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
+	if (!hisr)
+		goto out;
 
-	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);
-	}
-
-	/*
-	 * RTL8723BS keeps raising the interrupt after resume if undefined
-	 * status bits are written back, so acknowledge only the bits this
-	 * driver defines. Other chips keep the existing behaviour.
-	 */
-	if (rtw_is_8723bs(rtwdev))
-		hisr &= RTW_SDIO_HISR_CLEAR_MASK;
-
-	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
+	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

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

[Luka Gejak]

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

[PATCH rtw-next v5 4/4] wifi: rtw88: sdio: Split rtw_sdio_rx_isr into 8051 and 3081 variants

[Alastair D'Silva]

Split rtw_sdio_rx_isr() into separate 8051 and 3081 variants to be
clear. The two differences between the architectures are:
- Register size: REG_SDIO_RX0_REQ_LEN is 16-bit on 8051, 32-bit on 3081.
- HISR behavior: 8051 re-reads REG_SDIO_HISR on each iteration, while
  3081 drains based on rx_len without re-reading HISR.

This is a refactoring without any logic changes.

Assisted-by: LLM
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 58 +++++++++++++----------
 1 file changed, 33 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 9fbd7654ada4..a9312d0d7299 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1446,17 +1446,13 @@ static int rtw_sdio_rxfifo_recv(struct rtw_dev *rtwdev, u32 rx_len)
 	return 0;
 }
 
-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;
 	int ret;
 
 	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;
 
@@ -1466,33 +1462,45 @@ 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;
+	int ret;
+
+	do {
+		rx_len = rtw_read32(rtwdev, REG_SDIO_RX0_REQ_LEN);
+		if (!rx_len)
+			break;
+
+		ret = rtw_sdio_rxfifo_recv(rtwdev, rx_len);
+		if (ret)
+			break;
+
+		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);
 	}
 
 	/*
@@ -1516,7 +1524,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 */
 	scoped_guard(mutex, &rtwsdio->irq_lock) {
-- 
2.53.0

Re: [PATCH rtw-next v5 4/4] wifi: rtw88: sdio: Split rtw_sdio_rx_isr into 8051 and 3081 variants

[Luka Gejak]

On Sun Oct 11, 2026 at 10:07 PM UTC, Alastair D'Silva wrote:
> Split rtw_sdio_rx_isr() into separate 8051 and 3081 variants to be
> clear. The two differences between the architectures are:
> - Register size: REG_SDIO_RX0_REQ_LEN is 16-bit on 8051, 32-bit on 3081.
> - HISR behavior: 8051 re-reads REG_SDIO_HISR on each iteration, while
>   3081 drains based on rx_len without re-reading HISR.
>
> This is a refactoring without any logic changes.
>
> Assisted-by: LLM
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak