* 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(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 742616a..6bbee64 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -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++;
diff --git a/drivers/net/wireless/ath5k/debug.c b/drivers/net/wireless/ath5k/debug.c
index 4ba649e..0118ada 100644
--- a/drivers/net/wireless/ath5k/debug.c
+++ b/drivers/net/wireless/ath5k/debug.c
@@ -314,6 +314,78 @@ static const struct file_operations fops_reset = {
};
+/* debugfs: debug level */
+
+static struct {
+ enum ath5k_debug_level level;
+ const char *name;
+ const char *desc;
+} dbg_info[] = {
+ { ATH5K_DEBUG_RESET, "reset", "reset and initialization" },
+ { ATH5K_DEBUG_INTR, "intr", "interrupt handling" },
+ { ATH5K_DEBUG_MODE, "mode", "mode init/setup" },
+ { ATH5K_DEBUG_XMIT, "xmit", "basic xmit operation" },
+ { ATH5K_DEBUG_BEACON, "beacon", "beacon handling" },
+ { ATH5K_DEBUG_CALIBRATE, "calib", "periodic calibration" },
+ { ATH5K_DEBUG_TXPOWER, "txpower", "transmit power setting" },
+ { ATH5K_DEBUG_LED, "led", "LED mamagement" },
+ { ATH5K_DEBUG_DUMP_RX, "dumprx", "print received skb content" },
+ { ATH5K_DEBUG_DUMP_TX, "dumptx", "print transmit skb content" },
+ { ATH5K_DEBUG_DUMPMODES, "dumpmodes", "dump modes" },
+ { ATH5K_DEBUG_TRACE, "trace", "trace function calls" },
+ { ATH5K_DEBUG_ANY, "any", "show any debug level" },
+};
+
+#define TOGGLE_BIT(_x, _m) (_x) = (_x) & (_m) ? (_x) & ~(_m) : (_x) | (_m)
+
+static ssize_t read_file_debug(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath5k_softc *sc = file->private_data;
+ char buf[1000];
+ int len = 0;
+ int i;
+
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "DEBUG LEVEL: 0x%08x\n\n", sc->debug.level);
+
+ for (i = 0; i < ARRAY_SIZE(dbg_info) - 1; i++) {
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "%10s %c 0x%08x - %s\n", dbg_info[i].name,
+ sc->debug.level & dbg_info[i].level ? '+' : ' ',
+ dbg_info[i].level, dbg_info[i].desc);
+ }
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "%10s %c 0x%08x - %s\n", dbg_info[i].name,
+ sc->debug.level == dbg_info[i].level ? '+' : ' ',
+ dbg_info[i].level, dbg_info[i].desc);
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+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;
+
+ for (i = 0; i < ARRAY_SIZE(dbg_info); i++) {
+ if (strncmp(userbuf, dbg_info[i].name,
+ strlen(dbg_info[i].name)) == 0)
+ TOGGLE_BIT(sc->debug.level, dbg_info[i].level);
+ }
+ return count;
+}
+
+static const struct file_operations fops_debug = {
+ .read = read_file_debug,
+ .write = write_file_debug,
+ .open = ath5k_debugfs_open,
+ .owner = THIS_MODULE,
+};
+
+
/* init */
void
@@ -326,26 +398,24 @@ void
ath5k_debug_init_device(struct ath5k_softc *sc)
{
sc->debug.level = ath5k_debug;
+
sc->debug.debugfs_phydir = debugfs_create_dir(wiphy_name(sc->hw->wiphy),
- ath5k_global_debugfs);
- sc->debug.debugfs_debug = debugfs_create_u32("debug",
- 0666, sc->debug.debugfs_phydir, &sc->debug.level);
+ ath5k_global_debugfs);
+
+ sc->debug.debugfs_debug = debugfs_create_file("debug", 0666,
+ sc->debug.debugfs_phydir, sc, &fops_debug);
sc->debug.debugfs_registers = debugfs_create_file("registers", 0444,
- sc->debug.debugfs_phydir,
- sc, &fops_registers);
+ sc->debug.debugfs_phydir, sc, &fops_registers);
sc->debug.debugfs_tsf = debugfs_create_file("tsf", 0666,
- sc->debug.debugfs_phydir,
- sc, &fops_tsf);
+ sc->debug.debugfs_phydir, sc, &fops_tsf);
sc->debug.debugfs_beacon = debugfs_create_file("beacon", 0666,
- sc->debug.debugfs_phydir,
- sc, &fops_beacon);
+ sc->debug.debugfs_phydir, sc, &fops_beacon);
sc->debug.debugfs_reset = debugfs_create_file("reset", 0222,
- sc->debug.debugfs_phydir,
- sc, &fops_reset);
+ sc->debug.debugfs_phydir, sc, &fops_reset);
}
void
@@ -415,8 +485,7 @@ ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah)
struct ath5k_buf *bf;
int status;
- if (likely(!(sc->debug.level &
- (ATH5K_DEBUG_RESET | ATH5K_DEBUG_FATAL))))
+ if (likely(!(sc->debug.level & ATH5K_DEBUG_RESET)))
return;
printk(KERN_DEBUG "rx queue %x, link %p\n",
@@ -426,7 +495,7 @@ ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah)
list_for_each_entry(bf, &sc->rxbuf, list) {
ds = bf->desc;
status = ah->ah_proc_rx_desc(ah, ds);
- if (!status || (sc->debug.level & ATH5K_DEBUG_FATAL))
+ if (!status)
ath5k_debug_printrxbuf(bf, status == 0);
}
spin_unlock_bh(&sc->rxbuflock);
diff --git a/drivers/net/wireless/ath5k/debug.h b/drivers/net/wireless/ath5k/debug.h
index 2b491cb..c4fd8c4 100644
--- a/drivers/net/wireless/ath5k/debug.h
+++ b/drivers/net/wireless/ath5k/debug.h
@@ -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
};
On Jan 18, 2008 7:51 AM, Bruno Randolf <[email protected]> wrote:
> 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(-)
>
>
> diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
> index eb00818..3a4bf40 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -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);
>
>
Sure why not,
Acked-by: Luis R. Rodriguez <[email protected]>
Luis
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(-)
diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 878609f..c79066b 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -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)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index bb1745f..784b359 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -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;
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
index 7ba2223..20c9469 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -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 */
2008/1/18, Bruno Randolf <[email protected]>:
> 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(-)
>
>
> diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
> index 878609f..c79066b 100644
> --- a/drivers/net/wireless/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath5k/ath5k.h
> @@ -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)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index bb1745f..784b359 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -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;
> diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
> index 7ba2223..20c9469 100644
> --- a/drivers/net/wireless/ath5k/base.h
> +++ b/drivers/net/wireless/ath5k/base.h
> @@ -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]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> + { 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
Hi,
On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> > + * 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)?
>
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.
--
Derek Smithies Ph.D.
IndraNet Technologies Ltd.
Email: [email protected]
ph +64 3 365 6485
Web: http://www.indranet-technologies.com/
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(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 6bbee64..bb1745f 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -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;
On Saturday 19 January 2008 08:26:38 Luis R. Rodriguez wrote:
> On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> > + { 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
On Jan 18, 2008 7:50 AM, Bruno Randolf <[email protected]> wrote:
> 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(-)
>
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 6bbee64..bb1745f 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -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.
> + */
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)?
> + 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
On Friday 18 January 2008 21:55:38 Jiri Slaby wrote:
> On 01/18/2008 01:50 PM, Bruno Randolf wrote:
> > * 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]>
> > ---
>
> [...]
>
> > +#define TOGGLE_BIT(_x, _m) (_x) = (_x) & (_m) ? (_x) & ~(_m) : (_x) |
> > (_m)
>
> simple XOR :)?
>
> _x ^= _m;
oh yeah, looks much better :)
thanks!
bruno
On 01/18/2008 01:50 PM, Bruno Randolf wrote:
> * 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]>
> ---
[...]
> +#define TOGGLE_BIT(_x, _m) (_x) = (_x) & (_m) ? (_x) & ~(_m) : (_x) | (_m)
simple XOR :)?
_x ^= _m;
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(-)
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index eb00818..3a4bf40 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -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);
* 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(-)
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 742616a..6bbee64 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -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++;
diff --git a/drivers/net/wireless/ath5k/debug.c b/drivers/net/wireless/ath5k/debug.c
index 4ba649e..63e39f9 100644
--- a/drivers/net/wireless/ath5k/debug.c
+++ b/drivers/net/wireless/ath5k/debug.c
@@ -314,6 +314,76 @@ static const struct file_operations fops_reset = {
};
+/* debugfs: debug level */
+
+static struct {
+ enum ath5k_debug_level level;
+ const char *name;
+ const char *desc;
+} dbg_info[] = {
+ { ATH5K_DEBUG_RESET, "reset", "reset and initialization" },
+ { ATH5K_DEBUG_INTR, "intr", "interrupt handling" },
+ { ATH5K_DEBUG_MODE, "mode", "mode init/setup" },
+ { ATH5K_DEBUG_XMIT, "xmit", "basic xmit operation" },
+ { ATH5K_DEBUG_BEACON, "beacon", "beacon handling" },
+ { ATH5K_DEBUG_CALIBRATE, "calib", "periodic calibration" },
+ { ATH5K_DEBUG_TXPOWER, "txpower", "transmit power setting" },
+ { ATH5K_DEBUG_LED, "led", "LED mamagement" },
+ { ATH5K_DEBUG_DUMP_RX, "dumprx", "print received skb content" },
+ { ATH5K_DEBUG_DUMP_TX, "dumptx", "print transmit skb content" },
+ { ATH5K_DEBUG_DUMPMODES, "dumpmodes", "dump modes" },
+ { ATH5K_DEBUG_TRACE, "trace", "trace function calls" },
+ { ATH5K_DEBUG_ANY, "all", "show all debug levels" },
+};
+
+static ssize_t read_file_debug(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath5k_softc *sc = file->private_data;
+ char buf[1000];
+ int len = 0;
+ int i;
+
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "DEBUG LEVEL: 0x%08x\n\n", sc->debug.level);
+
+ for (i = 0; i < ARRAY_SIZE(dbg_info) - 1; i++) {
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "%10s %c 0x%08x - %s\n", dbg_info[i].name,
+ sc->debug.level & dbg_info[i].level ? '+' : ' ',
+ dbg_info[i].level, dbg_info[i].desc);
+ }
+ len += snprintf(buf+len, sizeof(buf)-len,
+ "%10s %c 0x%08x - %s\n", dbg_info[i].name,
+ sc->debug.level == dbg_info[i].level ? '+' : ' ',
+ dbg_info[i].level, dbg_info[i].desc);
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+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;
+
+ for (i = 0; i < ARRAY_SIZE(dbg_info); i++) {
+ if (strncmp(userbuf, dbg_info[i].name,
+ strlen(dbg_info[i].name)) == 0)
+ sc->debug.level ^= dbg_info[i].level; /* toggle bit */
+ }
+ return count;
+}
+
+static const struct file_operations fops_debug = {
+ .read = read_file_debug,
+ .write = write_file_debug,
+ .open = ath5k_debugfs_open,
+ .owner = THIS_MODULE,
+};
+
+
/* init */
void
@@ -326,26 +396,24 @@ void
ath5k_debug_init_device(struct ath5k_softc *sc)
{
sc->debug.level = ath5k_debug;
+
sc->debug.debugfs_phydir = debugfs_create_dir(wiphy_name(sc->hw->wiphy),
- ath5k_global_debugfs);
- sc->debug.debugfs_debug = debugfs_create_u32("debug",
- 0666, sc->debug.debugfs_phydir, &sc->debug.level);
+ ath5k_global_debugfs);
+
+ sc->debug.debugfs_debug = debugfs_create_file("debug", 0666,
+ sc->debug.debugfs_phydir, sc, &fops_debug);
sc->debug.debugfs_registers = debugfs_create_file("registers", 0444,
- sc->debug.debugfs_phydir,
- sc, &fops_registers);
+ sc->debug.debugfs_phydir, sc, &fops_registers);
sc->debug.debugfs_tsf = debugfs_create_file("tsf", 0666,
- sc->debug.debugfs_phydir,
- sc, &fops_tsf);
+ sc->debug.debugfs_phydir, sc, &fops_tsf);
sc->debug.debugfs_beacon = debugfs_create_file("beacon", 0666,
- sc->debug.debugfs_phydir,
- sc, &fops_beacon);
+ sc->debug.debugfs_phydir, sc, &fops_beacon);
sc->debug.debugfs_reset = debugfs_create_file("reset", 0222,
- sc->debug.debugfs_phydir,
- sc, &fops_reset);
+ sc->debug.debugfs_phydir, sc, &fops_reset);
}
void
@@ -415,8 +483,7 @@ ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah)
struct ath5k_buf *bf;
int status;
- if (likely(!(sc->debug.level &
- (ATH5K_DEBUG_RESET | ATH5K_DEBUG_FATAL))))
+ if (likely(!(sc->debug.level & ATH5K_DEBUG_RESET)))
return;
printk(KERN_DEBUG "rx queue %x, link %p\n",
@@ -426,7 +493,7 @@ ath5k_debug_printrxbuffs(struct ath5k_softc *sc, struct ath5k_hw *ah)
list_for_each_entry(bf, &sc->rxbuf, list) {
ds = bf->desc;
status = ah->ah_proc_rx_desc(ah, ds);
- if (!status || (sc->debug.level & ATH5K_DEBUG_FATAL))
+ if (!status)
ath5k_debug_printrxbuf(bf, status == 0);
}
spin_unlock_bh(&sc->rxbuflock);
diff --git a/drivers/net/wireless/ath5k/debug.h b/drivers/net/wireless/ath5k/debug.h
index 2b491cb..c4fd8c4 100644
--- a/drivers/net/wireless/ath5k/debug.h
+++ b/drivers/net/wireless/ath5k/debug.h
@@ -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
};
On Jan 18, 2008 7:51 AM, Bruno Randolf <[email protected]> wrote:
> 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(-)
>
>
> diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
> index 878609f..c79066b 100644
> --- a/drivers/net/wireless/ath5k/ath5k.h
> +++ b/drivers/net/wireless/ath5k/ath5k.h
> @@ -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)
>
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index bb1745f..784b359 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
> @@ -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;
Sure, although as I noted this value will be later set by mac80211
through ath5k_config().
Acked-by: Luis R. Rodriguez <[email protected]>
Luis
2008/1/18, Bruno Randolf <[email protected]>:
> 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(-)
>
>
> diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
> index eb00818..3a4bf40 100644
> --- a/drivers/net/wireless/ath5k/hw.c
> +++ b/drivers/net/wireless/ath5k/hw.c
> @@ -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: Nick Kossifidis <[email protected]>
--
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
On 01/19/2008 03:49 AM, Bruno Randolf wrote:
> * 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(-)
>
>
[...]
> diff --git a/drivers/net/wireless/ath5k/debug.c b/drivers/net/wireless/ath5k/debug.c
> index 4ba649e..63e39f9 100644
> --- a/drivers/net/wireless/ath5k/debug.c
> +++ b/drivers/net/wireless/ath5k/debug.c
> @@ -314,6 +314,76 @@ static const struct file_operations fops_reset = {
[...]
> +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.
> +
> + 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:
> + sc->debug.level ^= dbg_info[i].level; /* toggle bit */
but it's not mandatory at all.
thanks,
--js