Re: [PATCH v3 2/2] wifi: rtw88: ps: trigger firmware recovery only after consecutive LPS leave failures

[Luka Gejak]

Hi Kalpesh,

I also have a couple of comments for this patch.

November 8, 2026 at 9:51 AM, "Kalpesh Solanki" <skalpesh942@xxxxxxxxx> wrote:

> diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
> index add5a20b8432..864e2be243ea 100644
> --- a/drivers/net/wireless/realtek/rtw88/ps.c
> +++ b/drivers/net/wireless/realtek/rtw88/ps.c
> @@ -165,7 +165,14 @@ static void rtw_fw_leave_lps_check(struct rtw_dev *rtwdev)
>       if (ret) {
>               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_dump_dbg_info() returns right away unless RTW_DBG_FW is set, so I
don't think it can flood the log in normal use, and the rtw_warn() above
still prints on every failure. With that debug mask on, the dump from
the first failure seems useful too. Could it stay where it was?

> +
> +             rtwdev->ps_hang_cnt++;
> +             if (rtwdev->ps_hang_cnt >= RTW_PS_HANG_MAX_CNT) {
> +                     rtw_fw_dump_dbg_info(rtwdev);
> +                     rtw_fw_recovery(rtwdev);

This still reaches USB and SDIO, and as far as I can see
__fw_recovery_work() can't run there yet. It clears the keys from an
atomic iterator:

        __fw_recovery_work()
          rtw_iterate_keys_atomic()
            ieee80211_iter_keys_atomic()    /* holds local->key_lock */
              rtw_reset_key_iter()
                rtw_sec_clear_cam()
                  rtw_write32()

On USB that write ends in usb_control_msg() and on SDIO it needs
sdio_claim_host(), so I think both sleep with the spinlock held.
78d5bf925f30 ("wifi: rtw88: iterate over vif/sta list non-atomically")
moved other iterators away from this for USB, but until now nothing
outside PCI could get to the recovery code.

Would it be ok to only call rtw_fw_recovery() on PCI for now, with a
rtw_hci_type() check? USB and SDIO never had this recovery anyway.

This can also run during a disconnect. ieee80211_set_disassoc() leaves
LPS before it removes the AP station, so if that is the third failure,
the recovery work can remove the station first and rtw_ops_sta_remove()
then removes it again. If I read it right, sta_cnt underflows and
si->mask is freed twice if a bitrate mask was set. Maybe
rtw_sta_remove() could skip a station that was already reset?

Best regards,
Luka Gejak