On 14/07/2025 05:49, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@xxxxxxxxx> 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@xxxxxxxxx>
>> ---
>> 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
>
