brcm80211: Fix some initialisation failure paths

PATCH In review Ben Hutchings 3 participants 3 messages Sep 10, 2010 – Sep 11, 2010 Staging

lore

Share & tools

lore

Ben Hutchings

author

2010-09-10 00:38 UTC #0 lore raw

PATCH 1/1 brcm80211: Fix some initialisation failure paths

Initialise wl_info::tasklet early so that it's safe to tasklet_kill() it in wl_free().

Remove assertions from wl_free() that may not be true in case of initialisation failure.

Call wl_release_fw() in case of failure after wl_request_fw(). Don't rely on wl_firmware::fw_cnt in wl_release_fw().

Signed-off-by Ben Hutchings <[email protected]>

---
This is compile-tested only; I don't have any of the supported hardware myself.

Ben.

 drivers/staging/brcm80211/TODO              |    1 -
 drivers/staging/brcm80211/sys/wl_mac80211.c |   25 ++++++++++++-------------
 2 files changed, 12 insertions(+), 14 deletions(-)

drivers/staging/brcm80211/TODO +0 -1

index aa38d49..5870bca 100644
@@ -25,7 +25,6 @@ Bugs
 - Various occasional asserts/hangs
 - Scanning during data transfer sometimes causes major slowdowns.  Sometimes
   revcovers when scan is done, other times not.
-- Driver does not handle missing firmware gracefully.
 - Mac80211 API not completely implemented (ie ops_bss_info_changed,
   ops_get_stats, etc)

drivers/staging/brcm80211/sys/wl_mac80211.c +12 ~~-13~~

