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

[Ping-Ke Shih <pkshih@realtek.com>]

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.

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