RE: [PATCH v2 1/6] wifi: rtw89: 8922a: configure AP_LINK_PS if FW supports

[Ping-Ke Shih <pkshih@realtek.com>]

Kalle Valo <kvalo@kernel.org> wrote:
> Ping-Ke Shih <pkshih@realtek.com> writes:
> 
> > From: Zong-Zhe Yang <kevin_yang@realtek.com>
> >
> > After FW v0.35.46.0, for AP mode, RTL8922A FW supports a new FW feature,
> > called NOTIFY_AP_INFO, to notify driver information related to AP mode.
> > And, one function of it is to monitor PS states of remote stations. Once
> > one of them changes, FW will send a C2H event to tell driver. With this
> > FW feature, we can declare AP_LINK_PS.
> >
> > For now, driver still needs to determine if a frame is ps-poll or U-APSD
> > trigger. So, add the corresponding RX handling in driver, which activates
> > only when at least one AP is running.
> >
> > Signed-off-by: Zong-Zhe Yang <kevin_yang@realtek.com>
> > Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> 
> [...]
> 
> > +static inline void rtw89_assoc_link_clr(struct rtw89_sta_link *rtwsta_link)
> > +{
> > +     struct rtw89_sta *rtwsta = rtwsta_link->rtwsta;
> > +     struct rtw89_dev *rtwdev = rtwsta->rtwdev;
> > +
> > +     rcu_assign_pointer(rtwdev->assoc_link_on_macid[rtwsta_link->mac_id],
> > +                        NULL);
> > +     synchronize_rcu();
> > +}
> 
> In ath12k patches I got feedback that synchronize_rcu() should not be
> used unless we explicitly need it, for example if if we free something
> or similar. Just wanted to mention this, up to you if you want to keep
> it or not.

The 'rtwdev->assoc_link_on_macid[rtwsta_link->mac_id]' is to store pointer of
rtwsta that is drv_priv [] of ieee80211_sta. After returning from this
function (disassoc), ieee80211_sta is going to be freed by kfree, no other
synchronize_rcu(), so I think this is necessary here. 

Thanks for your reminder.