Thread (2 messages) 2 messages, 2 authors, 2015-08-10

Re: [PATCH] ath10k: report per-chain RSSI

From: Kalle Valo <hidden>
Date: 2015-08-10 18:31:06

Dmitry Ivanov [off-list ref] writes:
ath10k: report per-chain RSSI.

Signed-off-by: Dmitry Ivanov <redacted>
Please write a proper commit log. And CC ath10k mailing list when
submitting ath10k patches:

https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches
quoted hunk ↗ jump to hunk
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -832,6 +832,20 @@ static void ath10k_htt_rx_h_signal(struct ath10k *ar,
 				   struct ieee80211_rx_status *status,
 				   struct htt_rx_desc *rxd)
 {
+	{
The extra block is not needed and can be remove, right?
+		/* Linux array has size IEEE80211_MAX_CHAINS, FW array has size 4 */
+		BUILD_BUG_ON(IEEE80211_MAX_CHAINS != 4);
This is a bit evil, is this really necessary? Can't we just add a define
for FW array size and use that?
+		u32 i = IEEE80211_MAX_CHAINS;
Why u32? Wouldn't int be enough?
+		u8 signal_per_chain;
+		do {
+			i--;
+			signal_per_chain = rxd->ppdu_start.rssi_chains[i].pri20_mhz;
+			if (signal_per_chain != 0x80) {
What's the magic number 0x80? Is there an existing define you could use
or do we need to add a new one?
+				status->chains |= BIT(i);
+				status->chain_signal[i] = ATH10K_DEFAULT_NOISE_FLOOR + signal_per_chain;
+			}
+		} while (i);
+	}
Wouldn't a normal for loop be simpler?

for (i = 0; i < IEEE80211_MAX_CHAINS; i++)

-- 
Kalle Valo
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help