index d73ec44..3e0c450 100644
@@ -837,6 +837,9 @@ static wl_info_t *wl_attach(uint16 vendor, uint16 device, ulong regs,
 	wl->osh = osh;
 	atomic_set(&wl->callbacks, 0);
 
+	/* setup the bottom half handler */
+	tasklet_init(&wl->tasklet, wl_dpc, (ulong) wl);
+
 #ifdef WLC_HIGH_ONLY
 	wl->rpc_th = bcm_rpc_tp_attach(osh, NULL);
 	if (wl->rpc_th == NULL) {

@@ -905,17 +908,16 @@ static wl_info_t *wl_attach(uint16 vendor, uint16 device, ulong regs,
 #endif
 
 	/* common load-time initialization */
-	if (!
-	    (wl->wlc =
-	     wlc_attach((void *)wl, vendor, device, unit, wl->piomode, osh,
-			wl->regsva, wl->bcm_bustype, btparam, &err))) {
+	wl->wlc = wlc_attach((void *)wl, vendor, device, unit, wl->piomode, osh,
+			     wl->regsva, wl->bcm_bustype, btparam, &err);
+#ifndef WLC_HIGH_ONLY
+	wl_release_fw(wl);
+#endif
+	if (!wl->wlc) {
 		printf("%s: %s driver failed with code %d\n", KBUILD_MODNAME,
 		       EPI_VERSION_STR, err);
 		goto fail;
 	}
-#ifndef WLC_HIGH_ONLY
-	wl_release_fw(wl);
-#endif
 	wl->pub = wlc_pub(wl->wlc);
 
 	wl->pub->ieee_hw = hw;

@@ -942,9 +944,6 @@ static wl_info_t *wl_attach(uint16 vendor, uint16 device, ulong regs,
 	wlc_iovar_setint(wl->wlc, "sd_drivestrength", sd_drivestrength);
 #endif
 
-	/* setup the bottom half handler */
-	tasklet_init(&wl->tasklet, wl_dpc, (ulong) wl);
-
 #ifdef WLC_LOW
 	/* register our interrupt handler */
 	if (request_irq(irq, wl_isr, IRQF_SHARED, KBUILD_MODNAME, wl)) {

@@ -1710,11 +1709,9 @@ void wl_free(wl_info_t * wl)
 
 	ASSERT(wl);
 #ifndef WLC_HIGH_ONLY
-	ASSERT(wl->irq);	/* bmac does not use direct interrupt */
 	/* free ucode data */
 	if (wl->fw.fw_cnt)
 		wl_ucode_data_free();
-	ASSERT(wl->wlc);
 	if (wl->irq)
 		free_irq(wl->irq, wl);
 #endif

@@ -2509,6 +2506,7 @@ static int wl_request_fw(wl_info_t * wl, struct pci_dev *pdev)
 		status = request_firmware(&wl->fw.fw_bin[i], fw_name, device);
 		if (status) {
 			printf("fail to request firmware %s\n", fw_name);
+			wl_release_fw(wl);
 			return status;
 		}
 		WL_NONE(("request fw %s\n", fw_name));

@@ -2517,6 +2515,7 @@ static int wl_request_fw(wl_info_t * wl, struct pci_dev *pdev)
 		status = request_firmware(&wl->fw.fw_hdr[i], fw_name, device);
 		if (status) {
 			printf("fail to request firmware %s\n", fw_name);
+			wl_release_fw(wl);
 			return status;
 		}
 		wl->fw.hdr_num_entries[i] =

@@ -2539,7 +2538,7 @@ void wl_ucode_free_buf(void *p)
 static void wl_release_fw(wl_info_t * wl)
 {
 	int i;
-	for (i = 0; i < wl->fw.fw_cnt; i++) {
+	for (i = 0; i < WL_MAX_FW; i++) {
 		release_firmware(wl->fw.fw_bin[i]);
 		release_firmware(wl->fw.fw_hdr[i]);
 	}

Signature · 1 line

1.7.1

T3VyIGZpcnN0IHBhdGNoISAgTG9va3MgZ29vZCwgdGhhbmtzLg0KDQpCcmV0dA0KDQo+IC0tLS0t T3JpZ2luYWwgTWVzc2FnZS0tLS0tDQo+IEZyb206IEJlbiBIdXRjaGluZ3MgW21haWx0bzpiZW5A ZGVjYWRlbnQub3JnLnVrXQ0KPiBTZW50OiBUaHVyc2RheSwgU2VwdGVtYmVyIDA5LCAyMDEwIDU6 MzggUE0NCj4gVG86IEdyZWcgS3JvYWgtSGFydG1hbg0KPiBDYzogQnJldHQgUnVkbGV5OyBIZW5y eSBQdGFzaW5za2k7IE5vaGVlIEtvOyBsaW51eC0NCj4gd2lyZWxlc3NAdmdlci5rZXJuZWwub3Jn DQo+IFN1YmplY3Q6IFtQQVRDSF0gYnJjbTgwMjExOiBGaXggc29tZSBpbml0aWFsaXNhdGlvbiBm YWlsdXJlIHBhdGhzDQo+IA0KPiBJbml0aWFsaXNlIHdsX2luZm86OnRhc2tsZXQgZWFybHkgc28g dGhhdCBpdCdzIHNhZmUgdG8NCj4gdGFza2xldF9raWxsKCkNCj4gaXQgaW4gd2xfZnJlZSgpLg0K PiANCj4gUmVtb3ZlIGFzc2VydGlvbnMgZnJvbSB3bF9mcmVlKCkgdGhhdCBtYXkgbm90IGJlIHRy dWUgaW4gY2FzZSBvZg0KPiBpbml0aWFsaXNhdGlvbiBmYWlsdXJlLg0KPiANCj4gQ2FsbCB3bF9y ZWxlYXNlX2Z3KCkgaW4gY2FzZSBvZiBmYWlsdXJlIGFmdGVyIHdsX3JlcXVlc3RfZncoKS4NCj4g RG9uJ3QgcmVseSBvbiB3bF9maXJtd2FyZTo6ZndfY250IGluIHdsX3JlbGVhc2VfZncoKS4NCj4g DQo+IFNpZ25lZC1vZmYtYnk6IEJlbiBIdXRjaGluZ3MgPGJlbkBkZWNhZGVudC5vcmcudWs+DQo+ IC0tLQ0KPiBUaGlzIGlzIGNvbXBpbGUtdGVzdGVkIG9ubHk7IEkgZG9uJ3QgaGF2ZSBhbnkgb2Yg dGhlIHN1cHBvcnRlZA0KPiBoYXJkd2FyZQ0KPiBteXNlbGYuDQo+IA0KPiBCZW4uDQo+IA0KPiAg ZHJpdmVycy9zdGFnaW5nL2JyY204MDIxMS9UT0RPICAgICAgICAgICAgICB8ICAgIDEgLQ0KPiAg ZHJpdmVycy9zdGFnaW5nL2JyY204MDIxMS9zeXMvd2xfbWFjODAyMTEuYyB8ICAgMjUgKysrKysr KysrKysrLS0NCj4gLS0tLS0tLS0tLS0NCj4gIDIgZmlsZXMgY2hhbmdlZCwgMTIgaW5zZXJ0aW9u cygrKSwgMTQgZGVsZXRpb25zKC0pDQo+IA0KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9zdGFnaW5n L2JyY204MDIxMS9UT0RPDQo+IGIvZHJpdmVycy9zdGFnaW5nL2JyY204MDIxMS9UT0RPDQo+IGlu ZGV4IGFhMzhkNDkuLjU4NzBiY2EgMTAwNjQ0DQo+IC0tLSBhL2RyaXZlcnMvc3RhZ2luZy9icmNt ODAyMTEvVE9ETw0KPiArKysgYi9kcml2ZXJzL3N0YWdpbmcvYnJjbTgwMjExL1RPRE8NCj4gQEAg LTI1LDcgKzI1LDYgQEAgQnVncw0KPiAgLSBWYXJpb3VzIG9jY2FzaW9uYWwgYXNzZXJ0cy9oYW5n cw0KPiAgLSBTY2FubmluZyBkdXJpbmcgZGF0YSB0cmFuc2ZlciBzb21ldGltZXMgY2F1c2VzIG1h am9yIHNsb3dkb3ducy4NCj4gU29tZXRpbWVzDQo+ICAgIHJldmNvdmVycyB3aGVuIHNjYW4gaXMg ZG9uZSwgb3RoZXIgdGltZXMgbm90Lg0KPiAtLSBEcml2ZXIgZG9lcyBub3QgaGFuZGxlIG1pc3Np bmcgZmlybXdhcmUgZ3JhY2VmdWxseS4NCj4gIC0gTWFjODAyMTEgQVBJIG5vdCBjb21wbGV0ZWx5 IGltcGxlbWVudGVkIChpZQ0KPiBvcHNfYnNzX2luZm9fY2hhbmdlZCwNCj4gICAgb3BzX2dldF9z dGF0cywgZXRjKQ0KPiANCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvc3RhZ2luZy9icmNtODAyMTEv c3lzL3dsX21hYzgwMjExLmMNCj4gYi9kcml2ZXJzL3N0YWdpbmcvYnJjbTgwMjExL3N5cy93bF9t YWM4MDIxMS5jDQo+IGluZGV4IGQ3M2VjNDQuLjNlMGM0NTAgMTAwNjQ0DQo+IC0tLSBhL2RyaXZl cnMvc3RhZ2luZy9icmNtODAyMTEvc3lzL3dsX21hYzgwMjExLmMNCj4gKysrIGIvZHJpdmVycy9z dGFnaW5nL2JyY204MDIxMS9zeXMvd2xfbWFjODAyMTEuYw0KPiBAQCAtODM3LDYgKzgzNyw5IEBA IHN0YXRpYyB3bF9pbmZvX3QgKndsX2F0dGFjaCh1aW50MTYgdmVuZG9yLA0KPiB1aW50MTYgZGV2 aWNlLCB1bG9uZyByZWdzLA0KPiAgCXdsLT5vc2ggPSBvc2g7DQo+ICAJYXRvbWljX3NldCgmd2wt PmNhbGxiYWNrcywgMCk7DQo+IA0KPiArCS8qIHNldHVwIHRoZSBib3R0b20gaGFsZiBoYW5kbGVy ICovDQo+ICsJdGFza2xldF9pbml0KCZ3bC0+dGFza2xldCwgd2xfZHBjLCAodWxvbmcpIHdsKTsN Cj4gKw0KPiAgI2lmZGVmIFdMQ19ISUdIX09OTFkNCj4gIAl3bC0+cnBjX3RoID0gYmNtX3JwY190 cF9hdHRhY2gob3NoLCBOVUxMKTsNCj4gIAlpZiAod2wtPnJwY190aCA9PSBOVUxMKSB7DQo+IEBA IC05MDUsMTcgKzkwOCwxNiBAQCBzdGF0aWMgd2xfaW5mb190ICp3bF9hdHRhY2godWludDE2IHZl bmRvciwNCj4gdWludDE2IGRldmljZSwgdWxvbmcgcmVncywNCj4gICNlbmRpZg0KPiANCj4gIAkv KiBjb21tb24gbG9hZC10aW1lIGluaXRpYWxpemF0aW9uICovDQo+IC0JaWYgKCENCj4gLQkgICAg KHdsLT53bGMgPQ0KPiAtCSAgICAgd2xjX2F0dGFjaCgodm9pZCAqKXdsLCB2ZW5kb3IsIGRldmlj ZSwgdW5pdCwgd2wtDQo+ID5waW9tb2RlLCBvc2gsDQo+IC0JCQl3bC0+cmVnc3ZhLCB3bC0+YmNt X2J1c3R5cGUsIGJ0cGFyYW0sICZlcnIpKSkNCj4gew0KPiArCXdsLT53bGMgPSB3bGNfYXR0YWNo KCh2b2lkICopd2wsIHZlbmRvciwgZGV2aWNlLCB1bml0LCB3bC0NCj4gPnBpb21vZGUsIG9zaCwN Cj4gKwkJCSAgICAgd2wtPnJlZ3N2YSwgd2wtPmJjbV9idXN0eXBlLCBidHBhcmFtLA0KPiAmZXJy KTsNCj4gKyNpZm5kZWYgV0xDX0hJR0hfT05MWQ0KPiArCXdsX3JlbGVhc2VfZncod2wpOw0KPiAr I2VuZGlmDQo+ICsJaWYgKCF3bC0+d2xjKSB7DQo+ICAJCXByaW50ZigiJXM6ICVzIGRyaXZlciBm YWlsZWQgd2l0aCBjb2RlICVkXG4iLA0KPiBLQlVJTERfTU9ETkFNRSwNCj4gIAkJICAgICAgIEVQ SV9WRVJTSU9OX1NUUiwgZXJyKTsNCj4gIAkJZ290byBmYWlsOw0KPiAgCX0NCj4gLSNpZm5kZWYg V0xDX0hJR0hfT05MWQ0KPiAtCXdsX3JlbGVhc2VfZncod2wpOw0KPiAtI2VuZGlmDQo+ICAJd2wt PnB1YiA9IHdsY19wdWIod2wtPndsYyk7DQo+IA0KPiAgCXdsLT5wdWItPmllZWVfaHcgPSBodzsN Cj4gQEAgLTk0Miw5ICs5NDQsNiBAQCBzdGF0aWMgd2xfaW5mb190ICp3bF9hdHRhY2godWludDE2 IHZlbmRvciwNCj4gdWludDE2IGRldmljZSwgdWxvbmcgcmVncywNCj4gIAl3bGNfaW92YXJfc2V0 aW50KHdsLT53bGMsICJzZF9kcml2ZXN0cmVuZ3RoIiwNCj4gc2RfZHJpdmVzdHJlbmd0aCk7DQo+ ICAjZW5kaWYNCj4gDQo+IC0JLyogc2V0dXAgdGhlIGJvdHRvbSBoYWxmIGhhbmRsZXIgKi8NCj4g LQl0YXNrbGV0X2luaXQoJndsLT50YXNrbGV0LCB3bF9kcGMsICh1bG9uZykgd2wpOw0KPiAtDQo+ ICAjaWZkZWYgV0xDX0xPVw0KPiAgCS8qIHJlZ2lzdGVyIG91ciBpbnRlcnJ1cHQgaGFuZGxlciAq Lw0KPiAgCWlmIChyZXF1ZXN0X2lycShpcnEsIHdsX2lzciwgSVJRRl9TSEFSRUQsIEtCVUlMRF9N T0ROQU1FLA0KPiB3bCkpIHsNCj4gQEAgLTE3MTAsMTEgKzE3MDksOSBAQCB2b2lkIHdsX2ZyZWUo d2xfaW5mb190ICogd2wpDQo+IA0KPiAgCUFTU0VSVCh3bCk7DQo+ICAjaWZuZGVmIFdMQ19ISUdI X09OTFkNCj4gLQlBU1NFUlQod2wtPmlycSk7CS8qIGJtYWMgZG9lcyBub3QgdXNlIGRpcmVjdCBp bnRlcnJ1cHQNCj4gKi8NCj4gIAkvKiBmcmVlIHVjb2RlIGRhdGEgKi8NCj4gIAlpZiAod2wtPmZ3 LmZ3X2NudCkNCj4gIAkJd2xfdWNvZGVfZGF0YV9mcmVlKCk7DQo+IC0JQVNTRVJUKHdsLT53bGMp Ow0KPiAgCWlmICh3bC0+aXJxKQ0KPiAgCQlmcmVlX2lycSh3bC0+aXJxLCB3bCk7DQo+ICAjZW5k aWYNCj4gQEAgLTI1MDksNiArMjUwNiw3IEBAIHN0YXRpYyBpbnQgd2xfcmVxdWVzdF9mdyh3bF9p bmZvX3QgKiB3bCwNCj4gc3RydWN0IHBjaV9kZXYgKnBkZXYpDQo+ICAJCXN0YXR1cyA9IHJlcXVl c3RfZmlybXdhcmUoJndsLT5mdy5md19iaW5baV0sDQo+IGZ3X25hbWUsIGRldmljZSk7DQo+ICAJ CWlmIChzdGF0dXMpIHsNCj4gIAkJCXByaW50ZigiZmFpbCB0byByZXF1ZXN0IGZpcm13YXJlICVz XG4iLA0KPiBmd19uYW1lKTsNCj4gKwkJCXdsX3JlbGVhc2VfZncod2wpOw0KPiAgCQkJcmV0dXJu IHN0YXR1czsNCj4gIAkJfQ0KPiAgCQlXTF9OT05FKCgicmVxdWVzdCBmdyAlc1xuIiwgZndfbmFt ZSkpOw0KPiBAQCAtMjUxNyw2ICsyNTE1LDcgQEAgc3RhdGljIGludCB3bF9yZXF1ZXN0X2Z3KHds X2luZm9fdCAqIHdsLA0KPiBzdHJ1Y3QgcGNpX2RldiAqcGRldikNCj4gIAkJc3RhdHVzID0gcmVx dWVzdF9maXJtd2FyZSgmd2wtPmZ3LmZ3X2hkcltpXSwNCj4gZndfbmFtZSwgZGV2aWNlKTsNCj4g IAkJaWYgKHN0YXR1cykgew0KPiAgCQkJcHJpbnRmKCJmYWlsIHRvIHJlcXVlc3QgZmlybXdhcmUg JXNcbiIsDQo+IGZ3X25hbWUpOw0KPiArCQkJd2xfcmVsZWFzZV9mdyh3bCk7DQo+ICAJCQlyZXR1 cm4gc3RhdHVzOw0KPiAgCQl9DQo+ICAJCXdsLT5mdy5oZHJfbnVtX2VudHJpZXNbaV0gPQ0KPiBA QCAtMjUzOSw3ICsyNTM4LDcgQEAgdm9pZCB3bF91Y29kZV9mcmVlX2J1Zih2b2lkICpwKQ0KPiAg c3RhdGljIHZvaWQgd2xfcmVsZWFzZV9mdyh3bF9pbmZvX3QgKiB3bCkNCj4gIHsNCj4gIAlpbnQg aTsNCj4gLQlmb3IgKGkgPSAwOyBpIDwgd2wtPmZ3LmZ3X2NudDsgaSsrKSB7DQo+ICsJZm9yIChp ID0gMDsgaSA8IFdMX01BWF9GVzsgaSsrKSB7DQo+ICAJCXJlbGVhc2VfZmlybXdhcmUod2wtPmZ3 LmZ3X2JpbltpXSk7DQo+ICAJCXJlbGVhc2VfZmlybXdhcmUod2wtPmZ3LmZ3X2hkcltpXSk7DQo+ ICAJfQ0KPiAtLQ0KPiAxLjcuMQ0KPiANCj4gDQoNCg==

Henry Ptasinski

2010-09-11 17:10 UTC #2 lore raw

In reply to Ben Hutchings

Reviewed, and tested with a BCM43224.

Signed-off-by Henry Ptasinski < [email protected]>

________________________________________
From: Ben Hutchings [[email protected]]
Sent: Thursday, September 09, 2010 5:38 PM
To: Greg Kroah-Hartman
Cc: Brett Rudley; Henry Ptasinski; Nohee Ko; [email protected]
Subject: [PATCH] brcm80211: Fix some initialisation failure paths