rt2x00: Decrease alignment headroom

PATCH Superseded Ivo van Doorn 2 participants 9 messages Jul 4, 2008 2 messages not in the archive Networking drivers (wireless)

lore

Share & tools

lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ivo van Doorn

author

2008-07-04 11:36 UTC #1 lore raw

PATCH 2/4 rt2x00: Decrease alignment headroom

We only need 4 bytes of headroom for alignment purposes in the RX frame. It was previously higher for optimization purposes which are no longer possible due to DMA mappings.

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net renamed from drivers/net/wireless/rt2x00/rt2x00queue.c +0 -0

index 8e86611..3ddce53 100644

/wireless/rt2x00/rt2x00queue.c
@@ -45,10 +45,11 @@ struct sk_buff *rt2x00queue_alloc_rxskb(struct rt2x00_dev *rt2x00dev,
frame_size = entry->queue->data_size + entry->queue->desc_size;

/*
- * Reserve a few bytes extra headroom to allow drivers some moving
- * space (e.g. for alignment), while keeping the skb aligned.
+ * The payload should be aligned to a 4-byte boundary,
+ * this means we need at least 3 bytes for moving the frame
+ * into the correct offset.
*/
- reserved_size = 8;
+ reserved_size = 4;

/*
* Allocate skbuffer.

Ivo van Doorn

author

2008-07-04 12:50 UTC #2 lore raw

PATCH v2 2/4 rt2x00: Decrease alignment headroom

We only need 4 bytes of headroom for alignment purposes in the RX frame. It was previously higher for optimization purposes which are no longer possible due to DMA mappings.

v2: Fix patch error

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/rt2x00/rt2x00queue.c +4 -3

index 8e86611..3ddce53 100644
@@ -45,10 +45,11 @@ struct sk_buff *rt2x00queue_alloc_rxskb(struct rt2x00_dev *rt2x00dev,
 	frame_size = entry->queue->data_size + entry->queue->desc_size;
 
 	/*
-	 * Reserve a few bytes extra headroom to allow drivers some moving
-	 * space (e.g. for alignment), while keeping the skb aligned.
+	 * The payload should be aligned to a 4-byte boundary,
+	 * this means we need at least 3 bytes for moving the frame
+	 * into the correct offset.
 	 */
-	reserved_size = 8;
+	reserved_size = 4;
 
 	/*
 	 * Allocate skbuffer.

A message that is not in the archive (sent off-list or not yet received): [email protected]

Ivo van Doorn

author

2008-07-04 11:38 UTC #4 lore raw

PATCH 3/4 rt2x00: Use ieee80211_hw->workqueue again

Remove the rt2x00 singlethreaded workqueue and move the link tuner and packet filter scheduled work to the ieee80211_hw->workqueue again.
The only exception is the interface scheduled work handler which uses the mac80211 interface iterator under the RTNL lock. This work needs to be handled on the kernel workqueue to prevent lockdep issues.

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/rt2x00/rt2x00.h +3 -1

index 1b28dad..c07d9ef 100644
@@ -787,8 +787,10 @@ struct rt2x00_dev {
 
 	/*
 	 * Scheduled work.
+	 * NOTE: intf_work will use ieee80211_iterate_active_interfaces()
+	 * which means it cannot be placed on the hw->workqueue
+	 * due to RTNL locking requirements.
 	 */
-	struct workqueue_struct *workqueue;
 	struct work_struct intf_work;
 	struct work_struct filter_work;

drivers/net/wireless/rt2x00/rt2x00dev.c +3 ~~-14~~

index 41b3289..8086263 100644
@@ -74,7 +74,7 @@ static void rt2x00lib_start_link_tuner(struct rt2x00_dev *rt2x00dev)
 
 	rt2x00lib_reset_link_tuner(rt2x00dev);
 
-	queue_delayed_work(rt2x00dev->workqueue,
+	queue_delayed_work(rt2x00dev->hw->workqueue,
 			   &rt2x00dev->link.work, LINK_TUNE_INTERVAL);
 }

@@ -392,7 +392,7 @@ static void rt2x00lib_link_tuner(struct work_struct *work)
 	 * Increase tuner counter, and reschedule the next link tuner run.
 	 */
 	rt2x00dev->link.count++;
-	queue_delayed_work(rt2x00dev->workqueue,
+	queue_delayed_work(rt2x00dev->hw->workqueue,
 			   &rt2x00dev->link.work, LINK_TUNE_INTERVAL);
 }

@@ -496,7 +496,7 @@ void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev)
 						   rt2x00lib_beacondone_iter,
 						   rt2x00dev);
 
-	queue_work(rt2x00dev->workqueue, &rt2x00dev->intf_work);
+	schedule_work(&rt2x00dev->intf_work);
 }
 EXPORT_SYMBOL_GPL(rt2x00lib_beacondone);

