RE: [PATCH v2 11/11] wifi: rtw88: run the RTL8723BS association register sequence

[Luka Gejak <luka.gejak@linux.dev>]

On July 31, 2026 2:48:45 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
>
>luka.gejak@linux.dev <luka.gejak@linux.dev> wrote;
>> On 27/07/2026 09:33, Ping-Ke Shih wrote:
>> [...]
>> On reducing the rest: this patch is smaller in v3, but not as much as
>> I would like, and I want to be straight about why. Two things went:
>>
>>  - the beacon wait, once you made me measure it (see my reply on
>>    patch 10), which took struct rtw_auth_sync with it
>
>I see the result is positive, so this part can be removed, right?

Right, it is already gone in v3.

>> What is left is the vendor start_clnt_join() register programming
>> itself: the BSSID-scoped receive filter, the response and basic rate
>> set from the BSS rate elements, preamble and slot time, TSF update, the
>> retry limit and the security config. I have removed these individually
>> on hardware and association fails or the AP drops us shortly after,
>> which is why they are still here rather than because I have not tried.
>
>I think you can implement these by chip->ops->mac_init(rtwdev);

Before answering that I have to correct the sentence you are replying
to. It is wrong.

I ran the experiment you asked for below, and the association register
sequence is not needed at all. What I told you was bring-up era data
that I repeated without re-measuring. That is the same mistake you
caught with the beacon wait, and I should have re-tested before
asserting it a second time.

>So the order of patches might be
>1. add specific handlings for RTL8723BS for common flow
>2. add RTL8723BS (but not enable right now)
>3. add *very* specific handlings for RTL8723BS for common flow
>   Here *very* means a lot of difference like you mentioned.
>   But if without this part, the RTL8723BS still can work (not perfect),
>   the step 4 can go ahead with step 2. Need your experiments to
>   confirm.
>4. enable RTL8723BS by Kconfig/Makefile

Confirmed, and it is better than "not perfect": I measured no
difference at all. Step 3 disappears.

I reverted the whole patch, 440 lines, which is the join sequence, the
chip_ops::prepare_tx callback, the six sites in
rtw_ops_bss_info_changed() and both sites in rtw_ops_set_key(). All
testing is WPA2-PSK/CCMP against one AP.

                        with patch    without
  scans seeing AP       23/24         18/18
  associations          8/8           6/6
  TCP up / down         21.8 / 38.9   18.6 / 39.0 Mbit/s
  UDP up / down         25.0 / 40.0   25.0 / 40.0, 0% loss both
  rtw88 errors          0             0

The uplink figures need a caveat, because the two columns were measured
on different days and uplink on this band drifts by several Mbit/s from
one day to the next, so 21.8 against 18.6 is not a comparison I can
stand behind. I repeated it as a paired test instead: ten alternating
cycles in a single session, rebuilding and reinstalling the driver each
time, five with the full series and five with all four patches
reverted, so both see the same air.

  full series      19.0 Mbit/s mean over 10 runs, 18.7 to 19.4
  four reverted    19.2 Mbit/s mean over 10 runs, 18.9 to 19.6

There is no uplink cost. Downlink was never lower without the patches
either, 38.9 with against 39.0 and 42.9 without, so the wider spread in
the tables above is the environment rather than the code.

Then a 5 minute stability run on the ablation: ping 1495/1495 at 0%
loss, reconnects 15/15, link cycles 5/5, module reloads 3/3, scans
under traffic 5/5, idle power save wake 0% loss, 24 rekeys completed,
zero deauth, zero rqpn relatch, zero SDIO errors, no warnings, peak TX
150 Mbit/s.

The reconnects and the rekeys are the point: the join sequence ran from
mgd_prepare_tx() on every authentication, so 15 reconnects exercise
exactly the path it lived on.

Since that patch is what added the chip tests to mac80211.c, dropping
it takes that file from 8 chip tests to 0, and three of your review
points on the other mail become unnecessary rather than needing rework.

Your instinct was better than my evidence, so I applied it to the
coexistence patches as well, whose justification came from the same
period. Reverting those too, together with the GNT_BT command whose
only caller they were:

  scans 18/18, associations 6/6, TCP 18.0 up / 42.9 down, UDP 25.0 up
  and 40.0 down both at 0% loss, 0 errors. Same 5 minute soak, same
  result: 15/15 reconnects, 5/5 scans under traffic, 3/3 reloads.

So the second correction: the commit message on the scan antenna
workaround claims the site survey does not hear the AP reliably without
it. That is also wrong. It scans 18/18 without it.

One thing worth stating precisely, because it looks like a gap and is
not. Those two coex patches are gated

  if (!rtw_is_8723bs(rtwdev) || !rtwdev->coex.stat.bt_disabled)
          return false;

so they only ever execute when bt_disabled is set, which was the case
throughout. That is their entire scope, not a subset of it.

That leaves prep at six patches:

  1 add the RTL8723B chip type and SDIO helper
  2 rx: mark zero length packets on RTL8723BS
  3 fw: fix the reserved page upload on RTL8723BS
  4 sdio: track free TX pages and OQT credits for RTL8723BS
  5 sdio: set up RX aggregation and interrupts for RTL8723BS
  6 sdio: add TX back-pressure and retry on page starvation

and no chip tests left in mac80211.c or coex.c. What remains is one in
rx.c, one in fw.c and eleven in sdio.c, all of which you have already
said are fine inline.

With step 3 gone your ordering collapses to prep, then the chip driver,
then Kconfig/Makefile, then MAINTAINERS. Would you rather have that as
one series of nine patches, or keep the current split with prep sent
first? I am happy either way; one series removes the "wait for prep to
land" coordination.

One detail for the ordering: a few register defines currently sit in
the patches being dropped but are used by rtw8723b.c, so they move into
the chip support patch. BIT_AMF is used by its WLAN_RCR_CFG, and
REG_PWR_DATA, BIT_EEPRPAD_RFE_CTRL_EN, REG_RFE_CTRL_ANT_SW,
BIT_RFE_CTRL_ANT_SW_SEL, REG_RFE_CTRL_ANTA_SRC and
BIT_BT_SEL_BY_WIFI_8723B by its own coexistence code.

REG_BB_SEL_BTG_8723B goes away entirely rather than moving. Its only
users were the coex patches being dropped, and it duplicates
REG_BB_SEL_BTG in rtw8723x.h at the same address, which is the one
rtw8723b.c actually uses.

Best regards,
Luka Gejak