Re: [PATCH rtw-next v2 1/2] wifi: rtw89: Lower the timeout in rtw89_fw_read_c2h_reg() for USB

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 16/07/2025 03:17, 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.
>>
>> Lower the timeout to 4000 for USB in order to reduce the maximum polling
>> time to ~1 second.
>>
>> 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>
>> ---
>> v2:
>>  - Lower the timeout for USB instead of increasing the delay.
>> ---
>>  drivers/net/wireless/realtek/rtw89/fw.c | 9 +++++++--
>>  drivers/net/wireless/realtek/rtw89/fw.h | 2 ++
>>  2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
>> index 73a4ec988d16..b774a329e7c7 100644
>> --- a/drivers/net/wireless/realtek/rtw89/fw.c
>> +++ b/drivers/net/wireless/realtek/rtw89/fw.c
>> @@ -6755,13 +6755,18 @@ static int rtw89_fw_read_c2h_reg(struct rtw89_dev *rtwdev,
>>         const struct rtw89_chip_info *chip = rtwdev->chip;
>>         struct rtw89_fw_info *fw_info = &rtwdev->fw;
>>         const u32 *c2h_reg = chip->c2h_regs;
>> -       u32 ret;
>> +       u32 ret, timeout;
>>         u8 i, val;
>>
>>         info->id = RTW89_FWCMD_C2HREG_FUNC_NULL;
>>
>> +       if (rtwdev->hci.type == RTW89_HCI_TYPE_USB)
>> +               timeout = RTW89_C2H_TIMEOUT_USB;
> 
> Should we have different timeout times for USB2 and USB3? 
> The same question for patch 2/2.
> 

I measured the time it takes to read R_AX_WCPU_FW_CTRL (patch 2/2):

		USB 2		USB 3
RTL8851BU	125 µs		not supported
RTL8852BU	250 µs		40 µs
RTL8852CU	375 µs		40 µs

In my setup the same timeout works for both. Even with the faster reads
it still waits long enough.

> Otherwise, looks good to me.
> 
> 
>> +       else
>> +               timeout = RTW89_C2H_TIMEOUT;
>> +
>>         ret = read_poll_timeout_atomic(rtw89_read8, val, val, 1,
>> -                                      RTW89_C2H_TIMEOUT, false, rtwdev,
>> +                                      timeout, false, rtwdev,
>>                                        chip->c2h_ctrl_reg);
>>         if (ret) {
>>                 rtw89_warn(rtwdev, "c2h reg timeout\n");
>> diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
>> index 98be7e72c685..a731c242f389 100644
>> --- a/drivers/net/wireless/realtek/rtw89/fw.h
>> +++ b/drivers/net/wireless/realtek/rtw89/fw.h
>> @@ -115,6 +115,8 @@ struct rtw89_h2creg_sch_tx_en {
>>  #define RTW89_C2HREG_HDR_LEN 2
>>  #define RTW89_H2CREG_HDR_LEN 2
>>  #define RTW89_C2H_TIMEOUT 1000000
>> +#define RTW89_C2H_TIMEOUT_USB 4000
>> +
>>  struct rtw89_mac_c2h_info {
>>         u8 id;
>>         u8 content_len;
>> --
>> 2.50.0
>