Re: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

Re: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Kalpesh Solanki]

Hi Luka,

Thanks for taking a look. You make very good points on both issues.

> As far as I can see, this timeout doesn't always mean the firmware is
> stuck. The comment in __rtw_fw_leave_lps_check_reg() says REG_TCR is
> only restored once the AP has acked the null frame, and 3a2dd6b7cadf
> ("rtw88: check firmware leave lps successfully") says that frame can
> take much longer in a noisy environment. That is why the old code just
> cleared the bit and warned.
> 
> So if I read it right, a busy channel or a weak link now ends in the
> WARN() in __fw_recovery_work() and a full ieee80211_restart_hw(). The
> watchdog leaves LPS every two seconds on an idle link, and
> BCN_FILTER_NOTIFY_BEACON_LOSS leaves it right when the AP may be gone.
> This also reaches USB and SDIO now, which had no way to trigger
> rtw_fw_recovery() before.
> 
> rtw89 only recovers after RTW89_PS_HANG_MAX_CNT failures in a row.
> Would you be willing to send a follow up that does the same here?

Yeah, resetting the whole hardware right away on a single timeout is
definitely too aggressive if the frame just got delayed or dropped due
to RF noise or beacon loss. Using a counter like rtw89 does
(RTW89_PS_HANG_MAX_CNT = 3) and only triggering recovery after consecutive
failures makes much more sense to prevent spurious restarts.

I'm happy to write and send a follow-up patch implementing this.

> There also seems to be a path where recovery never completes.
> rtw_leave_lps() also runs from ieee80211_set_disassoc() through
> rtw_recalc_lps(). If the check times out there while the last interface
> goes down, or on a suspend without WoWLAN, the queued work runs before
> drv_stop(). mac80211 then never calls drv_reconfig_complete() with
> IEEE80211_RECONFIG_TYPE_RESTART, so RTW_FLAG_RESTARTING stays set and
> rtw_fw_recovery() does nothing until the driver is reloaded, also for
> real firmware crashes. Maybe rtw_ops_stop() could clear it as well?

Good catch on the mac80211 lifecycle path. If open_count is zero or during
suspend without WoWLAN, drv_reconfig_complete() never gets called, which
leaves RTW_FLAG_RESTARTING stuck and permanently blocks recovery until module
reload.

Clearing RTW_FLAG_RESTARTING (and resetting the hang counter) in
rtw_ops_stop() sounds like the right way to handle it.

I can prepare a 2-patch follow-up series:
1. wifi: rtw88: ps: trigger recovery only after consecutive LPS leave failures
2. wifi: rtw88: clear RTW_FLAG_RESTARTING in rtw_ops_stop()

Ping-Ke, does that sound good to you?

Thanks,
Kalpesh

Re: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Luka Gejak]

Hi Kalpesh,

Thanks for the quick reply. I just have a few things to add.

[ ... ]

> Clearing RTW_FLAG_RESTARTING (and resetting the hang counter) in
> rtw_ops_stop() sounds like the right way to handle it.

That is fine for the flag, but I think the counter should be reset in
rtw_core_start(), like rtw89 does. A normal recovery never calls
rtw_ops_stop(), so the counter would stay at the limit and the next
single timeout would recover again.

> I can prepare a 2-patch follow-up series:
> 1. wifi: rtw88: ps: trigger recovery only after consecutive LPS leave failures
> 2. wifi: rtw88: clear RTW_FLAG_RESTARTING in rtw_ops_stop()

The RESTARTING gap predates your patch, since 5c831644e1f4 ("rtw88:
handle and recover when firmware crash"), so that one could go on its
own with a Fixes: tag.

For the counter, maybe it is cleaner to revert 2/2 (or rebase and drop
it since it is currently the HEAD commit of rtw-next) and send a new
patch, since it replaces that line anyway and keeps the current
behavior out of the next pull. 1/2 is fine as it is. Ping-Ke, would
that work for you, or do you prefer a follow up?

Also Kalpesh, your replies don't carry In-Reply-To or References, so
lore shows them as separate threads. In case you are using git send-email
git send-email --in-reply-to=<Message-ID>
should fix that.

Best regards,
Luka Gejak

Re: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Kalpesh Solanki]

Hi Luka,

Thanks for the clarification and the tip regarding --in-reply-to!

> That is fine for the flag, but I think the counter should be reset in
> rtw_core_start(), like rtw89 does. A normal recovery never calls
> rtw_ops_stop(), so the counter would stay at the limit and the next
> single timeout would recover again.

Good point. Putting rtwdev->ps_hang_cnt = 0 in rtw_core_start() ensures it gets
cleared after both normal starts and hw restarts. I'll place it there.

