Skip to contentSkip to conversation

brcm80211: remove multiple calls to wl_release_fw

PATCH In review Stanislav Fomichev 4 participants 9 messages Feb 12, 2011 – Feb 19, 2011 Staging

lore
Share & tools
lore

Stanislav Fomichev

author
#0 lore raw

PATCH 1/1 brcm80211: remove multiple calls to wl_release_fw

This causes an oops when the bcm43xx_hdr-0.fw binary is not found.

Signed-off-by Stanislav Fomichev <[email protected]>
---
 drivers/staging/brcm80211/sys/wl_mac80211.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/staging/brcm80211/sys/wl_mac80211.c +0 -2
index cd8392b..5faa521 100644
@@ -1767,7 +1767,6 @@ static int wl_request_fw(struct wl_info *wl, struct pci_dev *pdev)
 		if (status) {
 			printf("%s: fail to load firmware %s\n",
 				KBUILD_MODNAME, fw_name);
-			wl_release_fw(wl);
 			return status;
 		}
 		WL_NONE("request fw %s\n", fw_name);

@@ -1777,7 +1776,6 @@ static int wl_request_fw(struct wl_info *wl, struct pci_dev *pdev)
 		if (status) {
 			printf("%s: fail to load firmware %s\n",
 				KBUILD_MODNAME, fw_name);
-			wl_release_fw(wl);
 			return status;
 		}
 		wl->fw.hdr_num_entries[i] =
Signature · 1 line
1.7.1
Reviewed-by on v1 1/1
#1 lore raw

Hi Stanislav,

this patch is ok with me. For future patches, can you post to [email protected] since that is the main list for the brcm80211 driver ?

I will align yet unsubmitted patches having to do with firmware handling with your patch.

Thanks, Roland.

Reviewed-by Roland Vossen <[email protected]>
Quoted 27 lines · Stanislav Fomichev

This causes an oops when the bcm43xx_hdr-0.fw binary is not found.

Signed-off-by Stanislav Fomichev<[email protected]>
---
  drivers/staging/brcm80211/sys/wl_mac80211.c |    2 --
  1 files changed, 0 insertions(+), 2 deletions(-)
drivers/staging/brcm80211/sys/wl_mac80211.c +0 -2
index cd8392b..5faa521 100644
@@ -1767,7 +1767,6 @@ static int wl_request_fw(struct wl_info *wl, struct pci_dev *pdev)
  		if (status) {
  			printf("%s: fail to load firmware %s\n",
  				KBUILD_MODNAME, fw_name);
-			wl_release_fw(wl);
  			return status;
  		}
  		WL_NONE("request fw %s\n", fw_name);

@@ -1777,7 +1776,6 @@ static int wl_request_fw(struct wl_info *wl, struct pci_dev *pdev)
  		if (status) {
  			printf("%s: fail to load firmware %s\n",
  				KBUILD_MODNAME, fw_name);
-			wl_release_fw(wl);
  			return status;
  		}
  		wl->fw.hdr_num_entries[i] =
#2 lore raw
Quoted 3 lines · Stanislav Fomichev
Quoted 2 lines

This causes an oops when the bcm43xx_hdr-0.fw binary is not found.

This patch isn't right. It fixes the oops but it introduces some leaks. (We still need to release firmware that was allocated successfully).

regards,
dan carpenter

#3 lore raw

Hello Dan and Stanislav,

I already had a patch in the making that is a superset of Stanislav's patch. It fixes the leak issue. I agreed with Stanislav's patch since, I made the same (but: more) changes. Stanislav, can we agree that we wait for my patch (I plan on delivering this tomorrow) ?

Thanks, Roland.

Quoted 10 lines · Dan Carpenter
Quoted 3 lines · Stanislav Fomichev
Quoted 2 lines

This causes an oops when the bcm43xx_hdr-0.fw binary is not found.

This patch isn't right. It fixes the oops but it introduces some leaks. (We still need to release firmware that was allocated successfully).

regards,
dan carpenter

author
#4 lore raw

Hi all,

Quoted 5 lines

I already had a patch in the making that is a superset of Stanislav's patch. It fixes the leak issue. I agreed with Stanislav's patch since, I made the same (but: more) changes. Stanislav, can we agree that we wait for my patch (I plan on delivering this tomorrow) ?

It's alright with me. I just wanted to point out an error; so I can wait for a better fix.

Signature · 1 line
	Stas
author
#5 lore raw
In reply to Dan Carpenter
Quoted 2 lines

This patch isn't right. It fixes the oops but it introduces some leaks. (We still need to release firmware that was allocated successfully).

Sorry Dan, but where do you see the leaks? We make a call to wl_request_fw and if returns non-zero call wl_release_fw for a cleanup.

I just removed the call to wl_release_fw in the wl_request_fw itself; so it should not introduce any leak.

Could you pls clarify?

Signature · 1 line
	Stas
#6 lore raw
Quoted 9 lines · Stanislav Fomichev
Quoted 2 lines

This patch isn't right. It fixes the oops but it introduces some leaks. (We still need to release firmware that was allocated successfully).

Sorry Dan, but where do you see the leaks? We make a call to wl_request_fw and if returns non-zero call wl_release_fw for a cleanup.

I just removed the call to wl_release_fw in the wl_request_fw itself; so it should not introduce any leak.

Could you pls clarify?

Ah. Right. I see what you're saying. Your patch looks good to me.

regards,
dan carpenter

author
#8 lore raw
Quoted 1 line

This doesn't apply on linux-next, what tree is it supposed to be for?

I suppose it was based on the Linus's tree (I didn't know that the developing is going on on this list).

Anyway, pls drop this patch; the similar fixes should be done in one of the Roland Vossen's patches.

Signature · 1 line
	Stas