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: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 46 ++++++++
drivers/net/wireless/ath/ath11k/core.h | 12 +-
drivers/net/wireless/ath/ath11k/debugfs.c | 131 +++++-----------------
drivers/net/wireless/ath/ath11k/debugfs.h | 6 +-
drivers/net/wireless/ath/ath11k/wmi.c | 48 +++++++-
5 files changed, 136 insertions(+), 107 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index ca6fadd64b43..07a299a2e213 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -570,6 +570,52 @@ static inline struct ath11k_pdev *ath11k_core_get_single_pdev(struct ath11k_base
return &ab->pdevs[0];
}
+void ath11k_fw_stats_pdevs_free(struct list_head *head)
+{
+ struct ath11k_fw_stats_pdev *i, *tmp;
+
+ list_for_each_entry_safe(i, tmp, head, list) {
+ list_del(&i->list);
+ kfree(i);
+ }
+}
+
+void ath11k_fw_stats_vdevs_free(struct list_head *head)
+{
+ struct ath11k_fw_stats_vdev *i, *tmp;
+
+ list_for_each_entry_safe(i, tmp, head, list) {
+ list_del(&i->list);
+ kfree(i);
+ }
+}
+
+void ath11k_fw_stats_bcn_free(struct list_head *head)
+{
+ struct ath11k_fw_stats_bcn *i, *tmp;
+
+ list_for_each_entry_safe(i, tmp, head, list) {
+ list_del(&i->list);
+ kfree(i);
+ }
+}
+
+void ath11k_fw_stats_init(struct ath11k *ar)
+{
+ INIT_LIST_HEAD(&ar->fw_stats.pdevs);
+ INIT_LIST_HEAD(&ar->fw_stats.vdevs);
+ INIT_LIST_HEAD(&ar->fw_stats.bcn);
+
+ init_completion(&ar->fw_stats_complete);
+}
+
+void ath11k_fw_stats_free(struct ath11k_fw_stats *stats)
+{
+ ath11k_fw_stats_pdevs_free(&stats->pdevs);
+ ath11k_fw_stats_vdevs_free(&stats->vdevs);
+ ath11k_fw_stats_bcn_free(&stats->bcn);
+}
+
int ath11k_core_suspend(struct ath11k_base *ab)
{
int ret;
diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 65d54e9c15d9..672701f40a6b 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -545,9 +545,6 @@ struct ath11k_debug {
struct dentry *debugfs_pdev;
struct ath11k_dbg_htt_stats htt_stats;
u32 extd_tx_stats;
- struct ath11k_fw_stats fw_stats;
- struct completion fw_stats_complete;
- bool fw_stats_done;
u32 extd_rx_stats;
u32 pktlog_filter;
u32 pktlog_mode;
@@ -712,6 +709,9 @@ struct ath11k {
u8 twt_enabled;
bool nlo_enabled;
u8 alpha2[REG_ALPHA2_LEN + 1];
+ struct ath11k_fw_stats fw_stats;
+ struct completion fw_stats_complete;
+ bool fw_stats_done;
};
struct ath11k_band_cap {
@@ -1117,6 +1117,12 @@ struct ath11k_fw_stats_bcn {
u32 tx_bcn_outage_cnt;
};
+void ath11k_fw_stats_init(struct ath11k *ar);
+void ath11k_fw_stats_pdevs_free(struct list_head *head);
+void ath11k_fw_stats_vdevs_free(struct list_head *head);
+void ath11k_fw_stats_bcn_free(struct list_head *head);
+void ath11k_fw_stats_free(struct ath11k_fw_stats *stats);
+
extern const struct ce_pipe_config ath11k_target_ce_config_wlan_ipq8074[];
extern const struct service_to_pipe ath11k_target_service_to_ce_map_wlan_ipq8074[];
extern const struct service_to_pipe ath11k_target_service_to_ce_map_wlan_ipq6018[];
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
index 0d4843ef9dd1..e4a2fd3da481 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.c
+++ b/drivers/net/wireless/ath/ath11k/debugfs.c
@@ -93,91 +93,36 @@ void ath11k_debugfs_add_dbring_entry(struct ath11k *ar,
spin_unlock_bh(&dbr_data->lock);
}
-static void ath11k_fw_stats_pdevs_free(struct list_head *head)
-{
- struct ath11k_fw_stats_pdev *i, *tmp;
-
- list_for_each_entry_safe(i, tmp, head, list) {
- list_del(&i->list);
- kfree(i);
- }
-}
-
-static void ath11k_fw_stats_vdevs_free(struct list_head *head)
-{
- struct ath11k_fw_stats_vdev *i, *tmp;
-
- list_for_each_entry_safe(i, tmp, head, list) {
- list_del(&i->list);
- kfree(i);
- }
-}
-
-static void ath11k_fw_stats_bcn_free(struct list_head *head)
-{
- struct ath11k_fw_stats_bcn *i, *tmp;
-
- list_for_each_entry_safe(i, tmp, head, list) {
- list_del(&i->list);
- kfree(i);
- }
-}
static void ath11k_debugfs_fw_stats_reset(struct ath11k *ar)
{
spin_lock_bh(&ar->data_lock);
- ar->debug.fw_stats_done = false;
- ath11k_fw_stats_pdevs_free(&ar->debug.fw_stats.pdevs);
- ath11k_fw_stats_vdevs_free(&ar->debug.fw_stats.vdevs);
+ ar->fw_stats_done = false;
+ ath11k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
+ ath11k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
spin_unlock_bh(&ar->data_lock);
}
-void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb)
+void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct ath11k_fw_stats *stats)
{
- struct ath11k_fw_stats stats = {};
- struct ath11k *ar;
+ struct ath11k_base *ab = ar->ab;
struct ath11k_pdev *pdev;
bool is_end;
static unsigned int num_vdev, num_bcn;
size_t total_vdevs_started = 0;
- int i, ret;
-
- INIT_LIST_HEAD(&stats.pdevs);
- INIT_LIST_HEAD(&stats.vdevs);
- INIT_LIST_HEAD(&stats.bcn);
-
- ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
- if (ret) {
- ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
- goto free;
- }
-
- rcu_read_lock();
- ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
- if (!ar) {
- rcu_read_unlock();
- ath11k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
- stats.pdev_id, ret);
- goto free;
- }
-
- spin_lock_bh(&ar->data_lock);
+ int i;
- if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
- list_splice_tail_init(&stats.pdevs, &ar->debug.fw_stats.pdevs);
- ar->debug.fw_stats_done = true;
- goto complete;
- }
+ /* WMI_REQUEST_PDEV_STAT request has been already processed */
- if (stats.stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
- ar->debug.fw_stats_done = true;
- goto complete;
+ if (stats->stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
+ ar->fw_stats_done = true;
+ return;
}
- if (stats.stats_id == WMI_REQUEST_VDEV_STAT) {
- if (list_empty(&stats.vdevs)) {
+ if (stats->stats_id == WMI_REQUEST_VDEV_STAT) {
+ if (list_empty(&stats->vdevs)) {
ath11k_warn(ab, "empty vdev stats");
- goto complete;
+ return;
}
/* FW sends all the active VDEV stats irrespective of PDEV,
* hence limit until the count of all VDEVs started
@@ -190,43 +135,34 @@ void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb
is_end = ((++num_vdev) == total_vdevs_started);
- list_splice_tail_init(&stats.vdevs,
- &ar->debug.fw_stats.vdevs);
+ list_splice_tail_init(&stats->vdevs,
+ &ar->fw_stats.vdevs);
if (is_end) {
- ar->debug.fw_stats_done = true;
+ ar->fw_stats_done = true;
num_vdev = 0;
}
- goto complete;
+ return;
}
- if (stats.stats_id == WMI_REQUEST_BCN_STAT) {
- if (list_empty(&stats.bcn)) {
+ if (stats->stats_id == WMI_REQUEST_BCN_STAT) {
+ if (list_empty(&stats->bcn)) {
ath11k_warn(ab, "empty bcn stats");
- goto complete;
+ return;
}
/* Mark end until we reached the count of all started VDEVs
* within the PDEV
*/
is_end = ((++num_bcn) == ar->num_started_vdevs);
- list_splice_tail_init(&stats.bcn,
- &ar->debug.fw_stats.bcn);
+ list_splice_tail_init(&stats->bcn,
+ &ar->fw_stats.bcn);
if (is_end) {
- ar->debug.fw_stats_done = true;
+ ar->fw_stats_done = true;
num_bcn = 0;
}
}
-complete:
- complete(&ar->debug.fw_stats_complete);
- rcu_read_unlock();
- spin_unlock_bh(&ar->data_lock);
-
-free:
- ath11k_fw_stats_pdevs_free(&stats.pdevs);
- ath11k_fw_stats_vdevs_free(&stats.vdevs);
- ath11k_fw_stats_bcn_free(&stats.bcn);
}
static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
@@ -247,7 +183,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
ath11k_debugfs_fw_stats_reset(ar);
- reinit_completion(&ar->debug.fw_stats_complete);
+ reinit_completion(&ar->fw_stats_complete);
ret = ath11k_wmi_send_stats_request_cmd(ar, req_param);
@@ -258,7 +194,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
}
time_left =
- wait_for_completion_timeout(&ar->debug.fw_stats_complete,
+ wait_for_completion_timeout(&ar->fw_stats_complete,
1 * HZ);
if (!time_left)
return -ETIMEDOUT;
@@ -268,7 +204,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
break;
spin_lock_bh(&ar->data_lock);
- if (ar->debug.fw_stats_done) {
+ if (ar->fw_stats_done) {
spin_unlock_bh(&ar->data_lock);
break;
}
@@ -340,7 +276,7 @@ static int ath11k_open_pdev_stats(struct inode *inode, struct file *file)
goto err_free;
}
- ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
+ ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
buf);
file->private_data = buf;
@@ -412,7 +348,7 @@ static int ath11k_open_vdev_stats(struct inode *inode, struct file *file)
goto err_free;
}
- ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
+ ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
buf);
file->private_data = buf;
@@ -490,14 +426,14 @@ static int ath11k_open_bcn_stats(struct inode *inode, struct file *file)
}
}
- ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
+ ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
buf);
/* since beacon stats request is looped for all active VDEVs, saved fw
* stats is not freed for each request until done for all active VDEVs
*/
spin_lock_bh(&ar->data_lock);
- ath11k_fw_stats_bcn_free(&ar->debug.fw_stats.bcn);
+ ath11k_fw_stats_bcn_free(&ar->fw_stats.bcn);
spin_unlock_bh(&ar->data_lock);
file->private_data = buf;
@@ -1055,7 +991,7 @@ void ath11k_debugfs_fw_stats_init(struct ath11k *ar)
struct dentry *fwstats_dir = debugfs_create_dir("fw_stats",
ar->debug.debugfs_pdev);
- ar->debug.fw_stats.debugfs_fwstats = fwstats_dir;
+ ar->fw_stats.debugfs_fwstats = fwstats_dir;
/* all stats debugfs files created are under "fw_stats" directory
* created per PDEV
@@ -1067,11 +1003,6 @@ void ath11k_debugfs_fw_stats_init(struct ath11k *ar)
debugfs_create_file("beacon_stats", 0600, fwstats_dir, ar,
&fops_bcn_stats);
- INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
- INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
- INIT_LIST_HEAD(&ar->debug.fw_stats.bcn);
-
- init_completion(&ar->debug.fw_stats_complete);
}
static ssize_t ath11k_write_pktlog_filter(struct file *file,
diff --git a/drivers/net/wireless/ath/ath11k/debugfs.h b/drivers/net/wireless/ath/ath11k/debugfs.h
index 8fc125a71943..e45dc874ff23 100644
--- a/drivers/net/wireless/ath/ath11k/debugfs.h
+++ b/drivers/net/wireless/ath/ath11k/debugfs.h
@@ -271,7 +271,7 @@ int ath11k_debugfs_pdev_create(struct ath11k_base *ab);
void ath11k_debugfs_pdev_destroy(struct ath11k_base *ab);
int ath11k_debugfs_register(struct ath11k *ar);
void ath11k_debugfs_unregister(struct ath11k *ar);
-void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb);
+void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct ath11k_fw_stats *stats);
void ath11k_debugfs_fw_stats_init(struct ath11k *ar);
int ath11k_debugfs_get_fw_stats(struct ath11k *ar, u32 pdev_id,
@@ -352,8 +352,8 @@ static inline void ath11k_debugfs_unregister(struct ath11k *ar)
{
}
-static inline void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab,
- struct sk_buff *skb)
+static inline void ath11k_debugfs_fw_stats_process(struct ath11k *ar,
+ struct ath11k_fw_stats *stats)
{
}
diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
index 84d1c7054013..9c7a0a438b12 100644
--- a/drivers/net/wireless/ath/ath11k/wmi.c
+++ b/drivers/net/wireless/ath/ath11k/wmi.c
@@ -7412,7 +7412,53 @@ static void ath11k_peer_assoc_conf_event(struct ath11k_base *ab, struct sk_buff
static void ath11k_update_stats_event(struct ath11k_base *ab, struct sk_buff *skb)
{
- ath11k_debugfs_fw_stats_process(ab, skb);
+ struct ath11k_fw_stats stats = {};
+ struct ath11k *ar;
+ int ret;
+
+ INIT_LIST_HEAD(&stats.pdevs);
+ INIT_LIST_HEAD(&stats.vdevs);
+ INIT_LIST_HEAD(&stats.bcn);
+
+ ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
+ if (ret) {
+ ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
+ goto free;
+ }
+
+ rcu_read_lock();
+ ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
+ if (!ar) {
+ rcu_read_unlock();
+ ath11k_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_done = true;
+ goto complete;
+ }
+
+ /* WMI_REQUEST_VDEV_STAT, WMI_REQUEST_BCN_STAT and WMI_REQUEST_RSSI_PER_CHAIN_STAT
+ * are currently requested only via debugfs fw stats. Hence, processing these
+ * in debugfs context
+ */
+ ath11k_debugfs_fw_stats_process(ar, &stats);
+
+complete:
+ complete(&ar->fw_stats_complete);
+ rcu_read_unlock();
+ spin_unlock_bh(&ar->data_lock);
+
+free:
+ ath11k_fw_stats_free(&stats);
}
/* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the frequency scanned
--
2.17.1
On 6/1/2022 10:14 PM, Aditya Kumar Singh wrote:
> 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: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
>
> Signed-off-by: Aditya Kumar Singh <[email protected]>
> ---
> drivers/net/wireless/ath/ath11k/core.c | 46 ++++++++
> drivers/net/wireless/ath/ath11k/core.h | 12 +-
> drivers/net/wireless/ath/ath11k/debugfs.c | 131 +++++-----------------
> drivers/net/wireless/ath/ath11k/debugfs.h | 6 +-
> drivers/net/wireless/ath/ath11k/wmi.c | 48 +++++++-
> 5 files changed, 136 insertions(+), 107 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
> index ca6fadd64b43..07a299a2e213 100644
> --- a/drivers/net/wireless/ath/ath11k/core.c
> +++ b/drivers/net/wireless/ath/ath11k/core.c
> @@ -570,6 +570,52 @@ static inline struct ath11k_pdev *ath11k_core_get_single_pdev(struct ath11k_base
> return &ab->pdevs[0];
> }
>
> +void ath11k_fw_stats_pdevs_free(struct list_head *head)
> +{
> + struct ath11k_fw_stats_pdev *i, *tmp;
> +
> + list_for_each_entry_safe(i, tmp, head, list) {
> + list_del(&i->list);
> + kfree(i);
> + }
> +}
> +
> +void ath11k_fw_stats_vdevs_free(struct list_head *head)
> +{
> + struct ath11k_fw_stats_vdev *i, *tmp;
> +
> + list_for_each_entry_safe(i, tmp, head, list) {
> + list_del(&i->list);
> + kfree(i);
> + }
> +}
> +
> +void ath11k_fw_stats_bcn_free(struct list_head *head)
> +{
> + struct ath11k_fw_stats_bcn *i, *tmp;
> +
> + list_for_each_entry_safe(i, tmp, head, list) {
> + list_del(&i->list);
> + kfree(i);
> + }
> +}
> +
> +void ath11k_fw_stats_init(struct ath11k *ar)
> +{
> + INIT_LIST_HEAD(&ar->fw_stats.pdevs);
> + INIT_LIST_HEAD(&ar->fw_stats.vdevs);
> + INIT_LIST_HEAD(&ar->fw_stats.bcn);
> +
> + init_completion(&ar->fw_stats_complete);
> +}
> +
> +void ath11k_fw_stats_free(struct ath11k_fw_stats *stats)
> +{
> + ath11k_fw_stats_pdevs_free(&stats->pdevs);
> + ath11k_fw_stats_vdevs_free(&stats->vdevs);
> + ath11k_fw_stats_bcn_free(&stats->bcn);
> +}
> +
> int ath11k_core_suspend(struct ath11k_base *ab)
> {
> int ret;
> diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
> index 65d54e9c15d9..672701f40a6b 100644
> --- a/drivers/net/wireless/ath/ath11k/core.h
> +++ b/drivers/net/wireless/ath/ath11k/core.h
> @@ -545,9 +545,6 @@ struct ath11k_debug {
> struct dentry *debugfs_pdev;
> struct ath11k_dbg_htt_stats htt_stats;
> u32 extd_tx_stats;
> - struct ath11k_fw_stats fw_stats;
> - struct completion fw_stats_complete;
> - bool fw_stats_done;
> u32 extd_rx_stats;
> u32 pktlog_filter;
> u32 pktlog_mode;
> @@ -712,6 +709,9 @@ struct ath11k {
> u8 twt_enabled;
> bool nlo_enabled;
> u8 alpha2[REG_ALPHA2_LEN + 1];
> + struct ath11k_fw_stats fw_stats;
> + struct completion fw_stats_complete;
> + bool fw_stats_done;
> };
>
> struct ath11k_band_cap {
> @@ -1117,6 +1117,12 @@ struct ath11k_fw_stats_bcn {
> u32 tx_bcn_outage_cnt;
> };
>
> +void ath11k_fw_stats_init(struct ath11k *ar);
> +void ath11k_fw_stats_pdevs_free(struct list_head *head);
> +void ath11k_fw_stats_vdevs_free(struct list_head *head);
> +void ath11k_fw_stats_bcn_free(struct list_head *head);
> +void ath11k_fw_stats_free(struct ath11k_fw_stats *stats);
> +
> extern const struct ce_pipe_config ath11k_target_ce_config_wlan_ipq8074[];
> extern const struct service_to_pipe ath11k_target_service_to_ce_map_wlan_ipq8074[];
> extern const struct service_to_pipe ath11k_target_service_to_ce_map_wlan_ipq6018[];
> diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c b/drivers/net/wireless/ath/ath11k/debugfs.c
> index 0d4843ef9dd1..e4a2fd3da481 100644
> --- a/drivers/net/wireless/ath/ath11k/debugfs.c
> +++ b/drivers/net/wireless/ath/ath11k/debugfs.c
> @@ -93,91 +93,36 @@ void ath11k_debugfs_add_dbring_entry(struct ath11k *ar,
> spin_unlock_bh(&dbr_data->lock);
> }
>
> -static void ath11k_fw_stats_pdevs_free(struct list_head *head)
> -{
> - struct ath11k_fw_stats_pdev *i, *tmp;
> -
> - list_for_each_entry_safe(i, tmp, head, list) {
> - list_del(&i->list);
> - kfree(i);
> - }
> -}
> -
> -static void ath11k_fw_stats_vdevs_free(struct list_head *head)
> -{
> - struct ath11k_fw_stats_vdev *i, *tmp;
> -
> - list_for_each_entry_safe(i, tmp, head, list) {
> - list_del(&i->list);
> - kfree(i);
> - }
> -}
> -
> -static void ath11k_fw_stats_bcn_free(struct list_head *head)
> -{
> - struct ath11k_fw_stats_bcn *i, *tmp;
> -
> - list_for_each_entry_safe(i, tmp, head, list) {
> - list_del(&i->list);
> - kfree(i);
> - }
> -}
>
> static void ath11k_debugfs_fw_stats_reset(struct ath11k *ar)
> {
> spin_lock_bh(&ar->data_lock);
> - ar->debug.fw_stats_done = false;
> - ath11k_fw_stats_pdevs_free(&ar->debug.fw_stats.pdevs);
> - ath11k_fw_stats_vdevs_free(&ar->debug.fw_stats.vdevs);
> + ar->fw_stats_done = false;
> + ath11k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
> + ath11k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
> spin_unlock_bh(&ar->data_lock);
> }
>
> -void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb)
> +void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct ath11k_fw_stats *stats)
> {
> - struct ath11k_fw_stats stats = {};
> - struct ath11k *ar;
> + struct ath11k_base *ab = ar->ab;
> struct ath11k_pdev *pdev;
> bool is_end;
> static unsigned int num_vdev, num_bcn;
> size_t total_vdevs_started = 0;
> - int i, ret;
> -
> - INIT_LIST_HEAD(&stats.pdevs);
> - INIT_LIST_HEAD(&stats.vdevs);
> - INIT_LIST_HEAD(&stats.bcn);
> -
> - ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
> - if (ret) {
> - ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
> - goto free;
> - }
> -
> - rcu_read_lock();
> - ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
> - if (!ar) {
> - rcu_read_unlock();
> - ath11k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
> - stats.pdev_id, ret);
> - goto free;
> - }
> -
> - spin_lock_bh(&ar->data_lock);
> + int i;
>
> - if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
> - list_splice_tail_init(&stats.pdevs, &ar->debug.fw_stats.pdevs);
> - ar->debug.fw_stats_done = true;
> - goto complete;
> - }
> + /* WMI_REQUEST_PDEV_STAT request has been already processed */
>
> - if (stats.stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
> - ar->debug.fw_stats_done = true;
> - goto complete;
> + if (stats->stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
> + ar->fw_stats_done = true;
> + return;
> }
>
> - if (stats.stats_id == WMI_REQUEST_VDEV_STAT) {
> - if (list_empty(&stats.vdevs)) {
> + if (stats->stats_id == WMI_REQUEST_VDEV_STAT) {
> + if (list_empty(&stats->vdevs)) {
> ath11k_warn(ab, "empty vdev stats");
> - goto complete;
> + return;
> }
> /* FW sends all the active VDEV stats irrespective of PDEV,
> * hence limit until the count of all VDEVs started
> @@ -190,43 +135,34 @@ void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb
>
> is_end = ((++num_vdev) == total_vdevs_started);
>
> - list_splice_tail_init(&stats.vdevs,
> - &ar->debug.fw_stats.vdevs);
> + list_splice_tail_init(&stats->vdevs,
> + &ar->fw_stats.vdevs);
>
> if (is_end) {
> - ar->debug.fw_stats_done = true;
> + ar->fw_stats_done = true;
> num_vdev = 0;
> }
> - goto complete;
> + return;
> }
>
> - if (stats.stats_id == WMI_REQUEST_BCN_STAT) {
> - if (list_empty(&stats.bcn)) {
> + if (stats->stats_id == WMI_REQUEST_BCN_STAT) {
> + if (list_empty(&stats->bcn)) {
> ath11k_warn(ab, "empty bcn stats");
> - goto complete;
> + return;
> }
> /* Mark end until we reached the count of all started VDEVs
> * within the PDEV
> */
> is_end = ((++num_bcn) == ar->num_started_vdevs);
>
> - list_splice_tail_init(&stats.bcn,
> - &ar->debug.fw_stats.bcn);
> + list_splice_tail_init(&stats->bcn,
> + &ar->fw_stats.bcn);
>
> if (is_end) {
> - ar->debug.fw_stats_done = true;
> + ar->fw_stats_done = true;
> num_bcn = 0;
> }
> }
> -complete:
> - complete(&ar->debug.fw_stats_complete);
> - rcu_read_unlock();
> - spin_unlock_bh(&ar->data_lock);
> -
> -free:
> - ath11k_fw_stats_pdevs_free(&stats.pdevs);
> - ath11k_fw_stats_vdevs_free(&stats.vdevs);
> - ath11k_fw_stats_bcn_free(&stats.bcn);
> }
>
> static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
> @@ -247,7 +183,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
>
> ath11k_debugfs_fw_stats_reset(ar);
>
> - reinit_completion(&ar->debug.fw_stats_complete);
> + reinit_completion(&ar->fw_stats_complete);
>
> ret = ath11k_wmi_send_stats_request_cmd(ar, req_param);
>
> @@ -258,7 +194,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
> }
>
> time_left =
> - wait_for_completion_timeout(&ar->debug.fw_stats_complete,
> + wait_for_completion_timeout(&ar->fw_stats_complete,
this is a case where imo the existing code was not compliant with the
coding standard (descendant was not indented from the parent) so I'd
definitely want to either indent this or combine it with the prior line
> 1 * HZ);
> if (!time_left)
> return -ETIMEDOUT;
> @@ -268,7 +204,7 @@ static int ath11k_debugfs_fw_stats_request(struct ath11k *ar,
> break;
>
> spin_lock_bh(&ar->data_lock);
> - if (ar->debug.fw_stats_done) {
> + if (ar->fw_stats_done) {
> spin_unlock_bh(&ar->data_lock);
> break;
> }
> @@ -340,7 +276,7 @@ static int ath11k_open_pdev_stats(struct inode *inode, struct file *file)
> goto err_free;
> }
>
> - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
> + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> buf);
there are multiple places in this patch where, due to the fact you are
removing 6 characters from an existing line, a descendant may now fit on
the prior line without exceeding 80 columns. consider removing the line
break in those cases.
i wasn't going to mention these bikeshed comments but I see something in
the 2nd patch that imo should be addressed, so you'll probably want to
upload a v2 and this would be a good cleanup to apply as well
>
> file->private_data = buf;
> @@ -412,7 +348,7 @@ static int ath11k_open_vdev_stats(struct inode *inode, struct file *file)
> goto err_free;
> }
>
> - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
> + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> buf);
>
> file->private_data = buf;
> @@ -490,14 +426,14 @@ static int ath11k_open_bcn_stats(struct inode *inode, struct file *file)
> }
> }
>
> - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats, req_param.stats_id,
> + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> buf);
>
> /* since beacon stats request is looped for all active VDEVs, saved fw
> * stats is not freed for each request until done for all active VDEVs
> */
> spin_lock_bh(&ar->data_lock);
> - ath11k_fw_stats_bcn_free(&ar->debug.fw_stats.bcn);
> + ath11k_fw_stats_bcn_free(&ar->fw_stats.bcn);
> spin_unlock_bh(&ar->data_lock);
>
> file->private_data = buf;
> @@ -1055,7 +991,7 @@ void ath11k_debugfs_fw_stats_init(struct ath11k *ar)
> struct dentry *fwstats_dir = debugfs_create_dir("fw_stats",
> ar->debug.debugfs_pdev);
>
> - ar->debug.fw_stats.debugfs_fwstats = fwstats_dir;
> + ar->fw_stats.debugfs_fwstats = fwstats_dir;
>
> /* all stats debugfs files created are under "fw_stats" directory
> * created per PDEV
> @@ -1067,11 +1003,6 @@ void ath11k_debugfs_fw_stats_init(struct ath11k *ar)
> debugfs_create_file("beacon_stats", 0600, fwstats_dir, ar,
> &fops_bcn_stats);
>
> - INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
> - INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
> - INIT_LIST_HEAD(&ar->debug.fw_stats.bcn);
> -
> - init_completion(&ar->debug.fw_stats_complete);
> }
>
> static ssize_t ath11k_write_pktlog_filter(struct file *file,
> diff --git a/drivers/net/wireless/ath/ath11k/debugfs.h b/drivers/net/wireless/ath/ath11k/debugfs.h
> index 8fc125a71943..e45dc874ff23 100644
> --- a/drivers/net/wireless/ath/ath11k/debugfs.h
> +++ b/drivers/net/wireless/ath/ath11k/debugfs.h
> @@ -271,7 +271,7 @@ int ath11k_debugfs_pdev_create(struct ath11k_base *ab);
> void ath11k_debugfs_pdev_destroy(struct ath11k_base *ab);
> int ath11k_debugfs_register(struct ath11k *ar);
> void ath11k_debugfs_unregister(struct ath11k *ar);
> -void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff *skb);
> +void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct ath11k_fw_stats *stats);
>
> void ath11k_debugfs_fw_stats_init(struct ath11k *ar);
> int ath11k_debugfs_get_fw_stats(struct ath11k *ar, u32 pdev_id,
> @@ -352,8 +352,8 @@ static inline void ath11k_debugfs_unregister(struct ath11k *ar)
> {
> }
>
> -static inline void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab,
> - struct sk_buff *skb)
> +static inline void ath11k_debugfs_fw_stats_process(struct ath11k *ar,
> + struct ath11k_fw_stats *stats)
> {
> }
>
> diff --git a/drivers/net/wireless/ath/ath11k/wmi.c b/drivers/net/wireless/ath/ath11k/wmi.c
> index 84d1c7054013..9c7a0a438b12 100644
> --- a/drivers/net/wireless/ath/ath11k/wmi.c
> +++ b/drivers/net/wireless/ath/ath11k/wmi.c
> @@ -7412,7 +7412,53 @@ static void ath11k_peer_assoc_conf_event(struct ath11k_base *ab, struct sk_buff
>
> static void ath11k_update_stats_event(struct ath11k_base *ab, struct sk_buff *skb)
> {
> - ath11k_debugfs_fw_stats_process(ab, skb);
> + struct ath11k_fw_stats stats = {};
> + struct ath11k *ar;
> + int ret;
> +
> + INIT_LIST_HEAD(&stats.pdevs);
> + INIT_LIST_HEAD(&stats.vdevs);
> + INIT_LIST_HEAD(&stats.bcn);
> +
> + ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
> + if (ret) {
> + ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
> + goto free;
> + }
> +
> + rcu_read_lock();
> + ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
> + if (!ar) {
> + rcu_read_unlock();
> + ath11k_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_done = true;
> + goto complete;
> + }
> +
> + /* WMI_REQUEST_VDEV_STAT, WMI_REQUEST_BCN_STAT and WMI_REQUEST_RSSI_PER_CHAIN_STAT
> + * are currently requested only via debugfs fw stats. Hence, processing these
> + * in debugfs context
> + */
> + ath11k_debugfs_fw_stats_process(ar, &stats);
> +
> +complete:
> + complete(&ar->fw_stats_complete);
> + rcu_read_unlock();
> + spin_unlock_bh(&ar->data_lock);
> +
> +free:
> + ath11k_fw_stats_free(&stats);
> }
>
> /* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the frequency scanned
> -----Original Message-----
> From: Jeff Johnson (QUIC) <[email protected]>
> Sent: Thursday, June 2, 2022 20:03
> To: Aditya Kumar Singh (QUIC) <[email protected]>;
> [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH 1/2] ath11k: move firmware stats out of debugfs
>
> On 6/1/2022 10:14 PM, Aditya Kumar Singh wrote:
> > 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: IPQ8074 hw2.0 AHB
> > WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> > Tested-on: QCN9074 hw1.0 PCI
> > WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> > Tested-on: WCN6855 hw2.0 PCI
> > WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
> >
> > Signed-off-by: Aditya Kumar Singh <[email protected]>
> > ---
> > drivers/net/wireless/ath/ath11k/core.c | 46 ++++++++
> > drivers/net/wireless/ath/ath11k/core.h | 12 +-
> > drivers/net/wireless/ath/ath11k/debugfs.c | 131 +++++-----------------
> > drivers/net/wireless/ath/ath11k/debugfs.h | 6 +-
> > drivers/net/wireless/ath/ath11k/wmi.c | 48 +++++++-
> > 5 files changed, 136 insertions(+), 107 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath11k/core.c
> > b/drivers/net/wireless/ath/ath11k/core.c
> > index ca6fadd64b43..07a299a2e213 100644
> > --- a/drivers/net/wireless/ath/ath11k/core.c
> > +++ b/drivers/net/wireless/ath/ath11k/core.c
> > @@ -570,6 +570,52 @@ static inline struct ath11k_pdev
> *ath11k_core_get_single_pdev(struct ath11k_base
> > return &ab->pdevs[0];
> > }
> >
> > +void ath11k_fw_stats_pdevs_free(struct list_head *head) {
> > + struct ath11k_fw_stats_pdev *i, *tmp;
> > +
> > + list_for_each_entry_safe(i, tmp, head, list) {
> > + list_del(&i->list);
> > + kfree(i);
> > + }
> > +}
> > +
> > +void ath11k_fw_stats_vdevs_free(struct list_head *head) {
> > + struct ath11k_fw_stats_vdev *i, *tmp;
> > +
> > + list_for_each_entry_safe(i, tmp, head, list) {
> > + list_del(&i->list);
> > + kfree(i);
> > + }
> > +}
> > +
> > +void ath11k_fw_stats_bcn_free(struct list_head *head) {
> > + struct ath11k_fw_stats_bcn *i, *tmp;
> > +
> > + list_for_each_entry_safe(i, tmp, head, list) {
> > + list_del(&i->list);
> > + kfree(i);
> > + }
> > +}
> > +
> > +void ath11k_fw_stats_init(struct ath11k *ar) {
> > + INIT_LIST_HEAD(&ar->fw_stats.pdevs);
> > + INIT_LIST_HEAD(&ar->fw_stats.vdevs);
> > + INIT_LIST_HEAD(&ar->fw_stats.bcn);
> > +
> > + init_completion(&ar->fw_stats_complete);
> > +}
> > +
> > +void ath11k_fw_stats_free(struct ath11k_fw_stats *stats) {
> > + ath11k_fw_stats_pdevs_free(&stats->pdevs);
> > + ath11k_fw_stats_vdevs_free(&stats->vdevs);
> > + ath11k_fw_stats_bcn_free(&stats->bcn);
> > +}
> > +
> > int ath11k_core_suspend(struct ath11k_base *ab)
> > {
> > int ret;
> > diff --git a/drivers/net/wireless/ath/ath11k/core.h
> > b/drivers/net/wireless/ath/ath11k/core.h
> > index 65d54e9c15d9..672701f40a6b 100644
> > --- a/drivers/net/wireless/ath/ath11k/core.h
> > +++ b/drivers/net/wireless/ath/ath11k/core.h
> > @@ -545,9 +545,6 @@ struct ath11k_debug {
> > struct dentry *debugfs_pdev;
> > struct ath11k_dbg_htt_stats htt_stats;
> > u32 extd_tx_stats;
> > - struct ath11k_fw_stats fw_stats;
> > - struct completion fw_stats_complete;
> > - bool fw_stats_done;
> > u32 extd_rx_stats;
> > u32 pktlog_filter;
> > u32 pktlog_mode;
> > @@ -712,6 +709,9 @@ struct ath11k {
> > u8 twt_enabled;
> > bool nlo_enabled;
> > u8 alpha2[REG_ALPHA2_LEN + 1];
> > + struct ath11k_fw_stats fw_stats;
> > + struct completion fw_stats_complete;
> > + bool fw_stats_done;
> > };
> >
> > struct ath11k_band_cap {
> > @@ -1117,6 +1117,12 @@ struct ath11k_fw_stats_bcn {
> > u32 tx_bcn_outage_cnt;
> > };
> >
> > +void ath11k_fw_stats_init(struct ath11k *ar); void
> > +ath11k_fw_stats_pdevs_free(struct list_head *head); void
> > +ath11k_fw_stats_vdevs_free(struct list_head *head); void
> > +ath11k_fw_stats_bcn_free(struct list_head *head); void
> > +ath11k_fw_stats_free(struct ath11k_fw_stats *stats);
> > +
> > extern const struct ce_pipe_config
> ath11k_target_ce_config_wlan_ipq8074[];
> > extern const struct service_to_pipe
> ath11k_target_service_to_ce_map_wlan_ipq8074[];
> > extern const struct service_to_pipe
> > ath11k_target_service_to_ce_map_wlan_ipq6018[];
> > diff --git a/drivers/net/wireless/ath/ath11k/debugfs.c
> > b/drivers/net/wireless/ath/ath11k/debugfs.c
> > index 0d4843ef9dd1..e4a2fd3da481 100644
> > --- a/drivers/net/wireless/ath/ath11k/debugfs.c
> > +++ b/drivers/net/wireless/ath/ath11k/debugfs.c
> > @@ -93,91 +93,36 @@ void ath11k_debugfs_add_dbring_entry(struct
> ath11k *ar,
> > spin_unlock_bh(&dbr_data->lock);
> > }
> >
> > -static void ath11k_fw_stats_pdevs_free(struct list_head *head) -{
> > - struct ath11k_fw_stats_pdev *i, *tmp;
> > -
> > - list_for_each_entry_safe(i, tmp, head, list) {
> > - list_del(&i->list);
> > - kfree(i);
> > - }
> > -}
> > -
> > -static void ath11k_fw_stats_vdevs_free(struct list_head *head) -{
> > - struct ath11k_fw_stats_vdev *i, *tmp;
> > -
> > - list_for_each_entry_safe(i, tmp, head, list) {
> > - list_del(&i->list);
> > - kfree(i);
> > - }
> > -}
> > -
> > -static void ath11k_fw_stats_bcn_free(struct list_head *head) -{
> > - struct ath11k_fw_stats_bcn *i, *tmp;
> > -
> > - list_for_each_entry_safe(i, tmp, head, list) {
> > - list_del(&i->list);
> > - kfree(i);
> > - }
> > -}
> >
> > static void ath11k_debugfs_fw_stats_reset(struct ath11k *ar)
> > {
> > spin_lock_bh(&ar->data_lock);
> > - ar->debug.fw_stats_done = false;
> > - ath11k_fw_stats_pdevs_free(&ar->debug.fw_stats.pdevs);
> > - ath11k_fw_stats_vdevs_free(&ar->debug.fw_stats.vdevs);
> > + ar->fw_stats_done = false;
> > + ath11k_fw_stats_pdevs_free(&ar->fw_stats.pdevs);
> > + ath11k_fw_stats_vdevs_free(&ar->fw_stats.vdevs);
> > spin_unlock_bh(&ar->data_lock);
> > }
> >
> > -void ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct
> > sk_buff *skb)
> > +void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct
> > +ath11k_fw_stats *stats)
> > {
> > - struct ath11k_fw_stats stats = {};
> > - struct ath11k *ar;
> > + struct ath11k_base *ab = ar->ab;
> > struct ath11k_pdev *pdev;
> > bool is_end;
> > static unsigned int num_vdev, num_bcn;
> > size_t total_vdevs_started = 0;
> > - int i, ret;
> > -
> > - INIT_LIST_HEAD(&stats.pdevs);
> > - INIT_LIST_HEAD(&stats.vdevs);
> > - INIT_LIST_HEAD(&stats.bcn);
> > -
> > - ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
> > - if (ret) {
> > - ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
> > - goto free;
> > - }
> > -
> > - rcu_read_lock();
> > - ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
> > - if (!ar) {
> > - rcu_read_unlock();
> > - ath11k_warn(ab, "failed to get ar for pdev_id %d: %d\n",
> > - stats.pdev_id, ret);
> > - goto free;
> > - }
> > -
> > - spin_lock_bh(&ar->data_lock);
> > + int i;
> >
> > - if (stats.stats_id == WMI_REQUEST_PDEV_STAT) {
> > - list_splice_tail_init(&stats.pdevs, &ar-
> >debug.fw_stats.pdevs);
> > - ar->debug.fw_stats_done = true;
> > - goto complete;
> > - }
> > + /* WMI_REQUEST_PDEV_STAT request has been already processed
> */
> >
> > - if (stats.stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
> > - ar->debug.fw_stats_done = true;
> > - goto complete;
> > + if (stats->stats_id == WMI_REQUEST_RSSI_PER_CHAIN_STAT) {
> > + ar->fw_stats_done = true;
> > + return;
> > }
> >
> > - if (stats.stats_id == WMI_REQUEST_VDEV_STAT) {
> > - if (list_empty(&stats.vdevs)) {
> > + if (stats->stats_id == WMI_REQUEST_VDEV_STAT) {
> > + if (list_empty(&stats->vdevs)) {
> > ath11k_warn(ab, "empty vdev stats");
> > - goto complete;
> > + return;
> > }
> > /* FW sends all the active VDEV stats irrespective of PDEV,
> > * hence limit until the count of all VDEVs started @@ -190,43
> > +135,34 @@ void ath11k_debugfs_fw_stats_process(struct ath11k_base
> > *ab, struct sk_buff *skb
> >
> > is_end = ((++num_vdev) == total_vdevs_started);
> >
> > - list_splice_tail_init(&stats.vdevs,
> > - &ar->debug.fw_stats.vdevs);
> > + list_splice_tail_init(&stats->vdevs,
> > + &ar->fw_stats.vdevs);
> >
> > if (is_end) {
> > - ar->debug.fw_stats_done = true;
> > + ar->fw_stats_done = true;
> > num_vdev = 0;
> > }
> > - goto complete;
> > + return;
> > }
> >
> > - if (stats.stats_id == WMI_REQUEST_BCN_STAT) {
> > - if (list_empty(&stats.bcn)) {
> > + if (stats->stats_id == WMI_REQUEST_BCN_STAT) {
> > + if (list_empty(&stats->bcn)) {
> > ath11k_warn(ab, "empty bcn stats");
> > - goto complete;
> > + return;
> > }
> > /* Mark end until we reached the count of all started VDEVs
> > * within the PDEV
> > */
> > is_end = ((++num_bcn) == ar->num_started_vdevs);
> >
> > - list_splice_tail_init(&stats.bcn,
> > - &ar->debug.fw_stats.bcn);
> > + list_splice_tail_init(&stats->bcn,
> > + &ar->fw_stats.bcn);
> >
> > if (is_end) {
> > - ar->debug.fw_stats_done = true;
> > + ar->fw_stats_done = true;
> > num_bcn = 0;
> > }
> > }
> > -complete:
> > - complete(&ar->debug.fw_stats_complete);
> > - rcu_read_unlock();
> > - spin_unlock_bh(&ar->data_lock);
> > -
> > -free:
> > - ath11k_fw_stats_pdevs_free(&stats.pdevs);
> > - ath11k_fw_stats_vdevs_free(&stats.vdevs);
> > - ath11k_fw_stats_bcn_free(&stats.bcn);
> > }
> >
> > static int ath11k_debugfs_fw_stats_request(struct ath11k *ar, @@
> > -247,7 +183,7 @@ static int ath11k_debugfs_fw_stats_request(struct
> > ath11k *ar,
> >
> > ath11k_debugfs_fw_stats_reset(ar);
> >
> > - reinit_completion(&ar->debug.fw_stats_complete);
> > + reinit_completion(&ar->fw_stats_complete);
> >
> > ret = ath11k_wmi_send_stats_request_cmd(ar, req_param);
> >
> > @@ -258,7 +194,7 @@ static int ath11k_debugfs_fw_stats_request(struct
> ath11k *ar,
> > }
> >
> > time_left =
> > - wait_for_completion_timeout(&ar->debug.fw_stats_complete,
> > + wait_for_completion_timeout(&ar->fw_stats_complete,
>
> this is a case where imo the existing code was not compliant with the coding
> standard (descendant was not indented from the parent) so I'd definitely
> want to either indent this or combine it with the prior line
>
Sure, will address in v2. Thanks for pointing out.
> > 1 * HZ);
> > if (!time_left)
> > return -ETIMEDOUT;
> > @@ -268,7 +204,7 @@ static int ath11k_debugfs_fw_stats_request(struct
> ath11k *ar,
> > break;
> >
> > spin_lock_bh(&ar->data_lock);
> > - if (ar->debug.fw_stats_done) {
> > + if (ar->fw_stats_done) {
> > spin_unlock_bh(&ar->data_lock);
> > break;
> > }
> > @@ -340,7 +276,7 @@ static int ath11k_open_pdev_stats(struct inode
> *inode, struct file *file)
> > goto err_free;
> > }
> >
> > - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats,
> req_param.stats_id,
> > + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> > buf);
>
> there are multiple places in this patch where, due to the fact you are
> removing 6 characters from an existing line, a descendant may now fit on the
> prior line without exceeding 80 columns. consider removing the line break in
> those cases.
>
> i wasn't going to mention these bikeshed comments but I see something in
> the 2nd patch that imo should be addressed, so you'll probably want to
> upload a v2 and this would be a good cleanup to apply as well
>
Yes correct. Will address these v2. Thanks.
> >
> > file->private_data = buf;
> > @@ -412,7 +348,7 @@ static int ath11k_open_vdev_stats(struct inode
> *inode, struct file *file)
> > goto err_free;
> > }
> >
> > - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats,
> req_param.stats_id,
> > + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> > buf);
> >
> > file->private_data = buf;
> > @@ -490,14 +426,14 @@ static int ath11k_open_bcn_stats(struct inode
> *inode, struct file *file)
> > }
> > }
> >
> > - ath11k_wmi_fw_stats_fill(ar, &ar->debug.fw_stats,
> req_param.stats_id,
> > + ath11k_wmi_fw_stats_fill(ar, &ar->fw_stats, req_param.stats_id,
> > buf);
> >
> > /* since beacon stats request is looped for all active VDEVs, saved fw
> > * stats is not freed for each request until done for all active VDEVs
> > */
> > spin_lock_bh(&ar->data_lock);
> > - ath11k_fw_stats_bcn_free(&ar->debug.fw_stats.bcn);
> > + ath11k_fw_stats_bcn_free(&ar->fw_stats.bcn);
> > spin_unlock_bh(&ar->data_lock);
> >
> > file->private_data = buf;
> > @@ -1055,7 +991,7 @@ void ath11k_debugfs_fw_stats_init(struct ath11k
> *ar)
> > struct dentry *fwstats_dir = debugfs_create_dir("fw_stats",
> > ar-
> >debug.debugfs_pdev);
> >
> > - ar->debug.fw_stats.debugfs_fwstats = fwstats_dir;
> > + ar->fw_stats.debugfs_fwstats = fwstats_dir;
> >
> > /* all stats debugfs files created are under "fw_stats" directory
> > * created per PDEV
> > @@ -1067,11 +1003,6 @@ void ath11k_debugfs_fw_stats_init(struct
> ath11k *ar)
> > debugfs_create_file("beacon_stats", 0600, fwstats_dir, ar,
> > &fops_bcn_stats);
> >
> > - INIT_LIST_HEAD(&ar->debug.fw_stats.pdevs);
> > - INIT_LIST_HEAD(&ar->debug.fw_stats.vdevs);
> > - INIT_LIST_HEAD(&ar->debug.fw_stats.bcn);
> > -
> > - init_completion(&ar->debug.fw_stats_complete);
> > }
> >
> > static ssize_t ath11k_write_pktlog_filter(struct file *file, diff
> > --git a/drivers/net/wireless/ath/ath11k/debugfs.h
> > b/drivers/net/wireless/ath/ath11k/debugfs.h
> > index 8fc125a71943..e45dc874ff23 100644
> > --- a/drivers/net/wireless/ath/ath11k/debugfs.h
> > +++ b/drivers/net/wireless/ath/ath11k/debugfs.h
> > @@ -271,7 +271,7 @@ int ath11k_debugfs_pdev_create(struct
> ath11k_base *ab);
> > void ath11k_debugfs_pdev_destroy(struct ath11k_base *ab);
> > int ath11k_debugfs_register(struct ath11k *ar);
> > void ath11k_debugfs_unregister(struct ath11k *ar); -void
> > ath11k_debugfs_fw_stats_process(struct ath11k_base *ab, struct sk_buff
> > *skb);
> > +void ath11k_debugfs_fw_stats_process(struct ath11k *ar, struct
> > +ath11k_fw_stats *stats);
> >
> > void ath11k_debugfs_fw_stats_init(struct ath11k *ar);
> > int ath11k_debugfs_get_fw_stats(struct ath11k *ar, u32 pdev_id, @@
> > -352,8 +352,8 @@ static inline void ath11k_debugfs_unregister(struct
> ath11k *ar)
> > {
> > }
> >
> > -static inline void ath11k_debugfs_fw_stats_process(struct ath11k_base
> *ab,
> > - struct sk_buff *skb)
> > +static inline void ath11k_debugfs_fw_stats_process(struct ath11k *ar,
> > + struct ath11k_fw_stats
> *stats)
> > {
> > }
> >
> > diff --git a/drivers/net/wireless/ath/ath11k/wmi.c
> > b/drivers/net/wireless/ath/ath11k/wmi.c
> > index 84d1c7054013..9c7a0a438b12 100644
> > --- a/drivers/net/wireless/ath/ath11k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath11k/wmi.c
> > @@ -7412,7 +7412,53 @@ static void
> ath11k_peer_assoc_conf_event(struct
> > ath11k_base *ab, struct sk_buff
> >
> > static void ath11k_update_stats_event(struct ath11k_base *ab, struct
> sk_buff *skb)
> > {
> > - ath11k_debugfs_fw_stats_process(ab, skb);
> > + struct ath11k_fw_stats stats = {};
> > + struct ath11k *ar;
> > + int ret;
> > +
> > + INIT_LIST_HEAD(&stats.pdevs);
> > + INIT_LIST_HEAD(&stats.vdevs);
> > + INIT_LIST_HEAD(&stats.bcn);
> > +
> > + ret = ath11k_wmi_pull_fw_stats(ab, skb, &stats);
> > + if (ret) {
> > + ath11k_warn(ab, "failed to pull fw stats: %d\n", ret);
> > + goto free;
> > + }
> > +
> > + rcu_read_lock();
> > + ar = ath11k_mac_get_ar_by_pdev_id(ab, stats.pdev_id);
> > + if (!ar) {
> > + rcu_read_unlock();
> > + ath11k_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_done = true;
> > + goto complete;
> > + }
> > +
> > + /* WMI_REQUEST_VDEV_STAT, WMI_REQUEST_BCN_STAT and
> WMI_REQUEST_RSSI_PER_CHAIN_STAT
> > + * are currently requested only via debugfs fw stats. Hence,
> processing these
> > + * in debugfs context
> > + */
> > + ath11k_debugfs_fw_stats_process(ar, &stats);
> > +
> > +complete:
> > + complete(&ar->fw_stats_complete);
> > + rcu_read_unlock();
> > + spin_unlock_bh(&ar->data_lock);
> > +
> > +free:
> > + ath11k_fw_stats_free(&stats);
> > }
> >
> > /* PDEV_CTL_FAILSAFE_CHECK_EVENT is received from FW when the
> > frequency scanned
> -----Original Message-----
> From: Kalle Valo <[email protected]>
> Sent: Friday, June 3, 2022 12:21
> To: Aditya Kumar Singh (QUIC) <[email protected]>
> Cc: [email protected]; [email protected]; Aditya
> Kumar Singh (QUIC) <[email protected]>
> Subject: Re: [PATCH 1/2] ath11k: move firmware stats out of debugfs
>
> Aditya Kumar Singh <[email protected]> wrote:
>
> > 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: IPQ8074 hw2.0 AHB
> > WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> > Tested-on: QCN9074 hw1.0 PCI
> > WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> > Tested-on: WCN6855 hw2.0 PCI
> > WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
> >
> > Signed-off-by: Aditya Kumar Singh <[email protected]>
> > Signed-off-by: Kalle Valo <[email protected]>
>
> This added new warnings:
>
> drivers/net/wireless/ath/ath11k/debugfs.c:94: Please don't use multiple
> blank lines
> drivers/net/wireless/ath/ath11k/debugfs.c:976: Blank lines aren't necessary
> before a close brace '}'
>
I have sent [v2] before your mail. But that too will lead to these warnings hence
I have rectified and sent [v3].
> I fixed them in the pending branch.
Sure thanks.
>
> --
> https://patchwork.kernel.org/project/linux-
> wireless/patch/[email protected]/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> atches
Aditya Kumar Singh <[email protected]> wrote:
> 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: IPQ8074 hw2.0 AHB WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-03125-QCAHSPSWPL_V1_V2_SILICONZ_LITE-3
>
> Signed-off-by: Aditya Kumar Singh <[email protected]>
> Signed-off-by: Kalle Valo <[email protected]>
This added new warnings:
drivers/net/wireless/ath/ath11k/debugfs.c:94: Please don't use multiple blank lines
drivers/net/wireless/ath/ath11k/debugfs.c:976: Blank lines aren't necessary before a close brace '}'
I fixed them in the pending branch.
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
"Aditya Kumar Singh (QUIC)" <[email protected]> writes:
> Sure, will address in v2. Thanks for pointing out.
Please edit your quotes. You had over 500 lines of quotes in your one
line reply, this makes it horrible to use patchwork:
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches