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.