[PATCH v4 00/12] wifi: ath12k: Add single wiphy support

[PATCH v4 00/12] wifi: ath12k: Add single wiphy support

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

With the introduction of Multi Link Operation (MLO) support in
IEEE802.11be, each EHT AP/non AP interface is capable of
operating with multiple radio links.

cfg80211/mac80211 expects drivers to abstract the communication
between such Multi Link HW and mac80211/cfg80211 since it depends
on different driver/HW implementation. Hence the single wiphy
abstraction with changes in datastructures were introduced in
"wifi: ath12k: Introduce hw abstraction"

This patchset extends the implementation to allow combination
of multiple underlying radios into a single composite hw/wiphy
for registration. Since now multiple radios are represented by
a single wiphy, changes are required in various mac ops that the
driver supports since the driver now needs to learn on how to tunnel
various mac ops properly to a specific radio.

This patchset covers the basic mac80211 ops for an interface bring up
and operation.

Note:
Monitor and hw reconfig support for Single Wiphy will be done in future
patchsets.

---
 v4:
 - Updated missing Signed-off details for patches.

 v3:
  - Rebased on ToT (added additional ar check in PATCH 08/12 for p2p)
  - Introduced iterator to loop through ars in an ah(for_each_ar())
  - Addressed comments on reverse xmas tree declaration style.

 v2:
  - Rebased on ToT and dependent patchset


Karthikeyan Periyasamy (1):
  wifi: ath12k: add multiple radio support in a single MAC HW
    un/register

Sriram R (11):
  wifi: ath12k: Modify add and remove chanctx ops for single wiphy
    support
  wifi: ath12k: modify ath12k mac start/stop ops for single wiphy
  wifi: ath12k: vdev statemachine changes for single wiphy
  wifi: ath12k: scan statemachine changes for single wiphy
  wifi: ath12k: fetch correct radio based on vdev status
  wifi: ath12k: Cache vdev configs before vdev create
  wifi: ath12k: Add additional checks for vif and sta iterators
  wifi: ath12k: modify regulatory support for single wiphy architecture
  wifi: ath12k: Modify set and get antenna mac ops for single wiphy
  wifi: ath12k: Modify rts threshold mac op for single wiphy
  wifi: ath12k: support get_survey mac op for single wiphy

 drivers/net/wireless/ath/ath12k/core.h |  38 +-
 drivers/net/wireless/ath/ath12k/hw.h   |   1 +
 drivers/net/wireless/ath/ath12k/mac.c  | 911 +++++++++++++++++++------
 drivers/net/wireless/ath/ath12k/p2p.c  |   3 +-
 drivers/net/wireless/ath/ath12k/p2p.h  |   1 +
 drivers/net/wireless/ath/ath12k/reg.c  |  55 +-
 6 files changed, 785 insertions(+), 224 deletions(-)

-- 
2.25.1

[PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Rameshkumar Sundaram]

From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>

Currently MAC HW un/register helper function support the single radio.
To enable single/multi link operation in the future, the following helper
functions need to be refactored to accommodate multiple radios under a
single MAC HW un/register:

	* ath12k_ah_to_ar()
	* ath12k_mac_hw_allocate()
	* ath12k_mac_hw_register()
	* ath12k_mac_hw_unregister()

This refactoring will make it easier to scale these functionalities and
support Multi link operation.

                         Current Multi wiphy Model

 +---------------+          +---------------+            +---------------+
 |  Mac80211 hw  |          | Mac80211 hw   |            | Mac80211 hw   |
 |  private data |          | private data  |            | private data  |
 |               |          |               |            |               |
 |ath12k_hw (ah) |          |ath12k_hw (ah) |            |ath12k_hw (ah) |
 |               |          |               |            |               |
 | +-----------+ |          | +-----------+ |            | +-----------+ |
 | | ar (2GHz) | |          | | ar (5GHz) | |            | | ar (6GHz) | |
 | +-----------+ |          | +-----------+ |            | +-----------+ |
 |               |          |               |            |               |
 +---------------+          +---------------+            +---------------+

                           Single wiphy Model

                            +--------------+
                            | Mac80211 hw  |
                            | private data |
                            |              |
                            |ath12k hw (ah)|
                            | +----------+ |
                            | |ar (2GHz) | |
                            | +----------+ |
                            | |ar (5GHz) | |
                            | +----------+ |
                            | |ar (6GHz) | |
                            | +----------+ |
                            +--------------+

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  12 +-
 drivers/net/wireless/ath/ath12k/mac.c  | 184 ++++++++++++++++---------
 drivers/net/wireless/ath/ath12k/reg.c  |   2 +-
 3 files changed, 127 insertions(+), 71 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 97e5a0ccd233..ff831faa4945 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -951,13 +951,21 @@ static inline struct ath12k_hw *ath12k_hw_to_ah(struct ieee80211_hw  *hw)
 	return hw->priv;
 }
 
-static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah)
+static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah, u8 hw_link_id)
 {
-	return ah->radio;
+	if (WARN(hw_link_id >= ah->num_radio,
+		 "bad hw link id %d, so switch to default link\n", hw_link_id))
+		hw_link_id = 0;
+
+	return &ah->radio[hw_link_id];
 }
 
 static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
 {
 	return ar->ah->hw;
 }
+
+#define for_each_ar(index, ah, ar) \
+	for ((index) = 0; ((index) < (ah)->num_radio && \
+	     ((ar) = &(ah)->radio[(index)])); (index)++)
 #endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 52a5fb8b03e9..ba69fdfa9133 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -1169,7 +1169,7 @@ static int ath12k_mac_op_config(struct ieee80211_hw *hw, u32 changed)
 	struct ath12k *ar;
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	ret = ath12k_mac_config(ar, changed);
 	if (ret)
@@ -2970,7 +2970,7 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -3155,7 +3155,7 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 	int ret;
 	int i;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -3245,7 +3245,7 @@ static void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 	ath12k_scan_abort(ar);
@@ -3390,7 +3390,7 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
 		return 1;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags))
@@ -3980,7 +3980,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 	     new_state == IEEE80211_STA_NOTEXIST))
 		cancel_work_sync(&arsta->update_wk);
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -4109,7 +4109,7 @@ static int ath12k_mac_op_sta_set_txpwr(struct ieee80211_hw *hw,
 	if (txpwr > ATH12K_TX_POWER_MAX_VAL || txpwr < ATH12K_TX_POWER_MIN_VAL)
 		return -EINVAL;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -4138,7 +4138,7 @@ static void ath12k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
 	struct ath12k_peer *peer;
 	u32 bw, smps;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	spin_lock_bh(&ar->ab->base_lock);
 
@@ -4319,7 +4319,7 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_conf_tx(arvif, link_id, ac, params);
@@ -5446,7 +5446,7 @@ static int ath12k_mac_start(struct ath12k *ar)
 static int ath12k_mac_op_start(struct ieee80211_hw *hw)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 	struct ath12k_base *ab = ar->ab;
 	int ret;
 
@@ -5555,7 +5555,7 @@ static void ath12k_mac_stop(struct ath12k *ar)
 static void ath12k_mac_op_stop(struct ieee80211_hw *hw)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 
 	ath12k_mac_drain_tx(ar);
 
@@ -5749,7 +5749,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 
 	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6017,7 +6017,7 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 	unsigned long time_left;
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6132,7 +6132,7 @@ static void ath12k_mac_op_configure_filter(struct ieee80211_hw *hw,
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -6147,7 +6147,7 @@ static int ath12k_mac_op_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -6165,7 +6165,7 @@ static int ath12k_mac_op_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx
 	struct ath12k *ar;
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 	ret = __ath12k_set_antenna(ar, tx_ant, rx_ant);
@@ -6213,7 +6213,7 @@ static int ath12k_mac_op_ampdu_action(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret = -EINVAL;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_ampdu_action(arvif, params);
@@ -6233,7 +6233,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
@@ -6261,7 +6261,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
@@ -6641,7 +6641,7 @@ static void ath12k_mac_op_change_chanctx(struct ieee80211_hw *hw,
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6712,7 +6712,7 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 	int ret;
 	struct ath12k_wmi_peer_create_arg param;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6794,7 +6794,7 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6849,7 +6849,7 @@ ath12k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -6895,7 +6895,7 @@ static int ath12k_mac_op_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 	struct ath12k *ar;
 	int param_id = WMI_VDEV_PARAM_RTS_THRESHOLD, ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	ret = ath12k_set_vdev_param_to_all_vifs(ar, param_id, value);
 
@@ -6939,7 +6939,7 @@ static void ath12k_mac_op_flush(struct ieee80211_hw *hw, struct ieee80211_vif *v
 				u32 queues, bool drop)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 
 	if (drop)
 		return;
@@ -7306,7 +7306,7 @@ ath12k_mac_op_reconfig_complete(struct ieee80211_hw *hw,
 	if (reconfig_type != IEEE80211_RECONFIG_TYPE_RESTART)
 		return;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -7401,7 +7401,7 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 	if (idx >= ATH12K_NUM_CHANS)
 		return -ENOENT;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	ar_survey = &ar->survey[idx];
 
@@ -7478,7 +7478,7 @@ static int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -7508,7 +7508,7 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	u32 scan_time_msec;
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah);
+	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
 	spin_lock_bh(&ar->data_lock);
@@ -7757,10 +7757,12 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 
 static u16 ath12k_mac_get_ifmodes(struct ath12k_hw *ah)
 {
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar;
+	int i;
 	u16 interface_modes = U16_MAX;
 
-	interface_modes &= ar->ab->hw_params->interface_modes;
+	for_each_ar(i, ah, ar)
+		interface_modes &= ar->ab->hw_params->interface_modes;
 
 	return interface_modes == U16_MAX ? 0 : interface_modes;
 }
@@ -7768,15 +7770,19 @@ static u16 ath12k_mac_get_ifmodes(struct ath12k_hw *ah)
 static bool ath12k_mac_is_iface_mode_enable(struct ath12k_hw *ah,
 					    enum nl80211_iftype type)
 {
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar;
+	int i;
 	u16 interface_modes, mode;
 	bool is_enable = true;
 
 	mode = BIT(type);
-
-	interface_modes = ar->ab->hw_params->interface_modes;
-	if (!(interface_modes & mode))
-		is_enable = false;
+	for_each_ar(i, ah, ar) {
+		interface_modes = ar->ab->hw_params->interface_modes;
+		if (!(interface_modes & mode)) {
+			is_enable = false;
+			break;
+		}
+	}
 
 	return is_enable;
 }
@@ -7906,13 +7912,16 @@ static void ath12k_mac_hw_unregister(struct ath12k_hw *ah)
 {
 	struct ieee80211_hw *hw = ah->hw;
 	struct wiphy *wiphy = hw->wiphy;
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar;
+	int i;
 
-	cancel_work_sync(&ar->regd_update_work);
+	for_each_ar(i, ah, ar)
+		cancel_work_sync(&ar->regd_update_work);
 
 	ieee80211_unregister_hw(hw);
 
-	ath12k_mac_cleanup_unregister(ar);
+	for_each_ar(i, ah, ar)
+		ath12k_mac_cleanup_unregister(ar);
 
 	kfree(wiphy->iface_combinations[0].limits);
 	kfree(wiphy->iface_combinations);
@@ -7952,7 +7961,7 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 {
 	struct ieee80211_hw *hw = ah->hw;
 	struct wiphy *wiphy = hw->wiphy;
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 	struct ath12k_base *ab = ar->ab;
 	struct ath12k_pdev *pdev;
 	struct ath12k_pdev_cap *cap;
@@ -7967,39 +7976,71 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 		WLAN_CIPHER_SUITE_GCMP_256,
 		WLAN_CIPHER_SUITE_CCMP_256,
 	};
-	int ret;
-	u32 ht_cap = 0;
+	int ret, i, j;
+	u32 ht_cap = U32_MAX, antennas_rx = 0, antennas_tx = 0;
+	bool is_6ghz = false, is_raw_mode = false, is_monitor_disable = false;
+	u8 *mac_addr = NULL;
 
-	pdev = ar->pdev;
+	wiphy->max_ap_assoc_sta = 0;
 
-	if (ab->pdevs_macaddr_valid)
-		ether_addr_copy(ar->mac_addr, pdev->mac_addr);
-	else
-		ether_addr_copy(ar->mac_addr, ab->mac_addr);
+	for_each_ar(i, ah, ar) {
+		u32 ht_cap_info = 0;
+		pdev = ar->pdev;
 
-	ret = ath12k_mac_setup_register(ar, &ht_cap, hw->wiphy->bands);
-	if (ret)
-		goto out;
+		if (ar->ab->pdevs_macaddr_valid) {
+			ether_addr_copy(ar->mac_addr, pdev->mac_addr);
+		} else {
+			ether_addr_copy(ar->mac_addr, ar->ab->mac_addr);
+			ar->mac_addr[4] += i;
+		}
+
+		ret = ath12k_mac_setup_register(ar, &ht_cap_info, hw->wiphy->bands);
+		if (ret)
+			goto err_cleanup_unregister;
 
-	wiphy->max_ap_assoc_sta = ar->max_num_stations;
+		ht_cap &= ht_cap_info;
+		wiphy->max_ap_assoc_sta += ar->max_num_stations;
+
+		/* Advertise the max antenna support of all radios, driver can handle
+		 * per pdev specific antenna setting based on pdev cap when antenna
+		 * changes are made
+		 */
+		cap = &pdev->cap;
 
-	cap = &pdev->cap;
+		antennas_rx = max_t(u32, antennas_rx, cap->rx_chain_mask);
+		antennas_tx = max_t(u32, antennas_tx, cap->tx_chain_mask);
 
-	wiphy->available_antennas_rx = cap->rx_chain_mask;
-	wiphy->available_antennas_tx = cap->tx_chain_mask;
+		if (ar->supports_6ghz)
+			is_6ghz = true;
 
-	SET_IEEE80211_PERM_ADDR(hw, ar->mac_addr);
+		if (test_bit(ATH12K_FLAG_RAW_MODE, &ar->ab->dev_flags))
+			is_raw_mode = true;
+
+		if (!ar->ab->hw_params->supports_monitor)
+			is_monitor_disable = true;
+
+		if (i == 0)
+			mac_addr = ar->mac_addr;
+		else
+			mac_addr = ab->mac_addr;
+	}
+
+	wiphy->available_antennas_rx = antennas_rx;
+	wiphy->available_antennas_tx = antennas_tx;
+
+	SET_IEEE80211_PERM_ADDR(hw, mac_addr);
 	SET_IEEE80211_DEV(hw, ab->dev);
 
 	ret = ath12k_mac_setup_iface_combinations(ah);
 	if (ret) {
 		ath12k_err(ab, "failed to setup interface combinations: %d\n", ret);
-		goto err_cleanup_unregister;
+		goto err_complete_cleanup_unregister;
 	}
 
 	wiphy->interface_modes = ath12k_mac_get_ifmodes(ah);
 
-	if (wiphy->bands[NL80211_BAND_2GHZ] &&
+	if (ah->num_radio == 1 &&
+	    wiphy->bands[NL80211_BAND_2GHZ] &&
 	    wiphy->bands[NL80211_BAND_5GHZ] &&
 	    wiphy->bands[NL80211_BAND_6GHZ])
 		ieee80211_hw_set(hw, SINGLE_SCAN_ON_ALL_BANDS);
@@ -8067,7 +8108,7 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 	wiphy->iftype_ext_capab = ath12k_iftypes_ext_capa;
 	wiphy->num_iftype_ext_capab = ARRAY_SIZE(ath12k_iftypes_ext_capa);
 
-	if (ar->supports_6ghz) {
+	if (is_6ghz) {
 		wiphy_ext_feature_set(wiphy,
 				      NL80211_EXT_FEATURE_FILS_DISCOVERY);
 		wiphy_ext_feature_set(wiphy,
@@ -8078,7 +8119,7 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 
 	ath12k_reg_init(hw);
 
-	if (!test_bit(ATH12K_FLAG_RAW_MODE, &ab->dev_flags)) {
+	if (!is_raw_mode) {
 		hw->netdev_features = NETIF_F_HW_CSUM;
 		ieee80211_hw_set(hw, SW_CRYPTO_CONTROL);
 		ieee80211_hw_set(hw, SUPPORT_FAST_XMIT);
@@ -8090,7 +8131,7 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 		goto err_free_if_combs;
 	}
 
-	if (!ab->hw_params->supports_monitor)
+	if (is_monitor_disable)
 		/* There's a race between calling ieee80211_register_hw()
 		 * and here where the monitor mode is enabled for a little
 		 * while. But that time is so short and in practise it make
@@ -8098,11 +8139,13 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 		 */
 		wiphy->interface_modes &= ~BIT(NL80211_IFTYPE_MONITOR);
 
-	/* Apply the regd received during initialization */
-	ret = ath12k_regd_update(ar, true);
-	if (ret) {
-		ath12k_err(ar->ab, "ath12k regd update failed: %d\n", ret);
-		goto err_unregister_hw;
+	for_each_ar(i, ah, ar) {
+		/* Apply the regd received during initialization */
+		ret = ath12k_regd_update(ar, true);
+		if (ret) {
+			ath12k_err(ar->ab, "ath12k regd update failed: %d\n", ret);
+			goto err_unregister_hw;
+		}
 	}
 
 	return 0;
@@ -8114,10 +8157,15 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 	kfree(wiphy->iface_combinations[0].limits);
 	kfree(wiphy->iface_combinations);
 
+err_complete_cleanup_unregister:
+	i = ah->num_radio;
+
 err_cleanup_unregister:
-	ath12k_mac_cleanup_unregister(ar);
+	for (j = 0; j < i; j++) {
+		ar = ath12k_ah_to_ar(ah, j);
+		ath12k_mac_cleanup_unregister(ar);
+	}
 
-out:
 	SET_IEEE80211_DEV(hw, NULL);
 
 	return ret;
@@ -8243,7 +8291,7 @@ static struct ath12k_hw *ath12k_mac_hw_allocate(struct ath12k_base *ab,
 		pdev_idx = pdev_map[i].pdev_idx;
 		pdev = &ab->pdevs[pdev_idx];
 
-		ar = ath12k_ah_to_ar(ah);
+		ar = ath12k_ah_to_ar(ah, i);
 		ar->ah = ah;
 		ar->ab = ab;
 		ar->hw_link_id = i;
diff --git a/drivers/net/wireless/ath/ath12k/reg.c b/drivers/net/wireless/ath/ath12k/reg.c
index f308e9a6ed55..e73b716a2d80 100644
--- a/drivers/net/wireless/ath/ath12k/reg.c
+++ b/drivers/net/wireless/ath/ath12k/reg.c
@@ -49,7 +49,7 @@ ath12k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
 	struct ieee80211_hw *hw = wiphy_to_ieee80211_hw(wiphy);
 	struct ath12k_wmi_init_country_arg arg;
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah);
+	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 	int ret;
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_REG,
-- 
2.25.1

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> 
> Currently MAC HW un/register helper function support the single radio.
> To enable single/multi link operation in the future, the following helper
> functions need to be refactored to accommodate multiple radios under a
> single MAC HW un/register:
> 
> 	* ath12k_ah_to_ar()
> 	* ath12k_mac_hw_allocate()
> 	* ath12k_mac_hw_register()
> 	* ath12k_mac_hw_unregister()
> 
> This refactoring will make it easier to scale these functionalities and
> support Multi link operation.
> 
>                          Current Multi wiphy Model
> 
>  +---------------+          +---------------+            +---------------+
>  |  Mac80211 hw  |          | Mac80211 hw   |            | Mac80211 hw   |
>  |  private data |          | private data  |            | private data  |
>  |               |          |               |            |               |
>  |ath12k_hw (ah) |          |ath12k_hw (ah) |            |ath12k_hw (ah) |
>  |               |          |               |            |               |
>  | +-----------+ |          | +-----------+ |            | +-----------+ |
>  | | ar (2GHz) | |          | | ar (5GHz) | |            | | ar (6GHz) | |
>  | +-----------+ |          | +-----------+ |            | +-----------+ |
>  |               |          |               |            |               |
>  +---------------+          +---------------+            +---------------+
> 
>                            Single wiphy Model
> 
>                             +--------------+
>                             | Mac80211 hw  |
>                             | private data |
>                             |              |
>                             |ath12k hw (ah)|
>                             | +----------+ |
>                             | |ar (2GHz) | |
>                             | +----------+ |
>                             | |ar (5GHz) | |
>                             | +----------+ |
>                             | |ar (6GHz) | |
>                             | +----------+ |
>                             +--------------+
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.h |  12 +-
>  drivers/net/wireless/ath/ath12k/mac.c  | 184 ++++++++++++++++---------
>  drivers/net/wireless/ath/ath12k/reg.c  |   2 +-
>  3 files changed, 127 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 97e5a0ccd233..ff831faa4945 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -951,13 +951,21 @@ static inline struct ath12k_hw *ath12k_hw_to_ah(struct ieee80211_hw  *hw)
>  	return hw->priv;
>  }
>  
> -static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah)
> +static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah, u8 hw_link_id)
>  {
> -	return ah->radio;
> +	if (WARN(hw_link_id >= ah->num_radio,
> +		 "bad hw link id %d, so switch to default link\n", hw_link_id))
> +		hw_link_id = 0;
> +
> +	return &ah->radio[hw_link_id];
>  }
>  
>  static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
>  {
>  	return ar->ah->hw;
>  }
> +
> +#define for_each_ar(index, ah, ar) \
> +	for ((index) = 0; ((index) < (ah)->num_radio && \
> +	     ((ar) = &(ah)->radio[(index)])); (index)++)


this seems like logically the wrong order of operands
this is an operation on the ah object so IMO that should be first
the actual iterators i and ar should follow that

and guess we have to figure out how to suppress the ath12k-check issues with
this macro

that's my only comments on this patch.

/jeff

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Rameshkumar Sundaram]

On 3/13/2024 3:23 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>>
>> Currently MAC HW un/register helper function support the single radio.
>> To enable single/multi link operation in the future, the following helper
>> functions need to be refactored to accommodate multiple radios under a
>> single MAC HW un/register:
>>
>> 	* ath12k_ah_to_ar()
>> 	* ath12k_mac_hw_allocate()
>> 	* ath12k_mac_hw_register()
>> 	* ath12k_mac_hw_unregister()
>>
>> This refactoring will make it easier to scale these functionalities and
>> support Multi link operation.
>>
>>                           Current Multi wiphy Model
>>
>>   +---------------+          +---------------+            +---------------+
>>   |  Mac80211 hw  |          | Mac80211 hw   |            | Mac80211 hw   |
>>   |  private data |          | private data  |            | private data  |
>>   |               |          |               |            |               |
>>   |ath12k_hw (ah) |          |ath12k_hw (ah) |            |ath12k_hw (ah) |
>>   |               |          |               |            |               |
>>   | +-----------+ |          | +-----------+ |            | +-----------+ |
>>   | | ar (2GHz) | |          | | ar (5GHz) | |            | | ar (6GHz) | |
>>   | +-----------+ |          | +-----------+ |            | +-----------+ |
>>   |               |          |               |            |               |
>>   +---------------+          +---------------+            +---------------+
>>
>>                             Single wiphy Model
>>
>>                              +--------------+
>>                              | Mac80211 hw  |
>>                              | private data |
>>                              |              |
>>                              |ath12k hw (ah)|
>>                              | +----------+ |
>>                              | |ar (2GHz) | |
>>                              | +----------+ |
>>                              | |ar (5GHz) | |
>>                              | +----------+ |
>>                              | |ar (6GHz) | |
>>                              | +----------+ |
>>                              +--------------+
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Karthikeyan Periyasamy <quic_periyasa@quicinc.com>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.h |  12 +-
>>   drivers/net/wireless/ath/ath12k/mac.c  | 184 ++++++++++++++++---------
>>   drivers/net/wireless/ath/ath12k/reg.c  |   2 +-
>>   3 files changed, 127 insertions(+), 71 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 97e5a0ccd233..ff831faa4945 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -951,13 +951,21 @@ static inline struct ath12k_hw *ath12k_hw_to_ah(struct ieee80211_hw  *hw)
>>   	return hw->priv;
>>   }
>>   
>> -static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah)
>> +static inline struct ath12k *ath12k_ah_to_ar(struct ath12k_hw *ah, u8 hw_link_id)
>>   {
>> -	return ah->radio;
>> +	if (WARN(hw_link_id >= ah->num_radio,
>> +		 "bad hw link id %d, so switch to default link\n", hw_link_id))
>> +		hw_link_id = 0;
>> +
>> +	return &ah->radio[hw_link_id];
>>   }
>>   
>>   static inline struct ieee80211_hw *ath12k_ar_to_hw(struct ath12k *ar)
>>   {
>>   	return ar->ah->hw;
>>   }
>> +
>> +#define for_each_ar(index, ah, ar) \
>> +	for ((index) = 0; ((index) < (ah)->num_radio && \
>> +	     ((ar) = &(ah)->radio[(index)])); (index)++)
> 
> 
> this seems like logically the wrong order of operands
> this is an operation on the ah object so IMO that should be first
> the actual iterators i and ar should follow that
> 
Sure Jeff, will rearrange the operands as suggested.
> and guess we have to figure out how to suppress the ath12k-check issues with
> this macro
ath12k-check complains about the reuse of ah and index arguments which 
may get evaluated multiple times if its an arithmetic expression, But 
areas where we use the macro in our code aren't doing so.
Do you have any suggestions here ? or shall we go back and use this 
for-loop inline.
> 
> that's my only comments on this patch.
> 
> /jeff

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Jeff Johnson]

On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>> and guess we have to figure out how to suppress the ath12k-check issues with
>> this macro
> ath12k-check complains about the reuse of ah and index arguments which 
> may get evaluated multiple times if its an arithmetic expression, But 
> areas where we use the macro in our code aren't doing so.
> Do you have any suggestions here ? or shall we go back and use this 
> for-loop inline.

The macro makes sense -- we'll need to update the overrides in ath12k-check.

/jeff

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Kalle Valo]

Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>> this macro
>> ath12k-check complains about the reuse of ah and index arguments which 
>> may get evaluated multiple times if its an arithmetic expression, But 
>> areas where we use the macro in our code aren't doing so.
>> Do you have any suggestions here ? or shall we go back and use this 
>> for-loop inline.
>
> The macro makes sense -- we'll need to update the overrides in ath12k-check.

IIRC it is possible to avoid variable reuse in macros with typeof()
operator (or something like that). I can't remember the details right
now but I think there are examples in the kernel code.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Kalle Valo]

Kalle Valo <kvalo@kernel.org> writes:

> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>
>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>> this macro
>>> ath12k-check complains about the reuse of ah and index arguments which 
>>> may get evaluated multiple times if its an arithmetic expression, But 
>>> areas where we use the macro in our code aren't doing so.
>>> Do you have any suggestions here ? or shall we go back and use this 
>>> for-loop inline.
>>
>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>
> IIRC it is possible to avoid variable reuse in macros with typeof()
> operator (or something like that). I can't remember the details right
> now but I think there are examples in the kernel code.

Here's the GCC documentation with an example:

https://gcc.gnu.org/onlinedocs/gcc/Typeof.html

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Jeff Johnson]

On 3/13/2024 9:58 AM, Kalle Valo wrote:
> Kalle Valo <kvalo@kernel.org> writes:
> 
>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>
>>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>>> this macro
>>>> ath12k-check complains about the reuse of ah and index arguments which 
>>>> may get evaluated multiple times if its an arithmetic expression, But 
>>>> areas where we use the macro in our code aren't doing so.
>>>> Do you have any suggestions here ? or shall we go back and use this 
>>>> for-loop inline.
>>>
>>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>>
>> IIRC it is possible to avoid variable reuse in macros with typeof()
>> operator (or something like that). I can't remember the details right
>> now but I think there are examples in the kernel code.
> 
> Here's the GCC documentation with an example:
> 
> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
> 

the problem here is that the macro actually writes to those arguments multiple
times, so we actually need to reuse the arguments

the macro as defined exactly matches the semantics of other for_each macros in
the kernel, i.e. see in include/linux/list.h:
#define list_for_each(pos, head) \
	for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)

what I don't understand is why list_for_each() doesn't trigger the same
checkpatch issues. even stranger is that if I copy that macro into ath12k then
I do see the same checkpatch issues:
CHECK: Macro argument reuse 'pos' - possible side-effects?
#998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
+#define list_for_each(pos, head) \
+       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)

CHECK: Macro argument reuse 'head' - possible side-effects?
#998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
+#define list_for_each(pos, head) \
+       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)

So I'm really confused since I don't see anything in checkpatch.pl that would
cause the behavior to change between macros in include/linux/list.h vs macros
in drivers/net/wireless/ath/ath12k/core.h

<scratch head>

/jeff

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Jonas Gorski]

On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>
> On 3/13/2024 9:58 AM, Kalle Valo wrote:
> > Kalle Valo <kvalo@kernel.org> writes:
> >
> >> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
> >>
> >>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
> >>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
> >>>>> and guess we have to figure out how to suppress the ath12k-check issues with
> >>>>> this macro
> >>>> ath12k-check complains about the reuse of ah and index arguments which
> >>>> may get evaluated multiple times if its an arithmetic expression, But
> >>>> areas where we use the macro in our code aren't doing so.
> >>>> Do you have any suggestions here ? or shall we go back and use this
> >>>> for-loop inline.
> >>>
> >>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
> >>
> >> IIRC it is possible to avoid variable reuse in macros with typeof()
> >> operator (or something like that). I can't remember the details right
> >> now but I think there are examples in the kernel code.
> >
> > Here's the GCC documentation with an example:
> >
> > https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
> >
>
> the problem here is that the macro actually writes to those arguments multiple
> times, so we actually need to reuse the arguments
>
> the macro as defined exactly matches the semantics of other for_each macros in
> the kernel, i.e. see in include/linux/list.h:
> #define list_for_each(pos, head) \
>         for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>
> what I don't understand is why list_for_each() doesn't trigger the same
> checkpatch issues. even stranger is that if I copy that macro into ath12k then
> I do see the same checkpatch issues:
> CHECK: Macro argument reuse 'pos' - possible side-effects?
> #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
> +#define list_for_each(pos, head) \
> +       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>
> CHECK: Macro argument reuse 'head' - possible side-effects?
> #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
> +#define list_for_each(pos, head) \
> +       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>
> So I'm really confused since I don't see anything in checkpatch.pl that would
> cause the behavior to change between macros in include/linux/list.h vs macros
> in drivers/net/wireless/ath/ath12k/core.h

The definition of the macro causes the complaint, not the usage of it.
If you run checkpatch.pl on include/linux/list.h, you'll get the same
output:

$ ./scripts/checkpatch.pl --strict --file include/linux/list.h
...
CHECK: Macro argument reuse 'pos' - possible side-effects?
#686: FILE: include/linux/list.h:686:
+#define list_for_each(pos, head) \
+ for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)

CHECK: Macro argument reuse 'head' - possible side-effects?
#686: FILE: include/linux/list.h:686:
+#define list_for_each(pos, head) \
+ for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
...

Best Regards,
Jonas

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Rameshkumar Sundaram]

On 3/14/2024 1:26 AM, Jonas Gorski wrote:
> On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>>
>> On 3/13/2024 9:58 AM, Kalle Valo wrote:
>>> Kalle Valo <kvalo@kernel.org> writes:
>>>
>>>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>>>
>>>>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>>>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>>>>> this macro
>>>>>> ath12k-check complains about the reuse of ah and index arguments which
>>>>>> may get evaluated multiple times if its an arithmetic expression, But
>>>>>> areas where we use the macro in our code aren't doing so.
>>>>>> Do you have any suggestions here ? or shall we go back and use this
>>>>>> for-loop inline.
>>>>>
>>>>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>>>>
>>>> IIRC it is possible to avoid variable reuse in macros with typeof()
>>>> operator (or something like that). I can't remember the details right
>>>> now but I think there are examples in the kernel code.
>>>
>>> Here's the GCC documentation with an example:
>>>
>>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
>>>
Thanks Kalle for the references, as Jeff mentioned below, we need to 
reuse the arguments since we write to ar and index arguments on each 
iteration.

Defining local vars using typeof() without limiting their scope (since 
we are defining a for_each iterator{}) leads other issues like 
redefinition of variables in functions where we use this macro more than 
once :(

Also even if we somehow manage to convince check-patch, we'll still end 
up evaluating index and ar arguments in every iteration of loop.
This just gives an impression to check-patch that the macro is unsafe 
(although logically its not).
Experts, what is the standard we should follow here. Please suggest.

>>
>> the problem here is that the macro actually writes to those arguments multiple
>> times, so we actually need to reuse the arguments
>>
>> the macro as defined exactly matches the semantics of other for_each macros in
>> the kernel, i.e. see in include/linux/list.h:
>> #define list_for_each(pos, head) \
>>          for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>>
>> what I don't understand is why list_for_each() doesn't trigger the same
>> checkpatch issues. even stranger is that if I copy that macro into ath12k then
>> I do see the same checkpatch issues:
>> CHECK: Macro argument reuse 'pos' - possible side-effects?
>> #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
>> +#define list_for_each(pos, head) \
>> +       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>>
>> CHECK: Macro argument reuse 'head' - possible side-effects?
>> #998: FILE: drivers/net/wireless/ath/ath12k/core.h:998:
>> +#define list_for_each(pos, head) \
>> +       for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
>>
>> So I'm really confused since I don't see anything in checkpatch.pl that would
>> cause the behavior to change between macros in include/linux/list.h vs macros
>> in drivers/net/wireless/ath/ath12k/core.h
> 
> The definition of the macro causes the complaint, not the usage of it.
> If you run checkpatch.pl on include/linux/list.h, you'll get the same
> output:
> 
> $ ./scripts/checkpatch.pl --strict --file include/linux/list.h
> ...
> CHECK: Macro argument reuse 'pos' - possible side-effects?
> #686: FILE: include/linux/list.h:686:
> +#define list_for_each(pos, head) \
> + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
> 
> CHECK: Macro argument reuse 'head' - possible side-effects?
> #686: FILE: include/linux/list.h:686:
> +#define list_for_each(pos, head) \
> + for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
> ...
Thanks Jonas and Jeff for your insights!!

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Kalle Valo]

Rameshkumar Sundaram <quic_ramess@quicinc.com> writes:

> On 3/14/2024 1:26 AM, Jonas Gorski wrote:
>> On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>>>
>>> On 3/13/2024 9:58 AM, Kalle Valo wrote:
>>>> Kalle Valo <kvalo@kernel.org> writes:
>>>>
>>>>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>>>>
>>>>>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>>>>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>>>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>>>>>> this macro
>>>>>>> ath12k-check complains about the reuse of ah and index arguments which
>>>>>>> may get evaluated multiple times if its an arithmetic expression, But
>>>>>>> areas where we use the macro in our code aren't doing so.
>>>>>>> Do you have any suggestions here ? or shall we go back and use this
>>>>>>> for-loop inline.
>>>>>>
>>>>>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>>>>>
>>>>> IIRC it is possible to avoid variable reuse in macros with typeof()
>>>>> operator (or something like that). I can't remember the details right
>>>>> now but I think there are examples in the kernel code.
>>>>
>>>> Here's the GCC documentation with an example:
>>>>
>>>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
>>>>
> Thanks Kalle for the references, as Jeff mentioned below, we need to
> reuse the arguments since we write to ar and index arguments on each
> iteration.
>
> Defining local vars using typeof() without limiting their scope (since
> we are defining a for_each iterator{}) leads other issues like
> redefinition of variables in functions where we use this macro more
> than once :(
>
> Also even if we somehow manage to convince check-patch, we'll still
> end up evaluating index and ar arguments in every iteration of loop.
> This just gives an impression to check-patch that the macro is unsafe
> (although logically its not).
> Experts, what is the standard we should follow here. Please suggest.

Yeah, typeof() won't help here as we can't create a local variable. Or
at least I can't come up way to do that safely, ideas very welcome.

I think it's just best to ignore the checkpatch warning for now, unless
better proposals come up. ath12k-check has functionality to ignore
specific warnings (see checkpatch_filter array), I can add this warning
to the array.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH v4 01/12] wifi: ath12k: add multiple radio support in a single MAC HW un/register

[Rameshkumar Sundaram]

On 3/19/2024 12:06 AM, Kalle Valo wrote:
> Rameshkumar Sundaram <quic_ramess@quicinc.com> writes:
> 
>> On 3/14/2024 1:26 AM, Jonas Gorski wrote:
>>> On Wed, 13 Mar 2024 at 20:18, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>>>>
>>>> On 3/13/2024 9:58 AM, Kalle Valo wrote:
>>>>> Kalle Valo <kvalo@kernel.org> writes:
>>>>>
>>>>>> Jeff Johnson <quic_jjohnson@quicinc.com> writes:
>>>>>>
>>>>>>> On 3/13/2024 5:57 AM, Rameshkumar Sundaram wrote:
>>>>>>>> On 3/13/2024 3:23 AM, Jeff Johnson wrote:
>>>>>>>>> and guess we have to figure out how to suppress the ath12k-check issues with
>>>>>>>>> this macro
>>>>>>>> ath12k-check complains about the reuse of ah and index arguments which
>>>>>>>> may get evaluated multiple times if its an arithmetic expression, But
>>>>>>>> areas where we use the macro in our code aren't doing so.
>>>>>>>> Do you have any suggestions here ? or shall we go back and use this
>>>>>>>> for-loop inline.
>>>>>>>
>>>>>>> The macro makes sense -- we'll need to update the overrides in ath12k-check.
>>>>>>
>>>>>> IIRC it is possible to avoid variable reuse in macros with typeof()
>>>>>> operator (or something like that). I can't remember the details right
>>>>>> now but I think there are examples in the kernel code.
>>>>>
>>>>> Here's the GCC documentation with an example:
>>>>>
>>>>> https://gcc.gnu.org/onlinedocs/gcc/Typeof.html
>>>>>
>> Thanks Kalle for the references, as Jeff mentioned below, we need to
>> reuse the arguments since we write to ar and index arguments on each
>> iteration.
>>
>> Defining local vars using typeof() without limiting their scope (since
>> we are defining a for_each iterator{}) leads other issues like
>> redefinition of variables in functions where we use this macro more
>> than once :(
>>
>> Also even if we somehow manage to convince check-patch, we'll still
>> end up evaluating index and ar arguments in every iteration of loop.
>> This just gives an impression to check-patch that the macro is unsafe
>> (although logically its not).
>> Experts, what is the standard we should follow here. Please suggest.
> 
> Yeah, typeof() won't help here as we can't create a local variable. Or
> at least I can't come up way to do that safely, ideas very welcome.
> 
> I think it's just best to ignore the checkpatch warning for now, unless
> better proposals come up. ath12k-check has functionality to ignore
> specific warnings (see checkpatch_filter array), I can add this warning
> to the array.
> 
Sure, Thanks Kalle, I'll send v5 with Jeff's comments addressed.

[PATCH v4 02/12] wifi: ath12k: Modify add and remove chanctx ops for single wiphy support

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

Modify add and remove chanctx mac80211 ops to fetch the correct
radio(ar) based on channel context.

This change also introduces new helper function to fetch the
radio/ar based on channel context and ieee80211_chan which internally
uses the radio's low/high freq range.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  3 ++
 drivers/net/wireless/ath/ath12k/mac.c  | 50 ++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index ff831faa4945..53bcf9416efd 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -598,6 +598,9 @@ struct ath12k {
 	bool monitor_vdev_created;
 	bool monitor_started;
 	int monitor_vdev_id;
+
+	u32 freq_low;
+	u32 freq_high;
 };
 
 struct ath12k_hw {
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index ba69fdfa9133..0f33f5615170 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -613,6 +613,35 @@ struct ath12k *ath12k_mac_get_ar_by_pdev_id(struct ath12k_base *ab, u32 pdev_id)
 	return NULL;
 }
 
+static struct ath12k *ath12k_mac_get_ar_by_chan(struct ieee80211_hw *hw,
+						struct ieee80211_channel *channel)
+{
+	struct ath12k_hw *ah = hw->priv;
+	struct ath12k *ar;
+	int i;
+
+	ar = ah->radio;
+
+	if (ah->num_radio == 1)
+		return ar;
+
+	for_each_ar(i, ah, ar) {
+		if (channel->center_freq >= ar->freq_low &&
+		    channel->center_freq <= ar->freq_high)
+			return ar;
+	}
+	return NULL;
+}
+
+static struct ath12k *ath12k_get_ar_by_ctx(struct ieee80211_hw *hw,
+					   struct ieee80211_chanctx_conf *ctx)
+{
+	if (!ctx)
+		return NULL;
+
+	return ath12k_mac_get_ar_by_chan(hw, ctx->def.chan);
+}
+
 static void ath12k_pdev_caps_update(struct ath12k *ar)
 {
 	struct ath12k_base *ab = ar->ab;
@@ -6229,11 +6258,13 @@ static int ath12k_mac_op_ampdu_action(struct ieee80211_hw *hw,
 static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 				     struct ieee80211_chanctx_conf *ctx)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_ctx(hw, ctx);
+	if (!ar)
+		return -EINVAL;
+
 	ab = ar->ab;
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
@@ -6257,11 +6288,13 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
 					 struct ieee80211_chanctx_conf *ctx)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_ctx(hw, ctx);
+	if (!ar)
+		return;
+
 	ab = ar->ab;
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
@@ -6637,11 +6670,13 @@ static void ath12k_mac_op_change_chanctx(struct ieee80211_hw *hw,
 					 struct ieee80211_chanctx_conf *ctx,
 					 u32 changed)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_ctx(hw, ctx);
+	if (!ar)
+		return;
+
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -7643,6 +7678,9 @@ static void ath12k_mac_update_ch_list(struct ath12k *ar,
 		    band->channels[i].center_freq > freq_high)
 			band->channels[i].flags |= IEEE80211_CHAN_DISABLED;
 	}
