* [PATCH next] brcmfmac: don't remove interface on link down firmware event
@ 2016-05-29 21:39 Rafał Miłecki
2016-05-29 22:01 ` kbuild test robot
2016-05-30 4:40 ` [PATCH V2 " Rafał Miłecki
0 siblings, 2 replies; 4+ messages in thread
From: Rafał Miłecki @ 2016-05-29 21:39 UTC (permalink / raw)
To: Kalle Valo
Cc: Rafał Miłecki, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, Pieter-Paul Giesberts,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
There are two firmware events we handle similarly in brcmfmac:
BRCMF_E_LINK and BRCMF_E_IF. The difference from firmware point of view
is that the first one means BSS remains present in the firmware. Trying
to (re)create it (e.g. when adding new virtual interface) will result in
an error.
Current code treats both events in a similar way. It removes Linux
interface for each of them. It works OK with e.g. BCM43602. Its firmware
generates both events for each interface. It means we get BRCMF_E_LINK
and remove interface. That is soon followed by BRCMF_E_IF which means
BSS was also removed in a firmware. The only downside of this is a
harmless error like:
[ 208.643180] brcmfmac: brcmf_fweh_call_event_handler: no interface object
Unfortunately BCM4366 firmware doesn't automatically remove BSS and so
it doesn't generate BRCMF_E_IF. In such case we incorrectly remove Linux
interface on BRCMF_E_LINK as BSS is still present in the firmware. It
results in an error when trying to re-create virtual interface, e.g.:
> iw phy phy1 interface add wlan1-1 type __ap
[ 3602.929199] brcmfmac: brcmf_ap_add_vif: timeout occurred
command failed: I/O error (-5)
With this patch we don't remove Linux interface while firmware keeps
BSS. Thanks to this we keep a consistent states of host driver and
device firmware.
Further improvement should be to mark BSS as disabled and remove
interface on BRCMF_E_LINK. Then we should add support for reusing
BSS-es.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 38df1be..26ff0f4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5439,8 +5439,6 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
ndev != cfg_to_ndev(cfg)) {
brcmf_dbg(CONN, "AP mode link down\n");
complete(&cfg->vif_disabled);
- if (ifp->vif->mbss)
- brcmf_remove_interface(ifp);
return 0;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH next] brcmfmac: don't remove interface on link down firmware event
2016-05-29 21:39 [PATCH next] brcmfmac: don't remove interface on link down firmware event Rafał Miłecki
@ 2016-05-29 22:01 ` kbuild test robot
2016-05-30 4:40 ` [PATCH V2 " Rafał Miłecki
1 sibling, 0 replies; 4+ messages in thread
From: kbuild test robot @ 2016-05-29 22:01 UTC (permalink / raw)
To: Rafał Miłecki
Cc: kbuild-all, Kalle Valo, Rafał Miłecki, Brett Rudley,
Arend van Spriel, Franky (Zhenhui) Lin, Hante Meuleman,
Pieter-Paul Giesberts,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
[-- Attachment #1: Type: text/plain, Size: 4690 bytes --]
Hi,
[auto build test WARNING on next-20160527]
url: https://github.com/0day-ci/linux/commits/Rafa-Mi-ecki/brcmfmac-don-t-remove-interface-on-link-down-firmware-event/20160530-054251
config: sparc64-allyesconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 5.3.1-8) 5.3.1 20160205
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sparc64
All warnings (new ones prefixed by >>):
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c: In function 'brcmf_notify_connect_status_ap':
>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5339:19: warning: unused variable 'ifp' [-Wunused-variable]
struct brcmf_if *ifp = netdev_priv(ndev);
^
vim +/ifp +5339 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5323 conn_info->resp_ie_len,
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5324 completed ? WLAN_STATUS_SUCCESS :
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5325 WLAN_STATUS_AUTH_TIMEOUT,
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5326 GFP_KERNEL);
16886735 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-12-05 5327 brcmf_dbg(CONN, "Report connect result - connection %s\n",
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5328 completed ? "succeeded" : "failed");
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5329 }
d96b801f drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-12-05 5330 brcmf_dbg(TRACE, "Exit\n");
12f32370 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Peter Senna Tschudin 2014-05-31 5331 return 0;
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5332 }
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5333
5b435de0 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2011-10-05 5334 static s32
27a68fe3 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-09-27 5335 brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2012-09-27 5336 struct net_device *ndev,
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2012-09-27 5337 const struct brcmf_event_msg *e, void *data)
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2012-09-27 5338 {
a44aa400 drivers/net/wireless/brcm80211/brcmfmac/cfg80211.c Hante Meuleman 2014-12-03 @5339 struct brcmf_if *ifp = netdev_priv(ndev);
7ee29602 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2013-02-06 5340 static int generation;
5c36b99a drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-11-14 5341 u32 event = e->event_code;
5c36b99a drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-11-14 5342 u32 reason = e->reason;
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2012-09-27 5343 struct station_info sinfo;
1a873342 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Hante Meuleman 2012-09-27 5344
16886735 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2012-12-05 5345 brcmf_dbg(CONN, "event %d, reason %d\n", event, reason);
5f4f9f11 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2013-02-08 5346 if (event == BRCMF_E_LINK && reason == BRCMF_E_REASON_LINK_BSSCFG_DIS &&
5f4f9f11 drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c Arend van Spriel 2013-02-08 5347 ndev != cfg_to_ndev(cfg)) {
:::::: The code at line 5339 was first introduced by commit
:::::: a44aa4001a86d46f936ca449e5d6c268446bfae2 brcmfmac: add multiple BSS support.
:::::: TO: Hante Meuleman <meuleman@broadcom.com>
:::::: CC: John W. Linville <linville@tuxdriver.com>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 46462 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH V2 next] brcmfmac: don't remove interface on link down firmware event
2016-05-29 21:39 [PATCH next] brcmfmac: don't remove interface on link down firmware event Rafał Miłecki
2016-05-29 22:01 ` kbuild test robot
@ 2016-05-30 4:40 ` Rafał Miłecki
2016-06-16 15:12 ` [V2, " Kalle Valo
1 sibling, 1 reply; 4+ messages in thread
From: Rafał Miłecki @ 2016-05-30 4:40 UTC (permalink / raw)
To: Kalle Valo
Cc: Rafał Miłecki, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, Pieter-Paul Giesberts,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
There are two firmware events we handle similarly in brcmfmac:
BRCMF_E_LINK and BRCMF_E_IF. The difference from firmware point of view
is that the first one means BSS remains present in the firmware. Trying
to (re)create it (e.g. when adding new virtual interface) will result in
an error.
Current code treats both events in a similar way. It removes Linux
interface for each of them. It works OK with e.g. BCM43602. Its firmware
generates both events for each interface. It means we get BRCMF_E_LINK
and remove interface. That is soon followed by BRCMF_E_IF which means
BSS was also removed in a firmware. The only downside of this is a
harmless error like:
[ 208.643180] brcmfmac: brcmf_fweh_call_event_handler: no interface object
Unfortunately BCM4366 firmware doesn't automatically remove BSS and so
it doesn't generate BRCMF_E_IF. In such case we incorrectly remove Linux
interface on BRCMF_E_LINK as BSS is still present in the firmware. It
results in an error when trying to re-create virtual interface, e.g.:
> iw phy phy1 interface add wlan1-1 type __ap
[ 3602.929199] brcmfmac: brcmf_ap_add_vif: timeout occurred
command failed: I/O error (-5)
With this patch we don't remove Linux interface while firmware keeps
BSS. Thanks to this we keep a consistent states of host driver and
device firmware.
Further improvement should be to mark BSS as disabled and remove
interface on BRCMF_E_LINK. Then we should add support for reusing
BSS-es.
Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
---
V2: Fix drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5431:19: warning: unused variable ‘ifp’ [-Wunused-variable]
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 38df1be..d23b95e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -5428,7 +5428,6 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
struct net_device *ndev,
const struct brcmf_event_msg *e, void *data)
{
- struct brcmf_if *ifp = netdev_priv(ndev);
static int generation;
u32 event = e->event_code;
u32 reason = e->reason;
@@ -5439,8 +5438,6 @@ brcmf_notify_connect_status_ap(struct brcmf_cfg80211_info *cfg,
ndev != cfg_to_ndev(cfg)) {
brcmf_dbg(CONN, "AP mode link down\n");
complete(&cfg->vif_disabled);
- if (ifp->vif->mbss)
- brcmf_remove_interface(ifp);
return 0;
}
--
1.8.4.5
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [V2, next] brcmfmac: don't remove interface on link down firmware event
2016-05-30 4:40 ` [PATCH V2 " Rafał Miłecki
@ 2016-06-16 15:12 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2016-06-16 15:12 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rafał Miłecki, Brett Rudley, Arend van Spriel,
Franky (Zhenhui) Lin, Hante Meuleman, Pieter-Paul Giesberts,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:BROADCOM BRCM80211 IEEE802.11n WIRELESS DRIVER,
open list:NETWORKING DRIVERS, open list
Rafał Miłecki wrote:
> There are two firmware events we handle similarly in brcmfmac:
> BRCMF_E_LINK and BRCMF_E_IF. The difference from firmware point of view
> is that the first one means BSS remains present in the firmware. Trying
> to (re)create it (e.g. when adding new virtual interface) will result in
> an error.
>
> Current code treats both events in a similar way. It removes Linux
> interface for each of them. It works OK with e.g. BCM43602. Its firmware
> generates both events for each interface. It means we get BRCMF_E_LINK
> and remove interface. That is soon followed by BRCMF_E_IF which means
> BSS was also removed in a firmware. The only downside of this is a
> harmless error like:
> [ 208.643180] brcmfmac: brcmf_fweh_call_event_handler: no interface object
>
> Unfortunately BCM4366 firmware doesn't automatically remove BSS and so
> it doesn't generate BRCMF_E_IF. In such case we incorrectly remove Linux
> interface on BRCMF_E_LINK as BSS is still present in the firmware. It
> results in an error when trying to re-create virtual interface, e.g.:
> > iw phy phy1 interface add wlan1-1 type __ap
> [ 3602.929199] brcmfmac: brcmf_ap_add_vif: timeout occurred
> command failed: I/O error (-5)
>
> With this patch we don't remove Linux interface while firmware keeps
> BSS. Thanks to this we keep a consistent states of host driver and
> device firmware.
>
> Further improvement should be to mark BSS as disabled and remove
> interface on BRCMF_E_LINK. Then we should add support for reusing
> BSS-es.
>
> Signed-off-by: Rafał Miłecki <zajec5@gmail.com>
Thanks, 1 patch applied to wireless-drivers-next.git:
fd3667a8d1cb brcmfmac: don't remove interface on link down firmware event
--
Sent by pwcli
https://patchwork.kernel.org/patch/9140237/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-16 15:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-29 21:39 [PATCH next] brcmfmac: don't remove interface on link down firmware event Rafał Miłecki
2016-05-29 22:01 ` kbuild test robot
2016-05-30 4:40 ` [PATCH V2 " Rafał Miłecki
2016-06-16 15:12 ` [V2, " Kalle Valo
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).