[PATCH 2/2] staging: brcm80211: replace asserts close to Mac80211 interface

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

 



Code cleanup. Asserts have been replaced by less lethal WARN_ON's.
These WARN_ONs are a sanity check for the messages that Mac80211
gives to the driver, and vice versa.

Signed-off-by: Roland Vossen <rvossen@xxxxxxxxxxxx>
Reviewed-by: Arend van Spriel <arend@xxxxxxxxxxxx>
---
 drivers/staging/brcm80211/brcmsmac/wl_mac80211.c |   55 ++++++-----
 drivers/staging/brcm80211/brcmsmac/wlc_main.c    |  104 ++++++++++++---------
 2 files changed, 89 insertions(+), 70 deletions(-)

diff --git a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
index 774b4e9..1ae8e22 100644
--- a/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
+++ b/drivers/staging/brcm80211/brcmsmac/wl_mac80211.c
@@ -184,11 +184,9 @@ static int wl_ops_start(struct ieee80211_hw *hw)
 
 static void wl_ops_stop(struct ieee80211_hw *hw)
 {
-#ifdef BRCMDBG
 	struct wl_info *wl = hw->priv;
-	ASSERT(wl);
-#endif /*BRCMDBG*/
-	ieee80211_stop_queues(hw);
+	if (likely(!WARN_ON(wl == NULL)))
+		ieee80211_stop_queues(hw);
 }
 
 static int
@@ -276,7 +274,7 @@ static int wl_ops_config(struct ieee80211_hw *hw, u32 changed)
 			goto config_out;
 		}
 		wlc_iovar_getint(wl->wlc, "bcn_li_bcn", &new_int);
