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

[PATCH rtw-next v4 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 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 and cancels
  the TX worker synchronously on stop, preventing work items from
  accessing powered-down hardware.
- 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.
- 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:
- Bidirectional TCP throughput: 36.5 Mbps RX / 36.4 Mbps TX (over wlan0).
- Repeated interface down/up cycles verified clean with zero storms or
  hangs.

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 | 128 +++++++++++++++-------
 drivers/net/wireless/realtek/rtw88/sdio.h |   1 +
 2 files changed, 91 insertions(+), 38 deletions(-)

-- 
2.53.0

[PATCH rtw-next v4 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
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 v4 1/4] wifi: rtw88: sdio: Handle allocation and read errors in rtw_sdio_rxfifo_recv

[Ping-Ke Shih]

Alastair D'Silva <alastair@d-silva.org> 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
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

[PATCH rtw-next v4 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 (similar to PCI), check it
at the entry of rtw_sdio_handle_interrupt() and rtw_sdio_tx_handler(), and
cancel the TX worker synchronously in rtw_sdio_stop().

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 | 12 ++++++++++++
 drivers/net/wireless/realtek/rtw88/sdio.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index e39284b71837..d2f4d7e8bc83 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1054,6 +1054,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)) {
@@ -1075,6 +1076,7 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
 			rtw_write32(rtwdev, REG_SDIO_HISR, clear);
 	}
 
+	rtwsdio->running = true;
 	rtw_sdio_enable_interrupt(rtwdev);
 
 	return 0;
@@ -1082,7 +1084,11 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
 
 static void rtw_sdio_stop(struct rtw_dev *rtwdev)
 {
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	rtwsdio->running = false;
 	rtw_sdio_disable_interrupt(rtwdev);
+	cancel_delayed_work_sync(&rtwsdio->tx_handler_data->work);
 }
 
 static void rtw_sdio_deep_ps_enter(struct rtw_dev *rtwdev)
@@ -1470,6 +1476,9 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 	rtwdev = hw->priv;
 	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
+	if (!rtwsdio->running)
+		return;
+
 	rtwsdio->irq_thread = current;
 
 	hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
@@ -1703,6 +1712,9 @@ static void rtw_sdio_tx_handler(struct work_struct *work)
 	rtwdev = work_data->rtwdev;
 	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
+	if (!rtwsdio->running)
+		return;
+
 	if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_TX_WAKE))
 		rtw_sdio_deep_ps_leave(rtwdev);
 
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 6e7e6009744b..a3851d4a58e7 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -168,6 +168,7 @@ struct rtw_sdio {
 	bool sdio3_bus_mode;
 
 	void *irq_thread;
+	bool running;
 
 	struct workqueue_struct *txwq;
 	struct rtw_sdio_work_data *tx_handler_data;
-- 
2.53.0

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

[Ping-Ke Shih]

Alastair D'Silva <alastair@d-silva.org> 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 (similar to PCI), check it
> at the entry of rtw_sdio_handle_interrupt() and rtw_sdio_tx_handler(), and
> cancel the TX worker synchronously in rtw_sdio_stop().
> 
> 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 | 12 ++++++++++++
>  drivers/net/wireless/realtek/rtw88/sdio.h |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> index e39284b71837..d2f4d7e8bc83 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> @@ -1054,6 +1054,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)) {
> @@ -1075,6 +1076,7 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
>                         rtw_write32(rtwdev, REG_SDIO_HISR, clear);
>         }
> 
> +       rtwsdio->running = true;

Is there existing race between start/stop/interrupt? Need a lock?

>         rtw_sdio_enable_interrupt(rtwdev);
> 
>         return 0;
> @@ -1082,7 +1084,11 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
> 
>  static void rtw_sdio_stop(struct rtw_dev *rtwdev)
>  {
> +       struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> +
> +       rtwsdio->running = false;
>         rtw_sdio_disable_interrupt(rtwdev);
> +       cancel_delayed_work_sync(&rtwsdio->tx_handler_data->work);

I think this is the major statement added by this patch, but I'm not sure
if this is actually needed. (Maybe, Luka can help this)

By 'git blame', the work is added by
28eaf7971a20 ("wifi: rtw88: sdio: add TX back-pressure and retry on page starvation")

Maybe, the fixes tag should be 28eaf7971a20. I think there is no way to apply this
to 65371a3f14e7 (your fixes tag)

But I think you need this because patch 3/4. 

>  }
> 
>  static void rtw_sdio_deep_ps_enter(struct rtw_dev *rtwdev)
> @@ -1470,6 +1476,9 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
>         rtwdev = hw->priv;
>         rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> 
> +       if (!rtwsdio->running)
> +               return;
> +
>         rtwsdio->irq_thread = current;
> 
>         hisr = rtw_read32(rtwdev, REG_SDIO_HISR);
> @@ -1703,6 +1712,9 @@ static void rtw_sdio_tx_handler(struct work_struct *work)
>         rtwdev = work_data->rtwdev;
>         rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> 
> +       if (!rtwsdio->running)
> +               return;
> +

