On 1/27/2025 10:52 PM, Rameshkumar Sundaram wrote:
> From: Aditya Kumar Singh <aditya.kumar.singh@xxxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram@xxxxxxxxxxxxxxxx>
> ---
> 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
