diff mbox series

brcmfmac: fix RSSI report in AP mode

Message ID 20241122210346.2848578-1-alexthreed@gmail.com
State New
Headers show
Series brcmfmac: fix RSSI report in AP mode | expand

Commit Message

Alex Shumsky Nov. 22, 2024, 9:03 p.m. UTC
After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
station info") it is required from firmware to provide rx_lastpkt_rssi.
If this field is not provided brcmfmac doesn't report any RSSI at all.
Unfortunately some firmwares doesn't provide it. One example is firmware
for BCM43455 found in Raspbberry Pi.
See https://github.com/raspberrypi/linux/issues/4574

Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
(like it was before 9a1590934d9a).

Fixes: 9a1590934d9a ("brcmfmac: correctly report average RSSI in station info")
Signed-off-by: Alex Shumsky <alexthreed@gmail.com>
---

 .../wireless/broadcom/brcm80211/brcmfmac/cfg80211.c  | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Kalle Valo Nov. 25, 2024, 11:58 a.m. UTC | #1
Alex Shumsky <alexthreed@gmail.com> writes:

> After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
> station info") it is required from firmware to provide rx_lastpkt_rssi.
> If this field is not provided brcmfmac doesn't report any RSSI at all.
> Unfortunately some firmwares doesn't provide it. One example is firmware
> for BCM43455 found in Raspbberry Pi.
> See https://github.com/raspberrypi/linux/issues/4574
>
> Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
> (like it was before 9a1590934d9a).
>
> Fixes: 9a1590934d9a ("brcmfmac: correctly report average RSSI in station info")
> Signed-off-by: Alex Shumsky <alexthreed@gmail.com>

'wifi:' missing but I can add it, no need to resend because of this.
Arend van Spriel Nov. 26, 2024, 11:13 a.m. UTC | #2
On 11/22/2024 10:03 PM, Alex Shumsky wrote:
> After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
> station info") it is required from firmware to provide rx_lastpkt_rssi.
> If this field is not provided brcmfmac doesn't report any RSSI at all.
> Unfortunately some firmwares doesn't provide it. One example is firmware
> for BCM43455 found in Raspbberry Pi.
> See https://github.com/raspberrypi/linux/issues/4574
> 
> Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
> (like it was before 9a1590934d9a).

Sounds like a reasonable approach. However, I would like to learn more 
about the issue. Maybe it is a per-vendor issue so I am interested what 
the sta_info version is that we get from firmware. It is printed in 
brcmf_cfg80211_get_station() with brcmf_dbg(). You can make it a 
bphy_err() call instead or enable TRACE level debug messages in the driver.

Also would be good to know the firmware version and kernel version of 
the BCM43455.

Regards,
Arend
Alex Shumsky Dec. 9, 2024, 9:08 p.m. UTC | #3
On Tue, Nov 26, 2024 at 2:13 PM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 11/22/2024 10:03 PM, Alex Shumsky wrote:
> > After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
> > station info") it is required from firmware to provide rx_lastpkt_rssi.
> > If this field is not provided brcmfmac doesn't report any RSSI at all.
> > Unfortunately some firmwares doesn't provide it. One example is firmware
> > for BCM43455 found in Raspbberry Pi.
> > See https://github.com/raspberrypi/linux/issues/4574
> >
> > Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
> > (like it was before 9a1590934d9a).
>
> Sounds like a reasonable approach. However, I would like to learn more
> about the issue. Maybe it is a per-vendor issue so I am interested what
> the sta_info version is that we get from firmware. It is printed in
> brcmf_cfg80211_get_station() with brcmf_dbg(). You can make it a
> bphy_err() call instead or enable TRACE level debug messages in the driver.
>
> Also would be good to know the firmware version and kernel version of
> the BCM43455.
>
> Regards,
> Arend

I've just checked sta version in trace logs:
brcmf_cfg80211_get_station version 4

Firmware I currently use:
BCM4345/6 wl0: Aug 29 2023 01:47:08 version 7.45.265 (28bca26 CY) FWID
01-b677b91b
https://github.com/murata-wireless/cyw-fmac-fw/blob/e024d0c0a3ab241f547cb44303de7e1b49f0ca78/cyfmac43455-sdio.bin