-		ASSERT(new_int == conf->listen_interval);
+		WARN_ON(new_int != conf->listen_interval);
 	}
 	if (changed & IEEE80211_CONF_CHANGE_MONITOR)
 		WL_ERROR("%s: change monitor mode: %s (implement)\n", __func__,
@@ -614,13 +612,12 @@ wl_ops_ampdu_action(struct ieee80211_hw *hw,
 		    struct ieee80211_sta *sta, u16 tid, u16 *ssn,
 		    u8 buf_size)
 {
-#if defined(BCMDBG)
 	struct scb *scb = (struct scb *)sta->drv_priv;
-#endif
 	struct wl_info *wl = hw->priv;
 	int status;
 
-	ASSERT(scb->magic == SCB_MAGIC);
+	if (unlikely(WARN_ON(scb->magic != SCB_MAGIC)))
+		return -EIDRM;
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
 		WL_NONE("%s: action = IEEE80211_AMPDU_RX_START\n", __func__);
@@ -724,7 +721,7 @@ static int wl_set_hint(struct wl_info *wl, char *abbrev)
 static struct wl_info *wl_attach(u16 vendor, u16 device, unsigned long regs,
 			    uint bustype, void *btparam, uint irq)
 {
-	struct wl_info *wl;
+	struct wl_info *wl = NULL;
 	int unit, err;
 
 	unsigned long base_addr;
@@ -741,8 +738,10 @@ static struct wl_info *wl_attach(u16 vendor, u16 device, unsigned long regs,
 
 	/* allocate private info */
 	hw = pci_get_drvdata(btparam);	/* btparam == pdev */
-	wl = hw->priv;
-	ASSERT(wl);
+	if (likely(hw != NULL))
+		wl = hw->priv;
+	if (unlikely(WARN_ON(hw == NULL || wl == NULL)))
+		goto fail1;
 
 	atomic_set(&wl->callbacks, 0);
 
@@ -792,8 +791,10 @@ static struct wl_info *wl_attach(u16 vendor, u16 device, unsigned long regs,
 	wl->pub = wlc_pub(wl->wlc);
 
 	wl->pub->ieee_hw = hw;
-	ASSERT(wl->pub->ieee_hw);
-	ASSERT(wl->pub->ieee_hw->priv == wl);
+
+	if (unlikely(wl->pub->ieee_hw->priv != wl)) {
+		goto fail;
+	}
 
 
 	if (wlc_iovar_setint(wl->wlc, "mpc", 0)) {
@@ -817,7 +818,7 @@ static struct wl_info *wl_attach(u16 vendor, u16 device, unsigned long regs,
 	}
 
 	memcpy(perm, &wl->pub->cur_etheraddr, ETH_ALEN);
-	ASSERT(is_valid_ether_addr(perm));
+	WARN_ON(!is_valid_ether_addr(perm));
 	SET_IEEE80211_PERM_ADDR(hw, perm);
 
 	err = ieee80211_register_hw(hw);
@@ -1031,7 +1032,7 @@ static int ieee_hw_rate_init(struct ieee80211_hw *hw)
 	}
 	WL_NONE("%s: phylist = %c\n", __func__, phy_list[0]);
 
-	if (phy_list[0] == 'n' || phy_list[0] == 'c') {
+	if (likely(!WARN_ON(phy_list[0] != 'n' && phy_list[0] != 'c'))) {
 		if (phy_list[0] == 'c') {
 			/* Single stream */
 			wl_band_2GHz_nphy.ht_cap.mcs.rx_mask[1] = 0;
@@ -1039,7 +1040,6 @@ static int ieee_hw_rate_init(struct ieee80211_hw *hw)
 		}
 		hw->wiphy->bands[IEEE80211_BAND_2GHZ] = &wl_band_2GHz_nphy;
 	} else {
-		BUG();
 		return -1;
 	}
 
@@ -1099,13 +1099,13 @@ static int ieee_hw_init(struct ieee80211_hw *hw)
 static int __devinit
 wl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
-	int rc;
+	int rc = -ENODEV;
 	struct wl_info *wl;
 	struct ieee80211_hw *hw;
 	u32 val;
 
-	ASSERT(pdev);
-
+	if (unlikely(pdev == NULL))
+		goto err_1;
 	WL_TRACE("%s: bus %d slot %d func %d irq %d\n",
 		 __func__, pdev->bus->number, PCI_SLOT(pdev->devfn),
 		 PCI_FUNC(pdev->devfn), pdev->irq);
@@ -1121,7 +1121,8 @@ wl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		WL_ERROR("%s: Cannot enable device %d-%d_%d\n",
 			 __func__, pdev->bus->number, PCI_SLOT(pdev->devfn),
 			 PCI_FUNC(pdev->devfn));
-		return -ENODEV;
+		rc = -ENODEV;
+		goto err_1;
 	}
 	pci_set_master(pdev);
 
@@ -1152,8 +1153,8 @@ wl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 	return 0;
  err_1:
-	WL_ERROR("%s: err_1: Major hoarkage\n", __func__);
-	return 0;
+	WL_ERROR("err_1: Major hoarkage\n");
+	return rc;
 }
 
 static int wl_suspend(struct pci_dev *pdev, pm_message_t state)
@@ -1342,7 +1343,8 @@ static void wl_free(struct wl_info *wl)
 {
 	struct wl_timer *t, *next;
 
-	ASSERT(wl);
+	if (unlikely(WARN_ON(wl == NULL)))
+		return;
 	/* free ucode data */
 	if (wl->fw.fw_cnt)
 		wl_ucode_data_free();
@@ -1521,7 +1523,7 @@ static irqreturn_t BCMFASTPATH wl_isr(int irq, void *dev_id)
 
 			/* ...and call the second level interrupt handler */
 			/* schedule dpc */
-			ASSERT(wl->resched == false);
+			WARN_ON_ONCE(wl->resched == true);
 			tasklet_schedule(&wl->tasklet);
 		}
 	}
@@ -1810,7 +1812,10 @@ int wl_ucode_init_uint(struct wl_info *wl, u32 *data, u32 idx)
 		     entry++, hdr++) {
 			if (hdr->idx == idx) {
 				pdata = wl->fw.fw_bin[i]->data + hdr->offset;
-				ASSERT(hdr->len == 4);
+				if (hdr->len != 4) {
+					WL_ERROR("ERROR: fw hdr len\n");
+					return -1;
+				}
 				*data = *((u32 *) pdata);
 				return 0;
 			}
diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_main.c b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
index 98a5466..e3aa15b 100644
--- a/drivers/staging/brcm80211/brcmsmac/wlc_main.c
+++ b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
@@ -1179,7 +1179,7 @@ void wlc_protection_upd(struct wlc_info *wlc, uint idx, int val)
 		break;
 
 	default:
-		ASSERT(0);
+		WARN_ON(true);
 		break;
 	}
 
@@ -1367,7 +1367,8 @@ void wlc_wme_setparams(struct wlc_info *wlc, u16 aci, void *arg, bool suspend)
 	u16 *shm_entry;
 	struct ieee80211_tx_queue_params *params = arg;
 
-	ASSERT(wlc);
+	if (unlikely(WARN_ON(wlc == NULL || aci >= AC_COUNT)))
+		return;
 
 	/* Only apply params if the core is out of reset and has clocks */
 	if (!wlc->clk) {
@@ -1386,7 +1387,6 @@ void wlc_wme_setparams(struct wlc_info *wlc, u16 aci, void *arg, bool suspend)
 	do {
 		memset((char *)&acp_shm, 0, sizeof(shm_acparams_t));
 		/* find out which ac this set of params applies to */
-		ASSERT(aci < AC_COUNT);
 		/* set the admission control policy for this AC */
 		/* wlc->wme_admctl |= 1 << aci; *//* should be set ??  seems like off by default */
 
@@ -4361,29 +4361,37 @@ wlc_iovar_op(struct wlc_info *wlc, const char *name,
 	     void *params, int p_len, void *arg, int len,
 	     bool set, struct wlc_if *wlcif)
 {
-	int err = 0;
+	int err;
 	int val_size;
 	const bcm_iovar_t *vi = NULL;
 	u32 actionid;
 	int i;
 
-	ASSERT(name != NULL);
-
-	ASSERT(len >= 0);
+	if (name == NULL || len < 0) {
+		WL_ERROR("wl%d: %s name/arg error\n",
+			 wlc->pub->unit, __func__);
+		return BCME_UNSUPPORTED;
+	}
 
-	/* Get MUST have return space */
-	ASSERT(set || (arg && len));
+	if (!set && !(arg && len)) {
+		WL_ERROR("Get MUST have return space\n");
+		goto fail;
+	}
 
-	ASSERT(!(wlc->pub->hw_off && wlc->pub->up));
+	if (wlc->pub->hw_off && wlc->pub->up) {
+		WL_ERROR("hw is off but adapter is up!\n");
+		goto fail;
+	}
 
-	/* Set does NOT take qualifiers */
-	ASSERT(!set || (!params && !p_len));
+	if (set && (params || p_len)) {
+		WL_ERROR("Set does NOT take qualifiers\n");
+		goto fail;
+	}
 
-	if (!set && (len == sizeof(int)) &&
-	    !(IS_ALIGNED((unsigned long)(arg), (uint) sizeof(int)))) {
-		WL_ERROR("wl%d: %s unaligned get ptr for %s\n",
-			 wlc->pub->unit, __func__, name);
-		ASSERT(0);
+	if (!set && len == sizeof(int) &&
+	    !IS_ALIGNED((unsigned long)(arg), (uint) sizeof(int))) {
+		WL_ERROR("unaligned access on get\n");
+		goto fail;
 	}
 
 	/* find the given iovar name */
@@ -4394,10 +4402,10 @@ wlc_iovar_op(struct wlc_info *wlc, const char *name,
 		if (vi)
 			break;
 	}
-	/* iovar name not found */
+
 	if (i >= WLC_MAXMODULES) {
-		err = BCME_UNSUPPORTED;
-		goto exit;
+		WL_ERROR("iovar name not found\n");
+		goto fail;
 	}
 
 	/* set up 'params' pointer in case this is a set command so that
@@ -4423,8 +4431,12 @@ wlc_iovar_op(struct wlc_info *wlc, const char *name,
 					name, params, p_len, arg, len, val_size,
 					wlcif);
 
- exit:
 	return err;
+
+fail:
+	WL_ERROR("wl%d: %s(set=%d,%s) failed\n", wlc->pub->unit, __func__, set,
+						 name);
+	return BCME_UNSUPPORTED;
 }
 
 int
@@ -5173,27 +5185,30 @@ wlc_sendpkt_mac80211(struct wlc_info *wlc, struct sk_buff *sdu,
 	struct scb *scb = &global_scb;
 	struct ieee80211_hdr *d11_header = (struct ieee80211_hdr *)(sdu->data);
 
-	ASSERT(sdu);
-
+	if (unlikely(WARN_ON(sdu == NULL)))
+		goto wlc_sendpkt_fail;
 	/* 802.11 standard requires management traffic to go at highest priority */
 	prio = ieee80211_is_data(d11_header->frame_control) ? sdu->priority :
 		MAXPRIO;
 	fifo = prio2fifo[prio];
 
-	ASSERT((uint) skb_headroom(sdu) >= TXOFF);
-	ASSERT(!(sdu->next));
-	ASSERT(!(sdu->prev));
-	ASSERT(fifo < NFIFO);
+	if (unlikely(WARN_ON((uint)skb_headroom(sdu) < TXOFF ||
+		     sdu->next != NULL || sdu->prev != NULL || fifo >= NFIFO)))
+		goto wlc_sendpkt_fail;
 
 	pkt = sdu;
+
 	if (unlikely
 	    (wlc_d11hdrs_mac80211(wlc, hw, pkt, scb, 0, 1, fifo, 0, NULL, 0)))
-		return -EINVAL;
+		goto wlc_sendpkt_fail;
 	wlc_txq_enq(wlc, scb, pkt, WLC_PRIO_TO_PREC(prio));
 	wlc_send_q(wlc, wlc->active_queue);
 
 	wlc->pub->_cnt->ieee_tx++;
 	return 0;
+
+wlc_sendpkt_fail:
+	return -EINVAL;
 }
 
 void BCMFASTPATH wlc_send_q(struct wlc_info *wlc, struct wlc_txq_info *qi)
@@ -5740,8 +5755,6 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 	u16 mimo_txbw;
 	u8 mimo_preamble_type;
 
-	ASSERT(queue < NFIFO);
-
 	/* locate 802.11 MAC header */
 	h = (struct ieee80211_hdr *)(p->data);
 	qos = ieee80211_is_data_qos(h->frame_control);
@@ -5761,7 +5774,8 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 
 	/* Get tx_info */
 	tx_info = IEEE80211_SKB_CB(p);
-	ASSERT(tx_info);
+	if (unlikely(tx_info == NULL))
+		return -EINVAL;
 
 	/* add PLCP */
 	plcp = skb_push(p, D11_PHY_HDR_LEN);
@@ -5773,7 +5787,6 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 	/* setup frameid */
 	if (tx_info->flags & IEEE80211_TX_CTL_ASSIGN_SEQ) {
 		/* non-AP STA should never use BCMC queue */
-		ASSERT(queue != TX_BCMC_FIFO);
 		if (queue == TX_BCMC_FIFO) {
 			WL_ERROR("wl%d: %s: ASSERT queue == TX_BCMC!\n",
 				 WLCWLUNIT(wlc), __func__);
@@ -5799,13 +5812,15 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 	if (SCB_PS(scb) || ieee80211_is_beacon(h->frame_control))
 		mcl |= TXC_IGNOREPMQ;
 
-	ASSERT(hw->max_rates <= IEEE80211_TX_MAX_RATES);
-	ASSERT(hw->max_rates == 2);
+	if (unlikely(WARN_ON(hw->max_rates != 2)))
+		return -EINVAL;
 
 	txrate[0] = tx_info->control.rates;
 	txrate[1] = txrate[0] + 1;
 
-	ASSERT(txrate[0]->idx >= 0);
+	if (unlikely(WARN_ON(txrate[0]->idx < 0)))
+		return -EINVAL;
+
 	/* if rate control algorithm didn't give us a fallback rate, use the primary rate */
 	if (txrate[1]->idx < 0) {
 		txrate[1] = txrate[0];
@@ -5815,7 +5830,9 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 		is_mcs[k] =
 		    txrate[k]->flags & IEEE80211_TX_RC_MCS ? true : false;
 		if (!is_mcs[k]) {
-			ASSERT(!(tx_info->flags & IEEE80211_TX_CTL_AMPDU));
+			if (unlikely(WARN_ON(tx_info->flags &
+					     IEEE80211_TX_CTL_AMPDU)))
+				return -EINVAL;
 			if ((txrate[k]->idx >= 0)
 			    && (txrate[k]->idx <
 				hw->wiphy->bands[tx_info->band]->n_bitrates)) {
@@ -5827,10 +5844,10 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 				    flags & IEEE80211_TX_RC_USE_SHORT_PREAMBLE ?
 				    true : false;
 			} else {
-				ASSERT((txrate[k]->idx >= 0) &&
-				       (txrate[k]->idx <
-					hw->wiphy->bands[tx_info->band]->
-					n_bitrates));
+				if (unlikely(WARN_ON(txrate[k]->idx < 0 ||
+				    txrate[k]->idx >= hw->wiphy->bands[
+					tx_info->band]->n_bitrates)))
+					return -EINVAL;
 				rate_val[k] = WLC_RATE_1M;
 			}
 		} else {
@@ -5853,7 +5870,6 @@ wlc_d11hdrs_mac80211(struct wlc_info *wlc, struct ieee80211_hw *hw,
 
 		/* (1) RATE: determine and validate primary rate and fallback rates */
 		if (!RSPEC_ACTIVE(rspec[k])) {
-			ASSERT(RSPEC_ACTIVE(rspec[k]));
 			rspec[k] = WLC_RATE_1M;
 		} else {
 			if (!is_multicast_ether_addr(h->addr1)) {
@@ -6959,10 +6975,8 @@ wlc_recvctl(struct wlc_info *wlc, d11rxhdr_t *rxh, struct sk_buff *p)
 	skb_pull(p, D11_PHY_HDR_LEN);
 	__skb_trim(p, len_mpdu);
 
-	ASSERT(!(p->next));
-	ASSERT(!(p->prev));
-
-	ASSERT(IS_ALIGNED((unsigned long)skb->data, 2));
+	WARN_ON(p->next != NULL || p->prev != NULL);
+	WARN_ON(!IS_ALIGNED((unsigned long)skb->data, 2));
 
 	memcpy(IEEE80211_SKB_RXCB(p), &rx_status, sizeof(rx_status));
 	ieee80211_rx_irqsafe(wlc->pub->ieee_hw, p);
-- 
1.7.1


_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux