Re: [PATCH rtw-next v2 6/7] wifi: rtw88: 88xxa: adapt channel sensing to receiver gain

[Bitterblue Smith]

On 06/11/2026 19:34, Luka Gejak wrote:
> Hi Jie Feng and Bitterblue,
> 
> Thanks for fixing the scan latency. A few comments below.
> 
>> diff --git a/drivers/net/wireless/realtek/rtw88/phy.c b/drivers/net/wireless/realtek/rtw88/phy.c
>> index 69998c0d7c1f851bb30d2702233d62e14b305bf0..7a04aec9a045f1e308a4c5d331c900e013996bd5 100644
>> --- a/drivers/net/wireless/realtek/rtw88/phy.c
>> +++ b/drivers/net/wireless/realtek/rtw88/phy.c
>> @@ -377,6 +377,10 @@ void rtw_phy_dig_set_max_coverage(struct rtw_dev *rtwdev)
>>               DIG_CVRG_MIN);
>>
>>       rtw_phy_dig_write(rtwdev, DIG_CVRG_MIN);
>> +
>> +     /* EDCCA thresholds can be relative to the IGI */
>> +     if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_ADAPTIVITY))
>> +             rtw_phy_adaptivity(rtwdev);
>>  }
> 
> This and the call in rtw_phy_dig_reset() also reach
> rtw8822b_adaptivity() and rtw8822c_adaptivity() when the firmware
> doesn't handle adaptivity. They use igi_history[0], so as far as I can
> see they just write the same thresholds again, but since this is core
> code it would be good to mention it in the commit message.
> 
> [ ... ]
> 
>> diff --git a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
>> index 851d198f76f3b10d5ba77c7e07e4be9d7fd54e4b..ee72380757721dab7dc409aabc24b2b825969465 100644
>> --- a/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
>> +++ b/drivers/net/wireless/realtek/rtw88/rtw88xxa.c
> 
> [ ... ]
> 
>> +#define EDCCA_LB_START_88XXA         (EDCCA_L2H_H2L_DIFF - 30)
>> +#define EDCCA_LB_RETRY_88XXA         (EDCCA_LB_START_88XXA + 18)
> 
> Maybe a short comment on where the 18 comes from. As far as I can see it
> is the vendor condition for searching a second time (IGI upper bound of
> 0x26 or lower), worked out for an initial L2H of -17.
> 
> [ ... ]
> 
>> +     rtw_write32(rtwdev, REG_DBGSEL, 0);
>> +     for (i = 0; i < 3 &&
>> +          rtw_read32(rtwdev, REG_DBGRPT) & BIT_DBG_CCA_BUSY; i++)
>> +             usleep_range(3000, 3500);
> 
> The register read in the loop condition is a bit hard to follow. Maybe
> read_poll_timeout() would read easier, like efuse.c uses it:
> 
>         read_poll_timeout(rtw_read32, val, !(val & BIT_DBG_CCA_BUSY),
>                           3000, 9000, false, rtwdev, REG_DBGRPT);
> 
> [ ... ]
> 
>> +void rtw88xxa_adaptivity_init(struct rtw_dev *rtwdev)
>> +{
>> +     struct rtw_dm_info *dm_info = &rtwdev->dm_info;
>> +     u32 dbgsel = rtw_read32(rtwdev, REG_DBGSEL);
>> +     s8 l2h;
>> +
>> +     rtw_write32_clr(rtwdev, REG_TX_PTCL_CTRL, BIT_DIS_EDCCA);
> 
> Bitterblue reported on that "firmware failed to leave lps state" shows
> up a lot more often with this patch and stops when EDCCA is disabled,
> and you replied that it only prints a line in dmesg. On rtw-next it is
> more than a message now. Since dc16902cad9c ("wifi: rtw88: ps: Trigger
> firmware recovery when failing to leave LPS state"), rtw_fw_leave_lps_check() 
> goes on to a firmware recovery after the warning:
> 
>         rtw_warn(rtwdev, "firmware failed to leave lps state\n");
>         rtw_fw_dump_dbg_info(rtwdev);
>         rtw_fw_recovery(rtwdev);
> 
> So a failed leave now ends in a firmware recovery, with a "firmware
> crash, start reset and recover" WARN and ieee80211_restart_hw(). With
> power saving on, rtw_watch_dog_work() calls rtw_leave_lps() every 2
> seconds before the dynamic mechanisms, so I think this needs to be
> solved before the patch can go in.
> 
> About a longer timeout, according to the comment in
> __rtw_fw_leave_lps_check_reg() the firmware restores REG_TCR once the AP
> has acked its null frame, so I think it would mostly hide the problem.
> Leaving this patch out of v3 and sending it again once the cause is
> known sounds fine to me, then the rest of the series doesn't have to
> wait.
> 
> Bitterblue, did you turn EDCCA off through the edcca_enable debugfs file?

Yes, that's what I did.

> With this patch that only switches to RTW_EDCCA_NORMAL, and
> rtw88xxa_adaptivity() then keeps EDCCA on with the higher normal
> thresholds. If that is what made it stop, it would point at the
> thresholds used in adaptivity mode (ETSI and JP regions) rather than at
> EDCCA itself.
> 
> Jie Feng, maybe comparing the IGI in the hardware when the leave fails
> with the IGI the thresholds were computed from would show more.
> 
> [ ... ]
> 
>> +     /* igi_history[0] keeps the pre-scan IGI while scanning */
>> +     u8 igi = rtw_read32_mask(rtwdev, REG_RXIGI_A, 0x7f);
> 
> The comment sits on a line that doesn't use igi_history[0], so it reads
> a bit confusing. Maybe something like "read the IGI in use,
> igi_history[0] still holds the old value while scanning".
> 
> Best regards,
> Luka Gejak