Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle

2022-06-26 Thread Jonathan Gray
On Sun, Jun 26, 2022 at 12:23:14PM +0200, Stefan Sperling wrote:
> On Sun, Jun 26, 2022 at 07:48:46PM +1000, Jonathan Gray wrote:
> > sta.rssi is later used which is 'Fields valid for ver >= 4'
> > but it seems with the earlier zeroing the use here should be fine?
> 
> Thanks! I missed that.
> 
> Testing suggests it makes more sense to keep the RSSI value which
> was discovered during the scan phase.
> 
> With the new patch below ifconfig shows -51dBm.
> With the previous patch ifconfig showed -70dBm for the same AP (with
> client/AP location unchanged).

ok jsg@

> 
> diff /usr/src
> commit - 9badb9ad8932c12f4ece484255eb2703a2518c17
> path + /usr/src
> blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8
> file + sys/dev/ic/bwfm.c
> --- sys/dev/ic/bwfm.c
> +++ sys/dev/ic/bwfm.c
> @@ -703,22 +703,24 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni)
>   if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea))
>   return;
>  
> - if (le16toh(sta.ver) < 4)
> + if (le16toh(sta.ver) < 3)
>   return;
>  
>   flags = le32toh(sta.flags);
>   if ((flags & BWFM_STA_SCBSTATS) == 0)
>   return;
>  
> - rssi = 0;
> - for (i = 0; i < BWFM_ANT_MAX; i++) {
> - if (sta.rssi[i] >= 0)
> - continue;
> - if (rssi == 0 || sta.rssi[i] > rssi)
> - rssi = sta.rssi[i];
> + if (le16toh(sta.ver) >= 4) {
> + rssi = 0;
> + for (i = 0; i < BWFM_ANT_MAX; i++) {
> + if (sta.rssi[i] >= 0)
> + continue;
> + if (rssi == 0 || sta.rssi[i] > rssi)
> + rssi = sta.rssi[i];
> + }
> + if (rssi)
> + ni->ni_rssi = rssi;
>   }
> - if (rssi)
> - ni->ni_rssi = rssi;
>  
>   txrate = le32toh(sta.tx_rate); /* in kbit/s */
>   if (txrate == 0x) /* Seen this happening during association. */
> 
> 



Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle

2022-06-26 Thread Stefan Sperling
On Sun, Jun 26, 2022 at 07:48:46PM +1000, Jonathan Gray wrote:
> sta.rssi is later used which is 'Fields valid for ver >= 4'
> but it seems with the earlier zeroing the use here should be fine?

Thanks! I missed that.

Testing suggests it makes more sense to keep the RSSI value which
was discovered during the scan phase.

With the new patch below ifconfig shows -51dBm.
With the previous patch ifconfig showed -70dBm for the same AP (with
client/AP location unchanged).

diff /usr/src
commit - 9badb9ad8932c12f4ece484255eb2703a2518c17
path + /usr/src
blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8
file + sys/dev/ic/bwfm.c
--- sys/dev/ic/bwfm.c
+++ sys/dev/ic/bwfm.c
@@ -703,22 +703,24 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni)
if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea))
return;
 
-   if (le16toh(sta.ver) < 4)
+   if (le16toh(sta.ver) < 3)
return;
 
flags = le32toh(sta.flags);
if ((flags & BWFM_STA_SCBSTATS) == 0)
return;
 
-   rssi = 0;
-   for (i = 0; i < BWFM_ANT_MAX; i++) {
-   if (sta.rssi[i] >= 0)
-   continue;
-   if (rssi == 0 || sta.rssi[i] > rssi)
-   rssi = sta.rssi[i];
+   if (le16toh(sta.ver) >= 4) {
+   rssi = 0;
+   for (i = 0; i < BWFM_ANT_MAX; i++) {
+   if (sta.rssi[i] >= 0)
+   continue;
+   if (rssi == 0 || sta.rssi[i] > rssi)
+   rssi = sta.rssi[i];
+   }
+   if (rssi)
+   ni->ni_rssi = rssi;
}
-   if (rssi)
-   ni->ni_rssi = rssi;
 
txrate = le32toh(sta.tx_rate); /* in kbit/s */
if (txrate == 0x) /* Seen this happening during association. */



Re: bwfm: fix ifconfig media display on rpi2 usb wifi dongle

2022-06-26 Thread Jonathan Gray
On Sat, Jun 25, 2022 at 10:07:21PM +0200, Stefan Sperling wrote:
> There is an off-by-one in bwfm_update_node(). This function reads
> the tx_rate field from station information returned by firmware.
> 
> According to a comment in the Linux driver, this field is valid
> for sta_info command versions >= 3.
> 
> linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> struct brcmf_sta_info_le {
>   __le16 ver; /* version of this struct */
> [...]
>   /* Fields valid for ver >= 3 */
> [...]
>   __le32 tx_rate; /* Rate of last successful tx frame */
> 
> However, our driver only reads Tx rate info if the version is >= 4.
> 
> On the rpi2 usb wifi adapter (WLU6331) this breaks media mode display.
> Firmware on this device uses command version 3, and the media mode
> displayed is always "DS1 11g". Diff below fixes this. ifconfig will
> now display 11n mode while associated to an 11n AP.
> 
> ok?

sta.rssi is later used which is 'Fields valid for ver >= 4'
but it seems with the earlier zeroing the use here should be fine?

ok jsg@

> 
> diff /usr/src
> commit - 9badb9ad8932c12f4ece484255eb2703a2518c17
> path + /usr/src
> blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8
> file + sys/dev/ic/bwfm.c
> --- sys/dev/ic/bwfm.c
> +++ sys/dev/ic/bwfm.c
> @@ -703,7 +703,7 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni)
>   if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea))
>   return;
>  
> - if (le16toh(sta.ver) < 4)
> + if (le16toh(sta.ver) < 3)
>   return;
>  
>   flags = le32toh(sta.flags);
> 
> 



bwfm: fix ifconfig media display on rpi2 usb wifi dongle

2022-06-25 Thread Stefan Sperling
There is an off-by-one in bwfm_update_node(). This function reads
the tx_rate field from station information returned by firmware.

According to a comment in the Linux driver, this field is valid
for sta_info command versions >= 3.

linux/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
struct brcmf_sta_info_le {
__le16 ver; /* version of this struct */
[...]
/* Fields valid for ver >= 3 */
[...]
__le32 tx_rate; /* Rate of last successful tx frame */

However, our driver only reads Tx rate info if the version is >= 4.

On the rpi2 usb wifi adapter (WLU6331) this breaks media mode display.
Firmware on this device uses command version 3, and the media mode
displayed is always "DS1 11g". Diff below fixes this. ifconfig will
now display 11n mode while associated to an 11n AP.

ok?

diff /usr/src
commit - 9badb9ad8932c12f4ece484255eb2703a2518c17
path + /usr/src
blob - 0c9c948115a0c115a43bd365ad4e389ba694c4a8
file + sys/dev/ic/bwfm.c
--- sys/dev/ic/bwfm.c
+++ sys/dev/ic/bwfm.c
@@ -703,7 +703,7 @@ bwfm_update_node(void *arg, struct ieee80211_node *ni)
if (!IEEE80211_ADDR_EQ(ni->ni_macaddr, sta.ea))
return;
 
-   if (le16toh(sta.ver) < 4)
+   if (le16toh(sta.ver) < 3)
return;
 
flags = le32toh(sta.flags);