Re: [PATCH rtw-next v3] wifi: rtw89: usb: Support switching to USB 3 mode

[Bitterblue Smith <rtl8821cerfe2@gmail.com>]

On 18/05/2026 10:51, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> The Realtek wifi 6/7 devices which support USB 3 are weird: when first
>> plugged in, they pretend to be USB 2. The driver needs to send some
>> commands to the device, which make it disappear and come back as a
>> USB 3 device.
>>
>> Implement the required commands in rtw89.
>>
>> Add a new function rtw89_usb_write32_quiet() to avoid the warnings
>> when writing to R_{AX,BE}_PAD_CTRL2. Even though the write succeeds,
>> usb_control_msg() returns -EPROTO, probably because the USB device
>> disappears immediately. This results in some confusing warnings in
>> the kernel log.
>>
>> When a USB 3 device is plugged into a USB 2 port, rtw89 will try to
>> switch it to USB 3 mode only once. The device will disappear and come
>> back still in USB 2 mode, of course.
> 
> As we always try to switch USB 3, is it needed to add a hint to users
> to plug USB 2 port if he has a bad performance on 2GHz band?
> 

I can add a message like "2.4 GHz performance may be better in a USB 2
port".

>>
>> Tested with RTL8832AU, RTL8832BU, RTL8832CU, and RTL8912AU.
>>
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> 
> Some minor suggestions. Otherwise, 
> 
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> [...]
> 
>> diff --git a/drivers/net/wireless/realtek/rtw89/usb.c b/drivers/net/wireless/realtek/rtw89/usb.c
>> index c6d55e669776..bfe004a49ccc 100644
>> --- a/drivers/net/wireless/realtek/rtw89/usb.c
>> +++ b/drivers/net/wireless/realtek/rtw89/usb.c
>> @@ -12,7 +12,7 @@
>>  static void rtw89_usb_read_port_complete(struct urb *urb);
>>
>>  static void rtw89_usb_vendorreq(struct rtw89_dev *rtwdev, u32 addr,
>> -                               void *data, u16 len, u8 reqtype)
>> +                               void *data, u16 len, u8 reqtype, bool warn)
>>  {
>>         struct rtw89_usb *rtwusb = rtw89_usb_priv(rtwdev);
>>         struct usb_device *udev = rtwusb->udev;
>> @@ -52,7 +52,7 @@ static void rtw89_usb_vendorreq(struct rtw89_dev *rtwdev, u32 addr,
>>
>>                 if (ret == -ESHUTDOWN || ret == -ENODEV)
>>                         set_bit(RTW89_FLAG_UNPLUGGED, rtwdev->flags);
>> -               else if (ret < 0)
>> +               else if (ret < 0 && warn)
>>                         rtw89_warn(rtwdev,
>>                                    "usb %s%u 0x%x fail ret=%d value=0x%x attempt=%d\n",
>>                                    str_read_write(reqtype == RTW89_USB_VENQT_READ),
>> @@ -80,7 +80,7 @@ static u32 rtw89_usb_read_cmac(struct rtw89_dev *rtwdev, u32 addr)
>>
>>         for (count = 0; ; count++) {
>>                 rtw89_usb_vendorreq(rtwdev, addr32, &data, 4,
>> -                                   RTW89_USB_VENQT_READ);
>> +                                   RTW89_USB_VENQT_READ, true);
> 
> Not sure if you like to implement a __rtw89_usb_vendorreq(), and 
> preserve rtw89_usb_vendorreq() as a wrapper with original prototype.
> Then, no need to stir here. 
> 
> I mean:
> 
> __rtw89_usb_vendorreq(..., bool warn);
> 
> static void rtw89_usb_vendorreq(struct rtw89_dev *rtwdev, u32 addr,
> 				void *data, u16 len, u8 reqtype)
> {
> 	__rtw89_usb_vendorreq(rtwdev, addr, data, len, reqtype, true);
> }
> 
> (just a soft suggestion)
> 

Yes, that looks good.

>>
>>                 val32 = le32_to_cpu(data);
>>                 if (val32 != RTW89_R32_DEAD)
> 
> [...]
>