[PATCH 01/11] rt2x00: Calculate register offset during compile time

[PATCH 01/11] rt2x00: Calculate register offset during compile time

[Ivo van Doorn]

By using __ffs() the register offsets were always calculated
at run-time which all FIELD32/FIELD16 definitions were builtin
constants. This means we can heavily optimize the register handling
by allowing GCC to do all the work during compilation.

Add some compile_ffs() macros to perform the calculation at
compile time. After this each rt2x00 module size is reduced
by ~2500 bytes. And the stack size of several functions is reduced
as well which further limits the number of rt2x00 results in
'make checkstack'.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   55 +++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h
index 3f255df..03e846f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00reg.h
+++ b/drivers/net/wireless/rt2x00/rt2x00reg.h
@@ -130,40 +130,67 @@ struct rt2x00_field32 {
 
 /*
  * Power of two check, this will check
- * if the mask that has been given contains
- * and contiguous set of bits.
+ * if the mask that has been given contains and contiguous set of bits.
+ * Note that we cannot use the is_power_of_2() function since this
+ * check must be done at compile-time.
  */
 #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))
 
+/*
+ * Macro's to find first set bit in a variable.
+ * These macro's behaves the same as the __ffs() function with
+ * the most important difference that this is done during
+ * compile-time rather then run-time.
+ */
+#define compile_ffs2(__x) \
+	((__x) & 0x1) ? 0 : 1
+
+#define compile_ffs4(__x) \
+	((__x) & 0x3) ? compile_ffs2(__x) : compile_ffs2(__x >> 2) + 2
+
+#define compile_ffs8(__x) \
+	((__x) & 0xf) ? compile_ffs4(__x) : compile_ffs4(__x >> 4) + 4
+
+#define compile_ffs16(__x) \
+	((__x) & 0xff) ? compile_ffs8(__x) : compile_ffs8(__x >> 8) + 8
+
+#define compile_ffs32(__x) \
+	((__x) & 0xffff) ? compile_ffs16(__x) : compile_ffs16(__x >> 16) + 16
+
+/*
+ * This macro will check the requirements for the FIELD{8,16,32} macros
+ * The mask should be a constant non-zero contiguous set of bits which
+ * does not exceed the given typelimit.
+ */
+#define FIELD_CHECK(__mask, __type)			\
+	BUILD_BUG_ON(!__builtin_constant_p(__mask) ||	\
+		     !(__mask) ||			\
+		     !is_valid_mask(__mask) ||		\
+		     (__mask) != (__type)(__mask))	\
+
 #define FIELD8(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u8)(__mask));	\
+	FIELD_CHECK(__mask, u8);		\
 	(struct rt2x00_field8) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs8(__mask), (__mask)	\
 	};					\
 })
 
 #define FIELD16(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u16)(__mask));\
+	FIELD_CHECK(__mask, u16);		\
 	(struct rt2x00_field16) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs16(__mask), (__mask)	\
 	};					\
 })
 
 #define FIELD32(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u32)(__mask));\
+	FIELD_CHECK(__mask, u32);		\
 	(struct rt2x00_field32) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs32(__mask), (__mask)	\
 	};					\
 })
 
-- 
1.5.5.3

[PATCH 01/11 v2] rt2x00: Calculate register offset during compile time

[Ivo van Doorn]

rt2x00: Calculate register offset during compile time

By using __ffs() the register offsets were always calculated
at run-time which all FIELD32/FIELD16 definitions were builtin
constants. This means we can heavily optimize the register handling
by allowing GCC to do all the work during compilation.

Add some compile_ffs() macros to perform the calculation at
compile time. After this each rt2x00 module size is reduced
by ~2500 bytes. And the stack size of several functions is reduced
as well which further limits the number of rt2x00 results in
'make checkstack'.

v2: Merge GertJan's bugfix of patch [1/11] directly into this patch
      instead of providing it as seperate patch.

Signed-off-by: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>

---
John, this patch is a combination of patch [1/11] and [2/11] so please
either apply those 2 patches or just this single patch to get the correct result :)
---
diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h
index 3f255df..7999d54 100644
--- a/drivers/net/wireless/rt2x00/rt2x00reg.h
+++ b/drivers/net/wireless/rt2x00/rt2x00reg.h
@@ -130,40 +130,71 @@ struct rt2x00_field32 {
 
 /*
  * Power of two check, this will check
- * if the mask that has been given contains
- * and contiguous set of bits.
+ * if the mask that has been given contains and contiguous set of bits.
+ * Note that we cannot use the is_power_of_2() function since this
+ * check must be done at compile-time.
  */
 #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))
 
+/*
+ * Macro's to find first set bit in a variable.
+ * These macro's behaves the same as the __ffs() function with
+ * the most important difference that this is done during
+ * compile-time rather then run-time.
+ */
+#define compile_ffs2(__x) \
+	( ((__x) & 0x1) ? 0 : 1 )
+
+#define compile_ffs4(__x) \
+	( ((__x) & 0x3) ? \
+	    compile_ffs2(__x) : (compile_ffs2(__x >> 2) + 2) )
+
+#define compile_ffs8(__x) \
+	( ((__x) & 0xf) ? \
+	    compile_ffs4(__x) : (compile_ffs4(__x >> 4) + 4) )
+
+#define compile_ffs16(__x) \
+	( ((__x) & 0xff) ? \
+	    compile_ffs8(__x) : (compile_ffs8(__x >> 8) + 8) )
+
+#define compile_ffs32(__x) \
+	( ((__x) & 0xffff) ? \
+	    compile_ffs16(__x) : (compile_ffs16(__x >> 16) + 16) )
+
+/*
+ * This macro will check the requirements for the FIELD{8,16,32} macros
+ * The mask should be a constant non-zero contiguous set of bits which
+ * does not exceed the given typelimit.
+ */
+#define FIELD_CHECK(__mask, __type)			\
+	BUILD_BUG_ON(!__builtin_constant_p(__mask) ||	\
+		     !(__mask) ||			\
+		     !is_valid_mask(__mask) ||		\
+		     (__mask) != (__type)(__mask))	\
+
 #define FIELD8(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u8)(__mask));	\
+	FIELD_CHECK(__mask, u8);		\
 	(struct rt2x00_field8) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs8(__mask), (__mask)	\
 	};					\
 })
 
 #define FIELD16(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u16)(__mask));\
+	FIELD_CHECK(__mask, u16);		\
 	(struct rt2x00_field16) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs16(__mask), (__mask)	\
 	};					\
 })
 
 #define FIELD32(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) != (u32)(__mask));\
+	FIELD_CHECK(__mask, u32);		\
 	(struct rt2x00_field32) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs32(__mask), (__mask)	\
 	};					\
 })
 

Re: [PATCH 01/11 v2] rt2x00: Calculate register offset during compile time

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 382 bytes --]


> +#define compile_ffs2(__x) \
> +	( ((__x) & 0x1) ? 0 : 1 )
> +
> +#define compile_ffs4(__x) \
> +	( ((__x) & 0x3) ? \
> +	    compile_ffs2(__x) : (compile_ffs2(__x >> 2) + 2) )

It seems you should also add parentheses around the __x used in the
recursive macro invocation, like this:

... : (compile_ffs2((__x) >> 2) + 2) )

or am I missing something?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 01/11 v2] rt2x00: Calculate register offset during compile time

[Ivo van Doorn]

On Tuesday 03 June 2008, Johannes Berg wrote:
> 
> > +#define compile_ffs2(__x) \
> > +	( ((__x) & 0x1) ? 0 : 1 )
> > +
> > +#define compile_ffs4(__x) \
> > +	( ((__x) & 0x3) ? \
> > +	    compile_ffs2(__x) : (compile_ffs2(__x >> 2) + 2) )
> 
> It seems you should also add parentheses around the __x used in the
> recursive macro invocation, like this:
> 
> ... : (compile_ffs2((__x) >> 2) + 2) )
> 
> or am I missing something?

Nope, you are right. :)
Version 3 on its way.

Thanks,

Ivo

[PATCH 01/11 v3] rt2x00: Calculate register offset during compile time

[Ivo van Doorn]

By using __ffs() the register offsets were always calculated
at run-time which all FIELD32/FIELD16 definitions were builtin
constants. This means we can heavily optimize the register handling
by allowing GCC to do all the work during compilation.

Add some compile_ffs() macros to perform the calculation at
compile time. After this each rt2x00 module size is reduced
by ~2500 bytes. And the stack size of several functions is reduced
as well which further limits the number of rt2x00 results in
'make checkstack'.

v2: Merge GertJan's bugfix of patch [1/11] directly into this patch
=C2=A0 =C2=A0 =C2=A0 instead of providing it as seperate patch.
v3: Add extra parentheses when bitshifting __x

