[PATCH V2 1/3] ath6kl: Add initial debugfs changes

[PATCH V2 1/3] ath6kl: Add initial debugfs changes

[Vasanthakumar Thiagarajan]

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/core.h  |    1 +
 drivers/net/wireless/ath/ath6kl/debug.c |   10 ++++++++++
 drivers/net/wireless/ath/ath6kl/debug.h |    6 +++++-
 drivers/net/wireless/ath/ath6kl/init.c  |    6 ++++++
 4 files changed, 22 insertions(+), 1 deletions(-)

V2 - Remove a separate config option for debugfs. Use CONFIG_ATH6KL_DEBUG
     to enable it.

diff --git a/drivers/net/wireless/ath/ath6kl/core.h b/drivers/net/wireless/ath/ath6kl/core.h
index 4405ab5..c5537b3 100644
--- a/drivers/net/wireless/ath/ath6kl/core.h
+++ b/drivers/net/wireless/ath/ath6kl/core.h
@@ -467,6 +467,7 @@ struct ath6kl {
 	struct workqueue_struct *ath6kl_wq;
 
 	struct ath6kl_node_table scan_table;
+	struct dentry *debugfs_phy;
 };
 
 static inline void *ath6kl_priv(struct net_device *dev)
diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index 316136c..12775e8 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -147,4 +147,14 @@ void dump_cred_dist_stats(struct htc_target *target)
 		   target->cred_dist_cntxt->cur_free_credits);
 }
 
+int ath6kl_debug_init(struct ath6kl *ar)
+{
+	ar->debugfs_phy = debugfs_create_dir("ath6kl",
+					     ar->wdev->wiphy->debugfsdir);
+	if (!ar->debugfs_phy)
+		return -ENOMEM;
+
+	/* TODO: Create debugfs file entries for various target/host stats */
+	return 0;
+}
 #endif
diff --git a/drivers/net/wireless/ath/ath6kl/debug.h b/drivers/net/wireless/ath/ath6kl/debug.h
index 66b3999..e8c9ea9 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.h
+++ b/drivers/net/wireless/ath/ath6kl/debug.h
@@ -78,6 +78,7 @@ void ath6kl_dump_registers(struct ath6kl_device *dev,
 			   struct ath6kl_irq_proc_registers *irq_proc_reg,
 			   struct ath6kl_irq_enable_reg *irq_en_reg);
 void dump_cred_dist_stats(struct htc_target *target);
+int ath6kl_debug_init(struct ath6kl *ar);
 #else
 static inline int ath6kl_dbg(enum ATH6K_DEBUG_MASK dbg_mask,
 			     const char *fmt, ...)
@@ -100,6 +101,9 @@ static inline void ath6kl_dump_registers(struct ath6kl_device *dev,
 static inline void dump_cred_dist_stats(struct htc_target *target)
 {
 }
+static inline int ath6kl_debug_init(struct ath6kl *ar)
+{
+	return 0;
+}
 #endif
-
 #endif
diff --git a/drivers/net/wireless/ath/ath6kl/init.c b/drivers/net/wireless/ath/ath6kl/init.c
index 75230ac..ad9716c 100644
--- a/drivers/net/wireless/ath/ath6kl/init.c
+++ b/drivers/net/wireless/ath/ath6kl/init.c
@@ -573,6 +573,12 @@ struct ath6kl *ath6kl_core_alloc(struct device *sdev)
 	ar->wdev = wdev;
 	wdev->iftype = NL80211_IFTYPE_STATION;
 
+	if (ath6kl_debug_init(ar)) {
+		ath6kl_err("Failed to initialize debugfs\n");
+		ath6kl_cfg80211_deinit(ar);
+		return NULL;
+	}
+
 	dev = alloc_netdev(0, "wlan%d", ether_setup);
 	if (!dev) {
 		ath6kl_err("no memory for network device instance\n");
-- 
1.7.0.4

[PATCH V2 2/3] ath6kl: Add debugfs entry to dump target stats

[Vasanthakumar Thiagarajan]

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/debug.c |  147 ++++++++++++++++++++++++++++++-
 1 files changed, 146 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index 12775e8..5a082c0 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -147,6 +147,149 @@ void dump_cred_dist_stats(struct htc_target *target)
 		   target->cred_dist_cntxt->cur_free_credits);
 }
 
