All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH rtw-next v3] wifi: rtw88: usb: Download the reserved page synchronously
@ 2026-09-14 20:18 Bitterblue Smith
  2026-09-16  8:18 ` Ping-Ke Shih
  0 siblings, 1 reply; 3+ messages in thread
From: Bitterblue Smith @ 2026-09-14 20:18 UTC (permalink / raw)
  To: linux-wireless@vger.kernel.org; +Cc: Ping-Ke Shih, i.mafifi17@gmail.com

From: Mohammed Afifi <i.mafifi17@gmail.com>

In AP mode the reserved page (which contains the beacon) is downloaded
to the chip by rtw_fw_write_data_rsvd_page(). That function writes the
data via rtw_hci_write_data_rsvd_page() and then immediately polls the
hardware BCN_VALID bit to confirm the firmware accepted the beacon.

On USB, rtw_usb_write_data_rsvd_page() ends up in rtw_usb_write_port(),
which submits the URB with usb_submit_urb() and returns as soon as the
transfer is queued. The completion callback only frees the skb. As a
result the BCN_VALID poll can run while the beacon is still in flight
on the bus, and the poll times out even though the download itself is
fine.

This shows up as:

  rtw_8822bu: error beacon valid
  rtw_8822bu: failed to download drv rsvd page

The failure is intermittent because the poll retries sometimes cover
the transfer time. It occurs in bursts when something triggers repeated
beacon updates, such as stations associating and disassociating.

Fix this by making rtw_usb_write_data() wait for the URB to complete
when it's called from rtw_usb_write_data_rsvd_page(). Only the reserved
page / beacon path is made synchronous; the normal data TX path is left
untouched, so throughput is unaffected.

The reserved page download runs in process context under rtwdev->mutex,
so sleeping there is safe.

Closes: https://github.com/lwfinger/rtw88/issues/451
Signed-off-by: Mohammed Afifi <i.mafifi17@gmail.com>
Signed-off-by: Bitterblue Smith <rtl8821cerfe2@gmail.com>
---
Several people said that making rtw_usb_write_data_rsvd_page()
synchronous gets rid of the "error beacon valid" messages and makes AP
mode more stable. I wonder why the vendor driver doesn't need to do
this? It downloads the reserved page asynchronously and polls
BIT_BCN_VALID_V1 the same way rtw88 does.

v2:
 - Fix compilation error in rtw_usb_write_data_rsvd_page().

v3:
 - Reuse the existing rtw_usb_write_data() and
   rtw_usb_write_port_complete() functions instead of adding new ones.

 - It's not calling usb_kill_urb() anymore when the completion handler
   is not called within 5 seconds. This is probably fine because the
   documentation says "If the submission is successful, the complete()
   callback from the URB will be called exactly once, when the USB core
   and Host Controller Driver (HCD) are finished with the URB." So the
   handler will be called eventually.

 - Delete the unnecessary comments.
---
 drivers/net/wireless/realtek/rtw88/usb.c | 47 +++++++++++++++++++++---
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index c90802919473..6dad08f4bd4c 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -27,6 +27,13 @@ struct rtw_usb_txcb {
 	struct sk_buff_head tx_ack_queue;
 };
 
+struct rtw_usb_write_data_cb {
+	struct completion done;
+	struct sk_buff *skb;
+	int status;
+	bool sync;
+};
+
 static void rtw_usb_fill_tx_checksum(struct rtw_usb *rtwusb,
 				     struct sk_buff *skb, int agg_num)
 {
@@ -499,16 +506,21 @@ static void rtw_usb_tx_queue_purge(struct rtw_usb *rtwusb)
 
 static void rtw_usb_write_port_complete(struct urb *urb)
 {
-	struct sk_buff *skb = urb->context;
+	struct rtw_usb_write_data_cb *cb = urb->context;
 
-	dev_kfree_skb_any(skb);
+	cb->status = urb->status;
+	complete(&cb->done);
+	dev_kfree_skb_any(cb->skb);
+	if (!cb->sync)
+		kfree(cb);
 }
 
 static int rtw_usb_write_data(struct rtw_dev *rtwdev,
 			      struct rtw_tx_pkt_info *pkt_info,
-			      u8 *buf)
+			      u8 *buf, bool sync)
 {
 	const struct rtw_chip_info *chip = rtwdev->chip;
+	struct rtw_usb_write_data_cb *cb;
 	struct sk_buff *skb;
 	unsigned int size;
 	u8 qsel;
@@ -528,11 +540,29 @@ static int rtw_usb_write_data(struct rtw_dev *rtwdev,
 	rtw_tx_fill_tx_desc(rtwdev, pkt_info, skb);
 	rtw_tx_fill_txdesc_checksum(rtwdev, pkt_info, skb->data);
 
+	cb = kmalloc_obj(*cb, GFP_KERNEL);
+	if (!cb) {
+		dev_kfree_skb_any(skb);
+		return -ENOMEM;
+	}
+
+	init_completion(&cb->done);
+	cb->status = -EINPROGRESS;
+	cb->skb = skb;
+	cb->sync = sync;
+
 	ret = rtw_usb_write_port(rtwdev, qsel, skb,
-				 rtw_usb_write_port_complete, skb);
+				 rtw_usb_write_port_complete, cb);
 	if (unlikely(ret)) {
 		rtw_err(rtwdev, "failed to do USB write, ret=%d\n", ret);
 		dev_kfree_skb_any(skb);
+		kfree(cb);
+	} else if (sync) {
+		if (!wait_for_completion_timeout(&cb->done, secs_to_jiffies(5)))
+			ret = -ETIMEDOUT;
+		else
+			ret = cb->status;
+		kfree(cb);
 	}
 
 	return ret;
@@ -549,7 +579,12 @@ static int rtw_usb_write_data_rsvd_page(struct rtw_dev *rtwdev, u8 *buf,
 	pkt_info.offset = chip->tx_pkt_desc_sz;
 	pkt_info.ls = true;
 
-	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
+	/*
+	 * Download the beacon/reserved page synchronously so that the
+	 * caller's subsequent BCN_VALID poll observes the completed
+	 * transfer instead of racing the async TX path.
+	 */
+	return rtw_usb_write_data(rtwdev, &pkt_info, buf, true);
 }
 
 static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
@@ -559,7 +594,7 @@ static int rtw_usb_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
 	pkt_info.tx_pkt_size = size;
 	pkt_info.qsel = TX_DESC_QSEL_H2C;
 
-	return rtw_usb_write_data(rtwdev, &pkt_info, buf);
+	return rtw_usb_write_data(rtwdev, &pkt_info, buf, false);
 }
 
 static u8 rtw_usb_tx_queue_mapping_to_qsel(struct sk_buff *skb)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-09-16  9:00 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-14 20:18 [PATCH rtw-next v3] wifi: rtw88: usb: Download the reserved page synchronously Bitterblue Smith
2026-09-16  8:18 ` Ping-Ke Shih
2026-09-16  9:00   ` Ping-Ke Shih

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.