[PATCH v3 0/5] wifi: rtw88: preparations for RTL8723B/RTL8723BS

[PATCH v3 0/5] wifi: rtw88: preparations for RTL8723B/RTL8723BS

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

This is the first of two series adding support for the Realtek RTL8723B
802.11n chipset and its RTL8723BS SDIO variant to rtw88. It contains
only the changes to the shared rtw88 core that the chip driver depends
on. The chip itself, the build glue and the MAINTAINERS entry are a
second series.

v1 had 19 patches, v2 had 11, this has 5.

There is no rtw88-style firmware for this chip and no documentation for
the vendor blob it does use, so while bringing it up I deliberately kept
the driver as close to the vendor driver's behaviour as I could. With
that many unknowns at once, matching the vendor exactly was the only way
to tell which difference actually mattered when something did not work,
rather than guessing. A number of the v1 patches came from that: they
reproduced vendor behaviour that was useful to hold fixed during bring
up, not behaviour the chip turns out to require.

Bitterblue Smith's review of v1 prompted me to go back and remove each
of those on hardware to find which were actually load bearing; eight
were not. Ping-Ke Shih's review of v2 did the same for a further six,
and the changelog below gives the measurement for each. In three of
those cases I had told the list, or claimed in a commit message, that
the chip needed something it does not, on the strength of bring-up
notes I had not re-measured. All three corrections are in the
changelog.

What is left is there because removing it made something measurably
worse, not because the vendor driver does it.

What is left falls into two groups:

  - SDIO (patches 3, 4, 5): software free-page and output-queue
    accounting, RX aggregation and interrupt setup, and TX back-pressure
    with a retry on page starvation;
  - patches 1 and 2: the chip type and helper plus a one-line receive
    fix.

This series no longer touches fw.c, coex.c or mac80211.c at all.

Scope
=====

Everything is gated on rtw_is_8723bs(), which is false for every chip
currently supported, so behaviour for existing devices is unchanged.
There is one exception, deliberate: patch 2 extends an existing RTL8703B
zero-length-packet check and is gated on the SDIO interface rather than
the chip id, since that is the only place the behaviour has been
observed. Ping-Ke has suggested that check may not need a chip test at
all; I would rather measure that on RTL8703B hardware than assume it,
and I do not have that part, so it is unchanged here.

Patch 4 changes how the SDIO interrupt status is acknowledged. In v1
that applied to all SDIO parts; it is scoped to this chip only, and the
others keep writing the status word back unchanged.

Chip specific code in the common path
=====================================

Ping-Ke asked whether this needs a set of ops rather than a chip test
sprinkled through the core. Working through that question is what
removed most of this series: once the association and coexistence
patches turned out not to be needed, the chip tests they carried went
with them.

What remains is one test in rx.c and eleven in sdio.c. There are none
in fw.c, mac80211.c or coex.c. The SDIO ones are in the bus driver
rather than the chip driver, and rtw_sdio has no per-chip ops table
today; Ping-Ke confirmed an inline chip id check there is fine.

Fixed in the second series
==========================

Two of Bitterblue's review points are addressed in the chip driver
rather than here, because that is the right place for both:

  - the hardware capability is filled in from a chip-specific
    read_efuse, instead of special casing rtw_dump_hw_feature() in the
    core;
  - the receive FCS handling is a chip configuration fix. v1 stopped the
    core advertising RX_INCLUDES_FCS for this chip. The real cause was
    that the chip cleared BIT_APP_FCS by assigning WLAN_RCR_CFG over
    hal.rcr wholesale, where the 8723x siblings only write the register
    and leave hal.rcr alone. Setting that bit in the chip's WLAN_RCR_CFG
    makes the descriptor carry the FCS like every other rtw88 chip, so
    the core needs no special case and both hunks are gone from this
    series.

Neither change is visible here. I planned to send that series once this
one is applied to rtw-next, but I can send it now alongside if you would
rather review them together, or fold the two into one series if that is
easier.

Testing
=======

Tested on RTL8723BS hardware together with the second series: repeated
scan, authentication, association, WPA2-PSK/CCMP handshake, DHCP,
bidirectional traffic, reconnects and module reloads.

Throughput is measured against an iperf3 server one hop behind the AP,
with the wlan0 byte counters as ground truth because the test machine's
wired interface shares the same subnet. On one AP at strong signal
(-35 dBm), 2.4 GHz HT40: TCP 19.1 Mbit/s up and 37.8 Mbit/s down, UDP
25.0 Mbit/s up and 40.0 Mbit/s down, both at 0% loss.

Uplink on this band drifts by several Mbit/s from day to day, enough to
swamp the differences I was trying to measure, so where the changelog
below compares two builds the comparison is a paired test: the two
builds alternate within one session, rebuilt and reinstalled each time,
rather than being measured on different days.

Suspend and resume are not covered: this machine only offers s2idle and
does not reliably come back from it.

A five minute soak: 1495/1495 pings at 0% loss, 15/15 reconnects, 5/5
link up/down cycles, 3/3 module reloads, 5/5 scans under traffic, 24
group rekeys, no deauthentication, no SDIO errors, no warnings in the
log. Scanning saw the target AP in 18 of 18 attempts across 6 cold
module reloads, with 6/6 associations.

The soak and scan figures were taken on a build that also carried the
reserved page patch dropped below; the throughput figures above are from
the tree posted here.

Every patch builds with W=1 with no warnings on its own, sparse is clean
at the tip, and smatch reports nothing in any file this series touches.
checkpatch --strict is clean across this series. The other rtw88 bus and
chip modules continue to build.

Changes in v3:
  - Dropped "fw: fix the reserved page upload on RTL8723BS". Its commit
    message said the BIT_BCN_VALID handshake fails on 8723BS SDIO
    without it. That does not reproduce. The failure path is an explicit
    rtw_err("error beacon valid") returning -EBUSY, reached from
    BSS_CHANGED_ASSOC on every association; over three associations on
    each of five builds with the patch reverted it never fired,
    associations were 3/3, and there was no packet loss after 30 s idle
    in power save, which is the path the reserved page's null data and
    PS-poll frames serve. As a control, rtw_info messages appear in the
    same captures and rtw_err outranks them, so a failure would have
    been logged. AP mode and WoWLAN also download the reserved page and
    are not tested here, so if it turns out to matter there it can come
    back with evidence behind it.
  - Kept "sdio: add TX back-pressure and retry on page starvation", now
    with a measurement rather than an assertion, since you asked whether
    the remaining chip tests are necessary. Counting
    rtw_sdio_tx_handler invocations under an identical 60 Mbit/s UDP
    flood, because the retry re-arms that same work item: 1798 and 3095
    on two baseline runs against 69 with the patch reverted. Achievable
    transmit rate falls by about a third without it, 25.5 Mbit/s against
    38.3 and 40.2. tx_dropped and tx_errors stay at zero throughout, so
    the failure is not dropping frames, it is failing to push them.
  - Kept "sdio: set up RX aggregation and interrupts". The aggregation
    half makes no measurable difference to throughput here, and on that
    evidence alone I would have dropped it. The rest of the patch does
    not show up in a throughput test at all: the chip keeps raising the
    interrupt after resume if undefined status bits are written back
    when acknowledging.

    I have not been able to validate the suspend and resume path: the
    test machine only offers s2idle and does not reliably resume from
    it, so I cannot exercise the interrupt acknowledgement change. It is
    kept because the failure it addresses was observed during bring up,
    not because it has been re-measured. If someone with hardware that
    suspends cleanly finds it unnecessary, it should go.
  - Dropped "run the RTL8723BS association register sequence", 440
    lines, which was the vendor join sequence, six sites in
    rtw_ops_bss_info_changed() and both sites in rtw_ops_set_key().

    Ping-Ke asked for this to run from rtw_chip_prepare_tx() rather than
    a chip test in mgd_prepare_tx(), and I had done that with a new
    chip_ops::prepare_tx. Since the whole patch is gone, so is the
    callback; the core is left as it was rather than gaining an op with
    no user.

    I had told the list that association fails without these registers.
    That was wrong, and it was bring-up-era data I repeated without
    re-measuring. With the whole patch reverted: 18/18 scans saw the AP,
    6/6 associations, and the five minute soak above, whose 15 reconnects
    and 24 rekeys exercise the exact path the sequence ran on, since it
    ran from mgd_prepare_tx() on every authentication. Uplink measured
    paired against the full series, 19.0 against 19.2 Mbit/s over ten
    runs each.

    The default key search handling in that patch was also simply wrong:
    rtw_sec_enable_sec_engine() sets sec->default_key_search itself and
    programs all four USE_DK bits on that basis, so the enable path was
    a no-op and the disable path was clearing bits the core had
    deliberately set.
  - Dropped "coex: add the RTL8723BS scan antenna workaround" and "coex:
    reassert the antenna path when associating", and with them "fw: add
    the GNT_BT firmware command", whose only caller they were.

    The commit message on the first claimed the site survey does not
    hear the AP reliably without it. That is also wrong: 18/18 scans
    without it. Both were gated on the chip and on coex bt_disabled, so
    that gate is their entire scope rather than a subset of it, and the
    measurements above cover it.
  - Dropped "record beacons from the target BSSID before
    authenticating". Ping-Ke doubted the beacon wait affected the
    connection and he was right: with the wait removed, association
    succeeded 12/12 from a cold module reload and 12/12 from a warm
    reconnect, against 10/10 and 10/10 with it, at the same latency.
    The pre-auth deauth already sleeps 100 ms with the recording window
    open, so on a 100 ms beacon interval the wait was almost always
    already satisfied when it was reached. struct rtw_auth_sync and the
    RX-path hook go with it.
  - Dropped the BT_MP version queries and the BT_INFO query at scan
    start, and the BT_INFO query plus two of three repeated PS-TDMA
    H2Cs in the pre-auth replay. Measured over cold module reloads with
    three scans each, 8 reloads with the queries and 10 without: 29/30
    scans saw the target AP and 10/10 associations succeeded without
    them, against 24/24 and 8/8 with them.
  - The cached SDIO page counters are clamped at zero. A lost update
    between the free page check and the accounting could previously
    drive the public counter negative, and the sum was assigned to an
    unsigned, which made the check pass unconditionally from then on and
    suppressed the resync that was supposed to recover it.
  - REG_SDIO_FREE_TXPG now has field masks and is decoded with
    u32_get_bits(); the RX DMA burst count uses BIT_DMA_BURST_CNT;
    open-coded BIT(0)|BIT(1) on REG_SYS_FUNC_EN uses the existing names.
  - The RTL8723BS-specific SDIO helpers are named _8723bs_ and the chip
    test moved to their callers.
  - rtw_sdio_process_tx_queue() returns 1/0/-errno instead of taking a
    "processed" out-parameter, which makes the retry and back-pressure
    logic in the TX handler readable.
  - mod_delayed_work() in rtw_sdio_tx_kick_off() carries a comment
    saying why it is not queue_delayed_work(): a page-starvation retry
    may already be armed with a delay, and the newly queued frame should
    not wait for it.
  - An argument that had a single value at every call site is gone,
    along with its dead branch.
  - Block comments use the general kernel style, per commit 82b8000c28b5
    ("net: drop special comment style").

Changes in v2:
  - Dropped "tx: extend the TX report purge timeout". Instrumenting the
    report path shows payload[6] & 0xfc equals the enqueued sequence
    number, 32 out of 32 times, with rtw88's existing decode, and no
    report is missed. The longer timeout was covering for the next
    patch, not for the hardware.
  - Dropped "fw: handle the RTL8723BS management TX reports". 0x12 and
    0x32 are the first payload byte of C2H_CCX_TX_RPT, not C2H IDs;
    every C2H event this chip sends carries a known id, so the existing
    handler already covers them.
  - Dropped "fw: send rate adaptation and RSSI info in the vendor
    layout". The vendor byte layout is equivalent to what the existing
    macros produce; uplink and the negotiated rate are unchanged without
    it (16.4/16.5/17.0 against 15.4/16.7/16.6 Mbit/s, MCS7 both ways).
  - Dropped "fw: send the media status report in the vendor layout". The
    role field the vendor sets makes no difference here, as with the
    other chips.
  - Dropped "sdio: handle the RTL8723BS management TX path". Sequence
    numbers do work for management frames on this chip, so reporting
    completion at DMA completion is unnecessary.
  - Dropped "calibrate and tune the PHY" entirely. All three parts were
    vendor-matching scaffolding and none survived measurement: the
    scan-time initial gain override is worse than
    rtw_phy_dig_set_max_coverage() (five scans found 41 BSSes without it
    against 34 with it); the IQ calibration works from the normal
    phy_calibration path; and the hardcoded per-rate TX AGC table
    overrode the efuse calibration by up to 22 index units and bypassed
    the regulatory limit the by-rate path applies, which is not
    something a driver should do.
  - Dropped "match the RTL8723BS firmware connect and power save
    behaviour". Deferring the connect report changes nothing, and the
    LPS gating cannot be justified from measurement: with RTW_DBG_PS
    enabled, LPS never engages on this setup at all, so the gating never
    takes effect here.
  - Dropped "advertise the correct receive capabilities". Both halves
    move into the chip driver, as described above.
  - "fw: add the vendor firmware commands" now adds only GNT_BT. rtw88
    already implements MACID_CFG and the WL channel info report, and the
    coexistence antenna select reserve became unused once the PHY patch
    went.
  - The zero-length packet check is gated on SDIO rather than the chip
    id.
  - The SDIO interrupt acknowledgment change is scoped to this chip.
  - The association sequence no longer re-applies the BSS capability at
    association time; BSS_CHANGED_ERP_PREAMBLE and BSS_CHANGED_ERP_SLOT
    are handled later in the same callback.
  - Register accesses that had raw addresses now use the existing
    REG_GNT_BT and REG_BT_COEX_ENH_INTR_CTRL, plus a named define for
    the BB antenna select register.

The implementation is based on the initial RTL8723B work by Michael
Straube <straube.linux@gmail.com>:

  https://github.com/mistraube/rtw88/tree/rtl8723bs

Based on the rtw-next branch from pkshih/rtw.

Luka Gejak (5):
  wifi: rtw88: add the RTL8723B chip type and SDIO helper
  wifi: rtw88: rx: mark zero length packets on RTL8723BS
  wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
  wifi: rtw88: sdio: set up RX aggregation and interrupts for RTL8723BS
  wifi: rtw88: sdio: add TX back-pressure and retry on page starvation

 drivers/net/wireless/realtek/rtw88/main.h |   7 +
 drivers/net/wireless/realtek/rtw88/rx.c   |   8 +-
 drivers/net/wireless/realtek/rtw88/sdio.c | 373 ++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/sdio.h |  22 +-
 4 files changed, 383 insertions(+), 27 deletions(-)

-- 
2.53.0

[PATCH v3 1/5] wifi: rtw88: add the RTL8723B chip type and SDIO helper

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

The RTL8723B is a 802.11n combo chip whose SDIO variant, RTL8723BS, runs
a Realtek vendor firmware that differs from the firmware used by the
other rtw88 8723 family devices. Supporting it needs a number of small
adjustments spread across the shared core, all of which have to be
restricted to this one chip and bus combination.

Add the chip type and a helper that tests for it, so the changes that
follow can be gated without repeating the chip and bus comparison.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/main.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/main.h b/drivers/net/wireless/realtek/rtw88/main.h
index c6e981ba7986..8f86f7c12de5 100644
--- a/drivers/net/wireless/realtek/rtw88/main.h
+++ b/drivers/net/wireless/realtek/rtw88/main.h
@@ -194,6 +194,7 @@ enum rtw_chip_type {
 	RTW_CHIP_TYPE_8723D,
 	RTW_CHIP_TYPE_8821C,
 	RTW_CHIP_TYPE_8703B,
+	RTW_CHIP_TYPE_8723B,
 	RTW_CHIP_TYPE_8821A,
 	RTW_CHIP_TYPE_8812A,
 	RTW_CHIP_TYPE_8814A,
@@ -2194,6 +2195,12 @@ static inline bool rtw_chip_has_tx_stbc(struct rtw_dev *rtwdev)
 	return rtwdev->chip->tx_stbc;
 }
 
+static inline bool rtw_is_8723bs(struct rtw_dev *rtwdev)
+{
+	return rtwdev->chip->id == RTW_CHIP_TYPE_8723B &&
+	       rtwdev->hci.type == RTW_HCI_TYPE_SDIO;
+}
+
 static inline u8 rtw_acquire_macid(struct rtw_dev *rtwdev)
 {
 	unsigned long mac_id;
-- 
2.53.0

[PATCH v3 2/5] wifi: rtw88: rx: mark zero length packets on RTL8723BS

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

Like the RTL8703B, the RTL8723BS reports receive descriptors with a zero
packet length, which the vendor driver drops outright. rtw88 already
flags these as having no PSDU for the RTL8703B, so extend the same
handling rather than passing an empty frame up.

This is gated on the SDIO interface rather than the chip id, because it
has only been observed there; the USB variant is not known to do it.

Co-developed-by: Michael Straube <straube.linux@gmail.com>
Signed-off-by: Michael Straube <straube.linux@gmail.com>
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/rx.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/rx.c b/drivers/net/wireless/realtek/rtw88/rx.c
index 01fd299abb7f..a9361ef4b93c 100644
--- a/drivers/net/wireless/realtek/rtw88/rx.c
+++ b/drivers/net/wireless/realtek/rtw88/rx.c
@@ -253,10 +253,12 @@ static void rtw_rx_fill_rx_status(struct rtw_dev *rtwdev,
 
 	rtw_rx_addr_match(rtwdev, pkt_stat, hdr);
 
-	/* Rtl8723cs driver checks for size < 14 or size > 8192 and
-	 * simply drops the packet.
+	/*
+	 * Rtl8723cs and rtl8723bs drivers check for size < 14 or size > 8192
+	 * and simply drop the packet.
 	 */
-	if (rtwdev->chip->id == RTW_CHIP_TYPE_8703B && pkt_stat->pkt_len == 0) {
+	if ((rtwdev->chip->id == RTW_CHIP_TYPE_8703B || rtw_is_8723bs(rtwdev)) &&
+	    pkt_stat->pkt_len == 0) {
 		rx_status->flag |= RX_FLAG_NO_PSDU;
 		rtw_dbg(rtwdev, RTW_DBG_RX, "zero length packet");
 	}
-- 
2.53.0

[PATCH v3 3/5] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

The RTL8723BS reports free TX page counts that the generic 8051 path
reads back from the chip on every transfer, which is both slow over SDIO
and unreliable on this part: the register frequently reads back zero
while pages are in fact available. It also gates transmission on a free
count in the SDIO output queue, REG_SDIO_OQT_FREE_PG, which rtw88 does
not track at all. That count is how many more descriptors the SDIO
engine can accept, and the chip discards writes that arrive with none
left.

Mirror the vendor driver and keep the per-queue and public page counts
in software, seeded at start and resynchronised from the chip only when
the cached counts say there is not enough room. Wait for a free output
queue entry before writing, and account for the pages consumed after a
successful transfer.

Transfers also have to be padded up to the SDIO block size for this
chip rather than using the generic alignment, so size the write
separately from the frame and trim the skb back afterwards.

Measured on RTL8723BS hardware against an iperf3 server one hop behind
the AP, with the wlan0 byte counters as ground truth. On the generic
path the association completes but no data passes at all: TCP and UDP
both measure 0 bit/s in either direction. With this patch TCP is
25.3 Mbit/s up and 37.3 Mbit/s down, and UDP is 25.0 Mbit/s up at 0%
loss.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 245 ++++++++++++++++++++--
 drivers/net/wireless/realtek/rtw88/sdio.h |  10 +
 2 files changed, 243 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..493eda559607 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -20,6 +20,7 @@
 #include "tx.h"
 
 #define RTW_SDIO_INDIRECT_RW_RETRIES			50
+#define RTW_SDIO_OQT_TIMEOUT_MS				1000
 
 static bool rtw_sdio_is_bus_addr(u32 addr)
 {
@@ -548,12 +549,122 @@ static int rtw_sdio_read_port(struct rtw_dev *rtwdev, u8 *buf, size_t count)
 	return ret;
 }
 
+/*
+ * The cached free page counters are a fast path hint only. They are written
+ * from the single threaded TX work and, for H2C and reserved page writes,
+ * from process context, so they are atomic_t; whenever they claim there is
+ * not enough room they are resynchronised from the chip before the caller
+ * gives up, which also absorbs a lost update.
+ */
+static void rtw_sdio_8723bs_store_free_txpg(struct rtw_dev *rtwdev,
+					    u32 free_txpg)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	atomic_set(&rtwsdio->free_pg_high,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_HIGH));
+	atomic_set(&rtwsdio->free_pg_normal,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_NORMAL));
+	atomic_set(&rtwsdio->free_pg_low,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_LOW));
+	atomic_set(&rtwsdio->free_pg_pub,
+		   u32_get_bits(free_txpg, BIT_FREE_TXPG_PUB));
+}
+
+static void rtw_sdio_8723bs_init_free_txpg(struct rtw_dev *rtwdev)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	const struct rtw_page_table *pg_tbl;
+	u32 free_txpg;
+	u16 pubq_num;
+
+	free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+	if (free_txpg) {
+		rtw_sdio_8723bs_store_free_txpg(rtwdev, free_txpg);
+	} else {
+		pg_tbl = &rtwdev->chip->page_table[0];
+		pubq_num = rtwdev->fifo.acq_pg_num - pg_tbl->hq_num -
+			   pg_tbl->lq_num - pg_tbl->nq_num - pg_tbl->exq_num -
+			   pg_tbl->gapq_num;
+
+		atomic_set(&rtwsdio->free_pg_high, pg_tbl->hq_num);
+		atomic_set(&rtwsdio->free_pg_normal, pg_tbl->nq_num);
+		atomic_set(&rtwsdio->free_pg_low, pg_tbl->lq_num);
+		atomic_set(&rtwsdio->free_pg_pub, pubq_num);
+	}
+
+	atomic_set(&rtwsdio->tx_oqt_free,
+		   rtw_read8(rtwdev, REG_SDIO_OQT_FREE_PG));
+}
+
+static void rtw_sdio_8723bs_sync_free_txpg(struct rtw_dev *rtwdev)
+{
+	u32 free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+
+	if (free_txpg)
+		rtw_sdio_8723bs_store_free_txpg(rtwdev, free_txpg);
+}
+
+/*
+ * Sum of the queue's dedicated counter and the public pool, clamped at zero:
+ * a lost update between the check below and rtw_sdio_8723bs_consume_txpg()
+ * can briefly drive a counter negative, and letting that wrap would hide the
+ * shortage instead of triggering a resync from the chip.
+ */
+static unsigned int rtw_sdio_8723bs_pages_free(struct rtw_dev *rtwdev,
+					       atomic_t *dedicated)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	int free;
+
+	free = atomic_read(dedicated) + atomic_read(&rtwsdio->free_pg_pub);
+
+	return free > 0 ? free : 0;
+}
+
+static atomic_t *rtw_sdio_8723bs_free_txpg(struct rtw_dev *rtwdev, u8 queue)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+	switch (queue) {
+	case RTW_TX_QUEUE_VI:
+		return &rtwsdio->free_pg_normal;
+	case RTW_TX_QUEUE_BE:
+	case RTW_TX_QUEUE_BK:
+		return &rtwsdio->free_pg_low;
+	case RTW_TX_QUEUE_BCN:
+	case RTW_TX_QUEUE_H2C:
+	case RTW_TX_QUEUE_HI0:
+	case RTW_TX_QUEUE_MGMT:
+	case RTW_TX_QUEUE_VO:
+		return &rtwsdio->free_pg_high;
+	default:
+		return NULL;
+	}
+}
+
 static int rtw_sdio_check_free_txpg(struct rtw_dev *rtwdev, u8 queue,
 				    size_t count)
 {
 	unsigned int pages_free, pages_needed;
 
-	if (rtw_chip_wcpu_8051(rtwdev)) {
+	if (rtw_is_8723bs(rtwdev)) {
+		atomic_t *dedicated;
+
+		dedicated = rtw_sdio_8723bs_free_txpg(rtwdev, queue);
+		if (!dedicated) {
+			rtw_warn(rtwdev, "Unknown mapping for queue %u\n", queue);
+			return -EINVAL;
+		}
+
+		pages_free = rtw_sdio_8723bs_pages_free(rtwdev, dedicated);
+		pages_needed = DIV_ROUND_UP(count, rtwdev->chip->page_size);
+		if (pages_needed <= pages_free)
+			return 0;
+
+		rtw_sdio_8723bs_sync_free_txpg(rtwdev);
+		pages_free = rtw_sdio_8723bs_pages_free(rtwdev, dedicated);
+	} else if (rtw_chip_wcpu_8051(rtwdev)) {
 		u32 free_txpg;
 
 		free_txpg = rtw_sdio_read32(rtwdev, REG_SDIO_FREE_TXPG);
@@ -632,44 +743,123 @@ static int rtw_sdio_check_free_txpg(struct rtw_dev *rtwdev, u8 queue,
 	return 0;
 }
 
+static int rtw_sdio_8723bs_wait_tx_oqt(struct rtw_dev *rtwdev)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	u8 free;
+	int i;
+
+	if (atomic_add_unless(&rtwsdio->tx_oqt_free, -1, 0))
+		return 0;
+
+	for (i = 0; i < RTW_SDIO_OQT_TIMEOUT_MS; i++) {
+		free = rtw_read8(rtwdev, REG_SDIO_OQT_FREE_PG);
+		if (free) {
+			atomic_set(&rtwsdio->tx_oqt_free, free - 1);
+			return 0;
+		}
+		usleep_range(1000, 2000);
+	}
+
+	return -EBUSY;
+}
+
+static void rtw_sdio_8723bs_consume_txpg(struct rtw_dev *rtwdev, u8 queue,
+					 unsigned int pages)
+{
+	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	atomic_t *dedicated;
+	unsigned int taken;
+	int free;
+
+	dedicated = rtw_sdio_8723bs_free_txpg(rtwdev, queue);
+	if (!dedicated)
+		return;
+
+	free = atomic_read(dedicated);
+	taken = min_t(unsigned int, pages, free > 0 ? free : 0);
+	atomic_sub(taken, dedicated);
+
+	pages -= taken;
+	if (pages && atomic_sub_return(pages, &rtwsdio->free_pg_pub) < 0)
+		atomic_set(&rtwsdio->free_pg_pub, 0);
+}
+
 static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
 			       enum rtw_tx_queue_type queue)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	bool rtl8723bs = rtw_is_8723bs(rtwdev);
+	unsigned int orig_len = skb->len;
+	unsigned int pages;
+	size_t write_size;
 	bool bus_claim;
 	size_t txsize;
 	u32 txaddr;
 	int ret;
 
-	txaddr = rtw_sdio_get_tx_addr(rtwdev, skb->len, queue);
-	if (!txaddr)
-		return -EINVAL;
+	if (rtl8723bs) {
+		txsize = round_up(orig_len, 4);
+		write_size = txsize > RTW_SDIO_BLOCK_SIZE ?
+			     round_up(txsize, RTW_SDIO_BLOCK_SIZE) : txsize;
+	} else {
+		txsize = sdio_align_size(rtwsdio->sdio_func, orig_len);
+		write_size = txsize;
+	}
 
-	txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+	if (write_size > orig_len) {
+		unsigned int padding = write_size - orig_len;
+
+		if (skb_tailroom(skb) < padding) {
+			ret = pskb_expand_head(skb, 0,
+					       padding - skb_tailroom(skb),
+					       GFP_KERNEL);
+			if (ret)
+				return ret;
+		}
+		skb_put_zero(skb, padding);
+	}
+
+	txaddr = rtw_sdio_get_tx_addr(rtwdev, txsize, queue);
+	if (!txaddr) {
+		ret = -EINVAL;
+		goto out_trim;
+	}
 
 	ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
 	if (ret)
-		return ret;
+		goto out_trim;
+
+	if (rtl8723bs) {
+		ret = rtw_sdio_8723bs_wait_tx_oqt(rtwdev);
+		if (ret)
+			goto out_trim;
+	}
 
 	if (!IS_ALIGNED((unsigned long)skb->data, RTW_SDIO_DATA_PTR_ALIGN))
 		rtw_warn(rtwdev, "Got unaligned SKB in %s() for queue %u\n",
 			 __func__, queue);
 
 	bus_claim = rtw_sdio_bus_claim_needed(rtwsdio);
-
 	if (bus_claim)
 		sdio_claim_host(rtwsdio->sdio_func);
-
-	ret = sdio_memcpy_toio(rtwsdio->sdio_func, txaddr, skb->data, txsize);
-
+	ret = sdio_memcpy_toio(rtwsdio->sdio_func, txaddr, skb->data,
+			       write_size);
 	if (bus_claim)
 		sdio_release_host(rtwsdio->sdio_func);
 
-	if (ret)
+	if (ret) {
 		rtw_warn(rtwdev,
 			 "Failed to write %zu byte(s) to SDIO port 0x%08x",
-			 txsize, txaddr);
+			 write_size, txaddr);
+	} else if (rtl8723bs) {
+		pages = DIV_ROUND_UP(txsize, rtwdev->chip->page_size);
+		rtw_sdio_8723bs_consume_txpg(rtwdev, queue, pages);
+	}
 
