Dmitry Ivanov <dmitrijs.ivanovs@xxxxxxxx> writes: > ath10k: report per-chain RSSI. > > Signed-off-by: Dmitry Ivanov <dima@xxxxxxxx> 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 > --- 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
