* [PATCH 4/4] rt2x00: Fix power of 2 check
@ 2007-03-08 21:14 Ivo van Doorn
2007-03-09 10:18 ` Ivo van Doorn
2007-03-09 10:48 ` [PATCH v2] " Ivo van Doorn
0 siblings, 2 replies; 4+ messages in thread
From: Ivo van Doorn @ 2007-03-08 21:14 UTC (permalink / raw)
To: John Linville; +Cc: wireless
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;)
This cleans up the code used to check the validity of the register fields,
this will also reduce the required memory usage while working with sparse.
This patch was inspired on the codesuggestions from Linus.
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
index b64d7ee..558d3ee 100644
--- a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
@@ -206,171 +206,10 @@ enum cipher {
};
/*
- * Macros for determining which is the lowest or highest bit
- * set in a 16 or 32 bit variable.
- */
-#define BIT_SET(__val, __bit) (__val & (1 << __bit))
-
-#define BIT_OK(__val, __bit, __low, __high) \
- (__bit < __low ? 1 : \
- (__bit > __high ? 1 : \
- BIT_SET(__val, __bit) ? 1 : 0))
-
-#define LOWEST_BIT16(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- -EINVAL))))))))))))))))
-
-#define LOWEST_BIT32(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \
- (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \
- (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \
- (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \
- (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \
- (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \
- (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \
- (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define HIGHEST_BIT16(__val) \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))
-
-#define HIGHEST_BIT32(__val) \
- (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \
- (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \
- (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \
- (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \
- (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \
- (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \
- (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \
- (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define BITRANGE_OK16(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high)) ? \
- 0 : -EINVAL)
-
-#define BITRANGE_OK32(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high) && \
- BIT_OK(__val, 16, __low, __high) && \
- BIT_OK(__val, 17, __low, __high) && \
- BIT_OK(__val, 18, __low, __high) && \
- BIT_OK(__val, 19, __low, __high) && \
- BIT_OK(__val, 20, __low, __high) && \
- BIT_OK(__val, 21, __low, __high) && \
- BIT_OK(__val, 22, __low, __high) && \
- BIT_OK(__val, 23, __low, __high) && \
- BIT_OK(__val, 24, __low, __high) && \
- BIT_OK(__val, 25, __low, __high) && \
- BIT_OK(__val, 26, __low, __high) && \
- BIT_OK(__val, 27, __low, __high) && \
- BIT_OK(__val, 28, __low, __high) && \
- BIT_OK(__val, 29, __low, __high) && \
- BIT_OK(__val, 30, __low, __high) && \
- BIT_OK(__val, 31, __low, __high)) ? \
- 0 : -EINVAL)
-
-extern int error_lowest_bit_not_constant;
-extern int error_highest_bit_not_constant;
-extern int error_bitrange_not_constant;
-extern int error_bitrange_bad;
-
-#define BUILD_LOWEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT16(__val))
-
-#define BUILD_LOWEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT32(__val))
-
-#define BUILD_HIGHEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT16(__val))
-
-#define BUILD_HIGHEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT32(__val))
-
-#define BUILD_BITRANGE_OK16(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-#define BUILD_BITRANGE_OK32(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-/*
* Register handlers.
* We store the position of a register field inside a field structure,
* This will simplify the process of setting and reading a certain field
* inside the register while making sure the process remains byte order safe.
- * Before setting the value into the structure we use macros to determine
- * whether all bits in the field are contineous and valid.
- * These additional checks will be optimized away at compile time,
- * but do have a major impact on compile speed, therefor we only make this
- * check when compiling with debug enabled.
*/
struct rt2x00_field16 {
u16 bit_offset;
@@ -383,39 +222,29 @@ struct rt2x00_field32 {
};
/*
- * Before intitializing the rt2x00_field# structures,
- * we will check if the bitmask is correct and does
- * not contain any gaps.
- * This check is only done in debug mode, since it severely
- * impacts compilation speed.
+ * Power of two check from Linus Torvalds,
+ * this will check if the mask that has been
+ * given contains and contiguous set of bits.
*/
-#ifdef CONFIG_RT2X00_DEBUG
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \
- BUILD_HIGHEST_BIT16(__mask)) == 0) ? \
- BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \
- BUILD_HIGHEST_BIT32(__mask)) == 0) ? \
- BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-#else /* CONFIG_RT2X00_DEBUG */
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- BUILD_LOWEST_BIT16(__mask), (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- BUILD_LOWEST_BIT32(__mask), (__mask) \
- } )
-#endif /* CONFIG_RT2X00_DEBUG */
+#define is_power_of_two(x) ( !((x) & ((x)-1)) )
+#define low_bit_mask(x) ( ((x)-1) & ~(x) )
+#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x))
+
+#define FIELD16(__mask) \
+({ \
+ BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\
+ (struct rt2x00_field16) { \
+ (__scanbit((__mask), 16) - 1), (__mask) \
+ }; \
+})
+
+#define FIELD32(__mask) \
+({ \
+ BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\
+ (struct rt2x00_field32) { \
+ (__scanbit((__mask), 32) - 1), (__mask) \
+ }; \
+})
static inline void rt2x00_set_field32(u32 *reg,
const struct rt2x00_field32 field, const u32 value)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 4/4] rt2x00: Fix power of 2 check
2007-03-08 21:14 [PATCH 4/4] rt2x00: Fix power of 2 check Ivo van Doorn
@ 2007-03-09 10:18 ` Ivo van Doorn
2007-03-09 10:48 ` [PATCH v2] " Ivo van Doorn
1 sibling, 0 replies; 4+ messages in thread
From: Ivo van Doorn @ 2007-03-09 10:18 UTC (permalink / raw)
To: John Linville; +Cc: wireless
On Thursday 08 March 2007 22:14, Ivo van Doorn wrote:
> Cleanup code, make gcc happy, make sparse happy. What a happy patch ;)
> This cleans up the code used to check the validity of the register fields,
> this will also reduce the required memory usage while working with sparse.
> This patch was inspired on the codesuggestions from Linus.
Please cancel this 4th patch, the previous 3 were correct but this one is terribly wrong.
> +#define is_power_of_two(x) ( !((x) & ((x)-1)) )
> +#define low_bit_mask(x) ( ((x)-1) & ~(x) )
> +#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x))
> +
> +#define FIELD16(__mask) \
> +({ \
> + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\
> + (struct rt2x00_field16) { \
> + (__scanbit((__mask), 16) - 1), (__mask) \
> + }; \
> +})
> +
> +#define FIELD32(__mask) \
> +({ \
> + BUILD_BUG_ON(!__mask || !is_valid_mask(__mask));\
> + (struct rt2x00_field32) { \
> + (__scanbit((__mask), 32) - 1), (__mask) \
> + }; \
> +})
The problem is here, I have used __scanbit to determine the first bit
that is set. __scanbit was used since this doesn't require a pointer to the value
while the other functions do require the pointer (and the macro receives the direct
value hence the reason for the __scanbit choice).
Unfortunately I did not check if __scanbit was available on all arches, and this one
seems to only excist for x86_64.. :S
I'll try to fix this asap and come with a new patch.
Ivo
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH v2] rt2x00: Fix power of 2 check
2007-03-08 21:14 [PATCH 4/4] rt2x00: Fix power of 2 check Ivo van Doorn
2007-03-09 10:18 ` Ivo van Doorn
@ 2007-03-09 10:48 ` Ivo van Doorn
2007-03-09 15:11 ` [PATCH v3] " Ivo van Doorn
1 sibling, 1 reply; 4+ messages in thread
From: Ivo van Doorn @ 2007-03-09 10:48 UTC (permalink / raw)
To: John Linville; +Cc: wireless
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;)
This cleans up the code used to check the validity of the register fields,
this will also reduce the required memory usage while working with sparse.
This patch was inspired on the codesuggestions from Linus.
This update also includes a check if the first found bit does not exceed the
16bit or 32 bit limit.
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
index b64d7ee..25cb3ba 100644
--- a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
@@ -206,171 +206,10 @@ enum cipher {
};
/*
- * Macros for determining which is the lowest or highest bit
- * set in a 16 or 32 bit variable.
- */
-#define BIT_SET(__val, __bit) (__val & (1 << __bit))
-
-#define BIT_OK(__val, __bit, __low, __high) \
- (__bit < __low ? 1 : \
- (__bit > __high ? 1 : \
- BIT_SET(__val, __bit) ? 1 : 0))
-
-#define LOWEST_BIT16(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- -EINVAL))))))))))))))))
-
-#define LOWEST_BIT32(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \
- (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \
- (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \
- (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \
- (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \
- (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \
- (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \
- (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define HIGHEST_BIT16(__val) \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))
-
-#define HIGHEST_BIT32(__val) \
- (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \
- (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \
- (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \
- (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \
- (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \
- (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \
- (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \
- (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define BITRANGE_OK16(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high)) ? \
- 0 : -EINVAL)
-
-#define BITRANGE_OK32(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high) && \
- BIT_OK(__val, 16, __low, __high) && \
- BIT_OK(__val, 17, __low, __high) && \
- BIT_OK(__val, 18, __low, __high) && \
- BIT_OK(__val, 19, __low, __high) && \
- BIT_OK(__val, 20, __low, __high) && \
- BIT_OK(__val, 21, __low, __high) && \
- BIT_OK(__val, 22, __low, __high) && \
- BIT_OK(__val, 23, __low, __high) && \
- BIT_OK(__val, 24, __low, __high) && \
- BIT_OK(__val, 25, __low, __high) && \
- BIT_OK(__val, 26, __low, __high) && \
- BIT_OK(__val, 27, __low, __high) && \
- BIT_OK(__val, 28, __low, __high) && \
- BIT_OK(__val, 29, __low, __high) && \
- BIT_OK(__val, 30, __low, __high) && \
- BIT_OK(__val, 31, __low, __high)) ? \
- 0 : -EINVAL)
-
-extern int error_lowest_bit_not_constant;
-extern int error_highest_bit_not_constant;
-extern int error_bitrange_not_constant;
-extern int error_bitrange_bad;
-
-#define BUILD_LOWEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT16(__val))
-
-#define BUILD_LOWEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT32(__val))
-
-#define BUILD_HIGHEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT16(__val))
-
-#define BUILD_HIGHEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT32(__val))
-
-#define BUILD_BITRANGE_OK16(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-#define BUILD_BITRANGE_OK32(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-/*
* Register handlers.
* We store the position of a register field inside a field structure,
* This will simplify the process of setting and reading a certain field
* inside the register while making sure the process remains byte order safe.
- * Before setting the value into the structure we use macros to determine
- * whether all bits in the field are contineous and valid.
- * These additional checks will be optimized away at compile time,
- * but do have a major impact on compile speed, therefor we only make this
- * check when compiling with debug enabled.
*/
struct rt2x00_field16 {
u16 bit_offset;
@@ -383,39 +222,33 @@ struct rt2x00_field32 {
};
/*
- * Before intitializing the rt2x00_field# structures,
- * we will check if the bitmask is correct and does
- * not contain any gaps.
- * This check is only done in debug mode, since it severely
- * impacts compilation speed.
+ * Power of two check from Linus Torvalds,
+ * this will check if the mask that has been
+ * given contains and contiguous set of bits.
*/
-#ifdef CONFIG_RT2X00_DEBUG
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \
- BUILD_HIGHEST_BIT16(__mask)) == 0) ? \
- BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \
- BUILD_HIGHEST_BIT32(__mask)) == 0) ? \
- BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-#else /* CONFIG_RT2X00_DEBUG */
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- BUILD_LOWEST_BIT16(__mask), (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- BUILD_LOWEST_BIT32(__mask), (__mask) \
- } )
-#endif /* CONFIG_RT2X00_DEBUG */
+#define is_power_of_two(x) ( !((x) & ((x)-1)) )
+#define low_bit_mask(x) ( ((x)-1) & ~(x) )
+#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x))
+
+#define FIELD16(__mask) \
+({ \
+ BUILD_BUG_ON(!(__mask) || \
+ !is_valid_mask(__mask) || \
+ (__ffs((__mask) > 16))); \
+ (struct rt2x00_field16) { \
+ (__ffs(__mask) - 1), (__mask) \
+ }; \
+})
+
+#define FIELD32(__mask) \
+({ \
+ BUILD_BUG_ON(!(__mask) || \
+ !is_valid_mask(__mask) || \
+ (__ffs((__mask) > 32))); \
+ (struct rt2x00_field32) { \
+ (__ffs(__mask) - 1), (__mask) \
+ }; \
+})
static inline void rt2x00_set_field32(u32 *reg,
const struct rt2x00_field32 field, const u32 value)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v3] rt2x00: Fix power of 2 check
2007-03-09 10:48 ` [PATCH v2] " Ivo van Doorn
@ 2007-03-09 15:11 ` Ivo van Doorn
0 siblings, 0 replies; 4+ messages in thread
From: Ivo van Doorn @ 2007-03-09 15:11 UTC (permalink / raw)
To: John Linville; +Cc: wireless
Cleanup code, make gcc happy, make sparse happy. What a happy patch ;)
This cleans up the code used to check the validity of the register fields,
this will also reduce the required memory usage while working with sparse.
This patch was inspired on the codesuggestions from Linus.
This update also includes a check if the first found bit does not exceed the
16bit or 32 bit limit.
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
Sorry for all the spamming, this is the correct version of the patch.
diff --git a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
index b64d7ee..cea6bb1 100644
--- a/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/mac80211/rt2x00/rt2x00.h
@@ -206,171 +206,10 @@ enum cipher {
};
/*
- * Macros for determining which is the lowest or highest bit
- * set in a 16 or 32 bit variable.
- */
-#define BIT_SET(__val, __bit) (__val & (1 << __bit))
-
-#define BIT_OK(__val, __bit, __low, __high) \
- (__bit < __low ? 1 : \
- (__bit > __high ? 1 : \
- BIT_SET(__val, __bit) ? 1 : 0))
-
-#define LOWEST_BIT16(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- -EINVAL))))))))))))))))
-
-#define LOWEST_BIT32(__val) \
- (BIT_SET(__val, 0) ? 0 : (BIT_SET(__val, 1) ? 1 : \
- (BIT_SET(__val, 2) ? 2 : (BIT_SET(__val, 3) ? 3 : \
- (BIT_SET(__val, 4) ? 4 : (BIT_SET(__val, 5) ? 5 : \
- (BIT_SET(__val, 6) ? 6 : (BIT_SET(__val, 7) ? 7 : \
- (BIT_SET(__val, 8) ? 8 : (BIT_SET(__val, 9) ? 9 : \
- (BIT_SET(__val, 10) ? 10 : (BIT_SET(__val, 11) ? 11 : \
- (BIT_SET(__val, 12) ? 12 : (BIT_SET(__val, 13) ? 13 : \
- (BIT_SET(__val, 14) ? 14 : (BIT_SET(__val, 15) ? 15 : \
- (BIT_SET(__val, 16) ? 16 : (BIT_SET(__val, 17) ? 17 : \
- (BIT_SET(__val, 18) ? 18 : (BIT_SET(__val, 19) ? 19 : \
- (BIT_SET(__val, 20) ? 20 : (BIT_SET(__val, 21) ? 21 : \
- (BIT_SET(__val, 22) ? 22 : (BIT_SET(__val, 23) ? 23 : \
- (BIT_SET(__val, 24) ? 24 : (BIT_SET(__val, 25) ? 25 : \
- (BIT_SET(__val, 26) ? 27 : (BIT_SET(__val, 27) ? 27 : \
- (BIT_SET(__val, 28) ? 28 : (BIT_SET(__val, 29) ? 29 : \
- (BIT_SET(__val, 30) ? 30 : (BIT_SET(__val, 31) ? 31 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define HIGHEST_BIT16(__val) \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))
-
-#define HIGHEST_BIT32(__val) \
- (BIT_SET(__val, 31) ? 31 : (BIT_SET(__val, 30) ? 30 : \
- (BIT_SET(__val, 29) ? 29 : (BIT_SET(__val, 28) ? 28 : \
- (BIT_SET(__val, 27) ? 27 : (BIT_SET(__val, 26) ? 26 : \
- (BIT_SET(__val, 25) ? 25 : (BIT_SET(__val, 24) ? 24 : \
- (BIT_SET(__val, 23) ? 23 : (BIT_SET(__val, 22) ? 22 : \
- (BIT_SET(__val, 21) ? 21 : (BIT_SET(__val, 20) ? 20 : \
- (BIT_SET(__val, 19) ? 19 : (BIT_SET(__val, 18) ? 18 : \
- (BIT_SET(__val, 17) ? 17 : (BIT_SET(__val, 16) ? 16 : \
- (BIT_SET(__val, 15) ? 15 : (BIT_SET(__val, 14) ? 14 : \
- (BIT_SET(__val, 13) ? 13 : (BIT_SET(__val, 12) ? 12 : \
- (BIT_SET(__val, 11) ? 11 : (BIT_SET(__val, 10) ? 10 : \
- (BIT_SET(__val, 9) ? 9 : (BIT_SET(__val, 8) ? 8 : \
- (BIT_SET(__val, 7) ? 7 : (BIT_SET(__val, 6) ? 6 : \
- (BIT_SET(__val, 5) ? 5 : (BIT_SET(__val, 4) ? 4 : \
- (BIT_SET(__val, 3) ? 3 : (BIT_SET(__val, 2) ? 2 : \
- (BIT_SET(__val, 1) ? 1 : (BIT_SET(__val, 0) ? 0 : \
- -EINVAL))))))))))))))))))))))))))))))))
-
-#define BITRANGE_OK16(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high)) ? \
- 0 : -EINVAL)
-
-#define BITRANGE_OK32(__val, __low, __high) \
- ((BIT_OK(__val, 0, __low, __high) && \
- BIT_OK(__val, 1, __low, __high) && \
- BIT_OK(__val, 2, __low, __high) && \
- BIT_OK(__val, 3, __low, __high) && \
- BIT_OK(__val, 4, __low, __high) && \
- BIT_OK(__val, 5, __low, __high) && \
- BIT_OK(__val, 6, __low, __high) && \
- BIT_OK(__val, 7, __low, __high) && \
- BIT_OK(__val, 8, __low, __high) && \
- BIT_OK(__val, 9, __low, __high) && \
- BIT_OK(__val, 10, __low, __high) && \
- BIT_OK(__val, 11, __low, __high) && \
- BIT_OK(__val, 12, __low, __high) && \
- BIT_OK(__val, 13, __low, __high) && \
- BIT_OK(__val, 14, __low, __high) && \
- BIT_OK(__val, 15, __low, __high) && \
- BIT_OK(__val, 16, __low, __high) && \
- BIT_OK(__val, 17, __low, __high) && \
- BIT_OK(__val, 18, __low, __high) && \
- BIT_OK(__val, 19, __low, __high) && \
- BIT_OK(__val, 20, __low, __high) && \
- BIT_OK(__val, 21, __low, __high) && \
- BIT_OK(__val, 22, __low, __high) && \
- BIT_OK(__val, 23, __low, __high) && \
- BIT_OK(__val, 24, __low, __high) && \
- BIT_OK(__val, 25, __low, __high) && \
- BIT_OK(__val, 26, __low, __high) && \
- BIT_OK(__val, 27, __low, __high) && \
- BIT_OK(__val, 28, __low, __high) && \
- BIT_OK(__val, 29, __low, __high) && \
- BIT_OK(__val, 30, __low, __high) && \
- BIT_OK(__val, 31, __low, __high)) ? \
- 0 : -EINVAL)
-
-extern int error_lowest_bit_not_constant;
-extern int error_highest_bit_not_constant;
-extern int error_bitrange_not_constant;
-extern int error_bitrange_bad;
-
-#define BUILD_LOWEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT16(__val))
-
-#define BUILD_LOWEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_lowest_bit_not_constant : \
- LOWEST_BIT32(__val))
-
-#define BUILD_HIGHEST_BIT16(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT16(__val))
-
-#define BUILD_HIGHEST_BIT32(__val) \
- (!__builtin_constant_p(__val) ? error_highest_bit_not_constant : \
- HIGHEST_BIT32(__val))
-
-#define BUILD_BITRANGE_OK16(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-#define BUILD_BITRANGE_OK32(__val, __low, __high) \
- ((!__builtin_constant_p(__val) || !__builtin_constant_p(__low) || \
- !__builtin_constant_p(__high)) ? error_bitrange_not_constant : \
- BITRANGE_OK16(__val, __low, __high))
-
-/*
* Register handlers.
* We store the position of a register field inside a field structure,
* This will simplify the process of setting and reading a certain field
* inside the register while making sure the process remains byte order safe.
- * Before setting the value into the structure we use macros to determine
- * whether all bits in the field are contineous and valid.
- * These additional checks will be optimized away at compile time,
- * but do have a major impact on compile speed, therefor we only make this
- * check when compiling with debug enabled.
*/
struct rt2x00_field16 {
u16 bit_offset;
@@ -383,39 +222,33 @@ struct rt2x00_field32 {
};
/*
- * Before intitializing the rt2x00_field# structures,
- * we will check if the bitmask is correct and does
- * not contain any gaps.
- * This check is only done in debug mode, since it severely
- * impacts compilation speed.
+ * Power of two check from Linus Torvalds,
+ * this will check if the mask that has been
+ * given contains and contiguous set of bits.
*/
-#ifdef CONFIG_RT2X00_DEBUG
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- (BUILD_BITRANGE_OK16(__mask, BUILD_LOWEST_BIT16(__mask), \
- BUILD_HIGHEST_BIT16(__mask)) == 0) ? \
- BUILD_LOWEST_BIT16(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- (BUILD_BITRANGE_OK32(__mask, BUILD_LOWEST_BIT32(__mask), \
- BUILD_HIGHEST_BIT32(__mask)) == 0) ? \
- BUILD_LOWEST_BIT32(__mask) : error_bitrange_bad, \
- (__mask) \
- } )
-#else /* CONFIG_RT2X00_DEBUG */
-#define FIELD16(__mask) \
- ( (struct rt2x00_field16) { \
- BUILD_LOWEST_BIT16(__mask), (__mask) \
- } )
-
-#define FIELD32(__mask) \
- ( (struct rt2x00_field32) { \
- BUILD_LOWEST_BIT32(__mask), (__mask) \
- } )
-#endif /* CONFIG_RT2X00_DEBUG */
+#define is_power_of_two(x) ( !((x) & ((x)-1)) )
+#define low_bit_mask(x) ( ((x)-1) & ~(x) )
+#define is_valid_mask(x) is_power_of_two(1 + (x) + low_bit_mask(x))
+
+#define FIELD16(__mask) \
+({ \
+ BUILD_BUG_ON(!(__mask) || \
+ !is_valid_mask(__mask) || \
+ (__ffs((__mask) >= 16))); \
+ (struct rt2x00_field16) { \
+ __ffs(__mask), (__mask) \
+ }; \
+})
+
+#define FIELD32(__mask) \
+({ \
+ BUILD_BUG_ON(!(__mask) || \
+ !is_valid_mask(__mask) || \
+ (__ffs((__mask) >= 32))); \
+ (struct rt2x00_field32) { \
+ __ffs(__mask), (__mask) \
+ }; \
+})
static inline void rt2x00_set_field32(u32 *reg,
const struct rt2x00_field32 field, const u32 value)
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2007-03-09 15:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-03-08 21:14 [PATCH 4/4] rt2x00: Fix power of 2 check Ivo van Doorn
2007-03-09 10:18 ` Ivo van Doorn
2007-03-09 10:48 ` [PATCH v2] " Ivo van Doorn
2007-03-09 15:11 ` [PATCH v3] " Ivo van Doorn
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).