[PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Arend van Spriel]

Here a couple of patches in preparation of monitor mode support. It
is mostly about querying firmware for support. While at it I stumbled
upon the fact that 160MHz was not completely implemented in the driver
so there is a patch for that as well.

The first two patches are actually some changes to the patches that
Rafal submitted. So this series depend on:

[V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]

These apply to the master branch of the wireless-drivers-next repository.

[1] https://patchwork.kernel.org/patch/10474799/

Arend van Spriel (6):
  brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR
  brcmfmac: rename BRCMF_FEAT_MONITOR_FMT_RADIOTAP to
    BRCMF_FEAT_RADIOTAP
  brcmfmac: fix for proper support of 160MHz bandwidth
  brcmfmac: increase buffer for obtaining firmware capabilities
  brcmfmac: add new feature flags for monitor mode operation
  brcmfmac: fallback mechanism to determine monitor mode features

 .../wireless/broadcom/brcm80211/brcmfmac/core.c    |  2 +-
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 43 +++++++++-------------
 .../wireless/broadcom/brcm80211/brcmfmac/feature.h |  6 +--
 .../broadcom/brcm80211/brcmfmac/fwil_types.h       | 21 +++++++++++
 .../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 ++++++++++++++++-
 .../broadcom/brcm80211/include/brcmu_wifi.h        |  2 +
 6 files changed, 77 insertions(+), 31 deletions(-)

-- 
1.9.1

[PATCH 1/6] brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR

[Arend van Spriel]

Commit 19f505aa3ae0 ("brcmfmac: detect firmware support for monitor
interface") introduced a fallback mechanism attempting BRCMF_C_MONITOR
command. However, that does not indicate firmware support. Unfortunately,
it only indicates the core part of the stack supports it, but not whether
it is supported in full-dongle mode, ie. firmware has means to transfer
the monitor packets to the host. Firmwares that return the "monitor"
flag in the "cap" iovar are able to send the packets to the host.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/feature.c | 25 ----------------------
 1 file changed, 25 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 48d7978..a78b9ba 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -92,26 +92,6 @@ static int brcmf_feat_debugfs_read(struct seq_file *seq, void *data)
 }
 #endif /* DEBUG */
 
-static void brcmf_feat_cmd_int_get(struct brcmf_if *ifp, enum brcmf_feat_id id,
-				   u32 cmd)
-{
-	u32 data;
-	int err;
-
-	ifp->fwil_fwerr = true;
-
-	err = brcmf_fil_cmd_int_get(ifp, cmd, &data);
-	if (err == 0) {
-		brcmf_dbg(INFO, "enabling feature: %s\n", brcmf_feat_names[id]);
-		ifp->drvr->feat_flags |= BIT(id);
-	} else {
-		brcmf_dbg(TRACE, "%s feature check failed: %d\n",
-			  brcmf_feat_names[id], err);
-	}
-
-	ifp->fwil_fwerr = false;
-}
-
 /**
  * brcmf_feat_iovar_int_get() - determine feature through iovar query.
  *
@@ -272,11 +252,6 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
 	}
 	brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_FWSUP, "sup_wpa");
 
-	/* Fallback detection for older firmwares */
-	if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR))
-		brcmf_feat_cmd_int_get(ifp, BRCMF_FEAT_MONITOR,
-				       BRCMF_C_GET_MONITOR);
-
 	/* set chip related quirks */
 	switch (drvr->bus_if->chip) {
 	case BRCM_CC_43236_CHIP_ID:
-- 
1.9.1

Re: [1/6] brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR

[Kalle Valo]

Arend Van Spriel <arend.vanspriel@broadcom.com> wrote:

> Commit 19f505aa3ae0 ("brcmfmac: detect firmware support for monitor
> interface") introduced a fallback mechanism attempting BRCMF_C_MONITOR
> command. However, that does not indicate firmware support. Unfortunately,
> it only indicates the core part of the stack supports it, but not whether
> it is supported in full-dongle mode, ie. firmware has means to transfer
> the monitor packets to the host. Firmwares that return the "monitor"
> flag in the "cap" iovar are able to send the packets to the host.
> 
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

Dropping per Arend's request

6 patches set to Changes Requested.

10482829 [1/6] brcmfmac: remove fallback mechanism for BRCMF_FEAT_MONITOR
10482837 [2/6] brcmfmac: rename BRCMF_FEAT_MONITOR_FMT_RADIOTAP to BRCMF_FEAT_RADIOTAP
10482833 [3/6] brcmfmac: fix for proper support of 160MHz bandwidth
10482835 [4/6] brcmfmac: increase buffer for obtaining firmware capabilities
10482827 [5/6] brcmfmac: add new feature flags for monitor mode operation
10482831 [6/6] brcmfmac: fallback mechanism to determine monitor mode features

-- 
https://patchwork.kernel.org/patch/10482829/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

[PATCH 2/6] brcmfmac: rename BRCMF_FEAT_MONITOR_FMT_RADIOTAP to BRCMF_FEAT_RADIOTAP

[Arend van Spriel]

Just a bit of bike-shedding so nothing functionally changed.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c    | 2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index b1f702f..d3fdbb3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -407,7 +407,7 @@ void brcmf_netif_rx(struct brcmf_if *ifp, struct sk_buff *skb)
 
 void brcmf_netif_mon_rx(struct brcmf_if *ifp, struct sk_buff *skb)
 {
-	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MONITOR_FMT_RADIOTAP)) {
+	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_RADIOTAP)) {
 		/* Do nothing */
 	} else {
 		struct ieee80211_radiotap_header *radiotap;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index 0782f2a..31a746b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -34,7 +34,7 @@
  * GSCAN: enhanced scan offload feature.
  * FWSUP: Firmware supplicant.
  * MONITOR: monitor interface
- * MONITOR_FMT_RADIOTAP: monitor packets include radiotap header
+ * RADIOTAP: monitor packets include radiotap header
  */
 #define BRCMF_FEAT_LIST \
 	BRCMF_FEAT_DEF(MBSS) \
@@ -52,7 +52,7 @@
 	BRCMF_FEAT_DEF(GSCAN) \
 	BRCMF_FEAT_DEF(FWSUP) \
 	BRCMF_FEAT_DEF(MONITOR) \
-	BRCMF_FEAT_DEF(MONITOR_FMT_RADIOTAP)
+	BRCMF_FEAT_DEF(RADIOTAP)
 
 /*
  * Quirks:
-- 
1.9.1

[PATCH 3/6] brcmfmac: fix for proper support of 160MHz bandwidth

[Arend van Spriel]

Decoding of firmware channel information was not complete for 160MHz
support. This resulted in the following warning:

  WARNING: CPU: 2 PID: 2222 at .../broadcom/brcm80211/brcmutil/d11.c:196
	brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
  Modules linked in: brcmfmac(O) brcmutil(O) sha256_generic cfg80211 ...
  CPU: 2 PID: 2222 Comm: kworker/2:0 Tainted: G           O
  4.17.0-wt-testing-x64-00002-gf1bed50 #1
  Hardware name: Dell Inc. Latitude E6410/07XJP9, BIOS A07 02/15/2011
  Workqueue: events request_firmware_work_func
  RIP: 0010:brcmu_d11ac_decchspec+0x2e/0x100 [brcmutil]
  RSP: 0018:ffffc90000047bd0 EFLAGS: 00010206
  RAX: 000000000000e832 RBX: ffff8801146fe910 RCX: ffff8801146fd3c0
  RDX: 0000000000002800 RSI: 0000000000000070 RDI: ffffc90000047c30
  RBP: ffffc90000047bd0 R08: 0000000000000000 R09: ffffffffa0798c80
  R10: ffff88012bca55e0 R11: ffff880110a4ea00 R12: ffff8801146f8000
  R13: ffffc90000047c30 R14: ffff8801146fe930 R15: ffff8801138e02e0
  FS:  0000000000000000(0000) GS:ffff88012bc80000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007f18ce8b8070 CR3: 000000000200a003 CR4: 00000000000206e0
  Call Trace:
   brcmf_setup_wiphybands+0x212/0x780 [brcmfmac]
   brcmf_cfg80211_attach+0xae2/0x11a0 [brcmfmac]
   brcmf_attach+0x1fc/0x4b0 [brcmfmac]
   ? __kmalloc+0x13c/0x1c0
   brcmf_pcie_setup+0x99b/0xe00 [brcmfmac]
   brcmf_fw_request_done+0x16a/0x1f0 [brcmfmac]
   request_firmware_work_func+0x36/0x60
   process_one_work+0x146/0x350
   worker_thread+0x4a/0x3b0
   kthread+0x102/0x140
   ? process_one_work+0x350/0x350
   ? kthread_bind+0x20/0x20
   ret_from_fork+0x35/0x40
  Code: 66 90 0f b7 07 55 48 89 e5 89 c2 88 47 02 88 47 03 66 81 e2 00 38
	66 81 fa 00 18 74 6e 66 81 fa 00 20 74 39 66 81 fa 00 10 74 14 <0f>
	0b 66 25 00 c0 74 20 66 3d 00 c0 75 20 c6 47 04 01 5d c3 66
  ---[ end trace 550c46682415b26d ]---
  brcmfmac: brcmf_construct_chaninfo: Ignoring unexpected firmware channel 50

This patch adds the missing stuff to properly handle this.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../net/wireless/broadcom/brcm80211/brcmutil/d11.c | 34 +++++++++++++++++++++-
 .../broadcom/brcm80211/include/brcmu_wifi.h        |  2 ++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
index d8b79cb..e7584b8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmutil/d11.c
@@ -77,6 +77,8 @@ static u16 d11ac_bw(enum brcmu_chan_bw bw)
 		return BRCMU_CHSPEC_D11AC_BW_40;
 	case BRCMU_CHAN_BW_80:
 		return BRCMU_CHSPEC_D11AC_BW_80;
+	case BRCMU_CHAN_BW_160:
+		return BRCMU_CHSPEC_D11AC_BW_160;
 	default:
 		WARN_ON(1);
 	}
@@ -190,8 +192,38 @@ static void brcmu_d11ac_decchspec(struct brcmu_chan *ch)
 			break;
 		}
 		break;
-	case BRCMU_CHSPEC_D11AC_BW_8080:
 	case BRCMU_CHSPEC_D11AC_BW_160:
+		switch (ch->sb) {
+		case BRCMU_CHAN_SB_LLL:
+			ch->control_ch_num -= CH_70MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_LLU:
+			ch->control_ch_num -= CH_50MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_LUL:
+			ch->control_ch_num -= CH_30MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_LUU:
+			ch->control_ch_num -= CH_10MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_ULL:
+			ch->control_ch_num += CH_10MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_ULU:
+			ch->control_ch_num += CH_30MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_UUL:
+			ch->control_ch_num += CH_50MHZ_APART;
+			break;
+		case BRCMU_CHAN_SB_UUU:
+			ch->control_ch_num += CH_70MHZ_APART;
+			break;
+		default:
+			WARN_ON_ONCE(1);
+			break;
+		}
+		break;
+	case BRCMU_CHSPEC_D11AC_BW_8080:
 	default:
 		WARN_ON_ONCE(1);
 		break;
diff --git a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
index 7b9a779..75b2a04 100644
--- a/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
+++ b/drivers/net/wireless/broadcom/brcm80211/include/brcmu_wifi.h
@@ -29,6 +29,8 @@
 #define CH_UPPER_SB			0x01
 #define CH_LOWER_SB			0x02
 #define CH_EWA_VALID			0x04
+#define CH_70MHZ_APART			14
+#define CH_50MHZ_APART			10
 #define CH_30MHZ_APART			6
 #define CH_20MHZ_APART			4
 #define CH_10MHZ_APART			2
-- 
1.9.1

[PATCH 4/6] brcmfmac: increase buffer for obtaining firmware capabilities

[Arend van Spriel]

When obtaining the firmware capability a buffer is provided of 512
bytes. However, if all features in firmware are supported the buffer
needs to be 565 bytes as otherwise truncated information is retrieved
from firmware. Increasing the buffer to 768 bytes on stack.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index a78b9ba..0ee3a25 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -141,7 +141,7 @@ static void brcmf_feat_iovar_data_set(struct brcmf_if *ifp,
 	ifp->fwil_fwerr = false;
 }
 
-#define MAX_CAPS_BUFFER_SIZE	512
+#define MAX_CAPS_BUFFER_SIZE	768
 static void brcmf_feat_firmware_capabilities(struct brcmf_if *ifp)
 {
 	char caps[MAX_CAPS_BUFFER_SIZE];
-- 
1.9.1

[PATCH 5/6] brcmfmac: add new feature flags for monitor mode operation

[Arend van Spriel]

Add two new feature flags to be used for monitor mode. One indicating
support to enable packet capture and one indicating captured packets
from firmware already include radiotap header.

Signed-off-by: Rafal Milecki <rafal@milecki.pl>
[arend: change flags and description]
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c | 1 +
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
index 0ee3a25..f70fec6 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
@@ -49,6 +49,7 @@ struct brcmf_feat_fwcap {
 	{ BRCMF_FEAT_MCHAN, "mchan" },
 	{ BRCMF_FEAT_P2P, "p2p" },
 	{ BRCMF_FEAT_MONITOR, "monitor" },
+	{ BRCMF_FEAT_RADIOTAP, "rtap" },
 };
 
 #ifdef DEBUG
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
index 31a746b..31198d0 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
@@ -33,8 +33,8 @@
  * MFP: 802.11w Management Frame Protection.
  * GSCAN: enhanced scan offload feature.
  * FWSUP: Firmware supplicant.
- * MONITOR: monitor interface
- * RADIOTAP: monitor packets include radiotap header
+ * MONITOR: firmware can pass monitor packets to host.
+ * RADIOTAP: firmware provides monitor packets including radiotap header
  */
 #define BRCMF_FEAT_LIST \
 	BRCMF_FEAT_DEF(MBSS) \
-- 
1.9.1

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

[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 <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../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",
 					  &gscan_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(&stamon_cfg, 0, sizeof(stamon_cfg));
+		/* fails either way as firmware stack is not up yet */
+		err = brcmf_fil_iovar_data_set(ifp, "sta_monitor", &stamon_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

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

[Rafał Miłecki]

On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
<arend.vanspriel@broadcom.com> 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.

--=20
Rafa=C5=82

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

[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
> <arend.vanspriel@broadcom.com> 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 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> Here a couple of patches in preparation of monitor mode support. It
> is mostly about querying firmware for support. While at it I stumbled
> upon the fact that 160MHz was not completely implemented in the driver
> so there is a patch for that as well.
>
> The first two patches are actually some changes to the patches that
> Rafal submitted. So this series depend on:
>
> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
>
> These apply to the master branch of the wireless-drivers-next repository.
>
> [1] https://patchwork.kernel.org/patch/10474799/

I find it pointless to submit fixes for patches that weren't accepted
yet. Let's work on improving original patches while they are still
pending.

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

On Sun, 24 Jun 2018 at 13:48, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> > Here a couple of patches in preparation of monitor mode support. It
> > is mostly about querying firmware for support. While at it I stumbled
> > upon the fact that 160MHz was not completely implemented in the driver
> > so there is a patch for that as well.
> >
> > The first two patches are actually some changes to the patches that
> > Rafal submitted. So this series depend on:
> >
> > [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
> >
> > These apply to the master branch of the wireless-drivers-next repositor=
y.
> >
> > [1] https://patchwork.kernel.org/patch/10474799/
>
> I find it pointless to submit fixes for patches that weren't accepted
> yet. Let's work on improving original patches while they are still
> pending.

I sent V4 with changes pointed our by Arend. That obsoletes all (?)
monitor patches from this patchset. I believe it was cleaner to
improve original patchset (not pushed yet).

My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
related) and drop the others.

--=20
Rafa=C5=82

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Arend van Spriel]

On 6/25/2018 6:40 AM, Rafał Miłecki wrote:
> On Sun, 24 Jun 2018 at 13:48, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> Here a couple of patches in preparation of monitor mode support. It
>>> is mostly about querying firmware for support. While at it I stumbled
>>> upon the fact that 160MHz was not completely implemented in the driver
>>> so there is a patch for that as well.
>>>
>>> The first two patches are actually some changes to the patches that
>>> Rafal submitted. So this series depend on:
>>>
>>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
>>>
>>> These apply to the master branch of the wireless-drivers-next repository.
>>>
>>> [1] https://patchwork.kernel.org/patch/10474799/
>>
>> I find it pointless to submit fixes for patches that weren't accepted
>> yet. Let's work on improving original patches while they are still
>> pending.
>
> I sent V4 with changes pointed our by Arend. That obsoletes all (?)
> monitor patches from this patchset. I believe it was cleaner to
> improve original patchset (not pushed yet).
>
> My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
> related) and drop the others.

Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit 
the series addressing the issues you found.

Regards,
Arend

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

On Mon, 25 Jun 2018 at 10:31, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 6/25/2018 6:40 AM, Rafa=C5=82 Mi=C5=82ecki wrote:
> > On Sun, 24 Jun 2018 at 13:48, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com=
> wrote:
> >> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> >> <arend.vanspriel@broadcom.com> wrote:
> >>> Here a couple of patches in preparation of monitor mode support. It
> >>> is mostly about querying firmware for support. While at it I stumbled
> >>> upon the fact that 160MHz was not completely implemented in the drive=
r
> >>> so there is a patch for that as well.
> >>>
> >>> The first two patches are actually some changes to the patches that
> >>> Rafal submitted. So this series depend on:
> >>>
> >>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
> >>>
> >>> These apply to the master branch of the wireless-drivers-next reposit=
ory.
> >>>
> >>> [1] https://patchwork.kernel.org/patch/10474799/
> >>
> >> I find it pointless to submit fixes for patches that weren't accepted
> >> yet. Let's work on improving original patches while they are still
> >> pending.
> >
> > I sent V4 with changes pointed our by Arend. That obsoletes all (?)
> > monitor patches from this patchset. I believe it was cleaner to
> > improve original patchset (not pushed yet).
> >
> > My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
> > related) and drop the others.
>
> Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit
> the series addressing the issues you found.

I wasn't aware of this, good to know, thanks!

--=20
Rafa=C5=82

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Kalle Valo]

Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> writes:

