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

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

[Bitterblue Smith]

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.

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,
 				       RTW89_C2H_TIMEOUT, false, rtwdev,
 				       chip->c2h_ctrl_reg);
 	if (ret) {
-- 
2.50.0

[PATCH rtw-next 2/2] wifi: rtw89: Increase polling delay in rtw89_fwdl_check_path_ready_* for USB

[Bitterblue Smith]

When the chip is not powered on correctly (like during driver
development) rtw89_fwdl_check_path_ready_ax() can fail.
read_poll_timeout_atomic() with a delay of 1 µs and a timeout of
400000 µs can take 50 seconds with USB because of the time it takes to
send a USB control message. The firmware upload is tried 5 times, so
in total it takes 250 seconds.

Increase the delay to 250 µs in order to reduce the time
rtw89_fwdl_check_path_ready_ax() takes from 50 seconds to less than 1
second.

Make the same change in rtw89_fwdl_check_path_ready_be() as well.

Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtw89/mac.c    | 2 +-
 drivers/net/wireless/realtek/rtw89/mac_be.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw89/mac.c b/drivers/net/wireless/realtek/rtw89/mac.c
index ff4335ef4033..6878109e3f42 100644
--- a/drivers/net/wireless/realtek/rtw89/mac.c
+++ b/drivers/net/wireless/realtek/rtw89/mac.c
@@ -6928,7 +6928,7 @@ int rtw89_fwdl_check_path_ready_ax(struct rtw89_dev *rtwdev,
 	u8 val;
 
 	return read_poll_timeout_atomic(rtw89_read8, val, val & check,
-					1, FWDL_WAIT_CNT, false,
+					250, FWDL_WAIT_CNT, false,
 					rtwdev, R_AX_WCPU_FW_CTRL);
 }
 
diff --git a/drivers/net/wireless/realtek/rtw89/mac_be.c b/drivers/net/wireless/realtek/rtw89/mac_be.c
index 0078080b3999..cdb8005d2dc0 100644
--- a/drivers/net/wireless/realtek/rtw89/mac_be.c
+++ b/drivers/net/wireless/realtek/rtw89/mac_be.c
@@ -571,7 +571,7 @@ static int rtw89_fwdl_check_path_ready_be(struct rtw89_dev *rtwdev,
 	u32 val;
 
 	return read_poll_timeout_atomic(rtw89_read32, val, val & check,
-					1, 1000000, false,
+					250, 1000000, false,
 					rtwdev, R_BE_WCPU_FW_CTRL);
 }
 
-- 
2.50.0

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

[Ping-Ke Shih]

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. 

[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

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

[Bitterblue Smith]

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
> 

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

[Ping-Ke Shih]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> 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. 

Forgot to say, for PCIE devices please keep the timeout as was.

> (4000 * 250 = 1000000 and 3200 * 125 = 400000. I don't know
> why rtw89_read8() in the second patch takes only 125 µs.)

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

[Bitterblue Smith]

On 15/07/2025 03:28, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> 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. 
> 
> Forgot to say, for PCIE devices please keep the timeout as was.
> 

Yes, of course.

>> (4000 * 250 = 1000000 and 3200 * 125 = 400000. I don't know
>> why rtw89_read8() in the second patch takes only 125 µs.)
> 
>