+out_trim:
+	if (write_size > orig_len)
+		skb_trim(skb, orig_len);
 	return ret;
 }
 
@@ -749,8 +939,39 @@ static int rtw_sdio_setup(struct rtw_dev *rtwdev)
 	return 0;
 }
 
+static void rtw_sdio_8723bs_check_rqpn(struct rtw_dev *rtwdev)
+{
+	const struct rtw_chip_info *chip = rtwdev->chip;
+	struct rtw_fifo_conf *fifo = &rtwdev->fifo;
+	const struct rtw_page_table *pg_tbl;
+	u16 reserved_num;
+	u32 free_txpg;
+	u16 pubq_num;
+
+	free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+	if (free_txpg || !fifo->acq_pg_num)
+		return;
+
+	pg_tbl = &chip->page_table[0];
+	reserved_num = pg_tbl->hq_num + pg_tbl->lq_num + pg_tbl->nq_num +
+		       pg_tbl->exq_num + pg_tbl->gapq_num;
+	if (fifo->acq_pg_num <= reserved_num)
+		return;
+
+	pubq_num = fifo->acq_pg_num - reserved_num;
+	rtw_write32(rtwdev, REG_RQPN_NPQ,
+		    BIT_RQPN_NE(pg_tbl->nq_num, pg_tbl->exq_num));
+	rtw_write32(rtwdev, REG_RQPN,
+		    BIT_RQPN_HLP(pg_tbl->hq_num, pg_tbl->lq_num, pubq_num));
+}
+
 static int rtw_sdio_start(struct rtw_dev *rtwdev)
 {
+	if (rtw_is_8723bs(rtwdev)) {
+		rtw_sdio_8723bs_check_rqpn(rtwdev);
+		rtw_sdio_8723bs_init_free_txpg(rtwdev);
+	}
+
 	rtw_sdio_enable_rx_aggregation(rtwdev);
 	rtw_sdio_enable_interrupt(rtwdev);
 
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 457e8b02380e..5b000cb8fac2 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -86,6 +86,10 @@
 #define REG_SDIO_OQT_FREE_PG			(SDIO_LOCAL_OFFSET + 0x001E)
 /* Free Tx Buffer Page */
 #define REG_SDIO_FREE_TXPG			(SDIO_LOCAL_OFFSET + 0x0020)
+#define BIT_FREE_TXPG_HIGH			GENMASK(7, 0)
+#define BIT_FREE_TXPG_NORMAL			GENMASK(15, 8)
+#define BIT_FREE_TXPG_LOW			GENMASK(23, 16)
+#define BIT_FREE_TXPG_PUB			GENMASK(31, 24)
 /* HCI Current Power Mode 1 */
 #define REG_SDIO_HCPWM1				(SDIO_LOCAL_OFFSET + 0x0024)
 /* HCI Current Power Mode 2 */
@@ -159,6 +163,12 @@ struct rtw_sdio {
 	struct workqueue_struct *txwq;
 	struct rtw_sdio_work_data *tx_handler_data;
 	struct sk_buff_head tx_queue[RTK_MAX_TX_QUEUE_NUM];
+
+	atomic_t free_pg_high;
+	atomic_t free_pg_normal;
+	atomic_t free_pg_low;
+	atomic_t free_pg_pub;
+	atomic_t tx_oqt_free;
 };
 
 extern const struct dev_pm_ops rtw_sdio_pm_ops;
-- 
2.53.0

[PATCH v3 4/5] wifi: rtw88: sdio: set up RX aggregation and interrupts for RTL8723BS

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

Enable the existing RX aggregation setup for this chip and select the
larger DMA burst count it needs. The RTL8723BS does not raise CPWM1, so
leave that source out of its interrupt mask, and set the SDIO TX control
bit the vendor driver uses to have transfers always recognised.

The chip also keeps raising the interrupt after resume if undefined
status bits are written back when acknowledging, so acknowledge only the
defined and unmasked bits. That is scoped to this chip; the other SDIO
parts keep writing the status word back unchanged.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 29 ++++++++++++++++++++++-
 drivers/net/wireless/realtek/rtw88/sdio.h |  9 +++++++
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 493eda559607..e9f0d23113f3 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -867,7 +867,11 @@ static void rtw_sdio_init(struct rtw_dev *rtwdev)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
-	rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST | REG_SDIO_HIMR_CPWM1;
+	if (rtw_is_8723bs(rtwdev))
+		rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST;
+	else
+		rtwsdio->irq_mask = REG_SDIO_HIMR_RX_REQUEST |
+				    REG_SDIO_HIMR_CPWM1;
 }
 
 static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
