| Message ID | 20250203214448.1978156-3-aloka.dixit@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jeff Johnson |
| Headers | show |
| Series | wifi:ath11k/ath12k: refactor tx_arvif retrieval | expand |
On 2/4/25 03:14, Aloka Dixit wrote: > Function ath11k_mac_setup_bcn_tmpl() retrieves tx_arvif only for > a sanity check and then calls ath11k_mac_setup_bcn_tmpl_mbssid() > or ath11k_mac_setup_bcn_tmpl_ema() both of which again retrieve > the same pointer. Instead store the pointer and pass it to the > latter two functions. > Same, Is this tested? Perhaps you forgot to add "Tested-on:" tag? > Signed-off-by: Aloka Dixit <aloka.dixit@oss.qualcomm.com> > ---
On 2/3/2025 8:41 PM, Aditya Kumar Singh wrote: > On 2/4/25 03:14, Aloka Dixit wrote: >> Function ath11k_mac_setup_bcn_tmpl() retrieves tx_arvif only for >> a sanity check and then calls ath11k_mac_setup_bcn_tmpl_mbssid() >> or ath11k_mac_setup_bcn_tmpl_ema() both of which again retrieve >> the same pointer. Instead store the pointer and pass it to the >> latter two functions. >> > > Same, Is this tested? Perhaps you forgot to add "Tested-on:" tag? > >> Signed-off-by: Aloka Dixit <aloka.dixit@oss.qualcomm.com> >> --- > > No, only ath12k patches are tested hence no tag here for ath11k. This patch doesn't change handling functionally and I confirmed that all places using 'tx_arvif' first do NULL check because the refactored function can return NULL. Functions ath11k_mac_setup_bcn_tmpl_mbssid() and ath11k_mac_setup_bcn_tmpl_ema() always receive a non-NULL value now because the caller ath11k_mac_setup_bcn_tmpl() sets 'tx_arvif = arvif' whenever applicable.
On 2/4/25 23:58, Aloka Dixit wrote: > On 2/3/2025 8:41 PM, Aditya Kumar Singh wrote: >> On 2/4/25 03:14, Aloka Dixit wrote: >>> Function ath11k_mac_setup_bcn_tmpl() retrieves tx_arvif only for >>> a sanity check and then calls ath11k_mac_setup_bcn_tmpl_mbssid() >>> or ath11k_mac_setup_bcn_tmpl_ema() both of which again retrieve >>> the same pointer. Instead store the pointer and pass it to the >>> latter two functions. >>> >> >> Same, Is this tested? Perhaps you forgot to add "Tested-on:" tag? >> >>> Signed-off-by: Aloka Dixit <aloka.dixit@oss.qualcomm.com> >>> --- >> >> > > No, only ath12k patches are tested hence no tag here for ath11k. > This patch doesn't change handling functionally and I confirmed that all > places using 'tx_arvif' first do NULL check because the refactored > function can return NULL. Functions ath11k_mac_setup_bcn_tmpl_mbssid() > and ath11k_mac_setup_bcn_tmpl_ema() always receive a non-NULL value now > because the caller ath11k_mac_setup_bcn_tmpl() sets 'tx_arvif = arvif' > whenever applicable. Okay so in that case "No functionality change. Compile tested only." line should be there in commit text?
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c index 8b81f450a216..67f733aac759 100644 --- a/drivers/net/wireless/ath/ath11k/mac.c +++ b/drivers/net/wireless/ath/ath11k/mac.c @@ -1537,17 +1537,15 @@ static struct ath11k_vif *ath11k_mac_get_tx_arvif(struct ath11k_vif *arvif) return NULL; } -static int ath11k_mac_setup_bcn_tmpl_ema(struct ath11k_vif *arvif) +static int ath11k_mac_setup_bcn_tmpl_ema(struct ath11k_vif *arvif, + struct ath11k_vif *tx_arvif) { - struct ath11k_vif *tx_arvif; struct ieee80211_ema_beacons *beacons; int ret = 0; bool nontx_vif_params_set = false; u32 params = 0; u8 i = 0; - tx_arvif = ath11k_mac_get_tx_arvif(arvif); - beacons = ieee80211_beacon_get_template_ema_list(tx_arvif->ar->hw, tx_arvif->vif, 0); if (!beacons || !beacons->cnt) { @@ -1593,25 +1591,22 @@ static int ath11k_mac_setup_bcn_tmpl_ema(struct ath11k_vif *arvif) return ret; } -static int ath11k_mac_setup_bcn_tmpl_mbssid(struct ath11k_vif *arvif) +static int ath11k_mac_setup_bcn_tmpl_mbssid(struct ath11k_vif *arvif, + struct ath11k_vif *tx_arvif) { struct ath11k *ar = arvif->ar; struct ath11k_base *ab = ar->ab; - struct ath11k_vif *tx_arvif; struct ieee80211_hw *hw = ar->hw; struct ieee80211_vif *vif = arvif->vif; struct ieee80211_mutable_offsets offs = {}; struct sk_buff *bcn; int ret; - tx_arvif = ath11k_mac_get_tx_arvif(arvif); - if (tx_arvif && tx_arvif != arvif) { + if (tx_arvif != arvif) { ar = tx_arvif->ar; ab = ar->ab; hw = ar->hw; vif = tx_arvif->vif; - } else { - tx_arvif = arvif; } bcn = ieee80211_beacon_get_template(hw, vif, &offs, 0); @@ -1640,6 +1635,7 @@ static int ath11k_mac_setup_bcn_tmpl_mbssid(struct ath11k_vif *arvif) static int ath11k_mac_setup_bcn_tmpl(struct ath11k_vif *arvif) { struct ieee80211_vif *vif = arvif->vif; + struct ath11k_vif *tx_arvif; if (arvif->vdev_type != WMI_VDEV_TYPE_AP) return 0; @@ -1647,14 +1643,18 @@ static int ath11k_mac_setup_bcn_tmpl(struct ath11k_vif *arvif) /* Target does not expect beacon templates for the already up * non-transmitting interfaces, and results in a crash if sent. */ - if (vif->mbssid_tx_vif && - arvif != ath11k_vif_to_arvif(vif->mbssid_tx_vif) && arvif->is_up) - return 0; + tx_arvif = ath11k_mac_get_tx_arvif(arvif); + if (tx_arvif) { + if (arvif != tx_arvif && arvif->is_up) + return 0; - if (vif->bss_conf.ema_ap && vif->mbssid_tx_vif) - return ath11k_mac_setup_bcn_tmpl_ema(arvif); + if (vif->bss_conf.ema_ap) + return ath11k_mac_setup_bcn_tmpl_ema(arvif, tx_arvif); + } else { + tx_arvif = arvif; + } - return ath11k_mac_setup_bcn_tmpl_mbssid(arvif); + return ath11k_mac_setup_bcn_tmpl_mbssid(arvif, tx_arvif); } void ath11k_mac_bcn_tx_event(struct ath11k_vif *arvif)
Function ath11k_mac_setup_bcn_tmpl() retrieves tx_arvif only for a sanity check and then calls ath11k_mac_setup_bcn_tmpl_mbssid() or ath11k_mac_setup_bcn_tmpl_ema() both of which again retrieve the same pointer. Instead store the pointer and pass it to the latter two functions. Signed-off-by: Aloka Dixit <aloka.dixit@oss.qualcomm.com> --- drivers/net/wireless/ath/ath11k/mac.c | 32 +++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-)