linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] brcmfmac: rework probe/attach sequence
@ 2019-07-11  9:05 Arend van Spriel
  2019-07-11  9:05 ` [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect" Arend van Spriel
                   ` (6 more replies)
  0 siblings, 7 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

The brcmfmac driver spews some error message upon unloading and Stefan
Wahren was wondering whether it could be cleaned up. Related to this
was a recent fix for NULL pointer deref. That fix introduced a construct
that added to the itch to rework the probe sequence. So this series
reverts commit 5cdb0ef6144f ("brcmfmac: fix NULL pointer derefence during
USB disconnect").

The changes in this series are:
 * reorder brcmf_detach() code.
 * avoid firmware interaction when bus is down.
 * remove strlcpy() before issueing firmware version iovar.

This series applies to the master branch of the wireless-drivers-next
repository.

Arend van Spriel (7):
  Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
  brcmfmac: change the order of things in brcmf_detach()
  brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is
    down
  brcmfmac: clear events in brcmf_fweh_detach() will always fail
  brcmfmac: avoid firmware commands when bus is down
  brcmfmac: simply remove flowring if bus is down
  brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar

 .../wireless/broadcom/brcm80211/brcmfmac/bcdc.c    | 11 ++------
 .../wireless/broadcom/brcm80211/brcmfmac/bcdc.h    |  6 ++---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 23 +++++++++--------
 .../wireless/broadcom/brcm80211/brcmfmac/common.c  |  1 -
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 30 +++++++++++-----------
 .../wireless/broadcom/brcm80211/brcmfmac/fweh.c    |  9 -------
 .../broadcom/brcm80211/brcmfmac/fwsignal.c         | 16 +++---------
 .../broadcom/brcm80211/brcmfmac/fwsignal.h         |  3 +--
 .../wireless/broadcom/brcm80211/brcmfmac/msgbuf.c  |  7 +++++
 .../wireless/broadcom/brcm80211/brcmfmac/proto.c   | 10 ++------
 .../wireless/broadcom/brcm80211/brcmfmac/proto.h   |  3 +--
 11 files changed, 47 insertions(+), 72 deletions(-)

--
1.9.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-24 11:51   ` Kalle Valo
  2019-07-11  9:05 ` [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach() Arend van Spriel
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

This reverts commit 5cdb0ef6144f47440850553579aa923c20a63f23. Subsequent
changes make rework the driver code fixing the issue differently.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c  | 11 ++---------
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h  |  6 ++----
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c  |  4 +---
 .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c  | 16 ++++------------
 .../net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h  |  3 +--
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c | 10 ++--------
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h |  3 +--
 7 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
index 322e913..2c95a08 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -479,18 +479,11 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr)
 	return -ENOMEM;
 }
 
-void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr)
-{
-	struct brcmf_bcdc *bcdc = drvr->proto->pd;
-
-	brcmf_fws_detach_pre_delif(bcdc->fws);
-}
-
-void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr)
+void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr)
 {
 	struct brcmf_bcdc *bcdc = drvr->proto->pd;
 
 	drvr->proto->pd = NULL;
-	brcmf_fws_detach_post_delif(bcdc->fws);
+	brcmf_fws_detach(bcdc->fws);
 	kfree(bcdc);
 }
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
index 102e693..b051d28 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.h
@@ -7,16 +7,14 @@
 
 #ifdef CONFIG_BRCMFMAC_PROTO_BCDC
 int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr);
-void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr);
-void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr);
+void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr);
 void brcmf_proto_bcdc_txflowblock(struct device *dev, bool state);
 void brcmf_proto_bcdc_txcomplete(struct device *dev, struct sk_buff *txp,
 				 bool success);
 struct brcmf_fws_info *drvr_to_fws(struct brcmf_pub *drvr);
 #else
 static inline int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr) { return 0; }
-static void brcmf_proto_bcdc_detach_pre_delif(struct brcmf_pub *drvr) {};
-static inline void brcmf_proto_bcdc_detach_post_delif(struct brcmf_pub *drvr) {}
+static inline void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr) {}
 #endif
 
 #endif /* BRCMFMAC_BCDC_H */
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index bf18491..fda6044 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1314,8 +1314,6 @@ void brcmf_detach(struct device *dev)
 
 	brcmf_bus_change_state(bus_if, BRCMF_BUS_DOWN);
 
-	brcmf_proto_detach_pre_delif(drvr);
-
 	/* make sure primary interface removed last */
 	for (i = BRCMF_MAX_IFS-1; i > -1; i--)
 		brcmf_remove_interface(drvr->iflist[i], false);