+static int ath6kl_debugfs_open(struct inode *inode, struct file *file)
+{
+	file->private_data = inode->i_private;
+	return 0;
+}
+
+static ssize_t read_file_tgt_stats(struct file *file, char __user *user_buf,
+				   size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	struct target_stats *tgt_stats = &ar->target_stats;
+	char *buf;
+	unsigned int len = 0, buf_len = 1500;
+	int i;
+	long left;
+	ssize_t ret_cnt;
+
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	if (down_interruptible(&ar->sem)) {
+		kfree(buf);
+		return -EBUSY;
+	}
+
+	set_bit(STATS_UPDATE_PEND, &ar->flag);
+
+	if (ath6kl_wmi_get_stats_cmd(ar->wmi)) {
+		up(&ar->sem);
+		kfree(buf);
+		return -EIO;
+	}
+
+	left = wait_event_interruptible_timeout(ar->event_wq,
+						!test_bit(STATS_UPDATE_PEND,
+						&ar->flag), WMI_TIMEOUT);
+
+	up(&ar->sem);
+
+	if (left <= 0) {
+		kfree(buf);
+		return -ETIMEDOUT;
+	}
+
+	len += scnprintf(buf + len, buf_len - len, "\n");
+	len += scnprintf(buf + len, buf_len - len, "%25s\n",
+			 "Target Tx stats");
+	len += scnprintf(buf + len, buf_len - len, "%25s\n\n",
+			 "=================");
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Ucast packets", tgt_stats->tx_ucast_pkt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Bcast packets", tgt_stats->tx_bcast_pkt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Ucast byte", tgt_stats->tx_ucast_byte);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Bcast byte", tgt_stats->tx_bcast_byte);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Rts success cnt", tgt_stats->tx_rts_success_cnt);
+	for (i = 0; i < 4; i++)
+		len += scnprintf(buf + len, buf_len - len,
+				 "%18s %d %10llu\n", "PER on ac",
+				 i, tgt_stats->tx_pkt_per_ac[i]);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Error", tgt_stats->tx_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Fail count", tgt_stats->tx_fail_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Retry count", tgt_stats->tx_retry_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Multi retry cnt", tgt_stats->tx_mult_retry_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Rts fail cnt", tgt_stats->tx_rts_fail_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%25s %10llu\n\n",
+			 "TKIP counter measure used",
+			 tgt_stats->tkip_cnter_measures_invoked);
+
+	len += scnprintf(buf + len, buf_len - len, "%25s\n",
+			 "Target Rx stats");
+	len += scnprintf(buf + len, buf_len - len, "%25s\n",
+			 "=================");
+
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Ucast packets", tgt_stats->rx_ucast_pkt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10d\n",
+			 "Ucast Rate", tgt_stats->rx_ucast_rate);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Bcast packets", tgt_stats->rx_bcast_pkt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Ucast byte", tgt_stats->rx_ucast_byte);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Bcast byte", tgt_stats->rx_bcast_byte);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Fragmented pkt", tgt_stats->rx_frgment_pkt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Error", tgt_stats->rx_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "CRC Err", tgt_stats->rx_crc_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Key chache miss", tgt_stats->rx_key_cache_miss);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Decrypt Err", tgt_stats->rx_decrypt_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Duplicate frame", tgt_stats->rx_dupl_frame);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Tkip Mic failure", tgt_stats->tkip_local_mic_fail);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "TKIP format err", tgt_stats->tkip_fmt_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "CCMP format Err", tgt_stats->ccmp_fmt_err);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n\n",
+			 "CCMP Replay Err", tgt_stats->ccmp_replays);
+
+	len += scnprintf(buf + len, buf_len - len, "%25s\n",
+			 "Misc Target stats");
+	len += scnprintf(buf + len, buf_len - len, "%25s\n",
+			 "=================");
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Beacon Miss count", tgt_stats->cs_bmiss_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Num Connects", tgt_stats->cs_connect_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10llu\n",
+			 "Num disconnects", tgt_stats->cs_discon_cnt);
+	len += scnprintf(buf + len, buf_len - len, "%20s %10d\n",
+			 "Beacon avg rssi", tgt_stats->cs_ave_beacon_rssi);
+
+	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_tgt_stats = {
+	.read = read_file_tgt_stats,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debugfs_phy = debugfs_create_dir("ath6kl",
@@ -154,7 +297,9 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	if (!ar->debugfs_phy)
 		return -ENOMEM;
 
-	/* TODO: Create debugfs file entries for various target/host stats */
+	debugfs_create_file("tgt_stats", S_IRUSR, ar->debugfs_phy, ar,
+			    &fops_tgt_stats);
+
 	return 0;
 }
 #endif
-- 
1.7.0.4

[PATCH V2 3/3] ath6kl: Add debugfs file entry to dump credit distribution stats

[Vasanthakumar Thiagarajan]

Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
---
 drivers/net/wireless/ath/ath6kl/debug.c |   65 +++++++++++++++++++++++++++++++
 1 files changed, 65 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
index 5a082c0..02fdd43 100644
--- a/drivers/net/wireless/ath/ath6kl/debug.c
+++ b/drivers/net/wireless/ath/ath6kl/debug.c
@@ -290,6 +290,68 @@ static const struct file_operations fops_tgt_stats = {
 	.llseek = default_llseek,
 };
 
+static ssize_t read_file_credit_dist_stats(struct file *file,
+					   char __user *user_buf,
+					   size_t count, loff_t *ppos)
+{
+	struct ath6kl *ar = file->private_data;
+	struct htc_target *target = ar->htc_target;
+	struct htc_endpoint_credit_dist *ep_list;
+	char *buf;
+	unsigned int buf_len = 128, len = 0;
+	ssize_t ret_cnt;
+
+	buf_len += get_queue_depth(&target->cred_dist_list) * 128;
+	buf = kzalloc(buf_len, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	len += scnprintf(buf + len, buf_len - len,
+			 " Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd"
+			 "  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist"
+			 "  qdepth\n");
+
+	list_for_each_entry(ep_list, &target->cred_dist_list, list) {
+		len += scnprintf(buf + len, buf_len - len, "  %2d",
+				 ep_list->endpoint);
+		len += scnprintf(buf + len, buf_len - len, "%10x",
+				 ep_list->dist_flags);
+		len += scnprintf(buf + len, buf_len - len, "%8d",
+				 ep_list->cred_norm);
+		len += scnprintf(buf + len, buf_len - len, "%9d",
+				 ep_list->cred_min);
+		len += scnprintf(buf + len, buf_len - len, "%9d",
+				 ep_list->credits);
+		len += scnprintf(buf + len, buf_len - len, "%10d",
+				 ep_list->cred_assngd);
+		len += scnprintf(buf + len, buf_len - len, "%13d",
+				 ep_list->seek_cred);
+		len += scnprintf(buf + len, buf_len - len, "%12d",
+				 ep_list->cred_sz);
+		len += scnprintf(buf + len, buf_len - len, "%9d",
+				 ep_list->cred_per_msg);
+		len += scnprintf(buf + len, buf_len - len, "%14d",
+				 ep_list->cred_to_dist);
+		len += scnprintf(buf + len, buf_len - len, "%12d\n",
+				 get_queue_depth(&((struct htc_endpoint *)
+						 ep_list->htc_rsvd)->txq));
+	}
+
+	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_credit_dist_stats = {
+	.read = read_file_credit_dist_stats,
+	.open = ath6kl_debugfs_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
 int ath6kl_debug_init(struct ath6kl *ar)
 {
 	ar->debugfs_phy = debugfs_create_dir("ath6kl",
@@ -300,6 +362,9 @@ int ath6kl_debug_init(struct ath6kl *ar)
 	debugfs_create_file("tgt_stats", S_IRUSR, ar->debugfs_phy, ar,
 			    &fops_tgt_stats);
 
+	debugfs_create_file("credit_dist_stats", S_IRUSR, ar->debugfs_phy, ar,
+			    &fops_credit_dist_stats);
+
 	return 0;
 }
 #endif
-- 
1.7.0.4

Re: [PATCH V2 1/3] ath6kl: Add initial debugfs changes

[Luciano Coelho]

On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote: 
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
> ---

You should always add some text to the explanation body of your patches.

-- 
Cheers,
Luca.

Re: [PATCH V2 3/3] ath6kl: Add debugfs file entry to dump credit distribution stats

[Joe Perches]

On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
[]
> diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
[]
> +static ssize_t read_file_credit_dist_stats(struct file *file,
> +					   char __user *user_buf,
> +					   size_t count, loff_t *ppos)
> +{
> +	struct ath6kl *ar = file->private_data;
> +	struct htc_target *target = ar->htc_target;
> +	struct htc_endpoint_credit_dist *ep_list;
> +	char *buf;
> +	unsigned int buf_len = 128, len = 0;
> +	ssize_t ret_cnt;
> +
> +	buf_len += get_queue_depth(&target->cred_dist_list) * 128;

Doesn't look like 128 is the right size.

> +	buf = kzalloc(buf_len, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;

kmalloc can run out of space pretty quickly if cred_dist_list
is large.  Maybe vmalloc?

> +
> +	len += scnprintf(buf + len, buf_len - len,
> +			 " Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd"
> +			 "  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist"
> +			 "  qdepth\n");
> +
> +	list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> +		len += scnprintf(buf + len, buf_len - len, "  %2d",
> +				 ep_list->endpoint);
> +		len += scnprintf(buf + len, buf_len - len, "%10x",
> +				 ep_list->dist_flags);

I think this is nearly unreadable.
It doesn't line up well with your header.

Why not align them correctly or otherwise make no
attempt to align them at all?

Your current header and field widths:

          1         2         3         4         5         6         7         8         9        10        11        12 
 123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
	len += scnprintf(buf + len, buf_len - len,
" Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist  qdepth\n");
   12          12345678         123456789          1234567890123            123456789              123456789012
     1234567890        123456789         1234567890             123456789012         12345678901234

> +	list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> +		len += scnprintf(buf + len, buf_len - len, "  %2d",
> +				 ep_list->endpoint);
> +		len += scnprintf(buf + len, buf_len - len, "%10x",
> +				 ep_list->dist_flags);

maybe use a macro:
#define ep_list_field(fmt, field)	\ 
	len += scnprint(buf + len, buf_len - len, fmt, ep_list->field)

		ep_list_field("  %2d", endpoint);
		ep_list_field("%9d", cred_min);
		ep_list_field("%9d", credits);
etc

> +		len += scnprintf(buf + len, buf_len - len, "%12d\n",
> +				 get_queue_depth(&((struct htc_endpoint *)
> +						 ep_list->htc_rsvd)->txq));
> +	}

Re: [PATCH V2 1/3] ath6kl: Add initial debugfs changes

[Vasanthakumar Thiagarajan]

On Thu, Aug 25, 2011 at 10:53:15AM +0300, Luciano Coelho wrote:
> On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote: 
> > Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
> > ---
> 
> You should always add some text to the explanation body of your patches.

Sure, thanks.

vasanth

Re: [PATCH V2 3/3] ath6kl: Add debugfs file entry to dump credit distribution stats

[Vasanthakumar Thiagarajan]

On Thu, Aug 25, 2011 at 01:26:37AM -0700, Joe Perches wrote:
> On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
> > Signed-off-by: Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com>
> []
> > diff --git a/drivers/net/wireless/ath/ath6kl/debug.c b/drivers/net/wireless/ath/ath6kl/debug.c
> []
> > +static ssize_t read_file_credit_dist_stats(struct file *file,
> > +					   char __user *user_buf,
> > +					   size_t count, loff_t *ppos)
> > +{
> > +	struct ath6kl *ar = file->private_data;
> > +	struct htc_target *target = ar->htc_target;
> > +	struct htc_endpoint_credit_dist *ep_list;
> > +	char *buf;
> > +	unsigned int buf_len = 128, len = 0;
> > +	ssize_t ret_cnt;
> > +
> > +	buf_len += get_queue_depth(&target->cred_dist_list) * 128;
> 
> Doesn't look like 128 is the right size.

It is well enough, needed is only around 110 bytes.
> 
> > +	buf = kzalloc(buf_len, GFP_KERNEL);
> > +	if (!buf)
> > +		return -ENOMEM;
> 
> kmalloc can run out of space pretty quickly if cred_dist_list
> is large.  Maybe vmalloc?
> 

Well, right now the size of cred_dist_list can be maximum of 9.
The maximum memory required would be around 2.5K, I think for 
this size kmalloc is fine.

> > +
> > +	len += scnprintf(buf + len, buf_len - len,
> > +			 " Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd"
> > +			 "  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist"
> > +			 "  qdepth\n");
> > +
> > +	list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> > +		len += scnprintf(buf + len, buf_len - len, "  %2d",
> > +				 ep_list->endpoint);
> > +		len += scnprintf(buf + len, buf_len - len, "%10x",
> > +				 ep_list->dist_flags);
> 
> I think this is nearly unreadable.
> It doesn't line up well with your header.
> 
> Why not align them correctly or otherwise make no
> attempt to align them at all?

Right, totally unaligned is not readable either ;). Ok, the number
of digits that needed to represent the number of credits are not
going to exceed 3. I just tried to have it displayed around at the center
below the respective tag.

> 
> Your current header and field widths:
> 
>           1         2         3         4         5         6         7         8         9        10        11        12 
>  123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890
> 	len += scnprintf(buf + len, buf_len - len,
> " Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist  qdepth\n");
>    12          12345678         123456789          1234567890123            123456789              123456789012
>      1234567890        123456789         1234567890             123456789012         12345678901234
> 

It is very unlikely to have an output like above in real time.
Sample output that I got is


Epid  Flags    Cred_norm  Cred_min  Credits  Cred_assngd  Seek_cred  Cred_sz  Cred_per_msg  Cred_to_dist  qdepth
   0         0       0        0        0         0            0        1664        1             0           0
   1  80000000       1        1        1         1            0        1664        1             0           0
   5         0       5        1        0         0            0        1664        1             0           0
   4         0       5        1        0         0            0        1664        1             0           0
   2  80000000       5        1        2         3            0        1664        1             0           0
   3  80000000       5        1        1         1            0        1664        1             0           0

> > +	list_for_each_entry(ep_list, &target->cred_dist_list, list) {
> > +		len += scnprintf(buf + len, buf_len - len, "  %2d",
> > +				 ep_list->endpoint);
> > +		len += scnprintf(buf + len, buf_len - len, "%10x",
> > +				 ep_list->dist_flags);
> 
> maybe use a macro:
> #define ep_list_field(fmt, field)	\ 
> 	len += scnprint(buf + len, buf_len - len, fmt, ep_list->field)
> 
> 		ep_list_field("  %2d", endpoint);
> 		ep_list_field("%9d", cred_min);
> 		ep_list_field("%9d", credits);
> etc

Makes sense.

Thanks for the review

Vasanth

Re: [PATCH V2 3/3] ath6kl: Add debugfs file entry to dump credit distribution stats

[Kalle Valo]

Vasanthakumar Thiagarajan <vthiagar@qca.qualcomm.com> writes:

> On Thu, Aug 25, 2011 at 01:26:37AM -0700, Joe Perches wrote:
>> On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
>> > +	struct ath6kl *ar = file->private_data;
>> > +	struct htc_target *target = ar->htc_target;
>> > +	struct htc_endpoint_credit_dist *ep_list;
>> > +	char *buf;
>> > +	unsigned int buf_len = 128, len = 0;
>> > +	ssize_t ret_cnt;
>> > +
>> > +	buf_len += get_queue_depth(&target->cred_dist_list) * 128;
>> 
>> Doesn't look like 128 is the right size.
>
> It is well enough, needed is only around 110 bytes.

Proper defines for the values would be nice. Magic numbers are always
tricky.

-- 
Kalle Valo