+
+	ar->freq_low = freq_low;
+	ar->freq_high = freq_high;
 }
 
 static u32 ath12k_get_phy_id(struct ath12k *ar, u32 band)
-- 
2.25.1

Re: [PATCH v4 02/12] wifi: ath12k: Modify add and remove chanctx ops for single wiphy support

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> Modify add and remove chanctx mac80211 ops to fetch the correct
> radio(ar) based on channel context.
> 
> This change also introduces new helper function to fetch the
> radio/ar based on channel context and ieee80211_chan which internally
> uses the radio's low/high freq range.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 03/12] wifi: ath12k: modify ath12k mac start/stop ops for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

When mac80211 does drv start/stop, apply the state change
for all the radios within the wiphy in ath12k.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 43 +++++++++++++++++++--------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 0f33f5615170..4afaba3ba934 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -243,6 +243,7 @@ static const u32 ath12k_smps_map[] = {
 
 static int ath12k_start_vdev_delay(struct ath12k *ar,
 				   struct ath12k_vif *arvif);
+static void ath12k_mac_stop(struct ath12k *ar);
 
 static const char *ath12k_mac_phymode_str(enum wmi_phy_mode mode)
 {
@@ -5472,23 +5473,39 @@ static int ath12k_mac_start(struct ath12k *ar)
 	return ret;
 }
 
+static void ath12k_drain_tx(struct ath12k_hw *ah)
+{
+	struct ath12k *ar;
+	int i;
+
+	for_each_ar(i, ah, ar)
+		ath12k_mac_drain_tx(ar);
+}
+
 static int ath12k_mac_op_start(struct ieee80211_hw *hw)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
-	struct ath12k_base *ab = ar->ab;
-	int ret;
+	struct ath12k *ar;
+	int ret, i;
 
-	ath12k_mac_drain_tx(ar);
+	ath12k_drain_tx(ah);
 
-	ret = ath12k_mac_start(ar);
-	if (ret) {
-		ath12k_err(ab, "fail to start mac operations in pdev idx %d ret %d\n",
-			   ar->pdev_idx, ret);
-		return ret;
+	for_each_ar(i, ah, ar) {
+		ret = ath12k_mac_start(ar);
+		if (ret) {
+			ath12k_err(ar->ab, "fail to start mac operations in pdev idx %d ret %d\n",
+				   ar->pdev_idx, ret);
+			goto fail_start;
+		}
 	}
 
 	return 0;
+fail_start:
+	for (; i > 0; i--) {
+		ar = ath12k_ah_to_ar(ah, i);
+		ath12k_mac_stop(ar);
+	}
+	return ret;
 }
 
 int ath12k_mac_rfkill_config(struct ath12k *ar)
@@ -5584,11 +5601,13 @@ static void ath12k_mac_stop(struct ath12k *ar)
 static void ath12k_mac_op_stop(struct ieee80211_hw *hw)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
+	struct ath12k *ar;
+	int i;
 
-	ath12k_mac_drain_tx(ar);
+	ath12k_drain_tx(ah);
 
-	ath12k_mac_stop(ar);
+	for_each_ar(i, ah, ar)
+		ath12k_mac_stop(ar);
 }
 
 static u8
-- 
2.25.1

Re: [PATCH v4 03/12] wifi: ath12k: modify ath12k mac start/stop ops for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> When mac80211 does drv start/stop, apply the state change
> for all the radios within the wiphy in ath12k.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/mac.c | 43 +++++++++++++++++++--------
>  1 file changed, 31 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 0f33f5615170..4afaba3ba934 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -243,6 +243,7 @@ static const u32 ath12k_smps_map[] = {
>  
>  static int ath12k_start_vdev_delay(struct ath12k *ar,
>  				   struct ath12k_vif *arvif);
> +static void ath12k_mac_stop(struct ath12k *ar);
>  
>  static const char *ath12k_mac_phymode_str(enum wmi_phy_mode mode)
>  {
> @@ -5472,23 +5473,39 @@ static int ath12k_mac_start(struct ath12k *ar)
>  	return ret;
>  }
>  
> +static void ath12k_drain_tx(struct ath12k_hw *ah)
> +{
> +	struct ath12k *ar;
> +	int i;
> +
> +	for_each_ar(i, ah, ar)
> +		ath12k_mac_drain_tx(ar);
> +}
> +
>  static int ath12k_mac_op_start(struct ieee80211_hw *hw)
>  {
>  	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> -	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
> -	struct ath12k_base *ab = ar->ab;
> -	int ret;
> +	struct ath12k *ar;
> +	int ret, i;
>  
> -	ath12k_mac_drain_tx(ar);
> +	ath12k_drain_tx(ah);
>  
> -	ret = ath12k_mac_start(ar);
> -	if (ret) {
> -		ath12k_err(ab, "fail to start mac operations in pdev idx %d ret %d\n",
> -			   ar->pdev_idx, ret);
> -		return ret;
> +	for_each_ar(i, ah, ar) {
> +		ret = ath12k_mac_start(ar);
> +		if (ret) {
> +			ath12k_err(ar->ab, "fail to start mac operations in pdev idx %d ret %d\n",
> +				   ar->pdev_idx, ret);
> +			goto fail_start;
> +		}
>  	}
>  
>  	return 0;
> +fail_start:
> +	for (; i > 0; i--) {

should this be >= 0? otherwise you never stop radio 0
also note that this has a dependency upon how the macro is implemented since
that determines when i is incremented.

> +		ar = ath12k_ah_to_ar(ah, i);
> +		ath12k_mac_stop(ar);
> +	}
> +	return ret;
>  }
>  
>  int ath12k_mac_rfkill_config(struct ath12k *ar)
> @@ -5584,11 +5601,13 @@ static void ath12k_mac_stop(struct ath12k *ar)
>  static void ath12k_mac_op_stop(struct ieee80211_hw *hw)
>  {
>  	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> -	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
> +	struct ath12k *ar;
> +	int i;
>  
> -	ath12k_mac_drain_tx(ar);
> +	ath12k_drain_tx(ah);
>  
> -	ath12k_mac_stop(ar);
> +	for_each_ar(i, ah, ar)
> +		ath12k_mac_stop(ar);
>  }
>  
>  static u8

Re: [PATCH v4 03/12] wifi: ath12k: modify ath12k mac start/stop ops for single wiphy

[Rameshkumar Sundaram]

On 3/13/2024 3:43 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> When mac80211 does drv start/stop, apply the state change
>> for all the radios within the wiphy in ath12k.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 43 +++++++++++++++++++--------
>>   1 file changed, 31 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 0f33f5615170..4afaba3ba934 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -243,6 +243,7 @@ static const u32 ath12k_smps_map[] = {
>>   
>>   static int ath12k_start_vdev_delay(struct ath12k *ar,
>>   				   struct ath12k_vif *arvif);
>> +static void ath12k_mac_stop(struct ath12k *ar);
>>   
>>   static const char *ath12k_mac_phymode_str(enum wmi_phy_mode mode)
>>   {
>> @@ -5472,23 +5473,39 @@ static int ath12k_mac_start(struct ath12k *ar)
>>   	return ret;
>>   }
>>   
>> +static void ath12k_drain_tx(struct ath12k_hw *ah)
>> +{
>> +	struct ath12k *ar;
>> +	int i;
>> +
>> +	for_each_ar(i, ah, ar)
>> +		ath12k_mac_drain_tx(ar);
>> +}
>> +
>>   static int ath12k_mac_op_start(struct ieee80211_hw *hw)
>>   {
>>   	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>> -	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
>> -	struct ath12k_base *ab = ar->ab;
>> -	int ret;
>> +	struct ath12k *ar;
>> +	int ret, i;
>>   
>> -	ath12k_mac_drain_tx(ar);
>> +	ath12k_drain_tx(ah);
>>   
>> -	ret = ath12k_mac_start(ar);
>> -	if (ret) {
>> -		ath12k_err(ab, "fail to start mac operations in pdev idx %d ret %d\n",
>> -			   ar->pdev_idx, ret);
>> -		return ret;
>> +	for_each_ar(i, ah, ar) {
>> +		ret = ath12k_mac_start(ar);
>> +		if (ret) {
>> +			ath12k_err(ar->ab, "fail to start mac operations in pdev idx %d ret %d\n",
>> +				   ar->pdev_idx, ret);
>> +			goto fail_start;
>> +		}
>>   	}
>>   
>>   	return 0;
>> +fail_start:
>> +	for (; i > 0; i--) {
> 
> should this be >= 0? otherwise you never stop radio 0
Ah! yes, will fix it in next version
> also note that this has a dependency upon how the macro is implemented since
> that determines when i is incremented.
Yeah, we'll fall to fail_start only if we had to break prematurely (i.e. 
before i could reach ah->num_radios).
> 
>> +		ar = ath12k_ah_to_ar(ah, i);
>> +		ath12k_mac_stop(ar);
>> +	}
>> +	return ret;
>>   }
>>   
>>   int ath12k_mac_rfkill_config(struct ath12k *ar)
>> @@ -5584,11 +5601,13 @@ static void ath12k_mac_stop(struct ath12k *ar)
>>   static void ath12k_mac_op_stop(struct ieee80211_hw *hw)
>>   {
>>   	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>> -	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
>> +	struct ath12k *ar;
>> +	int i;
>>   
>> -	ath12k_mac_drain_tx(ar);
>> +	ath12k_drain_tx(ah);
>>   
>> -	ath12k_mac_stop(ar);
>> +	for_each_ar(i, ah, ar)
>> +		ath12k_mac_stop(ar);
>>   }
>>   
>>   static u8
> 

[PATCH v4 04/12] wifi: ath12k: vdev statemachine changes for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

With single wiphy, multiple radios are combined into a single wiphy.
Since any channel can be assigned to a vif being brought up,
the vdev cannot be created during add_interface(). Hence defer the
vdev creation till channel assignment.

If only one radio is part of the wiphy, then the existing logic
is maintained, i.e vdevs are created during add interface and
started during channel assignment. This ensures no functional changes
to single pdev devices which has only one radio in the SoC.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |   1 +
 drivers/net/wireless/ath/ath12k/hw.h   |   1 +
 drivers/net/wireless/ath/ath12k/mac.c  | 203 +++++++++++++++++--------
 3 files changed, 144 insertions(+), 61 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 53bcf9416efd..70daec38d486 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -251,6 +251,7 @@ struct ath12k_vif {
 		} ap;
 	} u;
 
+	bool is_created;
 	bool is_started;
 	bool is_up;
 	u32 aid;
diff --git a/drivers/net/wireless/ath/ath12k/hw.h b/drivers/net/wireless/ath/ath12k/hw.h
index 87965980b938..e34c4f76c1ec 100644
--- a/drivers/net/wireless/ath/ath12k/hw.h
+++ b/drivers/net/wireless/ath/ath12k/hw.h
@@ -80,6 +80,7 @@
 #define TARGET_RX_PEER_METADATA_VER_V1A	2
 #define TARGET_RX_PEER_METADATA_VER_V1B	3
 
+#define ATH12K_HW_DEFAULT_QUEUE		0
 #define ATH12K_HW_MAX_QUEUES		4
 #define ATH12K_QUEUE_LEN		4096
 
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 4afaba3ba934..b6afef81a2d8 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -5780,64 +5780,24 @@ static void ath12k_mac_op_update_vif_offload(struct ieee80211_hw *hw,
 	ath12k_mac_update_vif_offload(arvif);
 }
 
-static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
-				       struct ieee80211_vif *vif)
+static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar;
-	struct ath12k_base *ab;
+	struct ath12k_hw *ah = ar->ah;
+	struct ath12k_base *ab = ar->ab;
+	struct ieee80211_hw *hw = ah->hw;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_wmi_vdev_create_arg vdev_arg = {0};
 	struct ath12k_wmi_peer_create_arg peer_param;
 	u32 param_id, param_value;
 	u16 nss;
 	int i;
-	int ret;
-	int bit;
-
-	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
+	int ret, vdev_id;
 
-	ar = ath12k_ah_to_ar(ah, 0);
-	ab = ar->ab;
-
-	mutex_lock(&ar->conf_mutex);
-
-	if (vif->type == NL80211_IFTYPE_AP &&
-	    ar->num_peers > (ar->max_num_peers - 1)) {
-		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
-		ret = -ENOBUFS;
-		goto err;
-	}
-
-	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
-		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
-			    TARGET_NUM_VDEVS);
-		ret = -EBUSY;
-		goto err;
-	}
-
-	memset(arvif, 0, sizeof(*arvif));
+	lockdep_assert_held(&ar->conf_mutex);
 
 	arvif->ar = ar;
-	arvif->vif = vif;
-
-	INIT_LIST_HEAD(&arvif->list);
-
-	/* Should we initialize any worker to handle connection loss indication
-	 * from firmware in sta mode?
-	 */
-
-	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
-		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
-		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
-		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
-		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
-		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
-	}
-
-	bit = __ffs64(ab->free_vdev_map);
-
-	arvif->vdev_id = bit;
+	vdev_id = __ffs64(ab->free_vdev_map);
+	arvif->vdev_id = vdev_id;
 	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
 
 	switch (vif->type) {
@@ -5861,7 +5821,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 		break;
 	case NL80211_IFTYPE_MONITOR:
 		arvif->vdev_type = WMI_VDEV_TYPE_MONITOR;
-		ar->monitor_vdev_id = bit;
+		ar->monitor_vdev_id = vdev_id;
 		break;
 	case NL80211_IFTYPE_P2P_DEVICE:
 		arvif->vdev_type = WMI_VDEV_TYPE_STA;
@@ -5872,7 +5832,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 		break;
 	}
 
-	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac add interface id %d type %d subtype %d map %llx\n",
+	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev create id %d type %d subtype %d map %llx\n",
 		   arvif->vdev_id, arvif->vdev_type, arvif->vdev_subtype,
 		   ab->free_vdev_map);
 
@@ -5890,6 +5850,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 	}
 
 	ar->num_created_vdevs++;
+	arvif->is_created = true;
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM created, vdev_id %d\n",
 		   vif->addr, arvif->vdev_id);
 	ar->allocated_vdev_map |= 1LL << arvif->vdev_id;
@@ -5990,8 +5951,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 	if (vif->type != NL80211_IFTYPE_MONITOR && ar->monitor_conf_enabled)
 		ath12k_mac_monitor_vdev_create(ar);
 
-	mutex_unlock(&ar->conf_mutex);
-
 	return ret;
 
 err_peer_del:
@@ -6017,6 +5976,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 err_vdev_del:
 	ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->num_created_vdevs--;
+	arvif->is_created = false;
 	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
 	ab->free_vdev_map |= 1LL << arvif->vdev_id;
 	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
@@ -6025,9 +5985,104 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 err:
+	arvif->ar = NULL;
+	return ret;
+}
+
+static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
+						    struct ieee80211_vif *vif,
+						    struct ieee80211_chanctx_conf *ctx)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	struct ath12k_hw *ah = hw->priv;
+	struct ath12k_base *ab;
+	struct ath12k *ar;
+	int ret;
+	u8 bit;
+
+	if (arvif->ar) {
+		WARN_ON(!arvif->is_created);
+		goto out;
+	}
+
+	if (ah->num_radio == 1)
+		ar = ah->radio;
+	else if (ctx)
+		ar = ath12k_get_ar_by_ctx(hw, ctx);
+	else
+		return NULL;
+
+	if (!ar)
+		goto out;
+
+	ab = ar->ab;
+
+	mutex_lock(&ar->conf_mutex);
+
+	if (vif->type == NL80211_IFTYPE_AP &&
+	    ar->num_peers > (ar->max_num_peers - 1)) {
+		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
+		ret = -ENOBUFS;
+		goto unlock;
+	}
+
+	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
+		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
+			    TARGET_NUM_VDEVS);
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	ret = ath12k_mac_vdev_create(ar, vif);
+	if (ret) {
+		ath12k_warn(ab, "failed to create vdev %d ret %d", bit, ret);
+		goto unlock;
+	}
+
+	/* TODO If the vdev is created during channel assign and not during
+	 * add_interface(), Apply any parameters for the vdev which were received
+	 * after add_interface, corresponding to this vif.
+	 */
+unlock:
 	mutex_unlock(&ar->conf_mutex);
+out:
+	return arvif->ar;
+}
 
-	return ret;
+static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
+				       struct ieee80211_vif *vif)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	int i;
+
+	memset(arvif, 0, sizeof(*arvif));
+
+	arvif->vif = vif;
+
+	INIT_LIST_HEAD(&arvif->list);
+
+	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
+		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
+		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
+		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
+		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
+		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
+	}
+
+	/* Allocate Default Queue now and reassign during actual vdev create */
+	vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE;
+	for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++)
+		vif->hw_queue[i] = ATH12K_HW_DEFAULT_QUEUE;
+
+	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
+
+	/* For single radio wiphy(i.e ah->num_radio is 1), create the vdev
+	 * during add_interface itself, for multi radio wiphy, defer the vdev
+	 * creation until channel_assign to determine the radio on which the
+	 * vdev needs to be created
+	 */
+	ath12k_mac_assign_vif_to_vdev(hw, vif, NULL);
+	return 0;
 }
 
 static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif)
@@ -6058,14 +6113,16 @@ static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif
 static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 					   struct ieee80211_vif *vif)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_base *ab;
 	unsigned long time_left;
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	if (!arvif->is_created)
+		return;
+
+	ar = arvif->ar;
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6107,6 +6164,7 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
 	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
 	ar->num_created_vdevs--;
+	arvif->is_created = false;
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM deleted, vdev_id %d\n",
 		   vif->addr, arvif->vdev_id);
@@ -6759,14 +6817,21 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 				 struct ieee80211_bss_conf *link_conf,
 				 struct ieee80211_chanctx_conf *ctx)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 	struct ath12k_wmi_peer_create_arg param;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	/* For multi radio wiphy, the vdev was not created during add_interface
+	 * create now since we have a channel ctx now to assign to a specific ar/fw
+	 */
+	ar = ath12k_mac_assign_vif_to_vdev(hw, vif, ctx);
+	if (!ar) {
+		WARN_ON(1);
+		return -EINVAL;
+	}
+
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6842,13 +6907,22 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 				   struct ieee80211_bss_conf *link_conf,
 				   struct ieee80211_chanctx_conf *ctx)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	/* The vif is expected to be attached to an ar's VDEV.
+	 * We leave the vif/vdev in this function as is
+	 * and not delete the vdev symmetric to assign_vif_chanctx()
+	 * the VDEV will be deleted and unassigned either during
+	 * remove_interface() or when there is a change in channel
+	 * that moves the vif to a new ar
+	 */
+	if (!arvif->is_created)
+		return;
+
+	ar = arvif->ar;
 	ab = ar->ab;
 
 	mutex_lock(&ar->conf_mutex);
@@ -6900,13 +6974,20 @@ ath12k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
 				 int n_vifs,
 				 enum ieee80211_chanctx_switch_mode mode)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_ctx(hw, vifs->old_ctx);
+	if (!ar)
+		return -EINVAL;
 
 	mutex_lock(&ar->conf_mutex);
 
+	/* Switching channels across radio is not allowed */
+	if (ar != ath12k_get_ar_by_ctx(hw, vifs->new_ctx)) {
+		mutex_unlock(&ar->conf_mutex);
+		return -EINVAL;
+	}
+
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC,
 		   "mac chanctx switch n_vifs %d mode %d\n",
 		   n_vifs, mode);
-- 
2.25.1

