Skip to contentSkip to conversation

rt2x00: Fix queue initialization

PATCH Superseded Ivo van Doorn 4 participants 21 messages Jun 3, 2008 10 messages not in the archive

Subsystems (2)
Network drivers Networking drivers (wireless)
lore
Share & tools
lore

Ivo van Doorn

author
#2 lore raw

PATCH 10/11 rt2x00: Fix queue initialization

In reply to a message not in the archive

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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00queue.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
drivers/net/wireless/rt2x00/rt2x00queue.c +4 -3
index e69ef4b..6f3aa0f 100644
@@ -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;
 }
Signature · 1 line
1.5.5.3
author
#3 lore raw

PATCH 11/11 rt2x00: Release rt2x00 2.1.7

Signed-off-by Ivo van Doorn <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
drivers/net/wireless/rt2x00/rt2x00.h +1 -1
index 15ec797..2f79336 100644
@@ -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"
 
 /*
Signature · 1 line
1.5.5.3
author
#4 lore raw

PATCH 1/11 rt2x00: Calculate register offset during compile time

In reply to a message not in the archive

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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   55 +++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 14 deletions(-)
drivers/net/wireless/rt2x00/rt2x00reg.h +41 -14
index 3f255df..03e846f 100644
@@ -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)	\
 	};					\
 })
Signature · 1 line
1.5.5.3
author
#13 lore raw

PATCH 2/11 rt2x00: Fix compile-time ffs calculation macros.

In reply to a message not in the archive

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 <[email protected]>
Signed-off-by Ivo van Doorn <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
drivers/net/wireless/rt2x00/rt2x00reg.h +9 -5
index 03e846f..7999d54 100644
@@ -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
Signature · 1 line
1.5.5.3
author
#17 lore raw

PATCH 3/11 rt2x00: Make rt2x00_set/get_field macros

In reply to a message not in the archive

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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00reg.h |   71 +++++++++++++------------------
 1 files changed, 30 insertions(+), 41 deletions(-)
drivers/net/wireless/rt2x00/rt2x00reg.h +30 -41
index 7999d54..170c10d 100644
@@ -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 */
Signature · 1 line
1.5.5.3
author
#19 lore raw

PATCH 4/11 rt2x00: Restrict DMA to 32-bit addresses.

In reply to a message not in the archive

From: Gertjan van Wingerde <[email protected]>

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 <[email protected]>
Signed-off-by Ivo van Doorn <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00pci.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
drivers/net/wireless/rt2x00/rt2x00pci.c +1 -2
index 70a3d13..d508c80 100644
@@ -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;
Signature · 1 line
1.5.5.3
author
#21 lore raw

PATCH 5/11 rt2x00: Don't kill guardian_urb when it wasn't created

In reply to a message not in the archive

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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/net/wireless/rt2x00/rt2x00usb.c +4 -1
index 52d12fd..36a087a 100644
@@ -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)
Signature · 1 line
1.5.5.3
author
#24 lore raw

PATCH 6/11 rt2x00: Removed unused descriptor read in txdone

In reply to a message not in the archive
Quoted 1 line

From 5410c980823b6fd68d38f0230da51cdd70e0e7eb Mon Sep 17 00:00:00 2001

From: Ivo van Doorn <[email protected]>
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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00usb.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)
drivers/net/wireless/rt2x00/rt2x00usb.c +0 -4
index 36a087a..86f1ab7 100644
@@ -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.
 	 */
Signature · 1 line
1.5.5.3
author
#26 lore raw

PATCH 7/11 rt2x00: Remove CTS/RTS check in tx()

In reply to a message not in the archive
Quoted 1 line

From 38a62373d637d6e4915136033c628143061049f4 Mon Sep 17 00:00:00 2001

From: Ivo van Doorn <[email protected]>
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 <[email protected]>
---
 drivers/net/wireless/rt2x00/rt2x00mac.c |   12 +++++-------
 1 files changed, 5 insertions(+), 7 deletions(-)
