[PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Felix Fietkau]

Cold reset is more reliable for getting the hardware out of some specific
stuck states.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/hw.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index 8c840ca..d582cf7 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1386,10 +1386,16 @@ static bool ath9k_hw_set_reset_reg(struct ath_hw *ah, u32 type)
 static bool ath9k_hw_chip_reset(struct ath_hw *ah,
 				struct ath9k_channel *chan)
 {
-	if (AR_SREV_9280(ah) && ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL)) {
-		if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON))
-			return false;
-	} else if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_WARM))
+	int reset_type = ATH9K_RESET_WARM;
+
+	if (AR_SREV_9280(ah)) {
+		if (ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL))
+			reset_type = ATH9K_RESET_POWER_ON;
+		else
+			reset_type = ATH9K_RESET_COLD;
+	}
+
+	if (!ath9k_hw_set_reset_reg(ah, reset_type))
 		return false;
 
 	if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE))
-- 
1.7.3.2

[PATCH 2/5] ath9k: make MAC sample statistics optional

[Felix Fietkau]

They're more expensive than some of the other debug options and only used
in very rare situations, so it sometimes makes sense to disable them while
leaving in debugfs support.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/Kconfig |    8 ++++++++
 drivers/net/wireless/ath/ath9k/debug.c |   10 ++++++++++
 drivers/net/wireless/ath/ath9k/debug.h |   20 +++++++++++++++-----
 drivers/net/wireless/ath/ath9k/init.c  |    4 +++-
 4 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/Kconfig b/drivers/net/wireless/ath/ath9k/Kconfig
index 595a272..e507e78 100644
--- a/drivers/net/wireless/ath/ath9k/Kconfig
+++ b/drivers/net/wireless/ath/ath9k/Kconfig
@@ -81,6 +81,14 @@ config ATH9K_DFS_CERTIFIED
 	  developed. At this point enabling this option won't do anything
 	  except increase code size.
 
+config ATH9K_MAC_DEBUG
+	bool "Atheros MAC statistics"
+	depends on ATH9K_DEBUGFS
+	default y
+	---help---
+	  This option enables collection of statistics for Rx/Tx status
+	  data and some other MAC related statistics
+
 config ATH9K_RATE_CONTROL
 	bool "Atheros ath9k rate control"
 	depends on ATH9K
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 228c181..03a4740 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -818,6 +818,7 @@ void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
 	if (ts->ts_flags & ATH9K_TX_DELIM_UNDERRUN)
 		TX_STAT_INC(qnum, delim_underrun);
 
+#ifdef CONFIG_ATH9K_MAC_DEBUG
 	spin_lock(&sc->debug.samp_lock);
 	TX_SAMP_DBG(jiffies) = jiffies;
 	TX_SAMP_DBG(rssi_ctl0) = ts->ts_rssi_ctl0;
@@ -844,6 +845,7 @@ void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
 
 	sc->debug.tsidx = (sc->debug.tsidx + 1) % ATH_DBG_MAX_SAMPLES;
 	spin_unlock(&sc->debug.samp_lock);
+#endif
 
 #undef TX_SAMP_DBG
 }
@@ -1019,6 +1021,7 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
 
 	sc->debug.stats.rxstats.rs_antenna = rs->rs_antenna;
 
+#ifdef CONFIG_ATH9K_MAC_DEBUG
 	spin_lock(&sc->debug.samp_lock);
 	RX_SAMP_DBG(jiffies) = jiffies;
 	RX_SAMP_DBG(rssi_ctl0) = rs->rs_rssi_ctl0;
@@ -1035,6 +1038,8 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
 	sc->debug.rsidx = (sc->debug.rsidx + 1) % ATH_DBG_MAX_SAMPLES;
 	spin_unlock(&sc->debug.samp_lock);
 
+#endif
+
 #undef RX_STAT_INC
 #undef RX_PHY_ERR_INC
 #undef RX_SAMP_DBG
@@ -1278,6 +1283,8 @@ static const struct file_operations fops_modal_eeprom = {
 	.llseek = default_llseek,
 };
 
+#ifdef CONFIG_ATH9K_MAC_DEBUG
+
 void ath9k_debug_samp_bb_mac(struct ath_softc *sc)
 {
 #define ATH_SAMP_DBG(c) (sc->debug.bb_mac_samp[sc->debug.sampidx].c)
@@ -1551,6 +1558,7 @@ static const struct file_operations fops_samps = {
 	.llseek = default_llseek,
 };
 
+#endif
 
 int ath9k_init_debug(struct ath_hw *ah)
 {
@@ -1604,8 +1612,10 @@ int ath9k_init_debug(struct ath_hw *ah)
 			    &fops_base_eeprom);
 	debugfs_create_file("modal_eeprom", S_IRUSR, sc->debug.debugfs_phy, sc,
 			    &fops_modal_eeprom);
+#ifdef CONFIG_ATH9K_MAC_DEBUG
 	debugfs_create_file("samples", S_IRUSR, sc->debug.debugfs_phy, sc,
 			    &fops_samps);
+#endif
 
 	debugfs_create_u32("gpio_mask", S_IRUSR | S_IWUSR,
 			   sc->debug.debugfs_phy, &sc->sc_ah->gpio_mask);
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 776a24a..7a51958 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -235,16 +235,17 @@ struct ath9k_debug {
 	struct dentry *debugfs_phy;
 	u32 regidx;
 	struct ath_stats stats;
+#ifdef CONFIG_ATH9K_MAC_DEBUG
 	spinlock_t samp_lock;
 	struct ath_dbg_bb_mac_samp bb_mac_samp[ATH_DBG_MAX_SAMPLES];
 	u8 sampidx;
 	u8 tsidx;
 	u8 rsidx;
+#endif
 };
 
 int ath9k_init_debug(struct ath_hw *ah);
 
-void ath9k_debug_samp_bb_mac(struct ath_softc *sc);
 void ath_debug_stat_interrupt(struct ath_softc *sc, enum ath9k_int status);
 void ath_debug_stat_tx(struct ath_softc *sc, struct ath_buf *bf,
 		       struct ath_tx_status *ts, struct ath_txq *txq,
@@ -258,10 +259,6 @@ static inline int ath9k_init_debug(struct ath_hw *ah)
 	return 0;
 }
 
-static inline void ath9k_debug_samp_bb_mac(struct ath_softc *sc)
-{
-}
-
 static inline void ath_debug_stat_interrupt(struct ath_softc *sc,
 					    enum ath9k_int status)
 {
@@ -282,4 +279,17 @@ static inline void ath_debug_stat_rx(struct ath_softc *sc,
 
 #endif /* CONFIG_ATH9K_DEBUGFS */
 
+#ifdef CONFIG_ATH9K_MAC_DEBUG
+
+void ath9k_debug_samp_bb_mac(struct ath_softc *sc);
+
+#else
+
+static inline void ath9k_debug_samp_bb_mac(struct ath_softc *sc)
+{
+}
+
+#endif
+
+
 #endif /* DEBUG_H */
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index d8b0596..944e9b5 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -555,9 +555,11 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
 	mutex_init(&sc->mutex);
 #ifdef CONFIG_ATH9K_DEBUGFS
 	spin_lock_init(&sc->nodes_lock);
-	spin_lock_init(&sc->debug.samp_lock);
 	INIT_LIST_HEAD(&sc->nodes);
 #endif
+#ifdef CONFIG_ATH9K_MAC_DEBUG
+	spin_lock_init(&sc->debug.samp_lock);
+#endif
 	tasklet_init(&sc->intr_tq, ath9k_tasklet, (unsigned long)sc);
 	tasklet_init(&sc->bcon_tasklet, ath_beacon_tasklet,
 		     (unsigned long)sc);
-- 
1.7.3.2

[PATCH 3/5] ath9k: remove rssi/antenna information from recv debug stats

[Felix Fietkau]

The way this is implemented (simply storing the last value) is absolutely
worthless for debugging anything, and the same information is also available
through the MAC sample feature, so there's no point in keeping this around.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/debug.c |   31 -------------------------------
 drivers/net/wireless/ath/ath9k/debug.h |    7 -------
 2 files changed, 0 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 03a4740..c2edf68 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -944,27 +944,6 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,
 	PHY_ERR("HT-RATE ERR", ATH9K_PHYERR_HT_RATE_ILLEGAL);
 
 	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-CTL0",
-			sc->debug.stats.rxstats.rs_rssi_ctl0);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-CTL1",
-			sc->debug.stats.rxstats.rs_rssi_ctl1);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-CTL2",
-			sc->debug.stats.rxstats.rs_rssi_ctl2);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-EXT0",
-			sc->debug.stats.rxstats.rs_rssi_ext0);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-EXT1",
-			sc->debug.stats.rxstats.rs_rssi_ext1);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "RSSI-EXT2",
-			sc->debug.stats.rxstats.rs_rssi_ext2);
-	len += snprintf(buf + len, size - len,
-			"%22s : %10d\n", "Rx Antenna",
-			sc->debug.stats.rxstats.rs_antenna);
-	len += snprintf(buf + len, size - len,
 			"%22s : %10u\n", "RX-Pkts-All",
 			sc->debug.stats.rxstats.rx_pkts_all);
 	len += snprintf(buf + len, size - len,
@@ -1011,16 +990,6 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
 			RX_PHY_ERR_INC(rs->rs_phyerr);
 	}
 
-	sc->debug.stats.rxstats.rs_rssi_ctl0 = rs->rs_rssi_ctl0;
-	sc->debug.stats.rxstats.rs_rssi_ctl1 = rs->rs_rssi_ctl1;
-	sc->debug.stats.rxstats.rs_rssi_ctl2 = rs->rs_rssi_ctl2;
-
-	sc->debug.stats.rxstats.rs_rssi_ext0 = rs->rs_rssi_ext0;
-	sc->debug.stats.rxstats.rs_rssi_ext1 = rs->rs_rssi_ext1;
-	sc->debug.stats.rxstats.rs_rssi_ext2 = rs->rs_rssi_ext2;
-
-	sc->debug.stats.rxstats.rs_antenna = rs->rs_antenna;
-
 #ifdef CONFIG_ATH9K_MAC_DEBUG
 	spin_lock(&sc->debug.samp_lock);
 	RX_SAMP_DBG(jiffies) = jiffies;
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index 7a51958..64fcfad 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -165,13 +165,6 @@ struct ath_rx_stats {
 	u32 post_delim_crc_err;
 	u32 decrypt_busy_err;
 	u32 phy_err_stats[ATH9K_PHYERR_MAX];
-	int8_t rs_rssi_ctl0;
-	int8_t rs_rssi_ctl1;
-	int8_t rs_rssi_ctl2;
-	int8_t rs_rssi_ext0;
-	int8_t rs_rssi_ext1;
-	int8_t rs_rssi_ext2;
-	u8 rs_antenna;
 };
 
 enum ath_reset_type {
-- 
1.7.3.2

[PATCH 4/5] ath9k: get rid of double queueing of rx frames on EDMA

[Felix Fietkau]

Process rx status directly instead of separating the completion test from
the actual rx status processing.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/ar9003_mac.c |   18 +++-------
 drivers/net/wireless/ath/ath9k/ath9k.h      |    1 -
 drivers/net/wireless/ath/ath9k/recv.c       |   47 +++++++++++++--------------
 3 files changed, 29 insertions(+), 37 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
index 8d1bca0..acf2ca2 100644
--- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c
+++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
@@ -440,20 +440,14 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs,
 	struct ar9003_rxs *rxsp = (struct ar9003_rxs *) buf_addr;
 	unsigned int phyerr;
 
-	/* TODO: byte swap on big endian for ar9300_10 */
-
-	if (!rxs) {
-		if ((rxsp->status11 & AR_RxDone) == 0)
-			return -EINPROGRESS;
-
-		if (MS(rxsp->ds_info, AR_DescId) != 0x168c)
-			return -EINVAL;
+	if ((rxsp->status11 & AR_RxDone) == 0)
+		return -EINPROGRESS;
 
-		if ((rxsp->ds_info & (AR_TxRxDesc | AR_CtrlStat)) != 0)
-			return -EINPROGRESS;
+	if (MS(rxsp->ds_info, AR_DescId) != 0x168c)
+		return -EINVAL;
 
-		return 0;
-	}
+	if ((rxsp->ds_info & (AR_TxRxDesc | AR_CtrlStat)) != 0)
+		return -EINPROGRESS;
 
 	rxs->rs_status = 0;
 	rxs->rs_flags =  0;
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index c2ccba6..3d8e51c 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -299,7 +299,6 @@ struct ath_tx {
 
 struct ath_rx_edma {
 	struct sk_buff_head rx_fifo;
-	struct sk_buff_head rx_buffers;
 	u32 rx_fifo_hwsize;
 };
 
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 7e1a91a..cdba79f 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -232,7 +232,6 @@ static void ath_rx_edma_cleanup(struct ath_softc *sc)
 static void ath_rx_edma_init_queue(struct ath_rx_edma *rx_edma, int size)
 {
 	skb_queue_head_init(&rx_edma->rx_fifo);
-	skb_queue_head_init(&rx_edma->rx_buffers);
 	rx_edma->rx_fifo_hwsize = size;
 }
 
@@ -658,7 +657,9 @@ static void ath_rx_ps(struct ath_softc *sc, struct sk_buff *skb, bool mybeacon)
 }
 
 static bool ath_edma_get_buffers(struct ath_softc *sc,
-				 enum ath9k_rx_qtype qtype)
+				 enum ath9k_rx_qtype qtype,
+				 struct ath_rx_status *rs,
+				 struct ath_buf **dest)
 {
 	struct ath_rx_edma *rx_edma = &sc->rx.rx_edma[qtype];
 	struct ath_hw *ah = sc->sc_ah;
@@ -677,7 +678,7 @@ static bool ath_edma_get_buffers(struct ath_softc *sc,
 	dma_sync_single_for_cpu(sc->dev, bf->bf_buf_addr,
 				common->rx_bufsize, DMA_FROM_DEVICE);
 
-	ret = ath9k_hw_process_rxdesc_edma(ah, NULL, skb->data);
+	ret = ath9k_hw_process_rxdesc_edma(ah, rs, skb->data);
 	if (ret == -EINPROGRESS) {
 		/*let device gain the buffer again*/
 		dma_sync_single_for_device(sc->dev, bf->bf_buf_addr,
@@ -690,20 +691,21 @@ static bool ath_edma_get_buffers(struct ath_softc *sc,
 		/* corrupt descriptor, skip this one and the following one */
 		list_add_tail(&bf->list, &sc->rx.rxbuf);
 		ath_rx_edma_buf_link(sc, qtype);
-		skb = skb_peek(&rx_edma->rx_fifo);
-		if (!skb)
-			return true;
 
-		bf = SKB_CB_ATHBUF(skb);
-		BUG_ON(!bf);
+		skb = skb_peek(&rx_edma->rx_fifo);
+		if (skb) {
+			bf = SKB_CB_ATHBUF(skb);
+			BUG_ON(!bf);
 
-		__skb_unlink(skb, &rx_edma->rx_fifo);
-		list_add_tail(&bf->list, &sc->rx.rxbuf);
-		ath_rx_edma_buf_link(sc, qtype);
-		return true;
+			__skb_unlink(skb, &rx_edma->rx_fifo);
+			list_add_tail(&bf->list, &sc->rx.rxbuf);
+			ath_rx_edma_buf_link(sc, qtype);
+		} else {
+			bf = NULL;
+		}
 	}
-	skb_queue_tail(&rx_edma->rx_buffers, skb);
 
+	*dest = bf;
 	return true;
 }
 
@@ -711,18 +713,15 @@ static struct ath_buf *ath_edma_get_next_rx_buf(struct ath_softc *sc,
 						struct ath_rx_status *rs,
 						enum ath9k_rx_qtype qtype)
 {
-	struct ath_rx_edma *rx_edma = &sc->rx.rx_edma[qtype];
-	struct sk_buff *skb;
-	struct ath_buf *bf;
+	struct ath_buf *bf = NULL;
 
-	while (ath_edma_get_buffers(sc, qtype));
-	skb = __skb_dequeue(&rx_edma->rx_buffers);
-	if (!skb)
-		return NULL;
+	while (ath_edma_get_buffers(sc, qtype, rs, &bf)) {
+		if (!bf)
+			continue;
 
-	bf = SKB_CB_ATHBUF(skb);
-	ath9k_hw_process_rxdesc_edma(sc->sc_ah, rs, skb->data);
-	return bf;
+		return bf;
+	}
+	return NULL;
 }
 
 static struct ath_buf *ath_get_next_rx_buf(struct ath_softc *sc,
-- 
1.7.3.2

[PATCH 5/5] ath9k: fix signal strength reporting issues

[Felix Fietkau]

On A-MPDU frames, the hardware only reports valid signal strength data for
the last subframe. The driver also mangled rx_stats->rs_rssi using the
ATH_EP_RND macro in a way that may make sense for ANI, but definitely
not for reporting to mac80211.
This patch changes the code to calculate the signal strength from the rssi
directly instead of taking the average value, and flag everything but
the last subframe in an A-MPDU to tell mac80211 to ignore the signal strength
entirely, fixing signal strength fluctuation issues reported by various
users.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>
---
 drivers/net/wireless/ath/ath9k/recv.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index cdba79f..027d232 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -953,6 +953,7 @@ static void ath9k_process_rssi(struct ath_common *common,
 	struct ath_softc *sc = hw->priv;
 	struct ath_hw *ah = common->ah;
 	int last_rssi;
+	int rssi = rx_stats->rs_rssi;
 
 	if (!rx_stats->is_mybeacon ||
 	    ((ah->opmode != NL80211_IFTYPE_STATION) &&
@@ -964,13 +965,12 @@ static void ath9k_process_rssi(struct ath_common *common,
 
 	last_rssi = sc->last_rssi;
 	if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER))
-		rx_stats->rs_rssi = ATH_EP_RND(last_rssi,
-					      ATH_RSSI_EP_MULTIPLIER);
-	if (rx_stats->rs_rssi < 0)
-		rx_stats->rs_rssi = 0;
+		rssi = ATH_EP_RND(last_rssi, ATH_RSSI_EP_MULTIPLIER);
+	if (rssi < 0)
+		rssi = 0;
 
 	/* Update Beacon RSSI, this is used by ANI. */
-	ah->stats.avgbrssi = rx_stats->rs_rssi;
+	ah->stats.avgbrssi = rssi;
 }
 
 /*
@@ -1010,6 +1010,8 @@ static int ath9k_rx_skb_preprocess(struct ath_common *common,
 	rx_status->signal = ah->noise + rx_stats->rs_rssi;
 	rx_status->antenna = rx_stats->rs_antenna;
 	rx_status->flag |= RX_FLAG_MACTIME_MPDU;
+	if (rx_stats->rs_moreaggr)
+		rx_status->flag |= RX_FLAG_NO_SIGNAL_VAL;
 
 	return 0;
 }
-- 
1.7.3.2

Re: [PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Mohammed Shafi]

Hi Felix,

On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> Cold reset is more reliable for getting the hardware out of some specific
> stuck states.
>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath9k/hw.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index 8c840ca..d582cf7 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -1386,10 +1386,16 @@ static bool ath9k_hw_set_reset_reg(struct ath_hw *ah, u32 type)
>  static bool ath9k_hw_chip_reset(struct ath_hw *ah,
>                                struct ath9k_channel *chan)
>  {
> -       if (AR_SREV_9280(ah) && ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL)) {
> -               if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON))
> -                       return false;
> -       } else if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_WARM))
> +       int reset_type = ATH9K_RESET_WARM;
> +
> +       if (AR_SREV_9280(ah)) {
> +               if (ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL))
> +                       reset_type = ATH9K_RESET_POWER_ON;
> +               else
> +                       reset_type = ATH9K_RESET_COLD;
> +       }


cold reset seems to be applicable for older chip sets like AR9100,
should this be applicable to AR9280. won't there be any problem if we
perform cold reset in STA mode.

> +
> +       if (!ath9k_hw_set_reset_reg(ah, reset_type))
>                return false;
>
>        if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE))
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
thanks,
shafi

Re: [PATCH 3/5] ath9k: remove rssi/antenna information from recv debug stats

[Mohammed Shafi]

On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> The way this is implemented (simply storing the last value) is absolutely
> worthless for debugging anything, and the same information is also available
> through the MAC sample feature, so there's no point in keeping this around.

:) :)  please keep this, we were debugging antenna diversity related
issues in AR9485 with this stats. the antenna switching never
happened, then with these stats we found that the rss1 seems to be
always negative, same result with the windows driver also, then later
found its a EEPROM related issue.

>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath9k/debug.c |   31 -------------------------------
>  drivers/net/wireless/ath/ath9k/debug.h |    7 -------
>  2 files changed, 0 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
> index 03a4740..c2edf68 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.c
> +++ b/drivers/net/wireless/ath/ath9k/debug.c
> @@ -944,27 +944,6 @@ static ssize_t read_file_recv(struct file *file, char __user *user_buf,
>        PHY_ERR("HT-RATE ERR", ATH9K_PHYERR_HT_RATE_ILLEGAL);
>
>        len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-CTL0",
> -                       sc->debug.stats.rxstats.rs_rssi_ctl0);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-CTL1",
> -                       sc->debug.stats.rxstats.rs_rssi_ctl1);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-CTL2",
> -                       sc->debug.stats.rxstats.rs_rssi_ctl2);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-EXT0",
> -                       sc->debug.stats.rxstats.rs_rssi_ext0);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-EXT1",
> -                       sc->debug.stats.rxstats.rs_rssi_ext1);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "RSSI-EXT2",
> -                       sc->debug.stats.rxstats.rs_rssi_ext2);
> -       len += snprintf(buf + len, size - len,
> -                       "%22s : %10d\n", "Rx Antenna",
> -                       sc->debug.stats.rxstats.rs_antenna);
> -       len += snprintf(buf + len, size - len,
>                        "%22s : %10u\n", "RX-Pkts-All",
>                        sc->debug.stats.rxstats.rx_pkts_all);
>        len += snprintf(buf + len, size - len,
> @@ -1011,16 +990,6 @@ void ath_debug_stat_rx(struct ath_softc *sc, struct ath_rx_status *rs)
>                        RX_PHY_ERR_INC(rs->rs_phyerr);
>        }
>
> -       sc->debug.stats.rxstats.rs_rssi_ctl0 = rs->rs_rssi_ctl0;
> -       sc->debug.stats.rxstats.rs_rssi_ctl1 = rs->rs_rssi_ctl1;
> -       sc->debug.stats.rxstats.rs_rssi_ctl2 = rs->rs_rssi_ctl2;
> -
> -       sc->debug.stats.rxstats.rs_rssi_ext0 = rs->rs_rssi_ext0;
> -       sc->debug.stats.rxstats.rs_rssi_ext1 = rs->rs_rssi_ext1;
> -       sc->debug.stats.rxstats.rs_rssi_ext2 = rs->rs_rssi_ext2;
> -
> -       sc->debug.stats.rxstats.rs_antenna = rs->rs_antenna;
> -
>  #ifdef CONFIG_ATH9K_MAC_DEBUG
>        spin_lock(&sc->debug.samp_lock);
>        RX_SAMP_DBG(jiffies) = jiffies;
> diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
> index 7a51958..64fcfad 100644
> --- a/drivers/net/wireless/ath/ath9k/debug.h
> +++ b/drivers/net/wireless/ath/ath9k/debug.h
> @@ -165,13 +165,6 @@ struct ath_rx_stats {
>        u32 post_delim_crc_err;
>        u32 decrypt_busy_err;
>        u32 phy_err_stats[ATH9K_PHYERR_MAX];
> -       int8_t rs_rssi_ctl0;
> -       int8_t rs_rssi_ctl1;
> -       int8_t rs_rssi_ctl2;
> -       int8_t rs_rssi_ext0;
> -       int8_t rs_rssi_ext1;
> -       int8_t rs_rssi_ext2;
> -       u8 rs_antenna;
>  };
>
>  enum ath_reset_type {
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
thanks,
shafi

Re: [PATCH 3/5] ath9k: remove rssi/antenna information from recv debug stats

[Felix Fietkau]

On 2012-03-03 3:46 PM, Mohammed Shafi wrote:
> On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>> The way this is implemented (simply storing the last value) is absolutely
>> worthless for debugging anything, and the same information is also available
>> through the MAC sample feature, so there's no point in keeping this around.
> 
> :) :)  please keep this, we were debugging antenna diversity related
> issues in AR9485 with this stats. the antenna switching never
> happened, then with these stats we found that the rss1 seems to be
> always negative, same result with the windows driver also, then later
> found its a EEPROM related issue.
You can already get the antenna from the radiotap information of a
monitor mode interface, which I think is much more useful than this
limited debugfs parameter. If you really need this in debugfs, it could
also be added to the MAC sampling, though I think that one is rather
limited as well.

- Felix

Re: [PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Felix Fietkau]

On 2012-03-03 3:40 PM, Mohammed Shafi wrote:
> Hi Felix,
> 
> On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>> Cold reset is more reliable for getting the hardware out of some specific
>> stuck states.
>>
>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> ---
>>  drivers/net/wireless/ath/ath9k/hw.c |   14 ++++++++++----
>>  1 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
>> index 8c840ca..d582cf7 100644
>> --- a/drivers/net/wireless/ath/ath9k/hw.c
>> +++ b/drivers/net/wireless/ath/ath9k/hw.c
>> @@ -1386,10 +1386,16 @@ static bool ath9k_hw_set_reset_reg(struct ath_hw *ah, u32 type)
>>  static bool ath9k_hw_chip_reset(struct ath_hw *ah,
>>                                struct ath9k_channel *chan)
>>  {
>> -       if (AR_SREV_9280(ah) && ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL)) {
>> -               if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON))
>> -                       return false;
>> -       } else if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_WARM))
>> +       int reset_type = ATH9K_RESET_WARM;
>> +
>> +       if (AR_SREV_9280(ah)) {
>> +               if (ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL))
>> +                       reset_type = ATH9K_RESET_POWER_ON;
>> +               else
>> +                       reset_type = ATH9K_RESET_COLD;
>> +       }
> 
> 
> cold reset seems to be applicable for older chip sets like AR9100,
> should this be applicable to AR9280. won't there be any problem if we
> perform cold reset in STA mode.
What kind of issues do you expect from cold reset in STA mode? OpenWrt
has had this patch for a while now, and I didn't notice any issues with it.

- Felix

Re: [PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Mohammed Shafi]

> What kind of issues do you expect from cold reset in STA mode? OpenWrt
> has had this patch for a while now, and I didn't notice any issues with it.

fine, would be better if this is done to specifically , like this
recovers the chip from tx hung like that.

>
> - Felix



-- 
thanks,
shafi

Re: [PATCH 3/5] ath9k: remove rssi/antenna information from recv debug stats

[Mohammed Shafi]

On Sat, Mar 3, 2012 at 8:25 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> On 2012-03-03 3:46 PM, Mohammed Shafi wrote:
>> On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
>>> The way this is implemented (simply storing the last value) is absolutely
>>> worthless for debugging anything, and the same information is also available
>>> through the MAC sample feature, so there's no point in keeping this around.
>>
>> :) :)  please keep this, we were debugging antenna diversity related
>> issues in AR9485 with this stats. the antenna switching never
>> happened, then with these stats we found that the rss1 seems to be
>> always negative, same result with the windows driver also, then later
>> found its a EEPROM related issue.
> You can already get the antenna from the radiotap information of a
> monitor mode interface, which I think is much more useful than this
> limited debugfs parameter. If you really need this in debugfs, it could
> also be added to the MAC sampling, though I think that one is rather
> limited as well.
>

ok thanks, yes we can look it in MAC sampling for rssi stuff

-- 
thanks,
shafi

[PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Sujith Manoharan]

Felix Fietkau wrote:
> Cold reset is more reliable for getting the hardware out of some specific
> stuck states.
> 
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath9k/hw.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
> index 8c840ca..d582cf7 100644
> --- a/drivers/net/wireless/ath/ath9k/hw.c
> +++ b/drivers/net/wireless/ath/ath9k/hw.c
> @@ -1386,10 +1386,16 @@ static bool ath9k_hw_set_reset_reg(struct ath_hw *ah, u32 type)
>  static bool ath9k_hw_chip_reset(struct ath_hw *ah,
>  				struct ath9k_channel *chan)
>  {
> -	if (AR_SREV_9280(ah) && ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL)) {
> -		if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON))
> -			return false;
> -	} else if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_WARM))
> +	int reset_type = ATH9K_RESET_WARM;
> +
> +	if (AR_SREV_9280(ah)) {
> +		if (ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL))
> +			reset_type = ATH9K_RESET_POWER_ON;
> +		else
> +			reset_type = ATH9K_RESET_COLD;
> +	}
> +
> +	if (!ath9k_hw_set_reset_reg(ah, reset_type))
>  		return false;
>  
>  	if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE))

I think this needs to be verified a bit more thoroughly, since AR7010+AR9280 USB
devices would also be affected by this change. What issue is exactly fixed by this
change ? More background, etc. would be helpful.

Sujith

Re: [PATCH 4/5] ath9k: get rid of double queueing of rx frames on EDMA

[Mohammed Shafi]

Hi Felix,

On Sat, Mar 3, 2012 at 7:47 PM, Felix Fietkau <nbd@openwrt.org> wrote:
> Process rx status directly instead of separating the completion test from
> the actual rx status processing.
>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
> ---
>  drivers/net/wireless/ath/ath9k/ar9003_mac.c |   18 +++-------
>  drivers/net/wireless/ath/ath9k/ath9k.h      |    1 -
>  drivers/net/wireless/ath/ath9k/recv.c       |   47 +++++++++++++--------------
>  3 files changed, 29 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ar9003_mac.c b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
> index 8d1bca0..acf2ca2 100644
> --- a/drivers/net/wireless/ath/ath9k/ar9003_mac.c
> +++ b/drivers/net/wireless/ath/ath9k/ar9003_mac.c
> @@ -440,20 +440,14 @@ int ath9k_hw_process_rxdesc_edma(struct ath_hw *ah, struct ath_rx_status *rxs,
>        struct ar9003_rxs *rxsp = (struct ar9003_rxs *) buf_addr;
>        unsigned int phyerr;
>
> -       /* TODO: byte swap on big endian for ar9300_10 */
> -
> -       if (!rxs) {
> -               if ((rxsp->status11 & AR_RxDone) == 0)
> -                       return -EINPROGRESS;
> -
> -               if (MS(rxsp->ds_info, AR_DescId) != 0x168c)
> -                       return -EINVAL;
> +       if ((rxsp->status11 & AR_RxDone) == 0)
> +               return -EINPROGRESS;
>
> -               if ((rxsp->ds_info & (AR_TxRxDesc | AR_CtrlStat)) != 0)
> -                       return -EINPROGRESS;
> +       if (MS(rxsp->ds_info, AR_DescId) != 0x168c)
> +               return -EINVAL;
>
> -               return 0;
> -       }
> +       if ((rxsp->ds_info & (AR_TxRxDesc | AR_CtrlStat)) != 0)
> +               return -EINPROGRESS;
>
>        rxs->rs_status = 0;
>        rxs->rs_flags =  0;
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index c2ccba6..3d8e51c 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -299,7 +299,6 @@ struct ath_tx {
>
>  struct ath_rx_edma {
>        struct sk_buff_head rx_fifo;
> -       struct sk_buff_head rx_buffers;
>        u32 rx_fifo_hwsize;
>  };
>
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index 7e1a91a..cdba79f 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -232,7 +232,6 @@ static void ath_rx_edma_cleanup(struct ath_softc *sc)
>  static void ath_rx_edma_init_queue(struct ath_rx_edma *rx_edma, int size)
>  {
>        skb_queue_head_init(&rx_edma->rx_fifo);
> -       skb_queue_head_init(&rx_edma->rx_buffers);
>        rx_edma->rx_fifo_hwsize = size;
>  }
>
> @@ -658,7 +657,9 @@ static void ath_rx_ps(struct ath_softc *sc, struct sk_buff *skb, bool mybeacon)
>  }
>
>  static bool ath_edma_get_buffers(struct ath_softc *sc,
> -                                enum ath9k_rx_qtype qtype)
> +                                enum ath9k_rx_qtype qtype,
> +                                struct ath_rx_status *rs,
> +                                struct ath_buf **dest)
>  {
>        struct ath_rx_edma *rx_edma = &sc->rx.rx_edma[qtype];
>        struct ath_hw *ah = sc->sc_ah;
> @@ -677,7 +678,7 @@ static bool ath_edma_get_buffers(struct ath_softc *sc,
>        dma_sync_single_for_cpu(sc->dev, bf->bf_buf_addr,
>                                common->rx_bufsize, DMA_FROM_DEVICE);
>
> -       ret = ath9k_hw_process_rxdesc_edma(ah, NULL, skb->data);
> +       ret = ath9k_hw_process_rxdesc_edma(ah, rs, skb->data);
>        if (ret == -EINPROGRESS) {
>                /*let device gain the buffer again*/
>                dma_sync_single_for_device(sc->dev, bf->bf_buf_addr,
> @@ -690,20 +691,21 @@ static bool ath_edma_get_buffers(struct ath_softc *sc,
>                /* corrupt descriptor, skip this one and the following one */
>                list_add_tail(&bf->list, &sc->rx.rxbuf);
>                ath_rx_edma_buf_link(sc, qtype);
> -               skb = skb_peek(&rx_edma->rx_fifo);
> -               if (!skb)
> -                       return true;
>
> -               bf = SKB_CB_ATHBUF(skb);
> -               BUG_ON(!bf);
> +               skb = skb_peek(&rx_edma->rx_fifo);
> +               if (skb) {
> +                       bf = SKB_CB_ATHBUF(skb);
> +                       BUG_ON(!bf);
>
> -               __skb_unlink(skb, &rx_edma->rx_fifo);
> -               list_add_tail(&bf->list, &sc->rx.rxbuf);
> -               ath_rx_edma_buf_link(sc, qtype);
> -               return true;
> +                       __skb_unlink(skb, &rx_edma->rx_fifo);
> +                       list_add_tail(&bf->list, &sc->rx.rxbuf);
> +                       ath_rx_edma_buf_link(sc, qtype);
> +               } else {
> +                       bf = NULL;
> +               }
>        }
> -       skb_queue_tail(&rx_edma->rx_buffers, skb);
>
> +       *dest = bf;
>        return true;
>  }
>
> @@ -711,18 +713,15 @@ static struct ath_buf *ath_edma_get_next_rx_buf(struct ath_softc *sc,
>                                                struct ath_rx_status *rs,
>                                                enum ath9k_rx_qtype qtype)
>  {
> -       struct ath_rx_edma *rx_edma = &sc->rx.rx_edma[qtype];
> -       struct sk_buff *skb;
> -       struct ath_buf *bf;
> +       struct ath_buf *bf = NULL;
>
> -       while (ath_edma_get_buffers(sc, qtype));
> -       skb = __skb_dequeue(&rx_edma->rx_buffers);
> -       if (!skb)
> -               return NULL;
> +       while (ath_edma_get_buffers(sc, qtype, rs, &bf)) {
> +               if (!bf)
> +                       continue;
>
> -       bf = SKB_CB_ATHBUF(skb);
> -       ath9k_hw_process_rxdesc_edma(sc->sc_ah, rs, skb->data);
> -       return bf;
> +               return bf;
> +       }
> +       return NULL;
>  }

can we do something like this, as 'buf' is initialized to NULL

while (ath_edma_get_buffers(sc, qtype, rs, &bf))
> +               if (bf)
> +                       break;
>
> -       bf = SKB_CB_ATHBUF(skb);
> -       ath9k_hw_process_rxdesc_edma(sc->sc_ah, rs, skb->data);
> -       return bf;
> +       return bf;



>
>  static struct ath_buf *ath_get_next_rx_buf(struct ath_softc *sc,
> --
> 1.7.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
thanks,
shafi

Re: [PATCH 1/5] ath9k_hw: use cold instead of warm reset on AR9280

[Felix Fietkau]

On 2012-03-05 5:03 AM, Sujith Manoharan wrote:
> Felix Fietkau wrote:
>> Cold reset is more reliable for getting the hardware out of some specific
>> stuck states.
>> 
>> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>> ---
>>  drivers/net/wireless/ath/ath9k/hw.c |   14 ++++++++++----
>>  1 files changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
>> index 8c840ca..d582cf7 100644
>> --- a/drivers/net/wireless/ath/ath9k/hw.c
>> +++ b/drivers/net/wireless/ath/ath9k/hw.c
>> @@ -1386,10 +1386,16 @@ static bool ath9k_hw_set_reset_reg(struct ath_hw *ah, u32 type)
>>  static bool ath9k_hw_chip_reset(struct ath_hw *ah,
>>  				struct ath9k_channel *chan)
>>  {
>> -	if (AR_SREV_9280(ah) && ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL)) {
>> -		if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_POWER_ON))
>> -			return false;
>> -	} else if (!ath9k_hw_set_reset_reg(ah, ATH9K_RESET_WARM))
>> +	int reset_type = ATH9K_RESET_WARM;
>> +
>> +	if (AR_SREV_9280(ah)) {
>> +		if (ah->eep_ops->get_eeprom(ah, EEP_OL_PWRCTRL))
>> +			reset_type = ATH9K_RESET_POWER_ON;
>> +		else
>> +			reset_type = ATH9K_RESET_COLD;
>> +	}
>> +
>> +	if (!ath9k_hw_set_reset_reg(ah, reset_type))
>>  		return false;
>>  
>>  	if (!ath9k_hw_setpower(ah, ATH9K_PM_AWAKE))
> 
> I think this needs to be verified a bit more thoroughly, since AR7010+AR9280 USB
> devices would also be affected by this change. What issue is exactly fixed by this
> change ? More background, etc. would be helpful.
I don't have much background on the details. The issue description was
AR9280 in AP mode getting stuck in a reset loop from which the driver
was unable to recover. This usually happened only after a few days of
uptime. I gave this patch to some people that could reproduce the issue,
and with it the issue went away.
I don't see any negative consequences for AR7010+AR9280. This patch has
been in OpenWrt for a few months now, and OpenWrt+ath9k_htc users did
not report any issues with it either.

- Felix