@@ -1064,10 +1064,6 @@ int rt2x00lib_probe_dev(struct rt2x00_dev *rt2x00dev)
 	/*
 	 * Initialize configuration work.
 	 */
-	rt2x00dev->workqueue = create_singlethread_workqueue("rt2x00lib");
-	if (!rt2x00dev->workqueue)
-		goto exit;
-
 	INIT_WORK(&rt2x00dev->intf_work, rt2x00lib_intf_scheduled);
 	INIT_WORK(&rt2x00dev->filter_work, rt2x00lib_packetfilter_scheduled);
 	INIT_DELAYED_WORK(&rt2x00dev->link.work, rt2x00lib_link_tuner);

@@ -1128,13 +1124,6 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
 	rt2x00leds_unregister(rt2x00dev);
 
 	/*
-	 * Stop all queued work. Note that most tasks will already be halted
-	 * during rt2x00lib_disable_radio() and rt2x00lib_uninitialize().
-	 */
-	flush_workqueue(rt2x00dev->workqueue);
-	destroy_workqueue(rt2x00dev->workqueue);
-
-	/*
 	 * Free ieee80211_hw memory.
 	 */
 	rt2x00lib_remove_hw(rt2x00dev);

drivers/net/wireless/rt2x00/rt2x00mac.c +2 -2

index 1253da8..3a1fb6d 100644
@@ -431,7 +431,7 @@ void rt2x00mac_configure_filter(struct ieee80211_hw *hw,
 	if (!test_bit(DRIVER_REQUIRE_SCHEDULED, &rt2x00dev->flags))
 		rt2x00dev->ops->lib->config_filter(rt2x00dev, *total_flags);
 	else
-		queue_work(rt2x00dev->workqueue, &rt2x00dev->filter_work);
+		queue_work(rt2x00dev->hw->workqueue, &rt2x00dev->filter_work);
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_configure_filter);

@@ -512,7 +512,7 @@ void rt2x00mac_bss_info_changed(struct ieee80211_hw *hw,
 	memcpy(&intf->conf, bss_conf, sizeof(*bss_conf));
 	if (delayed) {
 		intf->delayed_flags |= delayed;
-		queue_work(rt2x00dev->workqueue, &rt2x00dev->intf_work);
+		schedule_work(&rt2x00dev->intf_work);
 	}
 	spin_unlock(&intf->lock);
 }

Ivo van Doorn

author

2008-07-04 11:38 UTC #5 lore raw

PATCH 4/4 rt2x00: Report RX end time for rt2400pci

rt2400 is the only currently available rt2x00 driver which supports reporting of the RX end time for frames. Since mac80211 uses this information for IBSS syncing, it is important that it is being reported.

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/rt2x00/rt2400pci.c +3 -0

index b3dffcf..54e2f1e 100644
@@ -1091,10 +1091,12 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 	u32 word0;
 	u32 word2;
 	u32 word3;
+	u32 word4;
 
 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);
 
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |= RX_FLAG_FAILED_FCS_CRC;

@@ -1106,6 +1108,7 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp = rt2x00_get_field32(word4, RXD_W4_RX_END_TIME);
 	rxdesc->signal = rt2x00_get_field32(word2, RXD_W2_SIGNAL) & ~0x08;
 	rxdesc->rssi = rt2x00_get_field32(word2, RXD_W3_RSSI) -
 	    entry->queue->rt2x00dev->rssi_offset;

drivers/net/wireless/rt2x00/rt2x00dev.c +1 -0

index 8086263..b48c04e 100644
@@ -664,6 +664,7 @@ void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00dev->link.qual.rx_success++;
 
+	rx_status->mactime = rxdesc.timestamp;
 	rx_status->rate_idx = idx;
 	rx_status->qual =
 	    rt2x00lib_calculate_link_signal(rt2x00dev, rxdesc.rssi);

drivers/net/wireless/rt2x00/rt2x00queue.h +2 -0

