RE: [PATCH v2 01/11] wifi: rtw88: add the RTL8723B chip type and SDIO helper

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

On July 31, 2026 3:29:40 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
>
>Luka Gejak <luka.gejak@linux.dev> wrote:
>> On July 30, 2026 11:17:41 AM GMT+02:00, Ping-Ke Shih <pkshih@realtek.com> wrote:
>> >luka.gejak@linux.dev <luka.gejak@linux.dev> wrote:
>> >> it would be four callbacks (free page check, page
>> >> accounting, output queue wait, transfer sizing) and I did not want to
>> >> introduce that structure without asking first.
>> > If these are called from sdio.c, just check chip ID in sdio.c.
>>
>> That is what v3 already does, so nothing changes there. All four are
>> reached from sdio.c only.
>>
>> > If they are used by common flow like coex.c, I prefer to implement
>> > them by chip_ops.
>>
>> Let me list them all so you can rule on all of them at once rather than
>> a file at a time.
>>
>> Two go regardless of your answer: the chip test inside
>> rtw8723bs_apply_basic_rates() and rtw8723bs_apply_bss_cap() is
>> redundant, both callers already establish the chip. In the second one
>> the NL80211_IFTYPE_STATION half of the same guard is still needed, so
>> only the chip half goes there.
>>
>> That leaves the following outside sdio.c.
>>
>> coex.c, 2 sites:
>>
>>   rtw_coex_scan_notify()    -> rtw_coex_8723bs_scan_notify()
>>   rtw_coex_connect_notify() -> rtw_coex_8723bs_connect_notify()
>>
>>   Each tests the chip and bt_disabled once and returns true when it has
>>   handled the notification; everything below them assumes both.
>>
>> mac80211.c, 6 sites in rtw_ops_bss_info_changed(): the receive filter

One piece of context first, because it changes most of the answers
below. I ran the ablation you asked for in your other mail, and the
association register sequence is not needed, nor are the two
coexistence patches. Four patches are going from prep, which takes it
from ten to six. The measurements and the two corrections I owe you are
in my reply to that mail.

> Should receive filter implement in rtw_ops_configure_filter()?

Moot now, the receive filter handling goes with the patch I am
dropping. For the record you were pointing at something real, although
not quite where I first placed it.

rtw8723b.c does seed hal.rcr in mac_init, so it is not the problem. The
asymmetry is in rtw_ops_configure_filter() itself: entering promiscuous
clears BIT_CBSSID_BCN and BIT_CBSSID_DATA together, but leaving it
restores only BIT_CBSSID_BCN. CBSSID_DATA is therefore gone from
hal.rcr for good after the first scan, on any chip, until the next
mac_init. That is what the per-association forcing was compensating
for.

There is a second one next to it: rtw8723x.c writes WLAN_RCR_CFG
straight to REG_RCR in mac_init without putting it into hal.rcr, so on
8723D and 8703B the first configure_filter() call replaces it with the
core default from rtw_core_init(), which carries neither CBSSID bit nor
AMF.

Both look worth fixing on their own and I am happy to send that
separately if you agree with the reading. It is unrelated to this
series.

> Please check existing codes related to sec->default_key_search to see
> how to support RTL8723BS by the same flow.

You were right, and more strongly than you put it.
rtw_sec_enable_sec_engine() sets default_key_search itself and then
programs all four USE_DK bits on that basis, so my enable path was a
no-op and my disable path was clearing bits the core had deliberately
set. Both sites are gone with the patch, and 24 rekeys completed in the
soak without them.

> I don't object this. More, I'm considering if no need to check chips,
> just be a common flow.

Happy to try that. Dropping the chip test would make the zero length
check apply to every chip, so I would rather measure it on the 8703B
path than assume; I do not have that hardware. If you are comfortable
with it on inspection I will send it as a separate patch that removes
the test entirely rather than fold it in here.

> That looks fine. I remember I don't have comments on this part, no?

Correct, you had no comments on the reserved page patch itself, only on
the DWBCN1 define placement and the comment style, both fixed. The
inline chip test there stays.

> Did you mean callees of rtw_coex_8723bs_scan_workaround() ?
> I think you can go with your mind.

Yes, those were the ones, and thank you, but it is moot now that the
coexistence patches are going as well.

> I'd say the version copy many stuffs from vendor driver. However, we
> should rewrite and consider the proper places, and if it is actually
> necessary. The vendor driver is based on cfg80211, and many stuffs
> have been done by mac80211, so we don't need to implement them in
> rtw88.

That turned out to be exactly right. Four of the ten patches were
reproducing vendor behaviour that mac80211 or the core already handles,
and they only survived this long because I kept quoting bring-up
results instead of re-measuring them. Thanks for pushing on it twice.

Best regards,
Luka Gejak