[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/core.h  | 76 +--------------------------------
 drivers/net/wireless/ath/ath10k/debug.h | 75 ++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 75 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 7f27148..5a588a6 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -27,6 +27,7 @@
 #include "hw.h"
 #include "targaddrs.h"
 #include "wmi.h"
+#include "debug.h"
 #include "../ath.h"
 #include "../regd.h"
 
@@ -121,72 +122,6 @@ struct ath10k_wmi {
 	struct work_struct wmi_event_work;
 };
 
-struct ath10k_peer_stat {
-	u8 peer_macaddr[ETH_ALEN];
-	u32 peer_rssi;
-	u32 peer_tx_rate;
-};
-
-struct ath10k_target_stats {
-	/* PDEV stats */
-	s32 ch_noise_floor;
-	u32 tx_frame_count;
-	u32 rx_frame_count;
-	u32 rx_clear_count;
-	u32 cycle_count;
-	u32 phy_err_count;
-	u32 chan_tx_power;
-
-	/* PDEV TX stats */
-	s32 comp_queued;
-	s32 comp_delivered;
-	s32 msdu_enqued;
-	s32 mpdu_enqued;
-	s32 wmm_drop;
-	s32 local_enqued;
-	s32 local_freed;
-	s32 hw_queued;
-	s32 hw_reaped;
-	s32 underrun;
-	s32 tx_abort;
-	s32 mpdus_requed;
-	u32 tx_ko;
-	u32 data_rc;
-	u32 self_triggers;
-	u32 sw_retry_failure;
-	u32 illgl_rate_phy_err;
-	u32 pdev_cont_xretry;
-	u32 pdev_tx_timeout;
-	u32 pdev_resets;
-	u32 phy_underrun;
-	u32 txop_ovf;
-
-	/* PDEV RX stats */
-	s32 mid_ppdu_route_change;
-	s32 status_rcvd;
-	s32 r0_frags;
-	s32 r1_frags;
-	s32 r2_frags;
-	s32 r3_frags;
-	s32 htt_msdus;
-	s32 htt_mpdus;
-	s32 loc_msdus;
-	s32 loc_mpdus;
-	s32 oversize_amsdu;
-	s32 phy_errs;
-	s32 phy_err_drop;
-	s32 mpdu_errs;
-
-	/* VDEV STATS */
-
-	/* PEER STATS */
-	u8 peers;
-	struct ath10k_peer_stat peer_stat[TARGET_NUM_PEERS];
-
-	/* TODO: Beacon filter stats */
-
-};
-
 #define ATH10K_MAX_NUM_PEER_IDS (1 << 11) /* htt rx_desc limit */
 
 struct ath10k_peer {
@@ -240,15 +175,6 @@ struct ath10k_vif_iter {
 	struct ath10k_vif *arvif;
 };
 
-struct ath10k_debug {
-	struct dentry *debugfs_phy;
-
-	struct ath10k_target_stats target_stats;
-	u32 wmi_service_bitmap[WMI_SERVICE_BM_SIZE];
-
-	struct completion event_stats_compl;
-};
-
 struct ath10k {
 	struct ath_common common;
 	struct ieee80211_hw *hw;
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index bbaaa37..b0549cd 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -40,6 +40,81 @@ extern __printf(1, 2) int ath10k_info(const char *fmt, ...);
 extern __printf(1, 2) int ath10k_err(const char *fmt, ...);
 extern __printf(1, 2) int ath10k_warn(const char *fmt, ...);
 
+struct ath10k_peer_stat {
+	u8 peer_macaddr[ETH_ALEN];
+	u32 peer_rssi;
+	u32 peer_tx_rate;
+};
+
+struct ath10k_target_stats {
+	/* PDEV stats */
+	s32 ch_noise_floor;
+	u32 tx_frame_count;
+	u32 rx_frame_count;
+	u32 rx_clear_count;
+	u32 cycle_count;
+	u32 phy_err_count;
+	u32 chan_tx_power;
+
+	/* PDEV TX stats */
+	s32 comp_queued;
+	s32 comp_delivered;
+	s32 msdu_enqued;
+	s32 mpdu_enqued;
+	s32 wmm_drop;
+	s32 local_enqued;
+	s32 local_freed;
+	s32 hw_queued;
+	s32 hw_reaped;
+	s32 underrun;
+	s32 tx_abort;
+	s32 mpdus_requed;
+	u32 tx_ko;
+	u32 data_rc;
+	u32 self_triggers;
+	u32 sw_retry_failure;
+	u32 illgl_rate_phy_err;
+	u32 pdev_cont_xretry;
+	u32 pdev_tx_timeout;
+	u32 pdev_resets;
+	u32 phy_underrun;
+	u32 txop_ovf;
+
+	/* PDEV RX stats */
+	s32 mid_ppdu_route_change;
+	s32 status_rcvd;
+	s32 r0_frags;
+	s32 r1_frags;
+	s32 r2_frags;
+	s32 r3_frags;
+	s32 htt_msdus;
+	s32 htt_mpdus;
+	s32 loc_msdus;
+	s32 loc_mpdus;
+	s32 oversize_amsdu;
+	s32 phy_errs;
+	s32 phy_err_drop;
+	s32 mpdu_errs;
+
+	/* VDEV STATS */
+
+	/* PEER STATS */
+	u8 peers;
+	struct ath10k_peer_stat peer_stat[TARGET_NUM_PEERS];
+
+	/* TODO: Beacon filter stats */
+
+};
+
+struct ath10k_debug {
+	struct dentry *debugfs_phy;
+
+	struct ath10k_target_stats target_stats;
+	u32 wmi_service_bitmap[WMI_SERVICE_BM_SIZE];
+
+	struct completion event_stats_compl;
+};
+
 #ifdef CONFIG_ATH10K_DEBUGFS
 int ath10k_debug_create(struct ath10k *ar);
 void ath10k_debug_read_service_map(struct ath10k *ar,
-- 
1.8.2.2

[ath9k-devel] [PATCH 2/2] ath10k: Add host statistics

[Sujith Manoharan]

From: Sujith Manoharan <c_manoha@qca.qualcomm.com>

Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath10k/debug.c  | 55 +++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/debug.h  | 22 +++++++++++++
 drivers/net/wireless/ath/ath10k/htt_rx.c | 30 +++++++++++++----
 3 files changed, 100 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 946185c..f334053 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -443,6 +443,58 @@ static const struct file_operations fops_fw_stats = {
 	.llseek = default_llseek,
 };
 
+static ssize_t ath10k_read_host_stats(struct file *file, char __user *user_buf,
+				      size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	struct ath10k_host_stats *host_stats = &ar->debug.host_stats;
+	unsigned int len = 0, buf_len = 1024;
+	char *buf;
+	ssize_t ret_cnt;
+
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT RX indication :", host_stats->htt_rx_ind);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDUs :", host_stats->htt_msdu);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT A-MSDUs :", host_stats->htt_amsdu);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDU early :", host_stats->htt_msdu_early);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDU invalid length :", host_stats->htt_msdu_len_invalid);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDU zero len :", host_stats->htt_msdu_zero_len);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDU chained :", host_stats->htt_msdu_chained);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT MSDU no data :", host_stats->htt_msdu_no_data);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT decrypt error :", host_stats->htt_decrypt_err);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT invalid status :", host_stats->htt_invalid_status);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n",
+			 "HTT FCS error :", host_stats->htt_fcs_err);
+
+	if (len > buf_len)
+		len = buf_len;
+
+	ret_cnt = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+
+	kfree(buf);
+	return ret_cnt;
+}
+
+static const struct file_operations fops_host_stats = {
+	.read = ath10k_read_host_stats,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath10k_debug_create(struct ath10k *ar)
 {
 	ar->debug.debugfs_phy = debugfs_create_dir("ath10k",
@@ -455,9 +507,10 @@ int ath10k_debug_create(struct ath10k *ar)
 
 	debugfs_create_file("fw_stats", S_IRUSR, ar->debug.debugfs_phy, ar,
 			    &fops_fw_stats);
-
 	debugfs_create_file("wmi_services", S_IRUSR, ar->debug.debugfs_phy, ar,
 			    &fops_wmi_services);
+	debugfs_create_file("host_stats", S_IRUSR, ar->debug.debugfs_phy, ar,
+			    &fops_host_stats);
 
 	return 0;
 }
diff --git a/drivers/net/wireless/ath/ath10k/debug.h b/drivers/net/wireless/ath/ath10k/debug.h
index b0549cd..45866ab 100644
--- a/drivers/net/wireless/ath/ath10k/debug.h
+++ b/drivers/net/wireless/ath/ath10k/debug.h
@@ -106,9 +106,31 @@ struct ath10k_target_stats {
 
 };
 
+#ifdef CONFIG_ATH10K_DEBUGFS
+#define HOST_STAT_INC(c) ar->debug.host_stats.c++
+#else
+#define HOST_STAT_INC(c) do { } while (0)
+#endif
+
+struct ath10k_host_stats {
+	/* RX */
+	u64 htt_rx_ind;
+	u64 htt_msdu;
+	u64 htt_amsdu;
+	u64 htt_msdu_early;
+	u64 htt_msdu_len_invalid;
+	u64 htt_msdu_zero_len;
+	u64 htt_msdu_chained;
+	u64 htt_msdu_no_data;
+	u64 htt_decrypt_err;
+	u64 htt_invalid_status;
+	u64 htt_fcs_err;
+};
+
 struct ath10k_debug {
 	struct dentry *debugfs_phy;
 
+	struct ath10k_host_stats host_stats;
 	struct ath10k_target_stats target_stats;
 	u32 wmi_service_bitmap[WMI_SERVICE_BM_SIZE];
 
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 6b1ca2b..f771684 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -449,6 +449,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 				   struct sk_buff **head_msdu,
 				   struct sk_buff **tail_msdu)
 {
+	struct ath10k *ar = htt->ar;
 	int msdu_len, msdu_chaining = 0;
 	struct sk_buff *msdu;
 	struct htt_rx_desc *rx_desc;
@@ -489,7 +490,8 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		 * just assert for now until we have a way to recover.
 		 */
 		if (!(__le32_to_cpu(rx_desc->attention.flags)
-				& RX_ATTENTION_FLAGS_MSDU_DONE)) {
+		      & RX_ATTENTION_FLAGS_MSDU_DONE)) {
+			HOST_STAT_INC(htt_msdu_early);
 			ath10k_htt_rx_free_msdu_chain(*head_msdu);
 			*head_msdu = NULL;
 			msdu = NULL;
@@ -541,14 +543,19 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
 		}
 
 		msdu_len_invalid = !!(__le32_to_cpu(rx_desc->attention.flags)
-					& (RX_ATTENTION_FLAGS_MPDU_LENGTH_ERR |
-					   RX_ATTENTION_FLAGS_MSDU_LENGTH_ERR));
+				      & (RX_ATTENTION_FLAGS_MPDU_LENGTH_ERR |
+					 RX_ATTENTION_FLAGS_MSDU_LENGTH_ERR));
 		msdu_len = MS(__le32_to_cpu(rx_desc->msdu_start.info0),
 			      RX_MSDU_START_INFO0_MSDU_LENGTH);
 		msdu_chained = rx_desc->frag_info.ring2_more_count;
 
-		if (msdu_len_invalid)
+		if (msdu_chained)
+			HOST_STAT_INC(htt_msdu_chained);
+
+		if (msdu_len_invalid) {
+			HOST_STAT_INC(htt_msdu_len_invalid);
 			msdu_len = 0;
+		}
 
 		skb_trim(msdu, 0);
 		skb_put(msdu, min(msdu_len, HTT_RX_MSDU_SIZE));
@@ -966,6 +973,7 @@ static bool ath10k_htt_rx_has_fcs_err(struct sk_buff *skb)
 static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 				  struct htt_rx_indication *rx)
 {
+	struct ath10k *ar = htt->ar;
 	struct htt_rx_info info;
 	struct htt_rx_indication_mpdu_range *mpdu_ranges;
 	int num_mpdu_ranges;
@@ -974,6 +982,8 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 	int i, j;
 	int ret;
 
+	HOST_STAT_INC(htt_rx_ind);
+
 	memset(&info, 0, sizeof(info));
 
 	fw_desc_len = __le16_to_cpu(rx->prefix.fw_rx_desc_bytes);
@@ -1004,11 +1014,13 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 							 &msdu_tail);
 
 			if (!msdu_head) {
+				HOST_STAT_INC(htt_msdu_no_data);
 				ath10k_warn("htt rx no data!\n");
 				continue;
 			}
 
 			if (msdu_head->len == 0) {
+				HOST_STAT_INC(htt_msdu_zero_len);
 				ath10k_dbg(ATH10K_DBG_HTT,
 					   "htt rx dropping due to zero-len\n");
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
@@ -1016,6 +1028,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			}
 
 			if (ath10k_htt_rx_has_decrypt_err(msdu_head)) {
+				HOST_STAT_INC(htt_decrypt_err);
 				ath10k_htt_rx_free_msdu_chain(msdu_head);
 				continue;
 			}
