Skip to contentSkip to conversation

ath5k: debug level improvements

PATCH In review Bruno Randolf 5 participants 16 messages Jan 18, 2008 – Jan 22, 2008 Networking drivers (wireless)

lore
Share & tools
lore

Bruno Randolf

author
#0 lore raw

PATCH 1/4 ath5k: debug level improvements

* use only one debug level for beacon debugging: unify ATH5K_DEBUG_BEACON and ATH5K_DEBUG_BEACON_PROC

* remove debug level ATH5K_DEBUG_FATAL. doesn't make sense as a debug level - if it's fatal it should be logged as an error.

* fancier printing of debug levels. cat /debugfs/ath5k/phy0/debug

* allow debug levels to be changed by echoing their name into /debugfs/ath5k/phy0/debug. this will toggle the state, when it was off it will be turned on and vice versa.

drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/debug.c:     Changes-licensed-under: GPL
drivers/net/wireless/ath5k/debug.h:     Changes-licensed-under: GPL
Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/base.c  |   10 ++--
 drivers/net/wireless/ath5k/debug.c |   97 +++++++++++++++++++++++++++++++-----
 drivers/net/wireless/ath5k/debug.h |   18 +++----
 3 files changed, 95 insertions(+), 30 deletions(-)
drivers/net/wireless/ath5k/base.c +5 -5
index 742616a..6bbee64 100644
@@ -1919,7 +1919,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	struct ath5k_buf *bf = sc->bbuf;
 	struct ath5k_hw *ah = sc->ah;
 
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC, "in beacon_send\n");
+	ATH5K_DBG_UNLIMIT(sc, ATH5K_DEBUG_BEACON, "in beacon_send\n");
 
 	if (unlikely(bf->skb == NULL || sc->opmode == IEEE80211_IF_TYPE_STA ||
 			sc->opmode == IEEE80211_IF_TYPE_MNTR)) {

@@ -1935,10 +1935,10 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	 */
 	if (unlikely(ath5k_hw_num_tx_pending(ah, sc->bhalq) != 0)) {
 		sc->bmisscount++;
-		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 			"missed %u consecutive beacons\n", sc->bmisscount);
 		if (sc->bmisscount > 3) {		/* NB: 3 is a guess */
-			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 				"stuck beacon time (%u missed)\n",
 				sc->bmisscount);
 			tasklet_schedule(&sc->restq);

@@ -1946,7 +1946,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 		return;
 	}
 	if (unlikely(sc->bmisscount != 0)) {
-		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 			"resume beacon xmit after %u misses\n",
 			sc->bmisscount);
 		sc->bmisscount = 0;

@@ -1966,7 +1966,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 
 	ath5k_hw_put_tx_buf(ah, sc->bhalq, bf->daddr);
 	ath5k_hw_tx_start(ah, sc->bhalq);
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC, "TXDP[%u] = %llx (%p)\n",
+	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "TXDP[%u] = %llx (%p)\n",
 		sc->bhalq, (unsigned long long)bf->daddr, bf->desc);
 
 	sc->bsent++;
drivers/net/wireless/ath5k/debug.c +83 -14

130 lines not on this page · raw message