> The RESTARTING gap predates your patch, since 5c831644e1f4 ("rtw88:
> handle and recover when firmware crash"), so that one could go on its
> own with a Fixes: tag.

Makes sense. I'll send that as a standalone fix with the Fixes: tag.

> For the counter, maybe it is cleaner to revert 2/2 (or rebase and drop
> it since it is currently the HEAD commit of rtw-next) and send a new
> patch, since it replaces that line anyway and keeps the current
> behavior out of the next pull. 1/2 is fine as it is. Ping-Ke, would
> that work for you, or do you prefer a follow up?

Either way works for me. If Ping-Ke prefers to drop 2/2 from rtw-next, I can
send the updated patch with the consecutive counter directly. Otherwise I can
send it as a follow-up on top.

Thanks,
Kalpesh

Re: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Luka Gejak]

Hi Kalpesh,

Thanks for the patch. I know this is already applied, but I have two
comments.

November 3, 2026 at 4:14 AM, "Kalpesh Solanki" <skalpesh942@gmail.com> wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
> index add5a20b8432..8dba60f3464c 100644
> --- a/drivers/net/wireless/realtek/rtw88/ps.c
> +++ b/drivers/net/wireless/realtek/rtw88/ps.c
> @@ -166,6 +166,7 @@ static void rtw_fw_leave_lps_check(struct rtw_dev *rtwdev)
>               rtw_write32_clr(rtwdev, REG_TCR, BIT_PWRMGT_HWDATA_EN);
>               rtw_warn(rtwdev, "firmware failed to leave lps state\n");
>               rtw_fw_dump_dbg_info(rtwdev);
> +             rtw_fw_recovery(rtwdev);
>       }
>  }

As far as I can see, this timeout doesn't always mean the firmware is
stuck. The comment in __rtw_fw_leave_lps_check_reg() says REG_TCR is
only restored once the AP has acked the null frame, and 3a2dd6b7cadf
("rtw88: check firmware leave lps successfully") says that frame can
take much longer in a noisy environment. That is why the old code just
cleared the bit and warned.

So if I read it right, a busy channel or a weak link now ends in the
WARN() in __fw_recovery_work() and a full ieee80211_restart_hw(). The
watchdog leaves LPS every two seconds on an idle link, and
BCN_FILTER_NOTIFY_BEACON_LOSS leaves it right when the AP may be gone.
This also reaches USB and SDIO now, which had no way to trigger
rtw_fw_recovery() before.

rtw89 only recovers after RTW89_PS_HANG_MAX_CNT failures in a row.
Would you be willing to send a follow up that does the same here?

There also seems to be a path where the recovery never completes.
rtw_leave_lps() also runs from ieee80211_set_disassoc() through
rtw_recalc_lps(). If the check times out there while the last interface
goes down, or on a suspend without WoWLAN, the queued work runs before
drv_stop(). mac80211 then never calls drv_reconfig_complete() with
IEEE80211_RECONFIG_TYPE_RESTART, so RTW_FLAG_RESTARTING stays set and
rtw_fw_recovery() does nothing until the driver is reloaded, also for
real firmware crashes. Maybe rtw_ops_stop() could clear it as well?

Best regards,
Luka Gejak

RE: [PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Ping-Ke Shih]

Kalpesh Solanki <skalpesh942@gmail.com> wrote:
> When rtw_fw_leave_lps_check() fails, the firmware has locked up or
> failed to exit its low power state. Currently, the driver only logs
> 'firmware failed to leave lps state' and clears REG_TCR, leaving the
> hardware in an unresponsive state where subsequent H2C commands fail
> and TX traffic hangs indefinitely.
> 
> Call rtw_fw_recovery() when rtw_fw_leave_lps_check() fails so the driver
> can automatically reset the hardware and re-sync without requiring manual
> user intervention.
> 
> Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>

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

[PATCH v2 2/2] wifi: rtw88: ps: Trigger firmware recovery when failing to leave LPS state

[Kalpesh Solanki]

When rtw_fw_leave_lps_check() fails, the firmware has locked up or
failed to exit its low power state. Currently, the driver only logs
'firmware failed to leave lps state' and clears REG_TCR, leaving the
hardware in an unresponsive state where subsequent H2C commands fail
and TX traffic hangs indefinitely.

Call rtw_fw_recovery() when rtw_fw_leave_lps_check() fails so the driver
can automatically reset the hardware and re-sync without requiring manual
user intervention.

Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
---
v2:
 - Rebase onto rtw-next.

 drivers/net/wireless/realtek/rtw88/ps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
index add5a20b843..8dba60f3464 100644
--- a/drivers/net/wireless/realtek/rtw88/ps.c
+++ b/drivers/net/wireless/realtek/rtw88/ps.c
@@ -166,6 +166,7 @@ static void rtw_fw_leave_lps_check(struct rtw_dev *rtwdev)
 		rtw_write32_clr(rtwdev, REG_TCR, BIT_PWRMGT_HWDATA_EN);
 		rtw_warn(rtwdev, "firmware failed to leave lps state\n");
 		rtw_fw_dump_dbg_info(rtwdev);
+		rtw_fw_recovery(rtwdev);
 	}
 }
 
-- 
2.53.0