ath9k: Merge wiphy and misc debugfs files
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
This patch merges the 'wiphy' and 'misc' debugfs files and consolidates the information.
Information about the current channel and other HT parameters can be obtained from both mac80211 and iw. Remove such redundant data.
The reset statistics have been removed, they will be re-added in a subsequent patch (in a new debugfs file).
--- drivers/net/wireless/ath/ath9k/debug.c | 224 +++++++++----------------------- 1 files changed, 64 insertions(+), 160 deletions(-)
drivers/net/wireless/ath/ath9k/debug.c +64 -160
@@ -451,109 +451,6 @@ static const struct file_operations fops_interrupt = { .llseek = default_llseek, }; -static const char *channel_type_str (enum nl80211_channel_type t) -{ -switch (t) { -case NL80211_CHAN_NO_HT : -return "no ht" ; -case NL80211_CHAN_HT20 : -return "ht20" ; -case NL80211_CHAN_HT40MINUS : -return "ht40-" ; -case NL80211_CHAN_HT40PLUS : -return "ht40+" ; -default : -return "???" ; - } -} - -static ssize_t read_file_wiphy (struct file *file,char __user *user_buf, -size_t count,loff_t *ppos) -{ -struct ath_softc *sc = file->private_data; -struct ieee80211_channel *chan = sc->hw->conf.channel; -struct ieee80211_conf *conf = &(sc->hw->conf); -char buf[512 ]; -unsigned int len =0 ; -u8 addr[ETH_ALEN ]; -u32 tmp; - - len +=snprintf (buf + len,sizeof (buf) - len, -"%s (chan=%d center-freq: %d MHz channel-type: %d (%s))\n" , -wiphy_name (sc->hw->wiphy), -ieee80211_frequency_to_channel (chan->center_freq), - chan->center_freq, - conf->channel_type, -channel_type_str (conf->channel_type)); - -ath9k_ps_wakeup (sc); -put_unaligned_le32 (REG_READ_D (sc->sc_ah,AR_STA_ID0 ), addr); -put_unaligned_le16 (REG_READ_D (sc->sc_ah,AR_STA_ID1 ) &0xffff , addr +4 ); - len +=snprintf (buf + len,sizeof (buf) - len, -"addr: %pM\n" , addr); -put_unaligned_le32 (REG_READ_D (sc->sc_ah,AR_BSSMSKL ), addr); -put_unaligned_le16 (REG_READ_D (sc->sc_ah,AR_BSSMSKU ) &0xffff , addr +4 ); - len +=snprintf (buf + len,sizeof (buf) - len, -"addrmask: %pM\n" , addr); - tmp =ath9k_hw_getrxfilter (sc->sc_ah); -ath9k_ps_restore (sc); - len +=snprintf (buf + len,sizeof (buf) - len, -"rfilt: 0x%x" , tmp); -if (tmp &ATH9K_RX_FILTER_UCAST ) - len +=snprintf (buf + len,sizeof (buf) - len," UCAST" ); -if (tmp &ATH9K_RX_FILTER_MCAST ) - len +=snprintf (buf + len,sizeof (buf) - len," MCAST" ); -if (tmp &ATH9K_RX_FILTER_BCAST ) - len +=snprintf (buf + len,sizeof (buf) - len," BCAST" ); -if (tmp &ATH9K_RX_FILTER_CONTROL ) - len +=snprintf (buf + len,sizeof (buf) - len," CONTROL" ); -if (tmp &ATH9K_RX_FILTER_BEACON ) - len +=snprintf (buf + len,sizeof (buf) - len," BEACON" ); -if (tmp &ATH9K_RX_FILTER_PROM ) - len +=snprintf (buf + len,sizeof (buf) - len," PROM" ); -if (tmp &ATH9K_RX_FILTER_PROBEREQ ) - len +=snprintf (buf + len,sizeof (buf) - len," PROBEREQ" ); -if (tmp &ATH9K_RX_FILTER_PHYERR ) - len +=snprintf (buf + len,sizeof (buf) - len," PHYERR" ); -if (tmp &ATH9K_RX_FILTER_MYBEACON ) - len +=snprintf (buf + len,sizeof (buf) - len," MYBEACON" ); -if (tmp &ATH9K_RX_FILTER_COMP_BAR ) - len +=snprintf (buf + len,sizeof (buf) - len," COMP_BAR" ); -if (tmp &ATH9K_RX_FILTER_PSPOLL ) - len +=snprintf (buf + len,sizeof (buf) - len," PSPOLL" ); -if (tmp &ATH9K_RX_FILTER_PHYRADAR ) - len +=snprintf (buf + len,sizeof (buf) - len," PHYRADAR" ); -if (tmp &ATH9K_RX_FILTER_MCAST_BCAST_ALL ) - len +=snprintf (buf + len,sizeof (buf) - len," MCAST_BCAST_ALL" ); - - len +=snprintf (buf + len,sizeof (buf) - len, -"\n\nReset causes:\n" -" baseband hang: %d\n" -" baseband watchdog: %d\n" -" fatal hardware error interrupt: %d\n" -" tx hardware error: %d\n" -" tx path hang: %d\n" -" pll rx hang: %d\n" , - sc->debug.stats.reset[RESET_TYPE_BB_HANG ], - sc->debug.stats.reset[RESET_TYPE_BB_WATCHDOG ], - sc->debug.stats.reset[RESET_TYPE_FATAL_INT ], - sc->debug.stats.reset[RESET_TYPE_TX_ERROR ], - sc->debug.stats.reset[RESET_TYPE_TX_HANG ], - sc->debug.stats.reset[RESET_TYPE_PLL_HANG ]); - -if (len >sizeof (buf)) - len =sizeof (buf); - -return simple_read_from_buffer (user_buf, count, ppos, buf, len); -} - -static const struct file_operations fops_wiphy = { - .read = read_file_wiphy, - .open = ath9k_debugfs_open, - .owner =THIS_MODULE , - .llseek = default_llseek, -}; -#define PR_QNUM (_n) sc->tx.txq_map[_n]->axq_qnum#define PR (str, elem) \ do { \
@@ -763,84 +660,93 @@ static ssize_t read_file_misc(struct file *file, char __user *user_buf, {struct ath_softc *sc = file->private_data;struct ath_common *common =ath9k_hw_common (sc->sc_ah); -struct ath_hw *ah = sc->sc_ah;struct ieee80211_hw *hw = sc->hw; -char *buf; -unsigned int len =0 , size =8000 ; +struct ath9k_vif_iter_data iter_data; +char buf[512 ]; +unsigned int len =0 ;ssize_t retval =0 ;unsigned int reg; -struct ath9k_vif_iter_data iter_data; +u32 rxfilter; -ath9k_calculate_iter_data (hw,NULL , &iter_data); - - buf =kzalloc (size,GFP_KERNEL ); -if (buf ==NULL ) -return -ENOMEM ; + len +=snprintf (buf + len,sizeof (buf) - len, +"BSSID: %pM\n" , common->curbssid); + len +=snprintf (buf + len,sizeof (buf) - len, +"BSSID-MASK: %pM\n" , common->bssidmask); + len +=snprintf (buf + len,sizeof (buf) - len, +"OPMODE: %s\n" ,ath_opmode_to_string (sc->sc_ah->opmode));ath9k_ps_wakeup (sc); - len +=snprintf (buf + len, size - len, -"curbssid: %pM\n" -"OP-Mode: %s(%i)\n" -"Beacon-Timer-Register: 0x%x\n" , - common->curbssid, -ath_opmode_to_string (sc->sc_ah->opmode), - (int )(sc->sc_ah->opmode), -REG_READ (ah,AR_BEACON_PERIOD )); - - reg =REG_READ (ah,AR_TIMER_MODE ); + rxfilter =ath9k_hw_getrxfilter (sc->sc_ah);ath9k_ps_restore (sc); - len +=snprintf (buf + len, size - len,"Timer-Mode-Register: 0x%x (" , - reg); -if (reg &AR_TBTT_TIMER_EN ) - len +=snprintf (buf + len, size - len,"TBTT " ); -if (reg &AR_DBA_TIMER_EN ) - len +=snprintf (buf + len, size - len,"DBA " ); -if (reg &AR_SWBA_TIMER_EN ) - len +=snprintf (buf + len, size - len,"SWBA " ); -if (reg &AR_HCF_TIMER_EN ) - len +=snprintf (buf + len, size - len,"HCF " ); -if (reg &AR_TIM_TIMER_EN ) - len +=snprintf (buf + len, size - len,"TIM " ); -if (reg &AR_DTIM_TIMER_EN ) - len +=snprintf (buf + len, size - len,"DTIM " ); - len +=snprintf (buf + len, size - len,")\n" ); + + len +=snprintf (buf + len,sizeof (buf) - len, +"RXFILTER: 0x%x" , rxfilter); + +if (rxfilter &ATH9K_RX_FILTER_UCAST ) + len +=snprintf (buf + len,sizeof (buf) - len," UCAST" ); +if (rxfilter &ATH9K_RX_FILTER_MCAST ) + len +=snprintf (buf + len,sizeof (buf) - len," MCAST" ); +if (rxfilter &ATH9K_RX_FILTER_BCAST ) + len +=snprintf (buf + len,sizeof (buf) - len," BCAST" ); +if (rxfilter &ATH9K_RX_FILTER_CONTROL ) + len +=snprintf (buf + len,sizeof (buf) - len," CONTROL" ); +if (rxfilter &ATH9K_RX_FILTER_BEACON ) + len +=snprintf (buf + len,sizeof (buf) - len," BEACON" ); +if (rxfilter &ATH9K_RX_FILTER_PROM ) + len +=snprintf (buf + len,sizeof (buf) - len," PROM" ); +if (rxfilter &ATH9K_RX_FILTER_PROBEREQ ) + len +=snprintf (buf + len,sizeof (buf) - len," PROBEREQ" ); +if (rxfilter &ATH9K_RX_FILTER_PHYERR ) + len +=snprintf (buf + len,sizeof (buf) - len," PHYERR" ); +if (rxfilter &ATH9K_RX_FILTER_MYBEACON ) + len +=snprintf (buf + len,sizeof (buf) - len," MYBEACON" ); +if (rxfilter &ATH9K_RX_FILTER_COMP_BAR ) + len +=snprintf (buf + len,sizeof (buf) - len," COMP_BAR" ); +if (rxfilter &ATH9K_RX_FILTER_PSPOLL ) + len +=snprintf (buf + len,sizeof (buf) - len," PSPOLL" ); +if (rxfilter &ATH9K_RX_FILTER_PHYRADAR ) + len +=snprintf (buf + len,sizeof (buf) - len," PHYRADAR" ); +if (rxfilter &ATH9K_RX_FILTER_MCAST_BCAST_ALL ) + len +=snprintf (buf + len,sizeof (buf) - len," MCAST_BCAST_ALL" ); +if (rxfilter &ATH9K_RX_FILTER_CONTROL_WRAPPER ) + len +=snprintf (buf + len,sizeof (buf) - len," CONTROL_WRAPPER" ); + + len +=snprintf (buf + len,sizeof (buf) - len,"\n" ); reg = sc->sc_ah->imask; - len +=snprintf (buf + len, size - len,"imask: 0x%x (" , reg); + + len +=snprintf (buf + len,sizeof (buf) - len,"INTERRUPT-MASK: 0x%x" , reg); +if (reg &ATH9K_INT_SWBA ) - len +=snprintf (buf + len, size - len,"SWBA " ); + len +=snprintf (buf + len,sizeof (buf) - len," SWBA" );if (reg &ATH9K_INT_BMISS ) - len +=snprintf (buf + len, size - len,"BMISS " ); + len +=snprintf (buf + len,sizeof (buf) - len," BMISS" );if (reg &ATH9K_INT_CST ) - len +=snprintf (buf + len, size - len,"CST " ); + len +=snprintf (buf + len,sizeof (buf) - len," CST" );if (reg &ATH9K_INT_RX ) - len +=snprintf (buf + len, size - len,"RX " ); + len +=snprintf (buf + len,sizeof (buf) - len," RX" );if (reg &ATH9K_INT_RXHP ) - len +=snprintf (buf + len, size - len,"RXHP " ); + len +=snprintf (buf + len,sizeof (buf) - len," RXHP" );if (reg &ATH9K_INT_RXLP ) - len +=snprintf (buf + len, size - len,"RXLP " ); + len +=snprintf (buf + len,sizeof (buf) - len," RXLP" );if (reg &ATH9K_INT_BB_WATCHDOG ) - len +=snprintf (buf + len, size - len,"BB_WATCHDOG " ); -/* there are other IRQs if one wanted to add them. */ - len +=snprintf (buf + len, size - len,")\n" ); + len +=snprintf (buf + len,sizeof (buf) - len," BB_WATCHDOG" ); - len +=snprintf (buf + len, size - len, -"VIF Counts: AP: %i STA: %i MESH: %i WDS: %i" -" ADHOC: %i OTHER: %i nvifs: %hi beacon-vifs: %hi\n" , + len +=snprintf (buf + len,sizeof (buf) - len,"\n" ); + +ath9k_calculate_iter_data (hw,NULL , &iter_data); + + len +=snprintf (buf + len,sizeof (buf) - len, +"VIF-COUNTS: AP: %i STA: %i MESH: %i WDS: %i" +" ADHOC: %i OTHER: %i TOTAL: %hi BEACON-VIF: %hi\n" , iter_data.naps, iter_data.nstations, iter_data.nmeshes, iter_data.nwds, iter_data.nadhocs, iter_data.nothers, sc->nvifs, sc->nbcnvifs); - len +=snprintf (buf + len, size - len, -"Calculated-BSSID-Mask: %pM\n" , - iter_data.mask); - -if (len > size) - len = size; +if (len >sizeof (buf)) + len =sizeof (buf); retval =simple_read_from_buffer (user_buf, count, ppos, buf, len); -kfree (buf); -return retval; }
@@ -1637,8 +1543,6 @@ int ath9k_init_debug(struct ath_hw *ah) &fops_dma);debugfs_create_file ("interrupt" ,S_IRUSR , sc->debug.debugfs_phy, sc, &fops_interrupt); -debugfs_create_file ("wiphy" ,S_IRUSR |S_IWUSR , sc->debug.debugfs_phy, - sc, &fops_wiphy);debugfs_create_file ("xmit" ,S_IRUSR , sc->debug.debugfs_phy, sc, &fops_xmit); debugfs_create_file("stations", S_IRUSR, sc->debug.debugfs_phy, sc,
Signature · 1 line
1.7.9
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 6 lines · Sujith Manoharan
This patch merges the 'wiphy' and 'misc' debugfs files and consolidates the information.
Information about the current channel and other HT parameters can be obtained from both mac80211 and iw. Remove such redundant data.
It is nice to be able to see what the driver/NIC thinks it is set to as well. It is always possible that the stack and the NIC are out of sync for some reason. I'd rather those values be left in the debugfs unless there is a good reason for getting rid of them.
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 5 lines · Ben Greear
It is nice to be able to see what the driver/NIC thinks it is set to as well. It is always possible that the stack and the NIC are out of sync for some reason. I'd rather those values be left in the debugfs unless there is a good reason for getting rid of them.
The 'wiphy' file is a remnant from the old virtual HW code. To know the channel parameters of multiple, virtual PHYs, it was useful. Since the operating parameters, channel type, HT capabilites etc. are all exposed by mac80211's debugfs, we might as well use that. Most of the other information - BSSID, RXfilter etc. have been retained.
Sujith
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 11 lines · Sujith Manoharan
Quoted 5 lines · Ben Greear
It is nice to be able to see what the driver/NIC thinks it is set to as well. It is always possible that the stack and the NIC are out of sync for some reason. I'd rather those values be left in the debugfs unless there is a good reason for getting rid of them.
The 'wiphy' file is a remnant from the old virtual HW code. To know the channel parameters of multiple, virtual PHYs, it was useful. Since the operating parameters, channel type, HT capabilites etc. are all exposed by mac80211's debugfs, we might
What if there is some bug that causes the NIC to be in a state different than mac80211 thinks it is? If you leave this in, then maybe someone will notice.
Ath9k is a tricky beast to debug, and I think you should not remove anything that is remotely useful to debugging unless removing it has a real benefit other than just decreasing lines of (conditionally compiled) code.
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 3 lines · Ben Greear
What if there is some bug that causes the NIC to be in a state different than mac80211 thinks it is? If you leave this in, then maybe someone will notice.
Well, if we are not operating in the channel that mac80211 tells us, then we are screwed in a major, non-obvious way. :)
Quoted 4 lines
Ath9k is a tricky beast to debug, and I think you should not remove anything that is remotely useful to debugging unless removing it has a real benefit other than just decreasing lines of (conditionally compiled) code.
I am aware that you have rather unique requirements regarding ath9k, which is why I left 'stations', 'xmit', etc.. untouched. :)
The only data points that have been removed are the current operating channel parameters. And removing redundant code will make a difference in memory-constrained environments like APs. (OpenWRT has DEBUGFS enabled by default).
Sujith
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 6 lines · Sujith Manoharan
Quoted 4 lines
Ath9k is a tricky beast to debug, and I think you should not remove anything that is remotely useful to debugging unless removing it has a real benefit other than just decreasing lines of (conditionally compiled) code.
I am aware that you have rather unique requirements regarding ath9k,
Not really. He just wants to have HALF a chance to find issues. I'm sure you understand that ath9k is far from bug free. Until that has changed (don't hold your breath) the driver can not have too much debug information!
Quoted 3 lines
removing redundant code will make a difference in memory-constrained environments like APs. (OpenWRT has DEBUGFS enabled by default).
It is not very wise for ath9k to reduce debug output with the motivation that some users leave it enabled even if they deploy into limited systems and they would actually prefer to have less debugging.
If ath9k users have an issue with debugging being too big then THEY can disable it. It seems senseless to delete debugging when the driver is clearly still buggy.
It also seems senseless to try to punt to mac80211. It is amazing that Ben's point is not absolutely obvious.
One purpose of debugging is to allow and enable consistency checking. Redundant information is by definition neccessary for that.
As a compromise, you could suggest a move to a two-level compile time option for debugging. You would enable only what you believe to be adequate for debugging in production systems (wait, what did I just write? oh well..) with the first option, and you would allow the rest of the world to enable complete debugging with the second option.
I would support such a scheme.
//Peter
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 16 lines · Peter Stuge
It is not very wise for ath9k to reduce debug output with the motivation that some users leave it enabled even if they deploy into limited systems and they would actually prefer to have less debugging.
If ath9k users have an issue with debugging being too big then THEY can disable it. It seems senseless to delete debugging when the driver is clearly still buggy.
It also seems senseless to try to punt to mac80211. It is amazing that Ben's point is not absolutely obvious.
One purpose of debugging is to allow and enable consistency checking. Redundant information is by definition neccessary for that.
Quoted 5 lines
As a compromise, you could suggest a move to a two-level compile time option for debugging. You would enable only what you believe to be adequate for debugging in production systems (wait, what did I just write? oh well..) with the first option, and you would allow the rest of the world to enable complete debugging with the second option.
/me disregards all the nerd-rage and the general snarkiness.
Having multiple levels of debugging makes things complex. If the general consensus is to keep the information, I can rework the patch.
Sujith
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 7 lines · Sujith Manoharan
Quoted 2 lines · Peter Stuge
As a compromise, you could suggest a move to a two-level compile time option for debugging. You would enable only what you believe to be
Having multiple levels of debugging makes things complex. If the general consensus is to keep the information, I can rework the patch.
Actually, I think it might be useful to have a second level of debugging.
I hope to soon have time & resources to add some logic to dump lots of register info and such in human-readable format, (like, when DMA times out). That is going to be a lot of strings added to the driver, so the compile size will definitely
increase. If keeping the size small is important, then this sort of verbose thing could be hidden behind a second level of debugging...
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 6 lines · Ben Greear
Actually, I think it might be useful to have a second level of debugging.
I hope to soon have time & resources to add some logic to dump lots of register info and such in human-readable format, (like, when DMA times out). That is going to be a lot of strings added to the driver, so the compile size will definitely
increase. If keeping the size small is important, then this sort of verbose thing could be hidden behind a second level of debugging...
That could be implemented similar to what usbmon does. A debugfs file that could be read and redirected to a file. And there would be no overhead to the driver, I think. We could call it the 'event log'. :)
Sujith
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 11 lines · Sujith
Quoted 6 lines · Ben Greear
Actually, I think it might be useful to have a second level of debugging.
I hope to soon have time& resources to add some logic to dump lots of register info and such in human-readable format, (like, when DMA times out). That is going to be a lot of strings added to the driver, so the compile size will definitely
increase. If keeping the size small is important, then this sort of verbose thing could be hidden behind a second level of debugging...That could be implemented similar to what usbmon does. A debugfs file that could be read and redirected to a file. And there would be no overhead to the driver, I think. We could call it the 'event log'. :)
I was thinking about adding a method that grabbed as many registers as I have info for and dumping them with printk when DMA errors hit. This would make kernel splats more useful.
And also have a debugfs file called 'registers' or similar that one could cat out and get similar info. And this can let folks look at steady-state or whatever.
But, the logic to turn the register bit values into strings would be in the driver (and thus add some code size bloat).
My hope is that this would allow a better chance of understanding the stop-DMA errors that some people get reliably (but which I can never reliably reproduce).
I'm not sure how that plays into your 'event log' idea, but maybe one will help the other.
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 24 lines · Ben Greear
Quoted 11 lines · Sujith
Quoted 6 lines · Ben Greear
Actually, I think it might be useful to have a second level of debugging.
I hope to soon have time& resources to add some logic to dump lots of register info and such in human-readable format, (like, when DMA times out). That is going to be a lot of strings added to the driver, so the compile size will definitely
increase. If keeping the size small is important, then this sort of verbose thing could be hidden behind a second level of debugging...That could be implemented similar to what usbmon does. A debugfs file that could be read and redirected to a file. And there would be no overhead to the driver, I think. We could call it the 'event log'. :)
I was thinking about adding a method that grabbed as many registers as I have info for and dumping them with printk when DMA errors hit. This would make kernel splats more useful.
And also have a debugfs file called 'registers' or similar that one could cat out and get similar info. And this can let folks look at steady-state or whatever.
But, the logic to turn the register bit values into strings would be in the driver (and thus add some code size bloat).
Quoted 6 lines
My hope is that this would allow a better chance of understanding the stop-DMA errors that some people get reliably (but which I can never reliably reproduce).
I'm not sure how that plays into your 'event log' idea, but maybe one will help the other.
I think the 'let's dump all kinds of random crap when the issue occurs until we find somebody that can parse it' approach won't work here, and I really think it's not a good idea in general.
In the past the stop-DMA crap has been a symptom with a wild variety of different causes, most of which were actually *software* race conditions, e.g. dma tx or rx enable during reset, locking issues, etc.
In addition to those software causes, there was one actual hardware condition that also triggered this error, and even this wouldn't have showed up in a normal register dump, because it required setting up the MAC observation bus in a particular way.
That hardware trigger for this issue was analyzed not by dumping random data, but by actually talking to hardware designers that could look through the code and guide the debugging process.
Let's not carpet-bomb the driver with lots of debug crap that probably won't ever lead anybody to any good solution for the remaining issues, let's fix stuff the old-fashioned way: by reading the code, understanding what's going on, analyzing problems in a systematic way, rather than clouding the whole process with assumptions based on old bugs that have since been fixed.
- Felix
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 31 lines · Felix Fietkau
Quoted 24 lines · Ben Greear
Quoted 11 lines · Sujith
Quoted 6 lines · Ben Greear
Actually, I think it might be useful to have a second level of debugging.
I hope to soon have time& resources to add some logic to dump lots of register
info and such in human-readable format, (like, when DMA times out). That is going to be a lot of strings added to the driver, so the compile size will definitely
increase. If keeping the size small is important, then this sort of verbose thing could be hidden behind a second level of debugging...That could be implemented similar to what usbmon does. A debugfs file that could be read and redirected to a file. And there would be no overhead to the driver, I think. We could call it the 'event log'. :)
I was thinking about adding a method that grabbed as many registers as I have info for and dumping them with printk when DMA errors hit. This would make kernel splats more useful.
And also have a debugfs file called 'registers' or similar that one could cat out and get similar info. And this can let folks look at steady-state or whatever.
But, the logic to turn the register bit values into strings would be in the driver (and thus add some code size bloat).
Quoted 6 lines
My hope is that this would allow a better chance of understanding the stop-DMA errors that some people get reliably (but which I can never reliably reproduce).
I'm not sure how that plays into your 'event log' idea, but maybe one will help the other.
Quoted 7 lines
I think the 'let's dump all kinds of random crap when the issue occurs until we find somebody that can parse it' approach won't work here, and I really think it's not a good idea in general.
In the past the stop-DMA crap has been a symptom with a wild variety of different causes, most of which were actually *software* race conditions, e.g. dma tx or rx enable during reset, locking issues, etc.
I'm interested in parsing it. There are folks that can reproduce this bug every time, and it seems none of the developers can reproduce it. So, the only thing I can think to do is to try to get more info from the folks that see the problem.
The good news is that some of them are desperate enough to run my hacked kernel, so if such patches are not wanted upstream, I'll just put it in my tree and see if I can get any useful info from them that way...
Quoted 6 lines
Let's not carpet-bomb the driver with lots of debug crap that probably won't ever lead anybody to any good solution for the remaining issues, let's fix stuff the old-fashioned way: by reading the code, understanding what's going on, analyzing problems in a systematic way, rather than clouding the whole process with assumptions based on old bugs that have since been fixed.
That sounds good, and I hope folks are doing that. But as for me, unless I can reproduce a problem I don't think I'll be able to do much, as I don't understand the code that well
and I don't have any access to folks who know the details of the hardware and such...
Thanks,
Ben
Signature · 2 lines
Ben Greear <[email protected]> Candela Technologies Inc http://www.candelatech.com
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
In reply to Ben GreearQuoted 11 lines · Ben Greear
I was thinking about adding a method that grabbed as many registers as I have info for and dumping them with printk when DMA errors hit. This would make kernel splats more useful.
And also have a debugfs file called 'registers' or similar that one could cat out and get similar info. And this can let folks look at steady-state or whatever.
But, the logic to turn the register bit values into strings would be in the driver (and thus add some code size bloat).
Quoted 6 lines
My hope is that this would allow a better chance of understanding the stop-DMA errors that some people get reliably (but which I can never reliably reproduce).
I'm not sure how that plays into your 'event log' idea, but maybe one will help the other.
I was thinking more on the lines of instrumentation, but what you are describing here looks like a VERBOSE_DEBUG option is needed.
Sujith
PATCH 3/7 ath9k: Merge wiphy and misc debugfs files
Quoted 16 lines · Sujith Manoharan
Quoted 15 lines · Ben Greear
I was thinking about adding a method that grabbed as many registers as I have info for and dumping them with printk when DMA errors hit. This would make kernel splats more useful.
And also have a debugfs file called 'registers' or similar that one could cat out and get similar info. And this can let folks look at steady-state or whatever.
But, the logic to turn the register bit values into strings would be in the driver (and thus add some code size bloat).
My hope is that this would allow a better chance of understanding the stop-DMA errors that some people get reliably (but which I can never reliably reproduce).
Quoted 5 lines
Quoted 2 lines
I'm not sure how that plays into your 'event log' idea, but maybe one will help the other.
I was thinking more on the lines of instrumentation, but what you are describing here looks like a
VERBOSE_DEBUGoption is needed.
Huh.
This discussion amounts to nothing. The stuff that was removed was dumping the information present in mac80211.
Sujith