[PATCH 0/4] brcmfmac: SoftAP creation and dcmd buffer size changes

[PATCH 0/4] brcmfmac: SoftAP creation and dcmd buffer size changes

[Chi-Hsien Lin]

Allow SoftAP creation via ioctl from test tool; also update dcmd buffer size
settings for new firmware. Also fix a sparse check error.

Chi-Hsien Lin (1):
  brcmfmac: fix a sparse incorrect type error in p2p.c

Double Lo (2):
  brcmfmac: support virtual interface creation from firmware
  brcmfmac: increase dcmd maximum buffer size

Kurt Lee (1):
  brcmfmac: set net carrier on via test tool for AP mode

 .../broadcom/brcm80211/brcmfmac/bcdc.c        |  4 +-
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 46 +++++++++++++++++--
 .../broadcom/brcm80211/brcmfmac/p2p.c         |  3 +-
 .../broadcom/brcm80211/brcmfmac/vendor.c      |  9 ++++
 4 files changed, 56 insertions(+), 6 deletions(-)

--
2.25.0


This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.

[PATCH 1/4] brcmfmac: fix a sparse incorrect type error in p2p.c

[Chi-Hsien Lin]

Fix the below sparse error introduced by commit 9c29da3f4e7e
("brcmfmac: Fix P2P Group Formation failure via Go-neg method").