index 5dd9cca..8945945 100644
@@ -146,6 +146,7 @@ enum rxdone_entry_desc_flags {
  *
  * Summary of information that has been read from the RX frame descriptor.
  *
+ * @timestamp: RX Timestamp
  * @signal: Signal of the received frame.
  * @rssi: RSSI of the received frame.
  * @size: Data size of the received frame.

@@ -154,6 +155,7 @@ enum rxdone_entry_desc_flags {
 
  */
 struct rxdone_entry_desc {
+	u64 timestamp;
 	int signal;
 	int rssi;
 	int size;

Johannes Berg

maintainer

2008-07-04 11:43 UTC #6 lore raw

Re: [PATCH 4/4] rt2x00: Report RX end time for rt2400pci

drivers/net/wireless/rt2x00/rt2400pci.c static void rt2400pci_fill_rxdone(struct queue_entry *entry, · line 1111

Quoted 13 lines

drivers/net/wireless/rt2x00/rt2400pci.c +2 -0

 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);
 
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |= RX_FLAG_FAILED_FCS_CRC;

@@ -1106,6 +1108,7 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp = rt2x00_get_field32(word4, RXD_W4_RX_END_TIME);

You really should extend that, like this it's going to cause trouble as soon as it has overflowed 32 bits once.

johannes

Ivo van Doorn

author

2008-07-04 12:18 UTC #7 lore raw

Re: [PATCH 4/4] rt2x00: Report RX end time for rt2400pci

Quoted 6 lines · Johannes Berg

Quoted 6 lines

drivers/net/wireless/rt2x00/rt2400pci.c +1 -0

 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);

Quoted 10 lines

drivers/net/wireless/rt2x00/rt2400pci.c static void rt2400pci_fill_rxdone(struct queue_entry *entry, · line 1111
Quoted 7 lines
drivers/net/wireless/rt2x00/rt2400pci.c +1 -0
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |= RX_FLAG_FAILED_FCS_CRC;
@@ -1106,6 +1108,7 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp = rt2x00_get_field32(word4, RXD_W4_RX_END_TIME);
You really should extend that, like this it's going to cause trouble as soon as it has overflowed 32 bits once.

Ok, but if I do the get_tsf() callback, to grab the upper 32 bytes of the timestamp, you will get the same problems during wraparounds, because the upper 32 bytes can be from _after_ the wrapround while the lower 32 bytes from the descriptor are still from _before_ the wraparound.

Ivo

Johannes Berg

maintainer

2008-07-04 12:25 UTC #8 lore raw

Re: [PATCH 4/4] rt2x00: Report RX end time for rt2400pci

Quoted 18 lines · Ivo van Doorn

Quoted 6 lines · Johannes Berg
Quoted 6 lines
drivers/net/wireless/rt2x00/rt2400pci.c +1 -0
 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);
Quoted 10 lines
drivers/net/wireless/rt2x00/rt2400pci.c static void rt2400pci_fill_rxdone(struct queue_entry *entry, · line 1111
Quoted 7 lines
drivers/net/wireless/rt2x00/rt2400pci.c +1 -0
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |= RX_FLAG_FAILED_FCS_CRC;
@@ -1106,6 +1108,7 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp = rt2x00_get_field32(word4, RXD_W4_RX_END_TIME);
You really should extend that, like this it's going to cause trouble as soon as it has overflowed 32 bits once.

Quoted 4 lines

Ok, but if I do the get_tsf() callback, to grab the upper 32 bytes of the timestamp, you will get the same problems during wraparounds, because the upper 32 bytes can be from _after_ the wrapround while the lower 32 bytes from the descriptor are still from _before_ the wraparound.

You mean bits. But no, it's not a problem, you can safely assume that you call get_tsf() within 2^32 usecs of getting the frame (that's quite a long time after all).

Then do something like this:

ts = word4
high32 | low32 = get_tsf()

if (ts >= low32)
// wrapped around while we were not looking
high32 -= 1

tsf = high32 | ts

johannes

Ivo van Doorn

author

2008-07-04 12:47 UTC #9 lore raw

Re: [PATCH 4/4] rt2x00: Report RX end time for rt2400pci

Quoted 11 lines · Johannes Berg

Quoted 10 lines · Ivo van Doorn
Quoted 9 lines · Johannes Berg
=20
Review static void rt2400pci_fill_rxdone(struct =
Quoted 8 lines
drivers/net/wireless/rt2x00/rt2400pci.c +1 -0
 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);
=20
(quoted diff) +0 -0
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |=3D RX_FLAG_FAILED_FCS_CRC;
@@ -1106,6 +1108,7 @@ static void rt2400pci_fill_rxdone(struct =

queue_entry *entry,

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
drivers/net/wireless/rt2x00/rt2400pci.c static void rt2400pci_fill_rxdone(struct queue_entry *entry,
Quoted 4 lines
drivers/net/wireless/rt2x00/rt2400pci.c +1 -0
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp =3D rt2x00_get_field32(word4, RXD_W4_RX_END=

_TIME);

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
=20
You really should extend that, like this it's going to cause trou=

ble as

Quoted 3 lines

Quoted 3 lines
Quoted 1 line
soon as it has overflowed 32 bits once.
=20
Ok, but if I do the get_tsf() callback, to grab the upper 32 bytes =

of the timestamp,

Quoted 1 line

Quoted 1 line
you will get the same problems during wraparounds, because the uppe=

r 32 bytes

Quoted 1 line

Quoted 1 line
can be from _after_ the wrapround while the lower 32 bytes from the=

descriptor

Quoted 3 lines

Quoted 1 line
are still from _before_ the wraparound.
=20
You mean bits.

Ehm yes, having a 64byte timestamp would be a bit overkill. ;)

Quoted 2 lines

But no, it's not a problem, you can safely assume that=20 you call get_tsf() within 2^32 usecs of getting the frame (that's qui=

Quoted 4 lines

a long time after all).
=20
Then do something like this:
=20

Quoted 8 lines

ts =3D word4
high32 | =EF=BB=BFlow32 =3D get_tsf()
=20
if (ts >=3D low32)
// wrapped around while we were not looking
high32 -=3D 1
=20
tsf =3D high32 | ts

Thanks, I'll update the patch.

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

Ivo van Doorn

author

2008-07-04 14:09 UTC #10 lore raw

PATCH v2 4/4 rt2x00: Report RX end time for rt2400pci

In reply to Ivo van Doorn

rt2x00: Report RX end time for rt2400pci

v2: Complement 32 bits of RX timestamp with upper 32bits from TSF

Signed-off-by Ivo van Doorn <[email protected]>

---

drivers/net/wireless/rt2x00/rt2400pci.c +23 -0

index b3dffcf..ee953ca 100644
@@ -1087,14 +1087,20 @@ static void rt2400pci_kick_tx_queue(struct rt2x00_dev *rt2x00dev,
 static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 				  struct rxdone_entry_desc *rxdesc)
 {
+	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
 	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
 	u32 word0;
 	u32 word2;
 	u32 word3;
+	u32 word4;
+	u64 tsf;
+	u32 rx_low;
+	u32 rx_high;
 
 	rt2x00_desc_read(entry_priv->desc, 0, &word0);
 	rt2x00_desc_read(entry_priv->desc, 2, &word2);
 	rt2x00_desc_read(entry_priv->desc, 3, &word3);
+	rt2x00_desc_read(entry_priv->desc, 4, &word4);
 
 	if (rt2x00_get_field32(word0, RXD_W0_CRC_ERROR))
 		rxdesc->flags |= RX_FLAG_FAILED_FCS_CRC;

@@ -1102,10 +1108,27 @@ static void rt2400pci_fill_rxdone(struct queue_entry *entry,
 		rxdesc->flags |= RX_FLAG_FAILED_PLCP_CRC;
 
 	/*
+	 * We only get the lower 32bits from the timestamp,
+	 * to get the full 64bits we must complement it with
+	 * the timestamp from get_tsf().
+	 * Note that when a wraparound of the lower 32bits
+	 * has occurred between the frame arrival and the get_tsf()
+	 * call, we must decrease the higher 32bits with 1 to get
+	 * to correct value.
+	 */
+	tsf = rt2x00dev->ops->hw->get_tsf(rt2x00dev->hw);
+	rx_low = rt2x00_get_field32(word4, RXD_W4_RX_END_TIME);
+	rx_high = upper_32_bits(tsf);
+
+	if ((u32)tsf <= rx_low)
+		rx_high--;
+
+	/*
 	 * Obtain the status about this packet.
 	 * The signal is the PLCP value, and needs to be stripped
 	 * of the preamble bit (0x08).
 	 */
+	rxdesc->timestamp = ((u64)rx_high << 32) | rx_low;
 	rxdesc->signal = rt2x00_get_field32(word2, RXD_W2_SIGNAL) & ~0x08;
 	rxdesc->rssi = rt2x00_get_field32(word2, RXD_W3_RSSI) -
 	    entry->queue->rt2x00dev->rssi_offset;

drivers/net/wireless/rt2x00/rt2x00dev.c +1 -0

index 8086263..b48c04e 100644
@@ -664,6 +664,7 @@ void rt2x00lib_rxdone(struct rt2x00_dev *rt2x00dev,
 
 	rt2x00dev->link.qual.rx_success++;
 
+	rx_status->mactime = rxdesc.timestamp;
 	rx_status->rate_idx = idx;
 	rx_status->qual =
 	    rt2x00lib_calculate_link_signal(rt2x00dev, rxdesc.rssi);

drivers/net/wireless/rt2x00/rt2x00queue.h +2 -0

index 5dd9cca..8945945 100644
@@ -146,6 +146,7 @@ enum rxdone_entry_desc_flags {
  *
  * Summary of information that has been read from the RX frame descriptor.
  *
+ * @timestamp: RX Timestamp
  * @signal: Signal of the received frame.
  * @rssi: RSSI of the received frame.
  * @size: Data size of the received frame.

@@ -154,6 +155,7 @@ enum rxdone_entry_desc_flags {
 
  */
 struct rxdone_entry_desc {
+	u64 timestamp;
 	int signal;
 	int rssi;
 	int size;