Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
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
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 <[email protected]>
> > ---
>
> You should always add some text to the explanation body of your patches.
Sure, thanks.
vasanth
Vasanthakumar Thiagarajan <[email protected]> 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
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
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
On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
> ---
You should always add some text to the explanation body of your patches.
--
Cheers,
Luca.
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 <[email protected]>
> []
> > 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
On Thu, 2011-08-25 at 13:00 +0530, Vasanthakumar Thiagarajan wrote:
> Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
[]
> 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));
> + }
Signed-off-by: Vasanthakumar Thiagarajan <[email protected]>
---
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