> On Mon, 25 Jun 2018 at 10:31, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>>
>> On 6/25/2018 6:40 AM, Rafa=C5=82 Mi=C5=82ecki wrote:
>> > On Sun, 24 Jun 2018 at 13:48, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.co=
m> wrote:
>> >> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
>> >> <arend.vanspriel@broadcom.com> wrote:
>> >>> Here a couple of patches in preparation of monitor mode support. It
>> >>> is mostly about querying firmware for support. While at it I stumbled
>> >>> upon the fact that 160MHz was not completely implemented in the driv=
er
>> >>> so there is a patch for that as well.
>> >>>
>> >>> The first two patches are actually some changes to the patches that
>> >>> Rafal submitted. So this series depend on:
>> >>>
>> >>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
>> >>>
>> >>> These apply to the master branch of the wireless-drivers-next reposi=
tory.
>> >>>
>> >>> [1] https://patchwork.kernel.org/patch/10474799/
>> >>
>> >> I find it pointless to submit fixes for patches that weren't accepted
>> >> yet. Let's work on improving original patches while they are still
>> >> pending.
>> >
>> > I sent V4 with changes pointed our by Arend. That obsoletes all (?)
>> > monitor patches from this patchset. I believe it was cleaner to
>> > improve original patchset (not pushed yet).
>> >
>> > My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
>> > related) and drop the others.
>>
>> Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit
>> the series addressing the issues you found.
>
> I wasn't aware of this, good to know, thanks!

