* [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS
@ 2026-08-25 16:33 luka.gejak
2026-08-25 16:33 ` [PATCH v8 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
` (6 more replies)
0 siblings, 7 replies; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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, v3 had 5, v4 had 7, v5 had 6, v6 had 6,
v7 had 6, and this has 6.
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 four 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 four 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 these groups:
- patches 1 and 2: the chip type and helper plus a one-line receive
fix;
- patch 3: a longer TX-report purge timeout, needed on slower SDIO
hosts (see below);
- SDIO (patches 4, 5, 6): software free-page and output-queue
accounting, RX aggregation and interrupt setup, and TX back-pressure
with a retry on page starvation.
This series no longer touches coex.c, mac80211.c or fw.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 5 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.
Three changes in patch 6 are not chip gated, because they are
structural rather than behavioural: the TX work item becomes a delayed
work, rtw_sdio_tx_kick_off() arms it with mod_delayed_work(..., 0)
instead of queue_work(), and rtw_sdio_deinit_tx() cancels it before
destroying the workqueue. Only this chip ever arms a delay, so on the
other parts the work is still queued immediately.
Patch 4 likewise initialises and destroys its mutex unconditionally, in
rtw_sdio_init_tx() and rtw_sdio_deinit_tx(). Only this chip ever takes
it, so on the other parts that is one init and one destroy and no
change in behaviour.
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, one in tx.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.
The full driver, this preparation series plus the chip support that
follows in a second series, is available for testing at:
https://github.com/MocLG/rtw/tree/8723bs-v8
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,
2.4 GHz HT40, TCP runs at roughly 35 Mbit/s down and 20 Mbit/s up, and
UDP reaches 40 Mbit/s down with no loss. The exact figures move by
several Mbit/s from one day to the next with the radio environment, so
they are a range rather than a benchmark.
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.
The locking change in patch 6 was retested that way for v8, against a
build carrying the v7 barrier version of the same patch, alternating
between the two within one session and reassociating each time. The two
came out the same within the usual run to run spread, uplink and
downlink alike, and the log was empty after every run.
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 in v3; 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.
Known limitation
================
After a long idle period the firmware intermittently fails to leave LPS,
which rtw88 reports as "firmware failed to leave lps state". That check
is the REG_TCR poll in __rtw_fw_leave_lps_check_reg(). On one occurrence
here the warning was followed fourteen seconds later by a full
reauthentication, so it is not only log noise.
Power save is enabled on my test machine and it reproduces there,
roughly once per day of mostly idle uptime, and the occurrences cluster.
It was first reported on ARM SDIO boards, but it is not specific to slow
hosts.
Instrumenting the poll settles what a failure is, and it is not a
budget that is slightly too small. A local debug patch logged REG_TCR on
every poll of __rtw_fw_leave_lps_check_reg() and counted how many polls
each wake needed. Over 110755 wakes:
poll0=158 poll1=110557 poll2=35 poll3=1 poll4=0 fails=4
pollN is the number of wakes that finished on the Nth read, so poll1
needed one 20 ms sleep and poll0 was already clear on entry; fails is
the number still set after all five. Nothing ever finished on the last
poll, yet four gave up completely, and in all four the patch kept
reading for a further 500 ms with the bit still set. The distribution
does not taper into the failures, it is bimodal: the wake is either
quick or the firmware never completes it, about once in every 28000
wakes. Raising the poll count would not have caught any of the four.
In v7 I described a failure as the far tail of the ordinary
distribution. That was wrong, and the measurement above is what
corrects it.
Ping-Ke suggests the firmware may be unable to leave LPS while packets
are still sitting in a hardware queue, which fits that shape better than
anything I had. I am extending the instrumentation to dump the per-queue
lengths and the free page and output queue counters at the failure.
It is not caused by anything in this series, and it needs the chip
driver from the second series to be reachable at all, but it is open and
I am chasing it.
Changes in v8:
- sdio: patch 4 accounted RTW_TX_QUEUE_VO against the high page pool,
but rtw_sdio_get_tx_addr() writes that queue to the normal transmit
FIFO, and the FIFO written to is the pool the chip charges. Polling
REG_SDIO_FREE_TXPG through a saturating VO flood, the high pool never
lost a page while the public pool drained, so the check was reading a
pool the traffic does not use and overstating the free space by the
whole dedicated high allocation. It is accounted against the normal
pool now, which is also what the other two rtw88 SDIO parts do.
RTW_TX_QUEUE_MGMT stays on the high pool: it goes to the extra FIFO,
which the register has no counter for and this chip gives no pages
to, and there is nowhere better to put it.
- sdio: patch 4 claimed an output queue entry before the transfer and
did not hand it back when the transfer failed, leaving the cached
count low until the next resync from the chip.
- sdio: patch 4 called mutex_init() without a matching mutex_destroy()
on teardown or on the init error path, unlike rtw_core_deinit().
- sdio: patch 6 no longer stops a queue and then undoes it. Ping-Ke
pointed out that pci.c has no such back and forth, and he is right:
it holds irq_lock across the descriptor check and the stop in
rtw_pci_tx_write(), and the wake in rtw_pci_tx_isr() runs with that
same lock already held by its caller, which is why there is not one
barrier in the whole file. Both SDIO sides now take the TX queue
lock across the length check and the flag update, so a queue can
only be stopped while it really is above the watermark and there is
nothing left to undo. The smp_mb() pair, the READ_ONCE and
WRITE_ONCE accessors and the re-check all go with it. It matches
pci.c in shape too: the enqueue is its own critical section and the
check and the stop are a second one.
- sdio: patch 4 moves the locked region into
rtw_sdio_8723bs_write_port() so the mutex can be taken with
guard(mutex). guard() expands to a declaration and so cannot be made
conditional, and the lock is only wanted for this chip, so the
generic path is a short branch that never reaches it. The transfer
itself is shared as rtw_sdio_write_to_port(); the unaligned SKB
warning stays in both callers so that the message other SDIO chips
log is unchanged.
- sdio: lockdep_assert_held() added to the three helpers that must run
under tx_credit_lock, and checked on a kernel built with
CONFIG_PROVE_LOCKING over bidirectional traffic and three module
reloads: nothing fired, and lockdep reported nothing against the TX
queue lock the back-pressure path now takes either.
- sdio: patch 4's padding uses a pad_size local, and its comment is
cut to the one point that matters, that __skb_pad() must not free
the skb. The comment above the lock is gone, since the one at the
declaration of tx_credit_lock already covers it.
- the LPS note under Known limitation is corrected against a much
longer instrumentation run; see there.
- patches 1, 2, 3 and 5 are unchanged.
Changes in v7:
- sdio: patch 6 read the mac80211 queue index from the skb after
skb_queue_tail() had published it to the TX worker, which can
dequeue, transmit and free it first. That is a use after free; the
index is now read into a local before the enqueue.
- sdio: patch 4 passed the padded size to rtw_sdio_get_tx_addr(),
where upstream passes skb->len. That value is encoded into the CMD53
address as the transfer length, so the encoded length changed for
every other SDIO chip whenever sdio_align_size() padded. For the
RTL8723BS both expressions are the same value, so the change bought
this chip nothing. The padding itself also ran on the generic path,
giving the other parts an allocation and an -ENOMEM path they did
not have. skb->len is restored and the padding is gated on the chip.
- sdio: the free page check, the output queue wait and the accounting
after the transfer are serialised. The TX worker and the H2C path
both reach rtw_sdio_write_port(), and two writers could each pass
the checks and claim the same pages and output queue entry, after
which the chip discards one of the transfers silently. The vendor
driver funnels all transmission through one thread and so never had
to handle this.
- sdio: the padding called skb_put_zero() after checking only the
tailroom, so a cloned skb with room to spare had its shared buffer
written. It uses __skb_pad() now, which reallocates in that case and
does not move skb->len, so the trim on the way out is gone too.
- sdio: the back-pressure stop could race the drain. The worker could
empty the queue after the length check but before the stopped flag
became visible, see the flag still clear in its wake check, and
never wake the queue, leaving an access category stopped with
nothing left to wake it. The stop path now re-checks after setting
the flag and undoes the stop if the queue drained.
- patches 1, 2, 3 and 5 are unchanged.
Most of these were found by Sashiko's automated review of v6. Each was
verified against the code before acting on it.
Changes in v6:
- Each patch now carries its own changelog after the --- delimiter, as
you asked, so the notes sit next to the code they describe.
- sdio: dropped rtw_sdio_reschedule_tx_work(). You did not ask for it
in v4 and it was a thin wrapper that hid queue_delayed_work() for no
gain. I had read your comment as asking for one, which was my
mistake.
- sdio: the reschedule conditions moved into
rtw_sdio_8723bs_reschedule_tx(). rtw_sdio_tx_handler() no longer
explains any chip specific condition in the common flow, which is
what you were actually asking for. No functional change.
- sdio: dropped an unconditional break on a failed transfer that v4
and v5 carried. It changed the other SDIO parts without meaning to:
upstream requeues the frame and the loop retries, and breaking gave
up after the first failure. The two errors this chip needs to retry
are handled in the helper above, so the rest keep the existing
behaviour and the other parts really are untouched again, as the
Scope section claims.
Changes in v5:
- Dropped "fw: handle the RTL8723BS management TX reports", which v4
had restored. It cannot do what its commit message claimed. This
firmware does not advertise FW_FEATURE_LPS_C2H, so
rtw_fw_leave_lps_check() takes the REG_TCR polling path and nothing
ever waits on the C2H completion the patch rerouted; and with the
driver instrumented, 93 TX reports over three scans all arrive as
C2H id 0x03, with no event ever arriving as a top level 0x12 or
0x32. Bitterblue Smith made exactly this point on v1 and was right.
A tester on a Rockchip RK3288 board confirmed the timeout patch in
this series is enough on its own. This is the fourth correction of
something I had claimed on the list; restoring it in v4 was based on
a tester reporting that a branch containing it cleared the warnings,
which showed the branch helped, not that this patch in it did.
With it gone the series no longer touches fw.c.
- Power save is now enabled on the test machine, so LPS is exercised
there. The v2 entry below dropped a patch that gated LPS entry on
smoothed throughput, partly on the grounds that LPS never engaged on
this setup at all, which was never a sound reason to drop it. It is
measured now, paired against this series in one session with power
save on: 60 idle pings average 5.1 ms without the gating and 6.0 ms
with it, and TCP is 30.0 down and 19.1 up against 32.3 and 18.9. It
makes no difference, because the gate tests the smoothed throughput
and interactive traffic rounds to zero there, so it never fires in
the case it was meant to help. It stays dropped, now on a
measurement rather than for want of one.
- sdio: fixed a transmit stall in the back-pressure patch, found while
auditing a tester report of transmit stopping under bidirectional
load with receive unaffected and nothing in the log. The TX work
item was only re-armed when a transfer failed with a page shortage.
Once the mac80211 queue can be stopped, that is not enough: a
stopped queue is handed no further frames, so on any other failure
nothing would kick the worker again and the affected access category
would stay stopped for good, with the link still up. It now also
re-arms on a failed skb expansion, which together with the page
shortage covers both of the failures that produce no log message.
The remaining errors are each logged where they happen, so unlike
those two they are visible rather than an unexplained hang, and they
keep the existing behaviour rather than being retried forever.
- rx: the zero length test now evaluates pkt_stat->pkt_len first, so
the unlikely case short circuits before the chip test.
- sdio: the commit message now says what OQT is, as far as the vendor
driver reveals it: the vendor calls the register the OQT free space
and never expands the acronym, and it holds the number of further
transfers the SDIO output queue will accept.
- sdio: rtw_sdio_8723bs_sync_free_txpg() now returns whether the chip
reported anything and is the only caller of
rtw_sdio_8723bs_store_free_txpg(), and
rtw_sdio_8723bs_init_free_txpg() returns an error rather than
nothing. Working through that suggestion found a real bug: the
public pool size was computed as acq_pg_num minus the reserved
queues with no check, so a chip that came up with no transmit page
allocation at all would underflow a u16 and leave the driver
believing it had about 65000 free pages. That calculation is now
rtw_sdio_8723bs_pubq_num(), shared with the queue page allocation
repair path, and it fails cleanly instead.
- sdio: the output queue wait is now bounded by a jiffies deadline
rather than a loop count, so RTW_SDIO_OQT_TIMEOUT_MS really is a
timeout in milliseconds. It was 1000 iterations of a 1 to 2 ms
sleep before, so the name was only approximately true.
- sdio: rtw_sdio_8723bs_check_rqpn() returns an error instead of
silently doing nothing when the transmit page pool cannot cover the
reserved queues, and rtw_sdio_start() propagates it. Both early
returns are now explained: a non-zero free page count means the
allocation latched during power on and must be left alone.
- sdio: fixed the interrupt acknowledgment. It masked the status word
with both irq_mask and RTW_SDIO_HISR_CLEAR_MASK, but for this chip
irq_mask is REG_SDIO_HIMR_RX_REQUEST alone and that bit is not in
the clear mask, so the two had no bits in common and the driver
acknowledged nothing at all. It now masks with the defined bits
only, which is what the comment described. Thanks to Ping-Ke for
catching this. Retested on hardware.
- sdio: the back-pressure and wake conditions moved into
rtw_sdio_8723bs_stop_tx_queue() and _wake_tx_queue(), and the two
requeue sites into rtw_sdio_reschedule_tx_work(), instead of long
conditions inline.
- sdio: rtw_sdio_process_tx_queue() now returns 0 on success and 1 for
the empty queue case, rather than the other way round.
- sdio: queue_stopped[] renamed to tx_queue_stopped[].
- RTW_SDIO_TX_RETRY_DELAY is left as msecs_to_jiffies(1). Ping-Ke
asked whether it becomes 0 when HZ is below 1000; it does not, since
msecs_to_jiffies() rounds up for HZ < 1000 and returns one jiffy.
Changes in v4:
- Restored "tx: extend the TX report purge timeout to RTL8723BS",
dropped in v2 because I could not reproduce the failure on my test
machine, a fast x86 host, so the path never ran for me. Testers on
slower ARM SDIO hosts, Peter Robinson on the RFC and Zhang Ning on a
Rockchip RK3288 board, both hit "failed to get tx report from
firmware". It extends the exact fix from commit c80788f7c5ae ("wifi:
rtw88: increase TX report timeout to fix race condition"), already
applied to the RTL8723DU, to the RTL8723BS, since they run the same
firmware and hit the same off-channel-scan race.
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 acknowledgment 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 a value 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 (6):
wifi: rtw88: add the RTL8723B chip type and SDIO helper
wifi: rtw88: rx: mark zero length packets on RTL8723BS
wifi: rtw88: tx: extend the TX report purge timeout to 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 | 544 ++++++++++++++++++++--
drivers/net/wireless/realtek/rtw88/sdio.h | 24 +-
drivers/net/wireless/realtek/rtw88/tx.c | 5 +-
5 files changed, 552 insertions(+), 36 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v8 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-25 16:33 ` [PATCH v8 2/6] wifi: rtw88: rx: mark zero length packets on RTL8723BS luka.gejak
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
---
Notes:
Changes in v8: none.
Changes in v7: none.
Changes in v6: none.
Changes in v5: none.
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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 2/6] wifi: rtw88: rx: mark zero length packets on RTL8723BS
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
2026-08-25 16:33 ` [PATCH v8 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-25 16:33 ` [PATCH v8 3/6] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS luka.gejak
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
---
Notes:
Changes in v8: none.
Changes in v7: none.
Changes in v6: none.
Changes in v5: the zero length test now evaluates pkt_stat->pkt_len
first, so the unlikely case short circuits before the chip test.
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..fde21c840ac0 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 (pkt_stat->pkt_len == 0 &&
+ (rtwdev->chip->id == RTW_CHIP_TYPE_8703B || rtw_is_8723bs(rtwdev))) {
rx_status->flag |= RX_FLAG_NO_PSDU;
rtw_dbg(rtwdev, RTW_DBG_RX, "zero length packet");
}
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 3/6] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
2026-08-25 16:33 ` [PATCH v8 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
2026-08-25 16:33 ` [PATCH v8 2/6] wifi: rtw88: rx: mark zero length packets on RTL8723BS luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-25 16:33 ` [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, Luka Gejak
From: Luka Gejak <luka.gejak@linux.dev>
Commit c80788f7c5ae ("wifi: rtw88: increase TX report timeout to fix
race condition") raised the purge timeout to 2500 ms for the RTL8723DU,
because the firmware can stay off channel during background scans for
longer than the 500 ms default, which delays the TX reports and lets the
purge timer drop the tracking skbs. The host stack then reads the missing
status as loss and collapses TCP throughput.
The RTL8723BS runs the same vendor firmware over a slower SDIO host and
hits the same race. Testers on ARM SDIO boards see "failed to get tx
report from firmware" under load, with the same throughput collapse.
Extend the 2500 ms timeout to the RTL8723BS.
Reported-by: Peter Robinson <pbrobinson@gmail.com>
Closes: https://lore.kernel.org/all/CALeDE9PgQmpMfDt1DgfLD4tBFGH0MZ7GncV6RUEOHhHbKF+TdQ@mail.gmail.com/
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
---
Notes:
Changes in v8: none.
Changes in v7: none.
Changes in v6: none.
Changes in v5: none. Restored in v4 after being dropped in v2, because
testers on slower ARM SDIO hosts hit the warning that commit
c80788f7c5ae already fixes for the RTL8723DU.
drivers/net/wireless/realtek/rtw88/tx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/tx.c b/drivers/net/wireless/realtek/rtw88/tx.c
index 9d747a060b98..797c1e0402f2 100644
--- a/drivers/net/wireless/realtek/rtw88/tx.c
+++ b/drivers/net/wireless/realtek/rtw88/tx.c
@@ -208,8 +208,9 @@ void rtw_tx_report_enqueue(struct rtw_dev *rtwdev, struct sk_buff *skb, u8 sn)
__skb_queue_tail(&tx_report->queue, skb);
spin_unlock_irqrestore(&tx_report->q_lock, flags);
- if (rtwdev->chip->id == RTW_CHIP_TYPE_8723D &&
- rtwdev->hci.type == RTW_HCI_TYPE_USB)
+ if ((rtwdev->chip->id == RTW_CHIP_TYPE_8723D &&
+ rtwdev->hci.type == RTW_HCI_TYPE_USB) ||
+ rtw_is_8723bs(rtwdev))
timeout = msecs_to_jiffies(2500);
mod_timer(&tx_report->purge_timer, jiffies + timeout);
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
` (2 preceding siblings ...)
2026-08-25 16:33 ` [PATCH v8 3/6] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-28 9:23 ` Ping-Ke Shih
2026-08-25 16:33 ` [PATCH v8 5/6] wifi: rtw88: sdio: set up RX aggregation and interrupts " luka.gejak
` (2 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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. The vendor driver calls this the OQT free space and
never expands the acronym; the register holds the number of further
transfers the SDIO output queue can accept, and the chip discards
writes that arrive when it has run out.
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 zero the padding with __skb_pad(), which
also reallocates a cloned skb instead of writing into a buffer a clone
still shares.
The check, the output queue wait and the accounting are serialised by
a mutex. The TX worker and the H2C path reach this function
concurrently, and two writers that both pass the checks can otherwise
claim the same pages and output queue entry, after which the chip
silently discards whichever transfer arrives second. The vendor driver
avoids the same race by funnelling all transmission through one thread.
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>
---
Notes:
Changes in v8:
- RTW_TX_QUEUE_VO is accounted against the normal page pool rather than
the high one. rtw_sdio_get_tx_addr() writes it to the normal transmit
FIFO, and that is the pool the chip charges: over a saturating VO
flood the high pool never lost a single page while the public pool
drained. The vendor maps VO to the high queue, but it also writes VO
to the high FIFO, which rtw88 does not.
- the output queue entry claimed before the transfer is handed back
when the transfer fails, rather than staying claimed until the cache
next resyncs from the chip.
- mutex_destroy() for tx_credit_lock on teardown and on the init error
path, matching rtw_core_deinit().
- the comment on the cached page counters no longer explains the
atomic_t by the several contexts that write them. Since v7 the
transmit paths all update them under tx_credit_lock; what is left
outside the lock is rtw_sdio_start() seeding them, and that is the
reason they stay atomic_t.
- the free page check, the output queue wait, the transfer and the
accounting moved into rtw_sdio_8723bs_write_port(), which takes the
lock with guard(mutex) as you suggested. The generic path is a short
branch in rtw_sdio_write_port() that never touches the lock, and the
transfer itself is shared as rtw_sdio_write_to_port().
- lockdep_assert_held() added to rtw_sdio_8723bs_write_port(),
rtw_sdio_8723bs_wait_tx_oqt() and rtw_sdio_8723bs_consume_txpg(),
and checked on a kernel built with CONFIG_PROVE_LOCKING over
bidirectional traffic and three module reloads: nothing fired.
- the padding uses a pad_size local and the comment is down to the one
point that matters, that __skb_pad() must not free the skb.
- the comment above the lock is gone; the one at the declaration of
tx_credit_lock covers it.
Changes in v7:
- the free page check, the output queue wait and the accounting after
the transfer are serialised by a mutex. The TX worker and the H2C
path run concurrently, and two writers that both passed the checks
could claim the same pages and output queue entry; the OQT refill
could also read the register while the other writer was between its
claim and its transfer. The chip silently discards the overcommitted
transfer, which for H2C means a lost firmware command.
- the CMD53 address is computed from skb->len again, as upstream does.
Passing the aligned size changed the encoded transfer length for
every other SDIO chip whenever sdio_align_size() padded; for the
RTL8723BS the two encodings are the same value.
- the padding is applied with __skb_pad() and only on the RTL8723BS
path. The open coded version wrote into a cloned skb's shared buffer
when the tailroom happened to be large enough, and it also ran on
the generic path, giving other chips a new allocation and failure
mode. __skb_pad() does not move skb->len, so the trim on the way out
is gone too. It must not free the skb on failure, since one caller
requeues it and the other frees it.
Changes in v6: none.
Changes in v5:
- the commit message now says what OQT is, as far as the vendor driver
reveals it.
- rtw_sdio_8723bs_sync_free_txpg() returns whether the chip reported
anything and is the only caller of _store_free_txpg(), and
_init_free_txpg() returns an error rather than nothing. That found a
real bug: the public pool size was acq_pg_num minus the reserved
queues with no check, so a chip coming up with no transmit page
allocation would underflow a u16 and leave about 65000 free pages.
It is now rtw_sdio_8723bs_pubq_num(), shared with the queue page
allocation repair path, and it fails cleanly.
- the output queue wait is bounded by a jiffies deadline rather than a
loop count, so RTW_SDIO_OQT_TIMEOUT_MS is really milliseconds.
- rtw_sdio_8723bs_check_rqpn() returns an error instead of silently
doing nothing when the pool cannot cover the reserved queues, and
rtw_sdio_start() propagates it. Both early returns are explained.
drivers/net/wireless/realtek/rtw88/sdio.c | 363 ++++++++++++++++++++--
drivers/net/wireless/realtek/rtw88/sdio.h | 12 +
2 files changed, 356 insertions(+), 19 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 5b40d74b16ee..1e5af37089f7 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,164 @@ 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. The transmit
+ * paths read and update them under tx_credit_lock; rtw_sdio_start() seeds
+ * them outside it, 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 any drift.
+ */
+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));
+}
+
+/*
+ * Refresh the cached counters from the chip. Returns false when the chip
+ * reports no free pages at all, which means the counts cannot be trusted
+ * and the caller has to decide what to do instead.
+ */
+static bool rtw_sdio_8723bs_sync_free_txpg(struct rtw_dev *rtwdev)
+{
+ u32 free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+
+ if (!free_txpg)
+ return false;
+
+ rtw_sdio_8723bs_store_free_txpg(rtwdev, free_txpg);
+
+ return true;
+}
+
+/*
+ * Size of the public page pool: whatever the transmit page allocation has
+ * left once the per queue pools are taken out. Fails if the allocation
+ * cannot cover the reserved queues, since the remainder would underflow and
+ * there would be no sensible pool to hand out.
+ */
+static int rtw_sdio_8723bs_pubq_num(struct rtw_dev *rtwdev, u16 *pubq_num)
+{
+ const struct rtw_page_table *pg_tbl = &rtwdev->chip->page_table[0];
+ u16 acq_pg_num = rtwdev->fifo.acq_pg_num;
+ u16 reserved_num;
+
+ reserved_num = pg_tbl->hq_num + pg_tbl->lq_num + pg_tbl->nq_num +
+ pg_tbl->exq_num + pg_tbl->gapq_num;
+ if (acq_pg_num <= reserved_num) {
+ rtw_err(rtwdev,
+ "no transmit pages left for the public queue: %u of %u reserved\n",
+ reserved_num, acq_pg_num);
+ return -EINVAL;
+ }
+
+ *pubq_num = acq_pg_num - reserved_num;
+
+ return 0;
+}
+
+static int 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;
+ u16 pubq_num;
+ int ret;
+
+ /* Seed from the page table when the chip has nothing to report yet. */
+ if (!rtw_sdio_8723bs_sync_free_txpg(rtwdev)) {
+ ret = rtw_sdio_8723bs_pubq_num(rtwdev, &pubq_num);
+ if (ret)
+ return ret;
+
+ pg_tbl = &rtwdev->chip->page_table[0];
+ 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));
+
+ return 0;
+}
+
+/*
+ * 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;
+}
+
+/*
+ * The pool a queue draws from follows the transmit FIFO that
+ * rtw_sdio_get_tx_addr() writes into, since that is the one the chip charges.
+ * RTW_TX_QUEUE_MGMT is the exception: it goes to the extra FIFO, which
+ * REG_SDIO_FREE_TXPG has no counter for and this chip allocates no pages to,
+ * so it is accounted against the high pool and served from the public one.
+ */
+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:
+ case RTW_TX_QUEUE_VO:
+ 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:
+ 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,35 +785,67 @@ static int rtw_sdio_check_free_txpg(struct rtw_dev *rtwdev, u8 queue,
return 0;
}
-static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
- enum rtw_tx_queue_type queue)
+static int rtw_sdio_8723bs_wait_tx_oqt(struct rtw_dev *rtwdev)
{
struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
- bool bus_claim;
- size_t txsize;
- u32 txaddr;
- int ret;
+ unsigned long deadline;
+ u8 free;
- txaddr = rtw_sdio_get_tx_addr(rtwdev, skb->len, queue);
- if (!txaddr)
- return -EINVAL;
+ lockdep_assert_held(&rtwsdio->tx_credit_lock);
- txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+ if (atomic_add_unless(&rtwsdio->tx_oqt_free, -1, 0))
+ return 0;
- ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
- if (ret)
- return ret;
+ deadline = jiffies + msecs_to_jiffies(RTW_SDIO_OQT_TIMEOUT_MS);
+ do {
+ 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);
+ } while (time_before(jiffies, deadline));
- 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);
+ 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;
+
+ lockdep_assert_held(&rtwsdio->tx_credit_lock);
+
+ 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_to_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
+ u32 txaddr, size_t write_size)
+{
+ struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+ bool bus_claim;
+ int ret;
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);
@@ -668,11 +853,103 @@ static int rtw_sdio_write_port(struct rtw_dev *rtwdev, struct sk_buff *skb,
if (ret)
rtw_warn(rtwdev,
"Failed to write %zu byte(s) to SDIO port 0x%08x",
- txsize, txaddr);
+ write_size, txaddr);
return ret;
}
+/*
+ * The free page check, the output queue wait and the accounting after the
+ * transfer have to be one unit, or two writers can both pass the checks and
+ * claim the same pages and output queue entry.
+ */
+static int rtw_sdio_8723bs_write_port(struct rtw_dev *rtwdev,
+ struct sk_buff *skb,
+ enum rtw_tx_queue_type queue, u32 txaddr,
+ size_t txsize, size_t write_size)
+{
+ struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+ unsigned int pages;
+ int ret;
+
+ lockdep_assert_held(&rtwsdio->tx_credit_lock);
+
+ ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
+ if (ret)
+ return ret;
+
+ ret = rtw_sdio_8723bs_wait_tx_oqt(rtwdev);
+ if (ret)
+ return ret;
+
+ 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);
+
+ ret = rtw_sdio_write_to_port(rtwdev, skb, txaddr, write_size);
+ if (ret) {
+ /* nothing was queued, so hand the output queue entry back */
+ atomic_inc(&rtwsdio->tx_oqt_free);
+ return ret;
+ }
+
+ pages = DIV_ROUND_UP(txsize, rtwdev->chip->page_size);
+ rtw_sdio_8723bs_consume_txpg(rtwdev, queue, pages);
+
+ return 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;
+ size_t write_size;
+ size_t txsize;
+ u32 txaddr;
+ int ret;
+
+ txaddr = rtw_sdio_get_tx_addr(rtwdev, skb->len, queue);
+ if (!txaddr)
+ return -EINVAL;
+
+ if (!rtw_is_8723bs(rtwdev)) {
+ txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
+
+ ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
+ if (ret)
+ return ret;
+
+ 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);
+
+ return rtw_sdio_write_to_port(rtwdev, skb, txaddr, txsize);
+ }
+
+ txsize = round_up(skb->len, 4);
+ write_size = txsize > RTW_SDIO_BLOCK_SIZE ?
+ round_up(txsize, RTW_SDIO_BLOCK_SIZE) : txsize;
+
+ if (write_size > skb->len) {
+ size_t pad_size = write_size - skb->len;
+
+ /*
+ * __skb_pad() must not free the skb on failure: both callers
+ * still own it, one requeues it and the other frees it.
+ */
+ ret = __skb_pad(skb, pad_size, false);
+ if (ret)
+ return ret;
+ }
+
+ guard(mutex)(&rtwsdio->tx_credit_lock);
+
+ return rtw_sdio_8723bs_write_port(rtwdev, skb, queue, txaddr, txsize,
+ write_size);
+}
+
static void rtw_sdio_init(struct rtw_dev *rtwdev)
{
struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
@@ -749,8 +1026,51 @@ static int rtw_sdio_setup(struct rtw_dev *rtwdev)
return 0;
}
+/*
+ * Reprogram the queue page allocation if the chip came up with none. This is
+ * a repair path, not part of the normal start sequence: a non-zero free page
+ * count means the allocation latched during power on and must be left alone,
+ * and without a transmit page pool there is nothing to divide up either.
+ */
+static int 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;
+ u32 free_txpg;
+ u16 pubq_num;
+ int ret;
+
+ free_txpg = rtw_read32(rtwdev, REG_SDIO_FREE_TXPG);
+ if (free_txpg || !fifo->acq_pg_num)
+ return 0;
+
+ ret = rtw_sdio_8723bs_pubq_num(rtwdev, &pubq_num);
+ if (ret)
+ return ret;
+
+ pg_tbl = &chip->page_table[0];
+ 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));
+
+ return 0;
+}
+
static int rtw_sdio_start(struct rtw_dev *rtwdev)
{
+ if (rtw_is_8723bs(rtwdev)) {
+ int ret = rtw_sdio_8723bs_check_rqpn(rtwdev);
+
+ if (ret)
+ return ret;
+
+ ret = rtw_sdio_8723bs_init_free_txpg(rtwdev);
+ if (ret)
+ return ret;
+ }
+
rtw_sdio_enable_rx_aggregation(rtwdev);
rtw_sdio_enable_interrupt(rtwdev);
@@ -1294,6 +1614,8 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
return -ENOMEM;
}
+ mutex_init(&rtwsdio->tx_credit_lock);
+
for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
skb_queue_head_init(&rtwsdio->tx_queue[i]);
rtwsdio->tx_handler_data = kmalloc_obj(*rtwsdio->tx_handler_data);
@@ -1306,6 +1628,7 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
return 0;
err_destroy_wq:
+ mutex_destroy(&rtwsdio->tx_credit_lock);
destroy_workqueue(rtwsdio->txwq);
return -ENOMEM;
}
@@ -1320,6 +1643,8 @@ static void rtw_sdio_deinit_tx(struct rtw_dev *rtwdev)
for (i = 0; i < RTK_MAX_TX_QUEUE_NUM; i++)
ieee80211_purge_tx_queue(rtwdev->hw, &rtwsdio->tx_queue[i]);
+
+ mutex_destroy(&rtwsdio->tx_credit_lock);
}
int rtw_sdio_probe(struct sdio_func *sdio_func,
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.h b/drivers/net/wireless/realtek/rtw88/sdio.h
index 457e8b02380e..f43f1c6309b7 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,14 @@ 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;
+ /* one writer at a time between the credit check and the accounting */
+ struct mutex tx_credit_lock;
};
extern const struct dev_pm_ops rtw_sdio_pm_ops;
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* RE: [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
2026-08-25 16:33 ` [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
@ 2026-08-28 9:23 ` Ping-Ke Shih
2026-08-28 9:40 ` Luka Gejak
0 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2026-08-28 9:23 UTC (permalink / raw)
To: luka.gejak@linux.dev, linux-wireless@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Michael Straube, Bitterblue Smith,
Peter Robinson, Hans de Goede
luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
[...]
> +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;
> + size_t write_size;
> + size_t txsize;
> + u32 txaddr;
> + int ret;
> +
> + txaddr = rtw_sdio_get_tx_addr(rtwdev, skb->len, queue);
> + if (!txaddr)
> + return -EINVAL;
> +
> + if (!rtw_is_8723bs(rtwdev)) {
> + txsize = sdio_align_size(rtwsdio->sdio_func, skb->len);
> +
> + ret = rtw_sdio_check_free_txpg(rtwdev, queue, txsize);
> + if (ret)
> + return ret;
> +
> + 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);
> +
> + return rtw_sdio_write_to_port(rtwdev, skb, txaddr, txsize);
Since you have two branches for RTL8723BS and others, let's implement them
by individual functions. Here can just dispatch, like
int rtw_sdio_write_port()
{
if (rtw_is_8723bs(rtwdev))
return rtw_sdio_write_port_8723bs();
else
return rtw_sdio_write_port_common();
}
> + }
> +
> + txsize = round_up(skb->len, 4);
> + write_size = txsize > RTW_SDIO_BLOCK_SIZE ?
> + round_up(txsize, RTW_SDIO_BLOCK_SIZE) : txsize;
> +
> + if (write_size > skb->len) {
> + size_t pad_size = write_size - skb->len;
> +
> + /*
> + * __skb_pad() must not free the skb on failure: both callers
> + * still own it, one requeues it and the other frees it.
> + */
> + ret = __skb_pad(skb, pad_size, false);
> + if (ret)
> + return ret;
> + }
> +
> + guard(mutex)(&rtwsdio->tx_credit_lock);
> +
> + return rtw_sdio_8723bs_write_port(rtwdev, skb, queue, txaddr, txsize,
> + write_size);
> +}
> +
^ permalink raw reply [flat|nested] 12+ messages in thread* RE: [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS
2026-08-28 9:23 ` Ping-Ke Shih
@ 2026-08-28 9:40 ` Luka Gejak
0 siblings, 0 replies; 12+ messages in thread
From: Luka Gejak @ 2026-08-28 9:40 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Michael Straube, Bitterblue Smith,
Peter Robinson, Hans de Goede, luka.gejak
Hi Ping-Ke,
On August 28, 2026 11:23:26 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
> Since you have two branches for RTL8723BS and others, let's implement
> them by individual functions. Here can just dispatch, like
Will do in v9.
Two details I would rather ask about than decide on my own, since both
touch the other SDIO parts rather than this chip.
The unaligned SKB warning uses __func__, and today that resolves to
rtw_sdio_write_port on every chip. Once the generic path moves into its
own function the string becomes that function's name for 8703b, 8723d,
8821c, 8822b and 8822c. Nothing reads it, but it is still a visible
change to parts this series is not about. The options I see are to let
it change and say so in the change log, to drop __func__ and word the
message without a function name, or to keep the check in the dispatcher
so the string stays as it is.
I would take the first, since the name then simply follows whichever
function ran. The third is the only one that leaves the other parts
completely alone, but it would move the warning ahead of
rtw_sdio_check_free_txpg() on the generic path, so a page shortage would
start logging a line it does not log today. That seems worse than the
string changing. Say if you would rather have it either of the other
ways.
> rtw_sdio_write_port_8723bs()
The chip specific helpers already use the _8723bs_ prefix, which is the
convention you asked for in v4, so I plan to keep
rtw_sdio_8723bs_write_port() and add rtw_sdio_write_port_common()
next to it. Happy to rename them to match your sketch if you would
rather they read that way.
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v8 5/6] wifi: rtw88: sdio: set up RX aggregation and interrupts for RTL8723BS
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
` (3 preceding siblings ...)
2026-08-25 16:33 ` [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-25 16:33 ` [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak
2026-08-26 17:53 ` [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS Luka Gejak
6 siblings, 0 replies; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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 was also seen to keep raising the interrupt after resume when
undefined status bits were written back on acknowledgment, so
acknowledge only the bits this driver defines. That observation dates
from bring up and I cannot re-measure it: the test machine only offers
s2idle and does not reliably return from it, so the resume path is not
exercised here. The change 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>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
---
Notes:
Changes in v8: none.
Changes in v7: none.
Changes in v6: none.
Changes in v5: fixed the interrupt acknowledgment. It masked the status
word with both irq_mask and RTW_SDIO_HISR_CLEAR_MASK, but for this chip
irq_mask is REG_SDIO_HIMR_RX_REQUEST alone and that bit is not in the
clear mask, so the two had no bits in common and the driver acknowledged
nothing at all. It now masks with the defined bits only, which is what
the comment described. The commit message also no longer presents the
resume behaviour as re-measured, because it is not.
drivers/net/wireless/realtek/rtw88/sdio.c | 28 ++++++++++++++++++++++-
drivers/net/wireless/realtek/rtw88/sdio.h | 9 ++++++++
2 files changed, 36 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index 1e5af37089f7..bc9f142cbf5e 100644
--- a/drivers/net/wireless/realtek/rtw88/sdio.c
+++ b/drivers/net/wireless/realtek/rtw88/sdio.c
@@ -954,7 +954,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)
@@ -962,6 +966,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:
@@ -989,6 +994,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)
@@ -1060,6 +1067,8 @@ static int 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)) {
int ret = rtw_sdio_8723bs_check_rqpn(rtwdev);
@@ -1072,6 +1081,13 @@ static int rtw_sdio_start(struct rtw_dev *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;
@@ -1151,6 +1167,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);
}
@@ -1415,6 +1433,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 this
+ * driver defines. Other chips keep the existing behaviour.
+ */
+ if (rtw_is_8723bs(rtwdev))
+ hisr &= 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 f43f1c6309b7..634c0b1339bb 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
` (4 preceding siblings ...)
2026-08-25 16:33 ` [PATCH v8 5/6] wifi: rtw88: sdio: set up RX aggregation and interrupts " luka.gejak
@ 2026-08-25 16:33 ` luka.gejak
2026-08-28 9:29 ` Ping-Ke Shih
2026-08-26 17:53 ` [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS Luka Gejak
6 siblings, 1 reply; 12+ messages in thread
From: luka.gejak @ 2026-08-25 16:33 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, 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 a transfer cannot be completed 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. Both sides
take the TX queue lock across the length check and the flag update, the
way rtw_pci_tx_write() and rtw_pci_tx_isr() use irq_lock. Without it the
producer could stop a queue on a length the drain path had already
emptied, having seen the flag still clear and so skipped the wake, and
the AC would have stayed stopped with nothing left to wake it.
Convert the TX work item to a delayed work and re-arm it when a transfer
fails for a reason that can clear on its own, so it is retried rather
than the AC abandoned, and cancel the work on teardown.
Retrying matters once the queue can be stopped. A stopped queue is
handed no further frames, so nothing else would kick the worker, and the
AC would stay stopped for good with the link still up and receive
unaffected. The two retried cases, a transmit page or output queue
shortage and a failed skb expansion, are also the two that fail
silently; the rest are logged where they happen, so they are visible
rather than an unexplained hang, and they keep the existing behaviour
rather than being retried indefinitely.
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>
---
Notes:
Changes in v8:
- the stop and the wake take the TX queue lock with
guard(spinlock_irqsave) across the length check and the flag update,
following pci.c, which holds irq_lock on both sides and needs no
barrier because of it. The smp_mb() pair, the READ_ONCE and
WRITE_ONCE accessors and the re-check that undid a stop all went
with it.
- q_map is read later, just before the call that uses it. It cannot move
past rtw_sdio_indicate_tx_status(), which consumes the skb, so that is
as far down as it goes.
Changes in v7:
- the mac80211 queue index is read before skb_queue_tail() publishes
the skb to the TX worker, which may process and free it immediately;
reading it afterwards was a use after free.
- the stop path re-checks the FIFO after setting the stopped flag and
undoes the stop if the worker drained it meanwhile. In that window
the drain side still saw the flag clear, so neither side would have
woken the queue and the AC stayed stopped for good. The flag is
accessed with READ_ONCE/WRITE_ONCE and the re-check is ordered
against the drain path with a barrier pair.
Changes in v6:
- dropped rtw_sdio_reschedule_tx_work(). It was a thin wrapper around
queue_delayed_work() and hid the kernel API for no gain.
- the reschedule conditions moved into rtw_sdio_8723bs_reschedule_tx(),
so rtw_sdio_tx_handler() no longer explains any chip specific
condition in the common flow.
- dropped the unconditional break on a failed transfer. v4 and v5 had
it, and it quietly changed the other SDIO parts: upstream requeues
the frame and the loop retries, and breaking gave up after the first
failure. The two errors this chip needs to retry are handled in the
helper above, so the rest can keep the existing behaviour and the
other parts are untouched again.
Changes in v5:
- the back-pressure and wake conditions moved into
rtw_sdio_8723bs_stop_tx_queue() and _wake_tx_queue().
- rtw_sdio_process_tx_queue() returns 0 on success and 1 for the empty
queue case, rather than the other way round.
- queue_stopped[] renamed to tx_queue_stopped[].
- fixed a transmit stall: the work item was only re-armed on a page
shortage, so on any other failure a stopped access category would
stay stopped for good. It now also re-arms on a failed skb
expansion, which with the page shortage covers both failures that
produce no log message.
- RTW_SDIO_TX_RETRY_DELAY is left as msecs_to_jiffies(1): it does not
become 0 for HZ < 1000, since msecs_to_jiffies() rounds up.
drivers/net/wireless/realtek/rtw88/sdio.c | 153 ++++++++++++++++++++--
drivers/net/wireless/realtek/rtw88/sdio.h | 3 +-
2 files changed, 145 insertions(+), 11 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/sdio.c b/drivers/net/wireless/realtek/rtw88/sdio.c
index bc9f142cbf5e..1a85ed1431b4 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);
@@ -1151,7 +1161,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)
@@ -1260,12 +1274,65 @@ static int rtw_sdio_write_data_h2c(struct rtw_dev *rtwdev, u8 *buf, u32 size)
return rtw_sdio_write_data(rtwdev, &pkt_info, skb, RTW_TX_QUEUE_H2C);
}
+/*
+ * Back-pressure on the data ACs (BK/BE/VI/VO): once the software FIFO fills
+ * past the high watermark, stop the corresponding mac80211 queue so it stops
+ * handing frames down, which bounds the queueing latency. The queue is woken
+ * again from the TX drain path once the FIFO falls back to the low watermark.
+ *
+ * Both sides hold the TX queue lock across the length check and the flag
+ * update, so a queue is only ever stopped while it really is above the
+ * watermark, and the drain path always sees the flag the producer set.
+ */
+static void rtw_sdio_8723bs_stop_tx_queue(struct rtw_dev *rtwdev,
+ enum rtw_tx_queue_type queue,
+ u16 q_map)
+{
+ struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+ if (!rtw_is_8723bs(rtwdev) || queue >= RTW_TX_QUEUE_BCN)
+ return;
+
+ guard(spinlock_irqsave)(&rtwsdio->tx_queue[queue].lock);
+
+ if (rtwsdio->tx_queue_stopped[queue])
+ return;
+
+ if (skb_queue_len(&rtwsdio->tx_queue[queue]) < RTW_SDIO_TX_FIFO_HIWATER)
+ return;
+
+ rtwsdio->tx_queue_stopped[queue] = true;
+ ieee80211_stop_queue(rtwdev->hw, q_map);
+}
+
+static void rtw_sdio_8723bs_wake_tx_queue(struct rtw_dev *rtwdev,
+ enum rtw_tx_queue_type queue,
+ u16 q_map)
+{
+ struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+
+ if (!rtw_is_8723bs(rtwdev) || queue >= RTW_TX_QUEUE_BCN)
+ return;
+
+ guard(spinlock_irqsave)(&rtwsdio->tx_queue[queue].lock);
+
+ if (!rtwsdio->tx_queue_stopped[queue])
+ return;
+
+ if (skb_queue_len(&rtwsdio->tx_queue[queue]) > RTW_SDIO_TX_FIFO_LOWATER)
+ return;
+
+ rtwsdio->tx_queue_stopped[queue] = false;
+ ieee80211_wake_queue(rtwdev->hw, q_map);
+}
+
static int rtw_sdio_tx_write(struct rtw_dev *rtwdev,
struct rtw_tx_pkt_info *pkt_info,
struct sk_buff *skb)
{
struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
enum rtw_tx_queue_type queue = rtw_tx_queue_mapping(skb);
+ u16 q_map = skb_get_queue_mapping(skb);
struct rtw_sdio_tx_data *tx_data;
rtw_sdio_tx_skb_prepare(rtwdev, pkt_info, skb, queue);
@@ -1273,8 +1340,15 @@ static int rtw_sdio_tx_write(struct rtw_dev *rtwdev,
tx_data = rtw_sdio_get_tx_data(skb);
tx_data->sn = pkt_info->sn;
+ /*
+ * skb_queue_tail() publishes the skb to the TX worker, which may
+ * process and free it immediately, so nothing may touch the skb
+ * past this point.
+ */
skb_queue_tail(&rtwsdio->tx_queue[queue], skb);
+ rtw_sdio_8723bs_stop_tx_queue(rtwdev, queue, q_map);
+
return 0;
}
@@ -1577,33 +1651,83 @@ 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 0 when a frame was written, 1 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 1;
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() consumes the skb, so read this first */
+ q_map = skb_get_queue_mapping(skb);
+
rtw_sdio_indicate_tx_status(rtwdev, skb);
+
+ rtw_sdio_8723bs_wake_tx_queue(rtwdev, queue, q_map);
+
+ return 0;
+}
+
+/*
+ * Decide whether the RTL8723BS wants the TX work to run again, and if so
+ * arrange it and tell the caller to stop draining. Two cases need it.
+ *
+ * A transmit page or output queue shortage and a failed skb expansion are
+ * transient and leave the frame queued, so come back for it shortly. That
+ * matters once the mac80211 queue can be stopped: a stopped queue is handed
+ * no further frames, so nothing else would kick this work item and the access
+ * category would stay stopped for good. The remaining errors are logged where
+ * they happen and are not retried.
+ *
+ * After a management frame, restart from the highest priority queue so the
+ * join sequence is not held up behind a data backlog.
+ */
+static bool rtw_sdio_8723bs_reschedule_tx(struct rtw_dev *rtwdev,
+ struct rtw_sdio_work_data *work_data,
+ enum rtw_tx_queue_type queue, int ret)
+{
+ struct rtw_sdio *rtwsdio = (struct rtw_sdio *)rtwdev->priv;
+ unsigned long delay;
+
+ if (!rtw_is_8723bs(rtwdev))
+ return false;
+
+ if (ret == -EBUSY || ret == -ENOMEM)
+ delay = RTW_SDIO_TX_RETRY_DELAY;
+ else if (ret == 0 && queue == RTW_TX_QUEUE_MGMT)
+ delay = 0;
+ else
+ return false;
+
+ queue_delayed_work(rtwsdio->txwq, &work_data->work, delay);
+
+ return true;
}
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;
rtwdev = work_data->rtwdev;
rtwsdio = (struct rtw_sdio *)rtwdev->priv;
@@ -1613,7 +1737,13 @@ static void rtw_sdio_tx_handler(struct work_struct *work)
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 (rtw_sdio_8723bs_reschedule_tx(rtwdev, work_data,
+ queue, ret))
+ return;
if (skb_queue_empty(&rtwsdio->tx_queue[queue]))
break;
@@ -1642,14 +1772,16 @@ static int rtw_sdio_init_tx(struct rtw_dev *rtwdev)
mutex_init(&rtwsdio->tx_credit_lock);
- 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->tx_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;
@@ -1664,6 +1796,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 634c0b1339bb..6e7e6009744b 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 tx_queue_stopped[RTK_MAX_TX_QUEUE_NUM];
atomic_t free_pg_high;
atomic_t free_pg_normal;
--
2.53.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* RE: [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation
2026-08-25 16:33 ` [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak
@ 2026-08-28 9:29 ` Ping-Ke Shih
2026-08-28 9:47 ` Luka Gejak
0 siblings, 1 reply; 12+ messages in thread
From: Ping-Ke Shih @ 2026-08-28 9:29 UTC (permalink / raw)
To: luka.gejak@linux.dev, linux-wireless@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Michael Straube, Bitterblue Smith,
Peter Robinson, Hans de Goede
luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
> 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 a transfer cannot be completed 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. Both sides
> take the TX queue lock across the length check and the flag update, the
> way rtw_pci_tx_write() and rtw_pci_tx_isr() use irq_lock. Without it the
> producer could stop a queue on a length the drain path had already
> emptied, having seen the flag still clear and so skipped the wake, and
> the AC would have stayed stopped with nothing left to wake it.
>
> Convert the TX work item to a delayed work and re-arm it when a transfer
> fails for a reason that can clear on its own, so it is retried rather
> than the AC abandoned, and cancel the work on teardown.
>
> Retrying matters once the queue can be stopped. A stopped queue is
> handed no further frames, so nothing else would kick the worker, and the
> AC would stay stopped for good with the link still up and receive
> unaffected. The two retried cases, a transmit page or output queue
> shortage and a failed skb expansion, are also the two that fail
> silently; the rest are logged where they happen, so they are visible
> rather than an unexplained hang, and they keep the existing behaviour
> rather than being retried indefinitely.
>
> 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>
Acked-by: Ping-Ke Shih <pkshih@realtek.com>
The v8 is almost done. Only one small nit.
Since I don't test the RTL8723BS at all, please test it by yourself
carefully. At the last review (I think v9), I will focus on the changes
of this patchset doesn't affect existing chips.
^ permalink raw reply [flat|nested] 12+ messages in thread
* RE: [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation
2026-08-28 9:29 ` Ping-Ke Shih
@ 2026-08-28 9:47 ` Luka Gejak
0 siblings, 0 replies; 12+ messages in thread
From: Luka Gejak @ 2026-08-28 9:47 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Michael Straube, Bitterblue Smith,
Peter Robinson, Hans de Goede, luka.gejak
Hi Ping-Ke,
On August 28, 2026 11:29:29 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
> At the last review (I think v9), I will focus on the changes of this
> patchset doesn't affect existing chips.
That is the right thing to focus on and it is the part I cannot test, so
let me set out what the other parts actually run, and correct something
in the v8 cover letter while I am at it.
New code that is not behind rtw_is_8723bs(), all of it structural:
sdio.h struct delayed_work work
rtw_sdio_init_tx() INIT_DELAYED_WORK(), mutex_init(), and
mutex_destroy() on its error path
rtw_sdio_tx_kick_off() mod_delayed_work(..., 0) in place of
queue_work()
rtw_sdio_deinit_tx() cancel_delayed_work_sync(), mutex_destroy()
Only this chip ever arms a delay, so on the other parts the work is
still queued immediately, and only this chip ever takes the mutex.
Two existing functions were also restructured and every chip runs
through them, so I should not describe the above as the whole story.
rtw_sdio_process_tx_queue() now returns a value instead of void, and
rtw_sdio_tx_handler() uses it. The behaviour is meant to be identical
for the other parts: on an empty queue the new "if (ret > 0) break"
takes the place of the old skb_queue_empty() break, and on a failed
write the frame is still requeued and the loop still retries, since
rtw_sdio_8723bs_reschedule_tx() returns false for anything that is not
this chip. That is the piece I would most like you to check.
Two things I verified against the base commit that I will spell out in
the v9 cover letter rather than leave implied:
rtw_sdio_check_free_txpg() differs by exactly one inserted branch ahead
of the existing 8051 test, which becomes an else if, everything else
unchanged; and the generic path through rtw_sdio_write_port() is step
for step what it was, same address from skb->len, same
sdio_align_size(), same check, same transfer, same failure message.
The correction: the v8 cover letter says patch 2 is "gated on the SDIO
interface rather than the chip id". That was true of an earlier version
and I did not update it. The code you have uses rtw_is_8723bs(), so it
is gated on the chip id as well, and the sentence understates how narrow
the check is. I will fix that section in v9.
One thing that would settle this better than any argument from me. rtw88
already supports five other SDIO parts that share sdio.c: RTL8723CS,
RTL8723DS, RTL8821CS, RTL8822BS and RTL8822CS. If anyone on the list has
any of them, a boot and some traffic with this series applied would
answer the question directly. I do not have that hardware. If nobody
does, I will say so in the cover letter rather than let it look tested.
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
` (5 preceding siblings ...)
2026-08-25 16:33 ` [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak
@ 2026-08-26 17:53 ` Luka Gejak
6 siblings, 0 replies; 12+ messages in thread
From: Luka Gejak @ 2026-08-26 17:53 UTC (permalink / raw)
To: Ping-Ke Shih, linux-wireless
Cc: linux-kernel, Michael Straube, Bitterblue Smith, Peter Robinson,
Hans de Goede, luka.gejak
On August 25, 2026 6:33:08 PM GMT+02:00, 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 Ping-Ke,
A tester has been running v8 on a slow ARM SDIO board. In a good signal
environment it passes a thousand iterations of his stress test with
nothing in the log. In a poor one he still gets "failed to get tx report
from firmware", and a reconnection along with it. Power save was off for
both.
Looking at why, the two ends of the tx report path are not symmetric:
rtw_tx_report_handle() -> rtw_tx_report_tx_status()
-> ieee80211_tx_status_irqsafe()
rtw_tx_report_purge_timer() -> skb_queue_purge()
When the report arrives mac80211 gets a verdict. When it does not, the
frames are freed with kfree_skb() and mac80211 is told nothing at all.
rtw_tx_report_enqueue() also re-arms the timer on every frame, so it
fires once, 500 ms after the last enqueue, and purges everything
outstanding, including frames queued a moment earlier.
One of the frames that asks for a report is the nullfunc mac80211 sends
to poll a link it suspects is dead. If no status comes back,
ieee80211_sta_tx_notify() is never called, the poll counts as
unanswered, and mac80211 tears the connection down. That fits what the
tester sees: the warning and the reconnection arriving together, and
only when the signal is poor enough for reports to go missing.
Raising the timeout further does not look like the answer. Patch 3 of
this series already takes it to 2500 ms for the RTL8723BS, five times
the default, and he still reaches it. A longer wait only delays the
status.
What looks right to me is to report the frames instead of dropping
them: walk the queue on timeout and hand each one back with
rtw_tx_report_tx_status(rtwdev, cur, false). That tells mac80211 the
frame was not acknowledged, which is true, and lets it act rather than
wait for a status that will never come. It would also stop mac80211 TX
skbs being freed with kfree_skb() instead of being returned through
ieee80211_tx_status().
That is core behaviour for every chip, not just this one, so I would
rather ask than send it. Would you want it as a separate patch outside
this series? And is reporting "not acked" what you would want on the
other parts, or would you rather the timeout stayed silent for them and
only this chip changed?
Best regards,
Luka Gejak
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2026-08-28 9:47 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-25 16:33 [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS luka.gejak
2026-08-25 16:33 ` [PATCH v8 1/6] wifi: rtw88: add the RTL8723B chip type and SDIO helper luka.gejak
2026-08-25 16:33 ` [PATCH v8 2/6] wifi: rtw88: rx: mark zero length packets on RTL8723BS luka.gejak
2026-08-25 16:33 ` [PATCH v8 3/6] wifi: rtw88: tx: extend the TX report purge timeout to RTL8723BS luka.gejak
2026-08-25 16:33 ` [PATCH v8 4/6] wifi: rtw88: sdio: track free TX pages and OQT credits for RTL8723BS luka.gejak
2026-08-28 9:23 ` Ping-Ke Shih
2026-08-28 9:40 ` Luka Gejak
2026-08-25 16:33 ` [PATCH v8 5/6] wifi: rtw88: sdio: set up RX aggregation and interrupts " luka.gejak
2026-08-25 16:33 ` [PATCH v8 6/6] wifi: rtw88: sdio: add TX back-pressure and retry on page starvation luka.gejak
2026-08-28 9:29 ` Ping-Ke Shih
2026-08-28 9:47 ` Luka Gejak
2026-08-26 17:53 ` [PATCH v8 0/6] wifi: rtw88: preparations for RTL8723B/RTL8723BS Luka Gejak
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.