drivers/net/wireless/ath5k/debug.h +7 -11
index 2b491cb..c4fd8c4 100644
@@ -91,7 +91,6 @@ struct ath5k_dbg_info {
  * @ATH5K_DEBUG_MODE: mode init/setup
  * @ATH5K_DEBUG_XMIT: basic xmit operation
  * @ATH5K_DEBUG_BEACON: beacon handling
- * @ATH5K_DEBUG_BEACON_PROC: beacon ISR proc
  * @ATH5K_DEBUG_CALIBRATE: periodic calibration
  * @ATH5K_DEBUG_TXPOWER: transmit power setting
  * @ATH5K_DEBUG_LED: led management

@@ -99,7 +98,6 @@ struct ath5k_dbg_info {
  * @ATH5K_DEBUG_DUMP_TX: print transmit skb content
  * @ATH5K_DEBUG_DUMPMODES: dump modes
  * @ATH5K_DEBUG_TRACE: trace function calls
- * @ATH5K_DEBUG_FATAL: fatal errors
  * @ATH5K_DEBUG_ANY: show at any debug level
  *
  * The debug level is used to control the amount and type of debugging output

@@ -115,15 +113,13 @@ enum ath5k_debug_level {
 	ATH5K_DEBUG_MODE	= 0x00000004,
 	ATH5K_DEBUG_XMIT	= 0x00000008,
 	ATH5K_DEBUG_BEACON	= 0x00000010,
-	ATH5K_DEBUG_BEACON_PROC	= 0x00000020,
-	ATH5K_DEBUG_CALIBRATE	= 0x00000100,
-	ATH5K_DEBUG_TXPOWER	= 0x00000200,
-	ATH5K_DEBUG_LED		= 0x00000400,
-	ATH5K_DEBUG_DUMP_RX	= 0x00001000,
-	ATH5K_DEBUG_DUMP_TX	= 0x00002000,
-	ATH5K_DEBUG_DUMPMODES	= 0x00004000,
-	ATH5K_DEBUG_TRACE	= 0x00010000,
-	ATH5K_DEBUG_FATAL	= 0x80000000,
+	ATH5K_DEBUG_CALIBRATE	= 0x00000020,
+	ATH5K_DEBUG_TXPOWER	= 0x00000040,
+	ATH5K_DEBUG_LED		= 0x00000080,
+	ATH5K_DEBUG_DUMP_RX	= 0x00000100,
+	ATH5K_DEBUG_DUMP_TX	= 0x00000200,
+	ATH5K_DEBUG_DUMPMODES	= 0x00000400,
+	ATH5K_DEBUG_TRACE	= 0x00001000,
 	ATH5K_DEBUG_ANY		= 0xffffffff
 };
author
#1 lore raw

PATCH 2/4 ath5k: always extend rx timestamp with tsf

always extend the rx timestamp with the local TSF, since this information is also needed for proper IBSS merging. this is done in the tasklet for now, maybe has to be moved to the interrupt handler like in madwifi.

drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD

Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/base.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)
drivers/net/wireless/ath5k/base.c +12 -5
index 6bbee64..bb1745f 100644
@@ -1725,11 +1725,18 @@ accept:
 			skb_pull(skb, pad);
 		}
 
-		if (sc->opmode == IEEE80211_IF_TYPE_MNTR)
-			rxs.mactime = ath5k_extend_tsf(sc->ah,
-					ds->ds_rxstat.rs_tstamp);
-		else
-			rxs.mactime = ds->ds_rxstat.rs_tstamp;
+		/*
+		 * always extend the mac timestamp, since this information is
+		 * also needed for proper IBSS merging.
+		 *
+		 * XXX: it might be too late to do it here, since rs_tstamp is
+		 * 15bit only. that means TSF extension has to be done within
+		 * 32.768usec = 32ms. it might be necessary to move this to the
+		 * interrupt handler, like it is done in madwifi.
+		 */
+		rxs.mactime = ath5k_extend_tsf(sc->ah, ds->ds_rxstat.rs_tstamp);
+		rxs.flag |= RX_FLAG_TSFT;
+
 		rxs.freq = sc->curchan->freq;
 		rxs.channel = sc->curchan->chan;
 		rxs.phymode = sc->curmode;
#2 lore raw

Re: [PATCH 2/4] ath5k: always extend rx timestamp with tsf

Quoted 25 lines · Bruno Randolf

always extend the rx timestamp with the local TSF, since this information is also needed for proper IBSS merging. this is done in the tasklet for now, maybe has to be moved to the interrupt handler like in madwifi.

drivers/net/wireless/ath5k/base.c: Changes-licensed-under: 3-Clause-BSD

Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/base.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)
drivers/net/wireless/ath5k/base.c +0 -4
index 6bbee64..bb1745f 100644
@@ -1725,11 +1725,18 @@ accept:
                        skb_pull(skb, pad);
                }
 
-               if (sc->opmode == IEEE80211_IF_TYPE_MNTR)
-                       rxs.mactime = ath5k_extend_tsf(sc->ah,
-                                       ds->ds_rxstat.rs_tstamp);
-               else

drivers/net/wireless/ath5k/base.c accept: · line 1736

Quoted 10 lines
drivers/net/wireless/ath5k/base.c +9 -1
-                       rxs.mactime = ds->ds_rxstat.rs_tstamp;
+               /*
+                * always extend the mac timestamp, since this information is
+                * also needed for proper IBSS merging.
+                *
+                * XXX: it might be too late to do it here, since rs_tstamp is
+                * 15bit only. that means TSF extension has to be done within
+                * 32.768usec = 32ms. it might be necessary to move this to the
+                * interrupt handler, like it is done in madwifi.
+                */