Yes, "all-or-nothing" method is both fastest and less error prone,
that's why I prefer that. I got a bit lost with your discussion so
please check carefully that I haven't dropped anything wrong.

--=20
Kalle Valo

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

Hi Arend,

On Mon, 25 Jun 2018 at 10:31, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 6/25/2018 6:40 AM, Rafa=C5=82 Mi=C5=82ecki wrote:
> > On Sun, 24 Jun 2018 at 13:48, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com=
> wrote:
> >> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> >> <arend.vanspriel@broadcom.com> wrote:
> >>> Here a couple of patches in preparation of monitor mode support. It
> >>> is mostly about querying firmware for support. While at it I stumbled
> >>> upon the fact that 160MHz was not completely implemented in the drive=
r
> >>> so there is a patch for that as well.
> >>>
> >>> The first two patches are actually some changes to the patches that
> >>> Rafal submitted. So this series depend on:
> >>>
> >>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
> >>>
> >>> These apply to the master branch of the wireless-drivers-next reposit=
ory.
> >>>
> >>> [1] https://patchwork.kernel.org/patch/10474799/
> >>
> >> I find it pointless to submit fixes for patches that weren't accepted
> >> yet. Let's work on improving original patches while they are still
> >> pending.
> >
> > I sent V4 with changes pointed our by Arend. That obsoletes all (?)
> > monitor patches from this patchset. I believe it was cleaner to
> > improve original patchset (not pushed yet).
> >
> > My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
> > related) and drop the others.
>
> Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit
> the series addressing the issues you found.

