[PATCH v3 0/3] wcn36xx: Implement explicit beacon filter tables

[PATCH v3 0/3] wcn36xx: Implement explicit beacon filter tables

[Bryan O'Donoghue]

V3:
Adds check to updating beacon filter for NL80211_IFTYPE_STATION only - Loic
Adds Reviewed-by as indicated - Loic

V2:
Removes duplicate definition of mask field defines

V1:
Downstream provides the ability to directly program the beacon filter
tables. Currently in upstream we rely on whatever is the default filtration
table in firmware.

A trivial packing fixup is required for the SMD structure. The downstream
filtration table from the Linux driver is applied but, we are not
necessarily constrained to using this table forever.

Tested on wcn3620 and wcn3680b.

Bryan O'Donoghue (3):
  wcn36xx: Fix beacon filter structure definitions
  wcn36xx: Fix physical location of beacon filter comment
  wcn36xx: Implement downstream compliant beacon filtering

 drivers/net/wireless/ath/wcn36xx/hal.h  | 30 +++++++--
 drivers/net/wireless/ath/wcn36xx/main.c |  2 +
 drivers/net/wireless/ath/wcn36xx/smd.c  | 87 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  3 +
 4 files changed, 116 insertions(+), 6 deletions(-)

-- 
2.33.0

[PATCH v3 1/3] wcn36xx: Fix beacon filter structure definitions

[Bryan O'Donoghue]

The beacon filter structures need to be packed. Right now its fine because
we don't yet use these structures so just pack them without marking it for
backporting.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 9bea2b01f9aab..9bce71592f743 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3469,7 +3469,7 @@ struct beacon_filter_ie {
 	u8 value;
 	u8 bitmask;
 	u8 ref;
-};
+} __packed;
 
 struct wcn36xx_hal_add_bcn_filter_req_msg {
 	struct wcn36xx_hal_msg_header header;
@@ -3480,14 +3480,14 @@ struct wcn36xx_hal_add_bcn_filter_req_msg {
 	u16 ie_num;
 	u8 bss_index;
 	u8 reserved;
-};
+} __packed;
 
 struct wcn36xx_hal_rem_bcn_filter_req {
 	struct wcn36xx_hal_msg_header header;
 
 	u8 ie_Count;
 	u8 rem_ie_id[1];
-};
+} __packed;
 
 #define WCN36XX_HAL_IPV4_ARP_REPLY_OFFLOAD                  0
 #define WCN36XX_HAL_IPV6_NEIGHBOR_DISCOVERY_OFFLOAD         1
-- 
2.33.0

Re: [PATCH v3 1/3] wcn36xx: Fix beacon filter structure definitions

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> wrote:

> The beacon filter structures need to be packed. Right now its fine because
> we don't yet use these structures so just pack them without marking it for
> backporting.
> 
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
> Reviewed-by: Loic Poulain <loic.poulain@linaro.org>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

3 patches applied to ath-next branch of ath.git, thanks.

7effbf7af91e wcn36xx: Fix beacon filter structure definitions
bc4e7f2432bb wcn36xx: Fix physical location of beacon filter comment
bebd87eea29a wcn36xx: Implement beacon filtering

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20211214134630.2214840-2-bryan.odonoghue@linaro.org/

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

[PATCH v3 2/3] wcn36xx: Fix physical location of beacon filter comment

