2013-03-15 08:59:08

by Helmut Schaa

[permalink] [raw]
Subject: [PATCH 1/2] rt2x00: Revert "rt2x00: remove unused argument"

This reverts commit db36f792370959ff26458f80942cf98fe8249d95
since I'm going to use the data pointer that was removed in
a follow up patch.

Signed-off-by: Helmut Schaa <[email protected]>
---
drivers/net/wireless/rt2x00/rt2x00queue.c | 10 ++++++----
drivers/net/wireless/rt2x00/rt2x00queue.h | 5 ++++-
drivers/net/wireless/rt2x00/rt2x00usb.c | 20 +++++++++++++-------
3 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.c b/drivers/net/wireless/rt2x00/rt2x00queue.c
index 4d91795..952a049 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.c
@@ -832,7 +832,9 @@ int rt2x00queue_update_beacon(struct rt2x00_dev *rt2x00dev,
bool rt2x00queue_for_each_entry(struct data_queue *queue,
enum queue_index start,
enum queue_index end,
- bool (*fn)(struct queue_entry *entry))
+ void *data,
+ bool (*fn)(struct queue_entry *entry,
+ void *data))
{
unsigned long irqflags;
unsigned int index_start;
@@ -863,17 +865,17 @@ bool rt2x00queue_for_each_entry(struct data_queue *queue,
*/
if (index_start < index_end) {
for (i = index_start; i < index_end; i++) {
- if (fn(&queue->entries[i]))
+ if (fn(&queue->entries[i], data))
return true;
}
} else {
for (i = index_start; i < queue->limit; i++) {
- if (fn(&queue->entries[i]))
+ if (fn(&queue->entries[i], data))
return true;
}

for (i = 0; i < index_end; i++) {
- if (fn(&queue->entries[i]))
+ if (fn(&queue->entries[i], data))
return true;
}
}
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
index 9b8c10a..5f1392c 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.h
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
@@ -584,6 +584,7 @@ struct data_queue_desc {
* @queue: Pointer to @data_queue
* @start: &enum queue_index Pointer to start index
* @end: &enum queue_index Pointer to end index
+ * @data: Data to pass to the callback function
* @fn: The function to call for each &struct queue_entry
*
* This will walk through all entries in the queue, in chronological
@@ -596,7 +597,9 @@ struct data_queue_desc {
bool rt2x00queue_for_each_entry(struct data_queue *queue,
enum queue_index start,
enum queue_index end,
- bool (*fn)(struct queue_entry *entry));
+ void *data,
+ bool (*fn)(struct queue_entry *entry,
+ void *data));

/**
* rt2x00queue_empty - Check if the queue is empty.
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index 40ea807..5e50d4f 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -285,7 +285,7 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb)
queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
}

-static bool rt2x00usb_kick_tx_entry(struct queue_entry *entry)
+static bool rt2x00usb_kick_tx_entry(struct queue_entry *entry, void *data)
{
struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
@@ -390,7 +390,7 @@ static void rt2x00usb_interrupt_rxdone(struct urb *urb)
queue_work(rt2x00dev->workqueue, &rt2x00dev->rxdone_work);
}

-static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry)
+static bool rt2x00usb_kick_rx_entry(struct queue_entry *entry, void *data)
{
struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
struct usb_device *usb_dev = to_usb_device_intf(rt2x00dev->dev);
@@ -427,12 +427,18 @@ void rt2x00usb_kick_queue(struct data_queue *queue)
case QID_AC_BE:
case QID_AC_BK:
if (!rt2x00queue_empty(queue))
- rt2x00queue_for_each_entry(queue, Q_INDEX_DONE, Q_INDEX,
+ rt2x00queue_for_each_entry(queue,
+ Q_INDEX_DONE,
+ Q_INDEX,
+ NULL,
rt2x00usb_kick_tx_entry);
break;
case QID_RX:
if (!rt2x00queue_full(queue))
- rt2x00queue_for_each_entry(queue, Q_INDEX, Q_INDEX_DONE,
+ rt2x00queue_for_each_entry(queue,
+ Q_INDEX,
+ Q_INDEX_DONE,
+ NULL,
rt2x00usb_kick_rx_entry);
break;
default:
@@ -441,7 +447,7 @@ void rt2x00usb_kick_queue(struct data_queue *queue)
}
EXPORT_SYMBOL_GPL(rt2x00usb_kick_queue);

-static bool rt2x00usb_flush_entry(struct queue_entry *entry)
+static bool rt2x00usb_flush_entry(struct queue_entry *entry, void *data)
{
struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
struct queue_entry_priv_usb *entry_priv = entry->priv_data;
@@ -468,7 +474,7 @@ void rt2x00usb_flush_queue(struct data_queue *queue, bool drop)
unsigned int i;

if (drop)
- rt2x00queue_for_each_entry(queue, Q_INDEX_DONE, Q_INDEX,
+ rt2x00queue_for_each_entry(queue, Q_INDEX_DONE, Q_INDEX, NULL,
rt2x00usb_flush_entry);

/*
@@ -559,7 +565,7 @@ void rt2x00usb_clear_entry(struct queue_entry *entry)
entry->flags = 0;

if (entry->queue->qid == QID_RX)
- rt2x00usb_kick_rx_entry(entry);
+ rt2x00usb_kick_rx_entry(entry, NULL);
}
EXPORT_SYMBOL_GPL(rt2x00usb_clear_entry);

--
1.7.10.4



2013-03-15 08:59:13

by Helmut Schaa

[permalink] [raw]
Subject: [PATCH 2/2] rt2x00: Fix tx status reporting for reordered frames in rt2800pci

rt2800 hardware sometimes reorders tx frames when transmitting to
multiple BA enabled STAs concurrently.

For example a tx queue
[ STA1 | STA2 | STA1 | STA2 ]
can result in the tx status reports
[ STA1 | STA1 | STA2 | STA2 ]
when the hw decides to put the frames for STA1 in one AMPDU.

To mitigate this effect associate the currently processed tx status
to the first frame in the tx queue with a matching wcid.

This patch fixes several problems related to incorrect tx status
reporting. Furthermore the tx rate selection is much more stable when
communicating with multiple STAs.

Signed-off-by: Helmut Schaa <[email protected]>
---
drivers/net/wireless/rt2x00/rt2800pci.c | 111 ++++++++++++++++++++++++++++-
drivers/net/wireless/rt2x00/rt2x00queue.h | 4 ++
2 files changed, 112 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 48a01aa..da179ae 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -742,10 +742,90 @@ static void rt2800pci_wakeup(struct rt2x00_dev *rt2x00dev)
rt2800_config(rt2x00dev, &libconf, IEEE80211_CONF_CHANGE_PS);
}

+static bool rt2800pci_txdone_entry_check(struct queue_entry *entry, u32 status)
+{
+ __le32 *txwi;
+ u32 word;
+ int wcid, tx_wcid;
+
+ wcid = rt2x00_get_field32(status, TX_STA_FIFO_WCID);
+
+ txwi = rt2800_drv_get_txwi(entry);
+ rt2x00_desc_read(txwi, 1, &word);
+ tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
+
+ return (tx_wcid == wcid);
+}
+
+static bool rt2800pci_txdone_find_entry(struct queue_entry *entry, void *data)
+{
+ u32 status = *(u32 *)data;
+
+ /*
+ * rt2800pci hardware might reorder frames when exchanging traffic
+ * with multiple BA enabled STAs.
+ *
+ * For example, a tx queue
+ * [ STA1 | STA2 | STA1 | STA2 ]
+ * can result in tx status reports
+ * [ STA1 | STA1 | STA2 | STA2 ]
+ * when the hw decides to aggregate the frames for STA1 into one AMPDU.
+ *
+ * To mitigate this effect, associate the tx status to the first frame
+ * in the tx queue with a matching wcid.
+ */
+ if (rt2800pci_txdone_entry_check(entry, status) &&
+ !test_bit(ENTRY_DATA_STATUS_SET, &entry->flags)) {
+ /*
+ * Got a matching frame, associate the tx status with
+ * the frame
+ */
+ entry->status = status;
+ set_bit(ENTRY_DATA_STATUS_SET, &entry->flags);
+ return true;
+ }
+
+ /* Check the next frame */
+ return false;
+}
+
+static bool rt2800pci_txdone_match_first(struct queue_entry *entry, void *data)
+{
+ u32 status = *(u32 *)data;
+
+ /*
+ * Find the first frame without tx status and assign this status to it
+ * regardless if it matches or not.
+ */
+ if (!test_bit(ENTRY_DATA_STATUS_SET, &entry->flags)) {
+ /*
+ * Got a matching frame, associate the tx status with
+ * the frame
+ */
+ entry->status = status;
+ set_bit(ENTRY_DATA_STATUS_SET, &entry->flags);
+ return true;
+ }
+
+ /* Check the next frame */
+ return false;
+}
+static bool rt2800pci_txdone_release_entries(struct queue_entry *entry,
+ void *data)
+{
+ if (test_bit(ENTRY_DATA_STATUS_SET, &entry->flags)) {
+ rt2800_txdone_entry(entry, entry->status,
+ rt2800pci_get_txwi(entry));
+ return false;
+ }
+
+ /* No more frames to release */
+ return true;
+}
+
static bool rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
{
struct data_queue *queue;
- struct queue_entry *entry;
u32 status;
u8 qid;
int max_tx_done = 16;
@@ -783,8 +863,33 @@ static bool rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
break;
}

- entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
- rt2800_txdone_entry(entry, status, rt2800pci_get_txwi(entry));
+ /*
+ * Let's associate this tx status with the first
+ * matching frame.
+ */
+ if (!rt2x00queue_for_each_entry(queue, Q_INDEX_DONE,
+ Q_INDEX, &status,
+ rt2800pci_txdone_find_entry)) {
+ /*
+ * We cannot match the tx status to any frame, so just
+ * use the first one.
+ */
+ if (!rt2x00queue_for_each_entry(queue, Q_INDEX_DONE,
+ Q_INDEX, &status,
+ rt2800pci_txdone_match_first)) {
+ WARNING(rt2x00dev, "No frame found for TX "
+ "status on queue %u, dropping\n",
+ qid);
+ break;
+ }
+ }
+
+ /*
+ * Release all frames with a valid tx status.
+ */
+ rt2x00queue_for_each_entry(queue, Q_INDEX_DONE,
+ Q_INDEX, NULL,
+ rt2800pci_txdone_release_entries);

if (--max_tx_done == 0)
break;
diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
index 5f1392c..3d01371 100644
--- a/drivers/net/wireless/rt2x00/rt2x00queue.h
+++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
@@ -359,6 +359,7 @@ enum queue_entry_flags {
ENTRY_DATA_PENDING,
ENTRY_DATA_IO_FAILED,
ENTRY_DATA_STATUS_PENDING,
+ ENTRY_DATA_STATUS_SET,
};

/**
@@ -372,6 +373,7 @@ enum queue_entry_flags {
* @entry_idx: The entry index number.
* @priv_data: Private data belonging to this queue entry. The pointer
* points to data specific to a particular driver and queue type.
+ * @status: Device specific status
*/
struct queue_entry {
unsigned long flags;
@@ -383,6 +385,8 @@ struct queue_entry {

unsigned int entry_idx;

+ u32 status;
+
void *priv_data;
};

--
1.7.10.4