Would you have a moment to resubmit these 2 patches or do you mind if I do =
that?

--=20
Rafa=C5=82

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Arend van Spriel]

On 8/29/2018 10:17 PM, Rafał Miłecki wrote:
> Hi Arend,
>
> On Mon, 25 Jun 2018 at 10:31, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 6/25/2018 6:40 AM, Rafał Miłecki wrote:
>>> On Sun, 24 Jun 2018 at 13:48, Rafał Miłecki <zajec5@gmail.com> wrote:
>>>> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
>>>> <arend.vanspriel@broadcom.com> wrote:
>>>>> Here a couple of patches in preparation of monitor mode support. It
>>>>> is mostly about querying firmware for support. While at it I stumbled
>>>>> upon the fact that 160MHz was not completely implemented in the driver
>>>>> so there is a patch for that as well.
>>>>>
>>>>> The first two patches are actually some changes to the patches that
>>>>> Rafal submitted. So this series depend on:
>>>>>
>>>>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
>>>>>
>>>>> These apply to the master branch of the wireless-drivers-next repository.
>>>>>
>>>>> [1] https://patchwork.kernel.org/patch/10474799/
>>>>
>>>> I find it pointless to submit fixes for patches that weren't accepted
>>>> yet. Let's work on improving original patches while they are still
>>>> pending.
>>>
>>> I sent V4 with changes pointed our by Arend. That obsoletes all (?)
>>> monitor patches from this patchset. I believe it was cleaner to
>>> improve original patchset (not pushed yet).
>>>
>>> My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
>>> related) and drop the others.
>>
>> Well. Given that Kalle prefers applying "all-or-nothing" I will resubmit
>> the series addressing the issues you found.
>
> Would you have a moment to resubmit these 2 patches or do you mind if I do that?

Found a moment. Should be in patchwork now.

Regards,
Arend

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

On Wed, 5 Sep 2018 at 13:06, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 8/29/2018 10:17 PM, Rafa=C5=82 Mi=C5=82ecki wrote:
> > Hi Arend,
> >
> > On Mon, 25 Jun 2018 at 10:31, Arend van Spriel
> > <arend.vanspriel@broadcom.com> wrote:
> >> On 6/25/2018 6:40 AM, Rafa=C5=82 Mi=C5=82ecki wrote:
> >>> On Sun, 24 Jun 2018 at 13:48, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.c=
om> wrote:
> >>>> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> >>>> <arend.vanspriel@broadcom.com> wrote:
> >>>>> Here a couple of patches in preparation of monitor mode support. It
> >>>>> is mostly about querying firmware for support. While at it I stumbl=
ed
> >>>>> upon the fact that 160MHz was not completely implemented in the dri=
ver
> >>>>> so there is a patch for that as well.
> >>>>>
> >>>>> The first two patches are actually some changes to the patches that
> >>>>> Rafal submitted. So this series depend on:
> >>>>>
> >>>>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
> >>>>>
> >>>>> These apply to the master branch of the wireless-drivers-next repos=
itory.
> >>>>>
> >>>>> [1] https://patchwork.kernel.org/patch/10474799/
> >>>>
> >>>> I find it pointless to submit fixes for patches that weren't accepte=
d
> >>>> yet. Let's work on improving original patches while they are still
> >>>> pending.
> >>>
> >>> I sent V4 with changes pointed our by Arend. That obsoletes all (?)
> >>> monitor patches from this patchset. I believe it was cleaner to
> >>> improve original patchset (not pushed yet).
> >>>
> >>> My suggestion is to apply patches 3/6 and 4/6 (they aren't monitor
> >>> related) and drop the others.
> >>
> >> Well. Given that Kalle prefers applying "all-or-nothing" I will resubm=
it
> >> the series addressing the issues you found.
> >
> > Would you have a moment to resubmit these 2 patches or do you mind if I=
 do that?