Re: [PATCH v4 04/12] wifi: ath12k: vdev statemachine changes for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> With single wiphy, multiple radios are combined into a single wiphy.
> Since any channel can be assigned to a vif being brought up,
> the vdev cannot be created during add_interface(). Hence defer the
> vdev creation till channel assignment.
> 
> If only one radio is part of the wiphy, then the existing logic
> is maintained, i.e vdevs are created during add interface and
> started during channel assignment. This ensures no functional changes
> to single pdev devices which has only one radio in the SoC.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.h |   1 +
>  drivers/net/wireless/ath/ath12k/hw.h   |   1 +
>  drivers/net/wireless/ath/ath12k/mac.c  | 203 +++++++++++++++++--------
>  3 files changed, 144 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 53bcf9416efd..70daec38d486 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -251,6 +251,7 @@ struct ath12k_vif {
>  		} ap;
>  	} u;
>  
> +	bool is_created;
>  	bool is_started;
>  	bool is_up;
>  	u32 aid;
> diff --git a/drivers/net/wireless/ath/ath12k/hw.h b/drivers/net/wireless/ath/ath12k/hw.h
> index 87965980b938..e34c4f76c1ec 100644
> --- a/drivers/net/wireless/ath/ath12k/hw.h
> +++ b/drivers/net/wireless/ath/ath12k/hw.h
> @@ -80,6 +80,7 @@
>  #define TARGET_RX_PEER_METADATA_VER_V1A	2
>  #define TARGET_RX_PEER_METADATA_VER_V1B	3
>  
> +#define ATH12K_HW_DEFAULT_QUEUE		0
>  #define ATH12K_HW_MAX_QUEUES		4
>  #define ATH12K_QUEUE_LEN		4096
>  
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 4afaba3ba934..b6afef81a2d8 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -5780,64 +5780,24 @@ static void ath12k_mac_op_update_vif_offload(struct ieee80211_hw *hw,
>  	ath12k_mac_update_vif_offload(arvif);
>  }
>  
> -static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
> -				       struct ieee80211_vif *vif)
> +static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
> -	struct ath12k *ar;
> -	struct ath12k_base *ab;
> +	struct ath12k_hw *ah = ar->ah;
> +	struct ath12k_base *ab = ar->ab;
> +	struct ieee80211_hw *hw = ah->hw;
>  	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>  	struct ath12k_wmi_vdev_create_arg vdev_arg = {0};
>  	struct ath12k_wmi_peer_create_arg peer_param;
>  	u32 param_id, param_value;
>  	u16 nss;
>  	int i;
> -	int ret;
> -	int bit;
> -
> -	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
> +	int ret, vdev_id;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> -	ab = ar->ab;
> -
> -	mutex_lock(&ar->conf_mutex);
> -
> -	if (vif->type == NL80211_IFTYPE_AP &&
> -	    ar->num_peers > (ar->max_num_peers - 1)) {
> -		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
> -		ret = -ENOBUFS;
> -		goto err;
> -	}
> -
> -	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
> -		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
> -			    TARGET_NUM_VDEVS);
> -		ret = -EBUSY;
> -		goto err;
> -	}
> -
> -	memset(arvif, 0, sizeof(*arvif));
> +	lockdep_assert_held(&ar->conf_mutex);
>  
>  	arvif->ar = ar;
> -	arvif->vif = vif;
> -
> -	INIT_LIST_HEAD(&arvif->list);
> -
> -	/* Should we initialize any worker to handle connection loss indication
> -	 * from firmware in sta mode?
> -	 */
> -
> -	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
> -		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
> -		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
> -		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
> -		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
> -		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
> -	}
> -
> -	bit = __ffs64(ab->free_vdev_map);
> -
> -	arvif->vdev_id = bit;
> +	vdev_id = __ffs64(ab->free_vdev_map);
> +	arvif->vdev_id = vdev_id;
>  	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
>  
>  	switch (vif->type) {
> @@ -5861,7 +5821,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  		break;
>  	case NL80211_IFTYPE_MONITOR:
>  		arvif->vdev_type = WMI_VDEV_TYPE_MONITOR;
> -		ar->monitor_vdev_id = bit;
> +		ar->monitor_vdev_id = vdev_id;
>  		break;
>  	case NL80211_IFTYPE_P2P_DEVICE:
>  		arvif->vdev_type = WMI_VDEV_TYPE_STA;
> @@ -5872,7 +5832,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  		break;
>  	}
>  
> -	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac add interface id %d type %d subtype %d map %llx\n",
> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev create id %d type %d subtype %d map %llx\n",
>  		   arvif->vdev_id, arvif->vdev_type, arvif->vdev_subtype,
>  		   ab->free_vdev_map);
>  
> @@ -5890,6 +5850,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  	}
>  
>  	ar->num_created_vdevs++;
> +	arvif->is_created = true;
>  	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM created, vdev_id %d\n",
>  		   vif->addr, arvif->vdev_id);
>  	ar->allocated_vdev_map |= 1LL << arvif->vdev_id;
> @@ -5990,8 +5951,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  	if (vif->type != NL80211_IFTYPE_MONITOR && ar->monitor_conf_enabled)
>  		ath12k_mac_monitor_vdev_create(ar);
>  
> -	mutex_unlock(&ar->conf_mutex);
> -
>  	return ret;
>  
>  err_peer_del:
> @@ -6017,6 +5976,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  err_vdev_del:
>  	ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
>  	ar->num_created_vdevs--;
> +	arvif->is_created = false;
>  	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
>  	ab->free_vdev_map |= 1LL << arvif->vdev_id;
>  	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
> @@ -6025,9 +5985,104 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>  	spin_unlock_bh(&ar->data_lock);
>  
>  err:
> +	arvif->ar = NULL;
> +	return ret;
> +}
> +
> +static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
> +						    struct ieee80211_vif *vif,
> +						    struct ieee80211_chanctx_conf *ctx)
> +{
> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> +	struct ath12k_hw *ah = hw->priv;
> +	struct ath12k_base *ab;
> +	struct ath12k *ar;
> +	int ret;
> +	u8 bit;
> +
> +	if (arvif->ar) {
> +		WARN_ON(!arvif->is_created);
> +		goto out;
> +	}
> +
> +	if (ah->num_radio == 1)
> +		ar = ah->radio;
> +	else if (ctx)
> +		ar = ath12k_get_ar_by_ctx(hw, ctx);
> +	else
> +		return NULL;
> +
> +	if (!ar)
> +		goto out;

why does this goto out instead of just return NULL?

> +
> +	ab = ar->ab;
> +
> +	mutex_lock(&ar->conf_mutex);
> +
> +	if (vif->type == NL80211_IFTYPE_AP &&
> +	    ar->num_peers > (ar->max_num_peers - 1)) {
> +		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
> +		ret = -ENOBUFS;
> +		goto unlock;

nothing is done with ret so setting it is pointless

> +	}
> +
> +	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
> +		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
> +			    TARGET_NUM_VDEVS);
> +		ret = -EBUSY;
> +		goto unlock;

nothing is done with ret so setting it is pointless

> +	}
> +
> +	ret = ath12k_mac_vdev_create(ar, vif);
> +	if (ret) {
> +		ath12k_warn(ab, "failed to create vdev %d ret %d", bit, ret);
> +		goto unlock;
> +	}
> +
> +	/* TODO If the vdev is created during channel assign and not during
> +	 * add_interface(), Apply any parameters for the vdev which were received
> +	 * after add_interface, corresponding to this vif.
> +	 */
> +unlock:
>  	mutex_unlock(&ar->conf_mutex);
> +out:
> +	return arvif->ar;
> +}
>  
> -	return ret;
> +static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
> +				       struct ieee80211_vif *vif)
> +{
> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> +	int i;
> +
> +	memset(arvif, 0, sizeof(*arvif));
> +
> +	arvif->vif = vif;
> +
> +	INIT_LIST_HEAD(&arvif->list);
> +
> +	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
> +		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
> +		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
> +		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
> +		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
> +		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
> +	}
> +
> +	/* Allocate Default Queue now and reassign during actual vdev create */
> +	vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE;
> +	for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++)
> +		vif->hw_queue[i] = ATH12K_HW_DEFAULT_QUEUE;
> +
> +	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
> +
> +	/* For single radio wiphy(i.e ah->num_radio is 1), create the vdev
> +	 * during add_interface itself, for multi radio wiphy, defer the vdev
> +	 * creation until channel_assign to determine the radio on which the
> +	 * vdev needs to be created
> +	 */
> +	ath12k_mac_assign_vif_to_vdev(hw, vif, NULL);
> +	return 0;
>  }
>  
>  static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif)
> @@ -6058,14 +6113,16 @@ static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif
>  static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>  					   struct ieee80211_vif *vif)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>  	struct ath12k *ar;
>  	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>  	struct ath12k_base *ab;
>  	unsigned long time_left;
>  	int ret;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> +	if (!arvif->is_created)
> +		return;
> +
> +	ar = arvif->ar;
>  	ab = ar->ab;
>  
>  	mutex_lock(&ar->conf_mutex);
> @@ -6107,6 +6164,7 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>  	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
>  	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
>  	ar->num_created_vdevs--;
> +	arvif->is_created = false;
>  
>  	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM deleted, vdev_id %d\n",
>  		   vif->addr, arvif->vdev_id);
> @@ -6759,14 +6817,21 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>  				 struct ieee80211_bss_conf *link_conf,
>  				 struct ieee80211_chanctx_conf *ctx)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>  	struct ath12k *ar;
>  	struct ath12k_base *ab;
>  	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>  	int ret;
>  	struct ath12k_wmi_peer_create_arg param;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> +	/* For multi radio wiphy, the vdev was not created during add_interface
> +	 * create now since we have a channel ctx now to assign to a specific ar/fw
> +	 */
> +	ar = ath12k_mac_assign_vif_to_vdev(hw, vif, ctx);
> +	if (!ar) {
> +		WARN_ON(1);
> +		return -EINVAL;
> +	}
> +
>  	ab = ar->ab;
>  
>  	mutex_lock(&ar->conf_mutex);
> @@ -6842,13 +6907,22 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>  				   struct ieee80211_bss_conf *link_conf,
>  				   struct ieee80211_chanctx_conf *ctx)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>  	struct ath12k *ar;
>  	struct ath12k_base *ab;
>  	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>  	int ret;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> +	/* The vif is expected to be attached to an ar's VDEV.
> +	 * We leave the vif/vdev in this function as is
> +	 * and not delete the vdev symmetric to assign_vif_chanctx()
> +	 * the VDEV will be deleted and unassigned either during
> +	 * remove_interface() or when there is a change in channel
> +	 * that moves the vif to a new ar
> +	 */
> +	if (!arvif->is_created)
> +		return;
> +
> +	ar = arvif->ar;
>  	ab = ar->ab;
>  
>  	mutex_lock(&ar->conf_mutex);
> @@ -6900,13 +6974,20 @@ ath12k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
>  				 int n_vifs,
>  				 enum ieee80211_chanctx_switch_mode mode)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>  	struct ath12k *ar;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> +	ar = ath12k_get_ar_by_ctx(hw, vifs->old_ctx);
> +	if (!ar)
> +		return -EINVAL;
>  
>  	mutex_lock(&ar->conf_mutex);
>  
> +	/* Switching channels across radio is not allowed */
> +	if (ar != ath12k_get_ar_by_ctx(hw, vifs->new_ctx)) {
> +		mutex_unlock(&ar->conf_mutex);
> +		return -EINVAL;
> +	}
> +
>  	ath12k_dbg(ar->ab, ATH12K_DBG_MAC,
>  		   "mac chanctx switch n_vifs %d mode %d\n",
>  		   n_vifs, mode);

Re: [PATCH v4 04/12] wifi: ath12k: vdev statemachine changes for single wiphy

[Rameshkumar Sundaram]

On 3/13/2024 3:55 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> With single wiphy, multiple radios are combined into a single wiphy.
>> Since any channel can be assigned to a vif being brought up,
>> the vdev cannot be created during add_interface(). Hence defer the
>> vdev creation till channel assignment.
>>
>> If only one radio is part of the wiphy, then the existing logic
>> is maintained, i.e vdevs are created during add interface and
>> started during channel assignment. This ensures no functional changes
>> to single pdev devices which has only one radio in the SoC.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.h |   1 +
>>   drivers/net/wireless/ath/ath12k/hw.h   |   1 +
>>   drivers/net/wireless/ath/ath12k/mac.c  | 203 +++++++++++++++++--------
>>   3 files changed, 144 insertions(+), 61 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 53bcf9416efd..70daec38d486 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -251,6 +251,7 @@ struct ath12k_vif {
>>   		} ap;
>>   	} u;
>>   
>> +	bool is_created;
>>   	bool is_started;
>>   	bool is_up;
>>   	u32 aid;
>> diff --git a/drivers/net/wireless/ath/ath12k/hw.h b/drivers/net/wireless/ath/ath12k/hw.h
>> index 87965980b938..e34c4f76c1ec 100644
>> --- a/drivers/net/wireless/ath/ath12k/hw.h
>> +++ b/drivers/net/wireless/ath/ath12k/hw.h
>> @@ -80,6 +80,7 @@
>>   #define TARGET_RX_PEER_METADATA_VER_V1A	2
>>   #define TARGET_RX_PEER_METADATA_VER_V1B	3
>>   
>> +#define ATH12K_HW_DEFAULT_QUEUE		0
>>   #define ATH12K_HW_MAX_QUEUES		4
>>   #define ATH12K_QUEUE_LEN		4096
>>   
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 4afaba3ba934..b6afef81a2d8 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -5780,64 +5780,24 @@ static void ath12k_mac_op_update_vif_offload(struct ieee80211_hw *hw,
>>   	ath12k_mac_update_vif_offload(arvif);
>>   }
>>   
>> -static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>> -				       struct ieee80211_vif *vif)
>> +static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>> -	struct ath12k *ar;
>> -	struct ath12k_base *ab;
>> +	struct ath12k_hw *ah = ar->ah;
>> +	struct ath12k_base *ab = ar->ab;
>> +	struct ieee80211_hw *hw = ah->hw;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>>   	struct ath12k_wmi_vdev_create_arg vdev_arg = {0};
>>   	struct ath12k_wmi_peer_create_arg peer_param;
>>   	u32 param_id, param_value;
>>   	u16 nss;
>>   	int i;
>> -	int ret;
>> -	int bit;
>> -
>> -	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
>> +	int ret, vdev_id;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> -	ab = ar->ab;
>> -
>> -	mutex_lock(&ar->conf_mutex);
>> -
>> -	if (vif->type == NL80211_IFTYPE_AP &&
>> -	    ar->num_peers > (ar->max_num_peers - 1)) {
>> -		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
>> -		ret = -ENOBUFS;
>> -		goto err;
>> -	}
>> -
>> -	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
>> -		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
>> -			    TARGET_NUM_VDEVS);
>> -		ret = -EBUSY;
>> -		goto err;
>> -	}
>> -
>> -	memset(arvif, 0, sizeof(*arvif));
>> +	lockdep_assert_held(&ar->conf_mutex);
>>   
>>   	arvif->ar = ar;
>> -	arvif->vif = vif;
>> -
>> -	INIT_LIST_HEAD(&arvif->list);
>> -
>> -	/* Should we initialize any worker to handle connection loss indication
>> -	 * from firmware in sta mode?
>> -	 */
>> -
>> -	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
>> -		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
>> -		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
>> -		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
>> -		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
>> -		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
>> -	}
>> -
>> -	bit = __ffs64(ab->free_vdev_map);
>> -
>> -	arvif->vdev_id = bit;
>> +	vdev_id = __ffs64(ab->free_vdev_map);
>> +	arvif->vdev_id = vdev_id;
>>   	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
>>   
>>   	switch (vif->type) {
>> @@ -5861,7 +5821,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   		break;
>>   	case NL80211_IFTYPE_MONITOR:
>>   		arvif->vdev_type = WMI_VDEV_TYPE_MONITOR;
>> -		ar->monitor_vdev_id = bit;
>> +		ar->monitor_vdev_id = vdev_id;
>>   		break;
>>   	case NL80211_IFTYPE_P2P_DEVICE:
>>   		arvif->vdev_type = WMI_VDEV_TYPE_STA;
>> @@ -5872,7 +5832,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   		break;
>>   	}
>>   
>> -	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac add interface id %d type %d subtype %d map %llx\n",
>> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev create id %d type %d subtype %d map %llx\n",
>>   		   arvif->vdev_id, arvif->vdev_type, arvif->vdev_subtype,
>>   		   ab->free_vdev_map);
>>   
>> @@ -5890,6 +5850,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   	}
>>   
>>   	ar->num_created_vdevs++;
>> +	arvif->is_created = true;
>>   	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM created, vdev_id %d\n",
>>   		   vif->addr, arvif->vdev_id);
>>   	ar->allocated_vdev_map |= 1LL << arvif->vdev_id;
>> @@ -5990,8 +5951,6 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   	if (vif->type != NL80211_IFTYPE_MONITOR && ar->monitor_conf_enabled)
>>   		ath12k_mac_monitor_vdev_create(ar);
>>   
>> -	mutex_unlock(&ar->conf_mutex);
>> -
>>   	return ret;
>>   
>>   err_peer_del:
>> @@ -6017,6 +5976,7 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   err_vdev_del:
>>   	ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
>>   	ar->num_created_vdevs--;
>> +	arvif->is_created = false;
>>   	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
>>   	ab->free_vdev_map |= 1LL << arvif->vdev_id;
>>   	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
>> @@ -6025,9 +5985,104 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>>   	spin_unlock_bh(&ar->data_lock);
>>   
>>   err:
>> +	arvif->ar = NULL;
>> +	return ret;
>> +}
>> +
>> +static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
>> +						    struct ieee80211_vif *vif,
>> +						    struct ieee80211_chanctx_conf *ctx)
>> +{
>> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>> +	struct ath12k_hw *ah = hw->priv;
>> +	struct ath12k_base *ab;
>> +	struct ath12k *ar;
>> +	int ret;
>> +	u8 bit;
>> +
>> +	if (arvif->ar) {
>> +		WARN_ON(!arvif->is_created);
>> +		goto out;
>> +	}
>> +
>> +	if (ah->num_radio == 1)
>> +		ar = ah->radio;
>> +	else if (ctx)
>> +		ar = ath12k_get_ar_by_ctx(hw, ctx);
>> +	else
>> +		return NULL;
>> +
>> +	if (!ar)
>> +		goto out;
> 
> why does this goto out instead of just return NULL?
yeah, we can. Thanks for pointing out, will fix it in next version.
> 
>> +
>> +	ab = ar->ab;
>> +
>> +	mutex_lock(&ar->conf_mutex);
>> +
>> +	if (vif->type == NL80211_IFTYPE_AP &&
>> +	    ar->num_peers > (ar->max_num_peers - 1)) {
>> +		ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
>> +		ret = -ENOBUFS;
>> +		goto unlock;
> 
> nothing is done with ret so setting it is pointless
> 
Sure, will remove this and below instance too.
>> +	}
>> +
>> +	if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
>> +		ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
>> +			    TARGET_NUM_VDEVS);
>> +		ret = -EBUSY;
>> +		goto unlock;
> 
> nothing is done with ret so setting it is pointless
> 
>> +	}
>> +
>> +	ret = ath12k_mac_vdev_create(ar, vif);
>> +	if (ret) {
>> +		ath12k_warn(ab, "failed to create vdev %d ret %d", bit, ret);
>> +		goto unlock;
>> +	}
>> +
>> +	/* TODO If the vdev is created during channel assign and not during
>> +	 * add_interface(), Apply any parameters for the vdev which were received
>> +	 * after add_interface, corresponding to this vif.
>> +	 */
>> +unlock:
>>   	mutex_unlock(&ar->conf_mutex);
>> +out:
>> +	return arvif->ar;
>> +}
>>   
>> -	return ret;
>> +static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
>> +				       struct ieee80211_vif *vif)
>> +{
>> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>> +	int i;
>> +
>> +	memset(arvif, 0, sizeof(*arvif));
>> +
>> +	arvif->vif = vif;
>> +
>> +	INIT_LIST_HEAD(&arvif->list);
>> +
>> +	for (i = 0; i < ARRAY_SIZE(arvif->bitrate_mask.control); i++) {
>> +		arvif->bitrate_mask.control[i].legacy = 0xffffffff;
>> +		memset(arvif->bitrate_mask.control[i].ht_mcs, 0xff,
>> +		       sizeof(arvif->bitrate_mask.control[i].ht_mcs));
>> +		memset(arvif->bitrate_mask.control[i].vht_mcs, 0xff,
>> +		       sizeof(arvif->bitrate_mask.control[i].vht_mcs));
>> +	}
>> +
>> +	/* Allocate Default Queue now and reassign during actual vdev create */
>> +	vif->cab_queue = ATH12K_HW_DEFAULT_QUEUE;
>> +	for (i = 0; i < ARRAY_SIZE(vif->hw_queue); i++)
>> +		vif->hw_queue[i] = ATH12K_HW_DEFAULT_QUEUE;
>> +
>> +	vif->driver_flags |= IEEE80211_VIF_SUPPORTS_UAPSD;
>> +
>> +	/* For single radio wiphy(i.e ah->num_radio is 1), create the vdev
>> +	 * during add_interface itself, for multi radio wiphy, defer the vdev
>> +	 * creation until channel_assign to determine the radio on which the
>> +	 * vdev needs to be created
>> +	 */
>> +	ath12k_mac_assign_vif_to_vdev(hw, vif, NULL);
>> +	return 0;
>>   }
>>   
>>   static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif)
>> @@ -6058,14 +6113,16 @@ static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif
>>   static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>>   					   struct ieee80211_vif *vif)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>>   	struct ath12k_base *ab;
>>   	unsigned long time_left;
>>   	int ret;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> +	if (!arvif->is_created)
>> +		return;
>> +
>> +	ar = arvif->ar;
>>   	ab = ar->ab;
>>   
>>   	mutex_lock(&ar->conf_mutex);
>> @@ -6107,6 +6164,7 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
>>   	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
>>   	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
>>   	ar->num_created_vdevs--;
>> +	arvif->is_created = false;
>>   
>>   	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM deleted, vdev_id %d\n",
>>   		   vif->addr, arvif->vdev_id);
>> @@ -6759,14 +6817,21 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
>>   				 struct ieee80211_bss_conf *link_conf,
>>   				 struct ieee80211_chanctx_conf *ctx)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   	struct ath12k_base *ab;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>>   	int ret;
>>   	struct ath12k_wmi_peer_create_arg param;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> +	/* For multi radio wiphy, the vdev was not created during add_interface
>> +	 * create now since we have a channel ctx now to assign to a specific ar/fw
>> +	 */
>> +	ar = ath12k_mac_assign_vif_to_vdev(hw, vif, ctx);
>> +	if (!ar) {
>> +		WARN_ON(1);
>> +		return -EINVAL;
>> +	}
>> +
>>   	ab = ar->ab;
>>   
>>   	mutex_lock(&ar->conf_mutex);
>> @@ -6842,13 +6907,22 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
>>   				   struct ieee80211_bss_conf *link_conf,
>>   				   struct ieee80211_chanctx_conf *ctx)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   	struct ath12k_base *ab;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>>   	int ret;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> +	/* The vif is expected to be attached to an ar's VDEV.
>> +	 * We leave the vif/vdev in this function as is
>> +	 * and not delete the vdev symmetric to assign_vif_chanctx()
>> +	 * the VDEV will be deleted and unassigned either during
>> +	 * remove_interface() or when there is a change in channel
>> +	 * that moves the vif to a new ar
>> +	 */
>> +	if (!arvif->is_created)
>> +		return;
>> +
>> +	ar = arvif->ar;
>>   	ab = ar->ab;
>>   
>>   	mutex_lock(&ar->conf_mutex);
>> @@ -6900,13 +6974,20 @@ ath12k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
>>   				 int n_vifs,
>>   				 enum ieee80211_chanctx_switch_mode mode)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> +	ar = ath12k_get_ar_by_ctx(hw, vifs->old_ctx);
>> +	if (!ar)
>> +		return -EINVAL;
>>   
>>   	mutex_lock(&ar->conf_mutex);
>>   
>> +	/* Switching channels across radio is not allowed */
>> +	if (ar != ath12k_get_ar_by_ctx(hw, vifs->new_ctx)) {
>> +		mutex_unlock(&ar->conf_mutex);
>> +		return -EINVAL;
>> +	}
>> +
>>   	ath12k_dbg(ar->ab, ATH12K_DBG_MAC,
>>   		   "mac chanctx switch n_vifs %d mode %d\n",
>>   		   n_vifs, mode);
> 

