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

[Ping-Ke Shih]

luka.gejak@xxxxxxxxx <luka.gejak@xxxxxxxxx> wrote;
> On 27/07/2026 09:33, Ping-Ke Shih wrote:
> >> +       if (rtw_is_8723bs(rtwdev)) {
> [...]
> > I'm surprising that RTL8723BS needs so many special handing. Can you reduce
> > them? At least, use rtw_chip_prepare_tx() to implement its specific
> > functions.
> 
> Done. v3 adds a chip_ops::prepare_tx callback, and
> rtw_ops_mgd_prepare_tx() is back to a single common path:
> 
>         mutex_lock(&rtwdev->mutex);
>         rtw_leave_lps_deep(rtwdev);
>         rtw_coex_connect_notify(rtwdev, COEX_ASSOCIATE_START);
>         rtw_chip_prepare_tx(rtwdev, vif, info);
>         mutex_unlock(&rtwdev->mutex);
> 
> rtw_chip_prepare_tx() calls the chip callback when one is set and
> otherwise keeps the existing deferred RF calibration, so no other chip
> changes behaviour. The RTL8723BS callback does the IPS wake and the
> join sequence, and the chip driver in the follow-up series points
> .prepare_tx at it. That also removes the chip test from the helper
> below it, per your comments on patch 05.
> 
> 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?

>  - two dead bool arguments, rtw8723bs_auth_rx_filter(accept_all) and
>    rtw_coex_8723bs_restore_pad_ctrl(keep_pta_owner), both of which had
>    a single value at every call site
> 
> 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);

> 
> If the concern is that it sits in mac80211.c rather than a chip file, I
> agree, and I would rather move the whole rtw8723bs_* block into
> rtw8723b.c in the driver series than leave it in the common file. It is
> in mac80211.c today only because this series lands before the chip
> file exists. Tell me which you prefer and I will do it that way.

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

Ping-Ke