From: "Luis R. Rodriguez" <rodrigue@qca.qualcomm.com> To: Zefir Kurtisi <zefir.kurtisi@neratec.com> Cc: linux-wireless@vger.kernel.org, ath9k-devel@lists.ath9k.org, kgiori@qca.qualcomm.com, nbd@openwrt.org Subject: Re: [RFC 1/6] ath9k: add DFS statistics to debugfs Date: Mon, 3 Oct 2011 11:14:54 -0700 [thread overview] Message-ID: <CAB=NE6UUspgQtFeVr+dfFtHEHxW6wMue+PS1RrwmJO1rqroW7A@mail.gmail.com> (raw) In-Reply-To: <1317637758-11907-2-git-send-email-zefir.kurtisi@neratec.com> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote: > > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com> > --- > drivers/net/wireless/ath/ath9k/debug.c | 51 ++++++++++++++++++++++++++++++++ > drivers/net/wireless/ath/ath9k/debug.h | 29 ++++++++++++++++++ > 2 files changed, 80 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c > index cdece82..6ad2266 100644 > --- a/drivers/net/wireless/ath/ath9k/debug.c > +++ b/drivers/net/wireless/ath/ath9k/debug.c > @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = { > > #endif > > + > +#ifdef CONFIG_ATH9K_DFS This kconfig entry doesn't exist yet, so no point in referring to it yet, you can add a patch that adds this but describe it as work in progress or something like that and later correct the text as you move on. > + > +#define DFS_STAT(s, p) \ > + len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \ > + sc->debug.stats.dfs_stats.p); > + Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage. > +static ssize_t read_file_dfs(struct file *file, char __user *user_buf, > + size_t count, loff_t *ppos) > +{ > + struct ath_softc *sc = file->private_data; > + char *buf; > + unsigned int len = 0, size = 8000; > + ssize_t retval = 0; > + > + buf = kzalloc(size, GFP_KERNEL); > + if (buf == NULL) > + return -ENOMEM; > + > + DFS_STAT("DFS pulses detected ", pulses_detected); > + DFS_STAT("Datalen discards ", datalen_discards); > + DFS_STAT("RSSI discards ", rssi_discards); > + DFS_STAT("BW info discards ", bwinfo_discards); > + DFS_STAT("Primary channel pulses ", pri_phy_errors); > + DFS_STAT("Secondary channel pulses", ext_phy_errors); > + DFS_STAT("Dual channel pulses ", dc_phy_errors); > + > + if (len > size) > + len = size; > + > + retval = simple_read_from_buffer(user_buf, count, ppos, buf, len); > + kfree(buf); > + > + return retval; > +} > + > + > +static const struct file_operations fops_dfs_stats = { > + .read = read_file_dfs, > + .open = ath9k_debugfs_open, > + .owner = THIS_MODULE, > + .llseek = default_llseek, > +}; > +#endif /* CONFIG_ATH9K_DFS */ I'd prefer to keep this apart into a debugfs_dfs.c but that's just me, I would like to ensure to keep *all* DFS stat as easy to review as possible and am not a fan of the ifdef sprinkle. > + > + > #define DMA_BUF_LEN 1024 > > static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf, > @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah) > debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy, > &sc->chan_bw); > > +#ifdef CONFIG_ATH9K_DFS > + debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc, > + &fops_dfs_stats); > +#endif If you stuff the code into a file here you'd just need a caller to ath9k_debugfs_dfs_create() or something like that. > sc->debug.regidx = 0; > return 0; > } > diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h > index 4a04510..3a3c3b7 100644 > --- a/drivers/net/wireless/ath/ath9k/debug.h > +++ b/drivers/net/wireless/ath/ath9k/debug.h > @@ -25,8 +25,10 @@ struct ath_buf; > > #ifdef CONFIG_ATH9K_DEBUGFS > #define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++ > +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++) > #else > #define TX_STAT_INC(q, c) do { } while (0) > +#define DFS_STAT_INC(sc, c) do { } while (0) Who's using this? If no one, then why add it? That is add it only when its in code. > #endif > > #ifdef CONFIG_ATH9K_DEBUGFS > @@ -171,10 +173,37 @@ struct ath_rx_stats { > u8 rs_antenna; > }; > > +#ifdef CONFIG_ATH9K_DFS > +/** > + * struct ath_dfs_stats - DFS Statistics > + * > + * @pulses_detected: No. of pulses detected so far > + * @datalen_discards: No. of pulses discarded due to invalid datalen > + * @rssi_discards: No. of pulses discarded due to invalid RSSI > + * @bwinfo_discards: No. of pulses discarded due to invalid BW info > + * @pri_phy_errors: No. of pulses reported for primary channel > + * @ext_phy_errors: No. of pulses reported for extension channel > + * @dc_phy_errors: No. of pulses reported for primary + extension channel > + */ > +struct ath_dfs_stats { > + u32 pulses_detected; > + u32 datalen_discards; > + u32 rssi_discards; > + u32 bwinfo_discards; > + u32 pri_phy_errors; > + u32 ext_phy_errors; > + u32 dc_phy_errors; > +}; > +#endif > + > + > struct ath_stats { > struct ath_interrupt_stats istats; > struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES]; > struct ath_rx_stats rxstats; > +#ifdef CONFIG_ATH9K_DFS > + struct ath_dfs_stats dfs_stats; > +#endif If code used this set of stats this would give a compile error if CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS and not CONFIG_ATH9K_DFS, but again, no one uses this code yet. Luis
WARNING: multiple messages have this Message-ID (diff)
From: Luis R. Rodriguez <rodrigue@qca.qualcomm.com> To: ath9k-devel@lists.ath9k.org Subject: [ath9k-devel] [RFC 1/6] ath9k: add DFS statistics to debugfs Date: Mon, 3 Oct 2011 11:14:54 -0700 [thread overview] Message-ID: <CAB=NE6UUspgQtFeVr+dfFtHEHxW6wMue+PS1RrwmJO1rqroW7A@mail.gmail.com> (raw) In-Reply-To: <1317637758-11907-2-git-send-email-zefir.kurtisi@neratec.com> On Mon, Oct 3, 2011 at 3:29 AM, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote: > > Signed-off-by: Zefir Kurtisi <zefir.kurtisi@neratec.com> > --- > ?drivers/net/wireless/ath/ath9k/debug.c | ? 51 ++++++++++++++++++++++++++++++++ > ?drivers/net/wireless/ath/ath9k/debug.h | ? 29 ++++++++++++++++++ > ?2 files changed, 80 insertions(+), 0 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c > index cdece82..6ad2266 100644 > --- a/drivers/net/wireless/ath/ath9k/debug.c > +++ b/drivers/net/wireless/ath/ath9k/debug.c > @@ -89,6 +89,53 @@ static const struct file_operations fops_debug = { > > ?#endif > > + > +#ifdef CONFIG_ATH9K_DFS This kconfig entry doesn't exist yet, so no point in referring to it yet, you can add a patch that adds this but describe it as work in progress or something like that and later correct the text as you move on. > + > +#define DFS_STAT(s, p) \ > + ? ? ? len += snprintf(buf + len, size - len, "%28s : %10u\n", s, \ > + ? ? ? ? ? ? ? ? ? ? ? sc->debug.stats.dfs_stats.p); > + Either rename DFS_STAT to ATH9K_DFS_STAT or undef DFS_STAT after its usage. > +static ssize_t read_file_dfs(struct file *file, char __user *user_buf, > + ? ? ? ? ? ? ? ? ? ? ? ? ? ?size_t count, loff_t *ppos) > +{ > + ? ? ? struct ath_softc *sc = file->private_data; > + ? ? ? char *buf; > + ? ? ? unsigned int len = 0, size = 8000; > + ? ? ? ssize_t retval = 0; > + > + ? ? ? buf = kzalloc(size, GFP_KERNEL); > + ? ? ? if (buf == NULL) > + ? ? ? ? ? ? ? return -ENOMEM; > + > + ? ? ? DFS_STAT("DFS pulses detected ? ? ", pulses_detected); > + ? ? ? DFS_STAT("Datalen discards ? ? ? ?", datalen_discards); > + ? ? ? DFS_STAT("RSSI discards ? ? ? ? ? ", rssi_discards); > + ? ? ? DFS_STAT("BW info discards ? ? ? ?", bwinfo_discards); > + ? ? ? DFS_STAT("Primary channel pulses ?", pri_phy_errors); > + ? ? ? DFS_STAT("Secondary channel pulses", ext_phy_errors); > + ? ? ? DFS_STAT("Dual channel pulses ? ? ", dc_phy_errors); > + > + ? ? ? if (len > size) > + ? ? ? ? ? ? ? len = size; > + > + ? ? ? retval = simple_read_from_buffer(user_buf, count, ppos, buf, len); > + ? ? ? kfree(buf); > + > + ? ? ? return retval; > +} > + > + > +static const struct file_operations fops_dfs_stats = { > + ? ? ? .read = read_file_dfs, > + ? ? ? .open = ath9k_debugfs_open, > + ? ? ? .owner = THIS_MODULE, > + ? ? ? .llseek = default_llseek, > +}; > +#endif /* CONFIG_ATH9K_DFS */ I'd prefer to keep this apart into a debugfs_dfs.c but that's just me, I would like to ensure to keep *all* DFS stat as easy to review as possible and am not a fan of the ifdef sprinkle. > + > + > ?#define DMA_BUF_LEN 1024 > > ?static ssize_t read_file_tx_chainmask(struct file *file, char __user *user_buf, > @@ -1385,6 +1432,10 @@ int ath9k_init_debug(struct ath_hw *ah) > ? ? ? ?debugfs_create_u32("chanbw", S_IRUSR | S_IWUSR, sc->debug.debugfs_phy, > ? ? ? ? ? ? ? ? ? ? ? ? ? &sc->chan_bw); > > +#ifdef CONFIG_ATH9K_DFS > + ? ? ? debugfs_create_file("dfs_stats", S_IRUSR, sc->debug.debugfs_phy, sc, > + ? ? ? ? ? ? ? ? ? ? ? ? ? &fops_dfs_stats); > +#endif If you stuff the code into a file here you'd just need a caller to ath9k_debugfs_dfs_create() or something like that. > ? ? ? ?sc->debug.regidx = 0; > ? ? ? ?return 0; > ?} > diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h > index 4a04510..3a3c3b7 100644 > --- a/drivers/net/wireless/ath/ath9k/debug.h > +++ b/drivers/net/wireless/ath/ath9k/debug.h > @@ -25,8 +25,10 @@ struct ath_buf; > > ?#ifdef CONFIG_ATH9K_DEBUGFS > ?#define TX_STAT_INC(q, c) sc->debug.stats.txstats[q].c++ > +#define DFS_STAT_INC(sc, c) (sc->debug.stats.dfs_stats.c++) > ?#else > ?#define TX_STAT_INC(q, c) do { } while (0) > +#define DFS_STAT_INC(sc, c) do { } while (0) Who's using this? If no one, then why add it? That is add it only when its in code. > ?#endif > > ?#ifdef CONFIG_ATH9K_DEBUGFS > @@ -171,10 +173,37 @@ struct ath_rx_stats { > ? ? ? ?u8 rs_antenna; > ?}; > > +#ifdef CONFIG_ATH9K_DFS > +/** > + * struct ath_dfs_stats - DFS Statistics > + * > + * @pulses_detected: No. of pulses detected so far > + * @datalen_discards: ?No. of pulses discarded due to invalid datalen > + * @rssi_discards: No. of pulses discarded due to invalid RSSI > + * @bwinfo_discards: No. of pulses discarded due to invalid BW info > + * @pri_phy_errors: No. of pulses reported for primary channel > + * @ext_phy_errors: No. of pulses reported for extension channel > + * @dc_phy_errors: No. of pulses reported for primary + extension channel > + */ > +struct ath_dfs_stats { > + ? ? ? u32 pulses_detected; > + ? ? ? u32 datalen_discards; > + ? ? ? u32 rssi_discards; > + ? ? ? u32 bwinfo_discards; > + ? ? ? u32 pri_phy_errors; > + ? ? ? u32 ext_phy_errors; > + ? ? ? u32 dc_phy_errors; > +}; > +#endif > + > + > ?struct ath_stats { > ? ? ? ?struct ath_interrupt_stats istats; > ? ? ? ?struct ath_tx_stats txstats[ATH9K_NUM_TX_QUEUES]; > ? ? ? ?struct ath_rx_stats rxstats; > +#ifdef CONFIG_ATH9K_DFS > + ? ? ? struct ath_dfs_stats dfs_stats; > +#endif If code used this set of stats this would give a compile error if CONFIG_ATH9K_DEBUGFS is enabled but CONFIG_ATH9K_DFS was disabled as the ifdef over the increment stat stuff is over CONFIG_ATH9K_DEBUGFS and not CONFIG_ATH9K_DFS, but again, no one uses this code yet. Luis
next prev parent reply other threads:[~2011-10-03 18:29 UTC|newest] Thread overview: 110+ messages / expand[flat|nested] mbox.gz Atom feed top 2011-10-03 10:29 [RFC 0/6] ath9k: DFS pattern detection Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 10:29 ` [RFC 1/6] ath9k: add DFS statistics to debugfs Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 18:14 ` Luis R. Rodriguez [this message] 2011-10-03 18:14 ` Luis R. Rodriguez 2011-10-04 8:27 ` Zefir Kurtisi 2011-10-04 8:27 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 10:29 ` [RFC 2/6] ath9k: add DFS debug flag Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 18:15 ` Luis R. Rodriguez 2011-10-03 18:15 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-04 8:31 ` Zefir Kurtisi 2011-10-04 8:31 ` [ath9k-devel] " Zefir Kurtisi 2011-10-04 9:40 ` Mohammed Shafi 2011-10-04 9:40 ` [ath9k-devel] " Mohammed Shafi 2011-10-03 10:29 ` [RFC 3/6] ath9k: initial radar pulse detection for DFS Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 11:57 ` Adrian Chadd 2011-10-03 11:57 ` [ath9k-devel] " Adrian Chadd 2011-10-03 12:23 ` Zefir Kurtisi 2011-10-03 12:23 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 12:43 ` Adrian Chadd 2011-10-03 12:43 ` [ath9k-devel] " Adrian Chadd 2011-10-03 14:21 ` Zefir Kurtisi 2011-10-03 14:21 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 14:23 ` Adrian Chadd 2011-10-03 14:23 ` [ath9k-devel] " Adrian Chadd 2011-10-03 10:29 ` [RFC 4/6] ath9k: add DFS build parameter Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 18:26 ` Luis R. Rodriguez 2011-10-03 18:26 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-04 9:55 ` Zefir Kurtisi 2011-10-04 9:55 ` [ath9k-devel] " Zefir Kurtisi 2011-10-04 10:37 ` Felix Fietkau 2011-10-04 10:37 ` [ath9k-devel] " Felix Fietkau 2011-10-04 12:25 ` Adrian Chadd 2011-10-04 12:25 ` [ath9k-devel] " Adrian Chadd 2011-10-05 22:20 ` Luis R. Rodriguez 2011-10-05 22:20 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-03 10:29 ` [RFC 5/6] ath9k: enable DFS pulse detection Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 18:27 ` Luis R. Rodriguez 2011-10-03 18:27 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-03 19:24 ` Christian Lamparter 2011-10-03 19:24 ` [ath9k-devel] " Christian Lamparter 2011-10-03 19:31 ` Luis R. Rodriguez 2011-10-03 19:31 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-04 13:38 ` Christian Lamparter 2011-10-04 13:38 ` [ath9k-devel] " Christian Lamparter 2011-10-04 14:17 ` Zefir Kurtisi 2011-10-04 14:17 ` [ath9k-devel] " Zefir Kurtisi 2011-10-04 14:34 ` Adrian Chadd 2011-10-04 14:34 ` [ath9k-devel] " Adrian Chadd 2011-10-05 22:31 ` Luis R. Rodriguez 2011-10-05 22:31 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-05 22:53 ` Peter Stuge 2011-10-05 23:02 ` Luis R. Rodriguez 2011-10-04 14:42 ` Christian Lamparter 2011-10-04 14:42 ` [ath9k-devel] " Christian Lamparter 2011-10-04 14:50 ` Adrian Chadd 2011-10-04 14:50 ` [ath9k-devel] " Adrian Chadd 2011-10-04 15:26 ` Christian Lamparter 2011-10-04 15:26 ` [ath9k-devel] " Christian Lamparter 2011-10-04 15:57 ` Adrian Chadd 2011-10-04 15:57 ` [ath9k-devel] " Adrian Chadd 2011-10-04 16:42 ` Christian Lamparter 2011-10-04 16:42 ` [ath9k-devel] " Christian Lamparter 2011-10-04 17:03 ` Adrian Chadd 2011-10-04 17:03 ` [ath9k-devel] " Adrian Chadd 2011-10-04 17:49 ` Christian Lamparter 2011-10-04 17:49 ` [ath9k-devel] " Christian Lamparter 2011-10-05 22:37 ` Luis R. Rodriguez 2011-10-05 22:37 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-04 16:26 ` Zefir Kurtisi 2011-10-04 16:26 ` [ath9k-devel] " Zefir Kurtisi 2011-10-05 22:30 ` Luis R. Rodriguez 2011-10-05 22:30 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-05 22:27 ` Luis R. Rodriguez 2011-10-05 22:27 ` Luis R. Rodriguez 2011-10-06 16:49 ` Christian Lamparter 2011-10-06 16:49 ` Christian Lamparter 2011-10-06 18:36 ` Luis R. Rodriguez 2011-10-06 18:36 ` Luis R. Rodriguez 2011-10-06 18:41 ` Luis R. Rodriguez 2011-10-06 18:41 ` Luis R. Rodriguez 2011-10-06 20:32 ` Zefir Kurtisi 2011-10-06 20:32 ` Zefir Kurtisi 2011-10-06 20:41 ` Luis R. Rodriguez 2011-10-06 20:41 ` Luis R. Rodriguez 2011-10-06 21:08 ` Zefir Kurtisi 2011-10-06 21:08 ` Zefir Kurtisi 2011-10-06 21:12 ` Luis R. Rodriguez 2011-10-06 21:12 ` Luis R. Rodriguez 2011-10-07 3:06 ` Adrian Chadd 2011-10-07 3:06 ` Adrian Chadd 2011-10-07 7:54 ` Luis R. Rodriguez 2011-10-07 7:54 ` Luis R. Rodriguez 2011-10-07 8:48 ` Zefir Kurtisi 2011-10-07 8:48 ` Zefir Kurtisi 2011-10-07 11:43 ` Adrian Chadd 2011-10-07 11:43 ` Adrian Chadd 2011-10-04 10:11 ` Zefir Kurtisi 2011-10-04 10:11 ` [ath9k-devel] " Zefir Kurtisi 2011-10-05 22:23 ` Luis R. Rodriguez 2011-10-05 22:23 ` [ath9k-devel] " Luis R. Rodriguez 2011-10-03 10:29 ` [RFC 6/6] ath9k: handle pulse data reported by DFS HW Zefir Kurtisi 2011-10-03 10:29 ` [ath9k-devel] " Zefir Kurtisi 2011-10-03 18:30 ` Luis R. Rodriguez 2011-10-03 18:30 ` [ath9k-devel] " Luis R. Rodriguez
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAB=NE6UUspgQtFeVr+dfFtHEHxW6wMue+PS1RrwmJO1rqroW7A@mail.gmail.com' \ --to=rodrigue@qca.qualcomm.com \ --cc=ath9k-devel@lists.ath9k.org \ --cc=kgiori@qca.qualcomm.com \ --cc=linux-wireless@vger.kernel.org \ --cc=nbd@openwrt.org \ --cc=zefir.kurtisi@neratec.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.