[PATCH 0/2] wifi: ath12k: add support for get_txpower mac ops

[PATCH 0/2] wifi: ath12k: add support for get_txpower mac ops

[Rameshkumar Sundaram]

Currently, driver does not support get_txpower mac ops because of which
cfg80211 returns vif->bss_conf.txpower to user space. bss_conf.txpower
gets its value from ieee80211_channel->max_reg_power. However, the final
txpower is dependent on few other parameters apart from max regulatory
supported power. It is the firmware which knows about all these parameters
and considers the minimum for each packet transmission.

Firmware reports the final TX power in firmware pdev stats which falls
under fw_stats. But currently, fw_stats is under debugfs.

Add support for get_txpower mac ops to get the TX power from firmware
leveraging fw_stats and return it accordingly.

Also, move fw_stats out of debugfs so that get_txpower mac ops can
function properly even when debugfs is disabled.

Aditya Kumar Singh (2):
  wifi: ath12k: move firmware stats out of debugfs
  wifi: ath12k: add get_txpower mac ops

 drivers/net/wireless/ath/ath12k/core.c    |  45 +++++++
 drivers/net/wireless/ath/ath12k/core.h    |   4 +
 drivers/net/wireless/ath/ath12k/debugfs.c |  44 +-----
 drivers/net/wireless/ath/ath12k/mac.c     | 155 +++++++++++++++++-----
 drivers/net/wireless/ath/ath12k/mac.h     |   3 +
 drivers/net/wireless/ath/ath12k/wmi.c     |  94 ++++++++++---
 6 files changed, 247 insertions(+), 98 deletions(-)


base-commit: b5aeca2e66899430827b8afcad061201f3b7861b
-- 
2.34.1

[PATCH 1/2] wifi: ath12k: move firmware stats out of debugfs

[Rameshkumar Sundaram]

From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>

Currently, firmware stats, comprising pdev, vdev and beacon stats are
part of debugfs. In firmware pdev stats, firmware reports the final
Tx power used to transmit each packet. If driver wants to know the
final Tx power being used at firmware level, it can leverage from
firmware pdev stats.

Move firmware stats out of debugfs context in order to leverage
the final Tx power reported in it even when debugfs is disabled.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/core.c    | 45 +++++++++++
 drivers/net/wireless/ath/ath12k/core.h    |  3 +
 drivers/net/wireless/ath/ath12k/debugfs.c | 44 +----------
 drivers/net/wireless/ath/ath12k/wmi.c     | 94 ++++++++++++++++++-----
 4 files changed, 124 insertions(+), 62 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 2dd0666959cd..122b407cd322 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -1052,6 +1052,51 @@ bool ath12k_core_hw_group_start_ready(struct ath12k_hw_group *ag)
 	return (ag->num_started == ag->num_devices);
 }
 
+static void ath12k_fw_stats_pdevs_free(struct list_head *head)
+{
+	struct ath12k_fw_stats_pdev *i, *tmp;
+
+	list_for_each_entry_safe(i, tmp, head, list) {
+		list_del(&i->list);
+		kfree(i);
+	}
+}
+
+void ath12k_fw_stats_bcn_free(struct list_head *head)
+{
+	struct ath12k_fw_stats_bcn *i, *tmp;
+
+	list_for_each_entry_safe(i, tmp, head, list) {
+		list_del(&i->list);
+		kfree(i);
+	}
+}
+
+static void ath12k_fw_stats_vdevs_free(struct list_head *head)
+{
+	struct ath12k_fw_stats_vdev *i, *tmp;
+
+	list_for_each_entry_safe(i, tmp, head, list) {
+		list_del(&i->list);
+		kfree(i);
+	}
+}
+
+void ath12k_fw_stats_init(struct ath12k *ar)
+{
+	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
+	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
+	INIT_LIST_HEAD(&ar->fw_stats.bcn);
+	init_completion(&ar->fw_stats_complete);
+}
+
+void ath12k_fw_stats_free(struct ath12k_fw_stats *stats)
+{
+	ath12k_fw_stats_pdevs_free(&stats->pdevs);
+	ath12k_fw_stats_vdevs_free(&stats->vdevs);
+	ath12k_fw_stats_bcn_free(&stats->bcn);
+}
+
 static void ath12k_core_trigger_partner(struct ath12k_base *ab)
 {
 	struct ath12k_hw_group *ag = ab->ag;
diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index 28db100cfac0..e4f51ad6a59f 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -1198,6 +1198,9 @@ u32 ath12k_core_get_max_peers_per_radio(struct ath12k_base *ab);
 u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab);
 
 void ath12k_core_hw_group_set_mlo_capable(struct ath12k_hw_group *ag);
+void ath12k_fw_stats_init(struct ath12k *ar);
+void ath12k_fw_stats_bcn_free(struct list_head *head);
+void ath12k_fw_stats_free(struct ath12k_fw_stats *stats);
 
 static inline const char *ath12k_scan_state_str(enum ath12k_scan_state state)
 {
diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
index 6d6708486d14..4e4c2ef6a7ce 100644
--- a/drivers/net/wireless/ath/ath12k/debugfs.c
+++ b/drivers/net/wireless/ath/ath12k/debugfs.c
@@ -69,43 +69,11 @@ void ath12k_debugfs_soc_destroy(struct ath12k_base *ab)
 	 */
 }
 
-static void ath12k_fw_stats_pdevs_free(struct list_head *head)
-{
-	struct ath12k_fw_stats_pdev *i, *tmp;
-
-	list_for_each_entry_safe(i, tmp, head, list) {
-		list_del(&i->list);
-		kfree(i);
-	}
-}
-
-static void ath12k_fw_stats_bcn_free(struct list_head *head)
-{
-	struct ath12k_fw_stats_bcn *i, *tmp;
-
-	list_for_each_entry_safe(i, tmp, head, list) {
-		list_del(&i->list);
-		kfree(i);
-	}
-}
-
-static void ath12k_fw_stats_vdevs_free(struct list_head *head)
-{
-	struct ath12k_fw_stats_vdev *i, *tmp;
-
-	list_for_each_entry_safe(i, tmp, head, list) {
-		list_del(&i->list);
-		kfree(i);
-	}
-}
-
 void ath12k_debugfs_fw_stats_reset(struct ath12k *ar)
 {
 	spin_lock_bh(&ar->data_lock);
 	ar->fw_stats.fw_stats_done = false;
-	ath12k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
-	ath12k_fw_stats_bcn_free(&ar->fw_stats.bcn);
-	ath12k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
+	ath12k_fw_stats_free(&ar->fw_stats);
 	spin_unlock_bh(&ar->data_lock);
 }
 
@@ -221,10 +189,6 @@ ath12k_debugfs_fw_stats_process(struct ath12k *ar,
 			num_bcn = 0;
 		}
 	}
-	if (stats->stats_id == WMI_REQUEST_PDEV_STAT) {
-		list_splice_tail_init(&stats->pdevs, &ar->fw_stats.pdevs);
-		ar->fw_stats.fw_stats_done = true;
-	}
 }
 
 static int ath12k_open_vdev_stats(struct inode *inode, struct file *file)
@@ -438,11 +402,7 @@ void ath12k_debugfs_fw_stats_register(struct ath12k *ar)
 	debugfs_create_file("pdev_stats", 0600, fwstats_dir, ar,
 			    &fops_pdev_stats);
 
-	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
-	INIT_LIST_HEAD(&ar->fw_stats.bcn);
-	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
-
-	init_completion(&ar->fw_stats_complete);
+	ath12k_fw_stats_init(ar);
 }
 
 void ath12k_debugfs_register(struct ath12k *ar)
diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
index 61aa5f509338..1a7af09853a4 100644
--- a/drivers/net/wireless/ath/ath12k/wmi.c
+++ b/drivers/net/wireless/ath/ath12k/wmi.c
@@ -29,6 +29,7 @@ struct ath12k_wmi_svc_ready_parse {
 
 struct wmi_tlv_fw_stats_parse {
 	const struct wmi_stats_event *ev;
+	struct ath12k_fw_stats *stats;
 };
 
 struct ath12k_wmi_dma_ring_caps_parse {
@@ -7314,7 +7315,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 					      u16 len)
 {
 	const struct wmi_stats_event *ev = parse->ev;
-	struct ath12k_fw_stats stats = {0};
+	struct ath12k_fw_stats *stats = parse->stats;
 	struct ath12k *ar;
 	struct ath12k_link_vif *arvif;
 	struct ieee80211_sta *sta;
@@ -7323,10 +7324,6 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 	int i, ret = 0;
 	const void *data = ptr;
 
-	INIT_LIST_HEAD(&stats.vdevs);
-	INIT_LIST_HEAD(&stats.bcn);
-	INIT_LIST_HEAD(&stats.pdevs);
-
 	if (!ev) {
 		ath12k_warn(ab, "failed to fetch update stats ev");
 		return -EPROTO;
@@ -7334,7 +7331,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 
 	rcu_read_lock();
 
-	ar = ath12k_mac_get_ar_by_pdev_id(ab, le32_to_cpu(ev->pdev_id));
+	stats->pdev_id = le32_to_cpu(ev->pdev_id);
+	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats->pdev_id);
 	if (!ar) {
 		ath12k_warn(ab, "invalid pdev id %d in update stats event\n",
 			    le32_to_cpu(ev->pdev_id));
@@ -7377,8 +7375,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 		if (!dst)
 			continue;
 		ath12k_wmi_pull_vdev_stats(src, dst);
-		stats.stats_id = WMI_REQUEST_VDEV_STAT;
-		list_add_tail(&dst->list, &stats.vdevs);
+		stats->stats_id = WMI_REQUEST_VDEV_STAT;
+		list_add_tail(&dst->list, &stats->vdevs);
 	}
 	for (i = 0; i < le32_to_cpu(ev->num_bcn_stats); i++) {
 		const struct ath12k_wmi_bcn_stats_params *src;
@@ -7396,8 +7394,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 		if (!dst)
 			continue;
 		ath12k_wmi_pull_bcn_stats(src, dst);
-		stats.stats_id = WMI_REQUEST_BCN_STAT;
-		list_add_tail(&dst->list, &stats.bcn);
+		stats->stats_id = WMI_REQUEST_BCN_STAT;
+		list_add_tail(&dst->list, &stats->bcn);
 	}
 	for (i = 0; i < le32_to_cpu(ev->num_pdev_stats); i++) {
 		const struct ath12k_wmi_pdev_stats_params *src;
@@ -7409,7 +7407,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 			goto exit;
 		}
 
-		stats.stats_id = WMI_REQUEST_PDEV_STAT;
+		stats->stats_id = WMI_REQUEST_PDEV_STAT;
 
 		data += sizeof(*src);
 		len -= sizeof(*src);
@@ -7421,11 +7419,9 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
 		ath12k_wmi_pull_pdev_stats_base(&src->base, dst);
 		ath12k_wmi_pull_pdev_stats_tx(&src->tx, dst);
 		ath12k_wmi_pull_pdev_stats_rx(&src->rx, dst);
-		list_add_tail(&dst->list, &stats.pdevs);
+		list_add_tail(&dst->list, &stats->pdevs);
 	}
 
-	complete(&ar->fw_stats_complete);
-	ath12k_debugfs_fw_stats_process(ar, &stats);
 exit:
 	rcu_read_unlock();
 	return ret;
@@ -7451,16 +7447,74 @@ static int ath12k_wmi_tlv_fw_stats_parse(struct ath12k_base *ab,
 	return ret;
 }
 
+static int ath12k_wmi_pull_fw_stats(struct ath12k_base *ab, struct sk_buff *skb,
+				    struct ath12k_fw_stats *stats)
+{
+	struct wmi_tlv_fw_stats_parse parse = {};
+
+	stats->stats_id = 0;
+	parse.stats = stats;
+
+	return ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
+				   ath12k_wmi_tlv_fw_stats_parse,
+				   &parse);
+}
+
 static void ath12k_update_stats_event(struct ath12k_base *ab, struct sk_buff *skb)
 {
+	struct ath12k_fw_stats stats = {};
+	struct ath12k *ar;
 	int ret;
-	struct wmi_tlv_fw_stats_parse parse = {};
 
-	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
-				  ath12k_wmi_tlv_fw_stats_parse,
-				  &parse);
-	if (ret)
-		ath12k_warn(ab, "failed to parse fw stats %d\n", ret);
+	INIT_LIST_HEAD(&stats.pdevs);
+	INIT_LIST_HEAD(&stats.vdevs);
+	INIT_LIST_HEAD(&stats.bcn);
+
+	ret = ath12k_wmi_pull_fw_stats(ab, skb, &stats);
+	if (ret) {
+		ath12k_warn(ab, "failed to pull fw stats: %d\n", ret);
+		goto free;
+	}
+
+	ath12k_dbg(ab, ATH12K_DBG_WMI, "event update stats");
+
+	rcu_read_lock();
+	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
+	if (!ar) {
+		rcu_read_unlock();
+		ath12k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
+			    stats.pdev_id, ret);
+		goto free;
+	}
+
+	spin_lock_bh(&ar->data_lock);
+
+	/* WMI_REQUEST_PDEV_STAT can be requested via .get_txpower mac ops or via
+	 * debugfs fw stats. Therefore, processing it separately.
+	 */
+	if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
+		list_splice_tail_init(&stats.pdevs, &ar->fw_stats.pdevs);
+		ar->fw_stats.fw_stats_done = true;
+		goto complete;
+	}
+
+	/* WMI_REQUEST_VDEV_STAT and WMI_REQUEST_BCN_STAT are currently requested only
+	 * via debugfs fw stats. Hence, processing these in debugfs context.
+	 */
+	ath12k_debugfs_fw_stats_process(ar, &stats);
+
+complete:
+	complete(&ar->fw_stats_complete);
+	spin_unlock_bh(&ar->data_lock);
+	rcu_read_unlock();
+
+	/* Since the stats's pdev, vdev and beacon list are spliced and reinitialised
+	 * at this point, no need to free the individual list.
+	 */
+	return;
+
+free:
+	ath12k_fw_stats_free(&stats);
 }
 
 /* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the frequency scanned
-- 
2.34.1

Re: [PATCH 1/2] wifi: ath12k: move firmware stats out of debugfs

[Mahendran P]

On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
> From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
> 
> Currently, firmware stats, comprising pdev, vdev and beacon stats are
> part of debugfs. In firmware pdev stats, firmware reports the final
> Tx power used to transmit each packet. If driver wants to know the
> final Tx power being used at firmware level, it can leverage from
> firmware pdev stats.
> 
> Move firmware stats out of debugfs context in order to leverage
> the final Tx power reported in it even when debugfs is disabled.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.c    | 45 +++++++++++
>  drivers/net/wireless/ath/ath12k/core.h    |  3 +
>  drivers/net/wireless/ath/ath12k/debugfs.c | 44 +----------
>  drivers/net/wireless/ath/ath12k/wmi.c     | 94 ++++++++++++++++++-----
>  4 files changed, 124 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
> index 2dd0666959cd..122b407cd322 100644
> --- a/drivers/net/wireless/ath/ath12k/core.c
> +++ b/drivers/net/wireless/ath/ath12k/core.c
> @@ -1052,6 +1052,51 @@ bool ath12k_core_hw_group_start_ready(struct ath12k_hw_group *ag)
>  	return (ag->num_started == ag->num_devices);
>  }
>  
> +static void ath12k_fw_stats_pdevs_free(struct list_head *head)
> +{
> +	struct ath12k_fw_stats_pdev *i, *tmp;
> +
> +	list_for_each_entry_safe(i, tmp, head, list) {
> +		list_del(&i->list);
> +		kfree(i);
> +	}
> +}
> +
> +void ath12k_fw_stats_bcn_free(struct list_head *head)
> +{
> +	struct ath12k_fw_stats_bcn *i, *tmp;
> +
> +	list_for_each_entry_safe(i, tmp, head, list) {
> +		list_del(&i->list);
> +		kfree(i);
> +	}
> +}
> +
> +static void ath12k_fw_stats_vdevs_free(struct list_head *head)
> +{
> +	struct ath12k_fw_stats_vdev *i, *tmp;
> +
> +	list_for_each_entry_safe(i, tmp, head, list) {
> +		list_del(&i->list);
> +		kfree(i);
> +	}
> +}
> +
> +void ath12k_fw_stats_init(struct ath12k *ar)
> +{
> +	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
> +	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
> +	INIT_LIST_HEAD(&ar->fw_stats.bcn);
> +	init_completion(&ar->fw_stats_complete);
> +}
> +
> +void ath12k_fw_stats_free(struct ath12k_fw_stats *stats)
> +{
> +	ath12k_fw_stats_pdevs_free(&stats->pdevs);
> +	ath12k_fw_stats_vdevs_free(&stats->vdevs);
> +	ath12k_fw_stats_bcn_free(&stats->bcn);
> +}
> +
>  static void ath12k_core_trigger_partner(struct ath12k_base *ab)
>  {
>  	struct ath12k_hw_group *ag = ab->ag;
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index 28db100cfac0..e4f51ad6a59f 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -1198,6 +1198,9 @@ u32 ath12k_core_get_max_peers_per_radio(struct ath12k_base *ab);
>  u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab);
>  
>  void ath12k_core_hw_group_set_mlo_capable(struct ath12k_hw_group *ag);
> +void ath12k_fw_stats_init(struct ath12k *ar);
> +void ath12k_fw_stats_bcn_free(struct list_head *head);
> +void ath12k_fw_stats_free(struct ath12k_fw_stats *stats);
>  
>  static inline const char *ath12k_scan_state_str(enum ath12k_scan_state state)
>  {
> diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
> index 6d6708486d14..4e4c2ef6a7ce 100644
> --- a/drivers/net/wireless/ath/ath12k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
> @@ -69,43 +69,11 @@ void ath12k_debugfs_soc_destroy(struct ath12k_base *ab)
>  	 */
>  }
>  
> -static void ath12k_fw_stats_pdevs_free(struct list_head *head)
> -{
> -	struct ath12k_fw_stats_pdev *i, *tmp;
> -
> -	list_for_each_entry_safe(i, tmp, head, list) {
> -		list_del(&i->list);
> -		kfree(i);
> -	}
> -}
> -
> -static void ath12k_fw_stats_bcn_free(struct list_head *head)
> -{
> -	struct ath12k_fw_stats_bcn *i, *tmp;
> -
> -	list_for_each_entry_safe(i, tmp, head, list) {
> -		list_del(&i->list);
> -		kfree(i);
> -	}
> -}
> -
> -static void ath12k_fw_stats_vdevs_free(struct list_head *head)
> -{
> -	struct ath12k_fw_stats_vdev *i, *tmp;
> -
> -	list_for_each_entry_safe(i, tmp, head, list) {
> -		list_del(&i->list);
> -		kfree(i);
> -	}
> -}
> -
>  void ath12k_debugfs_fw_stats_reset(struct ath12k *ar)
>  {
>  	spin_lock_bh(&ar->data_lock);
>  	ar->fw_stats.fw_stats_done = false;
> -	ath12k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
> -	ath12k_fw_stats_bcn_free(&ar->fw_stats.bcn);
> -	ath12k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
> +	ath12k_fw_stats_free(&ar->fw_stats);
>  	spin_unlock_bh(&ar->data_lock);
>  }
>  
> @@ -221,10 +189,6 @@ ath12k_debugfs_fw_stats_process(struct ath12k *ar,
>  			num_bcn = 0;
>  		}
>  	}
> -	if (stats->stats_id == WMI_REQUEST_PDEV_STAT) {
> -		list_splice_tail_init(&stats->pdevs, &ar->fw_stats.pdevs);
> -		ar->fw_stats.fw_stats_done = true;
> -	}
>  }
>  
>  static int ath12k_open_vdev_stats(struct inode *inode, struct file *file)
> @@ -438,11 +402,7 @@ void ath12k_debugfs_fw_stats_register(struct ath12k *ar)
>  	debugfs_create_file("pdev_stats", 0600, fwstats_dir, ar,
>  			    &fops_pdev_stats);
>  
> -	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
> -	INIT_LIST_HEAD(&ar->fw_stats.bcn);
> -	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
> -
> -	init_completion(&ar->fw_stats_complete);
> +	ath12k_fw_stats_init(ar);
>  }
>  
>  void ath12k_debugfs_register(struct ath12k *ar)
> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
> index 61aa5f509338..1a7af09853a4 100644
> --- a/drivers/net/wireless/ath/ath12k/wmi.c
> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
> @@ -29,6 +29,7 @@ struct ath12k_wmi_svc_ready_parse {
>  
>  struct wmi_tlv_fw_stats_parse {
>  	const struct wmi_stats_event *ev;
> +	struct ath12k_fw_stats *stats;
>  };
>  
>  struct ath12k_wmi_dma_ring_caps_parse {
> @@ -7314,7 +7315,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  					      u16 len)
>  {
>  	const struct wmi_stats_event *ev = parse->ev;
> -	struct ath12k_fw_stats stats = {0};
> +	struct ath12k_fw_stats *stats = parse->stats;

make sure to add null check before using stats pointer

>  	struct ath12k *ar;
>  	struct ath12k_link_vif *arvif;
>  	struct ieee80211_sta *sta;
> @@ -7323,10 +7324,6 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  	int i, ret = 0;
>  	const void *data = ptr;
>  
> -	INIT_LIST_HEAD(&stats.vdevs);
> -	INIT_LIST_HEAD(&stats.bcn);
> -	INIT_LIST_HEAD(&stats.pdevs);
> -
>  	if (!ev) {
>  		ath12k_warn(ab, "failed to fetch update stats ev");
>  		return -EPROTO;
> @@ -7334,7 +7331,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  
>  	rcu_read_lock();
>  
> -	ar = ath12k_mac_get_ar_by_pdev_id(ab, le32_to_cpu(ev->pdev_id));
> +	stats->pdev_id = le32_to_cpu(ev->pdev_id);
> +	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats->pdev_id);
>  	if (!ar) {
>  		ath12k_warn(ab, "invalid pdev id %d in update stats event\n",
>  			    le32_to_cpu(ev->pdev_id));
> @@ -7377,8 +7375,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  		if (!dst)
>  			continue;
>  		ath12k_wmi_pull_vdev_stats(src, dst);
> -		stats.stats_id = WMI_REQUEST_VDEV_STAT;
> -		list_add_tail(&dst->list, &stats.vdevs);
> +		stats->stats_id = WMI_REQUEST_VDEV_STAT;
> +		list_add_tail(&dst->list, &stats->vdevs);
>  	}
>  	for (i = 0; i < le32_to_cpu(ev->num_bcn_stats); i++) {
>  		const struct ath12k_wmi_bcn_stats_params *src;
> @@ -7396,8 +7394,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  		if (!dst)
>  			continue;
>  		ath12k_wmi_pull_bcn_stats(src, dst);
> -		stats.stats_id = WMI_REQUEST_BCN_STAT;
> -		list_add_tail(&dst->list, &stats.bcn);
> +		stats->stats_id = WMI_REQUEST_BCN_STAT;
> +		list_add_tail(&dst->list, &stats->bcn);
>  	}
>  	for (i = 0; i < le32_to_cpu(ev->num_pdev_stats); i++) {
>  		const struct ath12k_wmi_pdev_stats_params *src;
> @@ -7409,7 +7407,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  			goto exit;
>  		}
>  
> -		stats.stats_id = WMI_REQUEST_PDEV_STAT;
> +		stats->stats_id = WMI_REQUEST_PDEV_STAT;
>  
>  		data += sizeof(*src);
>  		len -= sizeof(*src);
> @@ -7421,11 +7419,9 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>  		ath12k_wmi_pull_pdev_stats_base(&src->base, dst);
>  		ath12k_wmi_pull_pdev_stats_tx(&src->tx, dst);
>  		ath12k_wmi_pull_pdev_stats_rx(&src->rx, dst);
> -		list_add_tail(&dst->list, &stats.pdevs);
> +		list_add_tail(&dst->list, &stats->pdevs);
>  	}
>  
> -	complete(&ar->fw_stats_complete);
> -	ath12k_debugfs_fw_stats_process(ar, &stats);
>  exit:
>  	rcu_read_unlock();
>  	return ret;
> @@ -7451,16 +7447,74 @@ static int ath12k_wmi_tlv_fw_stats_parse(struct ath12k_base *ab,
>  	return ret;
>  }
>  
> +static int ath12k_wmi_pull_fw_stats(struct ath12k_base *ab, struct sk_buff *skb,
> +				    struct ath12k_fw_stats *stats)
> +{
> +	struct wmi_tlv_fw_stats_parse parse = {};
> +
> +	stats->stats_id = 0;
> +	parse.stats = stats;
> +
> +	return ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
> +				   ath12k_wmi_tlv_fw_stats_parse,
> +				   &parse);
> +}
> +
>  static void ath12k_update_stats_event(struct ath12k_base *ab, struct sk_buff *skb)
>  {
> +	struct ath12k_fw_stats stats = {};
> +	struct ath12k *ar;
>  	int ret;
> -	struct wmi_tlv_fw_stats_parse parse = {};
>  
> -	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
> -				  ath12k_wmi_tlv_fw_stats_parse,
> -				  &parse);
> -	if (ret)
> -		ath12k_warn(ab, "failed to parse fw stats %d\n", ret);
> +	INIT_LIST_HEAD(&stats.pdevs);
> +	INIT_LIST_HEAD(&stats.vdevs);
> +	INIT_LIST_HEAD(&stats.bcn);
> +
> +	ret = ath12k_wmi_pull_fw_stats(ab, skb, &stats);
> +	if (ret) {
> +		ath12k_warn(ab, "failed to pull fw stats: %d\n", ret);
> +		goto free;
> +	}
> +
> +	ath12k_dbg(ab, ATH12K_DBG_WMI, "event update stats");
> +
> +	rcu_read_lock();
> +	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
> +	if (!ar) {
> +		rcu_read_unlock();
> +		ath12k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
> +			    stats.pdev_id, ret);
> +		goto free;
> +	}
> +
> +	spin_lock_bh(&ar->data_lock);
> +
> +	/* WMI_REQUEST_PDEV_STAT can be requested via .get_txpower mac ops or via
> +	 * debugfs fw stats. Therefore, processing it separately.
> +	 */
> +	if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
> +		list_splice_tail_init(&stats.pdevs, &ar->fw_stats.pdevs);
> +		ar->fw_stats.fw_stats_done = true;
> +		goto complete;
> +	}
> +
> +	/* WMI_REQUEST_VDEV_STAT and WMI_REQUEST_BCN_STAT are currently requested only
> +	 * via debugfs fw stats. Hence, processing these in debugfs context.
> +	 */
> +	ath12k_debugfs_fw_stats_process(ar, &stats);
> +
> +complete:
> +	complete(&ar->fw_stats_complete);
> +	spin_unlock_bh(&ar->data_lock);
> +	rcu_read_unlock();
> +
> +	/* Since the stats's pdev, vdev and beacon list are spliced and reinitialised
> +	 * at this point, no need to free the individual list.
> +	 */
> +	return;
> +
> +free:
> +	ath12k_fw_stats_free(&stats);
>  }
>  
>  /* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the frequency scanned

Re: [PATCH 1/2] wifi: ath12k: move firmware stats out of debugfs

[Rameshkumar Sundaram]

On 1/30/2025 11:52 AM, Mahendran P wrote:
> On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
>> From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
>>
>> Currently, firmware stats, comprising pdev, vdev and beacon stats are
>> part of debugfs. In firmware pdev stats, firmware reports the final
>> Tx power used to transmit each packet. If driver wants to know the
>> final Tx power being used at firmware level, it can leverage from
>> firmware pdev stats.
>>
>> Move firmware stats out of debugfs context in order to leverage
>> the final Tx power reported in it even when debugfs is disabled.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
>> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.c    | 45 +++++++++++
>>   drivers/net/wireless/ath/ath12k/core.h    |  3 +
>>   drivers/net/wireless/ath/ath12k/debugfs.c | 44 +----------
>>   drivers/net/wireless/ath/ath12k/wmi.c     | 94 ++++++++++++++++++-----
>>   4 files changed, 124 insertions(+), 62 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
>> index 2dd0666959cd..122b407cd322 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.c
>> +++ b/drivers/net/wireless/ath/ath12k/core.c
>> @@ -1052,6 +1052,51 @@ bool ath12k_core_hw_group_start_ready(struct ath12k_hw_group *ag)
>>   	return (ag->num_started == ag->num_devices);
>>   }
>>   
>> +static void ath12k_fw_stats_pdevs_free(struct list_head *head)
>> +{
>> +	struct ath12k_fw_stats_pdev *i, *tmp;
>> +
>> +	list_for_each_entry_safe(i, tmp, head, list) {
>> +		list_del(&i->list);
>> +		kfree(i);
>> +	}
>> +}
>> +
>> +void ath12k_fw_stats_bcn_free(struct list_head *head)
>> +{
>> +	struct ath12k_fw_stats_bcn *i, *tmp;
>> +
>> +	list_for_each_entry_safe(i, tmp, head, list) {
>> +		list_del(&i->list);
>> +		kfree(i);
>> +	}
>> +}
>> +
>> +static void ath12k_fw_stats_vdevs_free(struct list_head *head)
>> +{
>> +	struct ath12k_fw_stats_vdev *i, *tmp;
>> +
>> +	list_for_each_entry_safe(i, tmp, head, list) {
>> +		list_del(&i->list);
>> +		kfree(i);
>> +	}
>> +}
>> +
>> +void ath12k_fw_stats_init(struct ath12k *ar)
>> +{
>> +	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
>> +	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
>> +	INIT_LIST_HEAD(&ar->fw_stats.bcn);
>> +	init_completion(&ar->fw_stats_complete);
>> +}
>> +
>> +void ath12k_fw_stats_free(struct ath12k_fw_stats *stats)
>> +{
>> +	ath12k_fw_stats_pdevs_free(&stats->pdevs);
>> +	ath12k_fw_stats_vdevs_free(&stats->vdevs);
>> +	ath12k_fw_stats_bcn_free(&stats->bcn);
>> +}
>> +
>>   static void ath12k_core_trigger_partner(struct ath12k_base *ab)
>>   {
>>   	struct ath12k_hw_group *ag = ab->ag;
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index 28db100cfac0..e4f51ad6a59f 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -1198,6 +1198,9 @@ u32 ath12k_core_get_max_peers_per_radio(struct ath12k_base *ab);
>>   u32 ath12k_core_get_max_num_tids(struct ath12k_base *ab);
>>   
>>   void ath12k_core_hw_group_set_mlo_capable(struct ath12k_hw_group *ag);
>> +void ath12k_fw_stats_init(struct ath12k *ar);
>> +void ath12k_fw_stats_bcn_free(struct list_head *head);
>> +void ath12k_fw_stats_free(struct ath12k_fw_stats *stats);
>>   
>>   static inline const char *ath12k_scan_state_str(enum ath12k_scan_state state)
>>   {
>> diff --git a/drivers/net/wireless/ath/ath12k/debugfs.c b/drivers/net/wireless/ath/ath12k/debugfs.c
>> index 6d6708486d14..4e4c2ef6a7ce 100644
>> --- a/drivers/net/wireless/ath/ath12k/debugfs.c
>> +++ b/drivers/net/wireless/ath/ath12k/debugfs.c
>> @@ -69,43 +69,11 @@ void ath12k_debugfs_soc_destroy(struct ath12k_base *ab)
>>   	 */
>>   }
>>   
>> -static void ath12k_fw_stats_pdevs_free(struct list_head *head)
>> -{
>> -	struct ath12k_fw_stats_pdev *i, *tmp;
>> -
>> -	list_for_each_entry_safe(i, tmp, head, list) {
>> -		list_del(&i->list);
>> -		kfree(i);
>> -	}
>> -}
>> -
>> -static void ath12k_fw_stats_bcn_free(struct list_head *head)
>> -{
>> -	struct ath12k_fw_stats_bcn *i, *tmp;
>> -
>> -	list_for_each_entry_safe(i, tmp, head, list) {
>> -		list_del(&i->list);
>> -		kfree(i);
>> -	}
>> -}
>> -
>> -static void ath12k_fw_stats_vdevs_free(struct list_head *head)
>> -{
>> -	struct ath12k_fw_stats_vdev *i, *tmp;
>> -
>> -	list_for_each_entry_safe(i, tmp, head, list) {
>> -		list_del(&i->list);
>> -		kfree(i);
>> -	}
>> -}
>> -
>>   void ath12k_debugfs_fw_stats_reset(struct ath12k *ar)
>>   {
>>   	spin_lock_bh(&ar->data_lock);
>>   	ar->fw_stats.fw_stats_done = false;
>> -	ath12k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
>> -	ath12k_fw_stats_bcn_free(&ar->fw_stats.bcn);
>> -	ath12k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
>> +	ath12k_fw_stats_free(&ar->fw_stats);
>>   	spin_unlock_bh(&ar->data_lock);
>>   }
>>   
>> @@ -221,10 +189,6 @@ ath12k_debugfs_fw_stats_process(struct ath12k *ar,
>>   			num_bcn = 0;
>>   		}
>>   	}
>> -	if (stats->stats_id == WMI_REQUEST_PDEV_STAT) {
>> -		list_splice_tail_init(&stats->pdevs, &ar->fw_stats.pdevs);
>> -		ar->fw_stats.fw_stats_done = true;
>> -	}
>>   }
>>   
>>   static int ath12k_open_vdev_stats(struct inode *inode, struct file *file)
>> @@ -438,11 +402,7 @@ void ath12k_debugfs_fw_stats_register(struct ath12k *ar)
>>   	debugfs_create_file("pdev_stats", 0600, fwstats_dir, ar,
>>   			    &fops_pdev_stats);
>>   
>> -	INIT_LIST_HEAD(&ar->fw_stats.vdevs);
>> -	INIT_LIST_HEAD(&ar->fw_stats.bcn);
>> -	INIT_LIST_HEAD(&ar->fw_stats.pdevs);
>> -
>> -	init_completion(&ar->fw_stats_complete);
>> +	ath12k_fw_stats_init(ar);
>>   }
>>   
>>   void ath12k_debugfs_register(struct ath12k *ar)
>> diff --git a/drivers/net/wireless/ath/ath12k/wmi.c b/drivers/net/wireless/ath/ath12k/wmi.c
>> index 61aa5f509338..1a7af09853a4 100644
>> --- a/drivers/net/wireless/ath/ath12k/wmi.c
>> +++ b/drivers/net/wireless/ath/ath12k/wmi.c
>> @@ -29,6 +29,7 @@ struct ath12k_wmi_svc_ready_parse {
>>   
>>   struct wmi_tlv_fw_stats_parse {
>>   	const struct wmi_stats_event *ev;
>> +	struct ath12k_fw_stats *stats;
>>   };
>>   
>>   struct ath12k_wmi_dma_ring_caps_parse {
>> @@ -7314,7 +7315,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   					      u16 len)
>>   {
>>   	const struct wmi_stats_event *ev = parse->ev;
>> -	struct ath12k_fw_stats stats = {0};
>> +	struct ath12k_fw_stats *stats = parse->stats;
> make sure to add null check before using stats pointer

There's only 1 caller for this function which passes a valid stats 
pointer so this should be valid.
Anyway will add it for any future callers.


>
>>   	struct ath12k *ar;
>>   	struct ath12k_link_vif *arvif;
>>   	struct ieee80211_sta *sta;
>> @@ -7323,10 +7324,6 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   	int i, ret = 0;
>>   	const void *data = ptr;
>>   
>> -	INIT_LIST_HEAD(&stats.vdevs);
>> -	INIT_LIST_HEAD(&stats.bcn);
>> -	INIT_LIST_HEAD(&stats.pdevs);
>> -
>>   	if (!ev) {
>>   		ath12k_warn(ab, "failed to fetch update stats ev");
>>   		return -EPROTO;
>> @@ -7334,7 +7331,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   
>>   	rcu_read_lock();
>>   
>> -	ar = ath12k_mac_get_ar_by_pdev_id(ab, le32_to_cpu(ev->pdev_id));
>> +	stats->pdev_id = le32_to_cpu(ev->pdev_id);
>> +	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats->pdev_id);
>>   	if (!ar) {
>>   		ath12k_warn(ab, "invalid pdev id %d in update stats event\n",
>>   			    le32_to_cpu(ev->pdev_id));
>> @@ -7377,8 +7375,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   		if (!dst)
>>   			continue;
>>   		ath12k_wmi_pull_vdev_stats(src, dst);
>> -		stats.stats_id = WMI_REQUEST_VDEV_STAT;
>> -		list_add_tail(&dst->list, &stats.vdevs);
>> +		stats->stats_id = WMI_REQUEST_VDEV_STAT;
>> +		list_add_tail(&dst->list, &stats->vdevs);
>>   	}
>>   	for (i = 0; i < le32_to_cpu(ev->num_bcn_stats); i++) {
>>   		const struct ath12k_wmi_bcn_stats_params *src;
>> @@ -7396,8 +7394,8 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   		if (!dst)
>>   			continue;
>>   		ath12k_wmi_pull_bcn_stats(src, dst);
>> -		stats.stats_id = WMI_REQUEST_BCN_STAT;
>> -		list_add_tail(&dst->list, &stats.bcn);
>> +		stats->stats_id = WMI_REQUEST_BCN_STAT;
>> +		list_add_tail(&dst->list, &stats->bcn);
>>   	}
>>   	for (i = 0; i < le32_to_cpu(ev->num_pdev_stats); i++) {
>>   		const struct ath12k_wmi_pdev_stats_params *src;
>> @@ -7409,7 +7407,7 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   			goto exit;
>>   		}
>>   
>> -		stats.stats_id = WMI_REQUEST_PDEV_STAT;
>> +		stats->stats_id = WMI_REQUEST_PDEV_STAT;
>>   
>>   		data += sizeof(*src);
>>   		len -= sizeof(*src);
>> @@ -7421,11 +7419,9 @@ static int ath12k_wmi_tlv_fw_stats_data_parse(struct ath12k_base *ab,
>>   		ath12k_wmi_pull_pdev_stats_base(&src->base, dst);
>>   		ath12k_wmi_pull_pdev_stats_tx(&src->tx, dst);
>>   		ath12k_wmi_pull_pdev_stats_rx(&src->rx, dst);
>> -		list_add_tail(&dst->list, &stats.pdevs);
>> +		list_add_tail(&dst->list, &stats->pdevs);
>>   	}
>>   
>> -	complete(&ar->fw_stats_complete);
>> -	ath12k_debugfs_fw_stats_process(ar, &stats);
>>   exit:
>>   	rcu_read_unlock();
>>   	return ret;
>> @@ -7451,16 +7447,74 @@ static int ath12k_wmi_tlv_fw_stats_parse(struct ath12k_base *ab,
>>   	return ret;
>>   }
>>   
>> +static int ath12k_wmi_pull_fw_stats(struct ath12k_base *ab, struct sk_buff *skb,
>> +				    struct ath12k_fw_stats *stats)
>> +{
>> +	struct wmi_tlv_fw_stats_parse parse = {};
>> +
>> +	stats->stats_id = 0;
>> +	parse.stats = stats;
>> +
>> +	return ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
>> +				   ath12k_wmi_tlv_fw_stats_parse,
>> +				   &parse);
>> +}
>> +
>>   static void ath12k_update_stats_event(struct ath12k_base *ab, struct sk_buff *skb)
>>   {
>> +	struct ath12k_fw_stats stats = {};
>> +	struct ath12k *ar;
>>   	int ret;
>> -	struct wmi_tlv_fw_stats_parse parse = {};
>>   
>> -	ret = ath12k_wmi_tlv_iter(ab, skb->data, skb->len,
>> -				  ath12k_wmi_tlv_fw_stats_parse,
>> -				  &parse);
>> -	if (ret)
>> -		ath12k_warn(ab, "failed to parse fw stats %d\n", ret);
>> +	INIT_LIST_HEAD(&stats.pdevs);
>> +	INIT_LIST_HEAD(&stats.vdevs);
>> +	INIT_LIST_HEAD(&stats.bcn);
>> +
>> +	ret = ath12k_wmi_pull_fw_stats(ab, skb, &stats);
>> +	if (ret) {
>> +		ath12k_warn(ab, "failed to pull fw stats: %d\n", ret);
>> +		goto free;
>> +	}
>> +
>> +	ath12k_dbg(ab, ATH12K_DBG_WMI, "event update stats");
>> +
>> +	rcu_read_lock();
>> +	ar = ath12k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
>> +	if (!ar) {
>> +		rcu_read_unlock();
>> +		ath12k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
>> +			    stats.pdev_id, ret);
>> +		goto free;
>> +	}
>> +
>> +	spin_lock_bh(&ar->data_lock);
>> +
>> +	/* WMI_REQUEST_PDEV_STAT can be requested via .get_txpower mac ops or via
>> +	 * debugfs fw stats. Therefore, processing it separately.
>> +	 */
>> +	if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
>> +		list_splice_tail_init(&stats.pdevs, &ar->fw_stats.pdevs);
>> +		ar->fw_stats.fw_stats_done = true;
>> +		goto complete;
>> +	}
>> +
>> +	/* WMI_REQUEST_VDEV_STAT and WMI_REQUEST_BCN_STAT are currently requested only
>> +	 * via debugfs fw stats. Hence, processing these in debugfs context.
>> +	 */
>> +	ath12k_debugfs_fw_stats_process(ar, &stats);
>> +
>> +complete:
>> +	complete(&ar->fw_stats_complete);
>> +	spin_unlock_bh(&ar->data_lock);
>> +	rcu_read_unlock();
>> +
>> +	/* Since the stats's pdev, vdev and beacon list are spliced and reinitialised
>> +	 * at this point, no need to free the individual list.
>> +	 */
>> +	return;
>> +
>> +free:
>> +	ath12k_fw_stats_free(&stats);
>>   }
>>   
>>   /* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the frequency scanned

[PATCH 2/2] wifi: ath12k: add get_txpower mac ops

[Rameshkumar Sundaram]

From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>

Driver does not support get_txpower mac ops because of which
cfg80211 returns vif->bss_conf.txpower to user space. bss_conf.txpower
gets its value from ieee80211_channel->max_reg_power. However, the final
txpower is dependent on few other parameters apart from max regulatory
supported power. It is the firmware which knows about all these
parameters and considers the minimum for each packet transmission.

All ath12k firmware reports the final TX power in firmware pdev stats
which falls under fw_stats. add get_txpower mac ops to get the TX power
from firmware leveraging fw_stats and return it accordingly.

While at it, there is a possibility that repeated stats request WMI
commands are queued to FW if mac80211/userspace does get tx power back
to back(in Multiple BSS cases). This could potentially consume the WMI
queue completely. Hence limit this by fetching the power only for every
5 seconds and reusing the value until the refresh timeout or when there
is a change in channel.

Also remove init_completion(&ar->fw_stats_complete) in
ath12k_mac_hw_register() as ath12k_fw_stats_init() takes care of
it for each ar.

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/core.h |   1 +
 drivers/net/wireless/ath/ath12k/mac.c  | 155 +++++++++++++++++++------
 drivers/net/wireless/ath/ath12k/mac.h  |   3 +
 3 files changed, 123 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
index e4f51ad6a59f..42da19870713 100644
--- a/drivers/net/wireless/ath/ath12k/core.h
+++ b/drivers/net/wireless/ath/ath12k/core.h
@@ -731,6 +731,7 @@ struct ath12k {
 	u32 mlo_setup_status;
 	u8 ftm_msgref;
 	struct ath12k_fw_stats fw_stats;
+	unsigned long last_tx_power_update;
 };
 
 struct ath12k_hw {
diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
index 4fb7e235be66..54fe3a2c9c0b 100644
--- a/drivers/net/wireless/ath/ath12k/mac.c
+++ b/drivers/net/wireless/ath/ath12k/mac.c
@@ -4280,6 +4280,120 @@ static int ath12k_start_scan(struct ath12k *ar,
 	return 0;
 }
 
+static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
+				   u32 vdev_id, u32 stats_id)
+{
+	struct ath12k_base *ab = ar->ab;
+	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
+	unsigned long time_left;
+	int ret;
+
+	guard(mutex)(&ah->hw_mutex);
+
+	if (ah->state != ATH12K_HW_STATE_ON)
+		return -ENETDOWN;
+
+	spin_lock_bh(&ar->data_lock);
+	ar->fw_stats.fw_stats_done = false;
+	ath12k_fw_stats_free(&ar->fw_stats);
+	spin_unlock_bh(&ar->data_lock);
+
+	reinit_completion(&ar->fw_stats_complete);
+
+	ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
+
+	if (ret) {
+		ath12k_warn(ab, "failed to request fw stats: stats id %u ret %d\n",
+			    stats_id, ret);
+		return ret;
+	}
+
+	ath12k_dbg(ab, ATH12K_DBG_WMI,
+		   "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
+		   pdev_id, vdev_id, stats_id);
+
+	time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
+
+	if (!time_left)
+		ath12k_warn(ab, "time out while waiting for get fw stats\n");
+
+	return ret;
+}
+
+static int ath12k_mac_op_get_txpower(struct ieee80211_hw *hw,
+				     struct ieee80211_vif *vif,
+				     unsigned int link_id,
+				     int *dbm)
+{
+	struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
+	struct ath12k_fw_stats_pdev *pdev;
+	struct ath12k_hw *ah = hw->priv;
+	struct ath12k_link_vif *arvif;
+	struct ath12k_base *ab;
+	struct ath12k *ar;
+	int ret;
+
+	/* Final Tx power is minimum of Target Power, CTL power, Regulatory
+	 * Power, PSD EIRP Power. We just know the Regulatory power from the
+	 * regulatory rules obtained. FW knows all these power and sets the min
+	 * of these. Hence, we request the FW pdev stats in which FW reports
+	 * the minimum of all vdev's channel Tx power.
+	 */
+	lockdep_assert_wiphy(hw->wiphy);
+
+	arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[link_id]);
+	if (!arvif || !arvif->ar)
+		return -EINVAL;
+
+	ar = arvif->ar;
+	ab = ar->ab;
+	if (ah->state != ATH12K_HW_STATE_ON)
+		goto err_fallback;
+
+	if (test_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags))
+		return -EAGAIN;
+
+	/* Limit the requests to Firmware for fetching the tx power */
+	if (ar->chan_tx_pwr != ATH12K_PDEV_TX_POWER_INVALID &&
+	    time_before(jiffies,
+			msecs_to_jiffies(ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS) +
+					 ar->last_tx_power_update))
+		goto send_tx_power;
+
+	ret = ath12k_mac_get_fw_stats(ar, ar->pdev->pdev_id, arvif->vdev_id,
+				      WMI_REQUEST_PDEV_STAT);
+	if (ret) {
+		ath12k_warn(ab, "failed to request fw pdev stats: %d\n", ret);
+		goto err_fallback;
+	}
+
+	spin_lock_bh(&ar->data_lock);
+	pdev = list_first_entry_or_null(&ar->fw_stats.pdevs,
+					struct ath12k_fw_stats_pdev, list);
+	if (!pdev) {
+		spin_unlock_bh(&ar->data_lock);
+		goto err_fallback;
+	}
+
+	ar->chan_tx_pwr = pdev->chan_tx_power;
+	spin_unlock_bh(&ar->data_lock);
+	ar->last_tx_power_update = jiffies;
+
+send_tx_power:
+	/* tx power reported by firmware is in units of 0.5 dBm */
+	*dbm = ar->chan_tx_pwr / 2;
+	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware %d, reported %d dBm\n",
+		   ar->chan_tx_pwr, *dbm);
+	return 0;
+
+err_fallback:
+	/* We didn't get txpower from FW. Hence, relying on vif->bss_conf.txpower */
+	*dbm = vif->bss_conf.txpower;
+	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware NaN, reported %d dBm\n",
+		   *dbm);
+	return 0;
+}
+
 static u8
 ath12k_mac_find_link_id_by_ar(struct ath12k_vif *ahvif, struct ath12k *ar)
 {
@@ -7433,6 +7547,7 @@ static int ath12k_mac_start(struct ath12k *ar)
 	ar->num_created_vdevs = 0;
 	ar->num_peers = 0;
 	ar->allocated_vdev_map = 0;
+	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
 
 	/* Configure monitor status ring with default rx_filter to get rx status
 	 * such as rssi, rx_duration.
@@ -8638,6 +8753,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
 	 */
 	ar->rx_channel = ctx->def.chan;
 	spin_unlock_bh(&ar->data_lock);
+	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
 
 	return 0;
 }
@@ -8666,6 +8782,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
 	 */
 	ar->rx_channel = NULL;
 	spin_unlock_bh(&ar->data_lock);
+	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
 }
 
 static enum wmi_phy_mode
@@ -10109,40 +10226,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
 	return 0;
 }
 
-static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
-				   u32 vdev_id, u32 stats_id)
-{
-	struct ath12k_base *ab = ar->ab;
-	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
-	unsigned long time_left;
-	int ret;
-
-	guard(mutex)(&ah->hw_mutex);
-
-	if (ah->state != ATH12K_HW_STATE_ON)
-		return -ENETDOWN;
-
-	reinit_completion(&ar->fw_stats_complete);
-
-	ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
-
-	if (ret) {
-		ath12k_warn(ab, "failed to request fw stats: %d\n", ret);
-		return ret;
-	}
-
-	ath12k_dbg(ab, ATH12K_DBG_WMI,
-		   "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
-		   pdev_id, vdev_id, stats_id);
-
-	time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
-
-	if (!time_left)
-		ath12k_warn(ab, "time out while waiting for get fw stats\n");
-
-	return ret;
-}
-
 static void ath12k_mac_op_sta_statistics(struct ieee80211_hw *hw,
 					 struct ieee80211_vif *vif,
 					 struct ieee80211_sta *sta,
@@ -10431,6 +10514,7 @@ static const struct ieee80211_ops ath12k_ops = {
 	.assign_vif_chanctx		= ath12k_mac_op_assign_vif_chanctx,
 	.unassign_vif_chanctx		= ath12k_mac_op_unassign_vif_chanctx,
 	.switch_vif_chanctx		= ath12k_mac_op_switch_vif_chanctx,
+	.get_txpower			= ath12k_mac_op_get_txpower,
 	.set_rts_threshold		= ath12k_mac_op_set_rts_threshold,
 	.set_frag_threshold		= ath12k_mac_op_set_frag_threshold,
 	.set_bitrate_mask		= ath12k_mac_op_set_bitrate_mask,
@@ -11178,11 +11262,10 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
 			goto err_unregister_hw;
 		}
 
+		ath12k_fw_stats_init(ar);
 		ath12k_debugfs_register(ar);
 	}
 
-	init_completion(&ar->fw_stats_complete);
-
 	return 0;
 
 err_unregister_hw:
diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
index 1acaf3f68292..af0d3c6a2a6c 100644
--- a/drivers/net/wireless/ath/ath12k/mac.h
+++ b/drivers/net/wireless/ath/ath12k/mac.h
@@ -33,6 +33,9 @@ struct ath12k_generic_iter {
 #define ATH12K_KEEPALIVE_MAX_IDLE		3895
 #define ATH12K_KEEPALIVE_MAX_UNRESPONSIVE	3900
 
+#define ATH12K_PDEV_TX_POWER_INVALID		(-1)
+#define ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS	5000 /* msecs */
+
 /* FIXME: should these be in ieee80211.h? */
 #define IEEE80211_VHT_MCS_SUPPORT_0_11_MASK	GENMASK(23, 16)
 #define IEEE80211_DISABLE_VHT_MCS_SUPPORT_0_11	BIT(24)
-- 
2.34.1

Re: [PATCH 2/2] wifi: ath12k: add get_txpower mac ops

[Mahendran P]

On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
> From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
> 
> Driver does not support get_txpower mac ops because of which
> cfg80211 returns vif->bss_conf.txpower to user space. bss_conf.txpower
> gets its value from ieee80211_channel->max_reg_power. However, the final
> txpower is dependent on few other parameters apart from max regulatory
> supported power. It is the firmware which knows about all these
> parameters and considers the minimum for each packet transmission.
> 
> All ath12k firmware reports the final TX power in firmware pdev stats
> which falls under fw_stats. add get_txpower mac ops to get the TX power
> from firmware leveraging fw_stats and return it accordingly.
> 
> While at it, there is a possibility that repeated stats request WMI
> commands are queued to FW if mac80211/userspace does get tx power back
> to back(in Multiple BSS cases). This could potentially consume the WMI
> queue completely. Hence limit this by fetching the power only for every
> 5 seconds and reusing the value until the refresh timeout or when there
> is a change in channel.
> 
> Also remove init_completion(&ar->fw_stats_complete) in
> ath12k_mac_hw_register() as ath12k_fw_stats_init() takes care of
> it for each ar.
> 
> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
> ---
>  drivers/net/wireless/ath/ath12k/core.h |   1 +
>  drivers/net/wireless/ath/ath12k/mac.c  | 155 +++++++++++++++++++------
>  drivers/net/wireless/ath/ath12k/mac.h  |   3 +
>  3 files changed, 123 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
> index e4f51ad6a59f..42da19870713 100644
> --- a/drivers/net/wireless/ath/ath12k/core.h
> +++ b/drivers/net/wireless/ath/ath12k/core.h
> @@ -731,6 +731,7 @@ struct ath12k {
>  	u32 mlo_setup_status;
>  	u8 ftm_msgref;
>  	struct ath12k_fw_stats fw_stats;
> +	unsigned long last_tx_power_update;
>  };
>  
>  struct ath12k_hw {
> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
> index 4fb7e235be66..54fe3a2c9c0b 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.c
> +++ b/drivers/net/wireless/ath/ath12k/mac.c
> @@ -4280,6 +4280,120 @@ static int ath12k_start_scan(struct ath12k *ar,
>  	return 0;
>  }
>  
> +static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
> +				   u32 vdev_id, u32 stats_id)
> +{
> +	struct ath12k_base *ab = ar->ab;
> +	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
> +	unsigned long time_left;
> +	int ret;
> +
> +	guard(mutex)(&ah->hw_mutex);
> +
> +	if (ah->state != ATH12K_HW_STATE_ON)
> +		return -ENETDOWN;
> +
> +	spin_lock_bh(&ar->data_lock);
> +	ar->fw_stats.fw_stats_done = false;
> +	ath12k_fw_stats_free(&ar->fw_stats);
> +	spin_unlock_bh(&ar->data_lock);

rename ath12k_debugfs_fw_stats_reset and reuse instead of the above 4 lines

> +	reinit_completion(&ar->fw_stats_complete);
> +
> +	ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
> +
> +	if (ret) {
> +		ath12k_warn(ab, "failed to request fw stats: stats id %u ret %d\n",
> +			    stats_id, ret);
> +		return ret;
> +	}
> +
> +	ath12k_dbg(ab, ATH12K_DBG_WMI,
> +		   "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
> +		   pdev_id, vdev_id, stats_id);
> +
> +	time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
> +
> +	if (!time_left)
> +		ath12k_warn(ab, "time out while waiting for get fw stats\n");
> +

suggestion is to create a separate function and move some of the common code in ath12k_mac_get_fw_stats and ath12k_debugfs_fw_stats_request

> +	return ret;
> +}
> +
> +static int ath12k_mac_op_get_txpower(struct ieee80211_hw *hw,
> +				     struct ieee80211_vif *vif,
> +				     unsigned int link_id,
> +				     int *dbm)
> +{
> +	struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
> +	struct ath12k_fw_stats_pdev *pdev;
> +	struct ath12k_hw *ah = hw->priv;
> +	struct ath12k_link_vif *arvif;
> +	struct ath12k_base *ab;
> +	struct ath12k *ar;
> +	int ret;
> +
> +	/* Final Tx power is minimum of Target Power, CTL power, Regulatory
> +	 * Power, PSD EIRP Power. We just know the Regulatory power from the
> +	 * regulatory rules obtained. FW knows all these power and sets the min
> +	 * of these. Hence, we request the FW pdev stats in which FW reports
> +	 * the minimum of all vdev's channel Tx power.
> +	 */
> +	lockdep_assert_wiphy(hw->wiphy);
> +
> +	arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[link_id]);
> +	if (!arvif || !arvif->ar)
> +		return -EINVAL;
> +
> +	ar = arvif->ar;
> +	ab = ar->ab;
> +	if (ah->state != ATH12K_HW_STATE_ON)
> +		goto err_fallback;
> +
> +	if (test_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags))
> +		return -EAGAIN;
> +
> +	/* Limit the requests to Firmware for fetching the tx power */
> +	if (ar->chan_tx_pwr != ATH12K_PDEV_TX_POWER_INVALID &&
> +	    time_before(jiffies,
> +			msecs_to_jiffies(ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS) +
> +					 ar->last_tx_power_update))
> +		goto send_tx_power;
> +
> +	ret = ath12k_mac_get_fw_stats(ar, ar->pdev->pdev_id, arvif->vdev_id,
> +				      WMI_REQUEST_PDEV_STAT);
> +	if (ret) {
> +		ath12k_warn(ab, "failed to request fw pdev stats: %d\n", ret);
> +		goto err_fallback;
> +	}
> +
> +	spin_lock_bh(&ar->data_lock);
> +	pdev = list_first_entry_or_null(&ar->fw_stats.pdevs,
> +					struct ath12k_fw_stats_pdev, list);
> +	if (!pdev) {
> +		spin_unlock_bh(&ar->data_lock);
> +		goto err_fallback;
> +	}
> +
> +	ar->chan_tx_pwr = pdev->chan_tx_power;

It is better to divide and store

> +	spin_unlock_bh(&ar->data_lock);
> +	ar->last_tx_power_update = jiffies;
> +
> +send_tx_power:
> +	/* tx power reported by firmware is in units of 0.5 dBm */
> +	*dbm = ar->chan_tx_pwr / 2;

based on the above comment, we dont need to do divide everytime here during repeated calls

> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware %d, reported %d dBm\n",
> +		   ar->chan_tx_pwr, *dbm);
> +	return 0;
> +
> +err_fallback:
> +	/* We didn't get txpower from FW. Hence, relying on vif->bss_conf.txpower */
> +	*dbm = vif->bss_conf.txpower;
> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware NaN, reported %d dBm\n",
> +		   *dbm);
> +	return 0;
> +}
> +
>  static u8
>  ath12k_mac_find_link_id_by_ar(struct ath12k_vif *ahvif, struct ath12k *ar)
>  {
> @@ -7433,6 +7547,7 @@ static int ath12k_mac_start(struct ath12k *ar)
>  	ar->num_created_vdevs = 0;
>  	ar->num_peers = 0;
>  	ar->allocated_vdev_map = 0;
> +	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;

ar->chan_tx_pwr type u32..and assigning signed value. fix it.

>  
>  	/* Configure monitor status ring with default rx_filter to get rx status
>  	 * such as rssi, rx_duration.
> @@ -8638,6 +8753,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
>  	 */
>  	ar->rx_channel = ctx->def.chan;
>  	spin_unlock_bh(&ar->data_lock);
> +	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
>  
>  	return 0;
>  }
> @@ -8666,6 +8782,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
>  	 */
>  	ar->rx_channel = NULL;
>  	spin_unlock_bh(&ar->data_lock);
> +	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
>  }
>  
>  static enum wmi_phy_mode
> @@ -10109,40 +10226,6 @@ static int ath12k_mac_op_get_survey(struct ieee80211_hw *hw, int idx,
>  	return 0;
>  }
>  
> -static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
> -				   u32 vdev_id, u32 stats_id)
> -{
> -	struct ath12k_base *ab = ar->ab;
> -	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
> -	unsigned long time_left;
> -	int ret;
> -
> -	guard(mutex)(&ah->hw_mutex);
> -
> -	if (ah->state != ATH12K_HW_STATE_ON)
> -		return -ENETDOWN;
> -
> -	reinit_completion(&ar->fw_stats_complete);
> -
> -	ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
> -
> -	if (ret) {
> -		ath12k_warn(ab, "failed to request fw stats: %d\n", ret);
> -		return ret;
> -	}
> -
> -	ath12k_dbg(ab, ATH12K_DBG_WMI,
> -		   "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
> -		   pdev_id, vdev_id, stats_id);
> -
> -	time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
> -
> -	if (!time_left)
> -		ath12k_warn(ab, "time out while waiting for get fw stats\n");
> -
> -	return ret;
> -}
> -
>  static void ath12k_mac_op_sta_statistics(struct ieee80211_hw *hw,
>  					 struct ieee80211_vif *vif,
>  					 struct ieee80211_sta *sta,
> @@ -10431,6 +10514,7 @@ static const struct ieee80211_ops ath12k_ops = {
>  	.assign_vif_chanctx		= ath12k_mac_op_assign_vif_chanctx,
>  	.unassign_vif_chanctx		= ath12k_mac_op_unassign_vif_chanctx,
>  	.switch_vif_chanctx		= ath12k_mac_op_switch_vif_chanctx,
> +	.get_txpower			= ath12k_mac_op_get_txpower,
>  	.set_rts_threshold		= ath12k_mac_op_set_rts_threshold,
>  	.set_frag_threshold		= ath12k_mac_op_set_frag_threshold,
>  	.set_bitrate_mask		= ath12k_mac_op_set_bitrate_mask,
> @@ -11178,11 +11262,10 @@ static int ath12k_mac_hw_register(struct ath12k_hw *ah)
>  			goto err_unregister_hw;
>  		}
>  
> +		ath12k_fw_stats_init(ar);
>  		ath12k_debugfs_register(ar);
>  	}
>  
> -	init_completion(&ar->fw_stats_complete);
> -
>  	return 0;
>  
>  err_unregister_hw:
> diff --git a/drivers/net/wireless/ath/ath12k/mac.h b/drivers/net/wireless/ath/ath12k/mac.h
> index 1acaf3f68292..af0d3c6a2a6c 100644
> --- a/drivers/net/wireless/ath/ath12k/mac.h
> +++ b/drivers/net/wireless/ath/ath12k/mac.h
> @@ -33,6 +33,9 @@ struct ath12k_generic_iter {
>  #define ATH12K_KEEPALIVE_MAX_IDLE		3895
>  #define ATH12K_KEEPALIVE_MAX_UNRESPONSIVE	3900
>  
> +#define ATH12K_PDEV_TX_POWER_INVALID		(-1)
> +#define ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS	5000 /* msecs */
> +
>  /* FIXME: should these be in ieee80211.h? */
>  #define IEEE80211_VHT_MCS_SUPPORT_0_11_MASK	GENMASK(23, 16)
>  #define IEEE80211_DISABLE_VHT_MCS_SUPPORT_0_11	BIT(24)

Re: [PATCH 2/2] wifi: ath12k: add get_txpower mac ops

[Rameshkumar Sundaram]

On 1/30/2025 11:55 AM, Mahendran P wrote:
> On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
>> From: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
>>
>> Driver does not support get_txpower mac ops because of which
>> cfg80211 returns vif->bss_conf.txpower to user space. bss_conf.txpower
>> gets its value from ieee80211_channel->max_reg_power. However, the final
>> txpower is dependent on few other parameters apart from max regulatory
>> supported power. It is the firmware which knows about all these
>> parameters and considers the minimum for each packet transmission.
>>
>> All ath12k firmware reports the final TX power in firmware pdev stats
>> which falls under fw_stats. add get_txpower mac ops to get the TX power
>> from firmware leveraging fw_stats and return it accordingly.
>>
>> While at it, there is a possibility that repeated stats request WMI
>> commands are queued to FW if mac80211/userspace does get tx power back
>> to back(in Multiple BSS cases). This could potentially consume the WMI
>> queue completely. Hence limit this by fetching the power only for every
>> 5 seconds and reusing the value until the refresh timeout or when there
>> is a change in channel.
>>
>> Also remove init_completion(&ar->fw_stats_complete) in
>> ath12k_mac_hw_register() as ath12k_fw_stats_init() takes care of
>> it for each ar.
>>
>> Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.3.1-00173-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: Aditya Kumar Singh <aditya.kumar.singh@oss.qualcomm.com>
>> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@oss.qualcomm.com>
>> ---
>>   drivers/net/wireless/ath/ath12k/core.h |   1 +
>>   drivers/net/wireless/ath/ath12k/mac.c  | 155 +++++++++++++++++++------
>>   drivers/net/wireless/ath/ath12k/mac.h  |   3 +
>>   3 files changed, 123 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath12k/core.h b/drivers/net/wireless/ath/ath12k/core.h
>> index e4f51ad6a59f..42da19870713 100644
>> --- a/drivers/net/wireless/ath/ath12k/core.h
>> +++ b/drivers/net/wireless/ath/ath12k/core.h
>> @@ -731,6 +731,7 @@ struct ath12k {
>>   	u32 mlo_setup_status;
>>   	u8 ftm_msgref;
>>   	struct ath12k_fw_stats fw_stats;
>> +	unsigned long last_tx_power_update;
>>   };
>>   
>>   struct ath12k_hw {
>> diff --git a/drivers/net/wireless/ath/ath12k/mac.c b/drivers/net/wireless/ath/ath12k/mac.c
>> index 4fb7e235be66..54fe3a2c9c0b 100644
>> --- a/drivers/net/wireless/ath/ath12k/mac.c
>> +++ b/drivers/net/wireless/ath/ath12k/mac.c
>> @@ -4280,6 +4280,120 @@ static int ath12k_start_scan(struct ath12k *ar,
>>   	return 0;
>>   }
>>   
>> +static int ath12k_mac_get_fw_stats(struct ath12k *ar, u32 pdev_id,
>> +				   u32 vdev_id, u32 stats_id)
>> +{
>> +	struct ath12k_base *ab = ar->ab;
>> +	struct ath12k_hw *ah = ath12k_ar_to_ah(ar);
>> +	unsigned long time_left;
>> +	int ret;
>> +
>> +	guard(mutex)(&ah->hw_mutex);
>> +
>> +	if (ah->state != ATH12K_HW_STATE_ON)
>> +		return -ENETDOWN;
>> +
>> +	spin_lock_bh(&ar->data_lock);
>> +	ar->fw_stats.fw_stats_done = false;
>> +	ath12k_fw_stats_free(&ar->fw_stats);
>> +	spin_unlock_bh(&ar->data_lock);
> rename ath12k_debugfs_fw_stats_reset and reuse instead of the above 4 lines

Sure, will do it in next version

>
>> +	reinit_completion(&ar->fw_stats_complete);
>> +
>> +	ret = ath12k_wmi_send_stats_request_cmd(ar, stats_id, vdev_id, pdev_id);
>> +
>> +	if (ret) {
>> +		ath12k_warn(ab, "failed to request fw stats: stats id %u ret %d\n",
>> +			    stats_id, ret);
>> +		return ret;
>> +	}
>> +
>> +	ath12k_dbg(ab, ATH12K_DBG_WMI,
>> +		   "get fw stat pdev id %d vdev id %d stats id 0x%x\n",
>> +		   pdev_id, vdev_id, stats_id);
>> +
>> +	time_left = wait_for_completion_timeout(&ar->fw_stats_complete, 1 * HZ);
>> +
>> +	if (!time_left)
>> +		ath12k_warn(ab, "time out while waiting for get fw stats\n");
>> +
> suggestion is to create a separate function and move some of the common code in ath12k_mac_get_fw_stats and ath12k_debugfs_fw_stats_request

Will move the wait logic from ath12k_debugfs_fw_stats_request() to 
ath12k_mac_get_fw_stats() and use the same on both places.

>
>> +	return ret;
>> +}
>> +
>> +static int ath12k_mac_op_get_txpower(struct ieee80211_hw *hw,
>> +				     struct ieee80211_vif *vif,
>> +				     unsigned int link_id,
>> +				     int *dbm)
>> +{
>> +	struct ath12k_vif *ahvif = ath12k_vif_to_ahvif(vif);
>> +	struct ath12k_fw_stats_pdev *pdev;
>> +	struct ath12k_hw *ah = hw->priv;
>> +	struct ath12k_link_vif *arvif;
>> +	struct ath12k_base *ab;
>> +	struct ath12k *ar;
>> +	int ret;
>> +
>> +	/* Final Tx power is minimum of Target Power, CTL power, Regulatory
>> +	 * Power, PSD EIRP Power. We just know the Regulatory power from the
>> +	 * regulatory rules obtained. FW knows all these power and sets the min
>> +	 * of these. Hence, we request the FW pdev stats in which FW reports
>> +	 * the minimum of all vdev's channel Tx power.
>> +	 */
>> +	lockdep_assert_wiphy(hw->wiphy);
>> +
>> +	arvif = wiphy_dereference(ah->hw->wiphy, ahvif->link[link_id]);
>> +	if (!arvif || !arvif->ar)
>> +		return -EINVAL;
>> +
>> +	ar = arvif->ar;
>> +	ab = ar->ab;
>> +	if (ah->state != ATH12K_HW_STATE_ON)
>> +		goto err_fallback;
>> +
>> +	if (test_bit(ATH12K_FLAG_CAC_RUNNING, &ar->dev_flags))
>> +		return -EAGAIN;
>> +
>> +	/* Limit the requests to Firmware for fetching the tx power */
>> +	if (ar->chan_tx_pwr != ATH12K_PDEV_TX_POWER_INVALID &&
>> +	    time_before(jiffies,
>> +			msecs_to_jiffies(ATH12K_PDEV_TX_POWER_REFRESH_TIME_MSECS) +
>> +					 ar->last_tx_power_update))
>> +		goto send_tx_power;
>> +
>> +	ret = ath12k_mac_get_fw_stats(ar, ar->pdev->pdev_id, arvif->vdev_id,
>> +				      WMI_REQUEST_PDEV_STAT);
>> +	if (ret) {
>> +		ath12k_warn(ab, "failed to request fw pdev stats: %d\n", ret);
>> +		goto err_fallback;
>> +	}
>> +
>> +	spin_lock_bh(&ar->data_lock);
>> +	pdev = list_first_entry_or_null(&ar->fw_stats.pdevs,
>> +					struct ath12k_fw_stats_pdev, list);
>> +	if (!pdev) {
>> +		spin_unlock_bh(&ar->data_lock);
>> +		goto err_fallback;
>> +	}
>> +
>> +	ar->chan_tx_pwr = pdev->chan_tx_power;
> It is better to divide and store
>
>> +	spin_unlock_bh(&ar->data_lock);
>> +	ar->last_tx_power_update = jiffies;
>> +
>> +send_tx_power:
>> +	/* tx power reported by firmware is in units of 0.5 dBm */
>> +	*dbm = ar->chan_tx_pwr / 2;
> based on the above comment, we dont need to do divide everytime here during repeated calls

Thanks for the suggestion, will do it in next version

>
>> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware %d, reported %d dBm\n",
>> +		   ar->chan_tx_pwr, *dbm);
>> +	return 0;
>> +
>> +err_fallback:
>> +	/* We didn't get txpower from FW. Hence, relying on vif->bss_conf.txpower */
>> +	*dbm = vif->bss_conf.txpower;
>> +	ath12k_dbg(ar->ab, ATH12K_DBG_MAC, "txpower from firmware NaN, reported %d dBm\n",
>> +		   *dbm);
>> +	return 0;
>> +}
>> +
>>   static u8
>>   ath12k_mac_find_link_id_by_ar(struct ath12k_vif *ahvif, struct ath12k *ar)
>>   {
>> @@ -7433,6 +7547,7 @@ static int ath12k_mac_start(struct ath12k *ar)
>>   	ar->num_created_vdevs = 0;
>>   	ar->num_peers = 0;
>>   	ar->allocated_vdev_map = 0;
>> +	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
> ar->chan_tx_pwr type u32..and assigning signed value. fix it.

Thanks for pointing out, will fix it by assigning u32 max for 
ATH12K_PDEV_TX_POWER_INVALID

>
>>   
>>   	/* Configure monitor status ring with default rx_filter to get rx status
>>   	 * such as rssi, rx_duration.
>> @@ -8638,6 +8753,7 @@ static int ath12k_mac_op_add_chanctx(struct ieee80211_hw *hw,
>>   	 */
>>   	ar->rx_channel = ctx->def.chan;
>>   	spin_unlock_bh(&ar->data_lock);
>> +	ar->chan_tx_pwr = ATH12K_PDEV_TX_POWER_INVALID;
>>   
>>   	return 0;
>>   }
>> @@ -8666,6 +8782,7 @@ static void ath12k_mac_op_remove_chanctx(struct ieee80211_hw *hw,
>>