Re: bwfm(4): support scan v3

[Mark Kettenis]

> Date: Wed, 11 Oct 2023 10:10:58 +0200
> From: Stefan Sperling 
> 
> On Tue, Oct 10, 2023 at 11:41:39PM +0200, Mark Kettenis wrote:
> > The firmware for the BCM4388 has yet another version of the "escan"
> > command.  But we can treat it the same as v2 since it just added a new
> > parameter in place of some padding.  We just set that new parameter to
> > zero, which doesn't change anything.
> > 
> > As a bonus this adds some missing htole16() calls.
> > 
> > This is the equivalent of:
> > 
> > https://github.com/AsahiLinux/linux/commit/399ef7b1cb9094c1c64e0f9ad6caa5c4d114009f
> > 
> > ok?
> 
> > @@ -274,8 +275,10 @@ bwfm_preinit(struct bwfm_softc *sc)
> > nmode = 0;
> > if (bwfm_fwvar_var_get_int(sc, "vhtmode", ))
> > vhtmode = 0;
> > -   if (bwfm_fwvar_var_get_int(sc, "scan_ver", >sc_scan_ver))
> > -   sc->sc_scan_ver = 0;
> > +   if (bwfm_fwvar_var_get_data(sc, "scan_ver", _ver,
> > +   sizeof(scan_ver)))
> > +   scan_ver.scan_ver_major = 0;
> > +   sc->sc_scan_ver = letoh16(scan_ver.scan_ver_major);
> 
> Perhaps check whether firmware reports a supported scan command version,
> and fail or print a warning when it doesn't?
> That might make future firmware upgrades a bit easier in case the vendor
> changes this again.

It does seem that new versions of firmware interfaces are only
introduced together with new silicon.  So I don't think that is really
necessary.  The Linux code doesn't do this.

> Either way, ok by me.

Thanks

Re: bwfm(4): support scan v3

[Stefan Sperling]

On Tue, Oct 10, 2023 at 11:41:39PM +0200, Mark Kettenis wrote:
> The firmware for the BCM4388 has yet another version of the "escan"
> command.  But we can treat it the same as v2 since it just added a new
> parameter in place of some padding.  We just set that new parameter to
> zero, which doesn't change anything.
> 
> As a bonus this adds some missing htole16() calls.
> 
> This is the equivalent of:
> 
> https://github.com/AsahiLinux/linux/commit/399ef7b1cb9094c1c64e0f9ad6caa5c4d114009f
> 
> ok?

> @@ -274,8 +275,10 @@ bwfm_preinit(struct bwfm_softc *sc)
>   nmode = 0;
>   if (bwfm_fwvar_var_get_int(sc, "vhtmode", ))
>   vhtmode = 0;
> - if (bwfm_fwvar_var_get_int(sc, "scan_ver", >sc_scan_ver))
> - sc->sc_scan_ver = 0;
> + if (bwfm_fwvar_var_get_data(sc, "scan_ver", _ver,
> + sizeof(scan_ver)))
> + scan_ver.scan_ver_major = 0;
> + sc->sc_scan_ver = letoh16(scan_ver.scan_ver_major);

Perhaps check whether firmware reports a supported scan command version,
and fail or print a warning when it doesn't?
That might make future firmware upgrades a bit easier in case the vendor
changes this again.

Either way, ok by me.

bwfm(4): support scan v3

[Mark Kettenis]

The firmware for the BCM4388 has yet another version of the "escan"
command.  But we can treat it the same as v2 since it just added a new
parameter in place of some padding.  We just set that new parameter to
zero, which doesn't change anything.

As a bonus this adds some missing htole16() calls.

This is the equivalent of:

https://github.com/AsahiLinux/linux/commit/399ef7b1cb9094c1c64e0f9ad6caa5c4d114009f

ok?


Index: dev/ic/bwfm.c
===
RCS file: /cvs/src/sys/dev/ic/bwfm.c,v
retrieving revision 1.110
diff -u -p -r1.110 bwfm.c
--- dev/ic/bwfm.c   9 Oct 2023 21:49:34 -   1.110
+++ dev/ic/bwfm.c   10 Oct 2023 21:36:44 -
@@ -249,6 +249,7 @@ bwfm_preinit(struct bwfm_softc *sc)
struct ifnet *ifp = >ic_if;
int i, j, nbands, nmode, vhtmode;
uint32_t bandlist[3], tmp;
+   struct bwfm_scan_version scan_ver;
 