Re: [PATCH v4 04/12] wifi: ath12k: vdev statemachine changes for single wiphy

[kernel test robot]

Hi Rameshkumar,

kernel test robot noticed the following build warnings:

[auto build test WARNING on kvalo-ath/ath-next]
[also build test WARNING on wireless/main wireless-next/main linus/master next-20240314]
[cannot apply to v6.8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Rameshkumar-Sundaram/wifi-ath12k-add-multiple-radio-support-in-a-single-MAC-HW-un-register/20240312-215924
base:   https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git ath-next
patch link:    https://lore.kernel.org/r/20240312135557.1778379-5-quic_ramess%40quicinc.com
patch subject: [PATCH v4 04/12] wifi: ath12k: vdev statemachine changes for single wiphy
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20240314/202403141850.4ccwnsut-lkp@intel.com/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 503c55e17037436dcd45ac69dea8967e67e3f5e8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240314/202403141850.4ccwnsut-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403141850.4ccwnsut-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/net/mac80211.h:18:
   In file included from include/linux/if_ether.h:19:
   In file included from include/linux/skbuff.h:17:
   In file included from include/linux/bvec.h:10:
   In file included from include/linux/highmem.h:8:
   In file included from include/linux/cacheflush.h:5:
   In file included from arch/powerpc/include/asm/cacheflush.h:7:
   In file included from include/linux/mm.h:2188:
   include/linux/vmstat.h:508:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     508 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     509 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:515:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     515 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     516 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:522:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     522 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   include/linux/vmstat.h:527:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     527 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     528 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:536:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     536 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     537 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   In file included from drivers/net/wireless/ath/ath12k/mac.c:7:
   In file included from include/net/mac80211.h:20:
   In file included from include/linux/ieee80211.h:20:
   In file included from include/linux/etherdevice.h:21:
   In file included from include/linux/netdevice.h:45:
   In file included from include/uapi/linux/neighbour.h:6:
   In file included from include/linux/netlink.h:9:
   In file included from include/net/scm.h:9:
   In file included from include/linux/security.h:35:
   include/linux/bpf.h:742:48: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     742 |         ARG_PTR_TO_MAP_VALUE_OR_NULL    = PTR_MAYBE_NULL | ARG_PTR_TO_MAP_VALUE,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:743:43: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     743 |         ARG_PTR_TO_MEM_OR_NULL          = PTR_MAYBE_NULL | ARG_PTR_TO_MEM,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
   include/linux/bpf.h:744:43: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     744 |         ARG_PTR_TO_CTX_OR_NULL          = PTR_MAYBE_NULL | ARG_PTR_TO_CTX,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
   include/linux/bpf.h:745:45: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     745 |         ARG_PTR_TO_SOCKET_OR_NULL       = PTR_MAYBE_NULL | ARG_PTR_TO_SOCKET,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:746:44: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     746 |         ARG_PTR_TO_STACK_OR_NULL        = PTR_MAYBE_NULL | ARG_PTR_TO_STACK,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~
   include/linux/bpf.h:747:45: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     747 |         ARG_PTR_TO_BTF_ID_OR_NULL       = PTR_MAYBE_NULL | ARG_PTR_TO_BTF_ID,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:751:38: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     751 |         ARG_PTR_TO_UNINIT_MEM           = MEM_UNINIT | ARG_PTR_TO_MEM,
         |                                           ~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
   include/linux/bpf.h:753:45: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_arg_type') [-Wenum-enum-conversion]
     753 |         ARG_PTR_TO_FIXED_SIZE_MEM       = MEM_FIXED_SIZE | ARG_PTR_TO_MEM,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
   include/linux/bpf.h:776:48: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     776 |         RET_PTR_TO_MAP_VALUE_OR_NULL    = PTR_MAYBE_NULL | RET_PTR_TO_MAP_VALUE,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:777:45: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     777 |         RET_PTR_TO_SOCKET_OR_NULL       = PTR_MAYBE_NULL | RET_PTR_TO_SOCKET,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:778:47: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     778 |         RET_PTR_TO_TCP_SOCK_OR_NULL     = PTR_MAYBE_NULL | RET_PTR_TO_TCP_SOCK,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:779:50: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     779 |         RET_PTR_TO_SOCK_COMMON_OR_NULL  = PTR_MAYBE_NULL | RET_PTR_TO_SOCK_COMMON,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:781:49: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     781 |         RET_PTR_TO_DYNPTR_MEM_OR_NULL   = PTR_MAYBE_NULL | RET_PTR_TO_MEM,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
   include/linux/bpf.h:782:45: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     782 |         RET_PTR_TO_BTF_ID_OR_NULL       = PTR_MAYBE_NULL | RET_PTR_TO_BTF_ID,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:783:43: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_return_type') [-Wenum-enum-conversion]
     783 |         RET_PTR_TO_BTF_ID_TRUSTED       = PTR_TRUSTED    | RET_PTR_TO_BTF_ID,
         |                                           ~~~~~~~~~~~    ^ ~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:894:44: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_reg_type') [-Wenum-enum-conversion]
     894 |         PTR_TO_MAP_VALUE_OR_NULL        = PTR_MAYBE_NULL | PTR_TO_MAP_VALUE,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~
   include/linux/bpf.h:895:42: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_reg_type') [-Wenum-enum-conversion]
     895 |         PTR_TO_SOCKET_OR_NULL           = PTR_MAYBE_NULL | PTR_TO_SOCKET,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
   include/linux/bpf.h:896:46: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_reg_type') [-Wenum-enum-conversion]
     896 |         PTR_TO_SOCK_COMMON_OR_NULL      = PTR_MAYBE_NULL | PTR_TO_SOCK_COMMON,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~
   include/linux/bpf.h:897:44: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_reg_type') [-Wenum-enum-conversion]
     897 |         PTR_TO_TCP_SOCK_OR_NULL         = PTR_MAYBE_NULL | PTR_TO_TCP_SOCK,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~
   include/linux/bpf.h:898:42: warning: bitwise operation between different enumeration types ('enum bpf_type_flag' and 'enum bpf_reg_type') [-Wenum-enum-conversion]
     898 |         PTR_TO_BTF_ID_OR_NULL           = PTR_MAYBE_NULL | PTR_TO_BTF_ID,
         |                                           ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~
>> drivers/net/wireless/ath/ath12k/mac.c:6038:54: warning: variable 'bit' is uninitialized when used here [-Wuninitialized]
    6038 |                 ath12k_warn(ab, "failed to create vdev %d ret %d", bit, ret);
         |                                                                    ^~~
   drivers/net/wireless/ath/ath12k/mac.c:6001:8: note: initialize the variable 'bit' to silence this warning
    6001 |         u8 bit;
         |               ^
         |                = '\0'
   26 warnings generated.


vim +/bit +6038 drivers/net/wireless/ath/ath12k/mac.c

  5991	
  5992	static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
  5993							    struct ieee80211_vif *vif,
  5994							    struct ieee80211_chanctx_conf *ctx)
  5995	{
  5996		struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
  5997		struct ath12k_hw *ah = hw->priv;
  5998		struct ath12k_base *ab;
  5999		struct ath12k *ar;
  6000		int ret;
  6001		u8 bit;
  6002	
  6003		if (arvif->ar) {
  6004			WARN_ON(!arvif->is_created);
  6005			goto out;
  6006		}
  6007	
  6008		if (ah->num_radio == 1)
  6009			ar = ah->radio;
  6010		else if (ctx)
  6011			ar = ath12k_get_ar_by_ctx(hw, ctx);
  6012		else
  6013			return NULL;
  6014	
  6015		if (!ar)
  6016			goto out;
  6017	
  6018		ab = ar->ab;
  6019	
  6020		mutex_lock(&ar->conf_mutex);
  6021	
  6022		if (vif->type == NL80211_IFTYPE_AP &&
  6023		    ar->num_peers > (ar->max_num_peers - 1)) {
  6024			ath12k_warn(ab, "failed to create vdev due to insufficient peer entry resource in firmware\n");
  6025			ret = -ENOBUFS;
  6026			goto unlock;
  6027		}
  6028	
  6029		if (ar->num_created_vdevs > (TARGET_NUM_VDEVS - 1)) {
  6030			ath12k_warn(ab, "failed to create vdev, reached max vdev limit %d\n",
  6031				    TARGET_NUM_VDEVS);
  6032			ret = -EBUSY;
  6033			goto unlock;
  6034		}
  6035	
  6036		ret = ath12k_mac_vdev_create(ar, vif);
  6037		if (ret) {
> 6038			ath12k_warn(ab, "failed to create vdev %d ret %d", bit, ret);
  6039			goto unlock;
  6040		}
  6041	
  6042		/* TODO If the vdev is created during channel assign and not during
  6043		 * add_interface(), Apply any parameters for the vdev which were received
  6044		 * after add_interface, corresponding to this vif.
  6045		 */
  6046	unlock:
  6047		mutex_unlock(&ar->conf_mutex);
  6048	out:
  6049		return arvif->ar;
  6050	}
  6051	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v4 05/12] wifi: ath12k: scan statemachine changes for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

When multiple radios are advertised as a single wiphy which
supports various bands, a default scan request to mac80211
from cfg80211 will split the driver request based on band,
so each request will have channels belonging to the same band.
With this supported by default, the ath12k driver on receiving
this request checks for one of the channels in the request and
selects the corresponding radio(ar) on which the scan is going
to be performed and creates a vdev on that radio.

Note that on scan completion this vdev is not deleted. If a new
scan request is seen on that same vif for a different band the
vdev will be deleted and created on the new radio supporting the
request. The vdev delete logic is refactored to have this done
dynamically.

The reason for not deleting the vdev on scan stop is to avoid
repeated delete-create sequence if the scan is on the same band.
Also, during channel assign, new vdev creation can be optimized
as well.

Also if the scan is requested when the vdev is in started state,
no switching to new radio is allowed and scan on channels only
within same radio is allowed.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 210 +++++++++++++++++++++-----
 1 file changed, 175 insertions(+), 35 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index b6afef81a2d8..3ce1407e0e5f 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -244,6 +244,8 @@ static const u32 ath12k_smps_map[] = {
 static int ath12k_start_vdev_delay(struct ath12k *ar,
 				   struct ath12k_vif *arvif);
 static void ath12k_mac_stop(struct ath12k *ar);
+static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif);
+static int ath12k_mac_vdev_delete(struct ath12k *ar, struct ieee80211_vif *vif);
 
 static const char *ath12k_mac_phymode_str(enum wmi_phy_mode mode)
 {
@@ -3009,6 +3011,42 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 	mutex_unlock(&ar->conf_mutex);
 }
 
+static struct ath12k*
+ath12k_mac_select_scan_device(struct ieee80211_hw *hw,
+			      struct ieee80211_vif *vif,
+			      struct ieee80211_scan_request *req)
+{
+	struct ath12k_hw *ah = hw->priv;
+	enum nl80211_band band;
+	struct ath12k *ar;
+	int i;
+
+	if (ah->num_radio == 1)
+		return ah->radio;
+
+	/* Currently mac80211 supports splitting scan requests into
+	 * multiple scan requests per band.
+	 * Loop through first channel and determine the scan radio
+	 * TODO: There could be 5 GHz low/high channels in that case
+	 * split the hw request and perform multiple scans
+	 */
+
+	if (req->req.channels[0]->center_freq < ATH12K_MIN_5G_FREQ)
+		band = NL80211_BAND_2GHZ;
+	else if (req->req.channels[0]->center_freq < ATH12K_MIN_6G_FREQ)
+		band = NL80211_BAND_5GHZ;
+	else
+		band = NL80211_BAND_6GHZ;
+
+	for_each_ar(i, ah, ar) {
+		/* TODO 5 GHz low high split changes */
+		if (ar->mac.sbands[band].channels)
+			return ar;
+	}
+
+	return NULL;
+}
+
 void __ath12k_mac_scan_finish(struct ath12k *ar)
 {
 	struct ieee80211_hw *hw = ath12k_ar_to_hw(ar);
@@ -3178,15 +3216,68 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 				 struct ieee80211_scan_request *hw_req)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar;
+	struct ath12k *ar, *prev_ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct cfg80211_scan_request *req = &hw_req->req;
 	struct ath12k_wmi_scan_req_arg arg = {};
 	int ret;
 	int i;
+	bool create = true;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	if (ah->num_radio == 1) {
+		WARN_ON(!arvif->is_created);
+		ar = ath12k_ah_to_ar(ah, 0);
+		goto scan;
+	}
+
+	/* Since the targeted scan device could depend on the frequency
+	 * requested in the hw_req, select the corresponding radio
+	 */
+	ar = ath12k_mac_select_scan_device(hw, vif, hw_req);
+	if (!ar)
+		return -EINVAL;
+
+	/* If the vif is already assigned to a specific vdev of an ar,
+	 * check whether its already started, vdev which is started
+	 * are not allowed to switch to a new radio.
+	 * If the vdev is not started, but was earlier created on a
+	 * different ar, delete that vdev and create a new one. We don't
+	 * delete at the scan stop as an optimization to avoid reduntant
+	 * delete-create vdev's for the same ar, in case the request is
+	 * always on the same band for the vif
+	 */
+	if (arvif->is_created) {
+		if (WARN_ON(!arvif->ar))
+			return -EINVAL;
+
+		if (ar != arvif->ar && arvif->is_started)
+			return -EINVAL;
 
+		if (ar != arvif->ar) {
+			/* backup the previously used ar ptr, since the vdev delete
+			 * would assign the arvif->ar to NULL after the call
+			 */
+			prev_ar = arvif->ar;
+			mutex_lock(&prev_ar->conf_mutex);
+			ret = ath12k_mac_vdev_delete(prev_ar, vif);
+			mutex_unlock(&prev_ar->conf_mutex);
+			if (ret)
+				ath12k_warn(prev_ar->ab,
+					    "unable to delete scan vdev %d\n", ret);
+		} else {
+			create = false;
+		}
+	}
+	if (create) {
+		mutex_lock(&ar->conf_mutex);
+		ret = ath12k_mac_vdev_create(ar, vif);
+		mutex_unlock(&ar->conf_mutex);
+		if (ret) {
+			ath12k_warn(ar->ab, "unable to create scan vdev %d\n", ret);
+			return -EINVAL;
+		}
+	}
+scan:
 	mutex_lock(&ar->conf_mutex);
 
 	spin_lock_bh(&ar->data_lock);
@@ -3272,10 +3363,13 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 static void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
 					 struct ieee80211_vif *vif)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k *ar;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	if (!arvif->is_created)
+		return;
+
+	ar = arvif->ar;
 
 	mutex_lock(&ar->conf_mutex);
 	ath12k_scan_abort(ar);
@@ -5951,6 +6045,7 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 	if (vif->type != NL80211_IFTYPE_MONITOR && ar->monitor_conf_enabled)
 		ath12k_mac_monitor_vdev_create(ar);
 
+	arvif->ar = ar;
 	return ret;
 
 err_peer_del:
@@ -5977,6 +6072,7 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 	ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
 	ar->num_created_vdevs--;
 	arvif->is_created = false;
+	arvif->ar = NULL;
 	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
 	ab->free_vdev_map |= 1LL << arvif->vdev_id;
 	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
@@ -5995,13 +6091,45 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 {
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_hw *ah = hw->priv;
+	struct ath12k *ar, *prev_ar;
 	struct ath12k_base *ab;
-	struct ath12k *ar;
 	int ret;
 	u8 bit;
 
 	if (arvif->ar) {
-		WARN_ON(!arvif->is_created);
+		/* This is not expected really */
+		if (WARN_ON(!arvif->is_created)) {
+			arvif->ar = NULL;
+			return NULL;
+		}
+
+		if (ah->num_radio == 1)
+			goto out;
+
+		ar = ath12k_get_ar_by_ctx(hw, ctx);
+		/* This can happen as scan vdev gets created during multiple scans
+		 * across different radios before a vdev is brought up in
+		 * a certain radio
+		 */
+		if (ar != arvif->ar) {
+			if (WARN_ON(arvif->is_started))
+				return NULL;
+
+			/* backup the previously used ar ptr since arvif->ar would
+			 * be set to NULL after vdev delete is done
+			 */
+			prev_ar = arvif->ar;
+			mutex_lock(&prev_ar->conf_mutex);
+			ret = ath12k_mac_vdev_delete(prev_ar, vif);
+
+			if (ret)
+				ath12k_warn(prev_ar->ab, "unable to delete vdev %d\n", ret);
+			mutex_unlock(&prev_ar->conf_mutex);
+			ret = ath12k_mac_vdev_create(ar, vif);
+			if (ret)
+				ath12k_warn(ar->ab, "unable to create vdev %d\n", ret);
+			mutex_unlock(&ar->conf_mutex);
+		}
 		goto out;
 	}
 
@@ -6110,38 +6238,19 @@ static void ath12k_mac_vif_unref(struct ath12k_dp *dp, struct ieee80211_vif *vif
 	}
 }
 
-static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
-					   struct ieee80211_vif *vif)
+static int ath12k_mac_vdev_delete(struct ath12k *ar, struct ieee80211_vif *vif)
 {
-	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
-	struct ath12k_base *ab;
+	struct ath12k_base *ab = ar->ab;
 	unsigned long time_left;
 	int ret;
 
-	if (!arvif->is_created)
-		return;
-
-	ar = arvif->ar;
-	ab = ar->ab;
-
-	mutex_lock(&ar->conf_mutex);
-
-	ath12k_dbg(ab, ATH12K_DBG_MAC, "mac remove interface (vdev %d)\n",
-		   arvif->vdev_id);
-
-	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
-		ret = ath12k_peer_delete(ar, arvif->vdev_id, vif->addr);
-		if (ret)
-			ath12k_warn(ab, "failed to submit AP self-peer removal on vdev %d: %d\n",
-				    arvif->vdev_id, ret);
-	}
-
+	lockdep_assert_held(&ar->conf_mutex);
 	reinit_completion(&ar->vdev_delete_done);
 
 	ret = ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
 	if (ret) {
-		ath12k_warn(ab, "failed to delete WMI vdev %d: %d\n",
+		ath12k_warn(ab, "failed to delete WMI scan vdev %d: %d\n",
 			    arvif->vdev_id, ret);
 		goto err_vdev_del;
 	}
@@ -6153,6 +6262,10 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 		goto err_vdev_del;
 	}
 
