Re: [PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features

2018-06-25 Thread Arend van Spriel 

On 6/24/2018 4:08 PM, Rafał Miłecki wrote:

On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
 wrote:

Firmwares may not provide all monitor mode features in the "cap" iovar.
For those this fallback mechanism uses "sta_monitor" iovar. If firmware
is compiled with stamon, this iovar will fail with BCME_NOTUP; Otherwise
it fails with BCME_UNSUPPORTED.


It's probably not the first time ever, but it appears your research
(theory) doesn't match my experience (practice) ;) I'm afraid you
missed some important check when analyzing firmware code.


It was not all theory ;-) but apparently I did not cover all bases. I 
only checked with 4366c0 (actually with 43664 aka 4366E) on the release 
branch I am working on.



I've just tested all firmwares I got (for 43602a1, 4366b1 and 4366c0)
and all of them return -4 (BCME_NOTUP) for "sta_monitor" when
firmware/interface is down. It appears this test requires bringing
firmware/interface up to make it reliable. Apparently even firmwares
*without* sta_monitor return -4 (BCME_NOTUP) when firmware/interface
is down.


That is crap. So back to the drawing board. Thanks for keeping the taps 
on this.


Regards,
Arend

Re: [PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features

2018-06-24 Thread Rafał Miłecki 
On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
 wrote:
> Firmwares may not provide all monitor mode features in the "cap" iovar.
> For those this fallback mechanism uses "sta_monitor" iovar. If firmware
> is compiled with stamon, this iovar will fail with BCME_NOTUP; Otherwise
> it fails with BCME_UNSUPPORTED.

It's probably not the first time ever, but it appears your research
(theory) doesn't match my experience (practice) ;) I'm afraid you
missed some important check when analyzing firmware code.

I've just tested all firmwares I got (for 43602a1, 4366b1 and 4366c0)
and all of them return -4 (BCME_NOTUP) for "sta_monitor" when
firmware/interface is down. It appears this test requires bringing
firmware/interface up to make it reliable. Apparently even firmwares
*without* sta_monitor return -4 (BCME_NOTUP) when firmware/interface
is down.

-- 
Rafał

[PATCH 6/6] brcmfmac: fallback mechanism to determine monitor mode features

2018-06-22 Thread Arend van Spriel 
Firmwares may not provide all monitor mode features in the "cap" iovar.
For those this fallback mechanism uses "sta_monitor" iovar. If firmware
is compiled with stamon, this iovar will fail with BCME_NOTUP; Otherwise
it fails with BCME_UNSUPPORTED.

Reviewed-by: Hante Meuleman 
Reviewed-by: Pieter-Paul Giesberts 
Reviewed-by: Franky Lin 
Signed-off-by: Arend van Spriel 
---
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c  | 15 +++
 .../broadcom/brcm80211/brcmfmac/fwil_types.h| 21 +
 2 files changed, 36 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index f70fec6..cb57a4a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -207,6 +207,7 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
struct brcmf_pno_macaddr_le pfn_mac;
struct brcmf_gscan_config gscan_cfg;
+   struct brcmf_stamon_sta_config stamon_cfg;
u32 wowl_cap;
s32 err;
 
@@ -217,6 +218,20 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
brcmf_feat_iovar_data_set(ifp, BRCMF_FEAT_GSCAN,
  "pfn_gscan_cfg",
  _cfg, sizeof(gscan_cfg));
+
+   if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR) ||
+   !brcmf_feat_is_enabled(ifp, BRCMF_FEAT_RADIOTAP)) {
+   ifp->fwil_fwerr = true;
+   memset(_cfg, 0, sizeof(stamon_cfg));
+   /* fails either way as firmware stack is not up yet */
+   err = brcmf_fil_iovar_data_set(ifp, "sta_monitor", _cfg,
+  sizeof(stamon_cfg));
+   if (err != BRCMF_FW_UNSUPPORTED) {
+   ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_MONITOR);
+   ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_RADIOTAP);
+   }
+   ifp->fwil_fwerr = false;
+   }
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_PNO, "pfn");
if (drvr->bus_if->wowl_supported)
brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_WOWL, "wowl");
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h 
b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index 4b29070..db56c81 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -963,4 +963,25 @@ struct brcmf_gscan_config {
struct brcmf_gscan_bucket_config bucket[1];
 };
 
+/**
+ * struct brcmf_stamon_sta_config - configuration data for sta monitor.
+ *
+ * @cmd: subcommand for this configuration.
+ * @mac: mac address of STA for which @cmd is intended.
+ * @version: version of this configuration structure.
+ * @length: number of bytes following this field.
+ * @chanspec: channel of the STA.
+ * @mon_time: time for which STA's are monitored (ms).
+ * @offchan_time: timer for which off-channel STA's are monitored.
+ */
+struct brcmf_stamon_sta_config {
+   __le32 cmd;
+   u8 mac[ETH_ALEN];
+   __le16 version;
+   __le16 length;
+   __le16 chanspec;
+   __le32 monitor_time;
+   __le32 offchan_time;
+};
+
 #endif /* FWIL_TYPES_H_ */
-- 
1.9.1