Signed-off-by: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
John, this patch is a combination of patch [1/11] and [2/11] so please
either apply those 2 patches or just this single patch to get the corre=
ct result :)
---
diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wire=
less/rt2x00/rt2x00reg.h
index 3f255df..7999d54 100644
--- a/drivers/net/wireless/rt2x00/rt2x00reg.h
+++ b/drivers/net/wireless/rt2x00/rt2x00reg.h
@@ -130,40 +130,71 @@ struct rt2x00_field32 {
=20
 /*
  * Power of two check, this will check
- * if the mask that has been given contains
- * and contiguous set of bits.
+ * if the mask that has been given contains and contiguous set of bits=
=2E
+ * Note that we cannot use the is_power_of_2() function since this
+ * check must be done at compile-time.
  */
 #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))
=20
+/*
+ * Macro's to find first set bit in a variable.
+ * These macro's behaves the same as the __ffs() function with
+ * the most important difference that this is done during
+ * compile-time rather then run-time.
+ */
+#define compile_ffs2(__x) \
+	( ((__x) & 0x1) ? 0 : 1 )
+
+#define compile_ffs4(__x) \
+	( ((__x) & 0x3) ? \
+	    compile_ffs2(__x) : (compile_ffs2((__x) >> 2) + 2) )
+
+#define compile_ffs8(__x) \
+	( ((__x) & 0xf) ? \
+	    compile_ffs4(__x) : (compile_ffs4((__x) >> 4) + 4) )
+
+#define compile_ffs16(__x) \
+	( ((__x) & 0xff) ? \
+	    compile_ffs8(__x) : (compile_ffs8((__x) >> 8) + 8) )
+
+#define compile_ffs32(__x) \
+	( ((__x) & 0xffff) ? \
+	    compile_ffs16(__x) : (compile_ffs16((__x) >> 16) + 16) )
+
+/*
+ * This macro will check the requirements for the FIELD{8,16,32} macro=
s
+ * The mask should be a constant non-zero contiguous set of bits which
+ * does not exceed the given typelimit.
+ */
+#define FIELD_CHECK(__mask, __type)			\
+	BUILD_BUG_ON(!__builtin_constant_p(__mask) ||	\
+		     !(__mask) ||			\
+		     !is_valid_mask(__mask) ||		\
+		     (__mask) !=3D (__type)(__mask))	\
+
 #define FIELD8(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) !=3D (u8)(__mask));	\
+	FIELD_CHECK(__mask, u8);		\
 	(struct rt2x00_field8) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs8(__mask), (__mask)	\
 	};					\
 })
=20
 #define FIELD16(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) !=3D (u16)(__mask));\
+	FIELD_CHECK(__mask, u16);		\
 	(struct rt2x00_field16) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs16(__mask), (__mask)	\
 	};					\
 })
=20
 #define FIELD32(__mask)				\
 ({						\
-	BUILD_BUG_ON(!(__mask) ||		\
-		     !is_valid_mask(__mask) ||	\
-		     (__mask) !=3D (u32)(__mask));\
+	FIELD_CHECK(__mask, u32);		\
 	(struct rt2x00_field32) {		\
-		__ffs(__mask), (__mask)		\
+		compile_ffs32(__mask), (__mask)	\
 	};					\
 })
=20
--
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

Re: [PATCH 01/11 v3] rt2x00: Calculate register offset during compile time

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1157 bytes --]

On Tue, 2008-06-03 at 22:45 +0200, Ivo van Doorn wrote:
> By using __ffs() the register offsets were always calculated
> at run-time which all FIELD32/FIELD16 definitions were builtin
> constants. This means we can heavily optimize the register handling
> by allowing GCC to do all the work during compilation.
> 
> Add some compile_ffs() macros to perform the calculation at
> compile time. After this each rt2x00 module size is reduced
> by ~2500 bytes. And the stack size of several functions is reduced
> as well which further limits the number of rt2x00 results in
> 'make checkstack'.
> 
> v2: Merge GertJan's bugfix of patch [1/11] directly into this patch
>       instead of providing it as seperate patch.
> v3: Add extra parentheses when bitshifting __x
> 
> Signed-off-by: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
> ---
> John, this patch is a combination of patch [1/11] and [2/11] so please
> either apply those 2 patches or just this single patch to get the correct result :)

This, however, is no longer true ;) You need this one, not the other
two.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 01/11 v3] rt2x00: Calculate register offset during compile time

[Harvey Harrison]

On Tue, 2008-06-03 at 22:45 +0200, Ivo van Doorn wrote:
> By using __ffs() the register offsets were always calculated
> at run-time which all FIELD32/FIELD16 definitions were builtin
> constants. This means we can heavily optimize the register handling
> by allowing GCC to do all the work during compilation.
>=20
> Add some compile_ffs() macros to perform the calculation at
> compile time. After this each rt2x00 module size is reduced
> by ~2500 bytes. And the stack size of several functions is reduced
> as well which further limits the number of rt2x00 results in
> 'make checkstack'.
> +/*
> + * Macro's to find first set bit in a variable.
> + * These macro's behaves the same as the __ffs() function with
> + * the most important difference that this is done during
> + * compile-time rather then run-time.
> + */

#define const_ffs8(__x) ( \
	BUILD_BUG_ON(!__builtin_constant_p(__x));	\
	__builtin_choose_expr((__x) & 0x01, 0,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x02, 1,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x04, 2,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x08, 3,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x10, 4,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x20, 5,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x40, 6,		\
	=EF=BB=BF__builtin_choose_expr((__x) & 0x80, 7,		\
	8)))))))); )

#define const_ffs16(__x) ( \
	__builtin_choose_expr((__x) & 0xff,	\
 	const_ffs8(__x),			\
	const_ffs8((__x) >> 8) + 8); )

=EF=BB=BF
#define const_ffs32(__x) ( \
	__builtin_choose_expr((__x) & 0xffff,	\
	const_ffs16(__x),			\
	const_ffs16((__x) >> 16) + 16); )


Just a thought.

Harvey

--
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

Re: [PATCH 01/11 v3] rt2x00: Calculate register offset during compile time

[Ivo van Doorn]

On Tuesday 03 June 2008, Harvey Harrison wrote:
> On Tue, 2008-06-03 at 22:45 +0200, Ivo van Doorn wrote:
> > By using __ffs() the register offsets were always calculated
> > at run-time which all FIELD32/FIELD16 definitions were builtin
> > constants. This means we can heavily optimize the register handling
> > by allowing GCC to do all the work during compilation.
> >=20
> > Add some compile_ffs() macros to perform the calculation at
> > compile time. After this each rt2x00 module size is reduced
> > by ~2500 bytes. And the stack size of several functions is reduced
> > as well which further limits the number of rt2x00 results in
> > 'make checkstack'.
> > +/*
> > + * Macro's to find first set bit in a variable.
> > + * These macro's behaves the same as the __ffs() function with
> > + * the most important difference that this is done during
> > + * compile-time rather then run-time.
> > + */
>=20
> #define const_ffs8(__x) ( \
> 	BUILD_BUG_ON(!__builtin_constant_p(__x));	\
> 	__builtin_choose_expr((__x) & 0x01, 0,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x02, 1,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x04, 2,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x08, 3,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x10, 4,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x20, 5,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x40, 6,		\
> 	=EF=BB=BF__builtin_choose_expr((__x) & 0x80, 7,		\
> 	8)))))))); )
>=20
> #define const_ffs16(__x) ( \
> 	__builtin_choose_expr((__x) & 0xff,	\
>  	const_ffs8(__x),			\
> 	const_ffs8((__x) >> 8) + 8); )
>=20
> =EF=BB=BF
> #define const_ffs32(__x) ( \
> 	__builtin_choose_expr((__x) & 0xffff,	\
> 	const_ffs16(__x),			\
> 	const_ffs16((__x) >> 16) + 16); )
>=20
>=20
> Just a thought.

Thanks for the tipe, that does sound a lot cleaner then the ? : stateme=
nts,
I did a quick compilation check and it doesn't seem to have any influen=
ce.
I'll move the __builtin_choose_expr into rt2x00.git for testing and mov=
e
that patch upstream to wireless-2.6 later.

Ivo
--
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

[PATCH 02/11] rt2x00: Fix compile-time ffs calculation macros.

[Ivo van Doorn]

The compile_ffsx macros were missing a set of parentheses, which resulted in
wrong bit offset calculations.

This fixes rt61pci detection failures, whereby the rf chipset wasn't detected
properly due to improper bit shifting.

This patch is based on GertJan's report + patch to fix the parentheses problem

