Patchwork [1/1] UBUNTU: SAUCE: brcmsmac: fix tx status processing

login
register
mail settings
Submitter Seth Forshee
Date Jan. 24, 2013, 4:02 p.m.
Message ID <1359043361-13889-2-git-send-email-seth.forshee@canonical.com>
Download mbox | patch
Permalink /patch/215401/
State New
Headers show

Comments

Seth Forshee - Jan. 24, 2013, 4:02 p.m.
From: Arend van Spriel <arend@broadcom.com>

The tx status feedback is processed in a loop limiting the number
of frames processed in one run. Bisecting by Seth Forshee showed
following commit as culprit:

commit 57fe504817ccec9b6ac23e973d2925343bf1e3b6
Author: Piotr Haber <phaber@broadcom.com>
Date:   Wed Nov 28 21:44:07 2012 +0100

    brcmsmac: fix bounds checking in tx/rx

The code terminate processing when the limit is reached regardless
the txstatus value read from the device register. When that status
is flagged as being valid it must be processed.

Cc: Seth Forshee <seth.forshee@canonical.com>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Arend van Spriel <arend@broadcom.com>
Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
---
 drivers/net/wireless/brcm80211/brcmsmac/main.c |   25 +++++++++---------------
 1 file changed, 9 insertions(+), 16 deletions(-)
Stefan Bader - Jan. 24, 2013, 4:20 p.m.
I trust Andy's testing. Have heard much less cursing after using the patched
module (about his wireless at least)...
Leann Ogasawara - Jan. 24, 2013, 8:23 p.m.
Applied to Raring master-next.  I'll likely do an upload tomorrow.

Thanks,
Leann

On 01/24/2013 08:02 AM, Seth Forshee wrote:
> From: Arend van Spriel <arend@broadcom.com>
>
> The tx status feedback is processed in a loop limiting the number
> of frames processed in one run. Bisecting by Seth Forshee showed
> following commit as culprit:
>
> commit 57fe504817ccec9b6ac23e973d2925343bf1e3b6
> Author: Piotr Haber <phaber@broadcom.com>
> Date:   Wed Nov 28 21:44:07 2012 +0100
>
>     brcmsmac: fix bounds checking in tx/rx
>
> The code terminate processing when the limit is reached regardless
> the txstatus value read from the device register. When that status
> is flagged as being valid it must be processed.
>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Seth Forshee <seth.forshee@canonical.com>
> ---
>  drivers/net/wireless/brcm80211/brcmsmac/main.c |   25 +++++++++---------------
>  1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> index 17594de..9f3d7e9 100644
> --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
> +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
> @@ -1027,7 +1027,6 @@ brcms_c_dotxstatus(struct brcms_c_info *wlc, struct tx_status *txs)
>  static bool
>  brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
>  {
> -	bool morepending = false;
>  	struct bcma_device *core;
>  	struct tx_status txstatus, *txs;
>  	u32 s1, s2;
> @@ -1041,23 +1040,20 @@ brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
>  	txs = &txstatus;
>  	core = wlc_hw->d11core;
>  	*fatal = false;
> -	s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
> -	while (!(*fatal)
> -	       && (s1 & TXS_V)) {
> -		/* !give others some time to run! */
> -		if (n >= max_tx_num) {
> -			morepending = true;
> -			break;
> -		}
>  
> +	while (n < max_tx_num) {
> +		s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
>  		if (s1 == 0xffffffff) {
>  			brcms_err(core, "wl%d: %s: dead chip\n", wlc_hw->unit,
>  				  __func__);
>  			*fatal = true;
>  			return false;
>  		}
> -		s2 = bcma_read32(core, D11REGOFFS(frmtxstatus2));
> +		/* only process when valid */
> +		if (!(s1 & TXS_V))
> +			break;
>  
> +		s2 = bcma_read32(core, D11REGOFFS(frmtxstatus2));
>  		txs->status = s1 & TXS_STATUS_MASK;
>  		txs->frameid = (s1 & TXS_FID_MASK) >> TXS_FID_SHIFT;
>  		txs->sequence = s2 & TXS_SEQ_MASK;
> @@ -1065,15 +1061,12 @@ brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
>  		txs->lasttxtime = 0;
>  
>  		*fatal = brcms_c_dotxstatus(wlc_hw->wlc, txs);
> -
> -		s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
> +		if (*fatal == true)
> +			return false;
>  		n++;
>  	}
>  
> -	if (*fatal)
> -		return false;
> -
> -	return morepending;
> +	return n >= max_tx_num;
>  }
>  
>  static void brcms_c_tbtt(struct brcms_c_info *wlc)

Patch

diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 17594de..9f3d7e9 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -1027,7 +1027,6 @@  brcms_c_dotxstatus(struct brcms_c_info *wlc, struct tx_status *txs)
 static bool
 brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
 {
-	bool morepending = false;
 	struct bcma_device *core;
 	struct tx_status txstatus, *txs;
 	u32 s1, s2;
@@ -1041,23 +1040,20 @@  brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
 	txs = &txstatus;
 	core = wlc_hw->d11core;
 	*fatal = false;
-	s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
-	while (!(*fatal)
-	       && (s1 & TXS_V)) {
-		/* !give others some time to run! */
-		if (n >= max_tx_num) {
-			morepending = true;
-			break;
-		}
 
+	while (n < max_tx_num) {
+		s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
 		if (s1 == 0xffffffff) {
 			brcms_err(core, "wl%d: %s: dead chip\n", wlc_hw->unit,
 				  __func__);
 			*fatal = true;
 			return false;
 		}
-		s2 = bcma_read32(core, D11REGOFFS(frmtxstatus2));
+		/* only process when valid */
+		if (!(s1 & TXS_V))
+			break;
 
+		s2 = bcma_read32(core, D11REGOFFS(frmtxstatus2));
 		txs->status = s1 & TXS_STATUS_MASK;
 		txs->frameid = (s1 & TXS_FID_MASK) >> TXS_FID_SHIFT;
 		txs->sequence = s2 & TXS_SEQ_MASK;
@@ -1065,15 +1061,12 @@  brcms_b_txstatus(struct brcms_hardware *wlc_hw, bool bound, bool *fatal)
 		txs->lasttxtime = 0;
 
 		*fatal = brcms_c_dotxstatus(wlc_hw->wlc, txs);
-
-		s1 = bcma_read32(core, D11REGOFFS(frmtxstatus));
+		if (*fatal == true)
+			return false;
 		n++;
 	}
 
-	if (*fatal)
-		return false;
-
-	return morepending;
+	return n >= max_tx_num;
 }
 
 static void brcms_c_tbtt(struct brcms_c_info *wlc)