[PATCH 1/2] wifi: ath12k: move firmware stats out of debugfs
Rameshkumar Sundaram
rameshkumar.sundaram at oss.qualcomm.com
Fri Jan 31 10:50:02 PST 2025
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 at 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 at oss.qualcomm.com>
>> Signed-off-by: Rameshkumar Sundaram <rameshkumar.sundaram at 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
More information about the ath12k
mailing list