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

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

[Kalpesh Solanki]

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

[PATCH 1/2] wifi: rtw88: pci: Add DMI quirk for HP Laptop 15s-fr5xxx

[Kalpesh Solanki]

On the HP Laptop 15s-fr5xxx with RTL8822CE, PCIe ASPM L1/L1.2 and deep LPS
cause the card's firmware to fail to resume from low-power states,
resulting in repeated 'firmware failed to leave lps state' and
'failed to send h2c command' errors and stalled network traffic.

Add a DMI quirk entry for 'HP Laptop 15s-fr5xxx' to disable PCIe ASPM
and deep LPS, ensuring reliable Wi-Fi operation.

Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
---
 drivers/net/wireless/realtek/rtw88/pci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index ca4e779..30b7904 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -1813,6 +1813,16 @@ static const struct dmi_system_id rtw_pci_quirks[] = {
 		.driver_data = (void *)(BIT(QUIRK_DIS_CAP_PCI_ASPM) |
 					BIT(QUIRK_DIS_CAP_LPS_DEEP)),
 	},
+	{
+		.callback = rtw_pci_disable_caps,
+		.ident = "HP Laptop 15s-fr5xxx",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP Laptop 15s-fr5xxx"),
+		},
+		.driver_data = (void *)(BIT(QUIRK_DIS_CAP_PCI_ASPM) |
+					BIT(QUIRK_DIS_CAP_LPS_DEEP)),
+	},
 	{
 		.callback = rtw_pci_disable_caps,
 		.ident = "ASUS TUF Gaming A15 FA506II",
-- 
2.53.0

[PATCH 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>
---
 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 c6360cf..db1e9e7 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_once(rtwdev, "firmware failed to leave lps state\n");
 		rtw_fw_dump_dbg_info(rtwdev);
+		rtw_fw_recovery(rtwdev);
 	}
 }
 
-- 
2.53.0

RE: [PATCH 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.

After patch 1/2, does it still happen?

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

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);

> 
> 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.

> 
> Signed-off-by: Kalpesh Solanki <skalpesh942@gmail.com>
> ---
>  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 c6360cf..db1e9e7 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_once(rtwdev, "firmware failed to leave lps state\n");
>                 rtw_fw_dump_dbg_info(rtwdev);
> +               rtw_fw_recovery(rtwdev);

failed to apply, because rebase to rtw-next.

https://github.com/pkshih/rtw.git

>         }
>  }
> 
> --
> 2.53.0
>