From: Ben Greear <greearb@candelatech.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH v2] ath9k: Gather and report IRQ sync_cause errors.
Date: Wed, 04 Apr 2012 11:25:51 -0700 [thread overview]
Message-ID: <4F7C922F.7060201@candelatech.com> (raw)
In-Reply-To: <1333556182.16978.9.camel@joe2Laptop>
On 04/04/2012 09:16 AM, Joe Perches wrote:
> On Tue, 2012-04-03 at 21:20 -0700, greearb at candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> Report all defined sync_cause errors in debugfs
>> to aid with debugging.
>
> just trivia:
>
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> []
>> @@ -385,63 +385,130 @@ static ssize_t read_file_interrupt(struct file *file, char __user *user_buf,
>> size_t count, loff_t *ppos)
>> {
>> struct ath_softc *sc = file->private_data;
> []
>> if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_EDMA) {
> []
>> + len += snprintf(buf + len, mxlen - len,
>> + "%21s: %10u\n", "RXLP", sc->debug.stats.istats.rxlp);
>
> Alignment is overrated.
>
> I wouldn't change any of the original block
> though perhaps changing the sc pointer to an
> istats pointer so these dereferences become
> similar to istats->rxlp;
Other files in this driver are aligned, so it seems
nice to continue the tradition. debugfs files are
for human consumption, so making them readable is
a virtue.
If someone wants more efficient access to this data,
then we can use ethtool stats (I'll add ethtool stats
support for these counters if/when my first ethtool
patches make it in...)
>> +void ath9k_debug_sync_cause(struct ath_common *common, u32 sync_cause)
>> +{
>> + struct ath_softc *sc = common->priv;
>> + if (sync_cause)
>> + sc->debug.stats.istats.sync_cause_all++;
>> + if (sync_cause& AR_INTR_SYNC_RTC_IRQ)
>> + sc->debug.stats.istats.sync_rtc_irq++;
>
> And nicer here to use a pointer to istats too.
Hopefully the compiler is smart enough that this
doesn't really matter.
If the maintainers care, I'll respin the patch, but
otherwise I'm not sure it's worth the effort.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
WARNING: multiple messages have this Message-ID (diff)
From: Ben Greear <greearb@candelatech.com>
To: Joe Perches <joe@perches.com>
Cc: linux-wireless@vger.kernel.org, ath9k-devel@venema.h4ckr.net
Subject: Re: [PATCH v2] ath9k: Gather and report IRQ sync_cause errors.
Date: Wed, 04 Apr 2012 11:25:51 -0700 [thread overview]
Message-ID: <4F7C922F.7060201@candelatech.com> (raw)
In-Reply-To: <1333556182.16978.9.camel@joe2Laptop>
On 04/04/2012 09:16 AM, Joe Perches wrote:
> On Tue, 2012-04-03 at 21:20 -0700, greearb@candelatech.com wrote:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> Report all defined sync_cause errors in debugfs
>> to aid with debugging.
>
> just trivia:
>
>> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> []
>> @@ -385,63 +385,130 @@ static ssize_t read_file_interrupt(struct file *file, char __user *user_buf,
>> size_t count, loff_t *ppos)
>> {
>> struct ath_softc *sc = file->private_data;
> []
>> if (sc->sc_ah->caps.hw_caps& ATH9K_HW_CAP_EDMA) {
> []
>> + len += snprintf(buf + len, mxlen - len,
>> + "%21s: %10u\n", "RXLP", sc->debug.stats.istats.rxlp);
>
> Alignment is overrated.
>
> I wouldn't change any of the original block
> though perhaps changing the sc pointer to an
> istats pointer so these dereferences become
> similar to istats->rxlp;
Other files in this driver are aligned, so it seems
nice to continue the tradition. debugfs files are
for human consumption, so making them readable is
a virtue.
If someone wants more efficient access to this data,
then we can use ethtool stats (I'll add ethtool stats
support for these counters if/when my first ethtool
patches make it in...)
>> +void ath9k_debug_sync_cause(struct ath_common *common, u32 sync_cause)
>> +{
>> + struct ath_softc *sc = common->priv;
>> + if (sync_cause)
>> + sc->debug.stats.istats.sync_cause_all++;
>> + if (sync_cause& AR_INTR_SYNC_RTC_IRQ)
>> + sc->debug.stats.istats.sync_rtc_irq++;
>
> And nicer here to use a pointer to istats too.
Hopefully the compiler is smart enough that this
doesn't really matter.
If the maintainers care, I'll respin the patch, but
otherwise I'm not sure it's worth the effort.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
next prev parent reply other threads:[~2012-04-04 18:25 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-04-04 4:20 [ath9k-devel] [PATCH v2] ath9k: Gather and report IRQ sync_cause errors greearb at candelatech.com
2012-04-04 4:20 ` greearb
2012-04-04 16:16 ` [ath9k-devel] " Joe Perches
2012-04-04 16:16 ` Joe Perches
2012-04-04 18:25 ` Ben Greear [this message]
2012-04-04 18:25 ` Ben Greear
2012-04-04 18:51 ` [ath9k-devel] " Joe Perches
2012-04-04 18:51 ` Joe Perches
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=4F7C922F.7060201@candelatech.com \
--to=greearb@candelatech.com \
--cc=ath9k-devel@lists.ath9k.org \
/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: link
Be 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.