Signed-off-by: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h
index 03e846f..7999d54 100644
--- a/drivers/net/wireless/rt2x00/rt2x00reg.h
+++ b/drivers/net/wireless/rt2x00/rt2x00reg.h
@@ -145,19 +145,23 @@ struct rt2x00_field32 {
  * compile-time rather then run-time.
  */
 #define compile_ffs2(__x) \
-	((__x) & 0x1) ? 0 : 1
+	( ((__x) & 0x1) ? 0 : 1 )
 
 #define compile_ffs4(__x) \
-	((__x) & 0x3) ? compile_ffs2(__x) : compile_ffs2(__x >> 2) + 2
+	( ((__x) & 0x3) ? \
+	    compile_ffs2(__x) : (compile_ffs2(__x >> 2) + 2) )
 
 #define compile_ffs8(__x) \
-	((__x) & 0xf) ? compile_ffs4(__x) : compile_ffs4(__x >> 4) + 4
+	( ((__x) & 0xf) ? \
+	    compile_ffs4(__x) : (compile_ffs4(__x >> 4) + 4) )
 
 #define compile_ffs16(__x) \
-	((__x) & 0xff) ? compile_ffs8(__x) : compile_ffs8(__x >> 8) + 8
+	( ((__x) & 0xff) ? \
+	    compile_ffs8(__x) : (compile_ffs8(__x >> 8) + 8) )
 
 #define compile_ffs32(__x) \
-	((__x) & 0xffff) ? compile_ffs16(__x) : compile_ffs16(__x >> 16) + 16
+	( ((__x) & 0xffff) ? \
+	    compile_ffs16(__x) : (compile_ffs16(__x >> 16) + 16) )
 
 /*
  * This macro will check the requirements for the FIELD{8,16,32} macros
-- 
1.5.5.3

Re: [PATCH 02/11] rt2x00: Fix compile-time ffs calculation macros.

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

On Tue, 2008-06-03 at 20:29 +0200, Ivo van Doorn wrote:
> The compile_ffsx macros were missing a set of parentheses, which resulted in
> wrong bit offset calculations.
> 
> This fixes rt61pci detection failures, whereby the rf chipset wasn't detected
> properly due to improper bit shifting.
> 
> This patch is based on GertJan's report + patch to fix the parentheses problem

Sounds like you want to merge that with 1/11?

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH 02/11] rt2x00: Fix compile-time ffs calculation macros.

[Ivo van Doorn]

On Tuesday 03 June 2008, Johannes Berg wrote:
> On Tue, 2008-06-03 at 20:29 +0200, Ivo van Doorn wrote:
> > The compile_ffsx macros were missing a set of parentheses, which resulted in
> > wrong bit offset calculations.
> > 
> > This fixes rt61pci detection failures, whereby the rf chipset wasn't detected
> > properly due to improper bit shifting.
> > 
> > This patch is based on GertJan's report + patch to fix the parentheses problem
> 
> Sounds like you want to merge that with 1/11?

Good point. I'll resend it. :)

Ivo

[PATCH 03/11] rt2x00: Make rt2x00_set/get_field macros

[Ivo van Doorn]

The rt2x00_set_field functions are very often used,
but GCC is better able to optimize them when they
are macros instead of static inline functions.

After changing it to macro's each rt2x00 driver will
loose about ~3500 bytes in size.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   71 +++++++++++++------------------
 1 files changed, 30 insertions(+), 41 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00reg.h b/drivers/net/wireless/rt2x00/rt2x00reg.h
index 7999d54..170c10d 100644
--- a/drivers/net/wireless/rt2x00/rt2x00reg.h
+++ b/drivers/net/wireless/rt2x00/rt2x00reg.h
@@ -198,46 +198,35 @@ struct rt2x00_field32 {
 	};					\
 })
 
-static inline void rt2x00_set_field32(u32 *reg,
-				      const struct rt2x00_field32 field,
-				      const u32 value)
-{
-	*reg &= ~(field.bit_mask);
-	*reg |= (value << field.bit_offset) & field.bit_mask;
-}
-
-static inline u32 rt2x00_get_field32(const u32 reg,
-				     const struct rt2x00_field32 field)
-{
-	return (reg & field.bit_mask) >> field.bit_offset;
-}
-
-static inline void rt2x00_set_field16(u16 *reg,
-				      const struct rt2x00_field16 field,
-				      const u16 value)
-{
-	*reg &= ~(field.bit_mask);
-	*reg |= (value << field.bit_offset) & field.bit_mask;
-}
-
-static inline u16 rt2x00_get_field16(const u16 reg,
-				     const struct rt2x00_field16 field)
-{
-	return (reg & field.bit_mask) >> field.bit_offset;
-}
-
-static inline void rt2x00_set_field8(u8 *reg,
-				     const struct rt2x00_field8 field,
-				     const u8 value)
-{
-	*reg &= ~(field.bit_mask);
-	*reg |= (value << field.bit_offset) & field.bit_mask;
-}
-
-static inline u8 rt2x00_get_field8(const u8 reg,
-				   const struct rt2x00_field8 field)
-{
-	return (reg & field.bit_mask) >> field.bit_offset;
-}
+#define SET_FIELD(__reg, __type, __field, __value)\
+({						\
+	typecheck(__type, __field);		\
+	*(__reg) &= ~((__field).bit_mask);	\
+	*(__reg) |= ((__value) <<		\
+	    ((__field).bit_offset)) &		\
+	    ((__field).bit_mask);		\
+})
+
+#define GET_FIELD(__reg, __type, __field)	\
+({						\
+	typecheck(__type, __field);		\
+	((__reg) & ((__field).bit_mask)) >>	\
+	    ((__field).bit_offset);		\
+})
+
+#define rt2x00_set_field32(__reg, __field, __value) \
+	SET_FIELD(__reg, struct rt2x00_field32, __field, __value)
+#define rt2x00_get_field32(__reg, __field) \
+	GET_FIELD(__reg, struct rt2x00_field32, __field)
+
+#define rt2x00_set_field16(__reg, __field, __value) \
+	SET_FIELD(__reg, struct rt2x00_field16, __field, __value)
+#define rt2x00_get_field16(__reg, __field) \
+	GET_FIELD(__reg, struct rt2x00_field16, __field)
+
+#define rt2x00_set_field8(__reg, __field, __value) \
+	SET_FIELD(__reg, struct rt2x00_field8, __field, __value)
+#define rt2x00_get_field8(__reg, __field) \
+	GET_FIELD(__reg, struct rt2x00_field8, __field)
 
 #endif /* RT2X00REG_H */
-- 
1.5.5.3

[PATCH 04/11] rt2x00: Restrict DMA to 32-bit addresses.

[Ivo van Doorn]

From: Gertjan van Wingerde <gwingerde@kpnplanet.nl>

None of the rt2x00 PCI devices support 64-bit DMA addresses (they all only accept 32-bit
buffer addresses). Hence it makes no sense to try to enable 64-bit DMA addresses. Only
try to enable 32-bit DMA addresses.

Signed-off-by: Gertjan van Wingerde <gwingerde@kpnplanet.nl>
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00pci.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
index 70a3d13..d508c80 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -387,8 +387,7 @@ int rt2x00pci_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
 	if (pci_set_mwi(pci_dev))
 		ERROR_PROBE("MWI not available.\n");
 
-	if (pci_set_dma_mask(pci_dev, DMA_64BIT_MASK) &&
-	    pci_set_dma_mask(pci_dev, DMA_32BIT_MASK)) {
+	if (pci_set_dma_mask(pci_dev, DMA_32BIT_MASK)) {
 		ERROR_PROBE("PCI DMA not supported.\n");
 		retval = -EIO;
 		goto exit_disable_device;
-- 
1.5.5.3

[PATCH 05/11] rt2x00: Don't kill guardian_urb when it wasn't created

[Ivo van Doorn]

This fixes a "BUG: unable to handle kernel paging request"
bug in rt73usb which was caused by killing the guardian_urb
while it had never been allocated for rt73usb.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 52d12fd..36a087a 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -394,8 +394,11 @@ void rt2x00usb_disable_radio(struct rt2x00_dev *rt2x00dev)
 	}
 
 	/*
-	 * Kill guardian urb.
+	 * Kill guardian urb (if required by driver).
 	 */
+	if (!test_bit(DRIVER_REQUIRE_BEACON_GUARD, &rt2x00dev->flags))
+		return;
+
 	for (i = 0; i < rt2x00dev->bcn->limit; i++) {
 		bcn_priv = rt2x00dev->bcn->entries[i].priv_data;
 		if (bcn_priv->guardian_urb)
-- 
1.5.5.3

Re: [PATCH 05/11] rt2x00: Don't kill guardian_urb when it wasn't created

[Stefanik Gábor]

On Tue, Jun 3, 2008 at 8:29 PM, Ivo van Doorn <ivdoorn@gmail.com> wrote=
:
> This fixes a "BUG: unable to handle kernel paging request"
> bug in rt73usb which was caused by killing the guardian_urb
> while it had never been allocated for rt73usb.
>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
Acked-by: G=E1bor Stefanik <netrolller.3d@gmail.com>
> ---
>  drivers/net/wireless/rt2x00/rt2x00usb.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wi=
reless/rt2x00/rt2x00usb.c
> index 52d12fd..36a087a 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -394,8 +394,11 @@ void rt2x00usb_disable_radio(struct rt2x00_dev *=
rt2x00dev)
>        }
>
>        /*
> -        * Kill guardian urb.
> +        * Kill guardian urb (if required by driver).
>         */
> +       if (!test_bit(DRIVER_REQUIRE_BEACON_GUARD, &rt2x00dev->flags)=
)
> +               return;
> +
>        for (i =3D 0; i < rt2x00dev->bcn->limit; i++) {
>                bcn_priv =3D rt2x00dev->bcn->entries[i].priv_data;
>                if (bcn_priv->guardian_urb)
> --
> 1.5.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wirel=
ess" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)
--
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

[PATCH 06/11] rt2x00: Removed unused descriptor read in txdone

[Ivo van Doorn]

>From 5410c980823b6fd68d38f0230da51cdd70e0e7eb Mon Sep 17 00:00:00 2001
From: Ivo van Doorn <IvDoorn@gmail.com>
Date: Tue, 3 Jun 2008 18:49:49 +0200
Subject: [PATCH] rt2x00: Removed unused descriptor read in txdone

rt2x00usb doesn't need the TX descriptor in the TX done path.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 36a087a..86f1ab7 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -130,16 +130,12 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
 	struct queue_entry *entry = (struct queue_entry *)urb->context;
 	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	struct txdone_entry_desc txdesc;
-	__le32 *txd = (__le32 *)entry->skb->data;
 	enum data_queue_qid qid = skb_get_queue_mapping(entry->skb);
-	u32 word;
 
 	if (!test_bit(DEVICE_ENABLED_RADIO, &rt2x00dev->flags) ||
 	    !__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags))
 		return;
 
-	rt2x00_desc_read(txd, 0, &word);
-
 	/*
 	 * Remove the descriptor data from the buffer.
 	 */
-- 
1.5.5.3

[PATCH 07/11] rt2x00: Remove CTS/RTS check in tx()

[Ivo van Doorn]

>From 38a62373d637d6e4915136033c628143061049f4 Mon Sep 17 00:00:00 2001
From: Ivo van Doorn <IvDoorn@gmail.com>
Date: Tue, 3 Jun 2008 18:52:52 +0200
Subject: [PATCH] rt2x00: Remove CTS/RTS check in tx()

mac80211 doesn't send RTS or CTS-to-self frames through
the tx() callback functions so we don't need to check it.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00mac.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00mac.c b/drivers/net/wireless/rt2x00/rt2x00mac.c
index b02dbc8..97d9095 100644
--- a/drivers/net/wireless/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/rt2x00/rt2x00mac.c
@@ -135,18 +135,16 @@ int rt2x00mac_tx(struct ieee80211_hw *hw, struct sk_buff *skb)
 	}
 
 	/*
-	 * If CTS/RTS is required. and this frame is not CTS or RTS,
-	 * create and queue that frame first. But make sure we have
-	 * at least enough entries available to send this CTS/RTS
-	 * frame as well as the data frame.
+	 * If CTS/RTS is required. create and queue that frame first.
+	 * Make sure we have at least enough entries available to send
+	 * this CTS/RTS frame as well as the data frame.
 	 * Note that when the driver has set the set_rts_threshold()
 	 * callback function it doesn't need software generation of
-	 * neither RTS or CTS-to-self frames and handles everything
+	 * either RTS or CTS-to-self frame and handles everything
 	 * inside the hardware.
 	 */
 	frame_control = le16_to_cpu(ieee80211hdr->frame_control);
-	if (!is_rts_frame(frame_control) && !is_cts_frame(frame_control) &&
-	    (tx_info->flags & (IEEE80211_TX_CTL_USE_RTS_CTS |
+	if ((tx_info->flags & (IEEE80211_TX_CTL_USE_RTS_CTS |
 			       IEEE80211_TX_CTL_USE_CTS_PROTECT)) &&
 	    !rt2x00dev->ops->hw->set_rts_threshold) {
 		if (rt2x00queue_available(queue) <= 1) {
-- 
1.5.5.3

[PATCH 08/11] rt2x00: Move led initialization into function

[Ivo van Doorn]

Reduce code duplication by moving led structure initialization
into a per-driver function.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c |   32 +++++++++++-------------
 drivers/net/wireless/rt2x00/rt2500pci.c |   32 +++++++++++-------------
 drivers/net/wireless/rt2x00/rt2500usb.c |   32 +++++++++++-------------
 drivers/net/wireless/rt2x00/rt61pci.c   |   41 ++++++++++++-------------------
 drivers/net/wireless/rt2x00/rt73usb.c   |   41 ++++++++++++-------------------
 5 files changed, 77 insertions(+), 101 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 900140d..1101610 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -277,6 +277,17 @@ static int rt2400pci_blink_set(struct led_classdev *led_cdev,
 
 	return 0;
 }
+
+static void rt2400pci_init_led(struct rt2x00_dev *rt2x00dev,
+			       struct rt2x00_led *led,
+			       enum led_type type)
+{
+	led->rt2x00dev = rt2x00dev;
+	led->type = type;
+	led->led_dev.brightness_set = rt2400pci_brightness_set;
+	led->led_dev.blink_set = rt2400pci_blink_set;
+	led->flags = LED_INITIALIZED;
+}
 #endif /* CONFIG_RT2400PCI_LEDS */
 
 /*
@@ -1300,23 +1311,10 @@ static int rt2400pci_init_eeprom(struct rt2x00_dev *rt2x00dev)
 #ifdef CONFIG_RT2400PCI_LEDS
 	value = rt2x00_get_field16(eeprom, EEPROM_ANTENNA_LED_MODE);
 
-	rt2x00dev->led_radio.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_radio.type = LED_TYPE_RADIO;
-	rt2x00dev->led_radio.led_dev.brightness_set =
-	    rt2400pci_brightness_set;
-	rt2x00dev->led_radio.led_dev.blink_set =
-	    rt2400pci_blink_set;
-	rt2x00dev->led_radio.flags = LED_INITIALIZED;
-
-	if (value == LED_MODE_TXRX_ACTIVITY) {
-		rt2x00dev->led_qual.rt2x00dev = rt2x00dev;
-		rt2x00dev->led_qual.type = LED_TYPE_ACTIVITY;
-		rt2x00dev->led_qual.led_dev.brightness_set =
-		    rt2400pci_brightness_set;
-		rt2x00dev->led_qual.led_dev.blink_set =
-		    rt2400pci_blink_set;
-		rt2x00dev->led_qual.flags = LED_INITIALIZED;
-	}
+	rt2400pci_init_led(rt2x00dev, &rt2x00dev->led_radio, LED_TYPE_RADIO);
+	if (value == LED_MODE_TXRX_ACTIVITY)
+		rt2400pci_init_led(rt2x00dev, &rt2x00dev->led_qual,
+				   LED_TYPE_ACTIVITY);
 #endif /* CONFIG_RT2400PCI_LEDS */
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 6733509..0ac934a 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -277,6 +277,17 @@ static int rt2500pci_blink_set(struct led_classdev *led_cdev,
 
 	return 0;
 }
+
+static void rt2500pci_init_led(struct rt2x00_dev *rt2x00dev,
+			       struct rt2x00_led *led,
+			       enum led_type type)
+{
+	led->rt2x00dev = rt2x00dev;
+	led->type = type;
+	led->led_dev.brightness_set = rt2500pci_brightness_set;
+	led->led_dev.blink_set = rt2500pci_blink_set;
+	led->flags = LED_INITIALIZED;
+}
 #endif /* CONFIG_RT2500PCI_LEDS */
 
 /*
@@ -1478,23 +1489,10 @@ static int rt2500pci_init_eeprom(struct rt2x00_dev *rt2x00dev)
 #ifdef CONFIG_RT2500PCI_LEDS
 	value = rt2x00_get_field16(eeprom, EEPROM_ANTENNA_LED_MODE);
 
-	rt2x00dev->led_radio.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_radio.type = LED_TYPE_RADIO;
-	rt2x00dev->led_radio.led_dev.brightness_set =
-	    rt2500pci_brightness_set;
-	rt2x00dev->led_radio.led_dev.blink_set =
-	    rt2500pci_blink_set;
-	rt2x00dev->led_radio.flags = LED_INITIALIZED;
-
-	if (value == LED_MODE_TXRX_ACTIVITY) {
-		rt2x00dev->led_qual.rt2x00dev = rt2x00dev;
-		rt2x00dev->led_qual.type = LED_TYPE_ACTIVITY;
-		rt2x00dev->led_qual.led_dev.brightness_set =
-		    rt2500pci_brightness_set;
-		rt2x00dev->led_qual.led_dev.blink_set =
-		    rt2500pci_blink_set;
-		rt2x00dev->led_qual.flags = LED_INITIALIZED;
-	}
+	rt2500pci_init_led(rt2x00dev, &rt2x00dev->led_radio, LED_TYPE_RADIO);
+	if (value == LED_MODE_TXRX_ACTIVITY)
+		rt2500pci_init_led(rt2x00dev, &rt2x00dev->led_qual,
+				   LED_TYPE_ACTIVITY);
 #endif /* CONFIG_RT2500PCI_LEDS */
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index cca1504..0027f4b 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -316,6 +316,17 @@ static int rt2500usb_blink_set(struct led_classdev *led_cdev,
 
 	return 0;
 }
+
+static void rt2500usb_init_led(struct rt2x00_dev *rt2x00dev,
+			       struct rt2x00_led *led,
+			       enum led_type type)
+{
+	led->rt2x00dev = rt2x00dev;
+	led->type = type;
+	led->led_dev.brightness_set = rt2500usb_brightness_set;
+	led->led_dev.blink_set = rt2500usb_blink_set;
+	led->flags = LED_INITIALIZED;
+}
 #endif /* CONFIG_RT2500USB_LEDS */
 
 /*
@@ -1377,23 +1388,10 @@ static int rt2500usb_init_eeprom(struct rt2x00_dev *rt2x00dev)
 #ifdef CONFIG_RT2500USB_LEDS
 	value = rt2x00_get_field16(eeprom, EEPROM_ANTENNA_LED_MODE);
 
-	rt2x00dev->led_radio.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_radio.type = LED_TYPE_RADIO;
-	rt2x00dev->led_radio.led_dev.brightness_set =
-	    rt2500usb_brightness_set;
-	rt2x00dev->led_radio.led_dev.blink_set =
-	    rt2500usb_blink_set;
-	rt2x00dev->led_radio.flags = LED_INITIALIZED;
-
-	if (value == LED_MODE_TXRX_ACTIVITY) {
-		rt2x00dev->led_qual.rt2x00dev = rt2x00dev;
-		rt2x00dev->led_qual.type = LED_TYPE_ACTIVITY;
-		rt2x00dev->led_qual.led_dev.brightness_set =
-		    rt2500usb_brightness_set;
-		rt2x00dev->led_qual.led_dev.blink_set =
-		    rt2500usb_blink_set;
-		rt2x00dev->led_qual.flags = LED_INITIALIZED;
-	}
+	rt2500usb_init_led(rt2x00dev, &rt2x00dev->led_radio, LED_TYPE_RADIO);
+	if (value == LED_MODE_TXRX_ACTIVITY)
+		rt2500usb_init_led(rt2x00dev, &rt2x00dev->led_qual,
+				   LED_TYPE_ACTIVITY);
 #endif /* CONFIG_RT2500USB_LEDS */
 
 	/*
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index e13ed5c..f03d21d 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -330,6 +330,17 @@ static int rt61pci_blink_set(struct led_classdev *led_cdev,
 
 	return 0;
 }
+
+static void rt61pci_init_led(struct rt2x00_dev *rt2x00dev,
+			     struct rt2x00_led *led,
+			     enum led_type type)
+{
+	led->rt2x00dev = rt2x00dev;
+	led->type = type;
+	led->led_dev.brightness_set = rt61pci_brightness_set;
+	led->led_dev.blink_set = rt61pci_blink_set;
+	led->flags = LED_INITIALIZED;
+}
 #endif /* CONFIG_RT61PCI_LEDS */
 
 /*
@@ -2067,31 +2078,11 @@ static int rt61pci_init_eeprom(struct rt2x00_dev *rt2x00dev)
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_LED, &eeprom);
 	value = rt2x00_get_field16(eeprom, EEPROM_LED_LED_MODE);
 
-	rt2x00dev->led_radio.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_radio.type = LED_TYPE_RADIO;
-	rt2x00dev->led_radio.led_dev.brightness_set =
-	    rt61pci_brightness_set;
-	rt2x00dev->led_radio.led_dev.blink_set =
-	    rt61pci_blink_set;
-	rt2x00dev->led_radio.flags = LED_INITIALIZED;
-
-	rt2x00dev->led_assoc.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_assoc.type = LED_TYPE_ASSOC;
-	rt2x00dev->led_assoc.led_dev.brightness_set =
-	    rt61pci_brightness_set;
-	rt2x00dev->led_assoc.led_dev.blink_set =
-	    rt61pci_blink_set;
-	rt2x00dev->led_assoc.flags = LED_INITIALIZED;
-
-	if (value == LED_MODE_SIGNAL_STRENGTH) {
-		rt2x00dev->led_qual.rt2x00dev = rt2x00dev;
-		rt2x00dev->led_qual.type = LED_TYPE_QUALITY;
-		rt2x00dev->led_qual.led_dev.brightness_set =
-		    rt61pci_brightness_set;
-		rt2x00dev->led_qual.led_dev.blink_set =
-		    rt61pci_blink_set;
-		rt2x00dev->led_qual.flags = LED_INITIALIZED;
-	}
+	rt61pci_init_led(rt2x00dev, &rt2x00dev->led_radio, LED_TYPE_RADIO);
+	rt61pci_init_led(rt2x00dev, &rt2x00dev->led_assoc, LED_TYPE_ASSOC);
+	if (value == LED_MODE_SIGNAL_STRENGTH)
+		rt61pci_init_led(rt2x00dev, &rt2x00dev->led_qual,
+				 LED_TYPE_QUALITY);
 
 	rt2x00_set_field16(&rt2x00dev->led_mcu_reg, MCU_LEDCS_LED_MODE, value);
 	rt2x00_set_field16(&rt2x00dev->led_mcu_reg, MCU_LEDCS_POLARITY_GPIO_0,
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 26c2e0a..779fa5b 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -335,6 +335,17 @@ static int rt73usb_blink_set(struct led_classdev *led_cdev,
 
 	return 0;
 }
+
+static void rt73usb_init_led(struct rt2x00_dev *rt2x00dev,
+			     struct rt2x00_led *led,
+			     enum led_type type)
+{
+	led->rt2x00dev = rt2x00dev;
+	led->type = type;
+	led->led_dev.brightness_set = rt73usb_brightness_set;
+	led->led_dev.blink_set = rt73usb_blink_set;
+	led->flags = LED_INITIALIZED;
+}
 #endif /* CONFIG_RT73USB_LEDS */
 
 /*
@@ -1620,31 +1631,11 @@ static int rt73usb_init_eeprom(struct rt2x00_dev *rt2x00dev)
 #ifdef CONFIG_RT73USB_LEDS
 	rt2x00_eeprom_read(rt2x00dev, EEPROM_LED, &eeprom);
 
-	rt2x00dev->led_radio.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_radio.type = LED_TYPE_RADIO;
-	rt2x00dev->led_radio.led_dev.brightness_set =
-	    rt73usb_brightness_set;
-	rt2x00dev->led_radio.led_dev.blink_set =
-	    rt73usb_blink_set;
-	rt2x00dev->led_radio.flags = LED_INITIALIZED;
-
-	rt2x00dev->led_assoc.rt2x00dev = rt2x00dev;
-	rt2x00dev->led_assoc.type = LED_TYPE_ASSOC;
-	rt2x00dev->led_assoc.led_dev.brightness_set =
-	    rt73usb_brightness_set;
-	rt2x00dev->led_assoc.led_dev.blink_set =
-	    rt73usb_blink_set;
-	rt2x00dev->led_assoc.flags = LED_INITIALIZED;
-
-	if (value == LED_MODE_SIGNAL_STRENGTH) {
-		rt2x00dev->led_qual.rt2x00dev = rt2x00dev;
-		rt2x00dev->led_qual.type = LED_TYPE_QUALITY;
-		rt2x00dev->led_qual.led_dev.brightness_set =
-		    rt73usb_brightness_set;
-		rt2x00dev->led_qual.led_dev.blink_set =
-		    rt73usb_blink_set;
-		rt2x00dev->led_qual.flags = LED_INITIALIZED;
-	}
+	rt73usb_init_led(rt2x00dev, &rt2x00dev->led_radio, LED_TYPE_RADIO);
+	rt73usb_init_led(rt2x00dev, &rt2x00dev->led_assoc, LED_TYPE_ASSOC);
+	if (value == LED_MODE_SIGNAL_STRENGTH)
+		rt73usb_init_led(rt2x00dev, &rt2x00dev->led_qual,
+				 LED_TYPE_QUALITY);
 
 	rt2x00_set_field16(&rt2x00dev->led_mcu_reg, MCU_LEDCS_LED_MODE, value);
 	rt2x00_set_field16(&rt2x00dev->led_mcu_reg, MCU_LEDCS_POLARITY_GPIO_0,
-- 
1.5.5.3

[PATCH 10/11] rt2x00: Fix queue initialization

[Ivo van Doorn]

qid should be initialized to QID_BEACON and QID_ATIM
for the beacon and atim quue. This makes checking for
a particular queue much saner, and it shouldn't harm,
because the only places where the value is send to
the hardware, we are allowed to send any value we
want since it is only used as argument in the
TX done register.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00queue.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index e69ef4b..6f3aa0f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -439,7 +439,8 @@ int rt2x00queue_allocate(struct rt2x00_dev *rt2x00dev)
 	 * TX: qid = QID_AC_BE + index
 	 * TX: cw_min: 2^5 = 32.
 	 * TX: cw_max: 2^10 = 1024.
-	 * BCN & Atim: qid = QID_MGMT
+	 * BCN: qid = QID_BEACON
+	 * ATIM: qid = QID_ATIM
 	 */
 	rt2x00queue_init(rt2x00dev, rt2x00dev->rx, QID_RX);
 
@@ -447,9 +448,9 @@ int rt2x00queue_allocate(struct rt2x00_dev *rt2x00dev)
 	tx_queue_for_each(rt2x00dev, queue)
 		rt2x00queue_init(rt2x00dev, queue, qid++);
 
-	rt2x00queue_init(rt2x00dev, &rt2x00dev->bcn[0], QID_MGMT);
+	rt2x00queue_init(rt2x00dev, &rt2x00dev->bcn[0], QID_BEACON);
 	if (req_atim)
-		rt2x00queue_init(rt2x00dev, &rt2x00dev->bcn[1], QID_MGMT);
+		rt2x00queue_init(rt2x00dev, &rt2x00dev->bcn[1], QID_ATIM);
 
 	return 0;
 }
-- 
1.5.5.3

[PATCH 11/11] rt2x00: Release rt2x00 2.1.7

[Ivo van Doorn]

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2x00.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 15ec797..2f79336 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -44,7 +44,7 @@
 /*
  * Module information.
  */
-#define DRV_VERSION	"2.1.6"
+#define DRV_VERSION	"2.1.7"
 #define DRV_PROJECT	"http://rt2x00.serialmonkey.com"
 
 /*
-- 
1.5.5.3

[PATCH 09/11] rt2x00: Cleanup/optimize set_state() function callback function

[Ivo van Doorn]

>From 36d1e3ea615aec2dbafa2493021cf3117cc8f57c Mon Sep 17 00:00:00 2001
From: Ivo van Doorn <IvDoorn@gmail.com>
Date: Tue, 3 Jun 2008 18:58:56 +0200
Subject: [PATCH] rt2x00: Cleanup/optimize set_state() function callback function

* Reduce goto usage
* Mark if-statements which are true on hardware error unlikely()
* Cleanup debug messages

This makes the code look nicer and be better optimized since
the chance of hardware errors should be very small.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 drivers/net/wireless/rt2x00/rt2400pci.c |   56 ++++++++++++++---------------
 drivers/net/wireless/rt2x00/rt2500pci.c |   56 ++++++++++++++---------------
 drivers/net/wireless/rt2x00/rt2500usb.c |   44 +++++++++++++---------
 drivers/net/wireless/rt2x00/rt2x00dev.c |    3 ++
 drivers/net/wireless/rt2x00/rt61pci.c   |   61 +++++++++++++++----------------
 drivers/net/wireless/rt2x00/rt73usb.c   |   49 ++++++++++++++-----------
 6 files changed, 140 insertions(+), 129 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2400pci.c b/drivers/net/wireless/rt2x00/rt2400pci.c
index 1101610..94226b4 100644
--- a/drivers/net/wireless/rt2x00/rt2400pci.c
+++ b/drivers/net/wireless/rt2x00/rt2400pci.c
@@ -792,25 +792,32 @@ static int rt2400pci_init_registers(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
-static int rt2400pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+static int rt2400pci_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 {
 	unsigned int i;
-	u16 eeprom;
-	u8 reg_id;
 	u8 value;
 
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt2400pci_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
-			goto continue_csr_init;
-		NOTICE(rt2x00dev, "Waiting for BBP register.\n");
+			return 0;
 		udelay(REGISTER_BUSY_DELAY);
 	}
 
 	ERROR(rt2x00dev, "BBP register access failed, aborting.\n");
 	return -EACCES;
+}
+
+static int rt2400pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+{
+	unsigned int i;
+	u16 eeprom;
+	u8 reg_id;
+	u8 value;
+
+	if (unlikely(rt2400pci_wait_bbp_ready(rt2x00dev)))
+		return -EACCES;
 
-continue_csr_init:
 	rt2400pci_bbp_write(rt2x00dev, 1, 0x00);
 	rt2400pci_bbp_write(rt2x00dev, 3, 0x27);
 	rt2400pci_bbp_write(rt2x00dev, 4, 0x08);
@@ -849,7 +856,8 @@ static void rt2400pci_toggle_rx(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);
 	rt2x00_set_field32(&reg, RXCSR0_DISABLE_RX,
-			   state == STATE_RADIO_RX_OFF);
+			   (state == STATE_RADIO_RX_OFF) ||
+			   (state == STATE_RADIO_RX_OFF_LINK));
 	rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
 }
 
@@ -886,17 +894,10 @@ static int rt2400pci_enable_radio(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize all registers.
 	 */
-	if (rt2400pci_init_queues(rt2x00dev) ||
-	    rt2400pci_init_registers(rt2x00dev) ||
-	    rt2400pci_init_bbp(rt2x00dev)) {
-		ERROR(rt2x00dev, "Register initialization failed.\n");
+	if (unlikely(rt2400pci_init_queues(rt2x00dev) ||
+		     rt2400pci_init_registers(rt2x00dev) ||
+		     rt2400pci_init_bbp(rt2x00dev)))
 		return -EIO;
-	}
-
-	/*
-	 * Enable interrupts.
-	 */
-	rt2400pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_ON);
 
 	return 0;
 }
@@ -918,11 +919,6 @@ static void rt2400pci_disable_radio(struct rt2x00_dev *rt2x00dev)
 	rt2x00pci_register_read(rt2x00dev, TXCSR0, &reg);
 	rt2x00_set_field32(&reg, TXCSR0_ABORT, 1);
 	rt2x00pci_register_write(rt2x00dev, TXCSR0, reg);
-
-	/*
-	 * Disable interrupts.
-	 */
-	rt2400pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_OFF);
 }
 
 static int rt2400pci_set_state(struct rt2x00_dev *rt2x00dev,
@@ -957,10 +953,6 @@ static int rt2400pci_set_state(struct rt2x00_dev *rt2x00dev,
 		msleep(10);
 	}
 
-	NOTICE(rt2x00dev, "Device failed to enter state %d, "
-	       "current device state: bbp %d and rf %d.\n",
-	       state, bbp_state, rf_state);
-
 	return -EBUSY;
 }
 
@@ -978,11 +970,13 @@ static int rt2400pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	case STATE_RADIO_RX_ON:
 	case STATE_RADIO_RX_ON_LINK:
-		rt2400pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_ON);
-		break;
 	case STATE_RADIO_RX_OFF:
 	case STATE_RADIO_RX_OFF_LINK:
-		rt2400pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_OFF);
+		rt2400pci_toggle_rx(rt2x00dev, state);
+		break;
+	case STATE_RADIO_IRQ_ON:
+	case STATE_RADIO_IRQ_OFF:
+		rt2400pci_toggle_irq(rt2x00dev, state);
 		break;
 	case STATE_DEEP_SLEEP:
 	case STATE_SLEEP:
@@ -995,6 +989,10 @@ static int rt2400pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	}
 
+	if (unlikely(retval))
+		ERROR(rt2x00dev, "Device failed to enter state %d (%d).\n",
+		      state, retval);
+
 	return retval;
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2500pci.c b/drivers/net/wireless/rt2x00/rt2500pci.c
index 0ac934a..c8cf8c1 100644
--- a/drivers/net/wireless/rt2x00/rt2500pci.c
+++ b/drivers/net/wireless/rt2x00/rt2500pci.c
@@ -935,25 +935,32 @@ static int rt2500pci_init_registers(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
-static int rt2500pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+static int rt2500pci_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 {
 	unsigned int i;
-	u16 eeprom;
-	u8 reg_id;
 	u8 value;
 
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt2500pci_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
-			goto continue_csr_init;
-		NOTICE(rt2x00dev, "Waiting for BBP register.\n");
+			return 0;
 		udelay(REGISTER_BUSY_DELAY);
 	}
 
 	ERROR(rt2x00dev, "BBP register access failed, aborting.\n");
 	return -EACCES;
+}
+
+static int rt2500pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+{
+	unsigned int i;
+	u16 eeprom;
+	u8 reg_id;
+	u8 value;
+
+	if (unlikely(rt2500pci_wait_bbp_ready(rt2x00dev)))
+		return -EACCES;
 
-continue_csr_init:
 	rt2500pci_bbp_write(rt2x00dev, 3, 0x02);
 	rt2500pci_bbp_write(rt2x00dev, 4, 0x19);
 	rt2500pci_bbp_write(rt2x00dev, 14, 0x1c);
@@ -1008,7 +1015,8 @@ static void rt2500pci_toggle_rx(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00pci_register_read(rt2x00dev, RXCSR0, &reg);
 	rt2x00_set_field32(&reg, RXCSR0_DISABLE_RX,
-			   state == STATE_RADIO_RX_OFF);
+			   (state == STATE_RADIO_RX_OFF) ||
+			   (state == STATE_RADIO_RX_OFF_LINK));
 	rt2x00pci_register_write(rt2x00dev, RXCSR0, reg);
 }
 
@@ -1045,17 +1053,10 @@ static int rt2500pci_enable_radio(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize all registers.
 	 */
-	if (rt2500pci_init_queues(rt2x00dev) ||
-	    rt2500pci_init_registers(rt2x00dev) ||
-	    rt2500pci_init_bbp(rt2x00dev)) {
-		ERROR(rt2x00dev, "Register initialization failed.\n");
+	if (unlikely(rt2500pci_init_queues(rt2x00dev) ||
+		     rt2500pci_init_registers(rt2x00dev) ||
+		     rt2500pci_init_bbp(rt2x00dev)))
 		return -EIO;
-	}
-
-	/*
-	 * Enable interrupts.
-	 */
-	rt2500pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_ON);
 
 	return 0;
 }
@@ -1077,11 +1078,6 @@ static void rt2500pci_disable_radio(struct rt2x00_dev *rt2x00dev)
 	rt2x00pci_register_read(rt2x00dev, TXCSR0, &reg);
 	rt2x00_set_field32(&reg, TXCSR0_ABORT, 1);
 	rt2x00pci_register_write(rt2x00dev, TXCSR0, reg);
-
-	/*
-	 * Disable interrupts.
-	 */
-	rt2500pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_OFF);
 }
 
 static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
@@ -1116,10 +1112,6 @@ static int rt2500pci_set_state(struct rt2x00_dev *rt2x00dev,
 		msleep(10);
 	}
 
