[PATCH] ath10k: report per-chain RSSI

Kalle Valo kvalo at qca.qualcomm.com
Mon Aug 10 11:30:56 PDT 2015


Dmitry Ivanov <dmitrijs.ivanovs at ubnt.com> writes:

> ath10k: report per-chain RSSI.
>
> Signed-off-by: Dmitry Ivanov <dima at ubnt.com>

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



More information about the ath10k mailing list