From: Arend van Spriel <arend.vanspriel@broadcom.com>
To: "Rafał Miłecki" <zajec5@gmail.com>, "Kalle Valo" <kvalo@codeaurora.org>
Cc: Brett Rudley <brudley@broadcom.com>,
Arend van Spriel <arend@broadcom.com>,
"Franky (Zhenhui) Lin" <frankyl@broadcom.com>,
Hante Meuleman <meuleman@broadcom.com>,
Pieter-Paul Giesberts <pieterpg@broadcom.com>,
Franky Lin <franky.lin@broadcom.com>,
"open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER"
<linux-wireless@vger.kernel.org>,
"open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER"
<brcm80211-dev-list@broadcom.com>,
"open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] brcmfmac: rework function picking free BSS index
Date: Mon, 13 Jun 2016 21:30:31 +0200 [thread overview]
Message-ID: <575F09D7.6030505@broadcom.com> (raw)
In-Reply-To: <5759C09F.7020704@broadcom.com>
On 09-06-16 21:16, Arend van Spriel wrote:
> On 26-05-16 01:44, Rafał Miłecki wrote:
>> The old implementation was overcomplicated and slightly bugged in some
>> corner cases.
>>
[...]
>> New code is simpler, placed in file where it's really used, handles
>> running out of free BSS-es and allows using 4 interfaces at the same
>> time. It also looks for the first free BSS instead of one after the last
>> in use. It works well with current driver (which doesn't allow deleting
>> interfaces) and should be future proof (if we ever allow deleting).
>>
>> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
>> ---
>> .../broadcom/brcm80211/brcmfmac/cfg80211.c | 17 ++++++++++++++-
>> .../wireless/broadcom/brcm80211/brcmfmac/core.c | 24 ----------------------
>> .../wireless/broadcom/brcm80211/brcmfmac/core.h | 1 -
>> 3 files changed, 16 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> index 3d09d23..d00eef8 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>> @@ -541,6 +541,21 @@ brcmf_cfg80211_update_proto_addr_mode(struct wireless_dev *wdev)
>> ADDR_INDIRECT);
>> }
>>
>> +static int brcmf_get_first_free_bsscfgidx(struct brcmf_pub *drvr)
>> +{
>> + int bsscfgidx;
>> +
>> + for (bsscfgidx = 0; bsscfgidx < BRCMF_MAX_IFS; bsscfgidx++) {
>> + /* bsscfgidx 1 is reserved for legacy P2P */
>
> Hi Rafał,
>
> A bit late as the patch is already applied, but this reserved index is
> no longer needed as we removed all trickery that was build on the
> assumption that the P2P_DEVICE interface was always in bsscfgidx 1.
> Hence this could be removed.
I tested STA on bsscfgidx=0, AP on bsscfgidx=1, and P2P_DEV on
bsscfgidx=2. P2P discovery on P2P_DEV interface works as expected so we
can indeed drop the 'bsscfgidx 1 is reserved' statement. I want to
verify on an older device before creating a patch.
Regards,
Arend
> Regards,
> Arend
>
>> + if (bsscfgidx == 1)
>> + continue;
>> + if (!drvr->iflist[bsscfgidx])
>> + return bsscfgidx;
>> + }
>> +
>> + return -ENOMEM;
>> +}
>> +
>> static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp)
>> {
>> struct brcmf_mbss_ssid_le mbss_ssid_le;
>> @@ -548,7 +563,7 @@ static int brcmf_cfg80211_request_ap_if(struct brcmf_if *ifp)
>> int err;
>>
>> memset(&mbss_ssid_le, 0, sizeof(mbss_ssid_le));
>> - bsscfgidx = brcmf_get_next_free_bsscfgidx(ifp->drvr);
>> + bsscfgidx = brcmf_get_first_free_bsscfgidx(ifp->drvr);
>> if (bsscfgidx < 0)
>> return bsscfgidx;
>>
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> index b590499..6a76480 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
>> @@ -753,30 +753,6 @@ void brcmf_remove_interface(struct brcmf_if *ifp)
>> brcmf_del_if(ifp->drvr, ifp->bsscfgidx);
>> }
>>
>> -int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr)
>> -{
>> - int ifidx;
>> - int bsscfgidx;
>> - bool available;
>> - int highest;
>> -
>> - available = false;
>> - bsscfgidx = 2;
>> - highest = 2;
>> - for (ifidx = 0; ifidx < BRCMF_MAX_IFS; ifidx++) {
>> - if (drvr->iflist[ifidx]) {
>> - if (drvr->iflist[ifidx]->bsscfgidx == bsscfgidx)
>> - bsscfgidx = highest + 1;
>> - else if (drvr->iflist[ifidx]->bsscfgidx > highest)
>> - highest = drvr->iflist[ifidx]->bsscfgidx;
>> - } else {
>> - available = true;
>> - }
>> - }
>> -
>> - return available ? bsscfgidx : -ENOMEM;
>> -}
>> -
>> #ifdef CONFIG_INET
>> #define ARPOL_MAX_ENTRIES 8
>> static int brcmf_inetaddr_changed(struct notifier_block *nb,
>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>> index 647d3cc..2a075c5 100644
>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h
>> @@ -217,7 +217,6 @@ int brcmf_net_attach(struct brcmf_if *ifp, bool rtnl_locked);
>> struct brcmf_if *brcmf_add_if(struct brcmf_pub *drvr, s32 bsscfgidx, s32 ifidx,
>> bool is_p2pdev, char *name, u8 *mac_addr);
>> void brcmf_remove_interface(struct brcmf_if *ifp);
>> -int brcmf_get_next_free_bsscfgidx(struct brcmf_pub *drvr);
>> void brcmf_txflowblock_if(struct brcmf_if *ifp,
>> enum brcmf_netif_stop_reason reason, bool state);
>> void brcmf_txfinalize(struct brcmf_if *ifp, struct sk_buff *txp, bool success);
>>
next prev parent reply other threads:[~2016-06-13 19:30 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-05-25 23:44 [PATCH] brcmfmac: rework function picking free BSS index Rafał Miłecki
2016-06-09 16:17 ` Kalle Valo
2016-06-09 16:30 ` Kalle Valo
2016-06-09 16:48 ` Rafał Miłecki
2016-06-09 19:16 ` [PATCH] " Arend van Spriel
2016-06-13 19:30 ` Arend van Spriel [this message]
2016-06-13 20:25 ` Rafał Miłecki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=575F09D7.6030505@broadcom.com \
--to=arend.vanspriel@broadcom.com \
--cc=arend@broadcom.com \
--cc=brcm80211-dev-list@broadcom.com \
--cc=brudley@broadcom.com \
--cc=franky.lin@broadcom.com \
--cc=frankyl@broadcom.com \
--cc=kvalo@codeaurora.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@vger.kernel.org \
--cc=meuleman@broadcom.com \
--cc=netdev@vger.kernel.org \
--cc=pieterpg@broadcom.com \
--cc=zajec5@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).