[PATCH -next 0/2] brcmfmac: AP STA concurrency patches from Cypress/Infineon

[PATCH -next 0/2] brcmfmac: AP STA concurrency patches from Cypress/Infineon

[Alvin Šipraga]

From: Alvin Šipraga <alsi@bang-olufsen.dk>

We are using these two patches from Infineon (formerly Cypress) to
enable AP+STA mode on our CYW89359-based platform. They come from the
FMAC driver release package distributed by Infineon.

The key thing here is that apsta needs to be set to 1 in order for AP
mode to work concurrently with STA mode. I cannot speak for other
chipsets so a review from the Broadcom side would be welcome here.

For the ARP/ND offload being disabled in AP mode, I am of the view that
this is correct, but while Arend has given his Reviewed-by on it
previously, it was part of a different series [1], so I am sending
without in order to jog some memories.

[1] https://lore.kernel.org/linux-wireless/20201020022812.37064-3-wright.feng@cypress.com/#t

Soontak Lee (1):
  brcmfmac: Support multiple AP interfaces and fix STA disconnection
    issue

Ting-Ying Li (1):
  brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists

 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 65 +++++++++++++++----
 .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  2 +
 .../broadcom/brcm80211/brcmfmac/common.c      |  5 ++
 .../broadcom/brcm80211/brcmfmac/core.c        |  5 ++
 4 files changed, 66 insertions(+), 11 deletions(-)

-- 
2.37.0

[PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Alvin Šipraga]

From: Soontak Lee <soontak.lee@cypress.com>

Support multiple AP interfaces for STA + AP + AP usecase.
And fix STA disconnection when deactivating AP interface.

Signed-off-by: Soontak Lee <soontak.lee@cypress.com>
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 48 +++++++++++++++----
 .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  1 +
 .../broadcom/brcm80211/brcmfmac/common.c      |  5 ++
 3 files changed, 44 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 3ae6779fe153..856fd5516ddf 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -4747,6 +4747,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 		  settings->inactivity_timeout);
 	dev_role = ifp->vif->wdev.iftype;
 	mbss = ifp->vif->mbss;
+	brcmf_dbg(TRACE, "mbss %s\n", mbss ? "enabled" : "disabled");
 
 	/* store current 11d setting */
 	if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
@@ -4961,6 +4962,9 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
 	if ((err) && (!mbss)) {
 		brcmf_set_mpc(ifp, 1);
 		brcmf_configure_arp_nd_offload(ifp, true);
+	} else {
+		cfg->num_softap++;
+		brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
 	}
 	return err;
 }
@@ -4975,6 +4979,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
 	s32 err;
 	struct brcmf_fil_bss_enable_le bss_enable;
 	struct brcmf_join_params join_params;
+	s32 apsta = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
@@ -4983,6 +4988,27 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
 		/* first to make sure they get processed by fw. */
 		msleep(400);
 
+		cfg->num_softap--;
+
+		/* Clear bss configuration and SSID */
+		bss_enable.bsscfgidx = cpu_to_le32(ifp->bsscfgidx);
+		bss_enable.enable = cpu_to_le32(0);
+		err = brcmf_fil_iovar_data_set(ifp, "bss", &bss_enable,
+					       sizeof(bss_enable));
+		if (err < 0)
+			brcmf_err("bss_enable config failed %d\n", err);
+
+		memset(&join_params, 0, sizeof(join_params));
+		err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
+					     &join_params, sizeof(join_params));
+		if (err < 0)
+			bphy_err(drvr, "SET SSID error (%d)\n", err);
+
+		if (cfg->num_softap) {
+			brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
+			return 0;
+		}
+
 		if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
 			if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
 				brcmf_set_pmk(ifp, NULL, 0);
@@ -5000,17 +5026,18 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
 		if (ifp->bsscfgidx == 0)
 			brcmf_fil_iovar_int_set(ifp, "closednet", 0);
 