+	ab->free_vdev_map |= 1LL << arvif->vdev_id;
+	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
+	ar->num_created_vdevs--;
+
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR) {
 		ar->monitor_vdev_id = -1;
 		ar->monitor_vdev_created = false;
@@ -6160,12 +6273,6 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 		ret = ath12k_mac_monitor_vdev_delete(ar);
 	}
 
-	ab->free_vdev_map |= 1LL << (arvif->vdev_id);
-	ar->allocated_vdev_map &= ~(1LL << arvif->vdev_id);
-	ab->free_vdev_stats_id_map &= ~(1LL << arvif->vdev_stats_id);
-	ar->num_created_vdevs--;
-	arvif->is_created = false;
-
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "vdev %pM deleted, vdev_id %d\n",
 		   vif->addr, arvif->vdev_id);
 
@@ -6187,6 +6294,39 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 	clear_bit(ATH12K_FLAG_MONITOR_ENABLED, &ar->monitor_flags);
 
 	/* TODO: recal traffic pause state based on the available vdevs */
+	arvif->is_created = false;
+	arvif->ar = NULL;
+
+	return ret;
+}
+
+static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	struct ath12k_base *ab;
+	struct ath12k *ar;
+	int ret;
+
+	if (!arvif->is_created)
+		return;
+
+	ar = arvif->ar;
+	ab = ar->ab;
+
+	mutex_lock(&ar->conf_mutex);
+
+	ath12k_dbg(ab, ATH12K_DBG_MAC, "mac remove interface (vdev %d)\n",
+		   arvif->vdev_id);
+
+	if (arvif->vdev_type == WMI_VDEV_TYPE_AP) {
+		ret = ath12k_peer_delete(ar, arvif->vdev_id, vif->addr);
+		if (ret)
+			ath12k_warn(ab, "failed to submit AP self-peer removal on vdev %d: %d\n",
+				    arvif->vdev_id, ret);
+	}
+
+	ath12k_mac_vdev_delete(ar, vif);
 
 	mutex_unlock(&ar->conf_mutex);
 }
-- 
2.25.1

Re: [PATCH v4 05/12] wifi: ath12k: scan statemachine changes for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> When multiple radios are advertised as a single wiphy which
> supports various bands, a default scan request to mac80211
> from cfg80211 will split the driver request based on band,
> so each request will have channels belonging to the same band.
> With this supported by default, the ath12k driver on receiving
> this request checks for one of the channels in the request and
> selects the corresponding radio(ar) on which the scan is going
> to be performed and creates a vdev on that radio.
> 
> Note that on scan completion this vdev is not deleted. If a new
> scan request is seen on that same vif for a different band the
> vdev will be deleted and created on the new radio supporting the
> request. The vdev delete logic is refactored to have this done
> dynamically.
> 
> The reason for not deleting the vdev on scan stop is to avoid
> repeated delete-create sequence if the scan is on the same band.
> Also, during channel assign, new vdev creation can be optimized
> as well.
> 
> Also if the scan is requested when the vdev is in started state,
> no switching to new radio is allowed and scan on channels only
> within same radio is allowed.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
...
> +	/* If the vif is already assigned to a specific vdev of an ar,
> +	 * check whether its already started, vdev which is started
> +	 * are not allowed to switch to a new radio.
> +	 * If the vdev is not started, but was earlier created on a
> +	 * different ar, delete that vdev and create a new one. We don't
> +	 * delete at the scan stop as an optimization to avoid reduntant

s/reduntant/redundant/

> +	 * delete-create vdev's for the same ar, in case the request is
> +	 * always on the same band for the vif
> +	 */

Re: [PATCH v4 05/12] wifi: ath12k: scan statemachine changes for single wiphy

[Rameshkumar Sundaram]

On 3/13/2024 4:05 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> When multiple radios are advertised as a single wiphy which
>> supports various bands, a default scan request to mac80211
>> from cfg80211 will split the driver request based on band,
>> so each request will have channels belonging to the same band.
>> With this supported by default, the ath12k driver on receiving
>> this request checks for one of the channels in the request and
>> selects the corresponding radio(ar) on which the scan is going
>> to be performed and creates a vdev on that radio.
>>
>> Note that on scan completion this vdev is not deleted. If a new
>> scan request is seen on that same vif for a different band the
>> vdev will be deleted and created on the new radio supporting the
>> request. The vdev delete logic is refactored to have this done
>> dynamically.
>>
>> The reason for not deleting the vdev on scan stop is to avoid
>> repeated delete-create sequence if the scan is on the same band.
>> Also, during channel assign, new vdev creation can be optimized
>> as well.
>>
>> Also if the scan is requested when the vdev is in started state,
>> no switching to new radio is allowed and scan on channels only
>> within same radio is allowed.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
> ...
>> +	/* If the vif is already assigned to a specific vdev of an ar,
>> +	 * check whether its already started, vdev which is started
>> +	 * are not allowed to switch to a new radio.
>> +	 * If the vdev is not started, but was earlier created on a
>> +	 * different ar, delete that vdev and create a new one. We don't
>> +	 * delete at the scan stop as an optimization to avoid reduntant
> 
> s/reduntant/redundant/
Sure will fix it in next version
> 
>> +	 * delete-create vdev's for the same ar, in case the request is
>> +	 * always on the same band for the vif
>> +	 */
> 

[PATCH v4 06/12] wifi: ath12k: fetch correct radio based on vdev status

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

For ops which passes the vif info, fetch the radio(ar)
to be used for corresponding functions based on the
the vdev creation status. If the vdev is not created yet,
which could happen when the ops are called before channel
is assigned for the vif, the data needs to be cached and this
is done in followup changes.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 75 +++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 3ce1407e0e5f..6d2176b0a556 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -645,6 +645,24 @@ static struct ath12k *ath12k_get_ar_by_ctx(struct ieee80211_hw *hw,
 	return ath12k_mac_get_ar_by_chan(hw, ctx->def.chan);
 }
 
+static struct ath12k *ath12k_get_ar_by_vif(struct ieee80211_hw *hw,
+					   struct ieee80211_vif *vif)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
+
+	/* If there is one pdev within ah, then we return
+	 * ar directly.
+	 */
+	if (ah->num_radio == 1)
+		return ah->radio;
+
+	if (arvif->is_created)
+		return arvif->ar;
+
+	return NULL;
+}
+
 static void ath12k_pdev_caps_update(struct ath12k *ar)
 {
 	struct ath12k_base *ab = ar->ab;
@@ -2998,11 +3016,17 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 					   struct ieee80211_bss_conf *info,
 					   u64 changed)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_vif(hw, vif);
+
+	/* TODO if the vdev is not created on a certain radio,
+	 * cache the info to be updated later on vdev creation
+	 */
+
+	if (!ar)
+		return;
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -3497,7 +3521,6 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
 				 struct ieee80211_key_conf *key)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_base *ab;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
@@ -3514,7 +3537,11 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
 		return 1;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar) {
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
 	ab = ar->ab;
 
 	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags))
@@ -4092,7 +4119,6 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 				   enum ieee80211_sta_state old_state,
 				   enum ieee80211_sta_state new_state)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_sta *arsta = ath12k_sta_to_arsta(sta);
@@ -4104,7 +4130,11 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 	     new_state == IEEE80211_STA_NOTEXIST))
 		cancel_work_sync(&arsta->update_wk);
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar) {
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -4255,14 +4285,17 @@ static void ath12k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
 					struct ieee80211_sta *sta,
 					u32 changed)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_sta *arsta = ath12k_sta_to_arsta(sta);
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_peer *peer;
 	u32 bw, smps;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar) {
+		WARN_ON_ONCE(1);
+		return;
+	}
 
 	spin_lock_bh(&ar->ab->base_lock);
 
@@ -4438,12 +4471,15 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
 				 unsigned int link_id, u16 ac,
 				 const struct ieee80211_tx_queue_params *params)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
-	ar = ath12k_ah_to_ar(ah, 0);
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar) {
+		/* TODO cache the info and apply after vdev is created */
+		return -EINVAL;
+	}
 
 	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_conf_tx(arvif, link_id, ac, params);
@@ -6459,6 +6495,10 @@ static int ath12k_mac_op_ampdu_action(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret = -EINVAL;
 
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar)
+		return -EINVAL;
+
 	ar = ath12k_ah_to_ar(ah, 0);
 
 	mutex_lock(&ar->conf_mutex);
@@ -7214,11 +7254,24 @@ static void ath12k_mac_op_flush(struct ieee80211_hw *hw, struct ieee80211_vif *v
 				u32 queues, bool drop)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
-	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
+	struct ath12k *ar;
+	int i;
 
 	if (drop)
 		return;
 
+	/* vif can be NULL when flush() is considered for hw */
+	if (!vif) {
+		for_each_ar(i, ah, ar)
+			ath12k_mac_flush(ar);
+		return;
+	}
+
+	ar = ath12k_get_ar_by_vif(hw, vif);
+
+	if (!ar)
+		return;
+
 	ath12k_mac_flush(ar);
 }
 
-- 
2.25.1

Re: [PATCH v4 06/12] wifi: ath12k: fetch correct radio based on vdev status

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> For ops which passes the vif info, fetch the radio(ar)
> to be used for corresponding functions based on the
> the vdev creation status. If the vdev is not created yet,
> which could happen when the ops are called before channel
> is assigned for the vif, the data needs to be cached and this
> is done in followup changes.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 07/12] wifi: ath12k: Cache vdev configs before vdev create

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

Since the vdev create for a corresponding vif is deferred
until a channel is assigned, cache the information which
are received through mac80211 ops between add_interface()
and assign_vif_chanctx() and set them once the vdev is
created on one of the ath12k radios as the channel gets
assigned via assign_vif_chanctx().

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  19 ++++
 drivers/net/wireless/ath/ath12k/mac.c  | 116 +++++++++++++++++++------
 2 files changed, 107 insertions(+), 28 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 70daec38d486..fe151aa90687 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -214,6 +214,24 @@ enum ath12k_monitor_flags {
 	ATH12K_FLAG_MONITOR_ENABLED,
 };
 
+struct ath12k_tx_conf {
+	bool changed;
+	u16 ac;
+	struct ieee80211_tx_queue_params tx_queue_params;
+};
+
+struct ath12k_key_conf {
+	bool changed;
+	enum set_key_cmd cmd;
+	struct ieee80211_key_conf *key;
+};
+
+struct ath12k_vif_cache {
+	struct ath12k_tx_conf tx_conf;
+	struct ath12k_key_conf key_conf;
+	u32 bss_conf_changed;
+};
+
 struct ath12k_vif {
 	u32 vdev_id;
 	enum wmi_vdev_type vdev_type;
@@ -268,6 +286,7 @@ struct ath12k_vif {
 	u8 vdev_stats_id;
 	u32 punct_bitmap;
 	bool ps;
+	struct ath12k_vif_cache cache;
 };
 
 struct ath12k_vif_iter {
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 6d2176b0a556..fab92f08fdb7 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -3021,12 +3021,14 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 
 	ar = ath12k_get_ar_by_vif(hw, vif);
 
-	/* TODO if the vdev is not created on a certain radio,
+	/* if the vdev is not created on a certain radio,
 	 * cache the info to be updated later on vdev creation
 	 */
 
-	if (!ar)
+	if (!ar) {
+		arvif->cache.bss_conf_changed |= changed;
 		return;
+	}
 
 	mutex_lock(&ar->conf_mutex);
 
@@ -3517,12 +3519,11 @@ static int ath12k_clear_peer_keys(struct ath12k_vif *arvif,
 	return first_errno;
 }
 
-static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
-				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
-				 struct ieee80211_key_conf *key)
+static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
+			      struct ieee80211_vif *vif, struct ieee80211_sta *sta,
+			      struct ieee80211_key_conf *key)
 {
-	struct ath12k *ar;
-	struct ath12k_base *ab;
+	struct ath12k_base *ab = ar->ab;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_peer *peer;
 	struct ath12k_sta *arsta;
@@ -3530,28 +3531,11 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	int ret = 0;
 	u32 flags = 0;
 
-	/* BIP needs to be done in software */
-	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
-	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
-	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
-	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
-		return 1;
-
-	ar = ath12k_get_ar_by_vif(hw, vif);
-	if (!ar) {
-		WARN_ON_ONCE(1);
-		return -EINVAL;
-	}
-	ab = ar->ab;
+	lockdep_assert_held(&ar->conf_mutex);
 
-	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags))
+	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ab->dev_flags))
 		return 1;
 
-	if (key->keyidx > WMI_MAX_KEY_INDEX)
-		return -ENOSPC;
-
-	mutex_lock(&ar->conf_mutex);
-
 	if (sta)
 		peer_addr = sta->addr;
 	else if (arvif->vdev_type == WMI_VDEV_TYPE_STA)
@@ -3643,6 +3627,43 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	spin_unlock_bh(&ab->base_lock);
 
 exit:
+	return ret;
+}
+
+
+static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
+				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
+				 struct ieee80211_key_conf *key)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	struct ath12k *ar;
+	int ret;
+
+	/* BIP needs to be done in software */
+	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
+	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
+	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
+	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
+		return 1;
+
+	if (key->keyidx > WMI_MAX_KEY_INDEX)
+		return -ENOSPC;
+
+	ar = ath12k_get_ar_by_vif(hw, vif);
+	if (!ar) {
+		/* ar is expected to be valid when sta ptr is available */
+		if (sta) {
+			WARN_ON_ONCE(1);
+			return -EINVAL;
+		}
+		arvif->cache.key_conf.cmd = cmd;
+		arvif->cache.key_conf.key = key;
+		arvif->cache.key_conf.changed = true;
+		return 0;
+	}
+
+	mutex_lock(&ar->conf_mutex);
+	ret = ath12k_mac_set_key(ar, cmd, vif, sta, key);
 	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
@@ -4477,7 +4498,10 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
 
 	ar = ath12k_get_ar_by_vif(hw, vif);
 	if (!ar) {
-		/* TODO cache the info and apply after vdev is created */
+		/* cache the info and apply after vdev is created */
+		arvif->cache.tx_conf.changed = true;
+		arvif->cache.tx_conf.ac = ac;
+		arvif->cache.tx_conf.tx_queue_params = *params;
 		return -EINVAL;
 	}
 
@@ -6121,6 +6145,41 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 	return ret;
 }
 
+static void ath12k_mac_vif_cache_flush(struct ath12k *ar,  struct ieee80211_vif *vif)
+{
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
+	struct ath12k_base *ab = ar->ab;
+	int ret;
+
+	lockdep_assert_held(&ar->conf_mutex);
+
+	if (arvif->cache.tx_conf.changed) {
+		ret = ath12k_mac_conf_tx(arvif, 0, arvif->cache.tx_conf.ac,
+					 &arvif->cache.tx_conf.tx_queue_params);
+		if (ret)
+			ath12k_warn(ab,
+				    "unable to apply tx config parameters to vdev %d\n",
+				    ret);
+	}
+
+	if (arvif->cache.bss_conf_changed)
+		ath12k_mac_bss_info_changed(ar, arvif, &vif->bss_conf,
+					    arvif->cache.bss_conf_changed);
+
+	if (arvif->cache.key_conf.changed) {
+		ret = ath12k_mac_set_key(ar, arvif->cache.key_conf.cmd,
+					 vif, NULL,
+					 arvif->cache.key_conf.key);
+		if (ret) {
+			WARN_ON_ONCE(1);
+			ath12k_warn(ab, "unable to apply set key param to vdev %d ret %d\n",
+				    arvif->vdev_id, ret);
+		}
+	}
+
+	memset(&arvif->cache, 0, sizeof(struct ath12k_vif_cache));
+}
+
 static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 						    struct ieee80211_vif *vif,
 						    struct ieee80211_chanctx_conf *ctx)
@@ -6203,10 +6262,11 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 		goto unlock;
 	}
 
-	/* TODO If the vdev is created during channel assign and not during
+	/* If the vdev is created during channel assign and not during
 	 * add_interface(), Apply any parameters for the vdev which were received
 	 * after add_interface, corresponding to this vif.
 	 */
+	ath12k_mac_vif_cache_flush(ar, vif);
 unlock:
 	mutex_unlock(&ar->conf_mutex);
 out:
-- 
2.25.1

Re: [PATCH v4 07/12] wifi: ath12k: Cache vdev configs before vdev create

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> Since the vdev create for a corresponding vif is deferred
> until a channel is assigned, cache the information which
> are received through mac80211 ops between add_interface()
> and assign_vif_chanctx() and set them once the vdev is
> created on one of the ath12k radios as the channel gets
> assigned via assign_vif_chanctx().
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.h |  19 ++++
>  drivers/net/wireless/ath/ath12k/mac.c  | 116 +++++++++++++++++++------
>  2 files changed, 107 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 70daec38d486..fe151aa90687 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -214,6 +214,24 @@ enum ath12k_monitor_flags {
>  	ATH12K_FLAG_MONITOR_ENABLED,
>  };
>  
> +struct ath12k_tx_conf {
> +	bool changed;
> +	u16 ac;
> +	struct ieee80211_tx_queue_params tx_queue_params;
> +};
> +
> +struct ath12k_key_conf {
> +	bool changed;
> +	enum set_key_cmd cmd;
> +	struct ieee80211_key_conf *key;
> +};
> +
> +struct ath12k_vif_cache {
> +	struct ath12k_tx_conf tx_conf;
> +	struct ath12k_key_conf key_conf;
> +	u32 bss_conf_changed;
> +};
> +
>  struct ath12k_vif {
>  	u32 vdev_id;
>  	enum wmi_vdev_type vdev_type;
> @@ -268,6 +286,7 @@ struct ath12k_vif {
>  	u8 vdev_stats_id;
>  	u32 punct_bitmap;
>  	bool ps;
> +	struct ath12k_vif_cache cache;

is there a reason this isn't a *cache?
this allocation is only needed for the time between when a vdev is created and
when a channel is assigned, so why not have a dynamic allocation that is only
around for that time? otherwise for every vdev you waste this memory for the
lifetime of the vdev.

>  };
>  
>  struct ath12k_vif_iter {
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 6d2176b0a556..fab92f08fdb7 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -3021,12 +3021,14 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>  
>  	ar = ath12k_get_ar_by_vif(hw, vif);
>  
> -	/* TODO if the vdev is not created on a certain radio,
> +	/* if the vdev is not created on a certain radio,
>  	 * cache the info to be updated later on vdev creation
>  	 */
>  
> -	if (!ar)
> +	if (!ar) {
> +		arvif->cache.bss_conf_changed |= changed;

why don't you need to save the actual *info as well?

>  		return;
> +	}
>  
>  	mutex_lock(&ar->conf_mutex);
>  
> @@ -3517,12 +3519,11 @@ static int ath12k_clear_peer_keys(struct ath12k_vif *arvif,
>  	return first_errno;
>  }
>  
> -static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> -				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
> -				 struct ieee80211_key_conf *key)
> +static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
> +			      struct ieee80211_vif *vif, struct ieee80211_sta *sta,
> +			      struct ieee80211_key_conf *key)
>  {
> -	struct ath12k *ar;
> -	struct ath12k_base *ab;
> +	struct ath12k_base *ab = ar->ab;
>  	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>  	struct ath12k_peer *peer;
>  	struct ath12k_sta *arsta;
> @@ -3530,28 +3531,11 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  	int ret = 0;
>  	u32 flags = 0;
>  
> -	/* BIP needs to be done in software */
> -	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
> -		return 1;
> -
> -	ar = ath12k_get_ar_by_vif(hw, vif);
> -	if (!ar) {
> -		WARN_ON_ONCE(1);
> -		return -EINVAL;
> -	}
> -	ab = ar->ab;
> +	lockdep_assert_held(&ar->conf_mutex);
>  
> -	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags))
> +	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ab->dev_flags))
>  		return 1;
>  
> -	if (key->keyidx > WMI_MAX_KEY_INDEX)
> -		return -ENOSPC;
> -
> -	mutex_lock(&ar->conf_mutex);
> -
>  	if (sta)
>  		peer_addr = sta->addr;
>  	else if (arvif->vdev_type == WMI_VDEV_TYPE_STA)
> @@ -3643,6 +3627,43 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>  	spin_unlock_bh(&ab->base_lock);
>  
>  exit:
> +	return ret;
> +}
> +
> +
> +static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
> +				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
> +				 struct ieee80211_key_conf *key)
> +{
> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> +	struct ath12k *ar;
> +	int ret;
> +
> +	/* BIP needs to be done in software */
> +	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
> +		return 1;
> +
> +	if (key->keyidx > WMI_MAX_KEY_INDEX)
> +		return -ENOSPC;
> +
> +	ar = ath12k_get_ar_by_vif(hw, vif);
> +	if (!ar) {
> +		/* ar is expected to be valid when sta ptr is available */
> +		if (sta) {
> +			WARN_ON_ONCE(1);
> +			return -EINVAL;
> +		}
> +		arvif->cache.key_conf.cmd = cmd;
> +		arvif->cache.key_conf.key = key;
> +		arvif->cache.key_conf.changed = true;
> +		return 0;
> +	}
> +
> +	mutex_lock(&ar->conf_mutex);
> +	ret = ath12k_mac_set_key(ar, cmd, vif, sta, key);
>  	mutex_unlock(&ar->conf_mutex);
>  	return ret;
>  }
> @@ -4477,7 +4498,10 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
>  
>  	ar = ath12k_get_ar_by_vif(hw, vif);
>  	if (!ar) {
> -		/* TODO cache the info and apply after vdev is created */
> +		/* cache the info and apply after vdev is created */
> +		arvif->cache.tx_conf.changed = true;
> +		arvif->cache.tx_conf.ac = ac;
> +		arvif->cache.tx_conf.tx_queue_params = *params;
>  		return -EINVAL;
>  	}
>  
> @@ -6121,6 +6145,41 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
>  	return ret;
>  }
>  
> +static void ath12k_mac_vif_cache_flush(struct ath12k *ar,  struct ieee80211_vif *vif)
> +{
> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
> +	struct ath12k_base *ab = ar->ab;
> +	int ret;
> +
> +	lockdep_assert_held(&ar->conf_mutex);
> +
> +	if (arvif->cache.tx_conf.changed) {
> +		ret = ath12k_mac_conf_tx(arvif, 0, arvif->cache.tx_conf.ac,
> +					 &arvif->cache.tx_conf.tx_queue_params);
> +		if (ret)
> +			ath12k_warn(ab,
> +				    "unable to apply tx config parameters to vdev %d\n",
> +				    ret);
> +	}
> +
> +	if (arvif->cache.bss_conf_changed)
> +		ath12k_mac_bss_info_changed(ar, arvif, &vif->bss_conf,
> +					    arvif->cache.bss_conf_changed);
> +
> +	if (arvif->cache.key_conf.changed) {
> +		ret = ath12k_mac_set_key(ar, arvif->cache.key_conf.cmd,
> +					 vif, NULL,
> +					 arvif->cache.key_conf.key);
> +		if (ret) {
> +			WARN_ON_ONCE(1);
> +			ath12k_warn(ab, "unable to apply set key param to vdev %d ret %d\n",
> +				    arvif->vdev_id, ret);
> +		}
> +	}
> +
> +	memset(&arvif->cache, 0, sizeof(struct ath12k_vif_cache));

sizeof(arvif->cache)

> +}
> +
>  static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
>  						    struct ieee80211_vif *vif,
>  						    struct ieee80211_chanctx_conf *ctx)
> @@ -6203,10 +6262,11 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
>  		goto unlock;
>  	}
>  
> -	/* TODO If the vdev is created during channel assign and not during
> +	/* If the vdev is created during channel assign and not during
>  	 * add_interface(), Apply any parameters for the vdev which were received
>  	 * after add_interface, corresponding to this vif.
>  	 */
> +	ath12k_mac_vif_cache_flush(ar, vif);
>  unlock:
>  	mutex_unlock(&ar->conf_mutex);
>  out:

Re: [PATCH v4 07/12] wifi: ath12k: Cache vdev configs before vdev create

[Rameshkumar Sundaram]

On 3/13/2024 4:31 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> Since the vdev create for a corresponding vif is deferred
>> until a channel is assigned, cache the information which
>> are received through mac80211 ops between add_interface()
>> and assign_vif_chanctx() and set them once the vdev is
>> created on one of the ath12k radios as the channel gets
>> assigned via assign_vif_chanctx().
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.h |  19 ++++
>>   drivers/net/wireless/ath/ath12k/mac.c  | 116 +++++++++++++++++++------
>>   2 files changed, 107 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 70daec38d486..fe151aa90687 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -214,6 +214,24 @@ enum ath12k_monitor_flags {
>>   	ATH12K_FLAG_MONITOR_ENABLED,
>>   };
>>   
>> +struct ath12k_tx_conf {
>> +	bool changed;
>> +	u16 ac;
>> +	struct ieee80211_tx_queue_params tx_queue_params;
>> +};
>> +
>> +struct ath12k_key_conf {
>> +	bool changed;
>> +	enum set_key_cmd cmd;
>> +	struct ieee80211_key_conf *key;
>> +};
>> +
>> +struct ath12k_vif_cache {
>> +	struct ath12k_tx_conf tx_conf;
>> +	struct ath12k_key_conf key_conf;
>> +	u32 bss_conf_changed;
>> +};
>> +
>>   struct ath12k_vif {
>>   	u32 vdev_id;
>>   	enum wmi_vdev_type vdev_type;
>> @@ -268,6 +286,7 @@ struct ath12k_vif {
>>   	u8 vdev_stats_id;
>>   	u32 punct_bitmap;
>>   	bool ps;
>> +	struct ath12k_vif_cache cache;
> 
> is there a reason this isn't a *cache?
> this allocation is only needed for the time between when a vdev is created and
> when a channel is assigned, so why not have a dynamic allocation that is only
> around for that time? otherwise for every vdev you waste this memory for the
> lifetime of the vdev.
> 
There is no specific reason behind it.
Your point makes sense, we need not have this for lifetime of the vdev. 
Shall we have cache as such and have *tx_conf and *key_conf as 
dynamically allocated members ?
That way we can allocate them whenever corresponding config is received 
and have it in cache.
>>   };
>>   
>>   struct ath12k_vif_iter {
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 6d2176b0a556..fab92f08fdb7 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -3021,12 +3021,14 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
>>   
>>   	ar = ath12k_get_ar_by_vif(hw, vif);
>>   
>> -	/* TODO if the vdev is not created on a certain radio,
>> +	/* if the vdev is not created on a certain radio,
>>   	 * cache the info to be updated later on vdev creation
>>   	 */
>>   
>> -	if (!ar)
>> +	if (!ar) {
>> +		arvif->cache.bss_conf_changed |= changed;
> 
> why don't you need to save the actual *info as well?
> 
info(ieee80211_bss_conf) can be obtained from arvif->vif(ieee80211_vif) 
whenever needed.
>>   		return;
>> +	}
>>   
>>   	mutex_lock(&ar->conf_mutex);
>>   
>> @@ -3517,12 +3519,11 @@ static int ath12k_clear_peer_keys(struct ath12k_vif *arvif,
>>   	return first_errno;
>>   }
>>   
>> -static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> -				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
>> -				 struct ieee80211_key_conf *key)
>> +static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
>> +			      struct ieee80211_vif *vif, struct ieee80211_sta *sta,
>> +			      struct ieee80211_key_conf *key)
>>   {
>> -	struct ath12k *ar;
>> -	struct ath12k_base *ab;
>> +	struct ath12k_base *ab = ar->ab;
>>   	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>>   	struct ath12k_peer *peer;
>>   	struct ath12k_sta *arsta;
>> @@ -3530,28 +3531,11 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   	int ret = 0;
>>   	u32 flags = 0;
>>   
>> -	/* BIP needs to be done in software */
>> -	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
>> -	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
>> -		return 1;
>> -
>> -	ar = ath12k_get_ar_by_vif(hw, vif);
>> -	if (!ar) {
>> -		WARN_ON_ONCE(1);
>> -		return -EINVAL;
>> -	}
>> -	ab = ar->ab;
>> +	lockdep_assert_held(&ar->conf_mutex);
>>   
>> -	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ar->ab->dev_flags))
>> +	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ab->dev_flags))
>>   		return 1;
>>   
>> -	if (key->keyidx > WMI_MAX_KEY_INDEX)
>> -		return -ENOSPC;
>> -
>> -	mutex_lock(&ar->conf_mutex);
>> -
>>   	if (sta)
>>   		peer_addr = sta->addr;
>>   	else if (arvif->vdev_type == WMI_VDEV_TYPE_STA)
>> @@ -3643,6 +3627,43 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>>   	spin_unlock_bh(&ab->base_lock);
>>   
>>   exit:
>> +	return ret;
>> +}
>> +
>> +
>> +static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
>> +				 struct ieee80211_vif *vif, struct ieee80211_sta *sta,
>> +				 struct ieee80211_key_conf *key)
>> +{
>> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>> +	struct ath12k *ar;
>> +	int ret;
>> +
>> +	/* BIP needs to be done in software */
>> +	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
>> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
>> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_256 ||
>> +	    key->cipher == WLAN_CIPHER_SUITE_BIP_CMAC_256)
>> +		return 1;
>> +
>> +	if (key->keyidx > WMI_MAX_KEY_INDEX)
>> +		return -ENOSPC;
>> +
>> +	ar = ath12k_get_ar_by_vif(hw, vif);
>> +	if (!ar) {
>> +		/* ar is expected to be valid when sta ptr is available */
>> +		if (sta) {
>> +			WARN_ON_ONCE(1);
>> +			return -EINVAL;
>> +		}
>> +		arvif->cache.key_conf.cmd = cmd;
>> +		arvif->cache.key_conf.key = key;
>> +		arvif->cache.key_conf.changed = true;
>> +		return 0;
>> +	}
>> +
>> +	mutex_lock(&ar->conf_mutex);
>> +	ret = ath12k_mac_set_key(ar, cmd, vif, sta, key);
>>   	mutex_unlock(&ar->conf_mutex);
>>   	return ret;
>>   }
>> @@ -4477,7 +4498,10 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
>>   
>>   	ar = ath12k_get_ar_by_vif(hw, vif);
>>   	if (!ar) {
>> -		/* TODO cache the info and apply after vdev is created */
>> +		/* cache the info and apply after vdev is created */
>> +		arvif->cache.tx_conf.changed = true;
>> +		arvif->cache.tx_conf.ac = ac;
>> +		arvif->cache.tx_conf.tx_queue_params = *params;
>>   		return -EINVAL;
>>   	}
>>   
>> @@ -6121,6 +6145,41 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
>>   	return ret;
>>   }
>>   
>> +static void ath12k_mac_vif_cache_flush(struct ath12k *ar,  struct ieee80211_vif *vif)
>> +{
>> +	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
>> +	struct ath12k_base *ab = ar->ab;
>> +	int ret;
>> +
>> +	lockdep_assert_held(&ar->conf_mutex);
>> +
>> +	if (arvif->cache.tx_conf.changed) {
>> +		ret = ath12k_mac_conf_tx(arvif, 0, arvif->cache.tx_conf.ac,
>> +					 &arvif->cache.tx_conf.tx_queue_params);
>> +		if (ret)
>> +			ath12k_warn(ab,
>> +				    "unable to apply tx config parameters to vdev %d\n",
>> +				    ret);
>> +	}
>> +
>> +	if (arvif->cache.bss_conf_changed)
>> +		ath12k_mac_bss_info_changed(ar, arvif, &vif->bss_conf,
>> +					    arvif->cache.bss_conf_changed);
>> +
>> +	if (arvif->cache.key_conf.changed) {
>> +		ret = ath12k_mac_set_key(ar, arvif->cache.key_conf.cmd,
>> +					 vif, NULL,
>> +					 arvif->cache.key_conf.key);
>> +		if (ret) {
>> +			WARN_ON_ONCE(1);
>> +			ath12k_warn(ab, "unable to apply set key param to vdev %d ret %d\n",
>> +				    arvif->vdev_id, ret);
>> +		}
>> +	}
>> +
>> +	memset(&arvif->cache, 0, sizeof(struct ath12k_vif_cache));
> 
> sizeof(arvif->cache)
> 
>> +}
>> +
>>   static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
>>   						    struct ieee80211_vif *vif,
>>   						    struct ieee80211_chanctx_conf *ctx)
>> @@ -6203,10 +6262,11 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
>>   		goto unlock;
>>   	}
>>   
>> -	/* TODO If the vdev is created during channel assign and not during
>> +	/* If the vdev is created during channel assign and not during
>>   	 * add_interface(), Apply any parameters for the vdev which were received
>>   	 * after add_interface, corresponding to this vif.
>>   	 */
>> +	ath12k_mac_vif_cache_flush(ar, vif);
>>   unlock:
>>   	mutex_unlock(&ar->conf_mutex);
>>   out:
> 

[PATCH v4 08/12] wifi: ath12k: Add additional checks for vif and sta iterators

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