-	NOTICE(rt2x00dev, "Device failed to enter state %d, "
-	       "current device state: bbp %d and rf %d.\n",
-	       state, bbp_state, rf_state);
-
 	return -EBUSY;
 }
 
@@ -1137,11 +1129,13 @@ static int rt2500pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	case STATE_RADIO_RX_ON:
 	case STATE_RADIO_RX_ON_LINK:
-		rt2500pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_ON);
-		break;
 	case STATE_RADIO_RX_OFF:
 	case STATE_RADIO_RX_OFF_LINK:
-		rt2500pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_OFF);
+		rt2500pci_toggle_rx(rt2x00dev, state);
+		break;
+	case STATE_RADIO_IRQ_ON:
+	case STATE_RADIO_IRQ_OFF:
+		rt2500pci_toggle_irq(rt2x00dev, state);
 		break;
 	case STATE_DEEP_SLEEP:
 	case STATE_SLEEP:
@@ -1154,6 +1148,10 @@ static int rt2500pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	}
 
+	if (unlikely(retval))
+		ERROR(rt2x00dev, "Device failed to enter state %d (%d).\n",
+		      state, retval);
+
 	return retval;
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2500usb.c b/drivers/net/wireless/rt2x00/rt2500usb.c
index 0027f4b..0d51b74 100644
--- a/drivers/net/wireless/rt2x00/rt2500usb.c
+++ b/drivers/net/wireless/rt2x00/rt2500usb.c
@@ -858,25 +858,32 @@ static int rt2500usb_init_registers(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
-static int rt2500usb_init_bbp(struct rt2x00_dev *rt2x00dev)
+static int rt2500usb_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 {
 	unsigned int i;
-	u16 eeprom;
 	u8 value;
-	u8 reg_id;
 
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt2500usb_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
-			goto continue_csr_init;
-		NOTICE(rt2x00dev, "Waiting for BBP register.\n");
+			return 0;
 		udelay(REGISTER_BUSY_DELAY);
 	}
 
 	ERROR(rt2x00dev, "BBP register access failed, aborting.\n");
 	return -EACCES;
+}
+
+static int rt2500usb_init_bbp(struct rt2x00_dev *rt2x00dev)
+{
+	unsigned int i;
+	u16 eeprom;
+	u8 value;
+	u8 reg_id;
+
+	if (unlikely(rt2500usb_wait_bbp_ready(rt2x00dev)))
+		return -EACCES;
 
-continue_csr_init:
 	rt2500usb_bbp_write(rt2x00dev, 3, 0x02);
 	rt2500usb_bbp_write(rt2x00dev, 4, 0x19);
 	rt2500usb_bbp_write(rt2x00dev, 14, 0x1c);
@@ -932,7 +939,8 @@ static void rt2500usb_toggle_rx(struct rt2x00_dev *rt2x00dev,
 
 	rt2500usb_register_read(rt2x00dev, TXRX_CSR2, &reg);
 	rt2x00_set_field16(&reg, TXRX_CSR2_DISABLE_RX,
-			   state == STATE_RADIO_RX_OFF);
+			   (state == STATE_RADIO_RX_OFF) ||
+			   (state == STATE_RADIO_RX_OFF_LINK));
 	rt2500usb_register_write(rt2x00dev, TXRX_CSR2, reg);
 }
 
