Re: [PATCH v2] ath9k: Gather and report IRQ sync_cause errors.

[Ben Greear <greearb@candelatech.com>]

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