[PATCH] ath5k: fix exp off-by-one when computing OFDM delta slope

[PATCH] ath5k: fix exp off-by-one when computing OFDM delta slope

[Bob Copeland]

From: Forrest Zhang <forrest@hifulltech.com>

Commit e8f055f0c3ba226ca599c14c2e5fe829f6f57cbb subtly changed the
code that computes floating point values for the PHY3_TIMING register
such that the exponent is off by a decimal point, which can cause
problems with OFDM channel operation.

get_bitmask_order() actually returns the highest bit set plus one,
whereas the previous code wanted the highest bit set.  Instead, use
ilog2 which is what this code is really calculating.  Also check
coef_scaled to handle the (invalid) case where we need log2(0).

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---

John, this plus 706d64453cce7179e05924c24d87777584a1275c fixes a 
regression for 2.6.30 5ghz operation (bugzilla 13077).  Any chance
they can both make it in?  I can send a backport of this patch if
you like (usual path change).

 drivers/net/wireless/ath/ath5k/reset.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c
index 386bc8b..bd1940e 100644
--- a/drivers/net/wireless/ath/ath5k/reset.c
+++ b/drivers/net/wireless/ath/ath5k/reset.c
@@ -26,7 +26,7 @@
 \*****************************/
 
 #include <linux/pci.h> 		/* To determine if a card is pci-e */
-#include <linux/bitops.h>	/* For get_bitmask_order */
+#include <linux/log2.h>
 #include "ath5k.h"
 #include "reg.h"
 #include "base.h"
@@ -69,10 +69,10 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah,
 
 	/* Get exponent
 	 * ALGO: coef_exp = 14 - highest set bit position */
-	coef_exp = get_bitmask_order(coef_scaled);
+	coef_exp = ilog2(coef_scaled);
 
 	/* Doesn't make sense if it's zero*/
-	if (!coef_exp)
+	if (!coef_scaled || !coef_exp)
 		return -EINVAL;
 
 	/* Note: we've shifted coef_scaled by 24 */
-- 
1.6.0.6

-- 
Bob Copeland %% www.bobcopeland.com

Re: [PATCH] ath5k: fix exp off-by-one when computing OFDM delta slope

[John W. Linville]

On Wed, May 13, 2009 at 11:14:39AM -0400, Bob Copeland wrote:
> 
> From: Forrest Zhang <forrest@hifulltech.com>
> 
> Commit e8f055f0c3ba226ca599c14c2e5fe829f6f57cbb subtly changed the
> code that computes floating point values for the PHY3_TIMING register
> such that the exponent is off by a decimal point, which can cause
> problems with OFDM channel operation.
> 
> get_bitmask_order() actually returns the highest bit set plus one,
> whereas the previous code wanted the highest bit set.  Instead, use
> ilog2 which is what this code is really calculating.  Also check
> coef_scaled to handle the (invalid) case where we need log2(0).
> 
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
> 
> John, this plus 706d64453cce7179e05924c24d87777584a1275c fixes a 
> regression for 2.6.30 5ghz operation (bugzilla 13077).  Any chance
> they can both make it in?  I can send a backport of this patch if
> you like (usual path change).

That's fine.  But what about this last bit?

	http://bugzilla.kernel.org/show_bug.cgi?id=13077#c6

Is that not needed?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [ath5k-devel] [PATCH] ath5k: fix exp off-by-one when computing OFDM delta slope

[Bob Copeland]

On Wed, May 13, 2009 at 2:25 PM, John W. Linville
<linville@tuxdriver.com> wrote:
> That's fine. =A0But what about this last bit?
>
> =A0 =A0 =A0 =A0http://bugzilla.kernel.org/show_bug.cgi?id=3D13077#c6
>
> Is that not needed?

I think it is needed, but I checked back through 2.6.27 and it has the
same (probably buggy) code, so I think that's less urgent.  Also it
conflicts with the race condition fixes I sent earlier so it is better
in a separate patch.

--=20
Bob Copeland %% www.bobcopeland.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html