Skip to contentSkip to conversation

ath9k_htc: update RSSI values only when the device is associated

PATCH New Bernhard Urban 3 participants 4 messages Nov 9, 2012 – Nov 17, 2012 Qualcomm Atheros ATH9K wireless driver

lore
Share & tools
lore

Bernhard Urban

author
#0 lore raw

PATCH 1/1 ath9k_htc: update RSSI values only when the device is associated

add an if-guard, otherwise iw(8) reports weird signal strengths.

The behaviour was fine before this commit:
7c277349ecbd66e19fad3d949fa6ef6c131a3b62
This patch is therefore a partially revert of it.

Tested with "TP-Link TL-WN722N"

Thanks to indoo.rs http://indoo.rs/ for sponsoring

Reported-by Markus Krainz <[email protected]>
Tested-by Markus Krainz <[email protected]>
Signed-off-by Bernhard Urban <[email protected]>
---
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |   27 ++++++++++++++-----------
 1 file changed, 15 insertions(+), 12 deletions(-)
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c +15 -12
index 47e61d0..d0d329c 100644
@@ -1060,22 +1060,25 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
 	ath9k_process_rate(hw, rx_status, rxbuf->rxstatus.rs_rate,
 			   rxbuf->rxstatus.rs_flags);
 
-	if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
-	    !rxbuf->rxstatus.rs_moreaggr)
-		ATH_RSSI_LPF(priv->rx.last_rssi,
-			     rxbuf->rxstatus.rs_rssi);
+	if (priv->num_sta_assoc_vif != 0) {
+		if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
+		    !rxbuf->rxstatus.rs_moreaggr)
+			ATH_RSSI_LPF(priv->rx.last_rssi,
+				     rxbuf->rxstatus.rs_rssi);
 
-	last_rssi = priv->rx.last_rssi;
+		last_rssi = priv->rx.last_rssi;
 
-	if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER))
-		rxbuf->rxstatus.rs_rssi = ATH_EP_RND(last_rssi,
-						     ATH_RSSI_EP_MULTIPLIER);
+		if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER)) {
+			s8 rssi = ATH_EP_RND(last_rssi, ATH_RSSI_EP_MULTIPLIER);
+			rxbuf->rxstatus.rs_rssi = rssi;
+		}
 
-	if (rxbuf->rxstatus.rs_rssi < 0)
-		rxbuf->rxstatus.rs_rssi = 0;
+		if (rxbuf->rxstatus.rs_rssi < 0)
+			rxbuf->rxstatus.rs_rssi = 0;
 
-	if (ieee80211_is_beacon(fc))
-		priv->ah->stats.avgbrssi = rxbuf->rxstatus.rs_rssi;
+		if (ieee80211_is_beacon(fc))
+			priv->ah->stats.avgbrssi = rxbuf->rxstatus.rs_rssi;
+	}
 
 	rx_status->mactime = be64_to_cpu(rxbuf->rxstatus.rs_tstamp);
 	rx_status->band = hw->conf.channel->band;
Signature · 1 line
1.7.9.5
#1 lore raw

PATCH 1/1 ath9k_htc: update RSSI values only when the device is associated

Quoted 7 lines · Bernhard Urban

add an if-guard, otherwise iw(8) reports weird signal strengths.

The behaviour was fine before this commit:
7c277349ecbd66e19fad3d949fa6ef6c131a3b62
This patch is therefore a partially revert of it.

Tested with "TP-Link TL-WN722N"

Looks good, but a cleaner fix would be to update the RSSI only for beacons that are destined for the station.

Sujith

#2 lore raw

PATCH 1/1 ath9k_htc: update RSSI values only when the device is associated

In reply to Bernhard Urban
Quoted 50 lines · Bernhard Urban

add an if-guard, otherwise iw(8) reports weird signal strengths.

The behaviour was fine before this commit:
7c277349ecbd66e19fad3d949fa6ef6c131a3b62
This patch is therefore a partially revert of it.

Tested with "TP-Link TL-WN722N"

Thanks to indoo.rs http://indoo.rs/ for sponsoring

Reported-by Markus Krainz <[email protected]>
Tested-by Markus Krainz <[email protected]>
Signed-off-by Bernhard Urban <[email protected]>
---
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |   27 ++++++++++++++-----------
 1 file changed, 15 insertions(+), 12 deletions(-)
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c +11 -10
index 47e61d0..d0d329c 100644
@@ -1060,22 +1060,25 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
 	ath9k_process_rate(hw, rx_status, rxbuf->rxstatus.rs_rate,
 			   rxbuf->rxstatus.rs_flags);
 
-	if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
-	    !rxbuf->rxstatus.rs_moreaggr)
-		ATH_RSSI_LPF(priv->rx.last_rssi,
-			     rxbuf->rxstatus.rs_rssi);
+	if (priv->num_sta_assoc_vif != 0) {
+		if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
+		    !rxbuf->rxstatus.rs_moreaggr)
+			ATH_RSSI_LPF(priv->rx.last_rssi,
+				     rxbuf->rxstatus.rs_rssi);
 
-	last_rssi = priv->rx.last_rssi;
+		last_rssi = priv->rx.last_rssi;
 
-	if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER))
-		rxbuf->rxstatus.rs_rssi = ATH_EP_RND(last_rssi,
-						     ATH_RSSI_EP_MULTIPLIER);
+		if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER)) {
+			s8 rssi = ATH_EP_RND(last_rssi, ATH_RSSI_EP_MULTIPLIER);
+			rxbuf->rxstatus.rs_rssi = rssi;
+		}
 
-	if (rxbuf->rxstatus.rs_rssi < 0)
-		rxbuf->rxstatus.rs_rssi = 0;
+		if (rxbuf->rxstatus.rs_rssi < 0)

drivers/net/wireless/ath/ath9k/htc_drv_txrx.c static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv, · line 1084

Quoted 10 lines
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c +4 -2
+			rxbuf->rxstatus.rs_rssi = 0;
 
-	if (ieee80211_is_beacon(fc))
-		priv->ah->stats.avgbrssi = rxbuf->rxstatus.rs_rssi;
+		if (ieee80211_is_beacon(fc))
+			priv->ah->stats.avgbrssi = rxbuf->rxstatus.rs_rssi;
+	}
 
 	rx_status->mactime = be64_to_cpu(rxbuf->rxstatus.rs_tstamp);
 	rx_status->band = hw->conf.channel->band;

Please check how RSSI is handled in ath9k, and use that as reference. The per-packet RSSI should not be set from some internal average, it should contain the real RSSI value of the packet. This change disables RSSI reporting for anything but station mode, which is not a good idea.

- Felix

author
#3 lore raw

PATCH 1/1 ath9k_htc: update RSSI values only when the device is associated

Thanks for the reviews!

Quoted 2 lines · Sujith

Looks good, but a cleaner fix would be to update the RSSI only for beacons that are destined for the station.

You mean like in recv.c? There's a is_mybeacon' field, which is computed in ath_rx_tasklet(...)' and used in `ath9k_process_rssi(...)'.

Adding this check to the if-guard changes nothing at the result, so I think it's fine if you consider it as cleaner. See attached patch.

Btw, could you please elaborate what `is_mybeacon' represents?

Quoted 4 lines · Felix Fietkau

Please check how RSSI is handled in ath9k, and use that as reference. The per-packet RSSI should not be set from some internal average, it should contain the real RSSI value of the packet. This change disables RSSI reporting for anything but station mode, which is not a good idea.

Replacing

	if (priv->num_sta_assoc_vif != 0)
with
	if (priv->ah->opmode == NL80211_IFTYPE_STATION)

works out well (which makes sense to me). However, adding
&& priv->ah->opmode == NL80211_IFTYPE_ADHOC' (like in ath9k/recv.c) produces bad results like before. I don't understand the effect, as my device is always in "managed mode" according to iw wlan0 info'. I'm right by assuming "managed mode" is the same as NL80211_IFTYPE_STATION?

Also, I don't know what you mean with "some internal average". Can you please elaborate that?

Thanks for your help,
Bernhard

Quoted 1 line

From fc14c46ce75044485229be5bb12682a8fffcf178 Mon Sep 17 00:00:00 2001

From: Bernhard Urban <[email protected]>
Date: Sat, 17 Nov 2012 00:22:16 +0100
Subject: [PATCH] ath9k_htc: update RSSI values only when the device is
associated

add an if-guard, otherwise iw(8) reports weird signal strengths.

The behaviour was fine before this commit:
7c277349ecbd66e19fad3d949fa6ef6c131a3b62
This patch is therefore a partially revert of it.

Tested with "TP-Link TL-WN722N"

Thanks to indoo.rs http://indoo.rs/ for sponsoring

Reported-by Markus Krainz <[email protected]>
Tested-by Markus Krainz <[email protected]>
Signed-off-by Bernhard Urban <[email protected]>
---
 drivers/net/wireless/ath/ath9k/htc.h          |    1 +
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |   29 +++++++++++++++----------
 2 files changed, 19 insertions(+), 11 deletions(-)
drivers/net/wireless/ath/ath9k/htc.h +1 -0
index 936e920..00ebf1c 100644
@@ -22,6 +22,7 @@
 #include <linux/firmware.h>
 #include <linux/skbuff.h>
 #include <linux/netdevice.h>
+#include <linux/etherdevice.h>
 #include <linux/leds.h>
 #include <linux/slab.h>
 #include <net/mac80211.h>
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c +18 -11
index 47e61d0..8b4da3d 100644
@@ -969,6 +969,7 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
 	int hdrlen, padpos, padsize;
 	int last_rssi = ATH_RSSI_DUMMY_MARKER;
 	__le16 fc;
+	u8 is_mybeacon;
 
 	if (skb->len < HTC_RX_FRAME_HEADER_SIZE) {
 		ath_err(common, "Corrupted RX frame, dropping (len: %d)\n",

@@ -1060,22 +1061,28 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
 	ath9k_process_rate(hw, rx_status, rxbuf->rxstatus.rs_rate,
 			   rxbuf->rxstatus.rs_flags);
 
-	if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
-	    !rxbuf->rxstatus.rs_moreaggr)
-		ATH_RSSI_LPF(priv->rx.last_rssi,
-			     rxbuf->rxstatus.rs_rssi);
+	is_mybeacon = ieee80211_is_beacon(fc) &&
+		!is_zero_ether_addr(common->curbssid) &&
+		ether_addr_equal(hdr->addr3, common->curbssid);
 
-	last_rssi = priv->rx.last_rssi;
+	if (is_mybeacon && priv->ah->opmode == NL80211_IFTYPE_STATION) {
+		if (rxbuf->rxstatus.rs_rssi != ATH9K_RSSI_BAD &&
+		    !rxbuf->rxstatus.rs_moreaggr)
+			ATH_RSSI_LPF(priv->rx.last_rssi,
+				     rxbuf->rxstatus.rs_rssi);
 
-	if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER))
-		rxbuf->rxstatus.rs_rssi = ATH_EP_RND(last_rssi,
-						     ATH_RSSI_EP_MULTIPLIER);
+		last_rssi = priv->rx.last_rssi;
 
-	if (rxbuf->rxstatus.rs_rssi < 0)
-		rxbuf->rxstatus.rs_rssi = 0;
+		if (likely(last_rssi != ATH_RSSI_DUMMY_MARKER)) {
+			s8 rssi = ATH_EP_RND(last_rssi, ATH_RSSI_EP_MULTIPLIER);
+			rxbuf->rxstatus.rs_rssi = rssi;
+		}
+
+		if (rxbuf->rxstatus.rs_rssi < 0)
+			rxbuf->rxstatus.rs_rssi = 0;
 
-	if (ieee80211_is_beacon(fc))
 		priv->ah->stats.avgbrssi = rxbuf->rxstatus.rs_rssi;
+	}
 
 	rx_status->mactime = be64_to_cpu(rxbuf->rxstatus.rs_tstamp);
 	rx_status->band = hw->conf.channel->band;
Signature · 1 line
1.7.9.5