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

[Kalpesh Solanki <skalpesh942@gmail.com>]

Hi Ping-Ke,

Thank you for reviewing the series and for the Ack on patch 1/2!

Please find my responses inline below:

> After patch 1/2, does it still happen?

No, after patch 1/2 is applied (with PCIe ASPM and deep LPS disabled via the
DMI quirk for HP Laptop 15s-fr5xxx), this issue no longer occurs during
normal operation. 

The stall was observed before patch 1/2 (when deep LPS and ASPM were enabled),
where the firmware became unresponsive upon attempting to wake up. Patch 2/2
was therefore conceived as a general safeguard/watchdog for rtw88 in case the
firmware fails to exit LPS on any device (either unquirked hardware or due to
unexpected power-management glitches).

> Is it helpful if you enlarge timeout time of __rtw_fw_leave_lps_check_c2h()
> or __rtw_fw_leave_lps_check_reg()?

In our testing, enlarging the timeout did not help. When this failure occurs,
the on-chip microcontroller / PCIe link is completely deadlocked. It does not
clear the register or complete the C2H handshake even with extended wait
times. Subsequent H2C commands continuously fail with "failed to send h2c
command" because the firmware has locked up rather than simply being slow to
respond.

> or retry 3 times to leave PS with below three steps?
> 
>     rtw_fw_leave_lps_check_prepare(rtwdev);
>     rtw_fw_set_pwr_mode(rtwdev);
>     rtw_fw_leave_lps_check(rtwdev);

Because the microcontroller is locked up, resending H2C commands via
rtw_fw_set_pwr_mode() also fails to get processed by the firmware.

However, retrying 2-3 times could be beneficial as a first-line remedy for
transient timing glitches on other hardware before resorting to recovery.
If you think that is a better approach, I can update patch 2/2 to retry 3 times
first and only trigger rtw_fw_recovery() if all 3 attempts fail.

> 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.
> 
> Is this a real case? I'd just want to confirm this is real case it can help.

Yes, this was a reproducible real-world case observed on this RTL8822CE
platform.

Prior to calling rtw_fw_recovery(), once rtw_fw_leave_lps_check() failed, the
driver only cleared REG_TCR and logged the warning. The hardware remained in
an unresponsive state with 100% packet loss (traffic hung indefinitely while
NetworkManager still reported "Connected"), requiring manual user intervention
(cycling the interface down and up) to restore connectivity.

Calling rtw_fw_recovery() triggered the recovery workqueue and
ieee80211_restart_hw(), successfully resetting the hardware and reloading
firmware state, which automatically restored network connectivity without
requiring the user to restart the interface.

> failed to apply, because rebase to rtw-next.
>
> https://github.com/pkshih/rtw.git

Understood. I pulled your 'rtw-next' branch and saw the conflict in ps.c
(rtw-next uses rtw_warn() rather than rtw_warn_once()). I have rebased both
patches onto pkshih/rtw-next, and both apply cleanly now.

Please let me know if you would like me to submit a v2 patch series rebased on
rtw-next (either with the 3-retry fallback or with rtw_fw_recovery() directly),
or if you prefer to proceed with patch 1/2 alone.

Thanks,
Kalpesh