I don't think we need this, since you added cancel_delayed_work_sync() in
rtw_sdio_stop().

>         if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_TX_WAKE))
>                 rtw_sdio_deep_ps_leave(rtwdev);
> 
> diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
> index 6e7e6009744b..a3851d4a58e7 100644
> --- a/drivers/net/wireless/realtek/rtw88/sdio.h
> +++ b/drivers/net/wireless/realtek/rtw88/sdio.h
> @@ -168,6 +168,7 @@ struct rtw_sdio {
>         bool sdio3_bus_mode;
> 
>         void *irq_thread;
> +       bool running;
> 
>         struct workqueue_struct *txwq;
>         struct rtw_sdio_work_data *tx_handler_data;
> --
> 2.53.0

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

[Luka Gejak]

October 6, 2026 at 04:03, "Ping-Ke Shih" <pkshih@realtek.com mailto:pkshih@realtek.com?to=%22Ping-Ke%20Shih%22%20%3Cpkshih%40realtek.com%3E > wrote:


> 
> Alastair D'Silva <alastair@d-silva.org> 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 (similar to PCI), check it
> >  at the entry of rtw_sdio_handle_interrupt() and rtw_sdio_tx_handler(), and
> >  cancel the TX worker synchronously in rtw_sdio_stop().
> >  
> >  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 | 12 ++++++++++++
> >  drivers/net/wireless/realtek/rtw88/sdio.h | 1 +
> >  2 files changed, 13 insertions(+)
> >  
> >  diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
> >  index e39284b71837..d2f4d7e8bc83 100644
> >  --- a/drivers/net/wireless/realtek/rtw88/sdio.c
> >  +++ b/drivers/net/wireless/realtek/rtw88/sdio.c
> >  @@ -1054,6 +1054,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)) {
> >  @@ -1075,6 +1076,7 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
> >  rtw_write32(rtwdev, REG_SDIO_HISR, clear);
> >  }
> >  
> >  + rtwsdio->running = true;
> > 
> Is there existing race between start/stop/interrupt? Need a lock?
> 
> > 
> > rtw_sdio_enable_interrupt(rtwdev);
> >  
> >  return 0;
> >  @@ -1082,7 +1084,11 @@ static int rtw_sdio_start(struct rtw_dev *rtwdev)
> >  
> >  static void rtw_sdio_stop(struct rtw_dev *rtwdev)
> >  {
> >  + struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
> >  +
> >  + rtwsdio->running = false;
> >  rtw_sdio_disable_interrupt(rtwdev);
> >  + cancel_delayed_work_sync(&rtwsdio->tx_handler_data->work);
> > 
> I think this is the major statement added by this patch, but I'm not sure
> if this is actually needed. (Maybe, Luka can help this)
> 

I will take a look at this series later during the day.

> By 'git blame', the work is added by
> 28eaf7971a20 ("wifi: rtw88: sdio: add TX back-pressure and retry on page starvation")
> 
> Maybe, the fixes tag should be 28eaf7971a20. I think there is no way to apply this
> to 65371a3f14e7 (your fixes tag)
> 
>

Best regards,
Luka Gejak

[PATCH rtw-next v4 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.
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 | 60 +++++++++++++++++------
 1 file changed, 44 insertions(+), 16 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index d2f4d7e8bc83..c74644bf14ba 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1466,23 +1466,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;
-
-	if (!rtwsdio->running)
-		return;
-
-	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) {
@@ -1499,7 +1484,50 @@ 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)
+{
+	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 */
+	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);
+	struct rtw_sdio *rtwsdio;
+	struct rtw_dev *rtwdev;
+	u32 hisr;
+
+	rtwdev = hw->priv;
+	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	if (!rtwsdio->running)
+		return;
+
+	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

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

[Ping-Ke Shih]

Alastair D'Silva <alastair@d-silva.org> wrote:
> +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 */
> +       if (rtwsdio->running)

I think here needs a lock

> +               rtw_sdio_enable_interrupt(rtwdev);
> +}
> +

[PATCH rtw-next v4 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
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 57 +++++++++++++----------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index c74644bf14ba..8f4d18b03f68 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -1426,17 +1426,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;
 
@@ -1446,33 +1442,44 @@ 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);
 	}
 
 	/*
@@ -1496,7 +1503,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 */
 	if (rtwsdio->running)
-- 
2.53.0

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

[Ping-Ke Shih]

Alastair D'Silva <alastair@d-silva.org> 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
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>


[...]

> +               /* Stop if no more RX requests are pending, even if

nit: keep first line of block comment as empty. 

> +                * 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);
>  }
>