I'm not sure what firmware version is used in the raspberry pi distro.
Alex Shumsky Dec. 17, 2024, 6:54 p.m. UTC | #4
On Tue, Nov 26, 2024 at 2:13 PM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 11/22/2024 10:03 PM, Alex Shumsky wrote:
> > After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
> > station info") it is required from firmware to provide rx_lastpkt_rssi.
> > If this field is not provided brcmfmac doesn't report any RSSI at all.
> > Unfortunately some firmwares doesn't provide it. One example is firmware
> > for BCM43455 found in Raspbberry Pi.
> > See https://github.com/raspberrypi/linux/issues/4574
> >
> > Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
> > (like it was before 9a1590934d9a).
>
> Sounds like a reasonable approach. However, I would like to learn more
> about the issue. Maybe it is a per-vendor issue so I am interested what
> the sta_info version is that we get from firmware. It is printed in
> brcmf_cfg80211_get_station() with brcmf_dbg(). You can make it a
> bphy_err() call instead or enable TRACE level debug messages in the driver.
>
> Also would be good to know the firmware version and kernel version of
> the BCM43455.
>
> Regards,
> Arend

Hi Arend,

Is the info I have provided sufficient?
Arend van Spriel Dec. 17, 2024, 7:45 p.m. UTC | #5
On 12/17/2024 7:54 PM, Alex Shumsky wrote:
> On Tue, Nov 26, 2024 at 2:13 PM Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>>
>> On 11/22/2024 10:03 PM, Alex Shumsky wrote:
>>> After commit 9a1590934d9a ("brcmfmac: correctly report average RSSI in
>>> station info") it is required from firmware to provide rx_lastpkt_rssi.
>>> If this field is not provided brcmfmac doesn't report any RSSI at all.
>>> Unfortunately some firmwares doesn't provide it. One example is firmware
>>> for BCM43455 found in Raspbberry Pi.
>>> See https://github.com/raspberrypi/linux/issues/4574
>>>
>>> Fix it by falling back to rssi field if rx_lastpkt_rssi is not provided
>>> (like it was before 9a1590934d9a).
>>
>> Sounds like a reasonable approach. However, I would like to learn more
>> about the issue. Maybe it is a per-vendor issue so I am interested what
>> the sta_info version is that we get from firmware. It is printed in
>> brcmf_cfg80211_get_station() with brcmf_dbg(). You can make it a
>> bphy_err() call instead or enable TRACE level debug messages in the driver.
>>
>> Also would be good to know the firmware version and kernel version of
>> the BCM43455.
>>
>> Regards,
>> Arend
> 
> Hi Arend,
> 
> Is the info I have provided sufficient?

I am missing the kernel version and whether it is upstream kernel or not.

Regards,
Arend
diff mbox series

Patch

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 349aa3439502..8fc10858e936 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -3125,6 +3125,7 @@  brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
 	s32 total_rssi = 0;
 	s32 count_rssi = 0;
 	int rssi;
+	int rx_lastpkt_rssi;
 	u32 i;
 
 	brcmf_dbg(TRACE, "Enter, MAC %pM\n", mac);
@@ -3190,15 +3191,16 @@  brcmf_cfg80211_get_station(struct wiphy *wiphy, struct net_device *ndev,
 			sinfo->rx_bytes = le64_to_cpu(sta_info_le.rx_tot_bytes);
 		}
 		for (i = 0; i < BRCMF_ANT_MAX; i++) {
-			if (sta_info_le.rssi[i] == 0 ||
-			    sta_info_le.rx_lastpkt_rssi[i] == 0)
+			if (sta_info_le.rssi[i] == 0)
 				continue;
+			rx_lastpkt_rssi = sta_info_le.rx_lastpkt_rssi[i] != 0 ?
+				sta_info_le.rx_lastpkt_rssi[i] :
+				sta_info_le.rssi[i];
 			sinfo->chains |= BIT(count_rssi);
-			sinfo->chain_signal[count_rssi] =
-				sta_info_le.rx_lastpkt_rssi[i];
+			sinfo->chain_signal[count_rssi] = rx_lastpkt_rssi;
 			sinfo->chain_signal_avg[count_rssi] =
 				sta_info_le.rssi[i];
-			total_rssi += sta_info_le.rx_lastpkt_rssi[i];
+			total_rssi += rx_lastpkt_rssi;
 			total_rssi_avg += sta_info_le.rssi[i];
 			count_rssi++;
 		}