@@ -941,11 +949,9 @@ static int rt2500usb_enable_radio(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize all registers.
 	 */
-	if (rt2500usb_init_registers(rt2x00dev) ||
-	    rt2500usb_init_bbp(rt2x00dev)) {
-		ERROR(rt2x00dev, "Register initialization failed.\n");
+	if (unlikely(rt2500usb_init_registers(rt2x00dev) ||
+		     rt2500usb_init_bbp(rt2x00dev)))
 		return -EIO;
-	}
 
 	return 0;
 }
@@ -998,10 +1004,6 @@ static int rt2500usb_set_state(struct rt2x00_dev *rt2x00dev,
 		msleep(30);
 	}
 
-	NOTICE(rt2x00dev, "Device failed to enter state %d, "
-	       "current device state: bbp %d and rf %d.\n",
-	       state, bbp_state, rf_state);
-
 	return -EBUSY;
 }
 
@@ -1019,11 +1021,13 @@ static int rt2500usb_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	case STATE_RADIO_RX_ON:
 	case STATE_RADIO_RX_ON_LINK:
-		rt2500usb_toggle_rx(rt2x00dev, STATE_RADIO_RX_ON);
-		break;
 	case STATE_RADIO_RX_OFF:
 	case STATE_RADIO_RX_OFF_LINK:
-		rt2500usb_toggle_rx(rt2x00dev, STATE_RADIO_RX_OFF);
+		rt2500usb_toggle_rx(rt2x00dev, state);
+		break;
+	case STATE_RADIO_IRQ_ON:
+	case STATE_RADIO_IRQ_OFF:
+		/* No support, but no error either */
 		break;
 	case STATE_DEEP_SLEEP:
 	case STATE_SLEEP:
@@ -1036,6 +1040,10 @@ static int rt2500usb_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	}
 
+	if (unlikely(retval))
+		ERROR(rt2x00dev, "Device failed to enter state %d (%d).\n",
+		      state, retval);
+
 	return retval;
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index dc5ab90..48f4aec 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -112,6 +112,8 @@ int rt2x00lib_enable_radio(struct rt2x00_dev *rt2x00dev)
 	if (status)
 		return status;
 
+	rt2x00dev->ops->lib->set_device_state(rt2x00dev, STATE_RADIO_IRQ_ON);
+
 	rt2x00leds_led_radio(rt2x00dev, true);
 	rt2x00led_led_activity(rt2x00dev, true);
 
@@ -157,6 +159,7 @@ void rt2x00lib_disable_radio(struct rt2x00_dev *rt2x00dev)
 	 * Disable radio.
 	 */
 	rt2x00dev->ops->lib->set_device_state(rt2x00dev, STATE_RADIO_OFF);
+	rt2x00dev->ops->lib->set_device_state(rt2x00dev, STATE_RADIO_IRQ_OFF);
 	rt2x00led_led_activity(rt2x00dev, false);
 	rt2x00leds_led_radio(rt2x00dev, false);
 }
diff --git a/drivers/net/wireless/rt2x00/rt61pci.c b/drivers/net/wireless/rt2x00/rt61pci.c
index f03d21d..aa9ef66 100644
--- a/drivers/net/wireless/rt2x00/rt61pci.c
+++ b/drivers/net/wireless/rt2x00/rt61pci.c
@@ -1281,25 +1281,32 @@ static int rt61pci_init_registers(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
-static int rt61pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+static int rt61pci_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 {
 	unsigned int i;
-	u16 eeprom;
-	u8 reg_id;
 	u8 value;
 
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt61pci_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
-			goto continue_csr_init;
-		NOTICE(rt2x00dev, "Waiting for BBP register.\n");
+			return 0;
 		udelay(REGISTER_BUSY_DELAY);
 	}
 
 	ERROR(rt2x00dev, "BBP register access failed, aborting.\n");
 	return -EACCES;
+}
+
+static int rt61pci_init_bbp(struct rt2x00_dev *rt2x00dev)
+{
+	unsigned int i;
+	u16 eeprom;
+	u8 reg_id;
+	u8 value;
+
+	if (unlikely(rt61pci_wait_bbp_ready(rt2x00dev)))
+		return -EACCES;
 
-continue_csr_init:
 	rt61pci_bbp_write(rt2x00dev, 3, 0x00);
 	rt61pci_bbp_write(rt2x00dev, 15, 0x30);
 	rt61pci_bbp_write(rt2x00dev, 21, 0xc8);
@@ -1348,7 +1355,8 @@ static void rt61pci_toggle_rx(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00pci_register_read(rt2x00dev, TXRX_CSR0, &reg);
 	rt2x00_set_field32(&reg, TXRX_CSR0_DISABLE_RX,
-			   state == STATE_RADIO_RX_OFF);
+			   (state == STATE_RADIO_RX_OFF) ||
+			   (state == STATE_RADIO_RX_OFF_LINK));
 	rt2x00pci_register_write(rt2x00dev, TXRX_CSR0, reg);
 }
 
