Re: [PATCH v4 3/3] wifi: rtw88: ps: trigger firmware recovery only after consecutive LPS leave failures on PCI

[KALPESH SOLANKI]

Hi Luka, Ping-Ke,

Thank you for the reviews and feedback. I have had a bereavement in
the family yesterday and will be away for a few days.

I have noted the comments and will follow up with v5 once I am back
and after lore/patchwork is operational.

Best regards,
Kalpesh


On Mon, 9 Nov 2026 at 13:53, KALPESH SOLANKI <skalpesh942@xxxxxxxxx> wrote:
>
> Hi Luka, Ping-Ke,
>
> Thank you for the reviews and feedback. I have had a bereavement in the family yesterday and will be away for a few days.
>
> I have noted the comments and will follow up with v5 once I am back and after lore/patchwork is operational.
>
> Best regards,
> Kalpesh
>
> On Mon, 9 Nov 2026 at 12:43, Luka Gejak <luka.gejak@xxxxxxxxx> wrote:
>>
>> Hi Kalpesh,
>>
>> Thanks for the new version. Two comments below.
>>
>> On November 8, 2026 3:27:07 PM GMT+01:00, Kalpesh Solanki
>> <skalpesh942@xxxxxxxxx> wrote:
>>                                                                             > diff --git a/drivers/net/wireless/realtek/rtw88/main.c b/drivers/net/wireless/realtek/rtw88/main.c
>> > index a74f199a93ab..7009f8ae3e57 100644                                   > --- a/drivers/net/wireless/realtek/rtw88/main.c                           > +++ b/drivers/net/wireless/realtek/rtw88/main.c                           > @@ -638,6 +638,9 @@ static void rtw_reset_vif_iter(void *data, u8 *mac, struct ieee80211_vif *vif)
>> >
>> >  void rtw_fw_recovery(struct rtw_dev *rtwdev)
>> >  {
>> > +     if (rtw_hci_type(rtwdev) != RTW_HCI_TYPE_PCIE)
>> > +             return;
>> > +
>>
>> The same check is also in rtw_fw_leave_lps_check() below, and the other
>> callers of rtw_fw_recovery() are PCI only, so I think this one can go.
>>
>> [ ... ]
>>
>> > diff --git a/drivers/net/wireless/realtek/rtw88/ps.c b/drivers/net/wireless/realtek/rtw88/ps.c
>> > index add5a20b8432..fa3a4e476cf6 100644
>> > --- a/drivers/net/wireless/realtek/rtw88/ps.c
>> > +++ b/drivers/net/wireless/realtek/rtw88/ps.c
>> > @@ -166,6 +166,13 @@ 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);
>> > +
>> > +             rtwdev->ps_hang_cnt++;
>> > +             if (rtw_hci_type(rtwdev) == RTW_HCI_TYPE_PCIE &&
>> > +                 rtwdev->ps_hang_cnt >= RTW_PS_HANG_MAX_CNT)
>> > +                     rtw_fw_recovery(rtwdev);
>> > +     } else {
>> > +             rtwdev->ps_hang_cnt = 0;
>> >       }
>>
>> As far as I can see this can't reach the limit on stuck firmware without
>> FW_FEATURE_LPS_C2H, which seems to include rtw8822b and rtw8723d. The
>> register check passes once the bit is clear, the failure path above
>> clears it, and I can only see the firmware setting it again when it
>> enters LPS.
>>
>> So if I read it right, stuck firmware gives one failure, then the next
>> check passes and resets the counter, as long as the registers still read
>> fine. Three in a row would need firmware that is still running.
>>
>> Would it make sense to only count failures when FW_FEATURE_LPS_C2H is set?
>> Ping-Ke, do you know if only the firmware sets this bit?
>>
>>
>> Best regards,
>> Luka Gejak