@@ -1325,7 +1323,7 @@ void brcmf_detach(struct device *dev)
 
 	brcmf_bus_stop(drvr->bus_if);
 
-	brcmf_proto_detach_post_delif(drvr);
+	brcmf_proto_detach(drvr);
 
 	bus_if->drvr = NULL;
 	wiphy_free(drvr->wiphy);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
index b8452cb..2bd892d 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.c
@@ -2432,25 +2432,17 @@ struct brcmf_fws_info *brcmf_fws_attach(struct brcmf_pub *drvr)
 	return fws;
 
 fail:
-	brcmf_fws_detach_pre_delif(fws);
-	brcmf_fws_detach_post_delif(fws);
+	brcmf_fws_detach(fws);
 	return ERR_PTR(rc);
 }
 
-void brcmf_fws_detach_pre_delif(struct brcmf_fws_info *fws)
+void brcmf_fws_detach(struct brcmf_fws_info *fws)
 {
 	if (!fws)
 		return;
-	if (fws->fws_wq) {
-		destroy_workqueue(fws->fws_wq);
-		fws->fws_wq = NULL;
-	}
-}
 
-void brcmf_fws_detach_post_delif(struct brcmf_fws_info *fws)
-{
-	if (!fws)
-		return;
+	if (fws->fws_wq)
+		destroy_workqueue(fws->fws_wq);
 
 	/* cleanup */
 	brcmf_fws_lock(fws);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
index 10184ee..b486d57 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwsignal.h
@@ -7,8 +7,7 @@
 #define FWSIGNAL_H_
 
 struct brcmf_fws_info *brcmf_fws_attach(struct brcmf_pub *drvr);
-void brcmf_fws_detach_pre_delif(struct brcmf_fws_info *fws);
-void brcmf_fws_detach_post_delif(struct brcmf_fws_info *fws);
+void brcmf_fws_detach(struct brcmf_fws_info *fws);
 void brcmf_fws_debugfs_create(struct brcmf_pub *drvr);
 bool brcmf_fws_queue_skbs(struct brcmf_fws_info *fws);
 bool brcmf_fws_fc_active(struct brcmf_fws_info *fws);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
index e3d1b07..2e911d4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.c
@@ -56,22 +56,16 @@ int brcmf_proto_attach(struct brcmf_pub *drvr)
 	return -ENOMEM;
 }
 
-void brcmf_proto_detach_post_delif(struct brcmf_pub *drvr)
+void brcmf_proto_detach(struct brcmf_pub *drvr)
 {
 	brcmf_dbg(TRACE, "Enter\n");
 
 	if (drvr->proto) {
 		if (drvr->bus_if->proto_type == BRCMF_PROTO_BCDC)
-			brcmf_proto_bcdc_detach_post_delif(drvr);
+			brcmf_proto_bcdc_detach(drvr);
 		else if (drvr->bus_if->proto_type == BRCMF_PROTO_MSGBUF)
 			brcmf_proto_msgbuf_detach(drvr);
 		kfree(drvr->proto);
 		drvr->proto = NULL;
 	}
 }
-
-void brcmf_proto_detach_pre_delif(struct brcmf_pub *drvr)
-{
-	if (drvr->proto && drvr->bus_if->proto_type == BRCMF_PROTO_BCDC)
-		brcmf_proto_bcdc_detach_pre_delif(drvr);
-}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
index 8d55fad..bd08d3a 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/proto.h
@@ -43,8 +43,7 @@ struct brcmf_proto {
 
 
 int brcmf_proto_attach(struct brcmf_pub *drvr);
-void brcmf_proto_detach_pre_delif(struct brcmf_pub *drvr);
-void brcmf_proto_detach_post_delif(struct brcmf_pub *drvr);
+void brcmf_proto_detach(struct brcmf_pub *drvr);
 
 static inline int brcmf_proto_hdrpull(struct brcmf_pub *drvr, bool do_fws,
 				      struct sk_buff *skb,
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach()
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
  2019-07-11  9:05 ` [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect" Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-20 21:26   ` Rafał Miłecki
  2019-07-11  9:05 ` [PATCH 3/7] brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down Arend van Spriel
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

When brcmf_detach() from the bus layer upon rmmod we can no longer
communicate. Hence we will set the bus state to DOWN and cleanup
the event and protocol layer. The network interfaces need to be
deleted before brcmf_cfg80211_detach() because the latter does the
wiphy_unregister() which issues a warning if there are still network
devices linked to the wiphy instance.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../wireless/broadcom/brcm80211/brcmfmac/core.c    | 27 +++++++++++-----------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index fda6044..80d54d2 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -1307,25 +1307,26 @@ void brcmf_detach(struct device *dev)
 	unregister_inet6addr_notifier(&drvr->inet6addr_notifier);
 #endif
 
-	/* stop firmware event handling */
-	brcmf_fweh_detach(drvr);
-	if (drvr->config)
-		brcmf_p2p_detach(&drvr->config->p2p);
-
 	brcmf_bus_change_state(bus_if, BRCMF_BUS_DOWN);
-
-	/* make sure primary interface removed last */
-	for (i = BRCMF_MAX_IFS-1; i > -1; i--)
-		brcmf_remove_interface(drvr->iflist[i], false);
-
-	brcmf_cfg80211_detach(drvr->config);
-	drvr->config = NULL;
-
 	brcmf_bus_stop(drvr->bus_if);
 
+	brcmf_fweh_detach(drvr);
 	brcmf_proto_detach(drvr);
 
+	/* make sure primary interface removed last */
+	for (i = BRCMF_MAX_IFS - 1; i > -1; i--) {
+		if (drvr->iflist[i])
+			brcmf_del_if(drvr, drvr->iflist[i]->bsscfgidx, false);
+	}
+
+	if (drvr->config) {
+		brcmf_p2p_detach(&drvr->config->p2p);
+		brcmf_cfg80211_detach(drvr->config);
+		drvr->config = NULL;
+	}
+
 	bus_if->drvr = NULL;
+
 	wiphy_free(drvr->wiphy);
 }
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/7] brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
  2019-07-11  9:05 ` [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect" Arend van Spriel
  2019-07-11  9:05 ` [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach() Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-11  9:05 ` [PATCH 4/7] brcmfmac: clear events in brcmf_fweh_detach() will always fail Arend van Spriel
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

No point in sending a firmware command when bus is down so make it
conditional checking the state.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 80d54d2..705b8cc 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -579,7 +579,8 @@ static int brcmf_netdev_stop(struct net_device *ndev)
 
 	brcmf_cfg80211_down(ndev);
 
-	brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear", NULL, 0);
+	if (ifp->drvr->bus_if->state == BRCMF_BUS_UP)
+		brcmf_fil_iovar_data_set(ifp, "arp_hostip_clear", NULL, 0);
 
 	brcmf_net_setcarrier(ifp, false);
 
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/7] brcmfmac: clear events in brcmf_fweh_detach() will always fail
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
                   ` (2 preceding siblings ...)
  2019-07-11  9:05 ` [PATCH 3/7] brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-11  9:05 ` [PATCH 5/7] brcmfmac: avoid firmware commands when bus is down Arend van Spriel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

Clearing firmware events in brcmf_fweh_detach() is always failing
because it is called only upon driver remove and communication
with firmware is no longer possible.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
index adedd4f..79c8a85 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -303,16 +303,7 @@ void brcmf_fweh_attach(struct brcmf_pub *drvr)
 void brcmf_fweh_detach(struct brcmf_pub *drvr)
 {
 	struct brcmf_fweh_info *fweh = &drvr->fweh;
-	struct brcmf_if *ifp = brcmf_get_ifp(drvr, 0);
-	s8 eventmask[BRCMF_EVENTING_MASK_LEN];
 
-	if (ifp) {
-		/* clear all events */
-		memset(eventmask, 0, BRCMF_EVENTING_MASK_LEN);
-		(void)brcmf_fil_iovar_data_set(ifp, "event_msgs",
-					       eventmask,
-					       BRCMF_EVENTING_MASK_LEN);
-	}
 	/* cancel the worker */
 	cancel_work_sync(&fweh->event_work);
 	WARN_ON(!list_empty(&fweh->event_q));
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/7] brcmfmac: avoid firmware commands when bus is down
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
                   ` (3 preceding siblings ...)
  2019-07-11  9:05 ` [PATCH 4/7] brcmfmac: clear events in brcmf_fweh_detach() will always fail Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-11  9:05 ` [PATCH 6/7] brcmfmac: simply remove flowring if " Arend van Spriel
  2019-07-11  9:05 ` [PATCH 7/7] brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar Arend van Spriel
  6 siblings, 0 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

Upon rmmod a few attempts are made to inform firmware, but there is
no point as the bus is down and these will fail. Avoid them to keep
the logs clean.

Reported-by: Stefan Wahren <stefan.wahren@i2se.com>
Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 .../broadcom/brcm80211/brcmfmac/cfg80211.c         | 23 ++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index b6d0df3..4e3e9d44 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -1267,17 +1267,21 @@ static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason)
 {
 	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(vif->wdev.wiphy);
 	struct brcmf_pub *drvr = cfg->pub;
+	bool bus_up = drvr->bus_if->state == BRCMF_BUS_UP;
 	s32 err = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
 
 	if (test_and_clear_bit(BRCMF_VIF_STATUS_CONNECTED, &vif->sme_state)) {
-		brcmf_dbg(INFO, "Call WLC_DISASSOC to stop excess roaming\n");
-		err = brcmf_fil_cmd_data_set(vif->ifp,
-					     BRCMF_C_DISASSOC, NULL, 0);
-		if (err) {
-			bphy_err(drvr, "WLC_DISASSOC failed (%d)\n", err);
+		if (bus_up) {
+			brcmf_dbg(INFO, "Call WLC_DISASSOC to stop excess roaming\n");
+			err = brcmf_fil_cmd_data_set(vif->ifp,
+						     BRCMF_C_DISASSOC, NULL, 0);
+			if (err)
+				bphy_err(drvr, "WLC_DISASSOC failed (%d)\n",
+					 err);
 		}
+
 		if ((vif->wdev.iftype == NL80211_IFTYPE_STATION) ||
 		    (vif->wdev.iftype == NL80211_IFTYPE_P2P_CLIENT))
 			cfg80211_disconnected(vif->wdev.netdev, reason, NULL, 0,
@@ -1287,7 +1291,8 @@ static void brcmf_link_down(struct brcmf_cfg80211_vif *vif, u16 reason)
 	clear_bit(BRCMF_SCAN_STATUS_SUPPRESS, &cfg->scan_status);
 	brcmf_btcoex_set_mode(vif, BRCMF_BTCOEX_ENABLED, 0);
 	if (vif->profile.use_fwsup != BRCMF_PROFILE_FWSUP_NONE) {
-		brcmf_set_pmk(vif->ifp, NULL, 0);
+		if (bus_up)
+			brcmf_set_pmk(vif->ifp, NULL, 0);
 		vif->profile.use_fwsup = BRCMF_PROFILE_FWSUP_NONE;
 	}
 	brcmf_dbg(TRACE, "Exit\n");
@@ -4985,18 +4990,16 @@ static int brcmf_cfg80211_get_channel(struct wiphy *wiphy,
 	struct brcmf_cfg80211_info *cfg = wiphy_to_cfg(wiphy);
 	struct net_device *ndev = wdev->netdev;
 	struct brcmf_pub *drvr = cfg->pub;
-	struct brcmf_if *ifp;
 	struct brcmu_chan ch;
 	enum nl80211_band band = 0;
 	enum nl80211_chan_width width = 0;
 	u32 chanspec;
 	int freq, err;
 
-	if (!ndev)
+	if (!ndev || drvr->bus_if->state != BRCMF_BUS_UP)
 		return -ENODEV;
-	ifp = netdev_priv(ndev);
 
-	err = brcmf_fil_iovar_int_get(ifp, "chanspec", &chanspec);
+	err = brcmf_fil_iovar_int_get(netdev_priv(ndev), "chanspec", &chanspec);
 	if (err) {
 		bphy_err(drvr, "chanspec failed (%d)\n", err);
 		return err;
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/7] brcmfmac: simply remove flowring if bus is down
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
                   ` (4 preceding siblings ...)
  2019-07-11  9:05 ` [PATCH 5/7] brcmfmac: avoid firmware commands when bus is down Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  2019-07-11  9:05 ` [PATCH 7/7] brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar Arend van Spriel
  6 siblings, 0 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

When the bus is down, eg. due to rmmod, there is no need to
attempt to inform firmware about it.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
index 241747b..8428be8 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -1398,6 +1398,13 @@ void brcmf_msgbuf_delete_flowring(struct brcmf_pub *drvr, u16 flowid)
 	u8 ifidx;
 	int err;
 
+	/* no need to submit if firmware can not be reached */
+	if (drvr->bus_if->state != BRCMF_BUS_UP) {
+		brcmf_dbg(MSGBUF, "bus down, flowring will be removed\n");
+		brcmf_msgbuf_remove_flowring(msgbuf, flowid);
+		return;
+	}
+
 	commonring = msgbuf->commonrings[BRCMF_H2D_MSGRING_CONTROL_SUBMIT];
 	brcmf_commonring_lock(commonring);
 	ret_ptr = brcmf_commonring_reserve_for_write(commonring);
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/7] brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar
  2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
                   ` (5 preceding siblings ...)
  2019-07-11  9:05 ` [PATCH 6/7] brcmfmac: simply remove flowring if " Arend van Spriel
@ 2019-07-11  9:05 ` Arend van Spriel
  6 siblings, 0 replies; 10+ messages in thread
From: Arend van Spriel @ 2019-07-11  9:05 UTC (permalink / raw)
  To: Kalle Valo; +Cc: linux-wireless, Arend van Spriel

Recently a strcpy() was replaced by strlcpy(). However, the strcpy()
was not needed in the first place. So removing that line of code.

Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
Reviewed-by: Franky Lin <franky.lin@broadcom.com>
Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
index aa89d62..dec25e4 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/common.c
@@ -258,7 +258,6 @@ int brcmf_c_preinit_dcmds(struct brcmf_if *ifp)
 
 	/* query for 'ver' to get version info from firmware */
 	memset(buf, 0, sizeof(buf));
-	strlcpy(buf, "ver", sizeof(buf));
 	err = brcmf_fil_iovar_data_get(ifp, "ver", buf, sizeof(buf));
 	if (err < 0) {
 		bphy_err(drvr, "Retrieving version information failed, %d\n",
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach()
  2019-07-11  9:05 ` [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach() Arend van Spriel
@ 2019-07-20 21:26   ` Rafał Miłecki
  0 siblings, 0 replies; 10+ messages in thread
From: Rafał Miłecki @ 2019-07-20 21:26 UTC (permalink / raw)
  To: Arend van Spriel, Kalle Valo; +Cc: linux-wireless

On 11.07.2019 11:05, Arend van Spriel wrote:
> When brcmf_detach() from the bus layer upon rmmod we can no longer
> communicate. Hence we will set the bus state to DOWN and cleanup
> the event and protocol layer. The network interfaces need to be
> deleted before brcmf_cfg80211_detach() because the latter does the
> wiphy_unregister() which issues a warning if there are still network
> devices linked to the wiphy instance.
> 
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

This fixes a rmmod crash in brcmf_txfinalize() that I reported in the:
brcmfmac: NULL pointer dereference during brcmf_detach() after firmware crash
<b519e746-ddfd-421f-d897-7620d229e4b2@gmail.com>
https://www.spinics.net/lists/linux-wireless/msg182913.html

Tested-by: Rafał Miłecki <rafal@milecki.pl>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
  2019-07-11  9:05 ` [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect" Arend van Spriel
@ 2019-07-24 11:51   ` Kalle Valo
  0 siblings, 0 replies; 10+ messages in thread
From: Kalle Valo @ 2019-07-24 11:51 UTC (permalink / raw)
  To: Arend van Spriel; +Cc: linux-wireless, Arend van Spriel

Arend van Spriel <arend.vanspriel@broadcom.com> wrote:

> This reverts commit 5cdb0ef6144f47440850553579aa923c20a63f23. Subsequent
> changes make rework the driver code fixing the issue differently.
> 
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>

7 patches applied to wireless-drivers-next.git, thanks.

a84a60ccdd65 Revert "brcmfmac: fix NULL pointer derefence during USB disconnect"
14fcfd1cc0c0 brcmfmac: change the order of things in brcmf_detach()
c613085b7494 brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down
c33330ac06fe brcmfmac: clear events in brcmf_fweh_detach() will always fail
1ac11ae949dd brcmfmac: avoid firmware commands when bus is down
e0bfb9601d48 brcmfmac: simply remove flowring if bus is down
4b11c915f00c brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar

-- 
https://patchwork.kernel.org/patch/11039459/

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-07-24 11:51 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-11  9:05 [PATCH 0/7] brcmfmac: rework probe/attach sequence Arend van Spriel
2019-07-11  9:05 ` [PATCH 1/7] Revert "brcmfmac: fix NULL pointer derefence during USB disconnect" Arend van Spriel
2019-07-24 11:51   ` Kalle Valo
2019-07-11  9:05 ` [PATCH 2/7] brcmfmac: change the order of things in brcmf_detach() Arend van Spriel
2019-07-20 21:26   ` Rafał Miłecki
2019-07-11  9:05 ` [PATCH 3/7] brcmfmac: avoid firmware command in brcmf_netdev_open() when bus is down Arend van Spriel
2019-07-11  9:05 ` [PATCH 4/7] brcmfmac: clear events in brcmf_fweh_detach() will always fail Arend van Spriel
2019-07-11  9:05 ` [PATCH 5/7] brcmfmac: avoid firmware commands when bus is down Arend van Spriel
2019-07-11  9:05 ` [PATCH 6/7] brcmfmac: simply remove flowring if " Arend van Spriel
2019-07-11  9:05 ` [PATCH 7/7] brcmfmac: remove unnecessary strlcpy() upon obtaining "ver" iovar Arend van Spriel

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).