The sinfo->chains field is a bitmask for filled values in chain_signal
and chain_signal_avg, not a count. Treat it as such so that the driver
can properly report per-chain RSSI information.
Before (MIMO mode):
$ iw dev wlan0 station dump
...
signal: -51 [-51] dBm
After (MIMO mode):
$ iw dev wlan0 station dump
...
signal: -53 [-53, -54] dBm
Signed-off-by: Alvin Šipraga <[email protected]>
Fixes: cae355dc90db ("brcmfmac: Add RSSI information to get_station.")
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index f4405d7861b6..afa75cb83221 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2838,6 +2838,7 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
count_rssi = 0;
for (i = 0; i < BRCMF_ANT_MAX; i++) {
if (sta_info_le.rssi[i]) {
+ sinfo->chains |= BIT(count_rssi);
sinfo->chain_signal_avg[count_rssi] =
sta_info_le.rssi[i];
sinfo->chain_signal[count_rssi] =
@@ -2848,8 +2849,6 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
}
if (count_rssi) {
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL);
- sinfo->chains = count_rssi;
-
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL);
total_rssi /= count_rssi;
sinfo->signal = total_rssi;
--
2.31.1
The rx_lastpkt_rssi field provided by the firmware is suitable for
NL80211_STA_INFO_{SIGNAL,CHAIN_SIGNAL}, while the rssi field is an
average. Fix up the assignments and set the correct STA_INFO bits. This
lets userspace know that the average RSSI is part of the station info.
Signed-off-by: Alvin Šipraga <[email protected]>
Fixes: cae355dc90db ("brcmfmac: Add RSSI information to get_station.")
---
.../broadcom/brcm80211/brcmfmac/cfg80211.c | 36 ++++++++++---------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index afa75cb83221..d8822a01d277 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2767,8 +2767,9 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
struct brcmf_sta_info_le sta_info_le;
u32 sta_flags;
u32 is_tdls_peer;
- s32 total_rssi;
- s32 count_rssi;
+ s32 total_rssi_avg = 0;
+ s32 total_rssi = 0;
+ s32 count_rssi = 0;
int rssi;
u32 i;
@@ -2834,24 +2835,27 @@ brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_RX_BYTES);
sinfo->rx_bytes = le64_to_cpu(sta_info_le.rx_tot_bytes);
}
- total_rssi = 0;
- count_rssi = 0;
for (i = 0; i < BRCMF_ANT_MAX; i++) {
- if (sta_info_le.rssi[i]) {
- sinfo->chains |= BIT(count_rssi);
- sinfo->chain_signal_avg[count_rssi] =
- sta_info_le.rssi[i];
- sinfo->chain_signal[count_rssi] =
- sta_info_le.rssi[i];
- total_rssi += sta_info_le.rssi[i];
- count_rssi++;
- }
+ if (sta_info_le.rssi[i] == 0 ||
+ sta_info_le.rx_lastpkt_rssi[i] == 0)
+ continue;
+ sinfo->chains |= BIT(count_rssi);
+ sinfo->chain_signal[count_rssi] =
+ sta_info_le.rx_lastpkt_rssi[i];
+ sinfo->chain_signal_avg[count_rssi] =
+ sta_info_le.rssi[i];
+ total_rssi += sta_info_le.rx_lastpkt_rssi[i];
+ total_rssi_avg += sta_info_le.rssi[i];
+ count_rssi++;
}
if (count_rssi) {
- sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL);
sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL);
- total_rssi /= count_rssi;
- sinfo->signal = total_rssi;
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_SIGNAL_AVG);
+ sinfo->filled |= BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL);
+ sinfo->filled |=
+ BIT_ULL(NL80211_STA_INFO_CHAIN_SIGNAL_AVG);
+ sinfo->signal = total_rssi / count_rssi;
+ sinfo->signal_avg = total_rssi_avg / count_rssi;
} else if (test_bit(BRCMF_VIF_STATUS_CONNECTED,
&ifp->vif->sme_state)) {
memset(&scb_val, 0, sizeof(scb_val));
--
2.31.1
Hello,
On 5/6/21 3:20 PM, Alvin Šipraga wrote:
> The sinfo->chains field is a bitmask for filled values in chain_signal
> and chain_signal_avg, not a count. Treat it as such so that the driver
> can properly report per-chain RSSI information.
>
<snip>
This is a gentle ping to see if these two patches got lost. I was told
on another mailing list recently that mail sent from my work address is
finding its way into peoples' junk folders.
I will resend the patches in the near future if I don't see any response
here.
Thanks,
Alvin
Alvin Šipraga <[email protected]> writes:
> Hello,
>
> On 5/6/21 3:20 PM, Alvin Šipraga wrote:
>> The sinfo->chains field is a bitmask for filled values in chain_signal
>> and chain_signal_avg, not a count. Treat it as such so that the driver
>> can properly report per-chain RSSI information.
>>
>
> <snip>
>
> This is a gentle ping to see if these two patches got lost. I was told
> on another mailing list recently that mail sent from my work address
> is finding its way into peoples' junk folders.
>
> I will resend the patches in the near future if I don't see any
> response here.
I just have been busy and heavily backlogged. The patches are in
patchwork, so no need to resend:
https://patchwork.kernel.org/project/linux-wireless/list/?series=477877&state=*
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi Kalle,
On 6/14/21 4:33 PM, Kalle Valo wrote:
> Alvin Šipraga <[email protected]> writes:
>
>> Hello,
>>
>> On 5/6/21 3:20 PM, Alvin Šipraga wrote:
>>> The sinfo->chains field is a bitmask for filled values in chain_signal
>>> and chain_signal_avg, not a count. Treat it as such so that the driver
>>> can properly report per-chain RSSI information.
>>>
>>
>> <snip>
>>
>> This is a gentle ping to see if these two patches got lost. I was told
>> on another mailing list recently that mail sent from my work address
>> is finding its way into peoples' junk folders.
>>
>> I will resend the patches in the near future if I don't see any
>> response here.
>
> I just have been busy and heavily backlogged. The patches are in
> patchwork, so no need to resend:
>
> https://patchwork.kernel.org/project/linux-wireless/list/?series=477877&state=*>>
Great - thanks for the quick reply. I will not resend.
Kind regards,
Alvin
Alvin Šipraga <[email protected]> wrote:
> The sinfo->chains field is a bitmask for filled values in chain_signal
> and chain_signal_avg, not a count. Treat it as such so that the driver
> can properly report per-chain RSSI information.
>
> Before (MIMO mode):
>
> $ iw dev wlan0 station dump
> ...
> signal: -51 [-51] dBm
>
> After (MIMO mode):
>
> $ iw dev wlan0 station dump
> ...
> signal: -53 [-53, -54] dBm
>
> Fixes: cae355dc90db ("brcmfmac: Add RSSI information to get_station.")
> Signed-off-by: Alvin Šipraga <[email protected]>
2 patches applied to wireless-drivers-next.git, thanks.
feb456437621 brcmfmac: fix setting of station info chains bitmask
9a1590934d9a brcmfmac: correctly report average RSSI in station info
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches