linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf
@ 2008-01-20  5:37 bruno randolf
  2008-01-20  8:46 ` Luis R. Rodriguez
  0 siblings, 1 reply; 6+ messages in thread
From: bruno randolf @ 2008-01-20  5:37 UTC (permalink / raw)
  To: Derek Smithies
  Cc: Luis R. Rodriguez, jirislaby, Michael Wu, ath5k-devel,
	linux-wireless, linville, Johannes Berg

On Sunday 20 January 2008 11:12:11 Derek Smithies wrote:
> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> > On Jan 18, 2008 7:50 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* always extend the mac timestam=
p, since this
> > > information is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* also needed for=
 proper IBSS merging.
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*
> > > + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* XXX: it might be too late to d=
o it here, since
> > > rs_tstamp is + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 15bit only. that =
means TSF extension
> > > has to be done within + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* 32.768us=
ec =3D 32ms. it might be
> > > necessary to move this to the + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* =
interrupt handler,
> > > like it is done in madwifi. + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> >
> > 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 microsecon=
d
> > (us). For 15 bits that would mean 32768 us so don't you mean it sho=
uld
> > be done within 32.768 ms instead of usec (or us)?
>
> Hi,
> =A0it is a typo.

it's just a different way to use a "." to denote 1000. americans might
write 32,768usec, im not sure, different styles worldwide... what i mea=
n
is 32768us equals about 32ms. i'll remove that dot to make it unambigou=
s.

> You are correct. It should be done within 32.768 ms. On a high end la=
ptop,
> 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 h=
ave
> wrapped around. this is unfortunate.

i know that this might happen, and that's why i included this comment. =
i
was just too lazy to make the same act as is done in madwifi (lopping t=
hru
all rx descriptors in the interrupt handler). the current code is
sufficient for IBSS testing right now.

also even if we move TSF extension into the interrupt handler this will
not help in all cases. there are situations in IBSS mode - when a HW me=
rge
(=3D automatic HW TSF update upon receipt of a beacon with a higherr TS=
=46 and
the same BSSID) happens - where the rx timestamp is apparently based on
the old TSF, before the HW TSF is updated. in that case we cannot exten=
d
the timestamp in any meaningful way. i'm not sure how we should handle
this.

> On Sat, 19 Jan 2008, Luis R. Rodriguez wrote:
> Right now we only use mactime and even RX_FLAG_TSFT within mac80211 i=
n
> 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?

see my "[PATCH] mac80211: enable IBSS merging". it is used there to dec=
ide
wether we have to merge IBSS on receipt of a beacon. strictly speaking =
it
would be sufficient to extend the rxstamp only for beacons and in monit=
or
mode, but i thought checking for these cases is more overhead, so why n=
ot
extend TSF all the time...

bruno

-
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf
@ 2008-01-20  9:53 bruno randolf
  2008-01-20  9:57 ` Jiri Slaby
  0 siblings, 1 reply; 6+ messages in thread
From: bruno randolf @ 2008-01-20  9:53 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc:  Derek Smithies , jirislaby, Michael Wu, ath5k-devel,
	linux-wireless, linville, Johannes Berg

On Sunday 20 January 2008 17:46:05 Luis R. Rodriguez wrote:
> Yeah thanks, might be more clear for those of us not used to that
> convention, I didn't even know it existed. Where is this "." for 1000
> notation convention used, just so I know :) ?

in german speaking countries at least (i am austrian), don't really know
where else...

> > > 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.
> >
> > i know that this might happen, and that's why i included this comment. i
> > was just too lazy to make the same act as is done in madwifi (lopping
> > thru all rx descriptors in the interrupt handler). the current code is
> > sufficient for IBSS testing right now.
>
> Thanks for the explanation.
>
> BTW while on the topic of TSF extension, anyone get why we do the tsf
> -= 0x8000 if the hw's tsf's 15 bits value is < the rx descriptor's
> rstamp? This is what ath5k_extend_tsf() does right before the (tsf &
> ~0x7fff) | rstamp.

this is in order to account for situations where the last 15 bit of the
TSF have wrapped around in the mean time, since the rstamp was recorded
with the packet. it is not possible that we received the packet *after*
the current TSF (in the interrupt handler or in the tasklet), so we assume
that the 15bit value has overflown and therefore the time at packet
reception must have been - 0x8000.

> > also even if we move TSF extension into the interrupt handler this will
> > not help in all cases. there are situations in IBSS mode - when a HW
> > merge (= automatic HW TSF update upon receipt of a beacon with a higherr
> > TSF and the same BSSID) happens - where the rx timestamp is apparently
> > based on the old TSF, before the HW TSF is updated. in that case we
> > cannot extend the timestamp in any meaningful way. i'm not sure how we
> > should handle this.
>
> Hmm.. have you seen this with madwifi driver? What do you think is the
> cause? If we don't do proper locking I can see this being an issue on
> the driver side unless this is specifically a hardware issue.

i have seen this in ath5k and other people have seen it in madwifi.
therefore i'm pretty sure it's a HW issue. unfortunately this happens with
ad-hoc beacons where it's most critical to get a correct TSF.

bruno



^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/4] ath5k: debug level improvements
@ 2008-01-18 12:50 Bruno Randolf
  2008-01-18 12:50 ` [PATCH 2/4] ath5k: always extend rx timestamp with tsf Bruno Randolf
  0 siblings, 1 reply; 6+ messages in thread
From: Bruno Randolf @ 2008-01-18 12:50 UTC (permalink / raw)
  To: ath5k-devel; +Cc: mcgrof, jirislaby, mickflemm, linux-wireless, linville

* 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 <bruno@thinktube.com>
---

 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
 };
 


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2008-01-21  6:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-01-20  5:37 [ath5k-devel] [PATCH 2/4] ath5k: always extend rx timestamp with tsf bruno randolf
2008-01-20  8:46 ` Luis R. Rodriguez
2008-01-21  6:32   ` Kalle Valo
  -- strict thread matches above, loose matches on Subject: below --
2008-01-20  9:53 bruno randolf
2008-01-20  9:57 ` Jiri Slaby
2008-01-18 12:50 [PATCH 1/4] ath5k: debug level improvements Bruno Randolf
2008-01-18 12:50 ` [PATCH 2/4] ath5k: always extend rx timestamp with tsf Bruno Randolf
2008-01-19 22:20   ` Luis R. Rodriguez
2008-01-20  2:12     ` [ath5k-devel] " Derek Smithies

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).