-		memset(&join_params, 0, sizeof(join_params));
-		err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
-					     &join_params, sizeof(join_params));
-		if (err < 0)
-			bphy_err(drvr, "SET SSID error (%d)\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
-		if (err < 0)
-			bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
-		err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
+		err = brcmf_fil_iovar_int_get(ifp, "apsta", &apsta);
 		if (err < 0)
-			bphy_err(drvr, "setting AP mode failed %d\n", err);
+			brcmf_err("wl apsta failed (%d)\n", err);
+
+		if (!apsta) {
+			err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
+			if (err < 0)
+				bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
+			err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
+			if (err < 0)
+				bphy_err(drvr, "Set AP mode error %d\n", err);
+		}
 		if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
 			brcmf_fil_iovar_int_set(ifp, "mbss", 0);
 		brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
@@ -7641,6 +7668,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
 
 	cfg->wiphy = wiphy;
 	cfg->pub = drvr;
+	cfg->num_softap = 0;
 	init_vif_event(&cfg->vif_event);
 	INIT_LIST_HEAD(&cfg->vif_list);
 
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
index e90a30808c22..e4ebc2fa6ebb 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -371,6 +371,7 @@ struct brcmf_cfg80211_info {
 	struct brcmf_cfg80211_wowl wowl;
 	struct brcmf_pno_info *pno;
 	u8 ac_priority[MAX_8021D_PRIO];
+	u8 num_softap;
 };
 
 /**
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index fe01da9e620d..83e023a22f9b 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 		brcmf_dbg(INFO, "CLM version = %s\n", clmver);
 	}
 
+	/* set apsta */
+	err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
+	if (err)
+		brcmf_info("failed setting apsta, %d\n", err);
+
 	/* set mpc */
 	err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
 	if (err) {
-- 
2.37.0

[PATCH -next 2/2] brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists

[Alvin Šipraga]

From: Ting-Ying Li <tingying.li@cypress.com>

Add a condition to determine whether arp/nd offload enabling
request is allowed. If there is any interface acts as ap
mode and is operating, then reject the request of arp oflload
enabling from cfg80211.

Signed-off-by: Ting-Ying Li <tingying.li@cypress.com>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c      | 17 ++++++++++++++++-
 .../broadcom/brcm80211/brcmfmac/cfg80211.h      |  1 +
 .../wireless/broadcom/brcm80211/brcmfmac/core.c |  5 +++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 856fd5516ddf..29aa7613bcf3 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -770,6 +770,21 @@ void brcmf_set_mpc(struct brcmf_if *ifp, int mpc)
 	}
 }
 
+bool brcmf_is_apmode_operating(struct wiphy *wiphy)
+{
+	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
+	struct brcmf_cfg80211_vif *vif;
+	bool ret = false;
+
+	list_for_each_entry(vif, &cfg->vif_list, list) {
+		if (brcmf_is_apmode(vif) &&
+		    test_bit(BRCMF_VIF_STATUS_AP_CREATED, &vif->sme_state))
+			ret = true;
+	}
+
+	return ret;
+}
+
 s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 				struct brcmf_if *ifp, bool aborted,
 				bool fw_abort)
@@ -5057,8 +5072,8 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
 			bphy_err(drvr, "bss_enable config failed %d\n", err);
 	}
 	brcmf_set_mpc(ifp, 1);
-	brcmf_configure_arp_nd_offload(ifp, true);
 	clear_bit(BRCMF_VIF_STATUS_AP_CREATED, &ifp->vif->sme_state);
+	brcmf_configure_arp_nd_offload(ifp, true);
 	brcmf_net_setcarrier(ifp, false);
 
 	return err;
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
index e4ebc2fa6ebb..3133b6509870 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
@@ -468,5 +468,6 @@ s32 brcmf_notify_escan_complete(struct brcmf_cfg80211_info *cfg,
 void brcmf_set_mpc(struct brcmf_if *ndev, int mpc);
 void brcmf_abort_scanning(struct brcmf_cfg80211_info *cfg);
 void brcmf_cfg80211_free_netdev(struct net_device *ndev);
+bool brcmf_is_apmode_operating(struct wiphy *wiphy);
 
 #endif /* BRCMFMAC_CFG80211_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 87aef211b35f..ab5ed4a9e57a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -97,6 +97,11 @@ void brcmf_configure_arp_nd_offload(struct brcmf_if *ifp, bool enable)
 	s32 err;
 	u32 mode;
 
+	if (enable && brcmf_is_apmode_operating(ifp->drvr->wiphy)) {
+		brcmf_dbg(TRACE, "Skip ARP/ND offload enable when soft AP is running\n");
+		return;
+	}
+
 	if (enable)
 		mode = BRCMF_ARP_OL_AGENT | BRCMF_ARP_OL_PEER_AUTO_REPLY;
 	else
-- 
2.37.0

Re: [PATCH -next 0/2] brcmfmac: AP STA concurrency patches from Cypress/Infineon

[Kalle Valo]

Alvin Šipraga <alvin@pqrs.dk> writes:

> From: Alvin Šipraga <alsi@bang-olufsen.dk>
>
> We are using these two patches from Infineon (formerly Cypress) to
> enable AP+STA mode on our CYW89359-based platform. They come from the
> FMAC driver release package distributed by Infineon.
>
> The key thing here is that apsta needs to be set to 1 in order for AP
> mode to work concurrently with STA mode. I cannot speak for other
> chipsets so a review from the Broadcom side would be welcome here.
>
> For the ARP/ND offload being disabled in AP mode, I am of the view that
> this is correct, but while Arend has given his Reviewed-by on it
> previously, it was part of a different series [1], so I am sending
> without in order to jog some memories.
>
> [1] https://lore.kernel.org/linux-wireless/20201020022812.37064-3-wright.feng@cypress.com/#t
>
> Soontak Lee (1):
>   brcmfmac: Support multiple AP interfaces and fix STA disconnection
>     issue
>
> Ting-Ying Li (1):
>   brcmfmac: don't allow arp/nd offload to be enabled if ap mode exists

Arend, Franky & Hante: could you please take a look at this patchset?
Also I hope that people from Infineon could comment.

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

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

Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Franky Lin]

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

On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
>
> From: Soontak Lee <soontak.lee@cypress.com>
>
> Support multiple AP interfaces for STA + AP + AP usecase.

AFAIK, Broadcom's fullmac firmware doesn't support such 2AP + 1STA use case.

> And fix STA disconnection when deactivating AP interface.
>
> Signed-off-by: Soontak Lee <soontak.lee@cypress.com>
> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> ---
>  .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 48 +++++++++++++++----
>  .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  1 +
>  .../broadcom/brcm80211/brcmfmac/common.c      |  5 ++
>  3 files changed, 44 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index 3ae6779fe153..856fd5516ddf 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -4747,6 +4747,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
>                   settings->inactivity_timeout);
>         dev_role = ifp->vif->wdev.iftype;
>         mbss = ifp->vif->mbss;
> +       brcmf_dbg(TRACE, "mbss %s\n", mbss ? "enabled" : "disabled");
>
>         /* store current 11d setting */
>         if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
> @@ -4961,6 +4962,9 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
>         if ((err) && (!mbss)) {
>                 brcmf_set_mpc(ifp, 1);
>                 brcmf_configure_arp_nd_offload(ifp, true);
> +       } else {
> +               cfg->num_softap++;
> +               brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
>         }
>         return err;
>  }
> @@ -4975,6 +4979,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
>         s32 err;
>         struct brcmf_fil_bss_enable_le bss_enable;
>         struct brcmf_join_params join_params;
> +       s32 apsta = 0;
>
>         brcmf_dbg(TRACE, "Enter\n");
>
> @@ -4983,6 +4988,27 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
>                 /* first to make sure they get processed by fw. */
>                 msleep(400);
>
> +               cfg->num_softap--;
> +
> +               /* Clear bss configuration and SSID */
> +               bss_enable.bsscfgidx = cpu_to_le32(ifp->bsscfgidx);
> +               bss_enable.enable = cpu_to_le32(0);
> +               err = brcmf_fil_iovar_data_set(ifp, "bss", &bss_enable,
> +                                              sizeof(bss_enable));
> +               if (err < 0)
> +                       brcmf_err("bss_enable config failed %d\n", err);
> +
> +               memset(&join_params, 0, sizeof(join_params));
> +               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> +                                            &join_params, sizeof(join_params));
> +               if (err < 0)
> +                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> +
> +               if (cfg->num_softap) {
> +                       brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> +                       return 0;
> +               }
> +
>                 if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
>                         if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
>                                 brcmf_set_pmk(ifp, NULL, 0);
> @@ -5000,17 +5026,18 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
>                 if (ifp->bsscfgidx == 0)
>                         brcmf_fil_iovar_int_set(ifp, "closednet", 0);
>
> -               memset(&join_params, 0, sizeof(join_params));
> -               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> -                                            &join_params, sizeof(join_params));
> -               if (err < 0)
> -                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> -               if (err < 0)
> -                       bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> +               err = brcmf_fil_iovar_int_get(ifp, "apsta", &apsta);
>                 if (err < 0)
> -                       bphy_err(drvr, "setting AP mode failed %d\n", err);
> +                       brcmf_err("wl apsta failed (%d)\n", err);
> +
> +               if (!apsta) {
> +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> +                       if (err < 0)
> +                               bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> +                       if (err < 0)
> +                               bphy_err(drvr, "Set AP mode error %d\n", err);
> +               }
>                 if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
>                         brcmf_fil_iovar_int_set(ifp, "mbss", 0);
>                 brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
> @@ -7641,6 +7668,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
>
>         cfg->wiphy = wiphy;
>         cfg->pub = drvr;
> +       cfg->num_softap = 0;
>         init_vif_event(&cfg->vif_event);
>         INIT_LIST_HEAD(&cfg->vif_list);
>
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> index e90a30808c22..e4ebc2fa6ebb 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> @@ -371,6 +371,7 @@ struct brcmf_cfg80211_info {
>         struct brcmf_cfg80211_wowl wowl;
>         struct brcmf_pno_info *pno;
>         u8 ac_priority[MAX_8021D_PRIO];
> +       u8 num_softap;
>  };
>
>  /**
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> index fe01da9e620d..83e023a22f9b 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> @@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
>                 brcmf_dbg(INFO, "CLM version = %s\n", clmver);
>         }
>
> +       /* set apsta */
> +       err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
> +       if (err)
> +               brcmf_info("failed setting apsta, %d\n", err);
> +

I do not understand why entering apsta mode by default. The mode is
supposed to be enabled only when an AP interface is created in
brcmf_cfg80211_start_ap. I think one of the side effects of apsta mode
is that memory footprint significantly increases. It should remain
disabled for STA only mode (which is the major use case) for better
performance.

Regards,
- Franky

>         /* set mpc */
>         err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
>         if (err) {
> --
> 2.37.0
>

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

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

Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Alvin Šipraga]

Hi Franky,

On Wed, Aug 10, 2022 at 02:32:06PM -0700, Franky Lin wrote:
> On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
> >
> > From: Soontak Lee <soontak.lee@cypress.com>
> >
> > Support multiple AP interfaces for STA + AP + AP usecase.
> 
> AFAIK, Broadcom's fullmac firmware doesn't support such 2AP + 1STA use case.

Thanks for the clarification. The series should be ignored by Kalle
then.

