Re: [PATCH rtw-next v4 2/2] wifi: rtw88: support channel switch in AP mode

[Mehmet Fide <mehmet.fide@gmail.com>]

Hi Ping-Ke,

On 2026-10-07 Ping-Ke Shih wrote:
> add lockdep_assert_wiphy(wiphy) to rtw_fw_csa_{active,start,stop}?
> Since they access rtwdev->csa_vif.

Yes. Adding them showed that update_beacon_work, the TIM download,
reads csa_vif outside the wiphy lock: it is an ordinary work queued
from set_tim(). v5 makes it a wiphy work first, in a patch of its own,
the way rtw89 runs its update_beacon_work, and the three helpers then
assert the lock. The series then went through the whole AP test set
on a PROVE_LOCKING kernel (8821CU and 8822CU, including a firmware
crash during a countdown and the IPS path) without a lockdep report.

> > -                       rtw_fw_download_rsvd_page(rtwdev);
> > -                       rtw_send_rsvd_page_h2c(rtwdev);
> > +                       if (!rtw_fw_csa_active(rtwdev)) {
>
> Does it actually happen to AP mode?
>
> If it could happen, check the condition by conf->csa_active (like below)?

Not for the AP interface: mac80211 raises BSS_CHANGED_ASSOC for a
station interface only. The hunk is for the STA+AP combination the
driver registers for the 8822C, and it does happen there: with an AP
on an RTL8822CU counting down from channel 6 to 11, a station interface
on the same device that had a fresh scan result associated with an AP
on channel 11 while the countdown ran, so BSS_CHANGED_ASSOC arrived in
the middle of it. That download would rebuild the reserved page with a
fresh beacon. conf belongs to the station there, so conf->csa_active
would say nothing about the AP; that is why the check is on
rtwdev->csa_vif.

> > -       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG)
> > +       if (rtw_get_lps_deep_mode(rtwdev) == LPS_DEEP_MODE_PG &&
> > +           !rtw_fw_csa_active(rtwdev))
>
> Think of this deeper.... Is it existing race between set_key and !csa->active?

I do not see one. set_key(), the countdown work and the places that
set and clear csa_active all run under the wiphy lock, so the flag
cannot change under the check. The download set_key() skips is not
lost either: ieee80211_csa_finish() ends in ieee80211_csa_finalize(),
which assigns the next beacon with BSS_CHANGED_BEACON, and that path
rebuilds the whole page, CAM backup included, once the countdown is
over. Did you have another race in mind?

> > +       rtw_fw_csa_stop(rtwdev, rtwdev->csa_vif);
>
> Since you access rtwdev->csa_vif, should this take wiphy_lock?

It is held already: cfg80211's wiphy_suspend() calls rdev_suspend()
inside scoped_guard(wiphy, ...), and drv_suspend() asserts the lock.
The assert in rtw_fw_csa_stop() will make that visible.

> > +       mutex_lock(&rtwdev->mutex);
>
> guard(mutex)( &rtwdev->mutex); ?

Done in v5, which follows in a few days.

Thanks,
Mehmet