Since vif and sta objects of different radios are added to same
local hw list in mac80211, additional checks need to be done
in driver to ensure we are processing the intended vif
and sta corresponding to the radio when the vif and sta mac80211
iterator utils are used from driver.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  1 +
 drivers/net/wireless/ath/ath12k/mac.c  | 22 ++++++++++++++++++++--
 drivers/net/wireless/ath/ath12k/p2p.c  |  3 ++-
 drivers/net/wireless/ath/ath12k/p2p.h  |  1 +
 4 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index fe151aa90687..4b21e7bcec3f 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -291,6 +291,7 @@ struct ath12k_vif {
 
 struct ath12k_vif_iter {
 	u32 vdev_id;
+	struct ath12k *ar;
 	struct ath12k_vif *arvif;
 };
 
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index fab92f08fdb7..a77d6dbaea46 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -533,7 +533,8 @@ static void ath12k_get_arvif_iter(void *data, u8 *mac,
 	struct ath12k_vif_iter *arvif_iter = data;
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 
-	if (arvif->vdev_id == arvif_iter->vdev_id)
+	if (arvif->vdev_id == arvif_iter->vdev_id &&
+	    arvif->ar == arvif_iter->ar)
 		arvif_iter->arvif = arvif;
 }
 
@@ -543,6 +544,7 @@ struct ath12k_vif *ath12k_mac_get_arvif(struct ath12k *ar, u32 vdev_id)
 	u32 flags;
 
 	arvif_iter.vdev_id = vdev_id;
+	arvif_iter.ar = ar;
 
 	flags = IEEE80211_IFACE_ITER_RESUME_ALL;
 	ieee80211_iterate_active_interfaces_atomic(ath12k_ar_to_hw(ar),
@@ -6818,14 +6820,19 @@ struct ath12k_mac_change_chanctx_arg {
 	struct ieee80211_vif_chanctx_switch *vifs;
 	int n_vifs;
 	int next_vif;
+	struct ath12k *ar;
 };
 
 static void
 ath12k_mac_change_chanctx_cnt_iter(void *data, u8 *mac,
 				   struct ieee80211_vif *vif)
 {
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_mac_change_chanctx_arg *arg = data;
 
+	if (arvif->ar != arg->ar)
+		return;
+
 	if (rcu_access_pointer(vif->bss_conf.chanctx_conf) != arg->ctx)
 		return;
 
@@ -6836,9 +6843,13 @@ static void
 ath12k_mac_change_chanctx_fill_iter(void *data, u8 *mac,
 				    struct ieee80211_vif *vif)
 {
+	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_mac_change_chanctx_arg *arg = data;
 	struct ieee80211_chanctx_conf *ctx;
 
+	if (arvif->ar != arg->ar)
+		return;
+
 	ctx = rcu_access_pointer(vif->bss_conf.chanctx_conf);
 	if (ctx != arg->ctx)
 		return;
@@ -6957,7 +6968,7 @@ static void
 ath12k_mac_update_active_vif_chan(struct ath12k *ar,
 				  struct ieee80211_chanctx_conf *ctx)
 {
-	struct ath12k_mac_change_chanctx_arg arg = { .ctx = ctx };
+	struct ath12k_mac_change_chanctx_arg arg = { .ctx = ctx, .ar = ar };
 	struct ieee80211_hw *hw = ath12k_ar_to_hw(ar);
 
 	lockdep_assert_held(&ar->conf_mutex);
@@ -7533,6 +7544,9 @@ static void ath12k_mac_set_bitrate_mask_iter(void *data,
 	struct ath12k_sta *arsta = ath12k_sta_to_arsta(sta);
 	struct ath12k *ar = arvif->ar;
 
+	if (arsta->arvif != arvif)
+		return;
+
 	spin_lock_bh(&ar->data_lock);
 	arsta->changed |= IEEE80211_RC_SUPP_RATES_CHANGED;
 	spin_unlock_bh(&ar->data_lock);
@@ -7543,10 +7557,14 @@ static void ath12k_mac_set_bitrate_mask_iter(void *data,
 static void ath12k_mac_disable_peer_fixed_rate(void *data,
 					       struct ieee80211_sta *sta)
 {
+	struct ath12k_sta *arsta = ath12k_sta_to_arsta(sta);
 	struct ath12k_vif *arvif = data;
 	struct ath12k *ar = arvif->ar;
 	int ret;
 
+	if (arsta->arvif != arvif)
+		return;
+
 	ret = ath12k_wmi_set_peer_param(ar, sta->addr,
 					arvif->vdev_id,
 					WMI_PEER_PARAM_FIXED_RATE,
diff --git a/drivers/net/wireless/ath/ath12k/p2p.c b/drivers/net/wireless/ath/ath12k/p2p.c
index d334df720032..3a851ee15b2f 100644
--- a/drivers/net/wireless/ath/ath12k/p2p.c
+++ b/drivers/net/wireless/ath/ath12k/p2p.c
@@ -121,7 +121,7 @@ static void ath12k_p2p_noa_update_vdev_iter(void *data, u8 *mac,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	struct ath12k_p2p_noa_arg *arg = data;
 
-	if (arvif->vdev_id != arg->vdev_id)
+	if (arvif->ar != arg->ar || arvif->vdev_id != arg->vdev_id)
 		return;
 
 	ath12k_p2p_noa_update(arvif, arg->noa);
@@ -132,6 +132,7 @@ void ath12k_p2p_noa_update_by_vdev_id(struct ath12k *ar, u32 vdev_id,
 {
 	struct ath12k_p2p_noa_arg arg = {
 		.vdev_id = vdev_id,
+		.ar = ar,
 		.noa = noa,
 	};
 
diff --git a/drivers/net/wireless/ath/ath12k/p2p.h b/drivers/net/wireless/ath/ath12k/p2p.h
index 5768139a7844..b2eec51a9900 100644
--- a/drivers/net/wireless/ath/ath12k/p2p.h
+++ b/drivers/net/wireless/ath/ath12k/p2p.h
@@ -12,6 +12,7 @@ struct ath12k_wmi_p2p_noa_info;
 
 struct ath12k_p2p_noa_arg {
 	u32 vdev_id;
+	struct ath12k *ar;
 	const struct ath12k_wmi_p2p_noa_info *noa;
 };
 
-- 
2.25.1

Re: [PATCH v4 08/12] wifi: ath12k: Add additional checks for vif and sta iterators

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> Since vif and sta objects of different radios are added to same
> local hw list in mac80211, additional checks need to be done
> in driver to ensure we are processing the intended vif
> and sta corresponding to the radio when the vif and sta mac80211
> iterator utils are used from driver.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 09/12] wifi: ath12k: modify regulatory support for single wiphy architecture

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

With all the radios being combined and registered as a single
mac80211 hw/wiphy, separate regd built from firmware rules need
not be updated to cfg80211. Rather we can pick one of the regd
built from the rules to update to cfg80211 for the whole
registered device. We prefer 6 GHz pdev based rules since it has
the rules for all bands. If the hw doesn't support 6 GHz, then update
rules from one of the pdevs.

Also, when regulatory notification is received, update to all the
underlying radios/ar so that it becomes aware of the change and as
well us it updates its local regd with the new country rules. Later
pick the appropriate pdev's regd(6 GHz if available) and apply to
cfg80211.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  2 +
 drivers/net/wireless/ath/ath12k/mac.c  |  2 +
 drivers/net/wireless/ath/ath12k/reg.c  | 53 +++++++++++++++++++++-----
 3 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 4b21e7bcec3f..85e897357441 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -626,6 +626,8 @@ struct ath12k {
 
 struct ath12k_hw {
 	struct ieee80211_hw *hw;
+	bool regd_updated;
+	bool use_6ghz_regd;
 
 	u8 num_radio;
 	struct ath12k radio[] __aligned(sizeof(void *));
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index a77d6dbaea46..29c68aa2d30a 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -8076,6 +8076,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 {
 	struct ieee80211_supported_band *band;
 	struct ath12k_wmi_hal_reg_capabilities_ext_arg *reg_cap;
+	struct ath12k_hw *ah = ar->ah;
 	void *channels;
 	u32 phy_id;
 
@@ -8130,6 +8131,7 @@ static int ath12k_mac_setup_channels_rates(struct ath12k *ar,
 			ath12k_mac_update_ch_list(ar, band,
 						  reg_cap->low_5ghz_chan,
 						  reg_cap->high_5ghz_chan);
+			ah->use_6ghz_regd = true;
 		}
 
 		if (reg_cap->low_5ghz_chan < ATH12K_MIN_6G_FREQ) {
diff --git a/drivers/net/wireless/ath/ath12k/reg.c b/drivers/net/wireless/ath/ath12k/reg.c
index e73b716a2d80..aa809a6bc50a 100644
--- a/drivers/net/wireless/ath/ath12k/reg.c
+++ b/drivers/net/wireless/ath/ath12k/reg.c
@@ -50,7 +50,7 @@ ath12k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
 	struct ath12k_wmi_init_country_arg arg;
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
-	int ret;
+	int ret, i;
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_REG,
 		   "Regulatory Notification received for %s\n", wiphy_name(wiphy));
@@ -85,10 +85,16 @@ ath12k_reg_notifier(struct wiphy *wiphy, struct regulatory_request *request)
 	memcpy(&arg.cc_info.alpha2, request->alpha2, 2);
 	arg.cc_info.alpha2[2] = 0;
 
-	ret = ath12k_wmi_send_init_country_cmd(ar, &arg);
-	if (ret)
-		ath12k_warn(ar->ab,
-			    "INIT Country code set to fw failed : %d\n", ret);
+	/* Allow fresh updates to wiphy regd */
+	ah->regd_updated = false;
+
+	/* Send the reg change request to all the radios */
+	for_each_ar(i, ah, ar) {
+		ret = ath12k_wmi_send_init_country_cmd(ar, &arg);
+		if (ret)
+			ath12k_warn(ar->ab,
+				    "INIT Country code set to fw failed : %d\n", ret);
+	}
 }
 
 int ath12k_reg_update_chan_list(struct ath12k *ar)
@@ -202,10 +208,32 @@ int ath12k_regd_update(struct ath12k *ar, bool init)
 {
 	struct ieee80211_hw *hw = ath12k_ar_to_hw(ar);
 	struct ieee80211_regdomain *regd, *regd_copy = NULL;
+	struct ath12k_hw *ah = ar->ah;
 	int ret, regd_len, pdev_id;
 	struct ath12k_base *ab;
+	int i;
 
 	ab = ar->ab;
+
+	/* If one of the radios within ah has already updated the regd for
+	 * the wiphy, then avoid setting regd again
+	 */
+	if (ah->regd_updated)
+		return 0;
+
+	/* firmware provides reg rules which are similar for 2 GHz and 5 GHz
+	 * pdev but 6 GHz pdev has superset of all rules including rules for
+	 * all bands, we prefer 6 GHz pdev's rules to be used for setup of
+	 * the wiphy regd.
+	 * If 6 GHz pdev was part of the ath12k_hw, wait for the 6 GHz pdev,
+	 * else pick the first pdev which calls this function and use its
+	 * regd to update global hw regd.
+	 * The regd_updated flag set at the end will not allow any further
+	 * updates.
+	 */
+	if (ah->use_6ghz_regd && !ar->supports_6ghz)
+		return 0;
+
 	pdev_id = ar->pdev_idx;
 
 	spin_lock_bh(&ab->base_lock);
@@ -258,10 +286,17 @@ int ath12k_regd_update(struct ath12k *ar, bool init)
 	if (ret)
 		goto err;
 
-	if (ar->state == ATH12K_STATE_ON) {
-		ret = ath12k_reg_update_chan_list(ar);
-		if (ret)
-			goto err;
+	ah->regd_updated = true;
+	/* Apply the new regd to all the radios, this is expected to be received only once
+	 * since we check for ah->regd_updated and allow here only once.
+	 */
+	for_each_ar(i, ah, ar) {
+		if (ar->state == ATH12K_STATE_ON) {
+			ab = ar->ab;
+			ret = ath12k_reg_update_chan_list(ar);
+			if (ret)
+				goto err;
+		}
 	}
 
 	return 0;
-- 
2.25.1

Re: [PATCH v4 09/12] wifi: ath12k: modify regulatory support for single wiphy architecture

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> With all the radios being combined and registered as a single
> mac80211 hw/wiphy, separate regd built from firmware rules need
> not be updated to cfg80211. Rather we can pick one of the regd
> built from the rules to update to cfg80211 for the whole
> registered device. We prefer 6 GHz pdev based rules since it has
> the rules for all bands. If the hw doesn't support 6 GHz, then update
> rules from one of the pdevs.
> 
> Also, when regulatory notification is received, update to all the
> underlying radios/ar so that it becomes aware of the change and as
> well us it updates its local regd with the new country rules. Later
> pick the appropriate pdev's regd(6 GHz if available) and apply to
> cfg80211.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>

Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 10/12] wifi: ath12k: Modify set and get antenna mac ops for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

As multiple radios are combined into a single wiphy, and
the current infrastructure supports only set/get antenna
for the wiphy, the max Tx/Rx antenna capability is advertised
during wiphy register.
Hence, When antenna set/get is received we adjust the set/get
based on max radio capability and set/get antenna accordingly.

Multi radio capability needs to introduced with interface
combination changes to support single wiphy model in cfg80211
which would help extend the wiphy specific get/set configs similar
to this to per hw level.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 39 ++++++++++++++++++---------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 29c68aa2d30a..0934abc7995f 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -5183,6 +5183,13 @@ static int __ath12k_set_antenna(struct ath12k *ar, u32 tx_ant, u32 rx_ant)
 	if (ath12k_check_chain_mask(ar, rx_ant, false))
 		return -EINVAL;
 
+	/* Since we advertised the max cap of all radios combined during wiphy
+	 * registration, ensure we don't set the antenna config higher than the
+	 * limits
+	 */
+	tx_ant = min_t(u32, tx_ant, ar->pdev->cap.tx_chain_mask);
+	rx_ant = min_t(u32, rx_ant, ar->pdev->cap.rx_chain_mask);
+
 	ar->cfg_tx_chainmask = tx_ant;
 	ar->cfg_rx_chainmask = rx_ant;
 
@@ -6489,16 +6496,19 @@ static void ath12k_mac_op_configure_filter(struct ieee80211_hw *hw,
 static int ath12k_mac_op_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
+	int antennas_rx = 0, antennas_tx = 0;
 	struct ath12k *ar;
+	int i;
 
-	ar = ath12k_ah_to_ar(ah, 0);
-
-	mutex_lock(&ar->conf_mutex);
-
-	*tx_ant = ar->cfg_tx_chainmask;
-	*rx_ant = ar->cfg_rx_chainmask;
+	for_each_ar(i, ah, ar) {
+		mutex_lock(&ar->conf_mutex);
+		antennas_rx = max_t(u32, antennas_rx, ar->cfg_rx_chainmask);
+		antennas_tx = max_t(u32, antennas_tx, ar->cfg_tx_chainmask);
+		mutex_unlock(&ar->conf_mutex);
+	}
 
-	mutex_unlock(&ar->conf_mutex);
+	*tx_ant = antennas_tx;
+	*rx_ant = antennas_rx;
 
 	return 0;
 }
@@ -6507,13 +6517,16 @@ static int ath12k_mac_op_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
-	int ret;
-
-	ar = ath12k_ah_to_ar(ah, 0);
+	int ret = 0;
+	int i;
 
-	mutex_lock(&ar->conf_mutex);
-	ret = __ath12k_set_antenna(ar, tx_ant, rx_ant);
-	mutex_unlock(&ar->conf_mutex);
+	for_each_ar(i, ah, ar) {
+		mutex_lock(&ar->conf_mutex);
+		ret = __ath12k_set_antenna(ar, tx_ant, rx_ant);
+		mutex_unlock(&ar->conf_mutex);
+		if (ret)
+			break;
+	}
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH v4 10/12] wifi: ath12k: Modify set and get antenna mac ops for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> As multiple radios are combined into a single wiphy, and
> the current infrastructure supports only set/get antenna
> for the wiphy, the max Tx/Rx antenna capability is advertised
> during wiphy register.
> Hence, When antenna set/get is received we adjust the set/get
> based on max radio capability and set/get antenna accordingly.
> 
> Multi radio capability needs to introduced with interface
> combination changes to support single wiphy model in cfg80211
> which would help extend the wiphy specific get/set configs similar
> to this to per hw level.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 11/12] wifi: ath12k: Modify rts threshold mac op for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

Since multiple radios are abstracted under a single wiphy,
apply the rts threshold value to all the vdevs of the radios
combined under single wiphy.

This also implies that vif specific rts threshold support
needs to be added in future from cfg80211/mac80211.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 0934abc7995f..6123d7db2edc 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7292,11 +7292,21 @@ static int ath12k_mac_op_set_rts_threshold(struct ieee80211_hw *hw, u32 value)
 {
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
-	int param_id = WMI_VDEV_PARAM_RTS_THRESHOLD, ret;
+	int param_id = WMI_VDEV_PARAM_RTS_THRESHOLD, ret, i;
 
-	ar = ath12k_ah_to_ar(ah, 0);
-
-	ret = ath12k_set_vdev_param_to_all_vifs(ar, param_id, value);
+	/* Currently we set the rts threshold value to all the vifs across
+	 * all radios of the single wiphy.
+	 * TODO Once support for vif specific RTS threshold in mac80211 is
+	 * available, ath12k can make use of it.
+	 */
+	for_each_ar(i, ah, ar) {
+		ret = ath12k_set_vdev_param_to_all_vifs(ar, param_id, value);
+		if (ret) {
+			ath12k_warn(ar->ab, "failed to set RTS config for all vdevs of pdev %d",
+				    ar->pdev->pdev_id);
+			break;
+		}
+	}
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH v4 11/12] wifi: ath12k: Modify rts threshold mac op for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> Since multiple radios are abstracted under a single wiphy,
> apply the rts threshold value to all the vdevs of the radios
> combined under single wiphy.
> 
> This also implies that vif specific rts threshold support
> needs to be added in future from cfg80211/mac80211.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>

[PATCH v4 12/12] wifi: ath12k: support get_survey mac op for single wiphy

[Rameshkumar Sundaram]

From: Sriram R <quic_srirrama@quicinc.com>

The radio for which the survey info needs to be collected
depends on the channel idx which could be based on the band.
Use the idx to identify the appropriate sband since multiple
bands could be combined for single wiphy case.

Also use the channel idx and sband to identify the corresponding
radio on which the survey results needs to be populated.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 6123d7db2edc..a31003f8325d 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -7821,7 +7821,6 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
 static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 				    struct survey_info *survey)
 {
-	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar;
 	struct ieee80211_supported_band *sband;
 	struct survey_info *ar_survey;
@@ -7830,12 +7829,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 	if (idx >= ATH12K_NUM_CHANS)
 		return -ENOENT;
 
-	ar = ath12k_ah_to_ar(ah, 0);
-
-	ar_survey = &ar->survey[idx];
-
-	mutex_lock(&ar->conf_mutex);
-
 	sband = hw->wiphy->bands[NL80211_BAND_2GHZ];
 	if (sband && idx >= sband->n_channels) {
 		idx -= sband->n_channels;
@@ -7850,6 +7843,21 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 		goto exit;
 	}
 
+	ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[idx]);
+	if (!ar) {
+		if (sband->channels[idx].flags & IEEE80211_CHAN_DISABLED) {
+			ret = 0;
+			memset(survey, 0, sizeof(*survey));
+			goto exit;
+		}
+		ret = -ENOENT;
+		goto exit;
+	}
+
+	ar_survey = &ar->survey[idx];
+
+	mutex_lock(&ar->conf_mutex);
+
 	ath12k_mac_update_bss_chan_survey(ar, &sband->channels[idx]);
 
 	spin_lock_bh(&ar->data_lock);
@@ -7861,9 +7869,8 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 	if (ar->rx_channel == survey->channel)
 		survey->filled |= SURVEY_INFO_IN_USE;
 
-exit:
 	mutex_unlock(&ar->conf_mutex);
-
+exit:
 	return ret;
 }
 
-- 
2.25.1

Re: [PATCH v4 12/12] wifi: ath12k: support get_survey mac op for single wiphy

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> The radio for which the survey info needs to be collected
> depends on the channel idx which could be based on the band.
> Use the idx to identify the appropriate sband since multiple
> bands could be combined for single wiphy case.
> 
> Also use the channel idx and sband to identify the corresponding
> radio on which the survey results needs to be populated.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/mac.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 6123d7db2edc..a31003f8325d 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -7821,7 +7821,6 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
>  static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>  				    struct survey_info *survey)
>  {
> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>  	struct ath12k *ar;
>  	struct ieee80211_supported_band *sband;
>  	struct survey_info *ar_survey;
> @@ -7830,12 +7829,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>  	if (idx >= ATH12K_NUM_CHANS)
>  		return -ENOENT;
>  
> -	ar = ath12k_ah_to_ar(ah, 0);
> -
> -	ar_survey = &ar->survey[idx];
> -
> -	mutex_lock(&ar->conf_mutex);
> -
>  	sband = hw->wiphy->bands[NL80211_BAND_2GHZ];
>  	if (sband && idx >= sband->n_channels) {
>  		idx -= sband->n_channels;
> @@ -7850,6 +7843,21 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>  		goto exit;
>  	}
>  
> +	ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[idx]);
> +	if (!ar) {
> +		if (sband->channels[idx].flags & IEEE80211_CHAN_DISABLED) {
> +			ret = 0;
> +			memset(survey, 0, sizeof(*survey));
> +			goto exit;
> +		}
> +		ret = -ENOENT;
> +		goto exit;
> +	}
> +
> +	ar_survey = &ar->survey[idx];
> +
> +	mutex_lock(&ar->conf_mutex);
> +
>  	ath12k_mac_update_bss_chan_survey(ar, &sband->channels[idx]);
>  
>  	spin_lock_bh(&ar->data_lock);
> @@ -7861,9 +7869,8 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>  	if (ar->rx_channel == survey->channel)
>  		survey->filled |= SURVEY_INFO_IN_USE;
>  
> -exit:
>  	mutex_unlock(&ar->conf_mutex);
> -
> +exit:

goto should normally only be used when there is centralized cleanup.
since now there is no cleanup required, all of the goto exit calls should just
directly return the appropriate error

>  	return ret;
>  }
>  

Re: [PATCH v4 12/12] wifi: ath12k: support get_survey mac op for single wiphy

[Rameshkumar Sundaram]

On 3/13/2024 4:55 AM, Jeff Johnson wrote:
> On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
>> From: Sriram R <quic_srirrama@quicinc.com>
>>
>> The radio for which the survey info needs to be collected
>> depends on the channel idx which could be based on the band.
>> Use the idx to identify the appropriate sband since multiple
>> bands could be combined for single wiphy case.
>>
>> Also use the channel idx and sband to identify the corresponding
>> radio on which the survey results needs to be populated.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.0.1-00029-QCAHKSWPL_SILICONZ-1
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
>>
>> Signed-off-by: Sriram R <quic_srirrama@quicinc.com>
>> Signed-off-by: Rameshkumar Sundaram <quic_ramess@quicinc.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/mac.c | 25 ++++++++++++++++---------
>>   1 file changed, 16 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 6123d7db2edc..a31003f8325d 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -7821,7 +7821,6 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
>>   static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   				    struct survey_info *survey)
>>   {
>> -	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
>>   	struct ath12k *ar;
>>   	struct ieee80211_supported_band *sband;
>>   	struct survey_info *ar_survey;
>> @@ -7830,12 +7829,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   	if (idx >= ATH12K_NUM_CHANS)
>>   		return -ENOENT;
>>   
>> -	ar = ath12k_ah_to_ar(ah, 0);
>> -
>> -	ar_survey = &ar->survey[idx];
>> -
>> -	mutex_lock(&ar->conf_mutex);
>> -
>>   	sband = hw->wiphy->bands[NL80211_BAND_2GHZ];
>>   	if (sband && idx >= sband->n_channels) {
>>   		idx -= sband->n_channels;
>> @@ -7850,6 +7843,21 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   		goto exit;
>>   	}
>>   
>> +	ar = ath12k_mac_get_ar_by_chan(hw, &sband->channels[idx]);
>> +	if (!ar) {
>> +		if (sband->channels[idx].flags & IEEE80211_CHAN_DISABLED) {
>> +			ret = 0;
>> +			memset(survey, 0, sizeof(*survey));
>> +			goto exit;
>> +		}
>> +		ret = -ENOENT;
>> +		goto exit;
>> +	}
>> +
>> +	ar_survey = &ar->survey[idx];
>> +
>> +	mutex_lock(&ar->conf_mutex);
>> +
>>   	ath12k_mac_update_bss_chan_survey(ar, &sband->channels[idx]);
>>   
>>   	spin_lock_bh(&ar->data_lock);
>> @@ -7861,9 +7869,8 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>>   	if (ar->rx_channel == survey->channel)
>>   		survey->filled |= SURVEY_INFO_IN_USE;
>>   
>> -exit:
>>   	mutex_unlock(&ar->conf_mutex);
>> -
>> +exit:
> 
> goto should normally only be used when there is centralized cleanup.
> since now there is no cleanup required, all of the goto exit calls should just
> directly return the appropriate error
> 
Sure, this sneaked through while rebasing, thanks for pointing out. Will 
fix it in next version.
>>   	return ret;
>>   }
>>   
> 

Re: [PATCH v4 00/12] wifi: ath12k: Add single wiphy support

[Jeff Johnson]

On 3/12/2024 6:55 AM, Rameshkumar Sundaram wrote:
> From: Sriram R <quic_srirrama@quicinc.com>
> 
> With the introduction of Multi Link Operation (MLO) support in
> IEEE802.11be, each EHT AP/non AP interface is capable of
> operating with multiple radio links.
> 
> cfg80211/mac80211 expects drivers to abstract the communication
> between such Multi Link HW and mac80211/cfg80211 since it depends
> on different driver/HW implementation. Hence the single wiphy
> abstraction with changes in datastructures were introduced in
> "wifi: ath12k: Introduce hw abstraction"
> 
> This patchset extends the implementation to allow combination
> of multiple underlying radios into a single composite hw/wiphy
> for registration. Since now multiple radios are represented by
> a single wiphy, changes are required in various mac ops that the
> driver supports since the driver now needs to learn on how to tunnel
> various mac ops properly to a specific radio.
> 
> This patchset covers the basic mac80211 ops for an interface bring up
> and operation.
> 
> Note:
> Monitor and hw reconfig support for Single Wiphy will be done in future
> patchsets.
> 
> ---
>  v4:
>  - Updated missing Signed-off details for patches.
> 
>  v3:
>   - Rebased on ToT (added additional ar check in PATCH 08/12 for p2p)
>   - Introduced iterator to loop through ars in an ah(for_each_ar())
>   - Addressed comments on reverse xmas tree declaration style.
> 
>  v2:
>   - Rebased on ToT and dependent patchset
> 
> 
> Karthikeyan Periyasamy (1):
>   wifi: ath12k: add multiple radio support in a single MAC HW
>     un/register
> 
> Sriram R (11):
>   wifi: ath12k: Modify add and remove chanctx ops for single wiphy
>     support
>   wifi: ath12k: modify ath12k mac start/stop ops for single wiphy
>   wifi: ath12k: vdev statemachine changes for single wiphy
>   wifi: ath12k: scan statemachine changes for single wiphy
>   wifi: ath12k: fetch correct radio based on vdev status
>   wifi: ath12k: Cache vdev configs before vdev create
>   wifi: ath12k: Add additional checks for vif and sta iterators
>   wifi: ath12k: modify regulatory support for single wiphy architecture
>   wifi: ath12k: Modify set and get antenna mac ops for single wiphy
>   wifi: ath12k: Modify rts threshold mac op for single wiphy
>   wifi: ath12k: support get_survey mac op for single wiphy
> 
>  drivers/net/wireless/ath/ath12k/core.h |  38 +-
>  drivers/net/wireless/ath/ath12k/hw.h   |   1 +
>  drivers/net/wireless/ath/ath12k/mac.c  | 911 +++++++++++++++++++------
>  drivers/net/wireless/ath/ath12k/p2p.c  |   3 +-
>  drivers/net/wireless/ath/ath12k/p2p.h  |   1 +
>  drivers/net/wireless/ath/ath12k/reg.c  |  55 +-
>  6 files changed, 785 insertions(+), 224 deletions(-)
> 

ath12k-check reports the following issues when run against the series:
drivers/net/wireless/ath/ath12k/core.h:994: Macro argument reuse 'index' - possible side-effects?
drivers/net/wireless/ath/ath12k/core.h:994: Macro argument reuse 'ah' - possible side-effects?
drivers/net/wireless/ath/ath12k/mac.c:3635: Please don't use multiple blank lines
drivers/net/wireless/ath/ath12k/mac.c:6230: line length of 92 exceeds 90 columns
drivers/net/wireless/ath/ath12k/mac.c:8429: Missing a blank line after declarations