ath5k: beacon patches

ath5k: beacon patches

[Bruno Randolf]

hello!

here are some smaller, mostly cosmetic clean-up patches for ath5k concerning
beacons.  i came across these things while trying to make beaconing work for
IBSS mode.  right now (after patch 0005 specifically) beacons are sent, but
there is no backoff between the IBSS stations. for that to work we need more
info from mac80211, especially the timestamp of the last received beacon.  i'll
try to look into that next.

bruno

[PATCH 1/6] ath5k: code cosmetics

[Bruno Randolf]

  * move some enums to kdoc
  * small cosmetic changes of indentation etc

base.[ch]: Changes-licensed-under: 3-clause-BSD
all others: Changes-licensed-under: ISC
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/ath5k.h |   35 +++++++++++++++++++++--------------
 drivers/net/wireless/ath5k/base.c  |    4 ++--
 drivers/net/wireless/ath5k/base.h  |   12 ++++++------
 drivers/net/wireless/ath5k/reg.h   |    6 +++---
 4 files changed, 32 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index a30f28c..47b33d2 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -289,16 +289,22 @@ struct ath5k_tx_status {
 #define AR5K_TXERR_FILT		0x02
 #define AR5K_TXERR_FIFO		0x04
 
-/*
- * Queue types used to classify tx queues.
+/**
+ * enum ath5k_tx_queue - Queue types used to classify tx queues.
+ * @AR5K_TX_QUEUE_INACTIVE: q is unused -- see ath5k_hw_release_tx_queue
+ * @AR5K_TX_QUEUE_DATA: A normal data queue
+ * @AR5K_TX_QUEUE_XR_DATA: An XR-data queue
+ * @AR5K_TX_QUEUE_BEACON: The beacon queue
+ * @AR5K_TX_QUEUE_CAB: The after-beacon queue
+ * @AR5K_TX_QUEUE_UAPSD: Unscheduled Automatic Power Save Delivery queue
  */
 enum ath5k_tx_queue {
-	AR5K_TX_QUEUE_INACTIVE = 0, /* q is unused -- see ath5k_hw_release_tx_queue */
-	AR5K_TX_QUEUE_DATA,	  /*A normal data queue*/
-	AR5K_TX_QUEUE_XR_DATA,	  /*An XR-data queue*/
-	AR5K_TX_QUEUE_BEACON,	  /*The beacon queue*/
-	AR5K_TX_QUEUE_CAB,	  /*The ater-beacon queue*/
-	AR5K_TX_QUEUE_UAPSD,	  /*Unscheduled Automatic Power Save Delivery queue*/
+	AR5K_TX_QUEUE_INACTIVE = 0,
+	AR5K_TX_QUEUE_DATA,
+	AR5K_TX_QUEUE_XR_DATA,
+	AR5K_TX_QUEUE_BEACON,
+	AR5K_TX_QUEUE_CAB,
+	AR5K_TX_QUEUE_UAPSD,
 };
 
 #define	AR5K_NUM_TX_QUEUES		10
@@ -453,18 +459,19 @@ struct ath5k_mib_stats {
 #define AR5K_BEACON_ENA		0x00800000 /*enable beacon xmit*/
 #define AR5K_BEACON_RESET_TSF	0x01000000 /*force a TSF reset*/
 
-/*
- * Per-station beacon timer state.
+/**
+ * struct ath5k_beacon_state - Per-station beacon timer state.
+ * @bs_interval: in TU's, can also include the above flags
+ * @bs_cfp_max_duration: if non-zero hw is setup to coexist with a
+ * 	Point Coordination Function capable AP
  */
 struct ath5k_beacon_state {
 	u32	bs_next_beacon;
 	u32	bs_next_dtim;
-	u32	bs_interval;		/*in TU's -see net80211/ieee80211_var.h-
-						can also include the above flags*/
+	u32	bs_interval;
 	u8	bs_dtim_period;
 	u8	bs_cfp_period;
-	u16	bs_cfp_max_duration;	/*if non-zero hw is setup to coexist with
-						a Point Coordination Function capable AP*/
+	u16	bs_cfp_max_duration;
 	u16	bs_cfp_du_remain;
 	u16	bs_tim_offset;
 	u16	bs_sleep_duration;
diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index ff5117c..8d5ffcd 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1446,7 +1446,7 @@ ath5k_beaconq_config(struct ath5k_softc *sc)
 	if (ret)
 		return ret;
 	if (sc->opmode == IEEE80211_IF_TYPE_AP ||
-			sc->opmode == IEEE80211_IF_TYPE_IBSS) {
+	    sc->opmode == IEEE80211_IF_TYPE_IBSS) {
 		/*
 		 * Always burst out beacon and CAB traffic
 		 * (aifs = cwmin = cwmax = 0)
@@ -1504,7 +1504,7 @@ ath5k_txq_cleanup(struct ath5k_softc *sc)
 	/* XXX return value */
 	if (likely(!test_bit(ATH_STAT_INVALID, sc->status))) {
 		/* don't touch the hardware if marked invalid */
-		(void)ath5k_hw_stop_tx_dma(ah, sc->bhalq);
+		ath5k_hw_stop_tx_dma(ah, sc->bhalq);
 		ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "beacon queue %x\n",
 			ath5k_hw_get_tx_buf(ah, sc->bhalq));
 		for (i = 0; i < ARRAY_SIZE(sc->txqs); i++)
diff --git a/drivers/net/wireless/ath5k/base.h b/drivers/net/wireless/ath5k/base.h
index fce947c..7ba2223 100644
--- a/drivers/net/wireless/ath5k/base.h
+++ b/drivers/net/wireless/ath5k/base.h
@@ -56,7 +56,7 @@
 struct ath5k_buf {
 	struct list_head	list;
 	unsigned int		flags;	/* tx descriptor flags */
-	struct ath5k_desc		*desc;	/* virtual addr of desc */
+	struct ath5k_desc	*desc;	/* virtual addr of desc */
 	dma_addr_t		daddr;	/* physical addr of desc */
 	struct sk_buff		*skb;	/* skbuff for buf */
 	dma_addr_t		skbaddr;/* physical addr of skb data */
@@ -73,11 +73,11 @@ struct ath5k_buf {
  * hardware queue).
  */
 struct ath5k_txq {
-	unsigned int	qnum;		/* hardware q number */
-	u32		*link;		/* link ptr in last TX desc */
-	struct list_head q;		/* transmit queue */
-	spinlock_t	lock;		/* lock on q and link */
-	bool		setup;
+	unsigned int		qnum;	/* hardware q number */
+	u32			*link;	/* link ptr in last TX desc */
+	struct list_head	q;	/* transmit queue */
+	spinlock_t		lock;	/* lock on q and link */
+	bool			setup;
 };
 
 #if CHAN_DEBUG
diff --git a/drivers/net/wireless/ath5k/reg.h b/drivers/net/wireless/ath5k/reg.h
index bdd10a7..2f41c83 100644
--- a/drivers/net/wireless/ath5k/reg.h
+++ b/drivers/net/wireless/ath5k/reg.h
@@ -83,7 +83,7 @@
 /*
  * Interrupt enable register
  */
-#define AR5K_IER		0x0024			/* Register Address */
+#define AR5K_IER		0x0024		/* Register Address */
 #define AR5K_IER_DISABLE	0x00000000	/* Disable card interrupts */
 #define AR5K_IER_ENABLE		0x00000001	/* Enable card interrupts */
 
@@ -96,7 +96,7 @@
 /*
  * Beacon control register [5210]
  */
-#define AR5K_BCR		0x0028			/* Register Address */
+#define AR5K_BCR		0x0028		/* Register Address */
 #define AR5K_BCR_AP		0x00000000	/* AP mode */
 #define AR5K_BCR_ADHOC		0x00000001	/* Ad-Hoc mode */
 #define AR5K_BCR_BDMAE		0x00000002	/* DMA enable */
@@ -107,7 +107,7 @@
 /*
  * First RTS duration register [5211]
  */
-#define AR5K_RTSD0		0x0028			/* Register Address */
+#define AR5K_RTSD0		0x0028		/* Register Address */
 #define	AR5K_RTSD0_6		0x000000ff	/* 6Mb RTS duration mask (?) */
 #define	AR5K_RTSD0_6_S		0		/* 6Mb RTS duration shift (?) */
 #define	AR5K_RTSD0_9		0x0000ff00	/* 9Mb*/
-- 
1.5.3.4

[PATCH 2/6] ath5k: rename function ath5k_hw_wait_for_beacon

[Bruno Randolf]

rename to ath5k_hw_beaconq_finish, since this is more clear.
this function is unused at the moment

Changes-licensed-under: ISC
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/ath5k.h |    2 +-
 drivers/net/wireless/ath5k/hw.c    |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 47b33d2..8af2545 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -1084,7 +1084,7 @@ extern void ath5k_hw_reset_tsf(struct ath5k_hw *ah);
 extern void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval);
 extern int ath5k_hw_set_beacon_timers(struct ath5k_hw *ah, const struct ath5k_beacon_state *state);
 extern void ath5k_hw_reset_beacon(struct ath5k_hw *ah);
-extern int ath5k_hw_wait_for_beacon(struct ath5k_hw *ah, unsigned long phys_addr);
+extern int ath5k_hw_beaconq_finish(struct ath5k_hw *ah, unsigned long phys_addr);
 extern void ath5k_hw_update_mib_counters(struct ath5k_hw *ah, struct ath5k_mib_stats *statistics);
 /* ACK bit rate */
 void ath5k_hw_set_ack_bitrate_high(struct ath5k_hw *ah, bool high);
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index 402b868..5ca2563 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -2776,9 +2776,8 @@ void ath5k_hw_reset_beacon(struct ath5k_hw *ah)
 
 /*
  * Wait for beacon queue to finish
- * TODO: This function's name is misleading, rename
  */
-int ath5k_hw_wait_for_beacon(struct ath5k_hw *ah, unsigned long phys_addr)
+int ath5k_hw_beaconq_finish(struct ath5k_hw *ah, unsigned long phys_addr)
 {
 	unsigned int i;
 	int ret;
-- 
1.5.3.4

[PATCH 3/6] ath5k: comment out unused beacon functions and structures

[Bruno Randolf]

use '#if 0' to comment out unused functions and structures wrt beacons. we
might need that info later but right now they are useless and probably need to
be rewritten.

Changes-licensed-under: ISC
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/ath5k.h |    4 ++++
 drivers/net/wireless/ath5k/hw.c    |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 8af2545..0b6694b 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -459,6 +459,7 @@ struct ath5k_mib_stats {
 #define AR5K_BEACON_ENA		0x00800000 /*enable beacon xmit*/
 #define AR5K_BEACON_RESET_TSF	0x01000000 /*force a TSF reset*/
 
+#if 0
 /**
  * struct ath5k_beacon_state - Per-station beacon timer state.
  * @bs_interval: in TU's, can also include the above flags
@@ -478,6 +479,7 @@ struct ath5k_beacon_state {
 	u16	bs_bmiss_threshold;
 	u32  	bs_cfp_next;
 };
+#endif
 
 
 
@@ -1082,9 +1084,11 @@ extern u32 ath5k_hw_get_tsf32(struct ath5k_hw *ah);
 extern u64 ath5k_hw_get_tsf64(struct ath5k_hw *ah);
 extern void ath5k_hw_reset_tsf(struct ath5k_hw *ah);
 extern void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval);
+#if 0
 extern int ath5k_hw_set_beacon_timers(struct ath5k_hw *ah, const struct ath5k_beacon_state *state);
 extern void ath5k_hw_reset_beacon(struct ath5k_hw *ah);
 extern int ath5k_hw_beaconq_finish(struct ath5k_hw *ah, unsigned long phys_addr);
+#endif
 extern void ath5k_hw_update_mib_counters(struct ath5k_hw *ah, struct ath5k_mib_stats *statistics);
 /* ACK bit rate */
 void ath5k_hw_set_ack_bitrate_high(struct ath5k_hw *ah, bool high);
diff --git a/drivers/net/wireless/ath5k/hw.c b/drivers/net/wireless/ath5k/hw.c
index 5ca2563..64ab950 100644
--- a/drivers/net/wireless/ath5k/hw.c
+++ b/drivers/net/wireless/ath5k/hw.c
@@ -2628,6 +2628,7 @@ void ath5k_hw_init_beacon(struct ath5k_hw *ah, u32 next_beacon, u32 interval)
 		AR5K_BEACON);
 }
 
+#if 0
 /*
  * Set beacon timers
  */
@@ -2822,6 +2823,7 @@ int ath5k_hw_beaconq_finish(struct ath5k_hw *ah, unsigned long phys_addr)
 
 	return ret;
 }
+#endif
 
 /*
  * Update mib counters (statistics)
-- 
1.5.3.4

[PATCH 4/6] ath5k: centrally define TSF to TU conversion macro

[Bruno Randolf]

centrally define TSF_TO_TO macro for TSF to TU conversion and use it in all
instances. there will be more usage for this later.

ath5k.h: Changes-licensed-under: ISC
all others: Changes-licensed-under: 3-clause-BSD
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/ath5k.h |    9 +++++++++
 drivers/net/wireless/ath5k/base.c  |    4 +---
 drivers/net/wireless/ath5k/debug.c |    2 +-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath5k/ath5k.h b/drivers/net/wireless/ath5k/ath5k.h
index 0b6694b..52787ea 100644
--- a/drivers/net/wireless/ath5k/ath5k.h
+++ b/drivers/net/wireless/ath5k/ath5k.h
@@ -482,6 +482,15 @@ struct ath5k_beacon_state {
 #endif
 
 
+/*
+ * 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)
+ */
+#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 8d5ffcd..30b16fd 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1989,7 +1989,6 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 static void
 ath5k_beacon_config(struct ath5k_softc *sc)
 {
-#define TSF_TO_TU(_h, _l)	(((_h) << 22) | ((_l) >> 10))
 	struct ath5k_hw *ah = sc->ah;
 	u32 uninitialized_var(nexttbtt), intval, tsftu;
 	u64 tsf;
@@ -2000,7 +1999,7 @@ ath5k_beacon_config(struct ath5k_softc *sc)
 
 	/* current TSF converted to TU */
 	tsf = ath5k_hw_get_tsf64(ah);
-	tsftu = TSF_TO_TU((u32)(tsf >> 32), (u32)tsf);
+	tsftu = TSF_TO_TU(tsf);
 
 	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "intval %u hw tsftu %u\n",
 		intval, tsftu);
@@ -2051,7 +2050,6 @@ ath5k_beacon_config(struct ath5k_softc *sc)
 				ath5k_hw_hasveol(ah))
 			ath5k_beacon_send(sc);
 	}
-#undef TSF_TO_TU
 }
 
 /********************\
diff --git a/drivers/net/wireless/ath5k/debug.c b/drivers/net/wireless/ath5k/debug.c
index fcdd36e..4ba649e 100644
--- a/drivers/net/wireless/ath5k/debug.c
+++ b/drivers/net/wireless/ath5k/debug.c
@@ -266,7 +266,7 @@ static ssize_t read_file_beacon(struct file *file, char __user *user_buf,
 
 	tsf = ath5k_hw_get_tsf64(sc->ah);
 	len += snprintf(buf+len, sizeof(buf)-len,
-		"TSF\t\t0x%016llx\tTU: %08x\n", tsf, (u32)(tsf >> 10));
+		"TSF\t\t0x%016llx\tTU: %08x\n", tsf, TSF_TO_TU(tsf));
 
 	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
 }
-- 
1.5.3.4

[PATCH 5/6] ath5k: configure beacons

[Bruno Randolf]

call ath5k_beacon_config() on beacon_update and after channel changes. this is
necessary to ensure beacons are sent.

Changes-licensed-under: 3-clause-BSD
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/base.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 30b16fd..92361df 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1056,6 +1056,7 @@ ath5k_chan_set(struct ath5k_softc *sc, struct ieee80211_channel *chan)
 		 */
 /*		ath5k_chan_change(sc, chan); */
 
