[PATCH 0/7] firmware request changed for udev and other cleanup

[PATCH 0/7] firmware request changed for udev and other cleanup

[Arend van Spriel]

Both drivers have been changed to move firmware request from the init_module
syscall. Other patches are minor cleanup items

This patch series applies to the wireless-next repository.

Arend van Spriel (7):
  brcm80211: fmac: remove unnecessary NULL pointer check
  brcm80211: fmac: remove brcmf_usb_attrib structure
  brcm80211: fmac: use counters in brcmf_bus structure
  brcm80211: fmac: initialize host interface drivers regardless result
  brcm80211: fmac: remove firmware requests from init_module syscall
  brcm80211: smac: remove firmware requests from init_module syscall
  brcm80211: smac: cleanup couple of debug output statements

 .../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c |    4 +--
 drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h  |    4 +-
 .../net/wireless/brcm80211/brcmfmac/dhd_linux.c    |   30 ++++++++++--------
 drivers/net/wireless/brcm80211/brcmfmac/usb.c      |   32 +++++++++----------
 drivers/net/wireless/brcm80211/brcmfmac/usb.h      |   27 +---------------
 drivers/net/wireless/brcm80211/brcmsmac/ampdu.c    |   20 +++++-------
 .../net/wireless/brcm80211/brcmsmac/mac80211_if.c  |   31 ++++++++++++-------
 7 files changed, 64 insertions(+), 84 deletions(-)

-- 
1.7.5.4

[PATCH 1/7] brcm80211: fmac: remove unnecessary NULL pointer check

[Arend van Spriel]

In brcmf_usb_up() the variable devinfo was checked for being
a NULL pointer, but this can not happen. Also the check was done
after dereferencing the pointer. This patch removes the check.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Kan Yan <kanyan@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/usb.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index d4a9e8e..8616961 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -712,9 +712,6 @@ static int brcmf_usb_up(struct device *dev)
 	struct brcmf_usbdev_info *devinfo = brcmf_usb_get_businfo(dev);
 	u16 ifnum;
 
-	if (devinfo == NULL)
-		return -EINVAL;
-
 	if (devinfo->bus_pub.state == BCMFMAC_USB_STATE_UP)
 		return 0;
 
-- 
1.7.5.4

[PATCH 2/7] brcm80211: fmac: remove brcmf_usb_attrib structure

[Arend van Spriel]

Several fields in this structure are only written once or not used
at all. Remaining two fields have been moved and brcmf_usb_attrib
definition has been removed.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Reviewed-by: Kan Yan <kanyan@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/usb.c |   17 +++++++++--------
 drivers/net/wireless/brcm80211/brcmfmac/usb.h |   16 ++--------------
 2 files changed, 11 insertions(+), 22 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index 8616961..0418a40 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -897,8 +897,8 @@ brcmf_usb_dlneeded(struct brcmf_usbdev_info *devinfo)
 			sizeof(struct bootrom_id_le));
 		return false;
 	} else {
-		devinfo->bus_pub.attrib.devid = chipid;
-		devinfo->bus_pub.attrib.chiprev = chiprev;
+		devinfo->bus_pub.devid = chipid;
+		devinfo->bus_pub.chiprev = chiprev;
 	}
 	return true;
 }
@@ -1064,7 +1064,7 @@ static int brcmf_usb_dlstart(struct brcmf_usbdev_info *devinfo, u8 *fw, int len)
 	if (devinfo == NULL)
 		return -EINVAL;
 
-	if (devinfo->bus_pub.attrib.devid == 0xDEAD)
+	if (devinfo->bus_pub.devid == 0xDEAD)
 		return -EINVAL;
 
 	err = brcmf_usb_dl_writeimage(devinfo, fw, len);
@@ -1085,7 +1085,7 @@ static int brcmf_usb_dlrun(struct brcmf_usbdev_info *devinfo)
 	if (!devinfo)
 		return -EINVAL;
 
-	if (devinfo->bus_pub.attrib.devid == 0xDEAD)
+	if (devinfo->bus_pub.devid == 0xDEAD)
 		return -EINVAL;
 
 	/* Check we are runnable */
@@ -1124,18 +1124,19 @@ static bool brcmf_usb_chip_support(int chipid, int chiprev)
 static int
 brcmf_usb_fw_download(struct brcmf_usbdev_info *devinfo)
 {
-	struct brcmf_usb_attrib *attr;
+	int devid, chiprev;
 	int err;
 
 	brcmf_dbg(TRACE, "enter\n");
 	if (devinfo == NULL)
 		return -ENODEV;
 
-	attr = &devinfo->bus_pub.attrib;
+	devid = devinfo->bus_pub.devid;
+	chiprev = devinfo->bus_pub.chiprev;
 
-	if (!brcmf_usb_chip_support(attr->devid, attr->chiprev)) {
+	if (!brcmf_usb_chip_support(devid, chiprev)) {
 		brcmf_dbg(ERROR, "unsupported chip %d rev %d\n",
-			  attr->devid, attr->chiprev);
+			  devid, chiprev);
 		return -EINVAL;
 	}
 
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.h b/drivers/net/wireless/brcm80211/brcmfmac/usb.h
index b31da7b..3377d63 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.h
@@ -50,19 +50,6 @@ struct brcmf_stats {
 
 };
 
-struct brcmf_usb_attrib {
-	int bustype;
-	int vid;
-	int pid;
-	int devid;
-	int chiprev; /* chip revsion number */
-	int mtu;
-	int nchan; /* Data Channels */
-	int has_2nd_bulk_in_ep;
-};
-
-struct brcmf_usbdev_info;
-
 struct brcmf_usbdev {
 	struct brcmf_bus *bus;
 	struct brcmf_usbdev_info *devinfo;
@@ -70,7 +57,8 @@ struct brcmf_usbdev {
 	struct brcmf_stats stats;
 	int ntxq, nrxq, rxsize;
 	u32 bus_mtu;
-	struct brcmf_usb_attrib attrib;
+	int devid;
+	int chiprev; /* chip revsion number */
 };
 
 /* IO Request Block (IRB) */
-- 
1.7.5.4

[PATCH 3/7] brcm80211: fmac: use counters in brcmf_bus structure

[Arend van Spriel]

The usb code defines a structure for counting statistics. However,
it should use the statistics entry provided in brcmf_bus as that is
exposed to the net_device. The usb private statistics counter only
remains with counters for control packets between driver and usb
device.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmfmac/usb.c |    8 ++++----
 drivers/net/wireless/brcm80211/brcmfmac/usb.h |   11 -----------
 2 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index 0418a40..a2a0c45 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -514,9 +514,9 @@ static void brcmf_usb_tx_complete(struct urb *urb)
 
 	brcmf_usb_del_fromq(devinfo, req);
 	if (urb->status == 0)
-		devinfo->bus_pub.stats.tx_packets++;
+		devinfo->bus_pub.bus->dstats.tx_packets++;
 	else
-		devinfo->bus_pub.stats.tx_errors++;
+		devinfo->bus_pub.bus->dstats.tx_errors++;
 
 	dev_kfree_skb(req->skb);
 	req->skb = NULL;
@@ -536,9 +536,9 @@ static void brcmf_usb_rx_complete(struct urb *urb)
 	req->skb = NULL;
 
 	if (urb->status == 0) {
-		devinfo->bus_pub.stats.rx_packets++;
+		devinfo->bus_pub.bus->dstats.rx_packets++;
 	} else {
-		devinfo->bus_pub.stats.rx_errors++;
+		devinfo->bus_pub.bus->dstats.rx_errors++;
 		dev_kfree_skb(skb);
 		brcmf_usb_enq(devinfo, &devinfo->rx_freeq, req);
 		return;
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.h b/drivers/net/wireless/brcm80211/brcmfmac/usb.h
index 3377d63..acfa5e8 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.h
@@ -33,21 +33,10 @@ enum brcmf_usb_pnp_state {
 };
 
 struct brcmf_stats {
-	u32 tx_errors;
-	u32 tx_packets;
-	u32 tx_multicast;
 	u32 tx_ctlpkts;
 	u32 tx_ctlerrs;
-	u32 tx_dropped;
-	u32 tx_flushed;
-	u32 rx_errors;
-	u32 rx_packets;
-	u32 rx_multicast;
 	u32 rx_ctlpkts;
 	u32 rx_ctlerrs;
-	u32 rx_dropped;
-	u32 rx_flushed;
-
 };
 
 struct brcmf_usbdev {
-- 
1.7.5.4

[PATCH 4/7] brcm80211: fmac: initialize host interface drivers regardless result

[Arend van Spriel]

The module init function of brcmfmac calls init functions for SDIO and
USB doing driver registration. This patch removes terminating the module
init when a driver registration for one host interface fails.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 .../net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c |    4 +---
 drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h  |    4 ++--
 .../net/wireless/brcm80211/brcmfmac/dhd_linux.c    |   14 +++-----------
 drivers/net/wireless/brcm80211/brcmfmac/usb.c      |    4 ++--
 4 files changed, 8 insertions(+), 18 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
index 83ca3cc..4688904 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/bcmsdh_sdmmc.c
@@ -604,7 +604,7 @@ void brcmf_sdio_exit(void)
 	sdio_unregister_driver(&brcmf_sdmmc_driver);
 }
 
-int brcmf_sdio_init(void)
+void brcmf_sdio_init(void)
 {
 	int ret;
 
@@ -614,6 +614,4 @@ int brcmf_sdio_init(void)
 
 	if (ret)
 		brcmf_dbg(ERROR, "sdio_register_driver failed: %d\n", ret);
-
-	return ret;
 }
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h b/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
index b7671b3..3669164 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_bus.h
@@ -108,11 +108,11 @@ extern int brcmf_add_if(struct device *dev, int ifidx,
 
 #ifdef CONFIG_BRCMFMAC_SDIO
 extern void brcmf_sdio_exit(void);
-extern int brcmf_sdio_init(void);
+extern void brcmf_sdio_init(void);
 #endif
 #ifdef CONFIG_BRCMFMAC_USB
 extern void brcmf_usb_exit(void);
-extern int brcmf_usb_init(void);
+extern void brcmf_usb_init(void);
 #endif
 
 #endif				/* _BRCMF_BUS_H_ */
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index c4da058..e734556 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -1183,21 +1183,13 @@ exit:
 
 static int __init brcmfmac_init(void)
 {
-	int ret = 0;
-
 #ifdef CONFIG_BRCMFMAC_SDIO
-	ret = brcmf_sdio_init();
-	if (ret)
-		goto fail;
+	brcmf_sdio_init();
 #endif
 #ifdef CONFIG_BRCMFMAC_USB
-	ret = brcmf_usb_init();
-	if (ret)
-		goto fail;
+	brcmf_usb_init();
 #endif
-
-fail:
-	return ret;
+	return 0;
 }
 
 static void __exit brcmfmac_exit(void)
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/usb.c b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
index a2a0c45..8236422 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/usb.c
@@ -1615,7 +1615,7 @@ void brcmf_usb_exit(void)
 	g_image.len = 0;
 }
 
-int brcmf_usb_init(void)
+void brcmf_usb_init(void)
 {
-	return usb_register(&brcmf_usbdrvr);
+	usb_register(&brcmf_usbdrvr);
 }
-- 
1.7.5.4

[PATCH 5/7] brcm80211: fmac: remove firmware requests from init_module syscall

[Arend van Spriel]

As indicated in [1] on netdev mailing list drivers should not block
on the init_module() syscall. This patch defers the actual driver
registration to a workqueue so the init_module() syscall can complete
without delay.

[1] http://article.gmane.org/gmane.linux.network/217729/

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Franky (Zhenhui) Lin <frankyl@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 .../net/wireless/brcm80211/brcmfmac/dhd_linux.c    |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
index e734556..2a1e5ae 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/dhd_linux.c
@@ -1181,7 +1181,7 @@ exit:
 }
 #endif				/* DEBUG */
 
-static int __init brcmfmac_init(void)
+static void brcmf_driver_init(struct work_struct *work)
 {
 #ifdef CONFIG_BRCMFMAC_SDIO
 	brcmf_sdio_init();
@@ -1189,11 +1189,21 @@ static int __init brcmfmac_init(void)
 #ifdef CONFIG_BRCMFMAC_USB
 	brcmf_usb_init();
 #endif
+}
+static DECLARE_WORK(brcmf_driver_work, brcmf_driver_init);
+
+static int __init brcmfmac_module_init(void)
+{
+	if (!schedule_work(&brcmf_driver_work))
+		return -EBUSY;
+
 	return 0;
 }
 
-static void __exit brcmfmac_exit(void)
+static void __exit brcmfmac_module_exit(void)
 {
+	cancel_work_sync(&brcmf_driver_work);
+
 #ifdef CONFIG_BRCMFMAC_SDIO
 	brcmf_sdio_exit();
 #endif
@@ -1202,5 +1212,5 @@ static void __exit brcmfmac_exit(void)
 #endif
 }
 
-module_init(brcmfmac_init);
-module_exit(brcmfmac_exit);
+module_init(brcmfmac_module_init);
+module_exit(brcmfmac_module_exit);
-- 
1.7.5.4

[PATCH 6/7] brcm80211: smac: remove firmware requests from init_module syscall

[Arend van Spriel]

As indicated in [1] on netdev mailing list drivers should not block
on the init_module() syscall. This patch defers the actual driver
registration to a workqueue so the init_module() syscall can complete
without delay.

[1] http://article.gmane.org/gmane.linux.network/217729/

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Alwin Beukers <alwin@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 .../net/wireless/brcm80211/brcmsmac/mac80211_if.c  |   31 ++++++++++++-------
 1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
index fec0f10..569ab8a 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c
@@ -1169,25 +1169,31 @@ static struct bcma_driver brcms_bcma_driver = {
 /**
  * This is the main entry point for the brcmsmac driver.
  *
- * This function determines if a device pointed to by pdev is a WL device,
- * and if so, performs a brcms_attach() on it.
- *
+ * This function is scheduled upon module initialization and
+ * does the driver registration, which result in brcms_bcma_probe()
+ * call resulting in the driver bringup.
  */
-static int __init brcms_module_init(void)
+static void brcms_driver_init(struct work_struct *work)
 {
-	int error = -ENODEV;
+	int error;
 
+	error = bcma_driver_register(&brcms_bcma_driver);
+	if (error)
+		pr_err("%s: register returned %d\n", __func__, error);
+}
+
+static DECLARE_WORK(brcms_driver_work, brcms_driver_init);
+
+static int __init brcms_module_init(void)
+{
 #ifdef DEBUG
 	if (msglevel != 0xdeadbeef)
 		brcm_msg_level = msglevel;
-#endif				/* DEBUG */
-
-	error = bcma_driver_register(&brcms_bcma_driver);
-	pr_err("%s: register returned %d\n", __func__, error);
-	if (!error)
-		return 0;
+#endif
+	if (!schedule_work(&brcms_driver_work))
+		return -EBUSY;
 
-	return error;
+	return 0;
 }
 
 /**
@@ -1199,6 +1205,7 @@ static int __init brcms_module_init(void)
  */
 static void __exit brcms_module_exit(void)
 {
+	cancel_work_sync(&brcms_driver_work);
 	bcma_driver_unregister(&brcms_bcma_driver);
 }
 
-- 
1.7.5.4

[PATCH 7/7] brcm80211: smac: cleanup couple of debug output statements

[Arend van Spriel]

Tidying up some debug statements in brcms_c_ampdu_dotxstatus_complete()
that got broken strings to satisfy checkpatch, but the rules changed.

Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com>
Reviewed-by: Alwin Beukers <alwin@broadcom.com>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
---
 drivers/net/wireless/brcm80211/brcmsmac/ampdu.c |   20 +++++++++-----------
 1 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/ampdu.c b/drivers/net/wireless/brcm80211/brcmsmac/ampdu.c
index dbee696..95b5902 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/ampdu.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/ampdu.c
@@ -959,14 +959,13 @@ brcms_c_ampdu_dotxstatus_complete(struct ampdu_info *ampdu, struct scb *scb,
 		if (supr_status) {
 			update_rate = false;
 			if (supr_status == TX_STATUS_SUPR_BADCH) {
-				wiphy_err(wiphy, "%s: Pkt tx suppressed, "
-					  "illegal channel possibly %d\n",
+				wiphy_err(wiphy,
+					  "%s: Pkt tx suppressed, illegal channel possibly %d\n",
 					  __func__, CHSPEC_CHANNEL(
 					  wlc->default_bss->chanspec));
 			} else {
 				if (supr_status != TX_STATUS_SUPR_FRAG)
-					wiphy_err(wiphy, "%s:"
-						  "supr_status 0x%x\n",
+					wiphy_err(wiphy, "%s: supr_status 0x%x\n",
 						  __func__, supr_status);
 			}
 			/* no need to retry for badch; will fail again */
@@ -988,9 +987,8 @@ brcms_c_ampdu_dotxstatus_complete(struct ampdu_info *ampdu, struct scb *scb,
 			}
 		} else if (txs->phyerr) {
 			update_rate = false;
-			wiphy_err(wiphy, "wl%d: ampdu tx phy "
-				  "error (0x%x)\n", wlc->pub->unit,
-				  txs->phyerr);
+			wiphy_err(wiphy, "%s: ampdu tx phy error (0x%x)\n",
+				  __func__, txs->phyerr);
 
 			if (brcm_msg_level & LOG_ERROR_VAL) {
 				brcmu_prpkt("txpkt (AMPDU)", p);
@@ -1018,10 +1016,10 @@ brcms_c_ampdu_dotxstatus_complete(struct ampdu_info *ampdu, struct scb *scb,
 		ack_recd = false;
 		if (ba_recd) {
 			bindex = MODSUB_POW2(seq, start_seq, SEQNUM_MAX);
-			BCMMSG(wlc->wiphy, "tid %d seq %d,"
-				" start_seq %d, bindex %d set %d, index %d\n",
-				tid, seq, start_seq, bindex,
-				isset(bitmap, bindex), index);
+			BCMMSG(wiphy,
+			       "tid %d seq %d, start_seq %d, bindex %d set %d, index %d\n",
+			       tid, seq, start_seq, bindex,
+			       isset(bitmap, bindex), index);
 			/* if acked then clear bit and free packet */
 			if ((bindex < AMPDU_TX_BA_MAX_WSIZE)
 			    && isset(bitmap, bindex)) {
-- 
1.7.5.4

Re: [PATCH 4/7] brcm80211: fmac: initialize host interface drivers regardless result

[Julian Calaby]

Hi Arend,

On Sat, Mar 3, 2012 at 08:55, Arend van Spriel <arend@broadcom.com> wrote:
> The module init function of brcmfmac calls init functions for SDIO and
> USB doing driver registration. This patch removes terminating the module
> init when a driver registration for one host interface fails.

Would it be better to fail the module load if *none* of them succeed -
there's probably no point in having the module loaded if it's not
actually hooked up to anything.

You could write the init code like this:

static int __init brcmfmac_init(void)
{
       int ret = 0;
       bool loaded = false;

#ifdef CONFIG_BRCMFMAC_SDIO
       ret = brcmf_sdio_init();
       if (!ret)
               loaded = true;

       // An informative message could go here if it fails
#endif
#ifdef CONFIG_BRCMFMAC_USB
       ret = brcmf_usb_init();
       if (!ret)
               loaded = true;

       // An informative message could go here if it fails
#endif

       if (!loaded)
               return ret; // or -ENODEV

       return 0;
}

And you then wouldn't have to change any of the other code.

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/

Re: [PATCH 4/7] brcm80211: fmac: initialize host interface drivers regardless result

[Arend van Spriel]

On 03/04/2012 11:49 PM, Julian Calaby wrote:
> Hi Arend,
>
> On Sat, Mar 3, 2012 at 08:55, Arend van Spriel<arend@broadcom.com>  wrote:
>> The module init function of brcmfmac calls init functions for SDIO and
>> USB doing driver registration. This patch removes terminating the module
>> init when a driver registration for one host interface fails.
>
> Would it be better to fail the module load if *none* of them succeed -
> there's probably no point in having the module loaded if it's not
> actually hooked up to anything.
>

Thanks, Julian

However, the next patch in this series would undo the init code 
behaviour you are proposing. The brcmf_sdio_init() already logs the 
failed driver registration. I will add the same to brcmf_usb_init() in a 
separate patch. Is that ok with you?

Gr. AvS

Re: [PATCH 4/7] brcm80211: fmac: initialize host interface drivers regardless result

[Julian Calaby]

Hi Arend,

On Mon, Mar 5, 2012 at 20:31, Arend van Spriel <arend@broadcom.com> wrote:
> On 03/04/2012 11:49 PM, Julian Calaby wrote:
>>
>> Hi Arend,
>>
>> On Sat, Mar 3, 2012 at 08:55, Arend van Spriel<arend@broadcom.com>  wrote:
>>>
>>> The module init function of brcmfmac calls init functions for SDIO and
>>> USB doing driver registration. This patch removes terminating the module
>>> init when a driver registration for one host interface fails.
>>
>>
>> Would it be better to fail the module load if *none* of them succeed -
>> there's probably no point in having the module loaded if it's not
>> actually hooked up to anything.
>>
>
> Thanks, Julian
>
> However, the next patch in this series would undo the init code behaviour
> you are proposing. The brcmf_sdio_init() already logs the failed driver
> registration. I will add the same to brcmf_usb_init() in a separate patch.
> Is that ok with you?

I only skimmed the patch set and this is one of the bits that stuck
out, I didn't really read what happened in the next patch.

I didn't really care about the actual logging of failure - my point
was more that if both fail then the module's still loaded, rather than
failing and unloading - however, with the firmware loading being moved
out of the module init path, there's no way to detect this anymore, so
I suppose it doesn't matter.

And yes, logging the probe error for USB would be a good thing.

Thanks anyway,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
.Plan: http://sites.google.com/site/juliancalaby/