Please pull 'upstream' branch of rt2x00

PATCH New Ivo van Doorn 1 participant 4 messages May 5, 2008 Networking drivers (wireless)

Share & tools

Ivo van Doorn

author

2008-05-05 15:17 UTC #0 lore raw

Hi John,

Here are 3 patches for rt2x00, they are all intended for 2.6.26 since they fix serious bugs. Patch 1 should prevent problems as reported by Johannes Berg regarding skb->truesize by removing the only pskb_expand_head() call in rt2x00 (and use a already existing buffer instead).

Patches 2 and 3 fix problems when DMA allocation fails, that usually doesn't happen, hence the reason these problems have gone unnoticed for so long. :S

Ivo

---
The following changes since commit 0bacadffc86bcba2fdf2b989afc286a8184664a0:
Ivo van Doorn (1):
rt2x00: Fix quality/activity led handling

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/ivd/rt2x00.git/ upstream

Gertjan van Wingerde (1):
rt2x00: Clean up error handling of PCI queue DMA allocation.

Ivo van Doorn (2):
rt2x00: Don't use pskb_expand_head()
rt2x00: Fix broken recover-on-error path

 drivers/net/wireless/rt2x00/rt2x00dev.c |   11 ++++-------
 drivers/net/wireless/rt2x00/rt2x00pci.c |    5 +++--
 drivers/net/wireless/rt2x00/rt61pci.c   |   31 +++++++++++--------------------
 3 files changed, 18 insertions(+), 29 deletions(-)

Ivo van Doorn

author

2008-05-05 15:17 UTC #1 lore raw

PATCH 1/3 rt2x00: Don't use pskb_expand_head()

rt2x00pci allocates DMA for descriptor and data, rt61pci doesn't use this for the beacon, but it can use the descriptor part as temporary buffer instead of using pskb_expand_head().
Using this temporary buffer is obviously much better then reallocating the skb buffer...

At the same time we can set the data length for the beacon queue at 0, to make sure no DMA is allocated for data (but just for the descriptor).

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

---
 drivers/net/wireless/rt2x00/rt61pci.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

drivers/net/wireless/rt2x00/rt61pci.c +11 ~~-20~~

index 98af4d2..b64f2f5 100644
@@ -2362,6 +2362,7 @@ static int rt61pci_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb,
 {
 	struct rt2x00_dev *rt2x00dev = hw->priv;
 	struct rt2x00_intf *intf = vif_to_intf(control->vif);
+	struct queue_entry_priv_pci_tx *priv_tx;
 	struct skb_frame_desc *skbdesc;
 	unsigned int beacon_base;
 	u32 reg;

@@ -2369,21 +2370,8 @@ static int rt61pci_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb,
 	if (unlikely(!intf->beacon))
 		return -ENOBUFS;
 
-	/*
-	 * We need to append the descriptor in front of the
-	 * beacon frame.
-	 */
-	if (skb_headroom(skb) < intf->beacon->queue->desc_size) {
-		if (pskb_expand_head(skb, intf->beacon->queue->desc_size,
-				     0, GFP_ATOMIC))
-			return -ENOMEM;
-	}
-
-	/*
-	 * Add the descriptor in front of the skb.
-	 */
-	skb_push(skb, intf->beacon->queue->desc_size);
-	memset(skb->data, 0, intf->beacon->queue->desc_size);
+	priv_tx = intf->beacon->priv_data;
+	memset(priv_tx->desc, 0, intf->beacon->queue->desc_size);
 
 	/*
 	 * Fill in skb descriptor

@@ -2391,9 +2379,9 @@ static int rt61pci_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb,
 	skbdesc = get_skb_frame_desc(skb);
 	memset(skbdesc, 0, sizeof(*skbdesc));
 	skbdesc->flags |= FRAME_DESC_DRIVER_GENERATED;
-	skbdesc->data = skb->data + intf->beacon->queue->desc_size;
-	skbdesc->data_len = skb->len - intf->beacon->queue->desc_size;
-	skbdesc->desc = skb->data;
+	skbdesc->data = skb->data;
+	skbdesc->data_len = skb->len;
+	skbdesc->desc = priv_tx->desc;
 	skbdesc->desc_len = intf->beacon->queue->desc_size;
 	skbdesc->entry = intf->beacon;

@@ -2414,7 +2402,10 @@ static int rt61pci_beacon_update(struct ieee80211_hw *hw, struct sk_buff *skb,
 	rt2x00lib_write_tx_desc(rt2x00dev, skb, control);
 	beacon_base = HW_BEACON_OFFSET(intf->beacon->entry_idx);
 	rt2x00pci_register_multiwrite(rt2x00dev, beacon_base,
-				      skb->data, skb->len);
+				      skbdesc->desc, skbdesc->desc_len);
+	rt2x00pci_register_multiwrite(rt2x00dev,
+				      beacon_base + skbdesc->desc_len,
+				      skbdesc->data, skbdesc->data_len);
 	rt61pci_kick_tx_queue(rt2x00dev, QID_BEACON);
 
 	return 0;

@@ -2479,7 +2470,7 @@ static const struct data_queue_desc rt61pci_queue_tx = {
 
 static const struct data_queue_desc rt61pci_queue_bcn = {
 	.entry_num		= 4 * BEACON_ENTRIES,
-	.data_size		= MGMT_FRAME_SIZE,
+	.data_size		= 0, /* No DMA required for beacons */
 	.desc_size		= TXINFO_SIZE,
 	.priv_size		= sizeof(struct queue_entry_priv_pci_tx),
 };