+		ath5k_beacon_config(sc);
 		/*
 		 * Re-enable interrupts.
 		 */
@@ -2813,6 +2814,8 @@ ath5k_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb,
 	ret = ath5k_beacon_setup(sc, sc->bbuf, ctl);
 	if (ret)
 		sc->bbuf->skb = NULL;
+	else
+		ath5k_beacon_config(sc);
 
 end:
 	mutex_unlock(&sc->lock);
-- 
1.5.3.4

[PATCH 6/6] ath5k: simplify beacon configuration

[Bruno Randolf]

make ath5k_beacon_config() clearer and move timer configuration into a seperate
function. it will be needed later when we can detect HW merges.

Changes-licensed-under: 3-clause-BSD
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/base.c |  105 +++++++++++++++++-------------------
 1 files changed, 50 insertions(+), 55 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 92361df..4f1ff1a 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1972,23 +1972,9 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	sc->bsent++;
 }
 
-/*
- * Configure the beacon and sleep timers.
- *
- * When operating as an AP this resets the TSF and sets
- * up the hardware to notify us when we need to issue beacons.
- *
- * When operating in station mode this sets up the beacon
- * timers according to the timestamp of the last received
- * beacon and the current TSF, configures PCF and DTIM
- * handling, programs the sleep registers so the hardware
- * will wakeup in time to receive beacons, and configures
- * the beacon miss handling so we'll receive a BMISS
- * interrupt when we stop seeing beacons from the AP
- * we've associated with.
- */
+
 static void