if (sc->sc_initialized)
return 0;
@@ -274,8 +275,10 @@ bwfm_preinit(struct bwfm_softc *sc)
nmode = 0;
if (bwfm_fwvar_var_get_int(sc, "vhtmode", ))
vhtmode = 0;
-   if (bwfm_fwvar_var_get_int(sc, "scan_ver", >sc_scan_ver))
-   sc->sc_scan_ver = 0;
+   if (bwfm_fwvar_var_get_data(sc, "scan_ver", _ver,
+   sizeof(scan_ver)))
+   scan_ver.scan_ver_major = 0;
+   sc->sc_scan_ver = letoh16(scan_ver.scan_ver_major);
if (bwfm_fwvar_cmd_get_data(sc, BWFM_C_GET_BANDLIST, bandlist,
sizeof(bandlist))) {
printf("%s: couldn't get supported band list\n", DEVNAME(sc));
@@ -2201,17 +2204,21 @@ bwfm_scan_v2(struct bwfm_softc *sc)
ssid = (struct bwfm_ssid *)
(((uint8_t *)params) + sizeof(*params) + chan_size);
 
-   params->scan_params.version = 2;
-   params->scan_params.length = params_size;
+   params->scan_params.version = htole16(sc->sc_scan_ver);
+   params->scan_params.length = htole16(params_size);
memset(params->scan_params.bssid, 0xff,
sizeof(params->scan_params.bssid));
params->scan_params.bss_type = 2;
+   params->scan_params.ssid_type = 0;
params->scan_params.scan_type = BWFM_SCANTYPE_PASSIVE;
params->scan_params.nprobes = htole32(-1);
params->scan_params.active_time = htole32(-1);
params->scan_params.passive_time = htole32(-1);
params->scan_params.home_time = htole32(-1);
-   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V2);
+   if (sc->sc_scan_ver == 3)
+   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V3);
+   else
+   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V2);
params->action = htole16(WL_ESCAN_ACTION_START);
params->sync_id = htole16(0x1234);
 
@@ -2282,17 +2289,21 @@ bwfm_scan_abort_v2(struct bwfm_softc *sc
 
params_size = sizeof(*params) + sizeof(uint16_t);
params = malloc(params_size, M_TEMP, M_WAITOK | M_ZERO);
-   params->scan_params.version = 2;
-   params->scan_params.length = params_size;
+   params->scan_params.version = htole16(sc->sc_scan_ver);
+   params->scan_params.length = htole16(params_size);
memset(params->scan_params.bssid, 0xff,
sizeof(params->scan_params.bssid));
params->scan_params.bss_type = 2;
+   params->scan_params.ssid_type = 0;
params->scan_params.scan_type = BWFM_SCANTYPE_PASSIVE;
params->scan_params.nprobes = htole32(-1);
params->scan_params.active_time = htole32(-1);
params->scan_params.passive_time = htole32(-1);
params->scan_params.home_time = htole32(-1);
-   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V2);
+   if (sc->sc_scan_ver == 3)
+   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V3);
+   else
+   params->version = htole32(BWFM_ESCAN_REQ_VERSION_V2);
params->action = htole16(WL_ESCAN_ACTION_START);
params->sync_id = htole16(0x1234);
params->scan_params.channel_num = htole32(1);
Index: dev/ic/bwfmreg.h
===
RCS file: /cvs/src/sys/dev/ic/bwfmreg.h,v
retrieving revision 1.26
diff -u -p -r1.26 bwfmreg.h
--- dev/ic/bwfmreg.h4 Mar 2022 22:34:41 -   1.26
+++ dev/ic/bwfmreg.h10 Oct 2023 21:36:44 -
@@ -511,6 +511,12 @@ struct bwfm_ssid {
uint8_t ssid[BWFM_MAX_SSID_LEN];
 };
 
+struct bwfm_scan_version {
+   uint16_t version;
+   uint16_t len;
+   uint16_t scan_ver_major;
+};
+
 struct bwfm_scan_params_v0 {
struct bwfm_ssid ssid;
uint8_t bssid[ETHER_ADDR_LEN];
@@ -538,7 +544,7 @@ struct bwfm_scan_params_v2 {
struct bwfm_ssid ssid;
uint8_t bssid[ETHER_ADDR_LEN];
uint8_t bss_type;
-   uint8_t pad;
+   uint8_t ssid_type;
uint32_t scan_type;
uint32_t nprobes;
uint32_t active_time;
@@ -569,6 +575,7 @@ struct