@@ -1400,17 +1408,10 @@ static int rt61pci_enable_radio(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize all registers.
 	 */
-	if (rt61pci_init_queues(rt2x00dev) ||
-	    rt61pci_init_registers(rt2x00dev) ||
-	    rt61pci_init_bbp(rt2x00dev)) {
-		ERROR(rt2x00dev, "Register initialization failed.\n");
+	if (unlikely(rt61pci_init_queues(rt2x00dev) ||
+		     rt61pci_init_registers(rt2x00dev) ||
+		     rt61pci_init_bbp(rt2x00dev)))
 		return -EIO;
-	}
-
-	/*
-	 * Enable interrupts.
-	 */
-	rt61pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_ON);
 
 	/*
 	 * Enable RX.
@@ -1442,11 +1443,6 @@ static void rt61pci_disable_radio(struct rt2x00_dev *rt2x00dev)
 	rt2x00_set_field32(&reg, TX_CNTL_CSR_ABORT_TX_AC2, 1);
 	rt2x00_set_field32(&reg, TX_CNTL_CSR_ABORT_TX_AC3, 1);
 	rt2x00pci_register_write(rt2x00dev, TX_CNTL_CSR, reg);
-
-	/*
-	 * Disable interrupts.
-	 */
-	rt61pci_toggle_irq(rt2x00dev, STATE_RADIO_IRQ_OFF);
 }
 
 static int rt61pci_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state)
@@ -1454,7 +1450,6 @@ static int rt61pci_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state)
 	u32 reg;
 	unsigned int i;
 	char put_to_sleep;
-	char current_state;
 
 	put_to_sleep = (state != STATE_AWAKE);
 
@@ -1470,16 +1465,12 @@ static int rt61pci_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state)
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt2x00pci_register_read(rt2x00dev, MAC_CSR12, &reg);
-		current_state =
-		    rt2x00_get_field32(reg, MAC_CSR12_BBP_CURRENT_STATE);
-		if (current_state == !put_to_sleep)
+		state = rt2x00_get_field32(reg, MAC_CSR12_BBP_CURRENT_STATE);
+		if (state == !put_to_sleep)
 			return 0;
 		msleep(10);
 	}
 
-	NOTICE(rt2x00dev, "Device failed to enter state %d, "
-	       "current device state %d.\n", !put_to_sleep, current_state);
-
 	return -EBUSY;
 }
 
@@ -1497,11 +1488,13 @@ static int rt61pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	case STATE_RADIO_RX_ON:
 	case STATE_RADIO_RX_ON_LINK:
-		rt61pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_ON);
-		break;
 	case STATE_RADIO_RX_OFF:
 	case STATE_RADIO_RX_OFF_LINK:
-		rt61pci_toggle_rx(rt2x00dev, STATE_RADIO_RX_OFF);
+		rt61pci_toggle_rx(rt2x00dev, state);
+		break;
+	case STATE_RADIO_IRQ_ON:
+	case STATE_RADIO_IRQ_OFF:
+		rt61pci_toggle_irq(rt2x00dev, state);
 		break;
 	case STATE_DEEP_SLEEP:
 	case STATE_SLEEP:
@@ -1514,6 +1507,10 @@ static int rt61pci_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	}
 
+	if (unlikely(retval))
+		ERROR(rt2x00dev, "Device failed to enter state %d (%d).\n",
+		      state, retval);
+
 	return retval;
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt73usb.c b/drivers/net/wireless/rt2x00/rt73usb.c
index 779fa5b..db1fc13 100644
--- a/drivers/net/wireless/rt2x00/rt73usb.c
+++ b/drivers/net/wireless/rt2x00/rt73usb.c
@@ -1095,25 +1095,32 @@ static int rt73usb_init_registers(struct rt2x00_dev *rt2x00dev)
 	return 0;
 }
 
-static int rt73usb_init_bbp(struct rt2x00_dev *rt2x00dev)
+static int rt73usb_wait_bbp_ready(struct rt2x00_dev *rt2x00dev)
 {
 	unsigned int i;
-	u16 eeprom;
-	u8 reg_id;
 	u8 value;
 
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt73usb_bbp_read(rt2x00dev, 0, &value);
 		if ((value != 0xff) && (value != 0x00))
-			goto continue_csr_init;
-		NOTICE(rt2x00dev, "Waiting for BBP register.\n");
+			return 0;
 		udelay(REGISTER_BUSY_DELAY);
 	}
 
 	ERROR(rt2x00dev, "BBP register access failed, aborting.\n");
 	return -EACCES;
+}
+
+static int rt73usb_init_bbp(struct rt2x00_dev *rt2x00dev)
+{
+	unsigned int i;
+	u16 eeprom;
+	u8 reg_id;
+	u8 value;
+
+	if (unlikely(rt73usb_wait_bbp_ready(rt2x00dev)))
+		return -EACCES;
 
-continue_csr_init:
 	rt73usb_bbp_write(rt2x00dev, 3, 0x80);
 	rt73usb_bbp_write(rt2x00dev, 15, 0x30);
 	rt73usb_bbp_write(rt2x00dev, 21, 0xc8);
@@ -1163,7 +1170,8 @@ static void rt73usb_toggle_rx(struct rt2x00_dev *rt2x00dev,
 
 	rt73usb_register_read(rt2x00dev, TXRX_CSR0, &reg);
 	rt2x00_set_field32(&reg, TXRX_CSR0_DISABLE_RX,
-			   state == STATE_RADIO_RX_OFF);
+			   (state == STATE_RADIO_RX_OFF) ||
+			   (state == STATE_RADIO_RX_OFF_LINK));
 	rt73usb_register_write(rt2x00dev, TXRX_CSR0, reg);
 }
 
@@ -1172,11 +1180,9 @@ static int rt73usb_enable_radio(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize all registers.
 	 */
-	if (rt73usb_init_registers(rt2x00dev) ||
-	    rt73usb_init_bbp(rt2x00dev)) {
-		ERROR(rt2x00dev, "Register initialization failed.\n");
+	if (unlikely(rt73usb_init_registers(rt2x00dev) ||
+		     rt73usb_init_bbp(rt2x00dev)))
 		return -EIO;
-	}
 
 	return 0;
 }
@@ -1198,7 +1204,6 @@ static int rt73usb_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state)
 	u32 reg;
 	unsigned int i;
 	char put_to_sleep;
-	char current_state;
 
 	put_to_sleep = (state != STATE_AWAKE);
 
@@ -1214,16 +1219,12 @@ static int rt73usb_set_state(struct rt2x00_dev *rt2x00dev, enum dev_state state)
 	 */
 	for (i = 0; i < REGISTER_BUSY_COUNT; i++) {
 		rt73usb_register_read(rt2x00dev, MAC_CSR12, &reg);
-		current_state =
-		    rt2x00_get_field32(reg, MAC_CSR12_BBP_CURRENT_STATE);
-		if (current_state == !put_to_sleep)
+		state = rt2x00_get_field32(reg, MAC_CSR12_BBP_CURRENT_STATE);
+		if (state == !put_to_sleep)
 			return 0;
 		msleep(10);
 	}
 
-	NOTICE(rt2x00dev, "Device failed to enter state %d, "
-	       "current device state %d.\n", !put_to_sleep, current_state);
-
 	return -EBUSY;
 }
 
@@ -1241,11 +1242,13 @@ static int rt73usb_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	case STATE_RADIO_RX_ON:
 	case STATE_RADIO_RX_ON_LINK:
-		rt73usb_toggle_rx(rt2x00dev, STATE_RADIO_RX_ON);
-		break;
 	case STATE_RADIO_RX_OFF:
 	case STATE_RADIO_RX_OFF_LINK:
-		rt73usb_toggle_rx(rt2x00dev, STATE_RADIO_RX_OFF);
+		rt73usb_toggle_rx(rt2x00dev, state);
+		break;
+	case STATE_RADIO_IRQ_ON:
+	case STATE_RADIO_IRQ_OFF:
+		/* No support, but no error either */
 		break;
 	case STATE_DEEP_SLEEP:
 	case STATE_SLEEP:
@@ -1258,6 +1261,10 @@ static int rt73usb_set_device_state(struct rt2x00_dev *rt2x00dev,
 		break;
 	}
 
+	if (unlikely(retval))
+		ERROR(rt2x00dev, "Device failed to enter state %d (%d).\n",
+		      state, retval);
+
 	return retval;
 }
 
-- 
1.5.5.3