-ath5k_beacon_config(struct ath5k_softc *sc)
+ath5k_beacon_update_timers(struct ath5k_softc *sc)
 {
 	struct ath5k_hw *ah = sc->ah;
 	u32 uninitialized_var(nexttbtt), intval, tsftu;
@@ -2002,57 +1988,66 @@ ath5k_beacon_config(struct ath5k_softc *sc)
 	tsf = ath5k_hw_get_tsf64(ah);
 	tsftu = TSF_TO_TU(tsf);
 
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "intval %u hw tsftu %u\n",
-		intval, tsftu);
+	/*
+	 * Pull nexttbtt forward to reflect the current
+	 * TSF. Add one intval otherwise the timespan
+	 * can be too short for ibss merges.
+	 */
+	nexttbtt = tsftu + 2 * intval;
 
-	if (sc->opmode == IEEE80211_IF_TYPE_STA ||
-			(sc->opmode == IEEE80211_IF_TYPE_IBSS &&
-				!sc->bbuf->skb)) {
-		ath5k_hw_set_intr(ah, 0);
-		sc->imask |= AR5K_INT_BMISS;
-		sc->bmisscount = 0;
-		ath5k_hw_set_intr(ah, sc->imask);
-	} else if (sc->opmode == IEEE80211_IF_TYPE_IBSS /* TODO || AP */) {
-		ath5k_hw_set_intr(ah, 0);
-		if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {
-			/*
-			 * Pull nexttbtt forward to reflect the current
-			 * TSF. Add one intval otherwise the timespan
-			 * can be too short for ibss merges.
-			 */
-			nexttbtt = tsftu + 2 * intval;
+	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
+		"hw tsftu %u nexttbtt %u intval %u\n", tsftu, nexttbtt, intval);
 
-			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "nexttbtt %u "
-				"intval %u\n", nexttbtt, intval);
+	intval |= AR5K_BEACON_ENA;
 
-			/*
-			 * In IBSS mode enable the beacon timers but only
-			 * enable SWBA interrupts if we need to manually
-			 * prepare beacon frames.  Otherwise we use a
-			 * self-linked tx descriptor and let the hardware
-			 * deal with things.
-			 */
-			if (!ath5k_hw_hasveol(ah))
-				sc->imask |= AR5K_INT_SWBA;
-		} /* TODO else AP */
+	ath5k_hw_init_beacon(ah, nexttbtt, intval);
+}
 
-		intval |= AR5K_BEACON_ENA;
 
-		ath5k_beaconq_config(sc);
-		ath5k_hw_init_beacon(ah, nexttbtt, intval);
+/*
+ * Configure the beacon timers and interrupts based on the operating mode
+ *
+ * When operating in station mode we want to receive a BMISS interrupt when we
+ * stop seeing beacons from the AP we've associated with so we can look for
+ * another AP to associate with.
+ *
+ * In IBSS mode we need to configure the beacon timers and use a self-linked tx
+ * descriptor if possible. If the hardware cannot deal with that we enable SWBA
+ * interrupts to send the beacons from the interrupt handler.
+ */
+static void
+ath5k_beacon_config(struct ath5k_softc *sc)
+{
+	struct ath5k_hw *ah = sc->ah;
 
-		sc->bmisscount = 0;
-		ath5k_hw_set_intr(ah, sc->imask);
+	ath5k_hw_set_intr(ah, 0);
+	sc->bmisscount = 0;
+
+	if (sc->opmode == IEEE80211_IF_TYPE_STA) {
+		sc->imask |= AR5K_INT_BMISS;
+	}
+	else if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {
 		/*
-		 * When using a self-linked beacon descriptor in
-		 * ibss mode load it once here.
+		 * In IBSS mode enable the beacon timers but only enable SWBA
+		 * interrupts if we need to manually prepare beacon frames.
+		 * Otherwise we use a self-linked tx descriptor and let the
+		 * hardware deal with things. In that case we have to load it
+		 * only once here.
 		 */
-		if (sc->opmode == IEEE80211_IF_TYPE_IBSS &&
-				ath5k_hw_hasveol(ah))
+		ath5k_beaconq_config(sc);
+		ath5k_beacon_update_timers(sc);
+
+		if (!ath5k_hw_hasveol(ah))
+			sc->imask |= AR5K_INT_SWBA;
+		else
 			ath5k_beacon_send(sc);
 	}
+	/* TODO else AP */
+
+	ath5k_hw_set_intr(ah, sc->imask);
 }
 
+
 /********************\
 * Interrupt handling *
 \********************/
-- 
1.5.3.4

Re: [PATCH 6/6] ath5k: simplify beacon configuration

[Jiri Slaby]

On 01/09/2008 10:16 AM, Bruno Randolf wrote:
> make ath5k_beacon_config() clearer and move timer configuration into a seperate
> function. it will be needed later when we can detect HW merges.
> 
> Changes-licensed-under: 3-clause-BSD
> Signed-off-by: Bruno Randolf <bruno@thinktube.com>
> ---
>  drivers/net/wireless/ath5k/base.c |  105 +++++++++++++++++-------------------
>  1 files changed, 50 insertions(+), 55 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
> index 92361df..4f1ff1a 100644
> --- a/drivers/net/wireless/ath5k/base.c
> +++ b/drivers/net/wireless/ath5k/base.c
[...]
> +/*
> + * Configure the beacon timers and interrupts based on the operating mode
> + *
> + * When operating in station mode we want to receive a BMISS interrupt when we
> + * stop seeing beacons from the AP we've associated with so we can look for
> + * another AP to associate with.
> + *
> + * In IBSS mode we need to configure the beacon timers and use a self-linked tx
> + * descriptor if possible. If the hardware cannot deal with that we enable SWBA
> + * interrupts to send the beacons from the interrupt handler.
> + */
> +static void
> +ath5k_beacon_config(struct ath5k_softc *sc)
> +{
> +	struct ath5k_hw *ah = sc->ah;
>  
> -		sc->bmisscount = 0;
> -		ath5k_hw_set_intr(ah, sc->imask);
> +	ath5k_hw_set_intr(ah, 0);
> +	sc->bmisscount = 0;
> +
> +	if (sc->opmode == IEEE80211_IF_TYPE_STA) {
> +		sc->imask |= AR5K_INT_BMISS;
> +	}
> +	else if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {

checkpatch doesn't like this, please put the else path next time on the same 
line as } is.

the output:
WARNING: braces {} are not necessary for single statement blocks
#144: FILE: drivers/net/wireless/ath5k/base.c:2026:
+       if (sc->opmode == IEEE80211_IF_TYPE_STA) {
+               sc->imask |= AR5K_INT_BMISS;
+       }

ERROR: else should follow close brace '}'
#147: FILE: drivers/net/wireless/ath5k/base.c:2029:
+       }
+       else if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {

total: 1 errors, 1 warnings, 130 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Re: [PATCH 6/6] ath5k: simplify beacon configuration

[bruno randolf]

On Wednesday 09 January 2008 18:32:08 Jiri Slaby wrote:
> checkpatch doesn't like this, please put the else path next time on the
> same line as } is.

oh, sorry i forgot checkpatch this time. i will resend.

bruno

[PATCH] ath5k: simplify beacon configuration

[Bruno Randolf]

make ath5k_beacon_config() clearer and move timer configuration into a seperate
function. it will be needed later when we can detect HW merges.

Changes-licensed-under: 3-clause-BSD
Signed-off-by: Bruno Randolf <bruno@thinktube.com>
---
 drivers/net/wireless/ath5k/base.c |  104 +++++++++++++++++-------------------
 1 files changed, 49 insertions(+), 55 deletions(-)

diff --git a/drivers/net/wireless/ath5k/base.c b/drivers/net/wireless/ath5k/base.c
index 92361df..f9744cd 100644
--- a/drivers/net/wireless/ath5k/base.c
+++ b/drivers/net/wireless/ath5k/base.c
@@ -1972,23 +1972,9 @@ ath5k_beacon_send(struct ath5k_softc *sc)
 	sc->bsent++;
 }
 
-/*
- * Configure the beacon and sleep timers.
- *
- * When operating as an AP this resets the TSF and sets
- * up the hardware to notify us when we need to issue beacons.
- *
- * When operating in station mode this sets up the beacon
- * timers according to the timestamp of the last received
- * beacon and the current TSF, configures PCF and DTIM
- * handling, programs the sleep registers so the hardware
- * will wakeup in time to receive beacons, and configures
- * the beacon miss handling so we'll receive a BMISS
- * interrupt when we stop seeing beacons from the AP
- * we've associated with.
- */
+
 static void