@@ -1029,6 +1042,7 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			if (info.status != HTT_RX_IND_MPDU_STATUS_OK &&
 			    info.status != HTT_RX_IND_MPDU_STATUS_TKIP_MIC_ERR &&
 			    !htt->ar->monitor_enabled) {
+				HOST_STAT_INC(htt_invalid_status);
 				ath10k_dbg(ATH10K_DBG_HTT,
 					   "htt rx ignoring frame w/ status %d\n",
 					   info.status);
@@ -1053,12 +1067,16 @@ static void ath10k_htt_rx_handler(struct ath10k_htt *htt,
 			info.rate.info1 = __le32_to_cpu(rx->ppdu.info1);
 			info.rate.info2 = __le32_to_cpu(rx->ppdu.info2);
 
-			if (ath10k_htt_rx_hdr_is_amsdu(ath10k_htt_rx_skb_get_hdr(msdu_head)))
+			if (ath10k_htt_rx_hdr_is_amsdu(ath10k_htt_rx_skb_get_hdr(msdu_head))) {
+				HOST_STAT_INC(htt_amsdu);
 				ret = ath10k_htt_rx_amsdu(htt, &info);
-			else
+			} else {
+				HOST_STAT_INC(htt_msdu);
 				ret = ath10k_htt_rx_msdu(htt, &info);
+			}
 
 			if (ret && !info.fcs_err) {
+				HOST_STAT_INC(htt_fcs_err);
 				ath10k_warn("error processing msdus %d\n", ret);
 				dev_kfree_skb_any(info.skb);
 				continue;
-- 
1.8.2.2

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>

I would like to have have all struct ath10k substructures in core.h so
that they don't get too much spread around. Hence I need to drop this
patch.

-- 
Kalle Valo

[ath9k-devel] [PATCH 2/2] ath10k: Add host statistics

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> From: Sujith Manoharan <c_manoha@qca.qualcomm.com>
>
> Signed-off-by: Sujith Manoharan <c_manoha@qca.qualcomm.com>

[...]

> +static ssize_t ath10k_read_host_stats(struct file *file, char __user *user_buf,
> +				      size_t count, loff_t *ppos)
> +{

[...]

> +	if (len > buf_len)
> +		len = buf_len;

min()/min_t()?

> +#ifdef CONFIG_ATH10K_DEBUGFS
> +#define HOST_STAT_INC(c) ar->debug.host_stats.c++
> +#else
> +#define HOST_STAT_INC(c) do { } while (0)
> +#endif

Please add ATH10K_ prefix and argument for ar. Also we already have the
same ifdef for few debug functions which you can reuse, no need to add
new ifdef.

-- 
Kalle Valo

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

Kalle Valo wrote:
> I would like to have have all struct ath10k substructures in core.h so
> that they don't get too much spread around. Hence I need to drop this
> patch.

debug.h seems an appropriate place to have debug-specific structures.

Sujith

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> Kalle Valo wrote:
>> I would like to have have all struct ath10k substructures in core.h so
>> that they don't get too much spread around. Hence I need to drop this
>> patch.
>
> debug.h seems an appropriate place to have debug-specific structures.

The idea is that everything within ar variable can be found from core.h.
That way people don't need to track down where it's actually defined.

-- 
Kalle Valo

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

Kalle Valo wrote:
> The idea is that everything within ar variable can be found from core.h.
> That way people don't need to track down where it's actually defined.

That doesn't seem to be the way things are organized. "ar" contains instances of
HTT, HTC, WMI, BMI, HIF - which all have separate header files. Likewise, having
a self-contained debug header files makes things cleaner, IMO.

Sujith

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Kalle Valo]

Sujith Manoharan <sujith@msujith.org> writes:

> Kalle Valo wrote:
>> The idea is that everything within ar variable can be found from core.h.
>> That way people don't need to track down where it's actually defined.
>
> That doesn't seem to be the way things are organized. "ar" contains instances of
> HTT, HTC, WMI, BMI, HIF - which all have separate header files. Likewise, having
> a self-contained debug header files makes things cleaner, IMO.

That's what the original codebase had (separate structures for each
module), but we started moving it to the direction of having complete
struct ath10k in core.h. It's a lot easier to manage everything that
way.

-- 
Kalle Valo

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

Kalle Valo wrote:
> That's what the original codebase had (separate structures for each
> module), but we started moving it to the direction of having complete
> struct ath10k in core.h. It's a lot easier to manage everything that
> way.

I have lost interest - please drop this patch.

Sujith

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

Sujith Manoharan wrote:
> Kalle Valo wrote:
> > That's what the original codebase had (separate structures for each
> > module), but we started moving it to the direction of having complete
> > struct ath10k in core.h. It's a lot easier to manage everything that
> > way.
> 
> I have lost interest - please drop this patch.

.. and patch-5 too. Maybe I'll redo it later.

Sujith

[ath9k-devel] [PATCH 1/2] ath10k: Move debug structures to debug.h

[Sujith Manoharan]

Sujith Manoharan wrote:
> Sujith Manoharan wrote:
> > Kalle Valo wrote:
> > > That's what the original codebase had (separate structures for each
> > > module), but we started moving it to the direction of having complete
> > > struct ath10k in core.h. It's a lot easier to manage everything that
> > > way.
> > 
> > I have lost interest - please drop this patch.
> 
> .. and patch-5 too. Maybe I'll redo it later.

I'll rebase and resend the patches once my queue dealing with stupid
FW API changes are out of the way.

Sorry if I sounded cranky. :)

Sujith