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

On July 30, 2026 11:17:41 AM GMT+02:00, Ping-Ke Shih <[email protected]> wrote:
>[email protected] <[email protected]> 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
on assoc and on disassoc, BSSID change bookkeeping, and the
BSS_CHANGED_ERP_PREAMBLE and BSS_CHANGED_ERP_SLOT handlers.

mac80211.c, 2 sites in rtw_ops_set_key(): enabling and disabling
default key search for group keys.

rx.c, 1 site in rtw_rx_fill_rx_status(), the zero length packet test.
This one runs per received frame, so an indirect call there looks like
the wrong trade to me. I would keep it inline unless you disagree.

fw.c, 1 site in rtw_fw_write_data_rsvd_page(), two register
save/restore blocks interleaved with the generic ones. I do not see how
to lift those into an op without restructuring the whole function, so I
would keep that inline too.

For coex.c and the two mac80211.c callbacks I am happy to add ops. One
constraint on coex.c: the 8723BS code calls rtw_coex_set_ant_path() and
rtw_coex_set_table(), which are static in coex.c. Implementing the ops
in rtw8723b.c would mean making those two non-static, which seems worse
than the chip test it replaces. What I had in mind is leaving the
functions in coex.c, declaring them in coex.h, and having rtw8723b.c
point the ops at them, the same shape as the chip_ops::prepare_tx that
v3 adds for the association sequence. The generic helpers stay static
that way.

Tell me which of those you want as ops and I will do it.

The comment on mod_delayed_work() is already in v3.

Best regards,
Luka Gejak