-ath5k_beacon_config(struct ath5k_softc *sc)
+ath5k_beacon_update_timers(struct ath5k_softc *sc)
 {
 	struct ath5k_hw *ah = sc->ah;
 	u32 uninitialized_var(nexttbtt), intval, tsftu;
@@ -2002,57 +1988,65 @@ ath5k_beacon_config(struct ath5k_softc *sc)
 	tsf = ath5k_hw_get_tsf64(ah);
 	tsftu = TSF_TO_TU(tsf);
 
-	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "intval %u hw tsftu %u\n",
-		intval, tsftu);
+	/*
+	 * Pull nexttbtt forward to reflect the current
+	 * TSF. Add one intval otherwise the timespan
+	 * can be too short for ibss merges.
+	 */
+	nexttbtt = tsftu + 2 * intval;
 
-	if (sc->opmode == IEEE80211_IF_TYPE_STA ||
-			(sc->opmode == IEEE80211_IF_TYPE_IBSS &&
-				!sc->bbuf->skb)) {
-		ath5k_hw_set_intr(ah, 0);
-		sc->imask |= AR5K_INT_BMISS;
-		sc->bmisscount = 0;
-		ath5k_hw_set_intr(ah, sc->imask);
-	} else if (sc->opmode == IEEE80211_IF_TYPE_IBSS /* TODO || AP */) {
-		ath5k_hw_set_intr(ah, 0);
-		if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {
-			/*
-			 * Pull nexttbtt forward to reflect the current
-			 * TSF. Add one intval otherwise the timespan
-			 * can be too short for ibss merges.
-			 */
-			nexttbtt = tsftu + 2 * intval;
+	ATH5K_DBG(sc, ATH5K_DEBUG_BEACON,
+		"hw tsftu %u nexttbtt %u intval %u\n", tsftu, nexttbtt, intval);
 
-			ATH5K_DBG(sc, ATH5K_DEBUG_BEACON, "nexttbtt %u "
-				"intval %u\n", nexttbtt, intval);
+	intval |= AR5K_BEACON_ENA;
 
-			/*
-			 * In IBSS mode enable the beacon timers but only
-			 * enable SWBA interrupts if we need to manually
-			 * prepare beacon frames.  Otherwise we use a
-			 * self-linked tx descriptor and let the hardware
-			 * deal with things.
-			 */
-			if (!ath5k_hw_hasveol(ah))
-				sc->imask |= AR5K_INT_SWBA;
-		} /* TODO else AP */
+	ath5k_hw_init_beacon(ah, nexttbtt, intval);
+}
 
