[PATCH rtw-next 1/2] wifi: rtw88: usb: Reduce control message timeout to 500 ms

[PATCH rtw-next 1/2] wifi: rtw88: usb: Reduce control message timeout to 500 ms

[Bitterblue Smith]

RTL8811AU stops responding during the firmware download on some systems:

[  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
[  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
[  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
[  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
[  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
[  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110

Each write takes 30 seconds to fail because that's the timeout currently
used for control messages in rtw_usb_write().

In this scenario the firmware download takes at least 2000 seconds.
Because this is done from the USB probe function, the long delay makes
other things in the system hang.

Reduce the timeout to 500 ms. This is the value used by the official USB
wifi drivers from Realtek.

Of course this only makes things hang for ~30 seconds instead of ~30
minutes. It doesn't fix the firmware download.

Tested with RTL8822CU, RTL8812BU, RTL8811CU, RTL8814AU, RTL8811AU,
RTL8812AU, RTL8821AU, RTL8723DU.

Cc: stable@vger.kernel.org
Fixes: a82dfd33d123 ("wifi: rtw88: Add common USB chip support")
Link: https://github.com/lwfinger/rtw88/issues/344
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtw88/usb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 204343ac2558..b16db579fdce 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -139,7 +139,7 @@ static void rtw_usb_write(struct rtw_dev *rtwdev, u32 addr, u32 val, int len)
 
 	ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
 			      RTW_USB_CMD_REQ, RTW_USB_CMD_WRITE,
-			      addr, 0, data, len, 30000);
+			      addr, 0, data, len, 500);
 	if (ret < 0 && ret != -ENODEV && count++ < 4)
 		rtw_err(rtwdev, "write register 0x%x failed with %d\n",
 			addr, ret);
-- 
2.49.0

[PATCH rtw-next 2/2] wifi: rtw88: usb: Upload the firmware in bigger chunks

[Bitterblue Smith]

RTL8811AU stops responding during the firmware download on some systems:

[  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
[  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
[  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
[  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
[  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
[  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110

Maybe it takes too long when writing the firmware 4 bytes at a time.

Write 196 bytes at a time for RTL8821AU, RTL8811AU, and RTL8812AU,
and 254 bytes at a time for RTL8723DU. These are the sizes used in
their official drivers. Tested with all these chips.

Cc: stable@vger.kernel.org
Link: https://github.com/lwfinger/rtw88/issues/344
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
 drivers/net/wireless/realtek/rtw88/hci.h  |  8 ++++
 drivers/net/wireless/realtek/rtw88/mac.c  | 11 +++--
 drivers/net/wireless/realtek/rtw88/mac.h  |  2 +
 drivers/net/wireless/realtek/rtw88/pci.c  |  2 +
 drivers/net/wireless/realtek/rtw88/sdio.c |  2 +
 drivers/net/wireless/realtek/rtw88/usb.c  | 55 +++++++++++++++++++++++
 6 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/hci.h b/drivers/net/wireless/realtek/rtw88/hci.h
index 96aeda26014e..d4bee9c3ecfe 100644
--- a/drivers/net/wireless/realtek/rtw88/hci.h
+++ b/drivers/net/wireless/realtek/rtw88/hci.h
@@ -19,6 +19,8 @@ struct rtw_hci_ops {
 	void (*link_ps)(struct rtw_dev *rtwdev, bool enter);
 	void (*interface_cfg)(struct rtw_dev *rtwdev);
 	void (*dynamic_rx_agg)(struct rtw_dev *rtwdev, bool enable);
+	void (*write_firmware_page)(struct rtw_dev *rtwdev, u32 page,
+				    const u8 *data, u32 size);
 
 	int (*write_data_rsvd_page)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
 	int (*write_data_h2c)(struct rtw_dev *rtwdev, u8 *buf, u32 size);
@@ -79,6 +81,12 @@ static inline void rtw_hci_dynamic_rx_agg(struct rtw_dev *rtwdev, bool enable)
 		rtwdev->hci.ops->dynamic_rx_agg(rtwdev, enable);
 }
 
+static inline void rtw_hci_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
+					       const u8 *data, u32 size)
+{
+	rtwdev->hci.ops->write_firmware_page(rtwdev, page, data, size);
+}
+
 static inline int
 rtw_hci_write_data_rsvd_page(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 {
diff --git a/drivers/net/wireless/realtek/rtw88/mac.c b/drivers/net/wireless/realtek/rtw88/mac.c
index 0491f501c138..f66d1b302dc5 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.c
+++ b/drivers/net/wireless/realtek/rtw88/mac.c
@@ -856,8 +856,8 @@ static void en_download_firmware_legacy(struct rtw_dev *rtwdev, bool en)
 	}
 }
 
-static void
-write_firmware_page(struct rtw_dev *rtwdev, u32 page, const u8 *data, u32 size)
+void rtw_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
+			     const u8 *data, u32 size)
 {
 	u32 val32;
 	u32 block_nr;
@@ -887,6 +887,7 @@ write_firmware_page(struct rtw_dev *rtwdev, u32 page, const u8 *data, u32 size)
 		rtw_write32(rtwdev, write_addr, le32_to_cpu(remain_data));
 	}
 }
+EXPORT_SYMBOL(rtw_write_firmware_page);
 
 static int
 download_firmware_legacy(struct rtw_dev *rtwdev, const u8 *data, u32 size)
@@ -904,11 +905,13 @@ download_firmware_legacy(struct rtw_dev *rtwdev, const u8 *data, u32 size)
 	rtw_write8_set(rtwdev, REG_MCUFW_CTRL, BIT_FWDL_CHK_RPT);
 
 	for (page = 0; page < total_page; page++) {
-		write_firmware_page(rtwdev, page, data, DLFW_PAGE_SIZE_LEGACY);
+		rtw_hci_write_firmware_page(rtwdev, page, data,
+					    DLFW_PAGE_SIZE_LEGACY);
 		data += DLFW_PAGE_SIZE_LEGACY;
 	}
 	if (last_page_size)
-		write_firmware_page(rtwdev, page, data, last_page_size);
+		rtw_hci_write_firmware_page(rtwdev, page, data,
+					    last_page_size);
 
 	if (!check_hw_ready(rtwdev, REG_MCUFW_CTRL, BIT_FWDL_CHK_RPT, 1)) {
 		rtw_err(rtwdev, "failed to check download firmware report\n");
diff --git a/drivers/net/wireless/realtek/rtw88/mac.h b/drivers/net/wireless/realtek/rtw88/mac.h
index 6905e2747372..e92b1483728d 100644
--- a/drivers/net/wireless/realtek/rtw88/mac.h
+++ b/drivers/net/wireless/realtek/rtw88/mac.h
@@ -34,6 +34,8 @@ int rtw_pwr_seq_parser(struct rtw_dev *rtwdev,
 		       const struct rtw_pwr_seq_cmd * const *cmd_seq);
 int rtw_mac_power_on(struct rtw_dev *rtwdev);
 void rtw_mac_power_off(struct rtw_dev *rtwdev);
+void rtw_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
+			     const u8 *data, u32 size);
 int rtw_download_firmware(struct rtw_dev *rtwdev, struct rtw_fw_state *fw);
 int rtw_mac_init(struct rtw_dev *rtwdev);
 void rtw_mac_flush_queues(struct rtw_dev *rtwdev, u32 queues, bool drop);
diff --git a/drivers/net/wireless/realtek/rtw88/pci.c b/drivers/net/wireless/realtek/rtw88/pci.c
index bb4c4ccb31d4..7f2b6dc21f56 100644
--- a/drivers/net/wireless/realtek/rtw88/pci.c
+++ b/drivers/net/wireless/realtek/rtw88/pci.c
@@ -12,6 +12,7 @@
 #include "fw.h"
 #include "ps.h"
 #include "debug.h"
+#include "mac.h"
 
 static bool rtw_disable_msi;
 static bool rtw_pci_disable_aspm;
@@ -1602,6 +1603,7 @@ static const struct rtw_hci_ops rtw_pci_ops = {
 	.link_ps = rtw_pci_link_ps,
 	.interface_cfg = rtw_pci_interface_cfg,
 	.dynamic_rx_agg = NULL,
+	.write_firmware_page = rtw_write_firmware_page,
 
 	.read8 = rtw_pci_read8,
 	.read16 = rtw_pci_read16,
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 71cbe49b6c59..e733ed846123 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -10,6 +10,7 @@
 #include <linux/mmc/host.h>
 #include <linux/mmc/sdio_func.h>
 #include "main.h"
+#include "mac.h"
 #include "debug.h"
 #include "fw.h"
 #include "ps.h"
@@ -1164,6 +1165,7 @@ static const struct rtw_hci_ops rtw_sdio_ops = {
 	.link_ps = rtw_sdio_link_ps,
 	.interface_cfg = rtw_sdio_interface_cfg,
 	.dynamic_rx_agg = NULL,
+	.write_firmware_page = rtw_write_firmware_page,
 
 	.read8 = rtw_sdio_read8,
 	.read16 = rtw_sdio_read16,
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index b16db579fdce..ad15ce12ab7f 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -165,6 +165,60 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
 	rtw_usb_write(rtwdev, addr, val, 4);
 }
 
+static void rtw_usb_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
+					const u8 *data, u32 size)
+{
+	struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
+	struct usb_device *udev = rtwusb->udev;
+	u32 addr = FW_8192C_START_ADDRESS;
+	u8 *data_dup, *buf;
+	u32 n, block_size;
+	int ret;
+
+	switch (rtwdev->chip->id) {
+	case RTW_CHIP_TYPE_8723D:
+		block_size = 254;
+		break;
+	default:
+		block_size = 196;
+		break;
+	}
+
+	data_dup = kmemdup(data, size, GFP_KERNEL);
+	if (!data_dup)
+		return;
+
+	buf = data_dup;
+
+	rtw_write32_mask(rtwdev, REG_MCUFW_CTRL, BIT_ROM_PGE, page);
+
+	while (size > 0) {
+		if (size >= block_size)
+			n = block_size;
+		else if (size >= 8)
+			n = 8;
+		else
+			n = 1;
+
+		ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
+				      RTW_USB_CMD_REQ, RTW_USB_CMD_WRITE,
+				      addr, 0, buf, n, 500);
+		if (ret != n) {
+			if (ret != -ENODEV)
+				rtw_err(rtwdev,
+					"write 0x%x len %d failed: %d\n",
+					addr, n, ret);
+			break;
+		}
+
+		addr += n;
+		buf += n;
+		size -= n;
+	}
+
+	kfree(data_dup);
+}
+
 static int dma_mapping_to_ep(enum rtw_dma_mapping dma_mapping)
 {
 	switch (dma_mapping) {
@@ -892,6 +946,7 @@ static const struct rtw_hci_ops rtw_usb_ops = {
 	.link_ps = rtw_usb_link_ps,
 	.interface_cfg = rtw_usb_interface_cfg,
 	.dynamic_rx_agg = rtw_usb_dynamic_rx_agg,
+	.write_firmware_page = rtw_usb_write_firmware_page,
 
 	.write8  = rtw_usb_write8,
 	.write16 = rtw_usb_write16,
-- 
2.49.0

RE: [PATCH rtw-next 2/2] wifi: rtw88: usb: Upload the firmware in bigger chunks

[Ping-Ke Shih]

: Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> RTL8811AU stops responding during the firmware download on some systems:
> 
> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
> 
> Maybe it takes too long when writing the firmware 4 bytes at a time.
> 
> Write 196 bytes at a time for RTL8821AU, RTL8811AU, and RTL8812AU,
> and 254 bytes at a time for RTL8723DU. These are the sizes used in
> their official drivers. Tested with all these chips.
> 
> Cc: stable@vger.kernel.org
> Link: https://github.com/lwfinger/rtw88/issues/344
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

[..]

> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index b16db579fdce..ad15ce12ab7f 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -165,6 +165,60 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
>         rtw_usb_write(rtwdev, addr, val, 4);
>  }
> 
> +static void rtw_usb_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
> +                                       const u8 *data, u32 size)
> +{
> +       struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> +       struct usb_device *udev = rtwusb->udev;
> +       u32 addr = FW_8192C_START_ADDRESS;

FW_8192C_START_ADDRESS is existing already. But something like
RTW_USB_FW_START_ADDRESS would be better. 

> +       u8 *data_dup, *buf;
> +       u32 n, block_size;
> +       int ret;
> +
> +       switch (rtwdev->chip->id) {
> +       case RTW_CHIP_TYPE_8723D:
> +               block_size = 254;
> +               break;
> +       default:
> +               block_size = 196;
> +               break;
> +       }
> +
> +       data_dup = kmemdup(data, size, GFP_KERNEL);

This is because type of argument `data` of usb_control_msg() is not const, right?
Do you know if usb_control_msg() will actually modify the data?

Re: [PATCH rtw-next 2/2] wifi: rtw88: usb: Upload the firmware in bigger chunks

[Bitterblue Smith]

On 08/05/2025 06:29, Ping-Ke Shih wrote:
> : Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> RTL8811AU stops responding during the firmware download on some systems:
>>
>> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
>> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
>> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
>> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
>> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
>> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
>>
>> Maybe it takes too long when writing the firmware 4 bytes at a time.
>>
>> Write 196 bytes at a time for RTL8821AU, RTL8811AU, and RTL8812AU,
>> and 254 bytes at a time for RTL8723DU. These are the sizes used in
>> their official drivers. Tested with all these chips.
>>
>> Cc: stable@vger.kernel.org
>> Link: https://github.com/lwfinger/rtw88/issues/344
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> 
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> [..]
> 
>> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>> index b16db579fdce..ad15ce12ab7f 100644
>> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>> @@ -165,6 +165,60 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
>>         rtw_usb_write(rtwdev, addr, val, 4);
>>  }
>>
>> +static void rtw_usb_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
>> +                                       const u8 *data, u32 size)
>> +{
>> +       struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> +       struct usb_device *udev = rtwusb->udev;
>> +       u32 addr = FW_8192C_START_ADDRESS;
> 
> FW_8192C_START_ADDRESS is existing already. But something like
> RTW_USB_FW_START_ADDRESS would be better. 
> 

I agree, because rtw88 doesn't handle RTL8192C. There is
FW_START_ADDR_LEGACY in fw.h. I must not have noticed it before.
Should I send v2 for this?

>> +       u8 *data_dup, *buf;
>> +       u32 n, block_size;
>> +       int ret;
>> +
>> +       switch (rtwdev->chip->id) {
>> +       case RTW_CHIP_TYPE_8723D:
>> +               block_size = 254;
>> +               break;
>> +       default:
>> +               block_size = 196;
>> +               break;
>> +       }
>> +
>> +       data_dup = kmemdup(data, size, GFP_KERNEL);
> 
> This is because type of argument `data` of usb_control_msg() is not const, right?
> Do you know if usb_control_msg() will actually modify the data?
> 

No, it's because usb_control_msg() rejects memory allocated by
vmalloc(). I don't remember what error it printed. Maybe because the
memory is not suitable for DMA.

RE: [PATCH rtw-next 2/2] wifi: rtw88: usb: Upload the firmware in bigger chunks

[Ping-Ke Shih]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> On 08/05/2025 06:29, Ping-Ke Shih wrote:
> > : Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> >> RTL8811AU stops responding during the firmware download on some systems:
> >>
> >> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
> >> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
> >> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
> >> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
> >> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
> >> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
> >>
> >> Maybe it takes too long when writing the firmware 4 bytes at a time.
> >>
> >> Write 196 bytes at a time for RTL8821AU, RTL8811AU, and RTL8812AU,
> >> and 254 bytes at a time for RTL8723DU. These are the sizes used in
> >> their official drivers. Tested with all these chips.
> >>
> >> Cc: stable@vger.kernel.org
> >> Link: https://github.com/lwfinger/rtw88/issues/344
> >> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> >
> > Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> >
> > [..]
> >
> >> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> >> index b16db579fdce..ad15ce12ab7f 100644
> >> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> >> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> >> @@ -165,6 +165,60 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
> >>         rtw_usb_write(rtwdev, addr, val, 4);
> >>  }
> >>
> >> +static void rtw_usb_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
> >> +                                       const u8 *data, u32 size)
> >> +{
> >> +       struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> >> +       struct usb_device *udev = rtwusb->udev;
> >> +       u32 addr = FW_8192C_START_ADDRESS;
> >
> > FW_8192C_START_ADDRESS is existing already. But something like
> > RTW_USB_FW_START_ADDRESS would be better.
> >
> 
> I agree, because rtw88 doesn't handle RTL8192C. There is
> FW_START_ADDR_LEGACY in fw.h. I must not have noticed it before.
> Should I send v2 for this?

Yes, please. I don't change patch content during committing to prevent mess up
something. Since you only change the naming, please carry my Ack-by to next
version.

> 
> >> +       u8 *data_dup, *buf;
> >> +       u32 n, block_size;
> >> +       int ret;
> >> +
> >> +       switch (rtwdev->chip->id) {
> >> +       case RTW_CHIP_TYPE_8723D:
> >> +               block_size = 254;
> >> +               break;
> >> +       default:
> >> +               block_size = 196;
> >> +               break;
> >> +       }
> >> +
> >> +       data_dup = kmemdup(data, size, GFP_KERNEL);
> >
> > This is because type of argument `data` of usb_control_msg() is not const, right?
> > Do you know if usb_control_msg() will actually modify the data?
> >
> 
> No, it's because usb_control_msg() rejects memory allocated by
> vmalloc(). I don't remember what error it printed. Maybe because the
> memory is not suitable for DMA.

Do you mean firmware is placed in memory allocated by vmalloc()?
If so, it makes sense to the reason you said. 

Re: [PATCH rtw-next 2/2] wifi: rtw88: usb: Upload the firmware in bigger chunks

[Bitterblue Smith]

On 09/05/2025 03:43, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> On 08/05/2025 06:29, Ping-Ke Shih wrote:
>>> : Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>>>> RTL8811AU stops responding during the firmware download on some systems:
>>>>
>>>> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
>>>> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
>>>> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
>>>> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
>>>> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
>>>> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
>>>>
>>>> Maybe it takes too long when writing the firmware 4 bytes at a time.
>>>>
>>>> Write 196 bytes at a time for RTL8821AU, RTL8811AU, and RTL8812AU,
>>>> and 254 bytes at a time for RTL8723DU. These are the sizes used in
>>>> their official drivers. Tested with all these chips.
>>>>
>>>> Cc: stable@vger.kernel.org
>>>> Link: https://github.com/lwfinger/rtw88/issues/344
>>>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
>>>
>>> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
>>>
>>> [..]
>>>
>>>> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>>>> index b16db579fdce..ad15ce12ab7f 100644
>>>> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>>>> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>>>> @@ -165,6 +165,60 @@ static void rtw_usb_write32(struct rtw_dev *rtwdev, u32 addr, u32 val)
>>>>         rtw_usb_write(rtwdev, addr, val, 4);
>>>>  }
>>>>
>>>> +static void rtw_usb_write_firmware_page(struct rtw_dev *rtwdev, u32 page,
>>>> +                                       const u8 *data, u32 size)
>>>> +{
>>>> +       struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>>>> +       struct usb_device *udev = rtwusb->udev;
>>>> +       u32 addr = FW_8192C_START_ADDRESS;
>>>
>>> FW_8192C_START_ADDRESS is existing already. But something like
>>> RTW_USB_FW_START_ADDRESS would be better.
>>>
>>
>> I agree, because rtw88 doesn't handle RTL8192C. There is
>> FW_START_ADDR_LEGACY in fw.h. I must not have noticed it before.
>> Should I send v2 for this?
> 
> Yes, please. I don't change patch content during committing to prevent mess up
> something. Since you only change the naming, please carry my Ack-by to next
> version.
> 
>>
>>>> +       u8 *data_dup, *buf;
>>>> +       u32 n, block_size;
>>>> +       int ret;
>>>> +
>>>> +       switch (rtwdev->chip->id) {
>>>> +       case RTW_CHIP_TYPE_8723D:
>>>> +               block_size = 254;
>>>> +               break;
>>>> +       default:
>>>> +               block_size = 196;
>>>> +               break;
>>>> +       }
>>>> +
>>>> +       data_dup = kmemdup(data, size, GFP_KERNEL);
>>>
>>> This is because type of argument `data` of usb_control_msg() is not const, right?
>>> Do you know if usb_control_msg() will actually modify the data?
>>>
>>
>> No, it's because usb_control_msg() rejects memory allocated by
>> vmalloc(). I don't remember what error it printed. Maybe because the
>> memory is not suitable for DMA.
> 
> Do you mean firmware is placed in memory allocated by vmalloc()?
> If so, it makes sense to the reason you said. 
> 

Yes.

RE: [PATCH rtw-next 1/2] wifi: rtw88: usb: Reduce control message timeout to 500 ms

[Ping-Ke Shih]

Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
> RTL8811AU stops responding during the firmware download on some systems:
> 
> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
> 
> Each write takes 30 seconds to fail because that's the timeout currently
> used for control messages in rtw_usb_write().
> 
> In this scenario the firmware download takes at least 2000 seconds.
> Because this is done from the USB probe function, the long delay makes
> other things in the system hang.
> 
> Reduce the timeout to 500 ms. This is the value used by the official USB
> wifi drivers from Realtek.

A question about timeout time. Is this enough for USB 2 or older?

> 
> Of course this only makes things hang for ~30 seconds instead of ~30
> minutes. It doesn't fix the firmware download.
> 
> Tested with RTL8822CU, RTL8812BU, RTL8811CU, RTL8814AU, RTL8811AU,
> RTL8812AU, RTL8821AU, RTL8723DU.
> 
> Cc: stable@vger.kernel.org
> Fixes: a82dfd33d123 ("wifi: rtw88: Add common USB chip support")
> Link: https://github.com/lwfinger/rtw88/issues/344
> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>

Acked-by: Ping-Ke Shih <pkshih@realtek.com>

Re: [PATCH rtw-next 1/2] wifi: rtw88: usb: Reduce control message timeout to 500 ms

[Bitterblue Smith]

On 08/05/2025 06:11, Ping-Ke Shih wrote:
> Bitterblue Smith <rtl8821cerfe2@gmail.com> wrote:
>> RTL8811AU stops responding during the firmware download on some systems:
>>
>> [  809.256440] rtw_8821au 5-2.1:1.0: Firmware version 42.4.0, H2C version 0
>> [  812.759142] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: renamed from wlan0
>> [  837.315388] rtw_8821au 1-4:1.0: write register 0x1ef4 failed with -110
>> [  867.524259] rtw_8821au 1-4:1.0: write register 0x1ef8 failed with -110
>> [  868.930976] rtw_8821au 5-2.1:1.0 wlp48s0f4u2u1: entered promiscuous mode
>> [  897.730952] rtw_8821au 1-4:1.0: write register 0x1efc failed with -110
>>
>> Each write takes 30 seconds to fail because that's the timeout currently
>> used for control messages in rtw_usb_write().
>>
>> In this scenario the firmware download takes at least 2000 seconds.
>> Because this is done from the USB probe function, the long delay makes
>> other things in the system hang.
>>
>> Reduce the timeout to 500 ms. This is the value used by the official USB
>> wifi drivers from Realtek.
> 
> A question about timeout time. Is this enough for USB 2 or older?
> 

Yes, it's fine.

>>
>> Of course this only makes things hang for ~30 seconds instead of ~30
>> minutes. It doesn't fix the firmware download.
>>
>> Tested with RTL8822CU, RTL8812BU, RTL8811CU, RTL8814AU, RTL8811AU,
>> RTL8812AU, RTL8821AU, RTL8723DU.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: a82dfd33d123 ("wifi: rtw88: Add common USB chip support")
>> Link: https://github.com/lwfinger/rtw88/issues/344
>> Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
> 
> Acked-by: Ping-Ke Shih <pkshih@realtek.com>
> 
>