[Bryan O'Donoghue]

The comment in the header with respect to beacon filtering makes a
reference to "the structure above" and "the structure below" which would be
informative if the comment appeared in the right place but, it does not.

Fix the comment location so that it a least makes sense w/r/t the physical
location statements.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index 9bce71592f743..e9fec110721b5 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3459,9 +3459,6 @@ struct wcn36xx_hal_missed_beacon_ind_msg {
 
 /* Beacon Filtering data structures */
 
-/* The above structure would be followed by multiple of below mentioned
- * structure
- */
 struct beacon_filter_ie {
 	u8 element_id;
 	u8 check_ie_presence;
@@ -3471,6 +3468,10 @@ struct beacon_filter_ie {
 	u8 ref;
 } __packed;
 
+/* The above structure would be followed by multiple of below mentioned
+ * structure
+ */
+
 struct wcn36xx_hal_add_bcn_filter_req_msg {
 	struct wcn36xx_hal_msg_header header;
 
-- 
2.33.0

Re: [PATCH v3 2/3] wcn36xx: Fix physical location of beacon filter comment

[Loic Poulain]

On Tue, 14 Dec 2021 at 14:44, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> The comment in the header with respect to beacon filtering makes a
> reference to "the structure above" and "the structure below" which would be
> informative if the comment appeared in the right place but, it does not.
>
> Fix the comment location so that it a least makes sense w/r/t the physical
> location statements.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

[PATCH v3 3/3] wcn36xx: Implement downstream compliant beacon filtering

[Bryan O'Donoghue]

Downstream facilitates the direct programming of beacon filter tables via
SMD commands.

The purpose of beacon filters is quote:

/* When beacon filtering is enabled, firmware will
 * analyze the selected beacons received during BMPS,
 * and monitor any changes in the IEs as listed below.
 * The format of the table is:
 *    - EID
 *    - Check for IE presence
 *    - Byte offset
 *    - Byte value
 *    - Bit Mask
 *    - Byte reference
 */

The default downstream firmware filter table looks something like this:
tBeaconFilterIe gaBcnFilterTable[12] =
{
  { WLAN_EID_DS_PARAMS, 0u, { 0u, 0u, 0u, 0u } },
  { WLAN_EID_ERP_INFO, 0u, { 0u, 0u, 248u, 0u } },
  { WLAN_EID_EDCA_PARAM_SET, 0u, { 0u, 0u, 240u, 0u } },
  { WLAN_EID_QOS_CAPA, 0u, { 0u, 0u, 240u, 0u } },
  { WLAN_EID_CHANNEL_SWITCH, 1u, { 0u, 0u, 0u, 0u } },
  { WLAN_EID_QUIET, 1u, { 0u, 0u, 0u, 0u } },
  { WLAN_EID_HT_OPERATION, 0u, { 0u, 0u, 0u, 0u } },
  { WLAN_EID_HT_OPERATION, 0u, { 1u, 0u, 248u, 0u } },
  { WLAN_EID_HT_OPERATION, 0u, { 2u, 0u, 235u, 0u } },
  { WLAN_EID_HT_OPERATION, 0u, { 5u, 0u, 253u, 0u } },
  { WLAN_EID_PWR_CONSTRAINT, 0u, { 0u, 0u, 0u, 0u } },
  { WLAN_EID_OPMODE_NOTIF, 0u, { 0u, 0u, 0u, 0u } }
};

Add in an equivalent filter set as present in the downstream Linux driver.
For now omit the beacon filter "rem" command as downstream does not have an
explicit call to that SMD command. The filter mask should only count when
we are inside BMPS anyway.

Replicating the downstream ability to program the filter table gives us
scope to add and remove elements in future. For now though this patch
makes the rote-copy of the downstream Linux beacon filter table, which we
can tweak as desired from now on.

Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h  | 17 +++++
 drivers/net/wireless/ath/wcn36xx/main.c |  2 +
 drivers/net/wireless/ath/wcn36xx/smd.c  | 87 +++++++++++++++++++++++++
 drivers/net/wireless/ath/wcn36xx/smd.h  |  3 +
 4 files changed, 109 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index e9fec110721b5..a6968f9785d6f 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -3468,6 +3468,23 @@ struct beacon_filter_ie {
 	u8 ref;
 } __packed;
 
+/* Downstream values for the bitmask field */
+#define WCN36XX_FILTER_CAPABILITY_MASK		0x73cf
+#define WCN36XX_FILTER_IE_DS_CHANNEL_MASK	0x00
+#define WCN36XX_FILTER_IE_ERP_FILTER_MASK	0xF8
+#define WCN36XX_FILTER_IE_EDCA_FILTER_MASK	0xF0
+#define WCN36XX_FILTER_IE_QOS_FILTER_MASK	0xF0
+#define WCN36XX_FILTER_IE_CHANNEL_SWITCH_MASK	0x00
+#define WCN36XX_FILTER_IE_HT_BYTE0_FILTER_MASK	0x00
+#define WCN36XX_FILTER_IE_HT_BYTE1_FILTER_MASK	0xF8
+#define WCN36XX_FILTER_IE_HT_BYTE2_FILTER_MASK	0xEB
+#define WCN36XX_FILTER_IE_HT_BYTE5_FILTER_MASK	0xFD
+#define WCN36XX_FILTER_IE_PWR_CONSTRAINT_MASK	0x00
+#define WCN36XX_FILTER_IE_OPMODE_NOTIF_MASK	0x00
+#define WCN36XX_FILTER_IE_VHTOP_CHWIDTH_MASK	0xFC
+#define WCN36XX_FILTER_IE_RSN_MASK		0x00
+#define WCN36XX_FILTER_IE_VENDOR_MASK		0x00
+
 /* The above structure would be followed by multiple of below mentioned
  * structure
  */
diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index f59eb1119da88..ec355807f5817 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -934,6 +934,8 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw *hw,
 			 * place where AID is available.
 			 */
 			wcn36xx_smd_config_sta(wcn, vif, sta);
+			if (vif->type == NL80211_IFTYPE_STATION)
+				wcn36xx_smd_add_beacon_filter(wcn, vif);
 			wcn36xx_enable_keep_alive_null_packet(wcn, vif);
 		} else {
 			wcn36xx_dbg(WCN36XX_DBG_MAC,
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index b05d10f9f6005..6791853088d61 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -3193,6 +3193,92 @@ int wcn36xx_smd_host_resume(struct wcn36xx *wcn)
 	return ret;
 }
 
+#define BEACON_FILTER(eid, presence, offs, val, mask, ref_val) \
+	{					\
+		.element_id = eid,		\
+		.check_ie_presence = presence,	\
+		.offset = offs,			\
+		.value = val,			\
+		.bitmask = mask,		\
+		.ref = ref_val,			\
+	}
+
+/* CORE/MAC/src/pe/lim/limSendMessages.c::beaconFilterTable[] */
+static struct beacon_filter_ie bcn_filter_ies[] = {
+	BEACON_FILTER(WLAN_EID_DS_PARAMS, 0, 0, 0,
+		      WCN36XX_FILTER_IE_DS_CHANNEL_MASK, 0),
+	BEACON_FILTER(WLAN_EID_ERP_INFO, 0, 0, 0,
+		      WCN36XX_FILTER_IE_ERP_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_EDCA_PARAM_SET, 0, 0, 0,
+		      WCN36XX_FILTER_IE_EDCA_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_QOS_CAPA, 0, 0, 0,
+		      WCN36XX_FILTER_IE_QOS_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_CHANNEL_SWITCH, 1, 0, 0,
+		      WCN36XX_FILTER_IE_CHANNEL_SWITCH_MASK, 0),
+	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 0, 0,
+		      WCN36XX_FILTER_IE_HT_BYTE0_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 2, 0,
+		      WCN36XX_FILTER_IE_HT_BYTE2_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 5, 0,
+		      WCN36XX_FILTER_IE_HT_BYTE5_FILTER_MASK, 0),
+	BEACON_FILTER(WLAN_EID_PWR_CONSTRAINT, 0, 0, 0,
+		      WCN36XX_FILTER_IE_PWR_CONSTRAINT_MASK, 0),
+	BEACON_FILTER(WLAN_EID_OPMODE_NOTIF, 0, 0, 0,
+		      WCN36XX_FILTER_IE_OPMODE_NOTIF_MASK, 0),
+	BEACON_FILTER(WLAN_EID_VHT_OPERATION, 0, 0, 0,
+		      WCN36XX_FILTER_IE_VHTOP_CHWIDTH_MASK, 0),
+	BEACON_FILTER(WLAN_EID_RSN, 1, 0, 0,
+		      WCN36XX_FILTER_IE_RSN_MASK, 0),
+	BEACON_FILTER(WLAN_EID_VENDOR_SPECIFIC, 1, 0, 0,
+		      WCN36XX_FILTER_IE_VENDOR_MASK, 0),
+};
+
+int wcn36xx_smd_add_beacon_filter(struct wcn36xx *wcn,
+				  struct ieee80211_vif *vif)
+{
+	struct wcn36xx_hal_add_bcn_filter_req_msg msg_body, *body;
+	struct wcn36xx_vif *vif_priv = wcn36xx_vif_to_priv(vif);
+	u8 *payload;
+	size_t payload_size;
+	int ret;
+
+	if (!get_feat_caps(wcn->fw_feat_caps, BCN_FILTER))
+		return -EOPNOTSUPP;
+
+	mutex_lock(&wcn->hal_mutex);
+	INIT_HAL_MSG(msg_body, WCN36XX_HAL_ADD_BCN_FILTER_REQ);
+
+	PREPARE_HAL_BUF(wcn->hal_buf, msg_body);
+
+	body = (struct wcn36xx_hal_add_bcn_filter_req_msg *)wcn->hal_buf;
+	body->capability_info = vif->bss_conf.assoc_capability;
+	body->capability_mask = WCN36XX_FILTER_CAPABILITY_MASK;
+	body->beacon_interval = vif->bss_conf.beacon_int;
+	body->ie_num = ARRAY_SIZE(bcn_filter_ies);
+	body->bss_index = vif_priv->bss_index;
+
+	payload = ((u8 *)body) + body->header.len;
+	payload_size = sizeof(bcn_filter_ies);
+	memcpy(payload, &bcn_filter_ies, payload_size);
+
+	body->header.len += payload_size;
+
+	ret = wcn36xx_smd_send_and_wait(wcn, body->header.len);
+	if (ret) {
+		wcn36xx_err("Sending add bcn_filter failed\n");
+		goto out;
+	}
+
+	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	if (ret) {
+		wcn36xx_err("add bcn filter response failed err=%d\n", ret);
+		goto out;
+	}
+out:
+	mutex_unlock(&wcn->hal_mutex);
+	return ret;
+}
+
 int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 			    void *buf, int len, void *priv, u32 addr)
 {
@@ -3248,6 +3334,7 @@ int wcn36xx_smd_rsp_process(struct rpmsg_device *rpdev,
 	case WCN36XX_HAL_ENTER_IMPS_RSP:
 	case WCN36XX_HAL_EXIT_IMPS_RSP:
 	case WCN36XX_HAL_UPDATE_CHANNEL_LIST_RSP:
+	case WCN36XX_HAL_ADD_BCN_FILTER_RSP:
 		memcpy(wcn->hal_buf, buf, len);
 		wcn->hal_rsp_len = len;
 		complete(&wcn->hal_rsp_compl);
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index cfde15341a88d..957cfa87fbdea 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -167,4 +167,7 @@ int wcn36xx_smd_host_resume(struct wcn36xx *wcn);
 int wcn36xx_smd_enter_imps(struct wcn36xx *wcn);
 int wcn36xx_smd_exit_imps(struct wcn36xx *wcn);
 
+int wcn36xx_smd_add_beacon_filter(struct wcn36xx *wcn,
+				  struct ieee80211_vif *vif);
+
 #endif	/* _SMD_H_ */
-- 
2.33.0

Re: [PATCH v3 3/3] wcn36xx: Implement downstream compliant beacon filtering

[Loic Poulain]

On Tue, 14 Dec 2021 at 14:44, Bryan O'Donoghue
<bryan.odonoghue@linaro.org> wrote:
>
> Downstream facilitates the direct programming of beacon filter tables via
> SMD commands.
>
> The purpose of beacon filters is quote:
>
> /* When beacon filtering is enabled, firmware will
>  * analyze the selected beacons received during BMPS,
>  * and monitor any changes in the IEs as listed below.
>  * The format of the table is:
>  *    - EID
>  *    - Check for IE presence
>  *    - Byte offset
>  *    - Byte value
>  *    - Bit Mask
>  *    - Byte reference
>  */
>
> The default downstream firmware filter table looks something like this:
> tBeaconFilterIe gaBcnFilterTable[12] =
> {
>   { WLAN_EID_DS_PARAMS, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_ERP_INFO, 0u, { 0u, 0u, 248u, 0u } },
>   { WLAN_EID_EDCA_PARAM_SET, 0u, { 0u, 0u, 240u, 0u } },
>   { WLAN_EID_QOS_CAPA, 0u, { 0u, 0u, 240u, 0u } },
>   { WLAN_EID_CHANNEL_SWITCH, 1u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_QUIET, 1u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 1u, 0u, 248u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 2u, 0u, 235u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 5u, 0u, 253u, 0u } },
>   { WLAN_EID_PWR_CONSTRAINT, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_OPMODE_NOTIF, 0u, { 0u, 0u, 0u, 0u } }
> };
>
> Add in an equivalent filter set as present in the downstream Linux driver.
> For now omit the beacon filter "rem" command as downstream does not have an
> explicit call to that SMD command. The filter mask should only count when
> we are inside BMPS anyway.
>
> Replicating the downstream ability to program the filter table gives us
> scope to add and remove elements in future. For now though this patch
> makes the rote-copy of the downstream Linux beacon filter table, which we
> can tweak as desired from now on.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

Reviewed-by: Loic Poulain <loic.poulain@linaro.org>

Re: [PATCH v3 3/3] wcn36xx: Implement downstream compliant beacon filtering

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Downstream facilitates the direct programming of beacon filter tables via
> SMD commands.

I don't think the term downstream is clear for everyone, I guess with
that you mean the prima driver? I would prefer to use "prima driver" in
commit logs, I can change it and no need to resend because of this.

> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -3468,6 +3468,23 @@ struct beacon_filter_ie {
>  	u8 ref;
>  } __packed;
>  
> +/* Downstream values for the bitmask field */
> +#define WCN36XX_FILTER_CAPABILITY_MASK		0x73cf
> +#define WCN36XX_FILTER_IE_DS_CHANNEL_MASK	0x00
> +#define WCN36XX_FILTER_IE_ERP_FILTER_MASK	0xF8
> +#define WCN36XX_FILTER_IE_EDCA_FILTER_MASK	0xF0
> +#define WCN36XX_FILTER_IE_QOS_FILTER_MASK	0xF0
> +#define WCN36XX_FILTER_IE_CHANNEL_SWITCH_MASK	0x00
> +#define WCN36XX_FILTER_IE_HT_BYTE0_FILTER_MASK	0x00
> +#define WCN36XX_FILTER_IE_HT_BYTE1_FILTER_MASK	0xF8
> +#define WCN36XX_FILTER_IE_HT_BYTE2_FILTER_MASK	0xEB
> +#define WCN36XX_FILTER_IE_HT_BYTE5_FILTER_MASK	0xFD
> +#define WCN36XX_FILTER_IE_PWR_CONSTRAINT_MASK	0x00
> +#define WCN36XX_FILTER_IE_OPMODE_NOTIF_MASK	0x00
> +#define WCN36XX_FILTER_IE_VHTOP_CHWIDTH_MASK	0xFC
> +#define WCN36XX_FILTER_IE_RSN_MASK		0x00
> +#define WCN36XX_FILTER_IE_VENDOR_MASK		0x00

TBH I don't see much value in the comment, there's only one set of
values, so I would like to remove it.

> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -3193,6 +3193,92 @@ int wcn36xx_smd_host_resume(struct wcn36xx *wcn)
>  	return ret;
>  }
>  
> +#define BEACON_FILTER(eid, presence, offs, val, mask, ref_val) \
> +	{					\
> +		.element_id = eid,		\
> +		.check_ie_presence = presence,	\
> +		.offset = offs,			\
> +		.value = val,			\
> +		.bitmask = mask,		\
> +		.ref = ref_val,			\
> +	}
> +
> +/* CORE/MAC/src/pe/lim/limSendMessages.c::beaconFilterTable[] */
> +static struct beacon_filter_ie bcn_filter_ies[] = {

I guess the comment refers to the prima driver, we shouldn't have that
in upstream. I can remove it.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

Re: [PATCH v3 3/3] wcn36xx: Implement downstream compliant beacon filtering

[Bryan O'Donoghue]

On 16/12/2021 10:31, Kalle Valo wrote:
> I don't think the term downstream is clear for everyone, I guess with
> that you mean the prima driver? I would prefer to use "prima driver" in
> commit logs, I can change it and no need to resend because of this.

OK np thanks for zapping the comments directly.

Noted for future reference :g/downstream/s//prima\ driver/g

---
bod

Re: [PATCH v3 3/3] wcn36xx: Implement downstream compliant beacon filtering

[Kalle Valo]

Bryan O'Donoghue <bryan.odonoghue@linaro.org> writes:

> Downstream facilitates the direct programming of beacon filter tables via
> SMD commands.
>
> The purpose of beacon filters is quote:
>
> /* When beacon filtering is enabled, firmware will
>  * analyze the selected beacons received during BMPS,
>  * and monitor any changes in the IEs as listed below.
>  * The format of the table is:
>  *    - EID
>  *    - Check for IE presence
>  *    - Byte offset
>  *    - Byte value
>  *    - Bit Mask
>  *    - Byte reference
>  */
>
> The default downstream firmware filter table looks something like this:
> tBeaconFilterIe gaBcnFilterTable[12] =
> {
>   { WLAN_EID_DS_PARAMS, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_ERP_INFO, 0u, { 0u, 0u, 248u, 0u } },
>   { WLAN_EID_EDCA_PARAM_SET, 0u, { 0u, 0u, 240u, 0u } },
>   { WLAN_EID_QOS_CAPA, 0u, { 0u, 0u, 240u, 0u } },
>   { WLAN_EID_CHANNEL_SWITCH, 1u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_QUIET, 1u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 1u, 0u, 248u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 2u, 0u, 235u, 0u } },
>   { WLAN_EID_HT_OPERATION, 0u, { 5u, 0u, 253u, 0u } },
>   { WLAN_EID_PWR_CONSTRAINT, 0u, { 0u, 0u, 0u, 0u } },
>   { WLAN_EID_OPMODE_NOTIF, 0u, { 0u, 0u, 0u, 0u } }
> };
>
> Add in an equivalent filter set as present in the downstream Linux driver.
> For now omit the beacon filter "rem" command as downstream does not have an
> explicit call to that SMD command. The filter mask should only count when
> we are inside BMPS anyway.
>
> Replicating the downstream ability to program the filter table gives us
> scope to add and remove elements in future. For now though this patch
> makes the rote-copy of the downstream Linux beacon filter table, which we
> can tweak as desired from now on.
>
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

[...]

> +static struct beacon_filter_ie bcn_filter_ies[] = {
> +	BEACON_FILTER(WLAN_EID_DS_PARAMS, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_DS_CHANNEL_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_ERP_INFO, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_ERP_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_EDCA_PARAM_SET, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_EDCA_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_QOS_CAPA, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_QOS_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_CHANNEL_SWITCH, 1, 0, 0,
> +		      WCN36XX_FILTER_IE_CHANNEL_SWITCH_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_HT_BYTE0_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 2, 0,
> +		      WCN36XX_FILTER_IE_HT_BYTE2_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_HT_OPERATION, 0, 5, 0,
> +		      WCN36XX_FILTER_IE_HT_BYTE5_FILTER_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_PWR_CONSTRAINT, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_PWR_CONSTRAINT_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_OPMODE_NOTIF, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_OPMODE_NOTIF_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_VHT_OPERATION, 0, 0, 0,
> +		      WCN36XX_FILTER_IE_VHTOP_CHWIDTH_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_RSN, 1, 0, 0,
> +		      WCN36XX_FILTER_IE_RSN_MASK, 0),
> +	BEACON_FILTER(WLAN_EID_VENDOR_SPECIFIC, 1, 0, 0,
> +		      WCN36XX_FILTER_IE_VENDOR_MASK, 0),
> +};

All static variables should be const so I changed this to const as well.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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