@@ -875,6 +879,7 @@ static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
 	u8 size, timeout;
 
 	switch (rtwdev->chip->id) {
+	case RTW_CHIP_TYPE_8723B:
 	case RTW_CHIP_TYPE_8703B:
 	case RTW_CHIP_TYPE_8821A:
 	case RTW_CHIP_TYPE_8812A:
@@ -902,6 +907,8 @@ static void rtw_sdio_enable_rx_aggregation(struct rtw_dev *rtwdev)
 		    FIELD_PREP(BIT_DMA_AGG_TO_V1, timeout));
 
 	rtw_write8_set(rtwdev, REG_RXDMA_MODE, BIT_DMA_MODE);
+	if (rtw_is_8723bs(rtwdev))
+		rtw_write8_mask(rtwdev, REG_RXDMA_MODE, BIT_DMA_BURST_CNT, 0x3);
 }
 
 static void rtw_sdio_enable_interrupt(struct rtw_dev *rtwdev)
@@ -967,12 +974,22 @@ static void rtw_sdio_8723bs_check_rqpn(struct rtw_dev *rtwdev)
 
 static int rtw_sdio_start(struct rtw_dev *rtwdev)
 {
+	u32 clear;
+
 	if (rtw_is_8723bs(rtwdev)) {
 		rtw_sdio_8723bs_check_rqpn(rtwdev);
 		rtw_sdio_8723bs_init_free_txpg(rtwdev);
 	}
 
 	rtw_sdio_enable_rx_aggregation(rtwdev);
+
+	if (rtw_is_8723bs(rtwdev)) {
+		clear = rtw_read32(rtwdev, REG_SDIO_HISR) &
+			RTW_SDIO_HISR_CLEAR_MASK;
+		if (clear)
+			rtw_write32(rtwdev, REG_SDIO_HISR, clear);
+	}
+
 	rtw_sdio_enable_interrupt(rtwdev);
 
 	return 0;
@@ -1052,6 +1069,8 @@ static void rtw_sdio_interface_cfg(struct rtw_dev *rtwdev)
 
 	val = rtw_read32(rtwdev, REG_SDIO_TX_CTRL);
 	val &= 0xfff8;
+	if (rtw_is_8723bs(rtwdev))
+		val |= BIT_SDIO_TX_CTRL_ALWAYS_RECOGNIZE;
 	rtw_write32(rtwdev, REG_SDIO_TX_CTRL, val);
 }
 
@@ -1316,6 +1335,14 @@ static void rtw_sdio_handle_interrupt(struct sdio_func *sdio_func)
 		rtw_sdio_rx_isr(rtwdev);
 	}
 
+	/*
+	 * RTL8723BS keeps raising the interrupt after resume if undefined
+	 * status bits are written back, so acknowledge only the bits that are
+	 * both defined and unmasked. Other chips keep the existing behaviour.
+	 */
+	if (rtw_is_8723bs(rtwdev))
+		hisr &= rtwsdio->irq_mask & RTW_SDIO_HISR_CLEAR_MASK;
+
 	rtw_write32(rtwdev, REG_SDIO_HISR, hisr);
 
 	rtwsdio->irq_thread = NULL;
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 5b000cb8fac2..7474a7511811 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -22,6 +22,7 @@
 
 /* SDIO Tx Control */
 #define REG_SDIO_TX_CTRL			(SDIO_LOCAL_OFFSET + 0x0000)
+#define BIT_SDIO_TX_CTRL_ALWAYS_RECOGNIZE	BIT(4)
 
 /*SDIO status timeout*/
 #define REG_SDIO_TIMEOUT			(SDIO_LOCAL_OFFSET + 0x0002)
@@ -77,6 +78,14 @@
 /* the following two are RTL8188 SDIO Specific */
 #define REG_SDIO_HISR_MCU_ERR			BIT(28)
 #define REG_SDIO_HISR_TSF_BIT32_TOGGLE		BIT(29)
+#define RTW_SDIO_HISR_CLEAR_MASK		\
+	(REG_SDIO_HISR_TXERR | REG_SDIO_HISR_RXERR | \
+	 REG_SDIO_HISR_TXFOVW | REG_SDIO_HISR_RXFOVW | \
+	 REG_SDIO_HISR_TXBCNOK | REG_SDIO_HISR_TXBCNERR | \
+	 REG_SDIO_HISR_C2HCMD | REG_SDIO_HISR_CPWM1 | \
+	 REG_SDIO_HISR_CPWM2 | REG_SDIO_HISR_HSISR_IND | \
+	 REG_SDIO_HISR_GTINT3_IND | REG_SDIO_HISR_GTINT4_IND | \
+	 REG_SDIO_HISR_PSTIMEOUT | REG_SDIO_HISR_OCPINT)
 
 /* HCI Current Power Mode */
 #define REG_SDIO_HCPWM				(SDIO_LOCAL_OFFSET + 0x0019)
-- 
2.53.0

[PATCH v3 5/5] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation

[luka.gejak]

From: Luka Gejak <luka.gejak@linux.dev>

Two problems show up on RTL8723BS uplink. The per-AC software FIFO is
unbounded, so mac80211 keeps handing frames down until latency collapses
under load. And when the chip runs out of free TX pages the queue is
simply abandoned for that pass, which stalls the AC until something else
kicks the worker.

Stop the mac80211 queue once a data AC fills past a high watermark and
wake it from the drain path when it falls back to a low one. Convert the
TX work item to a delayed work so a temporary page shortage can be
retried shortly afterwards instead of stalling, and cancel it on
teardown.

Measured on RTL8723BS hardware, uplink goes from 11.9 Mbit/s with 204
TCP retransmits to 20.1 Mbit/s with 2.

Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
 drivers/net/wireless/realtek/rtw88/sdio.c | 99 ++++++++++++++++++++---
 drivers/net/wireless/realtek/rtw88/sdio.h |  3 +-
 2 files changed, 91 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index e9f0d23113f3..ed1bed8ea144 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -22,6 +22,16 @@
 #define RTW_SDIO_INDIRECT_RW_RETRIES			50
 #define RTW_SDIO_OQT_TIMEOUT_MS				1000
 
+/*
+ * 8723BS SDIO TX FIFO back-pressure watermarks: stop the mac80211 queue once
+ * the per-AC software FIFO fills past the high watermark, and wake it from the
+ * TX drain path once it falls back to the low one. Bounds the queueing latency
+ * that otherwise causes uplink bufferbloat / congestion collapse.
+ */
+#define RTW_SDIO_TX_FIFO_HIWATER			16
+#define RTW_SDIO_TX_FIFO_LOWATER			8
+#define RTW_SDIO_TX_RETRY_DELAY			msecs_to_jiffies(1)
+
 static bool rtw_sdio_is_bus_addr(u32 addr)
 {
 	return !!(addr & RTW_SDIO_BUS_MSK);
@@ -1053,7 +1063,11 @@ static void rtw_sdio_tx_kick_off(struct rtw_dev *rtwdev)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 
-	queue_work(rtwsdio->txwq, &rtwsdio->tx_handler_data->work);
+	/*
+	 * A retry may already be pending with a delay; re-arm it so a newly
+	 * queued frame is not held back by it.
+	 */
+	mod_delayed_work(rtwsdio->txwq, &rtwsdio->tx_handler_data->work, 0);
 }
 
 static void rtw_sdio_link_ps(struct rtw_dev *rtwdev, bool enter)
@@ -1177,6 +1191,19 @@ static int rtw_sdio_tx_write(struct rtw_dev *rtwdev,
 
 	skb_queue_tail(&rtwsdio->tx_queue[queue], skb);
 
+	/*
+	 * Back-pressure on the data ACs (BK/BE/VI/VO): once the FIFO fills past
+	 * the high watermark, stop the corresponding mac80211 queue so it stops
+	 * handing us frames, bounding the queueing latency. Resumed from the TX
+	 * drain path once the FIFO drains below the low watermark.
+	 */
+	if (rtw_is_8723bs(rtwdev) && queue < RTW_TX_QUEUE_BCN &&
+	    !rtwsdio->queue_stopped[queue] &&
+	    skb_queue_len(&rtwsdio->tx_queue[queue]) >= RTW_SDIO_TX_FIFO_HIWATER) {
+		rtwsdio->queue_stopped[queue] = true;
+		ieee80211_stop_queue(rtwdev->hw, skb_get_queue_mapping(skb));
+	}
+
 	return 0;
 }
 
@@ -1479,43 +1506,92 @@ static void rtw_sdio_indicate_tx_status(struct rtw_dev *rtwdev,
 	ieee80211_tx_status_irqsafe(hw, skb);
 }
 
-static void rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev,
-				      enum rtw_tx_queue_type queue)
+/*
+ * Send one frame from @queue. Returns 1 when a frame was written, 0 when the
+ * queue was empty and a negative errno when the write failed, in which case
+ * the frame is put back at the head of the queue.
+ */
+static int rtw_sdio_process_tx_queue(struct rtw_dev *rtwdev,
+				     enum rtw_tx_queue_type queue)
 {
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 	struct sk_buff *skb;
+	u16 q_map;
 	int ret;
 
 	skb = skb_dequeue(&rtwsdio->tx_queue[queue]);
 	if (!skb)
-		return;
+		return 0;
 
+	q_map = skb_get_queue_mapping(skb);
 	ret = rtw_sdio_write_port(rtwdev, skb, queue);
 	if (ret) {
 		skb_queue_head(&rtwsdio->tx_queue[queue], skb);
-		return;
+		return ret;
 	}
 
 	rtw_sdio_indicate_tx_status(rtwdev, skb);
+
+	if (rtw_is_8723bs(rtwdev) && queue < RTW_TX_QUEUE_BCN &&
+	    rtwsdio->queue_stopped[queue] &&
+	    skb_queue_len(&rtwsdio->tx_queue[queue]) <=
+	    RTW_SDIO_TX_FIFO_LOWATER) {
+		rtwsdio->queue_stopped[queue] = false;
+		ieee80211_wake_queue(rtwdev->hw, q_map);
+	}
+
+	return 1;
 }
 
 static void rtw_sdio_tx_handler(struct work_struct *work)
 {
 	struct rtw_sdio_work_data *work_data =
-		container_of(work, struct rtw_sdio_work_data, work);
+		container_of(to_delayed_work(work), struct rtw_sdio_work_data,
+			     work);
 	struct rtw_sdio *rtwsdio;
 	struct rtw_dev *rtwdev;
-	int limit, queue;
+	int limit, queue, ret;
+	bool rtl8723bs;
 
 	rtwdev = work_data->rtwdev;
 	rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+	rtl8723bs = rtw_is_8723bs(rtwdev);
 
 	if (!rtw_fw_feature_check(&rtwdev->fw, FW_FEATURE_TX_WAKE))
 		rtw_sdio_deep_ps_leave(rtwdev);
 
 	for (queue = RTK_MAX_TX_QUEUE_NUM - 1; queue >= 0; queue--) {
 		for (limit = 0; limit < 1000; limit++) {
-			rtw_sdio_process_tx_queue(rtwdev, queue);
+			ret = rtw_sdio_process_tx_queue(rtwdev, queue);
+			if (ret == 0)
+				break;
+
+			if (ret < 0) {
+				/*
+				 * A page shortage is transient: leave the
+				 * frame queued and come back shortly instead
+				 * of stalling this AC until something else
+				 * kicks the worker.
+				 */
+				if (rtl8723bs && ret == -EBUSY) {
+					queue_delayed_work(rtwsdio->txwq,
+							   &work_data->work,
+							   RTW_SDIO_TX_RETRY_DELAY);
+					return;
+				}
+				break;
+			}
+
+			/*
+			 * Restart from the highest priority queue after every
+			 * management frame so the join sequence is not held up
+			 * behind a data backlog.
+			 */
+			if (rtl8723bs && queue == RTW_TX_QUEUE_MGMT) {
+				queue_delayed_work(rtwsdio->txwq,
+						   &work_data->work, 0);
+				return;
+			}
 
 			if (skb_queue_empty(&rtwsdio->tx_queue[queue]))
 				break;
@@ -1542,14 +1618,16 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
 		return -ENOMEM;
 	}
 
-	for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
+	for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++) {
 		skb_queue_head_init(&rtwsdio->tx_queue[i]);
+		rtwsdio->queue_stopped[i] = false;
+	}
 	rtwsdio->tx_handler_data = kmalloc_obj(*rtwsdio->tx_handler_data);
 	if (!rtwsdio->tx_handler_data)
 		goto err_destroy_wq;
 
 	rtwsdio->tx_handler_data->rtwdev = rtwdev;
-	INIT_WORK(&rtwsdio->tx_handler_data->work, rtw_sdio_tx_handler);
+	INIT_DELAYED_WORK(&rtwsdio->tx_handler_data->work, rtw_sdio_tx_handler);
 
 	return 0;
 
@@ -1563,6 +1641,7 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
 	struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
 	int i;
 
+	cancel_delayed_work_sync(&rtwsdio->tx_handler_data->work);
 	destroy_workqueue(rtwsdio->txwq);
 	kfree(rtwsdio->tx_handler_data);
 
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 7474a7511811..2f1b67f4bbc7 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.h
+++ b/drivers/net/wireless/realtek/rtw88/sdio.h
@@ -156,7 +156,7 @@ struct rtw_sdio_tx_data {
 };
 
 struct rtw_sdio_work_data {
-	struct work_struct work;
+	struct delayed_work work;
 	struct rtw_dev *rtwdev;
 };
 
@@ -172,6 +172,7 @@ struct rtw_sdio {
 	struct workqueue_struct *txwq;
 	struct rtw_sdio_work_data *tx_handler_data;
 	struct sk_buff_head tx_queue[RTK_MAX_TX_QUEUE_NUM];
+	bool queue_stopped[RTK_MAX_TX_QUEUE_NUM];
 
 	atomic_t free_pg_high;
 	atomic_t free_pg_normal;
-- 
2.53.0

Re: [PATCH v3 0/5] wifi: rtw88: preparations for RTL8723B/RTL8723BS

[Peter Robinson]

Hi Luka,

Is there a branch somewhere that has the remaining patches to be able
to test the full stack?

Might be useful to reference that in the cover letter in future
versions as well so people can find it if they wish to test the whole
driver end game.

Peter

On Tue, 4 Aug 2026 at 07:55, <luka.gejak@linux.dev> wrote:
>
> From: Luka Gejak <luka.gejak@linux.dev>
>
> This is the first of two series adding support for the Realtek RTL8723B
> 802.11n chipset and its RTL8723BS SDIO variant to rtw88. It contains
> only the changes to the shared rtw88 core that the chip driver depends
> on. The chip itself, the build glue and the MAINTAINERS entry are a
> second series.
>
> v1 had 19 patches, v2 had 11, this has 5.
>
> There is no rtw88-style firmware for this chip and no documentation for
> the vendor blob it does use, so while bringing it up I deliberately kept
> the driver as close to the vendor driver's behaviour as I could. With
> that many unknowns at once, matching the vendor exactly was the only way
> to tell which difference actually mattered when something did not work,
> rather than guessing. A number of the v1 patches came from that: they
> reproduced vendor behaviour that was useful to hold fixed during bring
> up, not behaviour the chip turns out to require.
>
> Bitterblue Smith's review of v1 prompted me to go back and remove each
> of those on hardware to find which were actually load bearing; eight
> were not. Ping-Ke Shih's review of v2 did the same for a further six,
> and the changelog below gives the measurement for each. In three of
> those cases I had told the list, or claimed in a commit message, that
> the chip needed something it does not, on the strength of bring-up
> notes I had not re-measured. All three corrections are in the
> changelog.
>
> What is left is there because removing it made something measurably
> worse, not because the vendor driver does it.
>
> What is left falls into two groups:
>
>   - SDIO (patches 3, 4, 5): software free-page and output-queue
>     accounting, RX aggregation and interrupt setup, and TX back-pressure
>     with a retry on page starvation;
>   - patches 1 and 2: the chip type and helper plus a one-line receive
>     fix.
>
> This series no longer touches fw.c, coex.c or mac80211.c at all.
>
> Scope
> =====
>
> Everything is gated on rtw_is_8723bs(), which is false for every chip
> currently supported, so behaviour for existing devices is unchanged.
> There is one exception, deliberate: patch 2 extends an existing RTL8703B
> zero-length-packet check and is gated on the SDIO interface rather than
> the chip id, since that is the only place the behaviour has been
> observed. Ping-Ke has suggested that check may not need a chip test at
> all; I would rather measure that on RTL8703B hardware than assume it,
> and I do not have that part, so it is unchanged here.
>
> Patch 4 changes how the SDIO interrupt status is acknowledged. In v1
> that applied to all SDIO parts; it is scoped to this chip only, and the
> others keep writing the status word back unchanged.
>
> Chip specific code in the common path
> =====================================
>
> Ping-Ke asked whether this needs a set of ops rather than a chip test
> sprinkled through the core. Working through that question is what
> removed most of this series: once the association and coexistence
> patches turned out not to be needed, the chip tests they carried went
> with them.
>
> What remains is one test in rx.c and eleven in sdio.c. There are none
> in fw.c, mac80211.c or coex.c. The SDIO ones are in the bus driver
> rather than the chip driver, and rtw_sdio has no per-chip ops table
> today; Ping-Ke confirmed an inline chip id check there is fine.
>
> Fixed in the second series
> ==========================
>
> Two of Bitterblue's review points are addressed in the chip driver
> rather than here, because that is the right place for both:
>
>   - the hardware capability is filled in from a chip-specific
>     read_efuse, instead of special casing rtw_dump_hw_feature() in the
>     core;
>   - the receive FCS handling is a chip configuration fix. v1 stopped the
>     core advertising RX_INCLUDES_FCS for this chip. The real cause was
>     that the chip cleared BIT_APP_FCS by assigning WLAN_RCR_CFG over
>     hal.rcr wholesale, where the 8723x siblings only write the register
>     and leave hal.rcr alone. Setting that bit in the chip's WLAN_RCR_CFG
>     makes the descriptor carry the FCS like every other rtw88 chip, so
>     the core needs no special case and both hunks are gone from this
>     series.
>
> Neither change is visible here. I planned to send that series once this
> one is applied to rtw-next, but I can send it now alongside if you would
> rather review them together, or fold the two into one series if that is
> easier.
>
> Testing
> =======
>
> Tested on RTL8723BS hardware together with the second series: repeated
> scan, authentication, association, WPA2-PSK/CCMP handshake, DHCP,
> bidirectional traffic, reconnects and module reloads.
>
> Throughput is measured against an iperf3 server one hop behind the AP,
> with the wlan0 byte counters as ground truth because the test machine's
> wired interface shares the same subnet. On one AP at strong signal
> (-35 dBm), 2.4 GHz HT40: TCP 19.1 Mbit/s up and 37.8 Mbit/s down, UDP
> 25.0 Mbit/s up and 40.0 Mbit/s down, both at 0% loss.
>
> Uplink on this band drifts by several Mbit/s from day to day, enough to
> swamp the differences I was trying to measure, so where the changelog
> below compares two builds the comparison is a paired test: the two
> builds alternate within one session, rebuilt and reinstalled each time,
> rather than being measured on different days.
>
> Suspend and resume are not covered: this machine only offers s2idle and
> does not reliably come back from it.
>
> A five minute soak: 1495/1495 pings at 0% loss, 15/15 reconnects, 5/5
> link up/down cycles, 3/3 module reloads, 5/5 scans under traffic, 24
> group rekeys, no deauthentication, no SDIO errors, no warnings in the
> log. Scanning saw the target AP in 18 of 18 attempts across 6 cold
> module reloads, with 6/6 associations.
>
> The soak and scan figures were taken on a build that also carried the
> reserved page patch dropped below; the throughput figures above are from
> the tree posted here.
>
> Every patch builds with W=1 with no warnings on its own, sparse is clean
> at the tip, and smatch reports nothing in any file this series touches.
> checkpatch --strict is clean across this series. The other rtw88 bus and
> chip modules continue to build.
>
> Changes in v3:
>   - Dropped "fw: fix the reserved page upload on RTL8723BS". Its commit
>     message said the BIT_BCN_VALID handshake fails on 8723BS SDIO
>     without it. That does not reproduce. The failure path is an explicit
>     rtw_err("error beacon valid") returning -EBUSY, reached from
>     BSS_CHANGED_ASSOC on every association; over three associations on
>     each of five builds with the patch reverted it never fired,
>     associations were 3/3, and there was no packet loss after 30 s idle
>     in power save, which is the path the reserved page's null data and
>     PS-poll frames serve. As a control, rtw_info messages appear in the
>     same captures and rtw_err outranks them, so a failure would have
>     been logged. AP mode and WoWLAN also download the reserved page and
>     are not tested here, so if it turns out to matter there it can come
>     back with evidence behind it.
>   - Kept "sdio: add TX back-pressure and retry on page starvation", now
>     with a measurement rather than an assertion, since you asked whether
>     the remaining chip tests are necessary. Counting
>     rtw_sdio_tx_handler invocations under an identical 60 Mbit/s UDP
>     flood, because the retry re-arms that same work item: 1798 and 3095
>     on two baseline runs against 69 with the patch reverted. Achievable
>     transmit rate falls by about a third without it, 25.5 Mbit/s against
>     38.3 and 40.2. tx_dropped and tx_errors stay at zero throughout, so
>     the failure is not dropping frames, it is failing to push them.
>   - Kept "sdio: set up RX aggregation and interrupts". The aggregation
>     half makes no measurable difference to throughput here, and on that
>     evidence alone I would have dropped it. The rest of the patch does
>     not show up in a throughput test at all: the chip keeps raising the
>     interrupt after resume if undefined status bits are written back
>     when acknowledging.
>
>     I have not been able to validate the suspend and resume path: the
>     test machine only offers s2idle and does not reliably resume from
>     it, so I cannot exercise the interrupt acknowledgement change. It is
>     kept because the failure it addresses was observed during bring up,
>     not because it has been re-measured. If someone with hardware that
>     suspends cleanly finds it unnecessary, it should go.
>   - Dropped "run the RTL8723BS association register sequence", 440
>     lines, which was the vendor join sequence, six sites in
>     rtw_ops_bss_info_changed() and both sites in rtw_ops_set_key().
>
>     Ping-Ke asked for this to run from rtw_chip_prepare_tx() rather than
>     a chip test in mgd_prepare_tx(), and I had done that with a new
>     chip_ops::prepare_tx. Since the whole patch is gone, so is the
>     callback; the core is left as it was rather than gaining an op with
>     no user.
>
>     I had told the list that association fails without these registers.
>     That was wrong, and it was bring-up-era data I repeated without
>     re-measuring. With the whole patch reverted: 18/18 scans saw the AP,
>     6/6 associations, and the five minute soak above, whose 15 reconnects
>     and 24 rekeys exercise the exact path the sequence ran on, since it
>     ran from mgd_prepare_tx() on every authentication. Uplink measured
>     paired against the full series, 19.0 against 19.2 Mbit/s over ten
>     runs each.
>
>     The default key search handling in that patch was also simply wrong:
>     rtw_sec_enable_sec_engine() sets sec->default_key_search itself and
>     programs all four USE_DK bits on that basis, so the enable path was
>     a no-op and the disable path was clearing bits the core had
>     deliberately set.
>   - Dropped "coex: add the RTL8723BS scan antenna workaround" and "coex:
>     reassert the antenna path when associating", and with them "fw: add
>     the GNT_BT firmware command", whose only caller they were.
>
>     The commit message on the first claimed the site survey does not
>     hear the AP reliably without it. That is also wrong: 18/18 scans
>     without it. Both were gated on the chip and on coex bt_disabled, so
>     that gate is their entire scope rather than a subset of it, and the
>     measurements above cover it.
>   - Dropped "record beacons from the target BSSID before
>     authenticating". Ping-Ke doubted the beacon wait affected the
>     connection and he was right: with the wait removed, association
>     succeeded 12/12 from a cold module reload and 12/12 from a warm
>     reconnect, against 10/10 and 10/10 with it, at the same latency.
>     The pre-auth deauth already sleeps 100 ms with the recording window
>     open, so on a 100 ms beacon interval the wait was almost always
>     already satisfied when it was reached. struct rtw_auth_sync and the
>     RX-path hook go with it.
>   - Dropped the BT_MP version queries and the BT_INFO query at scan
>     start, and the BT_INFO query plus two of three repeated PS-TDMA
>     H2Cs in the pre-auth replay. Measured over cold module reloads with
>     three scans each, 8 reloads with the queries and 10 without: 29/30
>     scans saw the target AP and 10/10 associations succeeded without
>     them, against 24/24 and 8/8 with them.
>   - The cached SDIO page counters are clamped at zero. A lost update
>     between the free page check and the accounting could previously
>     drive the public counter negative, and the sum was assigned to an
>     unsigned, which made the check pass unconditionally from then on and
>     suppressed the resync that was supposed to recover it.
>   - REG_SDIO_FREE_TXPG now has field masks and is decoded with
>     u32_get_bits(); the RX DMA burst count uses BIT_DMA_BURST_CNT;
>     open-coded BIT(0)|BIT(1) on REG_SYS_FUNC_EN uses the existing names.
>   - The RTL8723BS-specific SDIO helpers are named _8723bs_ and the chip
>     test moved to their callers.
>   - rtw_sdio_process_tx_queue() returns 1/0/-errno instead of taking a
>     "processed" out-parameter, which makes the retry and back-pressure
>     logic in the TX handler readable.
>   - mod_delayed_work() in rtw_sdio_tx_kick_off() carries a comment
>     saying why it is not queue_delayed_work(): a page-starvation retry
>     may already be armed with a delay, and the newly queued frame should
>     not wait for it.
>   - An argument that had a single value at every call site is gone,
>     along with its dead branch.
>   - Block comments use the general kernel style, per commit 82b8000c28b5
>     ("net: drop special comment style").
>
> Changes in v2:
>   - Dropped "tx: extend the TX report purge timeout". Instrumenting the
>     report path shows payload[6] & 0xfc equals the enqueued sequence
>     number, 32 out of 32 times, with rtw88's existing decode, and no
>     report is missed. The longer timeout was covering for the next
>     patch, not for the hardware.
>   - Dropped "fw: handle the RTL8723BS management TX reports". 0x12 and
>     0x32 are the first payload byte of C2H_CCX_TX_RPT, not C2H IDs;
>     every C2H event this chip sends carries a known id, so the existing
>     handler already covers them.
>   - Dropped "fw: send rate adaptation and RSSI info in the vendor
>     layout". The vendor byte layout is equivalent to what the existing
>     macros produce; uplink and the negotiated rate are unchanged without
>     it (16.4/16.5/17.0 against 15.4/16.7/16.6 Mbit/s, MCS7 both ways).
>   - Dropped "fw: send the media status report in the vendor layout". The
>     role field the vendor sets makes no difference here, as with the
>     other chips.
>   - Dropped "sdio: handle the RTL8723BS management TX path". Sequence
>     numbers do work for management frames on this chip, so reporting
>     completion at DMA completion is unnecessary.
>   - Dropped "calibrate and tune the PHY" entirely. All three parts were
>     vendor-matching scaffolding and none survived measurement: the
>     scan-time initial gain override is worse than
>     rtw_phy_dig_set_max_coverage() (five scans found 41 BSSes without it
>     against 34 with it); the IQ calibration works from the normal
>     phy_calibration path; and the hardcoded per-rate TX AGC table
>     overrode the efuse calibration by up to 22 index units and bypassed
>     the regulatory limit the by-rate path applies, which is not
>     something a driver should do.
>   - Dropped "match the RTL8723BS firmware connect and power save
>     behaviour". Deferring the connect report changes nothing, and the
>     LPS gating cannot be justified from measurement: with RTW_DBG_PS
>     enabled, LPS never engages on this setup at all, so the gating never
>     takes effect here.
>   - Dropped "advertise the correct receive capabilities". Both halves
>     move into the chip driver, as described above.
>   - "fw: add the vendor firmware commands" now adds only GNT_BT. rtw88
>     already implements MACID_CFG and the WL channel info report, and the
>     coexistence antenna select reserve became unused once the PHY patch
>     went.
>   - The zero-length packet check is gated on SDIO rather than the chip
>     id.
>   - The SDIO interrupt acknowledgment change is scoped to this chip.
>   - The association sequence no longer re-applies the BSS capability at
>     association time; BSS_CHANGED_ERP_PREAMBLE and BSS_CHANGED_ERP_SLOT
>     are handled later in the same callback.
>   - Register accesses that had raw addresses now use the existing
>     REG_GNT_BT and REG_BT_COEX_ENH_INTR_CTRL, plus a named define for
>     the BB antenna select register.
>
> The implementation is based on the initial RTL8723B work by Michael
> Straube <straube.linux@gmail.com>:
>
>   https://github.com/mistraube/rtw88/tree/rtl8723bs
>
> Based on the rtw-next branch from pkshih/rtw.
>
> Luka Gejak (5):
>   wifi: rtw88: add the RTL8723B chip type and SDIO helper
>   wifi: rtw88: rx: mark zero length packets on RTL8723BS
>   wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
>   wifi: rtw88: sdio: set up RX aggregation and interrupts for RTL8723BS
>   wifi: rtw88: sdio: add TX back-pressure and retry on page starvation
>
>  drivers/net/wireless/realtek/rtw88/main.h |   7 +
>  drivers/net/wireless/realtek/rtw88/rx.c   |   8 +-
>  drivers/net/wireless/realtek/rtw88/sdio.c | 373 ++++++++++++++++++++--
>  drivers/net/wireless/realtek/rtw88/sdio.h |  22 +-
>  4 files changed, 383 insertions(+), 27 deletions(-)
>
> --
> 2.53.0
>

Re: [PATCH v3 0/5] wifi: rtw88: preparations for RTL8723B/RTL8723BS

[Luka Gejak]

On August 4, 2026 9:18:59 AM GMT+02:00, Peter Robinson <pbrobinson@gmail.com> wrote:
>Hi Luka,
>
>Is there a branch somewhere that has the remaining patches to be able
>to test the full stack?
>
>Might be useful to reference that in the cover letter in future
>versions as well so people can find it if they wish to test the whole
>driver end game.
>
>Peter
>
>On Tue, 4 Aug 2026 at 07:55, <luka.gejak@linux.dev> wrote:
>>
>> From: Luka Gejak <luka.gejak@linux.dev>
>>
>> This is the first of two series adding support for the Realtek RTL8723B
>> 802.11n chipset and its RTL8723BS SDIO variant to rtw88. It contains
>> only the changes to the shared rtw88 core that the chip driver depends
>> on. The chip itself, the build glue and the MAINTAINERS entry are a
>> second series.

Hi Peter,

Indeed there is a branch on my github fork of rtw repo and I will 
include it in cover letter of v4, if such exists in the future:

https://github.com/MocLG/rtw/tree/8723bs-v3

Also one thing, please avoid top posting so the conversation flows in 
chronological order.

Best regards,
Luka Gejak