I'm trying to understand this a bit more and am confused. The TSF timer is based on 1MHz clock so it has a resolution of 1 microsecond (us). For 15 bits that would mean 32768 us so don't you mean it should be done within 32.768 ms instead of usec (or us)?

drivers/net/wireless/ath5k/base.c accept: · line 1742

Quoted 8 lines
drivers/net/wireless/ath5k/base.c +3 -0
+               rxs.mactime = ath5k_extend_tsf(sc->ah, ds->ds_rxstat.rs_tstamp);
+               rxs.flag |= RX_FLAG_TSFT;
+
                rxs.freq = sc->curchan->freq;
                rxs.channel = sc->curchan->chan;
                rxs.phymode = sc->curmode;

Right now we only use mactime and even RX_FLAG_TSFT within mac80211 in rx.c on ieee80211_rx_monitor() for IEEE80211_RADIOTAP_TSFT so right now these changes would seem to do nothing. Should we be using this for something else -- if so what is it?

Luis

#3 lore raw

Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf

Hi,

Quoted 10 lines · Luis R. Rodriguez

drivers/net/wireless/ath5k/base.c accept: · line 1736

Quoted 8 lines · Bruno Randolf
drivers/net/wireless/ath5k/base.c +8 -0
+                * always extend the mac timestamp, since this information is
+                * also needed for proper IBSS merging.
+                *
+                * XXX: it might be too late to do it here, since rs_tstamp is
+                * 15bit only. that means TSF extension has to be done within
+                * 32.768usec = 32ms. it might be necessary to move this to the
+                * interrupt handler, like it is done in madwifi.
+                */
Quoted 5 lines

I'm trying to understand this a bit more and am confused. The TSF timer is based on 1MHz clock so it has a resolution of 1 microsecond (us). For 15 bits that would mean 32768 us so don't you mean it should be done within 32.768 ms instead of usec (or us)?

Hi,
it is a typo.

You are correct. It should be done within 32.768 ms. On a high end laptop, it is almost guaranteed that the bottom half will process the packet within 32 ms. However, on an embedded box (low spec cpu) that has a temporarily high load, 32 ms is a short time, and the timestamp may have wrapped around. this is unfortunate.

Derek.

Signature · 5 lines
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
Email: [email protected]
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
author
#4 lore raw

PATCH 3/4 ath5k: use 3 instead of 0x00000003

In reply to Bruno Randolf

reviewed beacon timer initialization with register traces from madwifi: what we are doing is correct :). one minor fix: use 3 instead of 0x00000003 - it's more readable.

drivers/net/wireless/ath5k/hw.c: Changes-licensed-under: ISC

Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/hw.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/net/wireless/ath5k/hw.c +2 -4
index eb00818..3a4bf40 100644
@@ -2605,10 +2605,8 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
 		break;
 
 	default:
-		timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) <<
-		    0x00000003;
-		timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) <<
-		    0x00000003;
+		timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) << 3;
+		timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) << 3;
 	}
 
 	timer3 = next_beacon + (ah->ah_atim_window ? ah->ah_atim_window : 1);
Acked-by on v1 3/4
#5 lore raw

Re: [PATCH 3/4] ath5k: use 3 instead of 0x00000003

Quoted 22 lines · Bruno Randolf

reviewed beacon timer initialization with register traces from madwifi: what we are doing is correct :). one minor fix: use 3 instead of 0x00000003 - it's more readable.

drivers/net/wireless/ath5k/hw.c: Changes-licensed-under: ISC

Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/hw.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/net/wireless/ath5k/hw.c +0 -0
index eb00818..3a4bf40 100644
@@ -2605,10 +2605,8 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
                break;

drivers/net/wireless/ath5k/hw.c void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval) · line 2612

Quoted 10 lines
drivers/net/wireless/ath5k/hw.c +2 -4
        default:
-               timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) <<
-                   0x00000003;
-               timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) <<
-                   0x00000003;
+               timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) << 3;
+               timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) << 3;
        }
 
        timer3 = next_beacon + (ah->ah_atim_window ? ah->ah_atim_window : 1);