-		intval |= AR5K_BEACON_ENA;
 
-		ath5k_beaconq_config(sc);
-		ath5k_hw_init_beacon(ah, nexttbtt, intval);
+/*
+ * Configure the beacon timers and interrupts based on the operating mode
+ *
+ * When operating in station mode we want to receive a BMISS interrupt when we
+ * stop seeing beacons from the AP we've associated with so we can look for
+ * another AP to associate with.
+ *
+ * In IBSS mode we need to configure the beacon timers and use a self-linked tx
+ * descriptor if possible. If the hardware cannot deal with that we enable SWBA
+ * interrupts to send the beacons from the interrupt handler.
+ */
+static void
+ath5k_beacon_config(struct ath5k_softc *sc)
+{
+	struct ath5k_hw *ah = sc->ah;
 
-		sc->bmisscount = 0;
-		ath5k_hw_set_intr(ah, sc->imask);
+	ath5k_hw_set_intr(ah, 0);
+	sc->bmisscount = 0;
+
+	if (sc->opmode == IEEE80211_IF_TYPE_STA) {
+		sc->imask |= AR5K_INT_BMISS;
+	} else if (sc->opmode == IEEE80211_IF_TYPE_IBSS) {
 		/*
-		 * When using a self-linked beacon descriptor in
-		 * ibss mode load it once here.
+		 * In IBSS mode enable the beacon timers but only enable SWBA
+		 * interrupts if we need to manually prepare beacon frames.
+		 * Otherwise we use a self-linked tx descriptor and let the
+		 * hardware deal with things. In that case we have to load it
+		 * only once here.
 		 */
-		if (sc->opmode == IEEE80211_IF_TYPE_IBSS &&
-				ath5k_hw_hasveol(ah))
+		ath5k_beaconq_config(sc);
+		ath5k_beacon_update_timers(sc);
+
+		if (!ath5k_hw_hasveol(ah))
+			sc->imask |= AR5K_INT_SWBA;
+		else
 			ath5k_beacon_send(sc);
 	}
+	/* TODO else AP */
+
+	ath5k_hw_set_intr(ah, sc->imask);
 }
 
+
 /********************\
 * Interrupt handling *
 \********************/
-- 
1.5.3.4

Re: [PATCH 1/6] ath5k: code cosmetics

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 233 bytes --]


> + * @AR5K_TX_QUEUE_UAPSD: Unscheduled Automatic Power Save Delivery queue

Out of curiosity, do you know how that works? Is that queue simply a way
to avoid the MAC rejecting a packet because the target is in PS?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 1/6] ath5k: code cosmetics

[Nick Kossifidis]

2008/1/10, Johannes Berg <johannes@sipsolutions.net>:
>
> > + * @AR5K_TX_QUEUE_UAPSD: Unscheduled Automatic Power Save Delivery queue
>
> Out of curiosity, do you know how that works? Is that queue simply a way
> to avoid the MAC rejecting a packet because the target is in PS?
>
> johannes
>
>

Sort off, this is what i found with a quick look on net80211...
http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_power.c
http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_input.c#L563
http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_output.c#L264

-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [PATCH 1/6] ath5k: code cosmetics

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]


> Sort off, this is what i found with a quick look on net80211...
> http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_power.c
> http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_input.c#L563
> http://madwifi.org/browser/madwifi/trunk/net80211/ieee80211_output.c#L264

Yeah, I know how u-apsd works, but I'm a bit confused that you need a
separate queue for it.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 3/6] ath5k: comment out unused beacon functions and structures

[Luis R. Rodriguez]

On Jan 9, 2008 4:16 AM, Bruno Randolf <bruno@thinktube.com> wrote:
> use '#if 0' to comment out unused functions and structures wrt beacons. we
> might need that info later but right now they are useless and probably need to
> be rewritten.

Instead you should nuke this stuff and add it back when you need it.

  Luis