drivers/net/wireless/broadcom/brcm80211//brcmfmac/p2p.c:1741:40: error:
incorrect type in initializer (different base types)
drivers/net/wireless/broadcom/brcm80211//brcmfmac/p2p.c:1741:40:
expected signed int [usertype] requested_dwell
drivers/net/wireless/broadcom/brcm80211//brcmfmac/p2p.c:1741:40:    got
restricted __le32 [usertype] dwell_time
make[3]: *** [drivers/net/wireless/broadcom/brcm80211//brcmfmac/p2p.o]
Error 1

Fixes: 9c29da3f4e7ef ("brcmfmac: Fix P2P Group Formation failure via Go-neg method")
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
index d2795dc17c46..194c1ac19402 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
@@ -1737,8 +1737,7 @@ bool brcmf_p2p_send_action_frame(struct brcmf_cfg80211_info *cfg,
        uint delta_ms;
        unsigned long dwell_jiffies = 0;
        bool dwell_overflow = false;
-
-       s32 requested_dwell = af_params->dwell_time;
+       s32 requested_dwell = le32_to_cpu(af_params->dwell_time);

        action_frame = &af_params->action_frame;
        action_frame_len = le16_to_cpu(action_frame->len);
--
2.25.0


This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.

[PATCH 2/4] brcmfmac: support virtual interface creation from firmware

[Chi-Hsien Lin]

From: Double Lo <double.lo@cypress.com>

Allow interface creation via IF_ADD event from firmware.

Signed-off-by: Double Lo <double.lo@cypress.com>
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c    | 46 +++++++++++++++++--
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index a757abd7a599..900376dc0b67 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -230,6 +230,9 @@ struct parsed_vndr_ies {
        struct parsed_vndr_ie_info ie_info[VNDR_IE_PARSE_LIMIT];
 };

+#define WLC_E_IF_ROLE_STA              0       /* Infra STA */
+#define WLC_E_IF_ROLE_AP               1       /* Access Point */
+
 static u8 nl80211_band_to_fwil(enum nl80211_band band)
 {
        switch (band) {
@@ -5532,8 +5535,10 @@ void brcmf_cfg80211_free_netdev(struct net_device *ndev)
        ifp = netdev_priv(ndev);
        vif = ifp->vif;

-       if (vif)
+       if (vif) {
                brcmf_free_vif(vif);
+               ifp->vif = NULL;
+       }
 }

 static bool brcmf_is_linkup(struct brcmf_cfg80211_vif *vif,
@@ -6090,6 +6095,9 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
        struct brcmf_if_event *ifevent = (struct brcmf_if_event *)data;
        struct brcmf_cfg80211_vif_event *event = &cfg->vif_event;
        struct brcmf_cfg80211_vif *vif;
+       enum nl80211_iftype iftype = NL80211_IFTYPE_UNSPECIFIED;
+       bool vif_pend = false;
+       int err;

        brcmf_dbg(TRACE, "Enter: action %u flags %u ifidx %u bsscfgidx %u\n",
                  ifevent->action, ifevent->flags, ifevent->ifidx,
@@ -6102,9 +6110,28 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
        switch (ifevent->action) {
        case BRCMF_E_IF_ADD:
                /* waiting process may have timed out */
-               if (!cfg->vif_event.vif) {
+               if (!vif) {
+                       /* handle IF_ADD event from firmware */
                        spin_unlock(&event->vif_event_lock);
-                       return -EBADF;
+                       vif_pend = true;
+                       if (ifevent->role == WLC_E_IF_ROLE_STA)
+                               iftype = NL80211_IFTYPE_STATION;
+                       else if (ifevent->role == WLC_E_IF_ROLE_AP)
+                               iftype = NL80211_IFTYPE_AP;
+                       else
+                               vif_pend = false;
+
+                       if (vif_pend) {
+                               vif = brcmf_alloc_vif(cfg, iftype);
+                               if (IS_ERR(vif)) {
+                                       brcmf_err("Role:%d failed to alloc vif\n",
+                                                 ifevent->role);
+                                       return PTR_ERR(vif);
+                               }
+                       } else {
+                               brcmf_err("Invalid Role:%d\n", ifevent->role);
+                               return -EBADF;
+                       }
                }

                ifp->vif = vif;
@@ -6114,6 +6141,19 @@ static s32 brcmf_notify_vif_event(struct brcmf_if *ifp,
                        ifp->ndev->ieee80211_ptr = &vif->wdev;
                        SET_NETDEV_DEV(ifp->ndev, wiphy_dev(cfg->wiphy));
                }
+
+               if (vif_pend) {
+                       err = brcmf_net_attach(ifp, false);
+                       if (err) {
+                               brcmf_err("netdevice register failed with err:%d\n",
+                                         err);
+                               brcmf_free_vif(vif);
+                               free_netdev(ifp->ndev);
+                       }
+                       spin_unlock(&event->vif_event_lock);
+                       return err;
+               }
+
                spin_unlock(&event->vif_event_lock);
                wake_up(&event->vif_wq);
                return 0;
--
2.25.0


This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.

[PATCH 3/4] brcmfmac: set net carrier on via test tool for AP mode

[Chi-Hsien Lin]

From: Kurt Lee <kurt.lee@cypress.com>

In manufacturing line, test tool may be used to enable SoftAP. Such
SoftAP can't pass traffic because netif carrier is off by default. To
allow such use case, let brcmfmac parse ioctl cmd, and then set iftype
to ap mode and report netif_carrier_on to upper layer.

Signed-off-by: Kurt Lee <kurt.lee@cypress.com>
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/vendor.c    | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
index d07e7c7355d9..5edf5ac1167a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
@@ -64,6 +64,15 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
                *(char *)(dcmd_buf + len)  = '\0';
        }

+       if (cmdhdr->cmd == BRCMF_C_SET_AP) {
+               if (*(int *)(dcmd_buf) == 1) {
+                       ifp->vif->wdev.iftype = NL80211_IFTYPE_AP;
+                       brcmf_net_setcarrier(ifp, true);
+               } else {
+                       ifp->vif->wdev.iftype = NL80211_IFTYPE_STATION;
+               }
+       }
+
        if (cmdhdr->set)
                ret = brcmf_fil_cmd_data_set(ifp, cmdhdr->cmd, dcmd_buf,
                                             ret_len);
--
2.25.0


This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.

[PATCH 4/4] brcmfmac: increase dcmd maximum buffer size

[Chi-Hsien Lin]

From: Double Lo <double.lo@cypress.com>

Increase dcmd maximum buffer size to match firmware configuration for
new chips.

Signed-off-by: Double Lo <double.lo@cypress.com>
Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
index 2c95a08a5871..705130cf27d5 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -87,6 +87,8 @@ struct brcmf_proto_bcdc_header {
                                         * plus any space that might be needed
                                         * for bus alignment padding.
                                         */
+#define ROUND_UP_MARGIN 2048
+
 struct brcmf_bcdc {
        u16 reqid;
        u8 bus_header[BUS_HEADER_LEN];
@@ -471,7 +473,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr)

        drvr->hdrlen += BCDC_HEADER_LEN + BRCMF_PROT_FW_SIGNAL_MAX_TXBYTES;
        drvr->bus_if->maxctl = BRCMF_DCMD_MAXLEN +
-                       sizeof(struct brcmf_proto_bcdc_dcmd);
+                       sizeof(struct brcmf_proto_bcdc_dcmd) + ROUND_UP_MARGIN;
        return 0;

 fail:
--
2.25.0


This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.

Re: [PATCH 3/4] brcmfmac: set net carrier on via test tool for AP mode

[Arend Van Spriel]

On 5/19/2020 1:09 PM, Chi-Hsien Lin wrote:
> From: Kurt Lee <kurt.lee@cypress.com>
> 
> In manufacturing line, test tool may be used to enable SoftAP. Such
> SoftAP can't pass traffic because netif carrier is off by default. To
> allow such use case, let brcmfmac parse ioctl cmd, and then set iftype
> to ap mode and report netif_carrier_on to upper layer.
> 
> Signed-off-by: Kurt Lee <kurt.lee@cypress.com>
> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
> ---
>   .../net/wireless/broadcom/brcm80211/brcmfmac/vendor.c    | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
> index d07e7c7355d9..5edf5ac1167a 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
> @@ -64,6 +64,15 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
>                  *(char *)(dcmd_buf + len)  = '\0';
>          }
> 
> +       if (cmdhdr->cmd == BRCMF_C_SET_AP) {
> +               if (*(int *)(dcmd_buf) == 1) {
> +                       ifp->vif->wdev.iftype = NL80211_IFTYPE_AP;
> +                       brcmf_net_setcarrier(ifp, true);
> +               } else {
> +                       ifp->vif->wdev.iftype = NL80211_IFTYPE_STATION;
> +               }
> +       }
> +

I prefer to keep this code path as flat as possible so I prefer no 
monitoring of firmware commands issued by user-space. Maybe another 
approach would be to set the carrier on a firmware event?

Regards,
Arend

Re: [PATCH 4/4] brcmfmac: increase dcmd maximum buffer size

[Arend Van Spriel]

On 5/19/2020 1:09 PM, Chi-Hsien Lin wrote:
> From: Double Lo <double.lo@cypress.com>
> 
> Increase dcmd maximum buffer size to match firmware configuration for
> new chips.

 From the description I was expecting BRCMF_DCMD_MAXLEN to be increased 
but that is not the case. Please explain the need for the 
ROUND_UP_MARGIN. Is it always needed or just on some platforms?

Regards,
Arend

Re: [PATCH 0/4] brcmfmac: SoftAP creation and dcmd buffer size changes

[Kalle Valo]

Chi-Hsien Lin <chi-hsien.lin@cypress.com> writes:

> Allow SoftAP creation via ioctl from test tool; also update dcmd buffer size
> settings for new firmware. Also fix a sparse check error.

What ioctl is this exactly? Wireless drivers should be using nl80211,
not any ioctl interface.

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

Re: [PATCH 0/4] brcmfmac: SoftAP creation and dcmd buffer size changes

[Arend Van Spriel]

On 5/19/2020 3:54 PM, Kalle Valo wrote:
> Chi-Hsien Lin <chi-hsien.lin@cypress.com> writes:
> 
>> Allow SoftAP creation via ioctl from test tool; also update dcmd buffer size
>> settings for new firmware. Also fix a sparse check error.
> 
> What ioctl is this exactly? Wireless drivers should be using nl80211,
> not any ioctl interface.

Hi Kalle,

The terminology is a bit off. This is vendor specific command 
interfacing directly with firmware and the content is opaque from driver 
perspective. However, a number of these commands have their nl80211 
equivalent. Especially regarding creation of AP interface I would simply 
go with creation through nl80211.

Regards,
Arend

Re: [PATCH 4/4] brcmfmac: increase dcmd maximum buffer size

[Chi-Hsien Lin]

On 05/19/2020 7:29, Arend Van Spriel wrote:
> 
> 
> On 5/19/2020 1:09 PM, Chi-Hsien Lin wrote:
>> From: Double Lo <double.lo@cypress.com>
>>
>> Increase dcmd maximum buffer size to match firmware configuration for
>> new chips.
> 
>   From the description I was expecting BRCMF_DCMD_MAXLEN to be increased
> but that is not the case. Please explain the need for the
> ROUND_UP_MARGIN. Is it always needed or just on some platforms?

Thanks for the feedback.

ROUND_UP_MARGIN is the biggest SDIO block size possible and is used to 
accomodate padding in brcmf_sdio_read_control(). This should be only 
related to SDIO block size.

The comment should be updated to "Add the padding round up value in the 
max control buffer size". Will update it in V2.

> 
> Regards,
> Arend
> 

Re: [PATCH 3/4] brcmfmac: set net carrier on via test tool for AP mode

[Chi-Hsien Lin]

On 05/19/2020 7:18, Arend Van Spriel wrote:
> On 5/19/2020 1:09 PM, Chi-Hsien Lin wrote:
>> From: Kurt Lee <kurt.lee@cypress.com>
>>
>> In manufacturing line, test tool may be used to enable SoftAP. Such
>> SoftAP can't pass traffic because netif carrier is off by default. To
>> allow such use case, let brcmfmac parse ioctl cmd, and then set iftype
>> to ap mode and report netif_carrier_on to upper layer.
>>
>> Signed-off-by: Kurt Lee <kurt.lee@cypress.com>
>> Signed-off-by: Chi-Hsien Lin <chi-hsien.lin@cypress.com>
>> ---
>>    .../net/wireless/broadcom/brcm80211/brcmfmac/vendor.c    | 9 +++++++++
>>    1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
>> index d07e7c7355d9..5edf5ac1167a 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/vendor.c
>> @@ -64,6 +64,15 @@ static int brcmf_cfg80211_vndr_cmds_dcmd_handler(struct wiphy *wiphy,
>>                   *(char *)(dcmd_buf + len)  = '\0';
>>           }
>>
>> +       if (cmdhdr->cmd == BRCMF_C_SET_AP) {
>> +               if (*(int *)(dcmd_buf) == 1) {
>> +                       ifp->vif->wdev.iftype = NL80211_IFTYPE_AP;
>> +                       brcmf_net_setcarrier(ifp, true);
>> +               } else {
>> +                       ifp->vif->wdev.iftype = NL80211_IFTYPE_STATION;
>> +               }
>> +       }
>> +
> 
> I prefer to keep this code path as flat as possible so I prefer no
> monitoring of firmware commands issued by user-space. Maybe another
> approach would be to set the carrier on a firmware event?

Event based implementation would be like below:
1. BRCMF_E_IF -> brcmf_fweh_handle_if_event: Change iftype to STA or AP
2. BRCMF_E_LINK -> brcmf_notify_connect_status: Set net carrier on or 
off only in AP mode

Such mechanism could run into race conditions when driver init (triggers 
BRCMF_E_IF with STA role) and start_ap (triggers BRCMF_E_IF with AP 
role) happens close enough. If hostapd sets key before BRCMF_E_IF for AP 
role comes back (iftype is still NL80211_IFTYPE_STATION), 
nl80211_key_allowed() fails with -NOLINK. This is observed when start_ap 
happens right after insmod, or when hostapd disable/enable happens.


> 
> Regards,
> Arend
> 

Re: [PATCH 0/4] brcmfmac: SoftAP creation and dcmd buffer size changes

[Kalle Valo]

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

> On 5/19/2020 3:54 PM, Kalle Valo wrote:
>> Chi-Hsien Lin <chi-hsien.lin@cypress.com> writes:
>>
>>> Allow SoftAP creation via ioctl from test tool; also update dcmd buffer size
>>> settings for new firmware. Also fix a sparse check error.
>>
>> What ioctl is this exactly? Wireless drivers should be using nl80211,
>> not any ioctl interface.
>
> Hi Kalle,
>
> The terminology is a bit off. This is vendor specific command
> interfacing directly with firmware and the content is opaque from
> driver perspective.

Ah, you mean nl80211 vendor command. That has nothing to do with ioctl :)

> However, a number of these commands have their nl80211 equivalent.
> Especially regarding creation of AP interface I would simply go with
> creation through nl80211.

And do note that we have new rules for nl80211 vendor commands:

https://wireless.wiki.kernel.org/en/developers/documentation/nl80211#vendor-specific_api

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