> 
> > And fix STA disconnection when deactivating AP interface.
> >
> > Signed-off-by: Soontak Lee <soontak.lee@cypress.com>
> > Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> > ---
> >  .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 48 +++++++++++++++----
> >  .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  1 +
> >  .../broadcom/brcm80211/brcmfmac/common.c      |  5 ++
> >  3 files changed, 44 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > index 3ae6779fe153..856fd5516ddf 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > @@ -4747,6 +4747,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> >                   settings->inactivity_timeout);
> >         dev_role = ifp->vif->wdev.iftype;
> >         mbss = ifp->vif->mbss;
> > +       brcmf_dbg(TRACE, "mbss %s\n", mbss ? "enabled" : "disabled");
> >
> >         /* store current 11d setting */
> >         if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
> > @@ -4961,6 +4962,9 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> >         if ((err) && (!mbss)) {
> >                 brcmf_set_mpc(ifp, 1);
> >                 brcmf_configure_arp_nd_offload(ifp, true);
> > +       } else {
> > +               cfg->num_softap++;
> > +               brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> >         }
> >         return err;
> >  }
> > @@ -4975,6 +4979,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> >         s32 err;
> >         struct brcmf_fil_bss_enable_le bss_enable;
> >         struct brcmf_join_params join_params;
> > +       s32 apsta = 0;
> >
> >         brcmf_dbg(TRACE, "Enter\n");
> >
> > @@ -4983,6 +4988,27 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> >                 /* first to make sure they get processed by fw. */
> >                 msleep(400);
> >
> > +               cfg->num_softap--;
> > +
> > +               /* Clear bss configuration and SSID */
> > +               bss_enable.bsscfgidx = cpu_to_le32(ifp->bsscfgidx);
> > +               bss_enable.enable = cpu_to_le32(0);
> > +               err = brcmf_fil_iovar_data_set(ifp, "bss", &bss_enable,
> > +                                              sizeof(bss_enable));
> > +               if (err < 0)
> > +                       brcmf_err("bss_enable config failed %d\n", err);
> > +
> > +               memset(&join_params, 0, sizeof(join_params));
> > +               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > +                                            &join_params, sizeof(join_params));
> > +               if (err < 0)
> > +                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> > +
> > +               if (cfg->num_softap) {
> > +                       brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> > +                       return 0;
> > +               }
> > +
> >                 if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
> >                         if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
> >                                 brcmf_set_pmk(ifp, NULL, 0);
> > @@ -5000,17 +5026,18 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> >                 if (ifp->bsscfgidx == 0)
> >                         brcmf_fil_iovar_int_set(ifp, "closednet", 0);
> >
> > -               memset(&join_params, 0, sizeof(join_params));
> > -               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > -                                            &join_params, sizeof(join_params));
> > -               if (err < 0)
> > -                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> > -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > -               if (err < 0)
> > -                       bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > +               err = brcmf_fil_iovar_int_get(ifp, "apsta", &apsta);
> >                 if (err < 0)
> > -                       bphy_err(drvr, "setting AP mode failed %d\n", err);
> > +                       brcmf_err("wl apsta failed (%d)\n", err);
> > +
> > +               if (!apsta) {
> > +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > +                       if (err < 0)
> > +                               bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > +                       if (err < 0)
> > +                               bphy_err(drvr, "Set AP mode error %d\n", err);
> > +               }
> >                 if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
> >                         brcmf_fil_iovar_int_set(ifp, "mbss", 0);
> >                 brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
> > @@ -7641,6 +7668,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> >
> >         cfg->wiphy = wiphy;
> >         cfg->pub = drvr;
> > +       cfg->num_softap = 0;
> >         init_vif_event(&cfg->vif_event);
> >         INIT_LIST_HEAD(&cfg->vif_list);
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > index e90a30808c22..e4ebc2fa6ebb 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > @@ -371,6 +371,7 @@ struct brcmf_cfg80211_info {
> >         struct brcmf_cfg80211_wowl wowl;
> >         struct brcmf_pno_info *pno;
> >         u8 ac_priority[MAX_8021D_PRIO];
> > +       u8 num_softap;
> >  };
> >
> >  /**
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > index fe01da9e620d..83e023a22f9b 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > @@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> >                 brcmf_dbg(INFO, "CLM version = %s\n", clmver);
> >         }
> >
> > +       /* set apsta */
> > +       err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
> > +       if (err)
> > +               brcmf_info("failed setting apsta, %d\n", err);
> > +
> 
> I do not understand why entering apsta mode by default. The mode is
> supposed to be enabled only when an AP interface is created in
> brcmf_cfg80211_start_ap. I think one of the side effects of apsta mode
> is that memory footprint significantly increases. It should remain
> disabled for STA only mode (which is the major use case) for better
> performance.

By better performance, do you just mean "lower chance of memory
exhaustion"? If so, surely the firmware would be designed such that it
doesn't run out of memory under the advertised use-cases (STA, AP+STA
etc.), regardless of the current apsta setting?

I would hope that somebody from Infineon could chime in here, as I am
not privy to the FW design.

Kind regards,
Alvin

> 
> Regards,
> - Franky
> 
> >         /* set mpc */
> >         err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
> >         if (err) {
> > --
> > 2.37.0
> >
> 
> -- 
> This electronic communication and the information and any files transmitted 
> with it, or attached to it, are confidential and are intended solely for 
> the use of the individual or entity to whom it is addressed and may contain 
> information that is confidential, legally privileged, protected by privacy 
> laws, or otherwise restricted from disclosure to anyone else. If you are 
> not the intended recipient or the person responsible for delivering the 
> e-mail to the intended recipient, you are hereby notified that any use, 
> copying, distributing, dissemination, forwarding, printing, or copying of 
> this e-mail is strictly prohibited. If you received this e-mail in error, 
> please return the e-mail to the sender, delete it from your computer, and 
> destroy any printed copy of it.

Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Franky Lin]

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

Hi Alvin,

On Wed, Aug 17, 2022 at 1:50 AM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
>
> Hi Franky,
>
> On Wed, Aug 10, 2022 at 02:32:06PM -0700, Franky Lin wrote:
> > On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
> > >
> > > From: Soontak Lee <soontak.lee@cypress.com>
> > >
> > > Support multiple AP interfaces for STA + AP + AP usecase.
> >
> > AFAIK, Broadcom's fullmac firmware doesn't support such 2AP + 1STA use case.
>
> Thanks for the clarification. The series should be ignored by Kalle
> then.
>
> >
> > > And fix STA disconnection when deactivating AP interface.
> > >
> > > Signed-off-by: Soontak Lee <soontak.lee@cypress.com>
> > > Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > > Signed-off-by: Alvin Šipraga <alsi@bang-olufsen.dk>
> > > ---
> > >  .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 48 +++++++++++++++----
> > >  .../broadcom/brcm80211/brcmfmac/cfg80211.h    |  1 +
> > >  .../broadcom/brcm80211/brcmfmac/common.c      |  5 ++
> > >  3 files changed, 44 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > index 3ae6779fe153..856fd5516ddf 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> > > @@ -4747,6 +4747,7 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> > >                   settings->inactivity_timeout);
> > >         dev_role = ifp->vif->wdev.iftype;
> > >         mbss = ifp->vif->mbss;
> > > +       brcmf_dbg(TRACE, "mbss %s\n", mbss ? "enabled" : "disabled");
> > >
> > >         /* store current 11d setting */
> > >         if (brcmf_fil_cmd_int_get(ifp, BRCMF_C_GET_REGULATORY,
> > > @@ -4961,6 +4962,9 @@ brcmf_cfg80211_start_ap(struct wiphy *wiphy, struct net_device *ndev,
> > >         if ((err) && (!mbss)) {
> > >                 brcmf_set_mpc(ifp, 1);
> > >                 brcmf_configure_arp_nd_offload(ifp, true);
> > > +       } else {
> > > +               cfg->num_softap++;
> > > +               brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> > >         }
> > >         return err;
> > >  }
> > > @@ -4975,6 +4979,7 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > >         s32 err;
> > >         struct brcmf_fil_bss_enable_le bss_enable;
> > >         struct brcmf_join_params join_params;
> > > +       s32 apsta = 0;
> > >
> > >         brcmf_dbg(TRACE, "Enter\n");
> > >
> > > @@ -4983,6 +4988,27 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > >                 /* first to make sure they get processed by fw. */
> > >                 msleep(400);
> > >
> > > +               cfg->num_softap--;
> > > +
> > > +               /* Clear bss configuration and SSID */
> > > +               bss_enable.bsscfgidx = cpu_to_le32(ifp->bsscfgidx);
> > > +               bss_enable.enable = cpu_to_le32(0);
> > > +               err = brcmf_fil_iovar_data_set(ifp, "bss", &bss_enable,
> > > +                                              sizeof(bss_enable));
> > > +               if (err < 0)
> > > +                       brcmf_err("bss_enable config failed %d\n", err);
> > > +
> > > +               memset(&join_params, 0, sizeof(join_params));
> > > +               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > > +                                            &join_params, sizeof(join_params));
> > > +               if (err < 0)
> > > +                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> > > +
> > > +               if (cfg->num_softap) {
> > > +                       brcmf_dbg(TRACE, "Num of SoftAP %u\n", cfg->num_softap);
> > > +                       return 0;
> > > +               }
> > > +
> > >                 if (profile->use_fwauth != BIT(BRCMF_PROFILE_FWAUTH_NONE)) {
> > >                         if (profile->use_fwauth & BIT(BRCMF_PROFILE_FWAUTH_PSK))
> > >                                 brcmf_set_pmk(ifp, NULL, 0);
> > > @@ -5000,17 +5026,18 @@ static int brcmf_cfg80211_stop_ap(struct wiphy *wiphy, struct net_device *ndev,
> > >                 if (ifp->bsscfgidx == 0)
> > >                         brcmf_fil_iovar_int_set(ifp, "closednet", 0);
> > >
> > > -               memset(&join_params, 0, sizeof(join_params));
> > > -               err = brcmf_fil_cmd_data_set(ifp, BRCMF_C_SET_SSID,
> > > -                                            &join_params, sizeof(join_params));
> > > -               if (err < 0)
> > > -                       bphy_err(drvr, "SET SSID error (%d)\n", err);
> > > -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > > -               if (err < 0)
> > > -                       bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > > -               err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > > +               err = brcmf_fil_iovar_int_get(ifp, "apsta", &apsta);
> > >                 if (err < 0)
> > > -                       bphy_err(drvr, "setting AP mode failed %d\n", err);
> > > +                       brcmf_err("wl apsta failed (%d)\n", err);
> > > +
> > > +               if (!apsta) {
> > > +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_DOWN, 1);
> > > +                       if (err < 0)
> > > +                               bphy_err(drvr, "BRCMF_C_DOWN error %d\n", err);
> > > +                       err = brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_AP, 0);
> > > +                       if (err < 0)
> > > +                               bphy_err(drvr, "Set AP mode error %d\n", err);
> > > +               }
> > >                 if (brcmf_feat_is_enabled(ifp, BRCMF_FEAT_MBSS))
> > >                         brcmf_fil_iovar_int_set(ifp, "mbss", 0);
> > >                 brcmf_fil_cmd_int_set(ifp, BRCMF_C_SET_REGULATORY,
> > > @@ -7641,6 +7668,7 @@ struct brcmf_cfg80211_info *brcmf_cfg80211_attach(struct brcmf_pub *drvr,
> > >
> > >         cfg->wiphy = wiphy;
> > >         cfg->pub = drvr;
> > > +       cfg->num_softap = 0;
> > >         init_vif_event(&cfg->vif_event);
> > >         INIT_LIST_HEAD(&cfg->vif_list);
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > > index e90a30808c22..e4ebc2fa6ebb 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.h
> > > @@ -371,6 +371,7 @@ struct brcmf_cfg80211_info {
> > >         struct brcmf_cfg80211_wowl wowl;
> > >         struct brcmf_pno_info *pno;
> > >         u8 ac_priority[MAX_8021D_PRIO];
> > > +       u8 num_softap;
> > >  };
> > >
> > >  /**
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > index fe01da9e620d..83e023a22f9b 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > @@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> > >                 brcmf_dbg(INFO, "CLM version = %s\n", clmver);
> > >         }
> > >
> > > +       /* set apsta */
> > > +       err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
> > > +       if (err)
> > > +               brcmf_info("failed setting apsta, %d\n", err);
> > > +
> >
> > I do not understand why entering apsta mode by default. The mode is
> > supposed to be enabled only when an AP interface is created in
> > brcmf_cfg80211_start_ap. I think one of the side effects of apsta mode
> > is that memory footprint significantly increases. It should remain
> > disabled for STA only mode (which is the major use case) for better
> > performance.
>
> By better performance, do you just mean "lower chance of memory
> exhaustion"? If so, surely the firmware would be designed such that it
> doesn't run out of memory under the advertised use-cases (STA, AP+STA
> etc.), regardless of the current apsta setting?

I think some packet related buffers will be adjusted for apsta mode so
the sta mode performance will hurt because there is less buffer to
use.

Another significant impact I am sure about is some power saving
features will be turned off once apsta mode is enabled. So the chip
will drain more power even the AP interface is not created.

Regards,
- Franky


>
> I would hope that somebody from Infineon could chime in here, as I am
> not privy to the FW design.
>
> Kind regards,
> Alvin
>
> >
> > Regards,
> > - Franky
> >
> > >         /* set mpc */
> > >         err = brcmf_fil_iovar_int_set(ifp, "mpc", 1);
> > >         if (err) {
> > > --
> > > 2.37.0
> > >
> >
> > --
> > This electronic communication and the information and any files transmitted
> > with it, or attached to it, are confidential and are intended solely for
> > the use of the individual or entity to whom it is addressed and may contain
> > information that is confidential, legally privileged, protected by privacy
> > laws, or otherwise restricted from disclosure to anyone else. If you are
> > not the intended recipient or the person responsible for delivering the
> > e-mail to the intended recipient, you are hereby notified that any use,
> > copying, distributing, dissemination, forwarding, printing, or copying of
> > this e-mail is strictly prohibited. If you received this e-mail in error,
> > please return the e-mail to the sender, delete it from your computer, and
> > destroy any printed copy of it.

-- 
This electronic communication and the information and any files transmitted 
with it, or attached to it, are confidential and are intended solely for 
the use of the individual or entity to whom it is addressed and may contain 
information that is confidential, legally privileged, protected by privacy 
laws, or otherwise restricted from disclosure to anyone else. If you are 
not the intended recipient or the person responsible for delivering the 
e-mail to the intended recipient, you are hereby notified that any use, 
copying, distributing, dissemination, forwarding, printing, or copying of 
this e-mail is strictly prohibited. If you received this e-mail in error, 
please return the e-mail to the sender, delete it from your computer, and 
destroy any printed copy of it.

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

Re: [PATCH -next 1/2] brcmfmac: Support multiple AP interfaces and fix STA disconnection issue

[Alvin Šipraga]

Hi Franky,

On Mon, Aug 22, 2022 at 10:12:14AM -0700, Franky Lin wrote:
> On Wed, Aug 17, 2022 at 1:50 AM Alvin Šipraga <ALSI@bang-olufsen.dk> wrote:
> > On Wed, Aug 10, 2022 at 02:32:06PM -0700, Franky Lin wrote:
> > > On Fri, Jul 22, 2022 at 5:30 AM Alvin Šipraga <alvin@pqrs.dk> wrote:
> > > >  /**
> > > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > > index fe01da9e620d..83e023a22f9b 100644
> > > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
> > > > @@ -303,6 +303,11 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
> > > >                 brcmf_dbg(INFO, "CLM version = %s\n", clmver);
> > > >         }
> > > >
> > > > +       /* set apsta */
> > > > +       err = brcmf_fil_iovar_int_set(ifp, "apsta", 1);
> > > > +       if (err)
> > > > +               brcmf_info("failed setting apsta, %d\n", err);
> > > > +
> > >
> > > I do not understand why entering apsta mode by default. The mode is
> > > supposed to be enabled only when an AP interface is created in
> > > brcmf_cfg80211_start_ap. I think one of the side effects of apsta mode
> > > is that memory footprint significantly increases. It should remain
> > > disabled for STA only mode (which is the major use case) for better
> > > performance.
> >
> > By better performance, do you just mean "lower chance of memory
> > exhaustion"? If so, surely the firmware would be designed such that it
> > doesn't run out of memory under the advertised use-cases (STA, AP+STA
> > etc.), regardless of the current apsta setting?
> 
> I think some packet related buffers will be adjusted for apsta mode so
> the sta mode performance will hurt because there is less buffer to
> use.
> 
> Another significant impact I am sure about is some power saving
> features will be turned off once apsta mode is enabled. So the chip
> will drain more power even the AP interface is not created.

OK. And this apsta mode seems only to be used for STA + P2P mode in the
upstream driver's current form. But doesn't the driver also support AP +
STA mode ordinarily? Would apsta=1 not be necessary for that use-case?
Of course I assume you can only answer me for Broadcom chipsets.

Trying to understand whether to drop this whole series or whether a
modified version can be suitably upstreamed.

Kind regards,
Alvin

> 
> Regards,
> - Franky