[PATCH RFC v2 0/4] wifi: ath12k: switch to using wiphy_lock()

[PATCH RFC v2 0/4] wifi: ath12k: switch to using wiphy_lock()

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

Convert all uses of struct ath12k::conf_mutex to use struct wiphy::mtx, which
is already used by mac80211, and remove conf_mutex from ath12k. This way we
have one mutex less in ath12k and simpler locking design.

I'm submitting this as RFC as I have only tested these patches and need
to investigate how this affects MLO implementation.

v2:

* rebase to ath-202409051620

* patch 1: ath12k_wow_op_suspend(): remove extra wiphy_lock()/unlock() (Baochen)

* patch 1: fix clang warnings about unused labels (Johannes)

* patch 2: s/no/now/ (Jeff)

* patch 4: ath12k_sta_rc_update_wk(): fix wiphy_priv() usage

v1: https://patchwork.kernel.org/project/linux-wireless/cover/20240821153728.2121600-1-kvalo@kernel.org/

Kalle Valo (4):
  wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex
  wifi: ath12k: cleanup unneeded labels
  wifi: ath12k: ath12k_mac_op_set_key(): remove exit label
  wifi: ath12k: convert struct ath12k_sta::update_wk to use struct
    wiphy_work

 drivers/net/wireless/ath/ath12k/core.c        |   6 +-
 drivers/net/wireless/ath/ath12k/core.h        |   7 +-
 drivers/net/wireless/ath/ath12k/debugfs.c     |   4 +-
 .../wireless/ath/ath12k/debugfs_htt_stats.c   |  26 +-
 drivers/net/wireless/ath/ath12k/mac.c         | 363 +++++++-----------
 drivers/net/wireless/ath/ath12k/peer.c        |   6 +-
 drivers/net/wireless/ath/ath12k/wow.c         |  26 +-
 7 files changed, 183 insertions(+), 255 deletions(-)


base-commit: 903aaf66edc97dd5b9e3118d19677291051a9c40
-- 
2.39.5

[PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

Switch from using driver specific ar->conf_mutex to wiphy->mtx. The benefits are:

* one lock less and simplified locking

* possibility to use wiphy_work_queue() without other locks

Most of the mac80211 ops are called within wiphy_lock(), most notable exception
being tx op. This can be checked with lockdep_assert_wiphy() from
net/mac80211/driver-ops.[ch] and I veried that by manually going through all
the ops in ath12k_ops which had lockdep_assert_wiphy().

The conversion was simple:

* All conf_mutex lock() and unlock() calls which
  already were called under wiphy_lock() I replaced with
  lockdep_assert_wiphy().

* The rest of conf_mutex calls I replaced with wiphy_lock() and wiphy_unlock().

* All lockdep_asset_held(conf_mutex) calls I replaced with
  lockdep_assert_wiphy().

There is now a new sparse warning, but to keep this long patch simple the
labels will be cleaned up in following patches:

drivers/net/wireless/ath/ath12k/mac.c:6635:1: warning: statement expected after label

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c        |   6 +-
 drivers/net/wireless/ath/ath12k/core.h        |   5 +-
 drivers/net/wireless/ath/ath12k/debugfs.c     |   4 +-
 .../wireless/ath/ath12k/debugfs_htt_stats.c   |  26 +-
 drivers/net/wireless/ath/ath12k/mac.c         | 273 +++++++-----------
 drivers/net/wireless/ath/ath12k/peer.c        |   6 +-
 drivers/net/wireless/ath/ath12k/wow.c         |  26 +-
 7 files changed, 143 insertions(+), 203 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 51252e8bc1ae..f67018b3111c 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1004,7 +1004,7 @@ void ath12k_core_halt(struct ath12k *ar)
 {
 	struct ath12k_base *ab = ar->ab;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ar->num_created_vdevs = 0;
 	ar->allocated_vdev_map = 0;
@@ -1087,9 +1087,9 @@ static void ath12k_core_post_reconfigure_recovery(struct ath12k_base *ab)
 			for (j = 0; j < ah->num_radio; j++) {
 				ar = &ah->radio[j];
 
-				mutex_lock(&ar->conf_mutex);
+				wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 				ath12k_core_halt(ar);
-				mutex_unlock(&ar->conf_mutex);
+				wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 			}
 
 			break;
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 7f2e9a9b4097..7551494716b5 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -562,10 +562,7 @@ struct ath12k {
 	u32 num_stations;
 	u32 max_num_stations;
 	bool monitor_present;
-	/* To synchronize concurrent synchronous mac80211 callback operations,
-	 * concurrent debugfs configuration and concurrent FW statistics events.
-	 */
-	struct mutex conf_mutex;
+
 	/* protects the radio specific data like debug stats, ppdu_stats_info stats,
 	 * vdev_stop_status info, scan data, ath12k_sta info, ath12k_vif info,
 	 * channel context data, survey info, test mode data.
diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
index 2a977c36af00..d4b32d1a431c 100644
--- a/drivers/net/wireless/ath/ath12k/debugfs.c
+++ b/drivers/net/wireless/ath/ath12k/debugfs.c
@@ -15,14 +15,14 @@ static ssize_t ath12k_write_simulate_radar(struct file *file,
 	struct ath12k *ar = file->private_data;
 	int ret;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	ret = ath12k_wmi_simulate_radar(ar);
 	if (ret)
 		goto exit;
 
 	ret = count;
 exit:
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 	return ret;
 }
 
diff --git a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
index f1b7e74aefe4..bbdef525c17e 100644
--- a/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
+++ b/drivers/net/wireless/ath/ath12k/debugfs_htt_stats.c
@@ -1627,9 +1627,9 @@ static ssize_t ath12k_read_htt_stats_type(struct file *file,
 	char buf[32];
 	size_t len;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	type = ar->debug.htt_stats.type;
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	len = scnprintf(buf, sizeof(buf), "%u\n", type);
 
@@ -1662,7 +1662,7 @@ static ssize_t ath12k_write_htt_stats_type(struct file *file,
 	    type >= ATH12K_DBG_HTT_NUM_EXT_STATS)
 		return -EINVAL;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 
 	ar->debug.htt_stats.type = type;
 	ar->debug.htt_stats.cfg_param[0] = cfg_param[0];
@@ -1670,7 +1670,7 @@ static ssize_t ath12k_write_htt_stats_type(struct file *file,
 	ar->debug.htt_stats.cfg_param[2] = cfg_param[2];
 	ar->debug.htt_stats.cfg_param[3] = cfg_param[3];
 
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	return count;
 }
@@ -1691,7 +1691,7 @@ static int ath12k_debugfs_htt_stats_req(struct ath12k *ar)
 	int ret, pdev_id;
 	struct htt_ext_stats_cfg_params cfg_params = { 0 };
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	init_completion(&stats_req->htt_stats_rcvd);
 
@@ -1741,7 +1741,7 @@ static int ath12k_open_htt_stats(struct inode *inode,
 	if (type == ATH12K_DBG_HTT_EXT_STATS_RESET)
 		return -EPERM;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (ah->state != ATH12K_HW_STATE_ON) {
 		ret = -ENETDOWN;
@@ -1776,14 +1776,14 @@ static int ath12k_open_htt_stats(struct inode *inode,
 
 	file->private_data = stats_req;
 
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	return 0;
 out:
 	kfree(stats_req);
 	ar->debug.htt_stats.stats_req = NULL;
 err_unlock:
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	return ret;
 }
@@ -1793,10 +1793,10 @@ static int ath12k_release_htt_stats(struct inode *inode,
 {
 	struct ath12k *ar = inode->i_private;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	kfree(file->private_data);
 	ar->debug.htt_stats.stats_req = NULL;
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	return 0;
 }
@@ -1840,7 +1840,7 @@ static ssize_t ath12k_write_htt_stats_reset(struct file *file,
 	    type == ATH12K_DBG_HTT_EXT_STATS_RESET)
 		return -E2BIG;
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	cfg_params.cfg0 = HTT_STAT_DEFAULT_RESET_START_OFFSET;
 	param_pos = (type >> 5) + 1;
 
@@ -1866,12 +1866,12 @@ static ssize_t ath12k_write_htt_stats_reset(struct file *file,
 						 0ULL);
 	if (ret) {
 		ath12k_warn(ar->ab, "failed to send htt stats request: %d\n", ret);
-		mutex_unlock(&ar->conf_mutex);
+		wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 		return ret;
 	}
 
 	ar->debug.htt_stats.reset = type;
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 
 	return count;
 }
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 137394c36460..ed900fbef9d8 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -677,7 +677,8 @@ static struct ath12k_vif *ath12k_mac_get_vif_up(struct ath12k *ar)
 {
 	struct ath12k_vif *arvif;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
+
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (arvif->is_up)
 			return arvif;
@@ -774,7 +775,7 @@ static int ath12k_mac_txpower_recalc(struct ath12k *ar)
 	int ret, txpower = -1;
 	u32 param;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (arvif->txpower <= 0)
@@ -830,7 +831,7 @@ static int ath12k_recalc_rtscts_prot(struct ath12k_vif *arvif)
 	u32 vdev_param, rts_cts;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	vdev_param = WMI_VDEV_PARAM_ENABLE_RTSCTS;
 
@@ -913,7 +914,7 @@ void ath12k_mac_peer_cleanup_all(struct ath12k *ar)
 	struct ath12k_peer *peer, *tmp;
 	struct ath12k_base *ab = ar->ab;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	spin_lock_bh(&ab->base_lock);
 	list_for_each_entry_safe(peer, tmp, &ab->peers, list) {
@@ -929,7 +930,7 @@ void ath12k_mac_peer_cleanup_all(struct ath12k *ar)
 
 static int ath12k_mac_vdev_setup_sync(struct ath12k *ar)
 {
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ar->ab->dev_flags))
 		return -ESHUTDOWN;
@@ -971,7 +972,7 @@ static int ath12k_mac_monitor_vdev_start(struct ath12k *ar, int vdev_id,
 	struct ath12k_wmi_vdev_up_params params = {};
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	channel = chandef->chan;
 	arg.vdev_id = vdev_id;
@@ -1034,7 +1035,7 @@ static int ath12k_mac_monitor_vdev_stop(struct ath12k *ar)
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	reinit_completion(&ar->vdev_setup_done);
 
@@ -1066,7 +1067,7 @@ static int ath12k_mac_monitor_vdev_create(struct ath12k *ar)
 	u8 tmp_addr[6];
 	u16 nss;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (ar->monitor_vdev_created)
 		return 0;
@@ -1132,7 +1133,7 @@ static int ath12k_mac_monitor_vdev_delete(struct ath12k *ar)
 	int ret;
 	unsigned long time_left;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (!ar->monitor_vdev_created)
 		return 0;
@@ -1178,7 +1179,7 @@ static int ath12k_mac_monitor_start(struct ath12k *ar)
 	struct cfg80211_chan_def *chandef = NULL;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (ar->monitor_started)
 		return 0;
@@ -1208,7 +1209,7 @@ static int ath12k_mac_monitor_stop(struct ath12k *ar)
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (!ar->monitor_started)
 		return 0;
@@ -1231,7 +1232,7 @@ static int ath12k_mac_vdev_stop(struct ath12k_vif *arvif)
 	struct ath12k *ar = arvif->ar;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	reinit_completion(&ar->vdev_setup_done);
 
@@ -1272,7 +1273,7 @@ static int ath12k_mac_config(struct ath12k *ar, u32 changed)
 	struct ieee80211_conf *conf = &hw->conf;
 	int ret = 0;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	if (changed & IEEE80211_CONF_CHANGE_MONITOR) {
 		ar->monitor_conf_enabled = conf->flags & IEEE80211_CONF_MONITOR;
@@ -1296,12 +1297,10 @@ static int ath12k_mac_config(struct ath12k *ar, u32 changed)
 	}
 
 exit:
-	mutex_unlock(&ar->conf_mutex);
 	return ret;
 
 err_mon_del:
 	ath12k_mac_monitor_vdev_delete(ar);
-	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
 
@@ -1602,7 +1601,7 @@ static void ath12k_control_beaconing(struct ath12k_vif *arvif,
 	struct ath12k *ar = arvif->ar;
 	int ret;
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(arvif->ar)->wiphy);
 
 	if (!info->enable_beacon) {
 		ret = ath12k_wmi_vdev_down(ar, arvif->vdev_id);
@@ -1724,7 +1723,7 @@ static void ath12k_peer_assoc_h_basic(struct ath12k *ar,
 	struct ieee80211_hw *hw = ath12k_ar_to_hw(ar);
 	u32 aid;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	if (vif->type == NL80211_IFTYPE_STATION)
 		aid = vif->cfg.aid;
@@ -1754,7 +1753,7 @@ static void ath12k_peer_assoc_h_crypto(struct ath12k *ar,
 	const u8 *rsnie = NULL;
 	const u8 *wpaie = NULL;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	if (WARN_ON(ath12k_mac_vif_chan(vif, &def)))
 		return;
@@ -1819,7 +1818,7 @@ static void ath12k_peer_assoc_h_rates(struct ath12k *ar,
 	u8 rate;
 	int i;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	if (WARN_ON(ath12k_mac_vif_chan(vif, &def)))
 		return;
@@ -1880,7 +1879,7 @@ static void ath12k_peer_assoc_h_ht(struct ath12k *ar,
 	u8 max_nss;
 	u32 stbc;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (WARN_ON(ath12k_mac_vif_chan(vif, &def)))
 		return;
@@ -2420,7 +2419,7 @@ static int ath12k_peer_assoc_qos_ap(struct ath12k *ar,
 	u32 uapsd;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	arg.vdev_id = arvif->vdev_id;
 
@@ -2807,7 +2806,7 @@ static void ath12k_peer_assoc_prepare(struct ath12k *ar,
 				      struct ath12k_wmi_peer_assoc_arg *arg,
 				      bool reassoc)
 {
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	memset(arg, 0, sizeof(*arg));
 
@@ -2860,7 +2859,7 @@ static void ath12k_bss_assoc(struct ath12k *ar,
 	bool is_auth = false;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev %i assoc bssid %pM aid %d\n",
 		   arvif->vdev_id, arvif->bssid, arvif->aid);
@@ -2953,7 +2952,7 @@ static void ath12k_bss_disassoc(struct ath12k *ar,
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac vdev %i disassoc bssid %pM\n",
 		   arvif->vdev_id, arvif->bssid);
@@ -3008,7 +3007,7 @@ static void ath12k_recalculate_mgmt_rate(struct ath12k *ar,
 	u16 bitrate;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	sband = hw->wiphy->bands[def->chan->band];
 	basic_rate_idx = ffs(vif->bss_conf.basic_rates) - 1;
@@ -3091,7 +3090,7 @@ static void ath12k_mac_vif_setup_ps(struct ath12k_vif *arvif)
 	int timeout;
 	bool enable_ps;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (vif->type != NL80211_IFTYPE_STATION)
 		return;
@@ -3146,7 +3145,7 @@ static void ath12k_mac_bss_info_changed(struct ath12k *ar,
 	u8 rateidx;
 	u32 rate;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (changed & BSS_CHANGED_BEACON_INT) {
 		arvif->beacon_interval = info->beacon_int;
@@ -3448,11 +3447,9 @@ static void ath12k_mac_op_bss_info_changed(struct ieee80211_hw *hw,
 		return;
 	}
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ath12k_mac_bss_info_changed(ar, arvif, info, changed);
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static struct ath12k*
@@ -3541,7 +3538,7 @@ static int ath12k_scan_stop(struct ath12k *ar)
 	};
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	/* TODO: Fill other STOP Params */
 	arg.pdev_id = ar->pdev->pdev_id;
@@ -3581,7 +3578,7 @@ static void ath12k_scan_abort(struct ath12k *ar)
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	spin_lock_bh(&ar->data_lock);
 
@@ -3616,9 +3613,9 @@ static void ath12k_scan_timeout_work(struct work_struct *work)
 	struct ath12k *ar = container_of(work, struct ath12k,
 					 scan.timeout.work);
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 	ath12k_scan_abort(ar);
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 }
 
 static int ath12k_start_scan(struct ath12k *ar,
@@ -3626,7 +3623,7 @@ static int ath12k_start_scan(struct ath12k *ar,
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ret = ath12k_wmi_send_scan_start_cmd(ar, arg);
 	if (ret)
@@ -3668,6 +3665,8 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 	int i;
 	bool create = true;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (ah->num_radio == 1) {
 		WARN_ON(!arvif->is_created);
 		ar = ath12k_ah_to_ar(ah, 0);
@@ -3702,9 +3701,7 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 			 * 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);
@@ -3713,17 +3710,13 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 		}
 	}
 	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);
 	switch (ar->scan.state) {
 	case ATH12K_SCAN_IDLE:
@@ -3805,8 +3798,6 @@ static int ath12k_mac_op_hw_scan(struct ieee80211_hw *hw,
 		kfree(arg);
 	}
 
-	mutex_unlock(&ar->conf_mutex);
-
 	return ret;
 }
 
@@ -3821,9 +3812,9 @@ static void ath12k_mac_op_cancel_hw_scan(struct ieee80211_hw *hw,
 
 	ar = arvif->ar;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
+
 	ath12k_scan_abort(ar);
-	mutex_unlock(&ar->conf_mutex);
 
 	cancel_delayed_work_sync(&ar->scan.timeout);
 }
@@ -3844,7 +3835,7 @@ static int ath12k_install_key(struct ath12k_vif *arvif,
 		.macaddr = macaddr,
 	};
 
-	lockdep_assert_held(&arvif->ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	reinit_completion(&ar->install_key_done);
 
@@ -3912,7 +3903,7 @@ static int ath12k_clear_peer_keys(struct ath12k_vif *arvif,
 	int i;
 	u32 flags = 0;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	spin_lock_bh(&ab->base_lock);
 	peer = ath12k_peer_find(ab, arvif->vdev_id, addr);
@@ -3955,7 +3946,7 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 	int ret = 0;
 	u32 flags = 0;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (test_bit(ATH12K_FLAG_HW_CRYPTO_DISABLED, &ab->dev_flags))
 		return 1;
@@ -3970,7 +3961,7 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 	key->hw_key_idx = key->keyidx;
 
 	/* the peer should not disappear in mid-way (unless FW goes awry) since
-	 * we already hold conf_mutex. we just make sure its there now.
+	 * we already hold wiphy lock. we just make sure its there now.
 	 */
 	spin_lock_bh(&ab->base_lock);
 	peer = ath12k_peer_find(ab, arvif->vdev_id, peer_addr);
@@ -4063,6 +4054,8 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 	struct ath12k *ar;
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	/* BIP needs to be done in software */
 	if (key->cipher == WLAN_CIPHER_SUITE_AES_CMAC ||
 	    key->cipher == WLAN_CIPHER_SUITE_BIP_GMAC_128 ||
@@ -4090,9 +4083,7 @@ static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
 		return 0;
 	}
 
-	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_set_key(ar, cmd, vif, sta, key);
-	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
 
@@ -4121,7 +4112,7 @@ ath12k_mac_set_peer_vht_fixed_rate(struct ath12k_vif *arvif,
 	u32 rate_code;
 	int ret, i;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	nss = 0;
 
@@ -4169,7 +4160,7 @@ static int ath12k_station_assoc(struct ath12k *ar,
 	struct cfg80211_bitrate_mask *mask;
 	u8 num_vht_rates;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (WARN_ON(ath12k_mac_vif_chan(vif, &def)))
 		return -EPERM;
@@ -4252,7 +4243,7 @@ static int ath12k_station_disassoc(struct ath12k *ar,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (!sta->wme) {
 		arvif->num_legacy_stations--;
@@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
 
 	spin_unlock_bh(&ar->data_lock);
 
-	mutex_lock(&ar->conf_mutex);
+	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
 
 	nss = max_t(u32, 1, nss);
 	nss = min(nss, max(ath12k_mac_max_ht_nss(ht_mcs_mask),
@@ -4426,7 +4417,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
 		}
 	}
 err_rc_bw_changed:
-	mutex_unlock(&ar->conf_mutex);
+	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
 }
 
 static int ath12k_mac_inc_num_stations(struct ath12k_vif *arvif,
@@ -4434,7 +4425,7 @@ static int ath12k_mac_inc_num_stations(struct ath12k_vif *arvif,
 {
 	struct ath12k *ar = arvif->ar;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_STA && !sta->tdls)
 		return 0;
@@ -4452,7 +4443,7 @@ static void ath12k_mac_dec_num_stations(struct ath12k_vif *arvif,
 {
 	struct ath12k *ar = arvif->ar;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_STA && !sta->tdls)
 		return;
@@ -4470,7 +4461,7 @@ static int ath12k_mac_station_add(struct ath12k *ar,
 	struct ath12k_wmi_peer_create_arg peer_param;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ret = ath12k_mac_inc_num_stations(arvif, sta);
 	if (ret) {
@@ -4591,7 +4582,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 		return -EINVAL;
 	}
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	if (old_state == IEEE80211_STA_NOTEXIST &&
 	    new_state == IEEE80211_STA_NONE) {
@@ -4692,8 +4683,6 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 				    sta->addr);
 	}
 
-	mutex_unlock(&ar->conf_mutex);
-
 	return ret;
 }
 
@@ -4720,7 +4709,7 @@ static int ath12k_mac_op_sta_set_txpwr(struct ieee80211_hw *hw,
 
 	ar = ath12k_ah_to_ar(ah, 0);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ret = ath12k_wmi_set_peer_param(ar, sta->addr, arvif->vdev_id,
 					WMI_PEER_USE_FIXED_PWR, txpwr);
@@ -4731,7 +4720,6 @@ static int ath12k_mac_op_sta_set_txpwr(struct ieee80211_hw *hw,
 	}
 
 out:
-	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
 
@@ -4877,7 +4865,7 @@ static int ath12k_mac_conf_tx(struct ath12k_vif *arvif,
 	struct ath12k_base *ab = ar->ab;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	switch (ac) {
 	case IEEE80211_AC_VO:
@@ -4931,6 +4919,8 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
 	struct ath12k_vif_cache *cache = arvif->cache;
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	ar = ath12k_get_ar_by_vif(hw, vif);
 	if (!ar) {
 		/* cache the info and apply after vdev is created */
@@ -4943,9 +4933,7 @@ static int ath12k_mac_op_conf_tx(struct ieee80211_hw *hw,
 		return 0;
 	}
 
-	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_conf_tx(arvif, link_id, ac, params);
-	mutex_unlock(&ar->conf_mutex);
 
 	return ret;
 }
@@ -5612,7 +5600,7 @@ static int __ath12k_set_antenna(struct ath12k *ar, u32 tx_ant, u32 rx_ant)
 	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (ath12k_check_chain_mask(ar, tx_ant, true))
 		return -EINVAL;
@@ -5939,8 +5927,7 @@ static int ath12k_mac_start(struct ath12k *ar)
 	int ret;
 
 	lockdep_assert_held(&ah->hw_mutex);
-
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ret = ath12k_wmi_pdev_set_param(ar, WMI_PDEV_PARAM_PMF_QOS,
 					1, pdev->pdev_id);
@@ -6025,14 +6012,11 @@ static int ath12k_mac_start(struct ath12k *ar)
 		}
 	}
 
-	mutex_unlock(&ar->conf_mutex);
-
 	rcu_assign_pointer(ab->pdevs_active[ar->pdev_idx],
 			   &ab->pdevs[ar->pdev_idx]);
 
 	return 0;
 err:
-	mutex_unlock(&ar->conf_mutex);
 
 	return ret;
 }
@@ -6158,15 +6142,14 @@ static void ath12k_mac_stop(struct ath12k *ar)
 	int ret;
 
 	lockdep_assert_held(&ah->hw_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
-	mutex_lock(&ar->conf_mutex);
 	ret = ath12k_mac_config_mon_status_default(ar, false);
 	if (ret && (ret != -EOPNOTSUPP))
 		ath12k_err(ar->ab, "failed to clear rx_filter for monitor status ring: (%d)\n",
 			   ret);
 
 	clear_bit(ATH12K_CAC_RUNNING, &ar->dev_flags);
-	mutex_unlock(&ar->conf_mutex);
 
 	cancel_delayed_work_sync(&ar->scan.timeout);
 	cancel_work_sync(&ar->regd_update_work);
@@ -6430,7 +6413,8 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 	int i;
 	int ret, vdev_id;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	arvif->ar = ar;
 	vdev_id = __ffs64(ab->free_vdev_map);
@@ -6641,7 +6625,7 @@ static void ath12k_mac_vif_cache_flush(struct ath12k *ar,  struct ieee80211_vif
 
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (!cache)
 		return;
@@ -6680,6 +6664,8 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 	struct ath12k_base *ab;
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (ah->num_radio == 1)
 		ar = ah->radio;
 	else if (ctx)
@@ -6712,20 +6698,15 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 			 * 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);
 		}
 	}
 
 	ab = ar->ab;
 
-	mutex_lock(&ar->conf_mutex);
-
 	if (arvif->is_created)
 		goto flush;
 
@@ -6754,7 +6735,6 @@ static struct ath12k *ath12k_mac_assign_vif_to_vdev(struct ieee80211_hw *hw,
 	 */
 	ath12k_mac_vif_cache_flush(ar, vif);
 unlock:
-	mutex_unlock(&ar->conf_mutex);
 	return arvif->ar;
 }
 
@@ -6764,6 +6744,8 @@ static int ath12k_mac_op_add_interface(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int i;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	memset(arvif, 0, sizeof(*arvif));
 
 	arvif->vif = vif;
@@ -6828,7 +6810,7 @@ static int ath12k_mac_vdev_delete(struct ath12k *ar, struct ieee80211_vif *vif)
 	unsigned long time_left;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 	reinit_completion(&ar->vdev_delete_done);
 
 	ret = ath12k_wmi_vdev_delete(ar, arvif->vdev_id);
@@ -6891,6 +6873,8 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 	struct ath12k *ar;
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (!arvif->is_created) {
 		/* if we cached some config but never received assign chanctx,
 		 * free the allocated cache.
@@ -6904,8 +6888,6 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 
 	cancel_delayed_work_sync(&arvif->connection_loss_work);
 
-	mutex_lock(&ar->conf_mutex);
-
 	ath12k_dbg(ab, ATH12K_DBG_MAC, "mac remove interface (vdev %d)\n",
 		   arvif->vdev_id);
 
@@ -6917,8 +6899,6 @@ static void ath12k_mac_op_remove_interface(struct ieee80211_hw *hw,
 	}
 
 	ath12k_mac_vdev_delete(ar, vif);
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 /* FIXME: Has to be verified. */
@@ -6937,7 +6917,7 @@ static void ath12k_mac_configure_filter(struct ath12k *ar,
 	bool reset_flag;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ar->filter_flags = total_flags;
 
@@ -6964,12 +6944,10 @@ static void ath12k_mac_op_configure_filter(struct ieee80211_hw *hw,
 
 	ar = ath12k_ah_to_ar(ah, 0);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	*total_flags &= SUPPORTED_FILTERS;
 	ath12k_mac_configure_filter(ar, *total_flags);
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static int ath12k_mac_op_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant)
@@ -6979,11 +6957,11 @@ static int ath12k_mac_op_get_antenna(struct ieee80211_hw *hw, u32 *tx_ant, u32 *
 	struct ath12k *ar;
 	int i;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	for_each_ar(ah, ar, i) {
-		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);
 	}
 
 	*tx_ant = antennas_tx;
@@ -6999,10 +6977,10 @@ static int ath12k_mac_op_set_antenna(struct ieee80211_hw *hw, u32 tx_ant, u32 rx
 	int ret = 0;
 	int i;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	for_each_ar(ah, ar, i) {
-		mutex_lock(&ar->conf_mutex);
 		ret = __ath12k_set_antenna(ar, tx_ant, rx_ant);
-		mutex_unlock(&ar->conf_mutex);
 		if (ret)
 			break;
 	}
@@ -7016,7 +6994,7 @@ static int ath12k_mac_ampdu_action(struct ath12k_vif *arvif,
 	struct ath12k *ar = arvif->ar;
 	int ret = -EINVAL;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	switch (params->action) {
 	case IEEE80211_AMPDU_RX_START:
@@ -7055,10 +7033,9 @@ static int ath12k_mac_op_ampdu_action(struct ieee80211_hw *hw,
 
 	ar = ath12k_ah_to_ar(ah, 0);
 
-	mutex_lock(&ar->conf_mutex);
-	ret = ath12k_mac_ampdu_action(arvif, params);
-	mutex_unlock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
+	ret = ath12k_mac_ampdu_action(arvif, params);
 	if (ret)
 		ath12k_warn(ar->ab, "pdev idx %d unable to perform ampdu action %d ret %d\n",
 			    ar->pdev_idx, params->action, ret);
@@ -7082,7 +7059,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 		   "mac chanctx add freq %u width %d ptr %p\n",
 		   ctx->def.chan->center_freq, ctx->def.width, ctx);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	spin_lock_bh(&ar->data_lock);
 	/* TODO: In case of multiple channel context, populate rx_channel from
@@ -7091,8 +7068,6 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 	ar->rx_channel = ctx->def.chan;
 	spin_unlock_bh(&ar->data_lock);
 
-	mutex_unlock(&ar->conf_mutex);
-
 	return 0;
 }
 
@@ -7112,7 +7087,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
 		   "mac chanctx remove freq %u width %d ptr %p\n",
 		   ctx->def.chan->center_freq, ctx->def.width, ctx);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	spin_lock_bh(&ar->data_lock);
 	/* TODO: In case of there is one more channel context left, populate
@@ -7120,8 +7095,6 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
 	 */
 	ar->rx_channel = NULL;
 	spin_unlock_bh(&ar->data_lock);
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static enum wmi_phy_mode
@@ -7199,7 +7172,7 @@ ath12k_mac_vdev_start_restart(struct ath12k_vif *arvif,
 	int he_support = arvif->vif->bss_conf.he_support;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	reinit_completion(&ar->vdev_setup_done);
 
@@ -7435,7 +7408,7 @@ ath12k_mac_update_vif_chan(struct ath12k *ar,
 	int i;
 	bool monitor_vif = false;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	for (i = 0; i < n_vifs; i++) {
 		vif = vifs[i].vif;
@@ -7528,7 +7501,7 @@ ath12k_mac_update_active_vif_chan(struct ath12k *ar,
 	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);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ieee80211_iterate_active_interfaces_atomic(hw,
 						   IEEE80211_IFACE_ITER_NORMAL,
@@ -7564,7 +7537,7 @@ static void ath12k_mac_op_change_chanctx(struct ieee80211_hw *hw,
 
 	ab = ar->ab;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
 		   "mac chanctx change freq %u width %d ptr %p changed %x\n",
@@ -7574,7 +7547,7 @@ static void ath12k_mac_op_change_chanctx(struct ieee80211_hw *hw,
 	 * switch_vif_chanctx().
 	 */
 	if (WARN_ON(changed & IEEE80211_CHANCTX_CHANGE_CHANNEL))
-		goto unlock;
+		return;
 
 	if (changed & IEEE80211_CHANCTX_CHANGE_WIDTH ||
 	    changed & IEEE80211_CHANCTX_CHANGE_RADAR ||
@@ -7582,9 +7555,6 @@ static void ath12k_mac_op_change_chanctx(struct ieee80211_hw *hw,
 		ath12k_mac_update_active_vif_chan(ar, ctx);
 
 	/* TODO: Recalc radar detection */
-
-unlock:
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static int ath12k_start_vdev_delay(struct ath12k *ar,
@@ -7630,6 +7600,8 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif = ath12k_vif_to_arvif(vif);
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	/* 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
 	 */
@@ -7641,7 +7613,7 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 
 	ab = ar->ab;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
 		   "mac chanctx assign ptr %p vdev_id %i\n",
@@ -7688,8 +7660,6 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 	/* TODO: Setup ps and cts/rts protection */
 
 out:
-	mutex_unlock(&ar->conf_mutex);
-
 	return ret;
 }
 
@@ -7717,7 +7687,7 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 	ar = arvif->ar;
 	ab = ar->ab;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ath12k_dbg(ab, ATH12K_DBG_MAC,
 		   "mac chanctx unassign ptr %p vdev_id %i\n",
@@ -7727,10 +7697,8 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR) {
 		ret = ath12k_mac_monitor_stop(ar);
-		if (ret) {
-			mutex_unlock(&ar->conf_mutex);
+		if (ret)
 			return;
-		}
 
 		arvif->is_started = false;
 	}
@@ -7748,8 +7716,6 @@ ath12k_mac_op_unassign_vif_chanctx(struct ieee80211_hw *hw,
 	if (arvif->vdev_type != WMI_VDEV_TYPE_MONITOR &&
 	    ar->num_started_vdevs == 1 && ar->monitor_vdev_created)
 		ath12k_mac_monitor_stop(ar);
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static int
@@ -7764,21 +7730,17 @@ ath12k_mac_op_switch_vif_chanctx(struct ieee80211_hw *hw,
 	if (!ar)
 		return -EINVAL;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	/* Switching channels across radio is not allowed */
-	if (ar != ath12k_get_ar_by_ctx(hw, vifs->new_ctx)) {
-		mutex_unlock(&ar->conf_mutex);
+	if (ar != ath12k_get_ar_by_ctx(hw, vifs->new_ctx))
 		return -EINVAL;
-	}
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC,
 		   "mac chanctx switch n_vifs %d mode %d\n",
 		   n_vifs, mode);
 	ath12k_mac_update_vif_chan(ar, vifs, n_vifs);
 
-	mutex_unlock(&ar->conf_mutex);
-
 	return 0;
 }
 
@@ -7788,7 +7750,8 @@ ath12k_set_vdev_param_to_all_vifs(struct ath12k *ar, int param, u32 value)
 	struct ath12k_vif *arvif;
 	int ret = 0;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
+
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "setting mac vdev %d param %d value %d\n",
 			   param, arvif->vdev_id, value);
@@ -7801,7 +7764,7 @@ ath12k_set_vdev_param_to_all_vifs(struct ath12k *ar, int param, u32 value)
 			break;
 		}
 	}
-	mutex_unlock(&ar->conf_mutex);
+
 	return ret;
 }
 
@@ -8027,7 +7990,7 @@ static int ath12k_mac_set_fixed_rate_params(struct ath12k_vif *arvif,
 	u32 vdev_param;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac set fixed rate params vdev %i rate 0x%02x nss %u sgi %u\n",
 		   arvif->vdev_id, rate, nss, sgi);
@@ -8153,6 +8116,8 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 	int ret;
 	int num_rates;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (ath12k_mac_vif_chan(vif, &def))
 		return -EPERM;
 
@@ -8234,26 +8199,18 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 						  ath12k_mac_disable_peer_fixed_rate,
 						  arvif);
 
-		mutex_lock(&ar->conf_mutex);
-
 		arvif->bitrate_mask = *mask;
 		ieee80211_iterate_stations_atomic(hw,
 						  ath12k_mac_set_bitrate_mask_iter,
 						  arvif);
-
-		mutex_unlock(&ar->conf_mutex);
 	}
 
-	mutex_lock(&ar->conf_mutex);
-
 	ret = ath12k_mac_set_fixed_rate_params(arvif, rate, nss, sgi, ldpc);
 	if (ret) {
 		ath12k_warn(ar->ab, "failed to set fixed rate params on vdev %i: %d\n",
 			    arvif->vdev_id, ret);
 	}
 
-	mutex_unlock(&ar->conf_mutex);
-
 out:
 	return ret;
 }
@@ -8268,6 +8225,8 @@ ath12k_mac_op_reconfig_complete(struct ieee80211_hw *hw,
 	struct ath12k_vif *arvif;
 	int recovery_count, i;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (reconfig_type != IEEE80211_RECONFIG_TYPE_RESTART)
 		return;
 
@@ -8280,8 +8239,6 @@ ath12k_mac_op_reconfig_complete(struct ieee80211_hw *hw,
 	ieee80211_wake_queues(hw);
 
 	for_each_ar(ah, ar, i) {
-		mutex_lock(&ar->conf_mutex);
-
 		ab = ar->ab;
 
 		ath12k_warn(ar->ab, "pdev %d successfully recovered\n",
@@ -8326,8 +8283,6 @@ ath12k_mac_op_reconfig_complete(struct ieee80211_hw *hw,
 					   "restart disconnect\n");
 			}
 		}
-
-		mutex_unlock(&ar->conf_mutex);
 	}
 }
 
@@ -8338,7 +8293,7 @@ ath12k_mac_update_bss_chan_survey(struct ath12k *ar,
 	int ret;
 	enum wmi_bss_chan_info_req_type type = WMI_BSS_SURVEY_REQ_TYPE_READ;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (!test_bit(WMI_TLV_SERVICE_BSS_CHANNEL_INFO_64, ar->ab->wmi_ab.svc_map) ||
 	    ar->rx_channel != channel)
@@ -8370,6 +8325,8 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 	struct ieee80211_supported_band *sband;
 	struct survey_info *ar_survey;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (idx >= ATH12K_NUM_CHANS)
 		return -ENOENT;
 
@@ -8403,8 +8360,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 
 	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);
@@ -8416,7 +8371,6 @@ 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;
 
-	mutex_unlock(&ar->conf_mutex);
 	return 0;
 }
 
@@ -8462,7 +8416,7 @@ static int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
 
 	ar = ath12k_ah_to_ar(ah, 0);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	spin_lock_bh(&ar->data_lock);
 	ar->scan.roc_notify = false;
@@ -8470,8 +8424,6 @@ static int ath12k_mac_op_cancel_remain_on_channel(struct ieee80211_hw *hw,
 
 	ath12k_scan_abort(ar);
 
-	mutex_unlock(&ar->conf_mutex);
-
 	cancel_delayed_work_sync(&ar->scan.timeout);
 
 	return 0;
@@ -8491,6 +8443,8 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	bool create = true;
 	int ret;
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	if (ah->num_radio == 1) {
 		WARN_ON(!arvif->is_created);
 		ar = ath12k_ah_to_ar(ah, 0);
@@ -8522,9 +8476,7 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 			 * 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 for roc: %d\n",
@@ -8537,9 +8489,7 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	}
 
 	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 for roc: %d\n",
 				    ret);
@@ -8548,7 +8498,6 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	}
 
 scan:
-	mutex_lock(&ar->conf_mutex);
 	spin_lock_bh(&ar->data_lock);
 
 	switch (ar->scan.state) {
@@ -8624,8 +8573,6 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 free_chan_list:
 	kfree(arg.chan_list);
 exit:
-	mutex_unlock(&ar->conf_mutex);
-
 	return ret;
 }
 
@@ -8638,11 +8585,11 @@ static void ath12k_mac_op_set_rekey_data(struct ieee80211_hw *hw,
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 
+	lockdep_assert_wiphy(hw->wiphy);
+
 	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "mac set rekey data vdev %d\n",
 		   arvif->vdev_id);
 
-	mutex_lock(&ar->conf_mutex);
-
 	memcpy(rekey_data->kck, data->kck, NL80211_KCK_LEN);
 	memcpy(rekey_data->kek, data->kek, NL80211_KEK_LEN);
 
@@ -8659,8 +8606,6 @@ static void ath12k_mac_op_set_rekey_data(struct ieee80211_hw *hw,
 			rekey_data->kck, NL80211_KEK_LEN);
 	ath12k_dbg_dump(ar->ab, ATH12K_DBG_MAC, "replay ctr", NULL,
 			&rekey_data->replay_ctr, sizeof(rekey_data->replay_ctr));
-
-	mutex_unlock(&ar->conf_mutex);
 }
 
 static const struct ieee80211_ops ath12k_ops = {
@@ -9320,7 +9265,7 @@ static void ath12k_mac_setup(struct ath12k *ar)
 	spin_lock_init(&ar->data_lock);
 	INIT_LIST_HEAD(&ar->arvifs);
 	INIT_LIST_HEAD(&ar->ppdu_stats_info);
-	mutex_init(&ar->conf_mutex);
+
 	init_completion(&ar->vdev_setup_done);
 	init_completion(&ar->vdev_delete_done);
 	init_completion(&ar->peer_assoc_done);
@@ -9509,7 +9454,7 @@ int ath12k_mac_vif_set_keepalive(struct ath12k_vif *arvif,
 	struct ath12k *ar = arvif->ar;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (arvif->vdev_type != WMI_VDEV_TYPE_STA)
 		return 0;
diff --git a/drivers/net/wireless/ath/ath12k/peer.c b/drivers/net/wireless/ath/ath12k/peer.c
index 19c0626fbff1..64c8a734bafb 100644
--- a/drivers/net/wireless/ath/ath12k/peer.c
+++ b/drivers/net/wireless/ath/ath12k/peer.c
@@ -186,7 +186,7 @@ void ath12k_peer_cleanup(struct ath12k *ar, u32 vdev_id)
 	struct ath12k_peer *peer, *tmp;
 	struct ath12k_base *ab = ar->ab;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	spin_lock_bh(&ab->base_lock);
 	list_for_each_entry_safe(peer, tmp, &ab->peers, list) {
@@ -235,7 +235,7 @@ int ath12k_peer_delete(struct ath12k *ar, u32 vdev_id, u8 *addr)
 {
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	reinit_completion(&ar->peer_delete_done);
 
@@ -268,7 +268,7 @@ int ath12k_peer_create(struct ath12k *ar, struct ath12k_vif *arvif,
 	struct ath12k_peer *peer;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	if (ar->num_peers > (ar->max_num_peers - 1)) {
 		ath12k_warn(ar->ab,
diff --git a/drivers/net/wireless/ath/ath12k/wow.c b/drivers/net/wireless/ath/ath12k/wow.c
index 9b8684abbe40..94a443307448 100644
--- a/drivers/net/wireless/ath/ath12k/wow.c
+++ b/drivers/net/wireless/ath/ath12k/wow.c
@@ -132,7 +132,7 @@ static int ath12k_wow_cleanup(struct ath12k *ar)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ret = ath12k_wow_vif_cleanup(arvif);
@@ -476,7 +476,7 @@ static int ath12k_wow_set_wakeups(struct ath12k *ar,
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (ath12k_wow_is_p2p_vdev(arvif))
@@ -535,7 +535,7 @@ static int ath12k_wow_nlo_cleanup(struct ath12k *ar)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (ath12k_wow_is_p2p_vdev(arvif))
@@ -558,7 +558,7 @@ static int ath12k_wow_set_hw_filter(struct ath12k *ar)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (arvif->vdev_type != WMI_VDEV_TYPE_STA)
@@ -584,7 +584,7 @@ static int ath12k_wow_clear_hw_filter(struct ath12k *ar)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (arvif->vdev_type != WMI_VDEV_TYPE_STA)
@@ -735,7 +735,7 @@ static int ath12k_wow_arp_ns_offload(struct ath12k *ar, bool enable)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	offload = kmalloc(sizeof(*offload), GFP_KERNEL);
 	if (!offload)
@@ -769,7 +769,7 @@ static int ath12k_gtk_rekey_offload(struct ath12k *ar, bool enable)
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		if (arvif->vdev_type != WMI_VDEV_TYPE_STA ||
@@ -827,7 +827,7 @@ static int ath12k_wow_set_keepalive(struct ath12k *ar,
 	struct ath12k_vif *arvif;
 	int ret;
 
-	lockdep_assert_held(&ar->conf_mutex);
+	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
 
 	list_for_each_entry(arvif, &ar->arvifs, list) {
 		ret = ath12k_mac_vif_set_keepalive(arvif, method, interval);
@@ -845,7 +845,7 @@ int ath12k_wow_op_suspend(struct ieee80211_hw *hw,
 	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 	int ret;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ret =  ath12k_wow_cleanup(ar);
 	if (ret) {
@@ -913,7 +913,6 @@ int ath12k_wow_op_suspend(struct ieee80211_hw *hw,
 	ath12k_wow_cleanup(ar);
 
 exit:
-	mutex_unlock(&ar->conf_mutex);
 	return ret ? 1 : 0;
 }
 
@@ -922,9 +921,9 @@ void ath12k_wow_op_set_wakeup(struct ieee80211_hw *hw, bool enabled)
 	struct ath12k_hw *ah = ath12k_hw_to_ah(hw);
 	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
+
 	device_set_wakeup_enable(ar->ab->dev, enabled);
-	mutex_unlock(&ar->conf_mutex);
 }
 
 int ath12k_wow_op_resume(struct ieee80211_hw *hw)
@@ -933,7 +932,7 @@ int ath12k_wow_op_resume(struct ieee80211_hw *hw)
 	struct ath12k *ar = ath12k_ah_to_ar(ah, 0);
 	int ret;
 
-	mutex_lock(&ar->conf_mutex);
+	lockdep_assert_wiphy(hw->wiphy);
 
 	ret = ath12k_hif_resume(ar->ab);
 	if (ret) {
@@ -995,7 +994,6 @@ int ath12k_wow_op_resume(struct ieee80211_hw *hw)
 		}
 	}
 
-	mutex_unlock(&ar->conf_mutex);
 	return ret;
 }
 
-- 
2.39.5

[PATCH RFC v2 2/4] wifi: ath12k: cleanup unneeded labels

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

After removing the conf_mutex in the previous patch there are now unnecessary
labels in mac.c. Sparse also warns one instance of it:

drivers/net/wireless/ath/ath12k/mac.c:6635:1: warning: statement expected after label

Remove the labels and instead use directly return.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 56 +++++++++++----------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index ed900fbef9d8..a9d37a59a8c2 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -1279,24 +1279,23 @@ static int ath12k_mac_config(struct ath12k *ar, u32 changed)
 		ar->monitor_conf_enabled = conf->flags & IEEE80211_CONF_MONITOR;
 		if (ar->monitor_conf_enabled) {
 			if (ar->monitor_vdev_created)
-				goto exit;
+				return ret;
 			ret = ath12k_mac_monitor_vdev_create(ar);
 			if (ret)
-				goto exit;
+				return ret;
 			ret = ath12k_mac_monitor_start(ar);
 			if (ret)
 				goto err_mon_del;
 		} else {
 			if (!ar->monitor_vdev_created)
-				goto exit;
+				return ret;
 			ret = ath12k_mac_monitor_stop(ar);
 			if (ret)
-				goto exit;
+				return ret;
 			ath12k_mac_monitor_vdev_delete(ar);
 		}
 	}
 
-exit:
 	return ret;
 
 err_mon_del:
@@ -4716,10 +4715,9 @@ static int ath12k_mac_op_sta_set_txpwr(struct ieee80211_hw *hw,
 	if (ret) {
 		ath12k_warn(ar->ab, "failed to set tx power for station ret: %d\n",
 			    ret);
-		goto out;
+		return ret;
 	}
 
-out:
 	return ret;
 }
 
@@ -6472,7 +6470,7 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 	if (ret) {
 		ath12k_warn(ab, "failed to create WMI vdev %d: %d\n",
 			    arvif->vdev_id, ret);
-		goto err;
+		return ret;
 	}
 
 	ar->num_created_vdevs++;
@@ -6589,13 +6587,13 @@ static int ath12k_mac_vdev_create(struct ath12k *ar, struct ieee80211_vif *vif)
 		if (ret) {
 			ath12k_warn(ar->ab, "failed to delete peer vdev_id %d addr %pM\n",
 				    arvif->vdev_id, vif->addr);
-			goto err;
+			return ret;
 		}
 
 		ret = ath12k_wait_for_peer_delete_done(ar, arvif->vdev_id,
 						       vif->addr);
 		if (ret)
-			goto err;
+			return ret;
 
 		ar->num_peers--;
 	}
@@ -7627,21 +7625,18 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 	    arvif->vdev_type != WMI_VDEV_TYPE_MONITOR &&
 	    !ath12k_peer_exist_by_vdev_id(ab, arvif->vdev_id)) {
 		memcpy(&arvif->chanctx, ctx, sizeof(*ctx));
-		ret = 0;
-		goto out;
+		return 0;
 	}
 
-	if (WARN_ON(arvif->is_started)) {
-		ret = -EBUSY;
-		goto out;
-	}
+	if (WARN_ON(arvif->is_started))
+		return -EBUSY;
 
 	if (arvif->vdev_type == WMI_VDEV_TYPE_MONITOR) {
 		ret = ath12k_mac_monitor_start(ar);
 		if (ret)
-			goto out;
+			return ret;
 		arvif->is_started = true;
-		goto out;
+		return ret;
 	}
 
 	ret = ath12k_mac_vdev_start(arvif, ctx);
@@ -7649,7 +7644,7 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 		ath12k_warn(ab, "failed to start vdev %i addr %pM on freq %d: %d\n",
 			    arvif->vdev_id, vif->addr,
 			    ctx->def.chan->center_freq, ret);
-		goto out;
+		return ret;
 	}
 
 	if (arvif->vdev_type != WMI_VDEV_TYPE_MONITOR && ar->monitor_vdev_created)
@@ -7659,7 +7654,6 @@ ath12k_mac_op_assign_vif_chanctx(struct ieee80211_hw *hw,
 
 	/* TODO: Setup ps and cts/rts protection */
 
-out:
 	return ret;
 }
 
@@ -8127,10 +8121,8 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 	ldpc = !!(ar->ht_cap_info & WMI_HT_CAP_LDPC);
 
 	sgi = mask->control[band].gi;
-	if (sgi == NL80211_TXRATE_FORCE_LGI) {
-		ret = -EINVAL;
-		goto out;
-	}
+	if (sgi == NL80211_TXRATE_FORCE_LGI)
+		return -EINVAL;
 
 	/* mac80211 doesn't support sending a fixed HT/VHT MCS alone, rather it
 	 * requires passing at least one of used basic rates along with them.
@@ -8146,7 +8138,7 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 		if (ret) {
 			ath12k_warn(ar->ab, "failed to get single legacy rate for vdev %i: %d\n",
 				    arvif->vdev_id, ret);
-			goto out;
+			return ret;
 		}
 		ieee80211_iterate_stations_atomic(hw,
 						  ath12k_mac_disable_peer_fixed_rate,
@@ -8191,8 +8183,7 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 			 */
 			ath12k_warn(ar->ab,
 				    "Setting more than one MCS Value in bitrate mask not supported\n");
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 
 		ieee80211_iterate_stations_atomic(hw,
@@ -8211,7 +8202,6 @@ ath12k_mac_op_set_bitrate_mask(struct ieee80211_hw *hw,
 			    arvif->vdev_id, ret);
 	}
 
-out:
 	return ret;
 }
 
@@ -8522,7 +8512,7 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	spin_unlock_bh(&ar->data_lock);
 
 	if (ret)
-		goto exit;
+		return ret;
 
 	scan_time_msec = hw->wiphy->max_remain_on_channel_duration * 2;
 
@@ -8531,10 +8521,8 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 	arg.num_chan = 1;
 	arg.chan_list = kcalloc(arg.num_chan, sizeof(*arg.chan_list),
 				GFP_KERNEL);
-	if (!arg.chan_list) {
-		ret = -ENOMEM;
-		goto exit;
-	}
+	if (!arg.chan_list)
+		return -ENOMEM;
 
 	arg.vdev_id = arvif->vdev_id;
 	arg.scan_id = ATH12K_SCAN_ID;
@@ -8572,7 +8560,7 @@ static int ath12k_mac_op_remain_on_channel(struct ieee80211_hw *hw,
 
 free_chan_list:
 	kfree(arg.chan_list);
-exit:
+
 	return ret;
 }
 
-- 
2.39.5

[PATCH RFC v2 3/4] wifi: ath12k: ath12k_mac_op_set_key(): remove exit label

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

In ath12k_mac_op_set_key() removing the exit label was a bit more complex as
checkpatch complained about the unnecessary else branch after a return. So
remove the else branch and remove now the unncessary ret initialisation.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/mac.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index a9d37a59a8c2..80db9004cdd7 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -3942,7 +3942,7 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 	struct ath12k_peer *peer;
 	struct ath12k_sta *arsta;
 	const u8 *peer_addr;
-	int ret = 0;
+	int ret;
 	u32 flags = 0;
 
 	lockdep_assert_wiphy(ath12k_ar_to_hw(ar)->wiphy);
@@ -3970,14 +3970,13 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 		if (cmd == SET_KEY) {
 			ath12k_warn(ab, "cannot install key for non-existent peer %pM\n",
 				    peer_addr);
-			ret = -EOPNOTSUPP;
-			goto exit;
-		} else {
-			/* if the peer doesn't exist there is no key to disable
-			 * anymore
-			 */
-			goto exit;
+			return -EOPNOTSUPP;
 		}
+
+		/* if the peer doesn't exist there is no key to disable
+		 * anymore
+		 */
+		return 0;
 	}
 
 	if (key->flags & IEEE80211_KEY_FLAG_PAIRWISE)
@@ -3988,13 +3987,13 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 	ret = ath12k_install_key(arvif, key, cmd, peer_addr, flags);
 	if (ret) {
 		ath12k_warn(ab, "ath12k_install_key failed (%d)\n", ret);
-		goto exit;
+		return ret;
 	}
 
 	ret = ath12k_dp_rx_peer_pn_replay_config(arvif, peer_addr, cmd, key);
 	if (ret) {
 		ath12k_warn(ab, "failed to offload PN replay detection %d\n", ret);
-		goto exit;
+		return ret;
 	}
 
 	spin_lock_bh(&ab->base_lock);
@@ -4040,8 +4039,7 @@ static int ath12k_mac_set_key(struct ath12k *ar, enum set_key_cmd cmd,
 
 	spin_unlock_bh(&ab->base_lock);
 
-exit:
-	return ret;
+	return 0;
 }
 
 static int ath12k_mac_op_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd,
-- 
2.39.5

[PATCH RFC v2 4/4] wifi: ath12k: convert struct ath12k_sta::update_wk to use struct wiphy_work

[Kalle Valo]

From: Kalle Valo <quic_kvalo@quicinc.com>

As ath12k is now converted to use wiphy lock we can convert
ath12k_sta_rc_update_wk() to use wiphy_work_queue(). This is just for
consistency.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3

Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.h |  2 +-
 drivers/net/wireless/ath/ath12k/mac.c  | 14 ++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 7551494716b5..ebfc1e370acc 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -451,7 +451,7 @@ struct ath12k_sta {
 	u32 smps;
 	enum hal_pn_type pn_type;
 
-	struct work_struct update_wk;
+	struct wiphy_work update_wk;
 	struct rate_info txrate;
 	struct rate_info last_txrate;
 	u64 rx_duration;
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 80db9004cdd7..e70b4212ae80 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -4258,7 +4258,7 @@ static int ath12k_station_disassoc(struct ath12k *ar,
 	return 0;
 }
 
-static void ath12k_sta_rc_update_wk(struct work_struct *wk)
+static void ath12k_sta_rc_update_wk(struct wiphy *wiphy, struct wiphy_work *work)
 {
 	struct ath12k *ar;
 	struct ath12k_vif *arvif;
@@ -4274,7 +4274,9 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
 	struct ath12k_wmi_peer_assoc_arg peer_arg;
 	enum wmi_phy_mode peer_phymode;
 
-	arsta = container_of(wk, struct ath12k_sta, update_wk);
+	lockdep_assert_wiphy(wiphy);
+
+	arsta = container_of(work, struct ath12k_sta, update_wk);
 	sta = container_of((void *)arsta, struct ieee80211_sta, drv_priv);
 	arvif = arsta->arvif;
 	ar = arvif->ar;
@@ -4571,7 +4573,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 	/* cancel must be done outside the mutex to avoid deadlock */
 	if ((old_state == IEEE80211_STA_NONE &&
 	     new_state == IEEE80211_STA_NOTEXIST))
-		cancel_work_sync(&arsta->update_wk);
+		wiphy_work_cancel(hw->wiphy, &arsta->update_wk);
 
 	ar = ath12k_get_ar_by_vif(hw, vif);
 	if (!ar) {
@@ -4585,7 +4587,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
 	    new_state == IEEE80211_STA_NONE) {
 		memset(arsta, 0, sizeof(*arsta));
 		arsta->arvif = arvif;
-		INIT_WORK(&arsta->update_wk, ath12k_sta_rc_update_wk);
+		wiphy_work_init(&arsta->update_wk, ath12k_sta_rc_update_wk);
 
 		ret = ath12k_mac_station_add(ar, vif, sta);
 		if (ret)
@@ -4792,7 +4794,7 @@ static void ath12k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
 
 	spin_unlock_bh(&ar->data_lock);
 
-	ieee80211_queue_work(hw, &arsta->update_wk);
+	wiphy_work_queue(hw->wiphy, &arsta->update_wk);
 }
 
 static int ath12k_conf_tx_uapsd(struct ath12k_vif *arvif,
@@ -8065,7 +8067,7 @@ static void ath12k_mac_set_bitrate_mask_iter(void *data,
 	arsta->changed |= IEEE80211_RC_SUPP_RATES_CHANGED;
 	spin_unlock_bh(&ar->data_lock);
 
-	ieee80211_queue_work(ath12k_ar_to_hw(ar), &arsta->update_wk);
+	wiphy_work_queue(ath12k_ar_to_hw(ar)->wiphy, &arsta->update_wk);
 }
 
 static void ath12k_mac_disable_peer_fixed_rate(void *data,
-- 
2.39.5

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Baochen Qiang]

On 9/19/2024 2:10 AM, Kalle Valo wrote:
> @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>  
>  	spin_unlock_bh(&ar->data_lock);
>  
> -	mutex_lock(&ar->conf_mutex);
> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
note in patch 4/4 ath12k_sta::update_wk is converted to use wiphy_work. While a wiphy work item is running wiphy lock is held already. So here try to acquire wiphy lock once again will lead to a deadlock.

>  
>  	nss = max_t(u32, 1, nss);
>  	nss = min(nss, max(ath12k_mac_max_ht_nss(ht_mcs_mask),
> @@ -4426,7 +4417,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>  		}
>  	}
>  err_rc_bw_changed:
> -	mutex_unlock(&ar->conf_mutex);
> +	wiphy_unlock(ath12k_ar_to_hw(ar)->wiphy);
>  }

Re: [PATCH RFC v2 4/4] wifi: ath12k: convert struct ath12k_sta::update_wk to use struct wiphy_work

[Baochen Qiang]

On 9/19/2024 2:10 AM, Kalle Valo wrote:
> From: Kalle Valo <quic_kvalo@quicinc.com>
> 
> As ath12k is now converted to use wiphy lock we can convert
> ath12k_sta_rc_update_wk() to use wiphy_work_queue(). This is just for
> consistency.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0.c5-00481-QCAHMTSWPL_V1.0_V2.0_SILICONZ-3
> 
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.h |  2 +-
>  drivers/net/wireless/ath/ath12k/mac.c  | 14 ++++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 7551494716b5..ebfc1e370acc 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -451,7 +451,7 @@ struct ath12k_sta {
>  	u32 smps;
>  	enum hal_pn_type pn_type;
>  
> -	struct work_struct update_wk;
> +	struct wiphy_work update_wk;
>  	struct rate_info txrate;
>  	struct rate_info last_txrate;
>  	u64 rx_duration;
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 80db9004cdd7..e70b4212ae80 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -4258,7 +4258,7 @@ static int ath12k_station_disassoc(struct ath12k *ar,
>  	return 0;
>  }
>  
> -static void ath12k_sta_rc_update_wk(struct work_struct *wk)
> +static void ath12k_sta_rc_update_wk(struct wiphy *wiphy, struct wiphy_work *work)
>  {
>  	struct ath12k *ar;
>  	struct ath12k_vif *arvif;
> @@ -4274,7 +4274,9 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>  	struct ath12k_wmi_peer_assoc_arg peer_arg;
>  	enum wmi_phy_mode peer_phymode;
>  
> -	arsta = container_of(wk, struct ath12k_sta, update_wk);
> +	lockdep_assert_wiphy(wiphy);
> +
> +	arsta = container_of(work, struct ath12k_sta, update_wk);
>  	sta = container_of((void *)arsta, struct ieee80211_sta, drv_priv);
>  	arvif = arsta->arvif;
>  	ar = arvif->ar;
> @@ -4571,7 +4573,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
>  	/* cancel must be done outside the mutex to avoid deadlock */
remove this comment

>  	if ((old_state == IEEE80211_STA_NONE &&
>  	     new_state == IEEE80211_STA_NOTEXIST))
> -		cancel_work_sync(&arsta->update_wk);
> +		wiphy_work_cancel(hw->wiphy, &arsta->update_wk);

with wiphy lock used, the race condition with ar->conf_mutex is gone so deadlock is not possible. therefore we don;t need to do work cancel individually here, better to squash this hunk into the other one. This also achieves symmetry.


	} else if ((old_state == IEEE80211_STA_NONE &&
		    new_state == IEEE80211_STA_NOTEXIST)) {
		...
		/* I mean here */
		...
	}

>  
>  	ar = ath12k_get_ar_by_vif(hw, vif);
>  	if (!ar) {
> @@ -4585,7 +4587,7 @@ static int ath12k_mac_op_sta_state(struct ieee80211_hw *hw,
>  	    new_state == IEEE80211_STA_NONE) {
>  		memset(arsta, 0, sizeof(*arsta));
>  		arsta->arvif = arvif;
> -		INIT_WORK(&arsta->update_wk, ath12k_sta_rc_update_wk);
> +		wiphy_work_init(&arsta->update_wk, ath12k_sta_rc_update_wk);
>  
>  		ret = ath12k_mac_station_add(ar, vif, sta);
>  		if (ret)
> @@ -4792,7 +4794,7 @@ static void ath12k_mac_op_sta_rc_update(struct ieee80211_hw *hw,
>  
>  	spin_unlock_bh(&ar->data_lock);
>  
> -	ieee80211_queue_work(hw, &arsta->update_wk);
> +	wiphy_work_queue(hw->wiphy, &arsta->update_wk);
>  }
>  
>  static int ath12k_conf_tx_uapsd(struct ath12k_vif *arvif,
> @@ -8065,7 +8067,7 @@ static void ath12k_mac_set_bitrate_mask_iter(void *data,
>  	arsta->changed |= IEEE80211_RC_SUPP_RATES_CHANGED;
>  	spin_unlock_bh(&ar->data_lock);
>  
> -	ieee80211_queue_work(ath12k_ar_to_hw(ar), &arsta->update_wk);
> +	wiphy_work_queue(ath12k_ar_to_hw(ar)->wiphy, &arsta->update_wk);
>  }
>  
>  static void ath12k_mac_disable_peer_fixed_rate(void *data,

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Johannes Berg]

On Wed, 2024-09-18 at 21:10 +0300, Kalle Valo wrote:
> 
> There is now a new sparse warning, but to keep this long patch simple the
> labels will be cleaned up in following patches:
> 
> drivers/net/wireless/ath/ath12k/mac.c:6635:1: warning: statement expected after label

I believe this is a compiler error on some compilers (in particular
clang), so you probably need to combine patches a little bit.

> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
> @@ -15,14 +15,14 @@ static ssize_t ath12k_write_simulate_radar(struct file *file,
>  	struct ath12k *ar = file->private_data;
>  	int ret;
>  
> -	mutex_lock(&ar->conf_mutex);
> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);

I don't think this is an issue here, but I'm not sure if you're aware
that in general, locking the wiphy mutex in some debugfs file handlers
can lead to deadlocks, specifically if those files are later _removed_
while holding the wiphy lock, as is e.g. the case for station, netdev
and link debugfs files. For this, we have wiphy_locked_debugfs_read()
and wiphy_locked_debugfs_write() [1].

[1] you are not required to understand how they are implemented ;-)

> @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>  
>  	spin_unlock_bh(&ar->data_lock);
>  
> -	mutex_lock(&ar->conf_mutex);
> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);

Baochen already pointed out that you seem to not remove this later in
patch 4, but in this patch alone you also introduce a bug (that lockdep
should point out to you), which is that you cancel_work_sync() this in
ath12k_mac_op_sta_state(), which clearly holds the wiphy mutex already.

This causes a deadlock. It's fine after patch 4:

>  	/* cancel must be done outside the mutex to avoid deadlock */
>  	if ((old_state == IEEE80211_STA_NONE &&
>  	     new_state == IEEE80211_STA_NOTEXIST))
> -		cancel_work_sync(&arsta->update_wk);
> +		wiphy_work_cancel(hw->wiphy, &arsta->update_wk);

since for wiphy work it's required to ca<ll it with the mutex held./

You really should remove that comment too though, and perhaps then the
code can be simplified by moving this to the later code also handling
removal (none->notexist transition).

johannes

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Kalle Valo]

Baochen Qiang <quic_bqiang@quicinc.com> writes:

> On 9/19/2024 2:10 AM, Kalle Valo wrote:
>> @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>>  
>>  	spin_unlock_bh(&ar->data_lock);
>>  
>> -	mutex_lock(&ar->conf_mutex);
>> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
>
> note in patch 4/4 ath12k_sta::update_wk is converted to use
> wiphy_work. While a wiphy work item is running wiphy lock is held
> already. So here try to acquire wiphy lock once again will lead to a
> deadlock.

Ouch again, thanks for catching this! This time I actually tested
changing bitrates and it shouldn't deadlock in v3. But I did notice
sleeping while atomic warnings (even without this patchset) and decided
to fix those in the same patchset as well.

Oh, and WCN6855 firmware was also crashing whenever I tried to change
the bitrates. But let's handle that separately.

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

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

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Kalle Valo]

Johannes Berg <johannes@sipsolutions.net> writes:

> On Wed, 2024-09-18 at 21:10 +0300, Kalle Valo wrote:
>> 
>> There is now a new sparse warning, but to keep this long patch simple the
>> labels will be cleaned up in following patches:
>> 
>> drivers/net/wireless/ath/ath12k/mac.c:6635:1: warning: statement expected after label
>
> I believe this is a compiler error on some compilers (in particular
> clang), so you probably need to combine patches a little bit.

I had actually already fixed this but forgot to update the commit
message, did that now in v3.

>> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
>> @@ -15,14 +15,14 @@ static ssize_t ath12k_write_simulate_radar(struct file *file,
>>  	struct ath12k *ar = file->private_data;
>>  	int ret;
>>  
>> -	mutex_lock(&ar->conf_mutex);
>> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
>
> I don't think this is an issue here, but I'm not sure if you're aware
> that in general, locking the wiphy mutex in some debugfs file handlers
> can lead to deadlocks, specifically if those files are later _removed_
> while holding the wiphy lock, as is e.g. the case for station, netdev
> and link debugfs files. For this, we have wiphy_locked_debugfs_read()
> and wiphy_locked_debugfs_write() [1].
>
> [1] you are not required to understand how they are implemented ;-)

Thanks, this is good to know. I'm not that worried about that, at least
it's not showing up in my tests, so my plan is to fix that in a separate
patchset.

>> @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>>  
>>  	spin_unlock_bh(&ar->data_lock);
>>  
>> -	mutex_lock(&ar->conf_mutex);
>> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
>
> Baochen already pointed out that you seem to not remove this later in
> patch 4, but in this patch alone you also introduce a bug (that lockdep
> should point out to you), which is that you cancel_work_sync() this in
> ath12k_mac_op_sta_state(), which clearly holds the wiphy mutex already.
>
> This causes a deadlock. It's fine after patch 4:
>
>>  	/* cancel must be done outside the mutex to avoid deadlock */
>>  	if ((old_state == IEEE80211_STA_NONE &&
>>  	     new_state == IEEE80211_STA_NOTEXIST))
>> -		cancel_work_sync(&arsta->update_wk);
>> +		wiphy_work_cancel(hw->wiphy, &arsta->update_wk);
>
> since for wiphy work it's required to ca<ll it with the mutex held./

Excellent find! I fixed it so that I moved this patch before switching
to use wiphy_lock(). There should not be a deadlock anymore, hopefully
:)

> You really should remove that comment too though, and perhaps then the
> code can be simplified by moving this to the later code also handling
> removal (none->notexist transition).

Good point. In v3 I added a new patch for this.

Thanks again for the review, I owe you so many beers[1] that it's starting
to get difficult to store them ;)

[1] https://en.wikipedia.org/wiki/Karhu_(beer)

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

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

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Johannes Berg]

On Tue, 2024-09-24 at 12:11 +0300, Kalle Valo wrote:
> > I don't think this is an issue here, but I'm not sure if you're aware
> > that in general, locking the wiphy mutex in some debugfs file handlers
> > can lead to deadlocks, specifically if those files are later _removed_
> > while holding the wiphy lock, as is e.g. the case for station, netdev
> > and link debugfs files. For this, we have wiphy_locked_debugfs_read()
> > and wiphy_locked_debugfs_write() [1].
> > 
> > [1] you are not required to understand how they are implemented ;-)
> 
> Thanks, this is good to know. I'm not that worried about that, at least
> it's not showing up in my tests, so my plan is to fix that in a separate
> patchset.

I don't think you'd ever even find that in tests, as far as I know
lockdep cannot track these dependencies, and to actually deadlock you'd
have to have the debugfs file(s) kept open by userspace while they're
removed.

In any case, just wanted to give a heads-up that this might be required
in some (future) cases, I didn't see any here where it was needed.

johannes

Re: [PATCH RFC v2 1/4] wifi: ath12k: switch to using wiphy_lock() and remove ar->conf_mutex

[Kalle Valo]

Kalle Valo <kvalo@kernel.org> writes:

> Baochen Qiang <quic_bqiang@quicinc.com> writes:
>
>> On 9/19/2024 2:10 AM, Kalle Valo wrote:
>>> @@ -4310,7 +4301,7 @@ static void ath12k_sta_rc_update_wk(struct work_struct *wk)
>>>  
>>>  	spin_unlock_bh(&ar->data_lock);
>>>  
>>> -	mutex_lock(&ar->conf_mutex);
>>> +	wiphy_lock(ath12k_ar_to_hw(ar)->wiphy);
>>
>> note in patch 4/4 ath12k_sta::update_wk is converted to use
>> wiphy_work. While a wiphy work item is running wiphy lock is held
>> already. So here try to acquire wiphy lock once again will lead to a
>> deadlock.
>
> Ouch again, thanks for catching this! This time I actually tested
> changing bitrates and it shouldn't deadlock in v3. But I did notice
> sleeping while atomic warnings (even without this patchset) and decided
> to fix those in the same patchset as well.
>
> Oh, and WCN6855 firmware was also crashing whenever I tried to change
> the bitrates. But let's handle that separately.

Bah, I of course mean WCN7850. Too many chipsets...

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

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