Hi all, Someone pointed out to me a change in the code with the reset rework, due to my suggestion to use get_bitmask_order() which actually returns highest bit set+1. Nick, can you comment whether the patch below does the right thing and that the original code was correct?
[Forrest, is this idea via you, and may I attribute you as the patch author like so? From: Forrest Zhang <your preferred email>] Here's a C program I used to confirm the problem (f1 is original code, f2 is get_bitmask_order with generic fls(). Output is: 1: 0 1 0: 0 0 32767: 14 15 65535: 15 16 65536: 16 17 1048576: 20 21 --- start --- #include <inttypes.h> #include <stdio.h> static int fls(int x) { int r = 32; if (!x) return 0; if (!(x & 0xffff0000u)) { x <<= 16; r -= 16; } if (!(x & 0xff000000u)) { x <<= 8; r -= 8; } if (!(x & 0xf0000000u)) { x <<= 4; r -= 4; } if (!(x & 0xc0000000u)) { x <<= 2; r -= 2; } if (!(x & 0x80000000u)) { x <<= 1; r -= 1; } return r; } int f1(int c) { int bit; for (bit = 31; bit > 0; bit--) if ((c >> bit) & 1) break; return bit; } int f2(int c) { return fls(c); } int main() { int i; int nums[] = { 1, 0, 32767, 65535, 65536, 1048576 }; for (i=0; i < sizeof(nums)/sizeof(int); i++) printf("%d: %d %d\n", nums[i], f1(nums[i]), f2(nums[i])); return 0; } --- end --- >From d668403b04487a90378fc4ec7cd1ebe1e4b7bfe1 Mon Sep 17 00:00:00 2001 From: Bob Copeland <m...@bobcopeland.com> Date: Tue, 12 May 2009 09:40:37 -0400 Subject: [PATCH] ath5k: fix off-by-one when computing OFDM delta slope get_bitmask_order() actually returns the highest bit set plus one, whereas the previous code wanted the highest bit set. Also change type of coef_exp to int to deal with degenerate case of coef_exp == 0. --- drivers/net/wireless/ath/ath5k/reset.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/net/wireless/ath/ath5k/reset.c b/drivers/net/wireless/ath/ath5k/reset.c index 4a2d93e..cc75e77 100644 --- a/drivers/net/wireless/ath/ath5k/reset.c +++ b/drivers/net/wireless/ath/ath5k/reset.c @@ -51,8 +51,8 @@ static inline int ath5k_hw_write_ofdm_timings(struct ath5k_hw *ah, struct ieee80211_channel *channel) { /* Get exponent and mantissa and set it */ - u32 coef_scaled, coef_exp, coef_man, - ds_coef_exp, ds_coef_man, clock; + u32 coef_scaled, coef_man, ds_coef_exp, ds_coef_man, clock; + int coef_exp; if (!(ah->ah_version == AR5K_AR5212) || !(channel->hw_value & CHANNEL_OFDM)) @@ -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 = get_bitmask_order(coef_scaled) - 1; /* Doesn't make sense if it's zero*/ - if (!coef_exp) + if (coef_exp <= 0) return -EINVAL; /* Note: we've shifted coef_scaled by 24 */ -- 1.6.0.6 -- Bob Copeland %% www.bobcopeland.com _______________________________________________ ath5k-devel mailing list ath5k-devel@lists.ath5k.org https://lists.ath5k.org/mailman/listinfo/ath5k-devel