Search Linux Wireless

Re: [PATCH 2/5] brcmfmac: move brcmf_fws_deinit to bcdc layer

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/28/2017 12:43 PM, Arend van Spriel wrote:
From: Franky Lin <franky.lin@xxxxxxxxxxxx>

Move brcmf_fws_deinit into brcmf_proto_bcdc_detach since it is a bcdc
exclusive feature.

Signed-off-by: Franky Lin <franky.lin@xxxxxxxxxxxx>
Change-Id: Iefa5db632b92185a49d538b1cd25b7d8be618ce0
Reviewed-on: http://hnd-swgit.sj.broadcom.com:8080/8157
Reviewed-by: brcm80211 ci <brcm80211-ci@xxxxxxxxxxxx>
Reviewed-by: Arend Van Spriel <arend.vanspriel@xxxxxxxxxxxx>
Signed-off-by: Arend van Spriel <arend.vanspriel@xxxxxxxxxxxx>
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c | 1 +
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c | 7 -------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
index bc24b00..67a132c1 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcdc.c
@@ -464,6 +464,7 @@ int brcmf_proto_bcdc_attach(struct brcmf_pub *drvr)

 void brcmf_proto_bcdc_detach(struct brcmf_pub *drvr)
 {
+	brcmf_fws_deinit(drvr);
 	kfree(drvr->proto->pd);
 	drvr->proto->pd = NULL;
 }

I'd appreciate you commenting on someones work. I wouldn't mind "Move deinit to
brcmf_proto_bcdc_detach" comment so I could update my
[PATCH] brcmfmac: wrap brcmf_fws_(de)init into bcdc layer
if that is so important.


diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
index 9886280..ba4da48 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -32,7 +32,6 @@
 #include "p2p.h"
 #include "cfg80211.h"
 #include "fwil.h"
-#include "fwsignal.h"
 #include "feature.h"
 #include "proto.h"
 #include "pcie.h"
@@ -1034,10 +1033,6 @@ int brcmf_bus_started(struct device *dev)
 		brcmf_cfg80211_detach(drvr->config);
 		drvr->config = NULL;
 	}
-	if (drvr->fws) {
-		brcmf_proto_del_if(ifp->drvr, ifp);
-		brcmf_fws_deinit(drvr);
-	}
 	brcmf_net_detach(ifp->ndev, false);
 	if (p2p_ifp)
 		brcmf_net_detach(p2p_ifp->ndev, false);

I don't like this. By removing del_if from error path you assume add_if doesn't
allocate any memory and it never will.

It's quite hacky for me. If you init something, then you should also deinit it.
Otherwise it's too easy to introduce an invalid state or add a memory leak.
Please keep code simple to follow.



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux