2023-10-20 09:58:50

by Arend van Spriel

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

On 10/19/2023 3:42 AM, Daniel Berlin wrote:
> From: Hector Martin <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Hector Martin <[email protected]>
> ---
> .../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.
> *


Attachments:
smime.p7s (4.12 kB)
S/MIME Cryptographic Signature