drivers/net/wireless/rt2x00/rt2x00mac.c +5 -7
index b02dbc8..97d9095 100644
@@ -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) {
Signature · 1 line
1.5.5.3
author
#28 lore raw

PATCH 8/11 rt2x00: Move led initialization into function

In reply to a message not in the archive

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

Signed-off-by Ivo van Doorn <[email protected]>
---
 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(-)
drivers/net/wireless/rt2x00/rt2400pci.c +15 -17
index 900140d..1101610 100644
@@ -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 */
 
 	/*
drivers/net/wireless/rt2x00/rt2500pci.c +15 -17
index 6733509..0ac934a 100644
@@ -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 */
 
 	/*
drivers/net/wireless/rt2x00/rt2500usb.c +15 -17
index cca1504..0027f4b 100644
@@ -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 */
 
 	/*
drivers/net/wireless/rt2x00/rt61pci.c +16 -25

53 lines not on this page · raw message

drivers/net/wireless/rt2x00/rt73usb.c +16 -25

53 lines not on this page · raw message

Signature · 1 line
1.5.5.3
author
#30 lore raw

PATCH 9/11 rt2x00: Cleanup/optimize set_state() function callback function

In reply to a message not in the archive
Quoted 1 line

From 36d1e3ea615aec2dbafa2493021cf3117cc8f57c Mon Sep 17 00:00:00 2001

From: Ivo van Doorn <[email protected]>
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 <[email protected]>
---
 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(-)
drivers/net/wireless/rt2x00/rt2400pci.c +27 -29
index 1101610..94226b4 100644
@@ -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;
 }
drivers/net/wireless/rt2x00/rt2500pci.c +27 -29

114 lines not on this page · raw message

drivers/net/wireless/rt2x00/rt2500usb.c +26 -18

96 lines not on this page · raw message

drivers/net/wireless/rt2x00/rt2x00dev.c +3 -0
index dc5ab90..48f4aec 100644
@@ -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);
 }
drivers/net/wireless/rt2x00/rt61pci.c +29 -32

129 lines not on this page · raw message

drivers/net/wireless/rt2x00/rt73usb.c +28 -21

111 lines not on this page · raw message

Signature · 1 line
1.5.5.3
maintainer
#14 lore raw

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

In reply to Ivo van Doorn
Quoted 7 lines · 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

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

johannes

author
#15 lore raw

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

Quoted 10 lines · Johannes Berg
Quoted 7 lines · 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

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

Good point. I'll resend it. :)

Ivo

author
#5 lore raw

PATCH v2 1/11 rt2x00: Calculate register offset during compile time

In reply to 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 <[email protected]>
Signed-off-by Ivo van Doorn <[email protected]>

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

drivers/net/wireless/rt2x00/rt2x00reg.h +45 -14
index 3f255df..7999d54 100644
@@ -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)	\
 	};					\
 })
Acked-by on v1 5/11
#22 lore raw

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

In reply to Ivo van Doorn

On Tue, Jun 3, 2008 at 8:29 PM, Ivo van Doorn <[email protected]> wrote= :

Quoted 5 lines

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 <[email protected]>
Acked-by G=E1bor Stefanik <[email protected]>

drivers/net/wi=

Quoted 5 lines
---
 drivers/net/wireless/rt2x00/rt2x00usb.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/net/wi= renamed from drivers/net/wireless/rt2x00/rt2x00usb.c +0 -0

reless/rt2x00/rt2x00usb.c

drivers/net/wireless/rt2x00/rt2x00usb.c void rt2x00usb_disable_radio(struct rt2x00_dev *=

Quoted 4 lines

index 52d12fd..36a087a 100644

drivers/net/wireless/rt2x00/rt2x00usb.c +0 -0
@@ -394,8 +394,11 @@ void rt2x00usb_disable_radio(struct rt2x00_dev *=

rt2x00dev)

drivers/net/wireless/rt2x00/rt2x00usb.c void rt2x00usb_disable_radio(struct rt2x00_dev *rt2x00dev)

Quoted 7 lines
drivers/net/wireless/rt2x00/rt2x00usb.c +2 -1
       }
 
       /*
-        * Kill guardian urb.
+        * Kill guardian urb (if required by driver).
        */
+       if (!test_bit(DRIVER_REQUIRE_BEACON_GUARD, &rt2x00dev->flags)=

)

Quoted 10 lines
drivers/net/wireless/rt2x00/rt2x00usb.c +2 -1
+               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

Quoted 3 lines

the body of a message to [email protected]
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#6 lore raw

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

In reply to Ivo van Doorn

drivers/net/wireless/rt2x00/rt2x00reg.h struct rt2x00_field32 { · line 152

Quoted 7 lines
drivers/net/wireless/rt2x00/rt2x00reg.h +6 -0
+#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

author
#7 lore raw

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

Quoted 11 lines · Johannes Berg

drivers/net/wireless/rt2x00/rt2x00reg.h struct rt2x00_field32 { · line 152

Quoted 7 lines
drivers/net/wireless/rt2x00/rt2x00reg.h +6 -0
+#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:

Quoted 3 lines

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

or am I missing something?

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

Thanks,

Ivo

author
#8 lore raw

PATCH v3 1/11 rt2x00: Calculate register offset during compile time

In reply to 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 <[email protected]>
Signed-off-by Ivo van Doorn <[email protected]>

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

drivers/net/wire= renamed from drivers/net/wireless/rt2x00/rt2x00reg.h +0 -0

less/rt2x00/rt2x00reg.h
index 3f255df..7999d54 100644

drivers/net/wireless/rt2x00/rt2x00reg.h +0 -0
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#9 lore raw

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

Quoted 15 lines · 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
instead of providing it as seperate patch.
v3: Add extra parentheses when bitshifting __x

Quoted 5 lines
Signed-off-by Gertjan van Wingerde <[email protected]>
Signed-off-by Ivo van Doorn <[email protected]>

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

#10 lore raw

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

In reply to Ivo van Doorn
Quoted 8 lines · 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. =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

Quoted 8 lines

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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

author
#11 lore raw

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

Quoted 40 lines · Harvey Harrison
Quoted 8 lines · 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. =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

Quoted 8 lines

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); )
Quoted 3 lines

=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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html