Signature · 1 line

1.5.5.1

Ivo van Doorn

author

2008-05-05 15:17 UTC #2 lore raw

PATCH 2/3 rt2x00: Fix broken recover-on-error path

In reply to Ivo van Doorn

During initialization the initialize() callback function in rt2x00pci and rt2x00usb will cleanup the mess they made.

rt2x00lib shouldn't call uninitialize because the callback function already cleaned up _and_ the DEVICE_INITIALIZED isn't set which causes the rt2x00lib_uninitialize() to halt directly anyway. All that is required to be cleaned up by rt2x00lib is the queue, and that can be done by calling rt2x00queue_uninitialize() directly.

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

---
 drivers/net/wireless/rt2x00/rt2x00dev.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

drivers/net/wireless/rt2x00/rt2x00dev.c +4 -7

index 9929b15..61510c5 100644
@@ -1028,8 +1028,10 @@ static int rt2x00lib_initialize(struct rt2x00_dev *rt2x00dev)
 	 * Initialize the device.
 	 */
 	status = rt2x00dev->ops->lib->initialize(rt2x00dev);
-	if (status)
-		goto exit;
+	if (status) {
+		rt2x00queue_uninitialize(rt2x00dev);
+		return status;
+	}
 
 	__set_bit(DEVICE_INITIALIZED, &rt2x00dev->flags);

@@ -1039,11 +1041,6 @@ static int rt2x00lib_initialize(struct rt2x00_dev *rt2x00dev)
 	rt2x00rfkill_register(rt2x00dev);
 
 	return 0;
-
-exit:
-	rt2x00lib_uninitialize(rt2x00dev);
-
-	return status;
 }
 
 int rt2x00lib_start(struct rt2x00_dev *rt2x00dev)

Signature · 1 line

1.5.5.1

Ivo van Doorn

author

2008-05-05 15:17 UTC #3 lore raw

PATCH 3/3 rt2x00: Clean up error handling of PCI queue DMA allocation.

In reply to Ivo van Doorn

When, for some reason, the rt2x00pci module fails to allocate DMA memory for the queues, it tries to undo the complete initialization of the PCI device, including freeing of the irq. This results in the following error in dmesg, as the irq hadn't been requested yet:

[  78.123456] Trying to free already-free IRQ 17

Fix this by implementing proper error handling code, instead of just using the full uninitialization function.

Signed-off-by Gertjan van Wingerde <[email protected]>

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

---
 drivers/net/wireless/rt2x00/rt2x00pci.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

drivers/net/wireless/rt2x00/rt2x00pci.c +3 -2

index 9e5d94e..c17078e 100644
@@ -314,13 +314,14 @@ int rt2x00pci_initialize(struct rt2x00_dev *rt2x00dev)
 	if (status) {
 		ERROR(rt2x00dev, "IRQ %d allocation failed (error %d).\n",
 		      pci_dev->irq, status);
-		return status;
+		goto exit;
 	}
 
 	return 0;
 
 exit:
-	rt2x00pci_uninitialize(rt2x00dev);
+	queue_for_each(rt2x00dev, queue)
+		rt2x00pci_free_queue_dma(rt2x00dev, queue);
 
 	return status;
 }

Signature · 1 line

1.5.5.1