Re: [PATCH 3/5] wifi: brcmfmac: Add support for SCAN_V3

[Arend van Spriel <arend.vanspriel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 7057 bytes --]

On 10/19/2023 3:42 AM, Daniel Berlin wrote:
> From: Hector Martin <marcan@marcan.st>
> 
> This is essentially identical to SCAN_V2 with an extra field where we
> had a padding byte, so don't bother duplicating the entire structure.
> Just add the field and the logic to set the version properly.

Reviewed-by: Arend van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 20 +++++++++++++------
>   .../broadcom/brcm80211/brcmfmac/feature.c     | 16 ++++++++++++++-
>   .../broadcom/brcm80211/brcmfmac/feature.h     |  1 +
>   .../broadcom/brcm80211/brcmfmac/fwil_types.h  | 15 +++++++++++++-
>   4 files changed, 44 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 7143ffe659f6..4cf728368892 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -1125,6 +1125,7 @@ static void brcmf_scan_params_v2_to_v1(struct brcmf_scan_params_v2_le *params_v2
>   }
>   
>   static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
> +			     struct brcmf_if *ifp,
>   			     struct brcmf_scan_params_v2_le *params_le,
>   			     struct cfg80211_scan_request *request)
>   {
> @@ -1141,8 +1142,13 @@ static void brcmf_escan_prep(struct brcmf_cfg80211_info *cfg,
>   
>   	length = BRCMF_SCAN_PARAMS_V2_FIXED_SIZE;
>   
> -	params_le->version = cpu_to_le16(BRCMF_SCAN_PARAMS_VERSION_V2);
> +	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V3))
> +		params_le->version = cpu_to_le16(BRCMF_SCAN_PARAMS_VERSION_V3);
> +	else
> +		params_le->version = cpu_to_le16(BRCMF_SCAN_PARAMS_VERSION_V2);
> +
>   	params_le->bss_type = DOT11_BSSTYPE_ANY;
> +	params_le->ssid_type = 0;
>   	params_le->scan_type = cpu_to_le32(BRCMF_SCANTYPE_ACTIVE);
>   	params_le->channel_num = 0;
>   	params_le->nprobes = cpu_to_le32(-1);
> @@ -1237,7 +1243,7 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
>   		/* Do a scan abort to stop the driver's scan engine */
>   		brcmf_dbg(SCAN, "ABORT scan in firmware\n");
>   
> -		brcmf_escan_prep(cfg, &params_v2_le, NULL);
> +		brcmf_escan_prep(cfg, ifp, &params_v2_le, NULL);
>   
>   		/* E-Scan (or anyother type) can be aborted by SCAN */
>   		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V2)) {
> @@ -1497,11 +1503,13 @@ brcmf_run_escan(struct brcmf_cfg80211_info *cfg, struct brcmf_if *ifp,
>   		goto exit;
>   	}
>   	BUG_ON(params_size + sizeof("escan") >= BRCMF_DCMD_MEDLEN);
> -	brcmf_escan_prep(cfg, &params->params_v2_le, request);
> +	brcmf_escan_prep(cfg, ifp, &params->params_v2_le, request);
>   
> -	params->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION_V2);
> -
> -	if (!brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V2)) {
> +	if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V3)) {
> +		params->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION_V3);
> +	} else if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_SCAN_V2)) {
> +		params->version = cpu_to_le32(BRCMF_ESCAN_REQ_VERSION_V2);
> +	} else {
>   		struct brcmf_escan_params_le *params_v1;
>   
>   		params_size -= BRCMF_SCAN_PARAMS_V2_FIXED_SIZE;
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> index 6d10c9efbe93..c545d3b250e1 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.c
> @@ -287,6 +287,7 @@ static int brcmf_feat_fwcap_debugfs_read(struct seq_file *seq, void *data)
>   void brcmf_feat_attach(struct brcmf_pub *drvr)
>   {
>   	struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
> +	struct brcmf_wl_scan_version_le scan_ver;
>   	struct brcmf_pno_macaddr_le pfn_mac;
>   	struct brcmf_gscan_config gscan_cfg;
>   	u32 wowl_cap;
> @@ -337,7 +338,20 @@ void brcmf_feat_attach(struct brcmf_pub *drvr)
>   		ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_SCAN_RANDOM_MAC);
>   
>   	brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_FWSUP, "sup_wpa");
> -	brcmf_feat_iovar_int_get(ifp, BRCMF_FEAT_SCAN_V2, "scan_ver");
> +
> +	err = brcmf_fil_iovar_data_get(ifp, "scan_ver", &scan_ver, sizeof(scan_ver));
> +	if (!err) {
> +		int ver = le16_to_cpu(scan_ver.scan_ver_major);
> +
> +		if (ver == 2) {
> +			ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_SCAN_V2);
> +		} else if (ver == 3) {
> +			/* We consider SCAN_V3 a subtype of SCAN_V2 since the
> +			 * structure is essentially the same.
> +			 */
> +			ifp->drvr->feat_flags |= BIT(BRCMF_FEAT_SCAN_V2) | BIT(BRCMF_FEAT_SCAN_V3);
> +		}
> +	}
>   
>   	if (drvr->settings->feature_disable) {
>   		brcmf_dbg(INFO, "Features: 0x%02x, disable: 0x%02x\n",
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
> index 7f4f0b3e4a7b..3317e80c398a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/feature.h
> @@ -56,6 +56,7 @@
>   	BRCMF_FEAT_DEF(FWAUTH) \
>   	BRCMF_FEAT_DEF(DUMP_OBSS) \
>   	BRCMF_FEAT_DEF(SCAN_V2) \
> +	BRCMF_FEAT_DEF(SCAN_V3) \
>   	BRCMF_FEAT_DEF(PMKID_V2) \
>   	BRCMF_FEAT_DEF(PMKID_V3)

Having API versioning marked as feature does not feel right. Will 
probably come up with different mechanism for that.

> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> index 9d248ba1c0b2..1077e6f1d61a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> @@ -52,6 +52,7 @@
>   
>   /* version of brcmf_scan_params structure */
>   #define BRCMF_SCAN_PARAMS_VERSION_V2	2
> +#define BRCMF_SCAN_PARAMS_VERSION_V3	3
>   
>   /* masks for channel and ssid count */
>   #define BRCMF_SCAN_PARAMS_COUNT_MASK	0x0000ffff
> @@ -72,6 +73,7 @@
>   #define DOT11_BSSTYPE_ANY		2
>   #define BRCMF_ESCAN_REQ_VERSION		1
>   #define BRCMF_ESCAN_REQ_VERSION_V2	2
> +#define BRCMF_ESCAN_REQ_VERSION_V3	3
>   
>   #define BRCMF_MAXRATES_IN_SET		16	/* max # of rates in rateset */
>   
> @@ -414,7 +416,7 @@ struct brcmf_scan_params_v2_le {
>   	s8 bss_type;		/* default: any,
>   				 * DOT11_BSSTYPE_ANY/INFRASTRUCTURE/INDEPENDENT
>   				 */
> -	u8 pad;
> +	u8 ssid_type;		/* v3 only */
>   	__le32 scan_type;	/* flags, 0 use default */
>   	__le32 nprobes;		/* -1 use default, number of probes per channel */
>   	__le32 active_time;	/* -1 use default, dwell time per channel for
> @@ -833,6 +835,17 @@ struct brcmf_wlc_version_le {
>   	__le16 wlc_ver_minor;
>   };
>   
> +/**
> + * struct brcmf_wl_scan_version_le - scan interface version
> + */
> +struct brcmf_wl_scan_version_le {
> +	__le16 version;
> +	__le16 length;
> +	__le16 scan_ver_major;
> +};
> +
> +#define BRCMF_WL_SCAN_VERSION_VERSION 1
> +
>   /**
>    * struct brcmf_assoclist_le - request assoc list.
>    *


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]