Re: [PATCH rtw-next 1/2] wifi: rtw89: Increase polling delay in rtw89_fw_read_c2h_reg() for USB

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 14/07/2025 05:49, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> This read_poll_timeout_atomic() with a delay of 1 µs and a timeout of
>> 1000000 µs can take ~250 seconds in the worst case because sending a
>> USB control message takes ~250 µs.
> 
> I was not aware of the change of [1]. The behavior of atomic version becomes
> different from non-atomic version. 
> 
> For this patch, I feel we can keep delay_us to 1 and treat timeout_us as
> 'count', which USB devices do smaller retries. The smaller delay_us can
> reduce total polling time, especially for PCIE devices (see my comments below)
> 
> Though I don't measure total polling time of patch 2/2, I feel we can apply
> similar idea. 
> 

Yes, a smaller timeout also works. I tested 4000 for this patch and 3200
for patch 2. (4000 * 250 = 1000000 and 3200 * 125 = 400000. I don't know
why rtw89_read8() in the second patch takes only 125 µs.)

> [1] 7349a69cf312 ("iopoll: Do not use timekeeping in read_poll_timeout_atomic()")
> 
>>
>> Increase the delay to 250 µs in order to reduce the maximum polling
>> time to ~2 seconds.
>>
>> This problem was observed with RTL8851BU while suspending to RAM with
>> WOWLAN enabled. The computer sat for 4 minutes with a black screen
>> before suspending.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>> ---
>>  drivers/net/wireless/realtek/rtw89/fw.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
>> index c613431e754f..27d84464347b 100644
>> --- a/drivers/net/wireless/realtek/rtw89/fw.c
>> +++ b/drivers/net/wireless/realtek/rtw89/fw.c
>> @@ -6665,7 +6665,7 @@ static int rtw89_fw_read_c2h_reg(struct rtw89_dev *rtwdev,
>>
>>         info->id = RTW89_FWCMD_C2HREG_FUNC_NULL;
>>
>> -       ret = read_poll_timeout_atomic(rtw89_read8, val, val, 1,
>> +       ret = read_poll_timeout_atomic(rtw89_read8, val, val, 250,
> 
> As my experiments, PCIE devices take about 30us for this polling, when
> setting delay 1. But it will take 256us, if delay is changed to 250.
> I feel we need to set this value by HCI type if needed.
> 
>>                                        RTW89_C2H_TIMEOUT, false, rtwdev,
>>                                        chip->c2h_ctrl_reg);
>>         if (ret) {
>> --
>> 2.50.0
>