Sure why not,

Acked-by Luis R. Rodriguez <[email protected]>

Luis

Acked-by on v1 3/4
#6 lore raw

Re: [PATCH 3/4] ath5k: use 3 instead of 0x00000003

In reply to Bruno Randolf

2008/1/18, Bruno Randolf <[email protected]>:

Quoted 22 lines

reviewed beacon timer initialization with register traces from madwifi: what we are doing is correct :). one minor fix: use 3 instead of 0x00000003 - it's more readable.

drivers/net/wireless/ath5k/hw.c: Changes-licensed-under: ISC

Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/hw.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/net/wireless/ath5k/hw.c +0 -0
index eb00818..3a4bf40 100644
@@ -2605,10 +2605,8 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
                break;

drivers/net/wireless/ath5k/hw.c void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval) · line 2612

Quoted 10 lines
drivers/net/wireless/ath5k/hw.c +2 -4
        default:
-               timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) <<
-                   0x00000003;
-               timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) <<
-                   0x00000003;
+               timer1 = (next_beacon - AR5K_TUNE_DMA_BEACON_RESP) << 3;
+               timer2 = (next_beacon - AR5K_TUNE_SW_BEACON_RESP) << 3;
        }
 
        timer3 = next_beacon + (ah->ah_atim_window ? ah->ah_atim_window : 1);
Acked-by Nick Kossifidis <[email protected]>
Signature · 3 lines
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
author
#7 lore raw

PATCH 4/4 ath5k: beacon interval is in TU

In reply to Bruno Randolf

the beacon interval is passed by mac80211 in TU already, so we can directly use it without conversion. also update the comments about TU (1 TU is defined by 802.11 as 1024usec).

drivers/net/wireless/ath5k/ath5k.h:     Changes-licensed-under: ISC
drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/base.h:      Changes-licensed-under: 3-Clause-BSD
Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/ath5k.h |    4 ++--
 drivers/net/wireless/ath5k/base.c  |    4 ++--
 drivers/net/wireless/ath5k/base.h  |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/net/wireless/ath5k/ath5k.h +2 -2