>
> Found a moment. Should be in patchwork now.

Thank you! :)

--=20
Rafa=C5=82

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Arend van Spriel]

On 6/24/2018 1:48 PM, Rafał Miłecki wrote:
> On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> Here a couple of patches in preparation of monitor mode support. It
>> is mostly about querying firmware for support. While at it I stumbled
>> upon the fact that 160MHz was not completely implemented in the driver
>> so there is a patch for that as well.
>>
>> The first two patches are actually some changes to the patches that
>> Rafal submitted. So this series depend on:
>>
>> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
>>
>> These apply to the master branch of the wireless-drivers-next repository.
>>
>> [1] https://patchwork.kernel.org/patch/10474799/
>
> I find it pointless to submit fixes for patches that weren't accepted
> yet. Let's work on improving original patches while they are still
> pending.

It is not unprecedented. There are enough patch series sent in the past 
that relied on other still pending series. I was unsure how to deal with 
it as I acked your changes already and did not exactly know when Kalle 
would start applying the patches. I am fine either way.

Regards,
Arend

Re: [PATCH 0/6] brcmfmac: fix 160MHz support and monitor mode

[Rafał Miłecki]

On Mon, 25 Jun 2018 at 10:18, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 6/24/2018 1:48 PM, Rafa=C5=82 Mi=C5=82ecki wrote:
> > On Fri, 22 Jun 2018 at 20:45, Arend van Spriel
> > <arend.vanspriel@broadcom.com> wrote:
> >> Here a couple of patches in preparation of monitor mode support. It
> >> is mostly about querying firmware for support. While at it I stumbled
> >> upon the fact that 160MHz was not completely implemented in the driver
> >> so there is a patch for that as well.
> >>
> >> The first two patches are actually some changes to the patches that
> >> Rafal submitted. So this series depend on:
> >>
> >> [V3,2/2] brcmfmac: handle monitor mode marked msgbuf packets [1]
> >>
> >> These apply to the master branch of the wireless-drivers-next reposito=
ry.
> >>
> >> [1] https://patchwork.kernel.org/patch/10474799/
> >
> > I find it pointless to submit fixes for patches that weren't accepted
> > yet. Let's work on improving original patches while they are still
> > pending.
>
> It is not unprecedented. There are enough patch series sent in the past
> that relied on other still pending series. I was unsure how to deal with
> it as I acked your changes already and did not exactly know when Kalle
> would start applying the patches. I am fine either way.

Well, things happen, I don't mind if you un-ack my patch, after
discovering some problem with it :)

--=20
Rafa=C5=82