index 878609f..c79066b 100644
@@ -486,8 +486,8 @@ struct ath5k_beacon_state {
  * TSF to TU conversion:
  *
  * TSF is a 64bit value in usec (microseconds).
- * TU is a 32bit value in roughly msec (milliseconds): usec / 1024
- * (1000ms equals 976 TU)
+ * TU is a 32bit value and defined by IEEE802.11 (page 6) as "A measurement of
+ * time equal to 1024 usec", so it's roughly milliseconds (usec / 1024).
  */
 #define TSF_TO_TU(_tsf) (u32)((_tsf) >> 10)
drivers/net/wireless/ath5k/base.c +2 -2
index bb1745f..784b359 100644
@@ -2550,7 +2550,7 @@ ath5k_config(struct ieee80211_hw *hw,
 {
 	struct ath5k_softc *sc = hw->priv;
 
-	sc->bintval = conf->beacon_int * 1000 / 1024;
+	sc->bintval = conf->beacon_int;
 	ath5k_setcurmode(sc, conf->phymode);
 
 	return ath5k_chan_set(sc, conf->chan);

@@ -2566,7 +2566,7 @@ ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
 	/* Set to a reasonable value. Note that this will
 	 * be set to mac80211's value at ath5k_config(). */
-	sc->bintval = 1000 * 1000 / 1024;
+	sc->bintval = 1000;
 	mutex_lock(&sc->lock);
 	if (sc->vif != vif) {
 		ret = -EIO;
drivers/net/wireless/ath5k/base.h +1 -1
index 7ba2223..20c9469 100644
@@ -164,7 +164,7 @@ struct ath5k_softc {
 	struct ath5k_buf	*bbuf;		/* beacon buffer */
 	unsigned int		bhalq,		/* SW q for outgoing beacons */
 				bmisscount,	/* missed beacon transmits */
-				bintval,	/* beacon interval */
+				bintval,	/* beacon interval in TU */
 				bsent;
 
 	struct timer_list	calib_tim;	/* calibration timer */
Acked-by on v1 4/4
#8 lore raw

Re: [PATCH 4/4] ath5k: beacon interval is in TU

Quoted 46 lines · Bruno Randolf

the beacon interval is passed by mac80211 in TU already, so we can directly use it without conversion. also update the comments about TU (1 TU is defined by 802.11 as 1024usec).

drivers/net/wireless/ath5k/ath5k.h:     Changes-licensed-under: ISC
drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/base.h:      Changes-licensed-under: 3-Clause-BSD
Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/ath5k.h |    4 ++--
 drivers/net/wireless/ath5k/base.c  |    4 ++--
 drivers/net/wireless/ath5k/base.h  |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/net/wireless/ath5k/ath5k.h +2 -2
index 878609f..c79066b 100644
@@ -486,8 +486,8 @@ struct ath5k_beacon_state {
  * TSF to TU conversion:
  *
  * TSF is a 64bit value in usec (microseconds).
- * TU is a 32bit value in roughly msec (milliseconds): usec / 1024
- * (1000ms equals 976 TU)
+ * TU is a 32bit value and defined by IEEE802.11 (page 6) as "A measurement of
+ * time equal to 1024 usec", so it's roughly milliseconds (usec / 1024).
  */
 #define TSF_TO_TU(_tsf) (u32)((_tsf) >> 10)
drivers/net/wireless/ath5k/base.c +1 -1
index bb1745f..784b359 100644
@@ -2550,7 +2550,7 @@ ath5k_config(struct ieee80211_hw *hw,
 {
        struct ath5k_softc *sc = hw->priv;
 
-       sc->bintval = conf->beacon_int * 1000 / 1024;
+       sc->bintval = conf->beacon_int;
        ath5k_setcurmode(sc, conf->phymode);
 
        return ath5k_chan_set(sc, conf->chan);

drivers/net/wireless/ath5k/base.c ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif, · line 2569

Quoted 5 lines
drivers/net/wireless/ath5k/base.c +1 -1
@@ -2566,7 +2566,7 @@ ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
        /* Set to a reasonable value. Note that this will
         * be set to mac80211's value at ath5k_config(). */
-       sc->bintval = 1000 * 1000 / 1024;
+       sc->bintval = 1000;

Sure, although as I noted this value will be later set by mac80211 through ath5k_config().

Acked-by Luis R. Rodriguez <[email protected]>

Luis

Acked-by on v1 4/4
#9 lore raw

Re: [PATCH 4/4] ath5k: beacon interval is in TU

In reply to Bruno Randolf

2008/1/18, Bruno Randolf <[email protected]>:

Quoted 61 lines

the beacon interval is passed by mac80211 in TU already, so we can directly use it without conversion. also update the comments about TU (1 TU is defined by 802.11 as 1024usec).

drivers/net/wireless/ath5k/ath5k.h:     Changes-licensed-under: ISC
drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/base.h:      Changes-licensed-under: 3-Clause-BSD
Signed-off-by Bruno Randolf <[email protected]>

---

 drivers/net/wireless/ath5k/ath5k.h |    4 ++--
 drivers/net/wireless/ath5k/base.c  |    4 ++--
 drivers/net/wireless/ath5k/base.h  |    2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/net/wireless/ath5k/ath5k.h +2 -2
index 878609f..c79066b 100644
@@ -486,8 +486,8 @@ struct ath5k_beacon_state {
  * TSF to TU conversion:
  *
  * TSF is a 64bit value in usec (microseconds).
- * TU is a 32bit value in roughly msec (milliseconds): usec / 1024
- * (1000ms equals 976 TU)
+ * TU is a 32bit value and defined by IEEE802.11 (page 6) as "A measurement of
+ * time equal to 1024 usec", so it's roughly milliseconds (usec / 1024).
  */
 #define TSF_TO_TU(_tsf) (u32)((_tsf) >> 10)
drivers/net/wireless/ath5k/base.c +2 -2
index bb1745f..784b359 100644
@@ -2550,7 +2550,7 @@ ath5k_config(struct ieee80211_hw *hw,
 {
        struct ath5k_softc *sc = hw->priv;
 
-       sc->bintval = conf->beacon_int * 1000 / 1024;
+       sc->bintval = conf->beacon_int;
        ath5k_setcurmode(sc, conf->phymode);
 
        return ath5k_chan_set(sc, conf->chan);

@@ -2566,7 +2566,7 @@ ath5k_config_interface(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 
        /* Set to a reasonable value. Note that this will
         * be set to mac80211's value at ath5k_config(). */
-       sc->bintval = 1000 * 1000 / 1024;
+       sc->bintval = 1000;
        mutex_lock(&sc->lock);
        if (sc->vif != vif) {
                ret = -EIO;
drivers/net/wireless/ath5k/base.h +0 -0
index 7ba2223..20c9469 100644

drivers/net/wireless/ath5k/base.h struct ath5k_softc { · line 170

Quoted 8 lines
drivers/net/wireless/ath5k/base.h +1 -1
@@ -164,7 +164,7 @@ struct ath5k_softc {
        struct ath5k_buf        *bbuf;          /* beacon buffer */
        unsigned int            bhalq,          /* SW q for outgoing beacons */
                                bmisscount,     /* missed beacon transmits */
-                               bintval,        /* beacon interval */
+                               bintval,        /* beacon interval in TU */
                                bsent;
 
        struct timer_list       calib_tim;      /* calibration timer */
Acked-by Nick Kossifidis <[email protected]>
Signature · 3 lines
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
#10 lore raw
In reply to Bruno Randolf
Quoted 18 lines · Bruno Randolf

* use only one debug level for beacon debugging: unify ATH5K_DEBUG_BEACON and ATH5K_DEBUG_BEACON_PROC

* remove debug level ATH5K_DEBUG_FATAL. doesn't make sense as a debug level - if it's fatal it should be logged as an error.

* fancier printing of debug levels. cat /debugfs/ath5k/phy0/debug

* allow debug levels to be changed by echoing their name into /debugfs/ath5k/phy0/debug. this will toggle the state, when it was off it will be turned on and vice versa.

drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/debug.c:     Changes-licensed-under: GPL
drivers/net/wireless/ath5k/debug.h:     Changes-licensed-under: GPL
Signed-off-by Bruno Randolf <[email protected]>

---

[...]

Quoted 1 line

+#define TOGGLE_BIT(_x, _m) (_x) = (_x) & (_m) ? (_x) & ~(_m) : (_x) | (_m)

simple XOR :)?

_x ^= _m;

author
#11 lore raw
Quoted 21 lines · On Friday 18 January 2008 21:55:38 Jiri Slaby wrote:
Quoted 12 lines · Bruno Randolf

* use only one debug level for beacon debugging: unify ATH5K_DEBUG_BEACON and ATH5K_DEBUG_BEACON_PROC

* remove debug level ATH5K_DEBUG_FATAL. doesn't make sense as a debug level - if it's fatal it should be logged as an error.

* fancier printing of debug levels. cat /debugfs/ath5k/phy0/debug

* allow debug levels to be changed by echoing their name into /debugfs/ath5k/phy0/debug. this will toggle the state, when it was off it will be turned on and vice versa.

Quoted 7 lines

drivers/net/wireless/ath5k/base.c: Changes-licensed-under:
3-Clause-BSD drivers/net/wireless/ath5k/debug.c:
Changes-licensed-under: GPL drivers/net/wireless/ath5k/debug.h:
Changes-licensed-under: GPL

Signed-off-by Bruno Randolf <[email protected]>

---

Quoted 8 lines

[...]

Quoted 2 lines

+#define TOGGLE_BIT(_x, _m) (_x) = (_x) & (_m) ? (_x) & ~(_m) : (_x) | (_m)

simple XOR :)?

_x ^= _m;

oh yeah, looks much better :)
thanks!

bruno

author
#13 lore raw
Quoted 9 lines · On Saturday 19 January 2008 08:26:38 Luis R. Rodriguez wrote:
Quoted 1 line · Bruno Randolf

+ { ATH5K_DEBUG_ANY, "any", "show any debug level" },

Very cool, can we call this "all" instead though, and "show all debug levels" ?

Acked-by Luis R. Rodriguez <[email protected]>

Luis

sure. i'll resend in a minute, also including jiri's hint.

bruno

author
#14 lore raw

PATCH 1/1 ath5k: debug level improvements

* use only one debug level for beacon debugging: unify ATH5K_DEBUG_BEACON and ATH5K_DEBUG_BEACON_PROC

* remove debug level ATH5K_DEBUG_FATAL. doesn't make sense as a debug level - if it's fatal it should be logged as an error.

* fancier printing of debug levels. cat /debugfs/ath5k/phy0/debug

* allow debug levels to be changed by echoing their name into /debugfs/ath5k/phy0/debug. this will toggle the state, when it was off it will be turned on and vice versa.

drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/debug.c:     Changes-licensed-under: GPL
drivers/net/wireless/ath5k/debug.h:     Changes-licensed-under: GPL
Signed-off-by Bruno Randolf <[email protected]>
Acked-by Luis R. Rodriguez <[email protected]>

---

 drivers/net/wireless/ath5k/base.c  |   10 ++--
 drivers/net/wireless/ath5k/debug.c |   95 +++++++++++++++++++++++++++++++-----
 drivers/net/wireless/ath5k/debug.h |   18 +++----
 3 files changed, 93 insertions(+), 30 deletions(-)
drivers/net/wireless/ath5k/base.c +5 -5
index 742616a..6bbee64 100644
@@ -1919,7 +1919,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	struct ath5k_buf *bf = sc->bbuf;
 	struct ath5k_hw *ah = sc->ah;
 
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC, "in beacon_send\n");
+	ATH5K_DBG_UNLIMIT(sc, ATH5K_DEBUG_BEACON, "in beacon_send\n");
 
 	if (unlikely(bf->skb == NULL || sc->opmode == IEEE80211_IF_TYPE_STA ||
 			sc->opmode == IEEE80211_IF_TYPE_MNTR)) {

@@ -1935,10 +1935,10 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	 */
 	if (unlikely(ath5k_hw_num_tx_pending(ah, sc->bhalq) != 0)) {
 		sc->bmisscount++;
-		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 			"missed %u consecutive beacons\n", sc->bmisscount);
 		if (sc->bmisscount > 3) {		/* NB: 3 is a guess */
-			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 				"stuck beacon time (%u missed)\n",
 				sc->bmisscount);
 			tasklet_schedule(&sc->restq);

@@ -1946,7 +1946,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 		return;
 	}
 	if (unlikely(sc->bmisscount != 0)) {
-		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC,
+		ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
 			"resume beacon xmit after %u misses\n",
 			sc->bmisscount);
 		sc->bmisscount = 0;

@@ -1966,7 +1966,7 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 
 	ath5k_hw_put_tx_buf(ah, sc->bhalq, bf->daddr);
 	ath5k_hw_tx_start(ah, sc->bhalq);
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON_PROC, "TXDP[%u] = %llx (%p)\n",
+	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "TXDP[%u] = %llx (%p)\n",
 		sc->bhalq, (unsigned long long)bf->daddr, bf->desc);
 
 	sc->bsent++;
drivers/net/wireless/ath5k/debug.c +81 -14

128 lines not on this page · raw message

drivers/net/wireless/ath5k/debug.h +7 -11
index 2b491cb..c4fd8c4 100644
@@ -91,7 +91,6 @@ struct ath5k_dbg_info {
  * @ATH5K_DEBUG_MODE: mode init/setup
  * @ATH5K_DEBUG_XMIT: basic xmit operation
  * @ATH5K_DEBUG_BEACON: beacon handling
- * @ATH5K_DEBUG_BEACON_PROC: beacon ISR proc
  * @ATH5K_DEBUG_CALIBRATE: periodic calibration
  * @ATH5K_DEBUG_TXPOWER: transmit power setting
  * @ATH5K_DEBUG_LED: led management

@@ -99,7 +98,6 @@ struct ath5k_dbg_info {
  * @ATH5K_DEBUG_DUMP_TX: print transmit skb content
  * @ATH5K_DEBUG_DUMPMODES: dump modes
  * @ATH5K_DEBUG_TRACE: trace function calls
- * @ATH5K_DEBUG_FATAL: fatal errors
  * @ATH5K_DEBUG_ANY: show at any debug level
  *
  * The debug level is used to control the amount and type of debugging output

@@ -115,15 +113,13 @@ enum ath5k_debug_level {
 	ATH5K_DEBUG_MODE	= 0x00000004,
 	ATH5K_DEBUG_XMIT	= 0x00000008,
 	ATH5K_DEBUG_BEACON	= 0x00000010,
-	ATH5K_DEBUG_BEACON_PROC	= 0x00000020,
-	ATH5K_DEBUG_CALIBRATE	= 0x00000100,
-	ATH5K_DEBUG_TXPOWER	= 0x00000200,
-	ATH5K_DEBUG_LED		= 0x00000400,
-	ATH5K_DEBUG_DUMP_RX	= 0x00001000,
-	ATH5K_DEBUG_DUMP_TX	= 0x00002000,
-	ATH5K_DEBUG_DUMPMODES	= 0x00004000,
-	ATH5K_DEBUG_TRACE	= 0x00010000,
-	ATH5K_DEBUG_FATAL	= 0x80000000,
+	ATH5K_DEBUG_CALIBRATE	= 0x00000020,
+	ATH5K_DEBUG_TXPOWER	= 0x00000040,
+	ATH5K_DEBUG_LED		= 0x00000080,
+	ATH5K_DEBUG_DUMP_RX	= 0x00000100,
+	ATH5K_DEBUG_DUMP_TX	= 0x00000200,
+	ATH5K_DEBUG_DUMPMODES	= 0x00000400,
+	ATH5K_DEBUG_TRACE	= 0x00001000,
 	ATH5K_DEBUG_ANY		= 0xffffffff
 };
#15 lore raw
Quoted 26 lines · Bruno Randolf

* use only one debug level for beacon debugging: unify ATH5K_DEBUG_BEACON and ATH5K_DEBUG_BEACON_PROC

* remove debug level ATH5K_DEBUG_FATAL. doesn't make sense as a debug level - if it's fatal it should be logged as an error.

* fancier printing of debug levels. cat /debugfs/ath5k/phy0/debug

* allow debug levels to be changed by echoing their name into /debugfs/ath5k/phy0/debug. this will toggle the state, when it was off it will be turned on and vice versa.

drivers/net/wireless/ath5k/base.c:      Changes-licensed-under: 3-Clause-BSD
drivers/net/wireless/ath5k/debug.c:     Changes-licensed-under: GPL
drivers/net/wireless/ath5k/debug.h:     Changes-licensed-under: GPL
Signed-off-by Bruno Randolf <[email protected]>
Acked-by Luis R. Rodriguez <[email protected]>

---

 drivers/net/wireless/ath5k/base.c  |   10 ++--
 drivers/net/wireless/ath5k/debug.c |   95 +++++++++++++++++++++++++++++++-----
 drivers/net/wireless/ath5k/debug.h |   18 +++----
 3 files changed, 93 insertions(+), 30 deletions(-)

[...]

Quoted 5 lines
drivers/net/wireless/ath5k/debug.c +0 -0
index 4ba649e..63e39f9 100644
@@ -314,6 +314,76 @@ static const struct file_operations fops_reset = {

[...]

drivers/net/wireless/ath5k/debug.c static const struct file_operations fops_reset = { · line 369

Quoted 6 lines
drivers/net/wireless/ath5k/debug.c +6 -0
+static ssize_t write_file_debug(struct file *file,
+				 const char __user *userbuf,
+				 size_t count, loff_t *ppos)
+{
+	struct ath5k_softc *sc = file->private_data;
+	int i;

BTW. unsigned int generates better code on most platforms.

drivers/net/wireless/ath5k/debug.c static const struct file_operations fops_reset = { · line 373

Quoted 4 lines
drivers/net/wireless/ath5k/debug.c +4 -0
+
+	for (i = 0; i < ARRAY_SIZE(dbg_info); i++) {
+		if (strncmp(userbuf, dbg_info[i].name,
+					strlen(dbg_info[i].name)) == 0)

Ah, we have bugs in debug write methods. You can't pass user buffer to strcmp. You must copy_from_user() it first. Otherwise kernel won't be happy from userspace code such as:
fd = open("path_to_the_debug_file", O_RDWR);
write(fd, 1234 or NULL or whatever meaningless, 1);

Also you don't need to call strncmp, strcmp is OK (you can rely on dbg_info.name being null terminated and also the static strings such as "disable" are...) and shorter.

Microoptimisation is to put "break" right after it:

drivers/net/wireless/ath5k/debug.c static const struct file_operations fops_reset = { · line 374

Quoted 1 line
drivers/net/wireless/ath5k/debug.c +1 -0
+			sc->debug.level ^= dbg_info[i].level; /* toggle bit */

but it's not mandatory at all.

thanks,
--js