[PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Stanislaw Gruszka]

This patchset make rt2800mmio txdone routines the same as rt2800usb.
It should address problems with TX status interrupt handling and
doing txdone for cases when we miss TX statuses from HW. We assume
that for PCIe/SOC we always read TX status in IRQ routine, but this
can be not true for example when CPU is busy with other interrupts.  

It was tested by  with positive feedback, some users report that
patches make MT7620 routers workable for them. This is documented
here: https://bugzilla.kernel.org/show_bug.cgi?id=82751

Stanislaw Gruszka (5):
  rt2800: move usb specific txdone/txstatus routines to rt2800lib
  rt2800mmio: use txdone/txstatus routines from lib
  rt2x00: do not check for txstatus timeout every time on tasklet
  rt2x00: use different txstatus timeouts when flushing
  rt2800: flush and txstatus rework for rt2800mmio

 drivers/net/wireless/ralink/rt2x00/rt2800lib.c   | 154 +++++++++++++
 drivers/net/wireless/ralink/rt2x00/rt2800lib.h   |   3 +
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.c  | 277 +++++++----------------
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.h  |   1 +
 drivers/net/wireless/ralink/rt2x00/rt2800pci.c   |   2 +-
 drivers/net/wireless/ralink/rt2x00/rt2800usb.c   | 143 +-----------
 drivers/net/wireless/ralink/rt2x00/rt2x00.h      |   3 +
 drivers/net/wireless/ralink/rt2x00/rt2x00mac.c   |   4 +
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c |   2 +
 9 files changed, 259 insertions(+), 330 deletions(-)

-- 
2.7.5

[PATCH 1/5] rt2800: move usb specific txdone/txstatus routines to rt2800lib

[Stanislaw Gruszka]

In order to reuse usb txdone/txstatus routines for mmio, move them
to common rt2800lib.c file.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 138 ++++++++++++++++++++++++
 drivers/net/wireless/ralink/rt2x00/rt2800lib.h |   3 +
 drivers/net/wireless/ralink/rt2x00/rt2800usb.c | 143 +------------------------
 3 files changed, 145 insertions(+), 139 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index a567bc273ffc..9f2835729016 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -957,6 +957,47 @@ static void rt2800_rate_from_status(struct skb_frame_desc *skbdesc,
 	skbdesc->tx_rate_flags = flags;
 }
 
+static bool rt2800_txdone_entry_check(struct queue_entry *entry, u32 reg)
+{
+	__le32 *txwi;
+	u32 word;
+	int wcid, ack, pid;
+	int tx_wcid, tx_ack, tx_pid, is_agg;
+
+	/*
+	 * This frames has returned with an IO error,
+	 * so the status report is not intended for this
+	 * frame.
+	 */
+	if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
+		return false;
+
+	wcid	= rt2x00_get_field32(reg, TX_STA_FIFO_WCID);
+	ack	= rt2x00_get_field32(reg, TX_STA_FIFO_TX_ACK_REQUIRED);
+	pid	= rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE);
+	is_agg	= rt2x00_get_field32(reg, TX_STA_FIFO_TX_AGGRE);
+
+	/*
+	 * Validate if this TX status report is intended for
+	 * this entry by comparing the WCID/ACK/PID fields.
+	 */
+	txwi = rt2800_drv_get_txwi(entry);
+
+	word = rt2x00_desc_read(txwi, 1);
+	tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
+	tx_ack  = rt2x00_get_field32(word, TXWI_W1_ACK);
+	tx_pid  = rt2x00_get_field32(word, TXWI_W1_PACKETID);
+
+	if (wcid != tx_wcid || ack != tx_ack || (!is_agg && pid != tx_pid)) {
+		rt2x00_dbg(entry->queue->rt2x00dev,
+			   "TX status report missed for queue %d entry %d\n",
+			   entry->queue->qid, entry->entry_idx);
+		return false;
+	}
+
+	return true;
+}
+
 void rt2800_txdone_entry(struct queue_entry *entry, u32 status, __le32 *txwi,
 			 bool match)
 {
@@ -1059,6 +1100,103 @@ void rt2800_txdone_entry(struct queue_entry *entry, u32 status, __le32 *txwi,
 }
 EXPORT_SYMBOL_GPL(rt2800_txdone_entry);
 
+void rt2800_txdone(struct rt2x00_dev *rt2x00dev)
+{
+	struct data_queue *queue;
+	struct queue_entry *entry;
+	u32 reg;
+	u8 qid;
+	bool match;
+
+	while (kfifo_get(&rt2x00dev->txstatus_fifo, &reg)) {
+		/*
+		 * TX_STA_FIFO_PID_QUEUE is a 2-bit field, thus qid is
+		 * guaranteed to be one of the TX QIDs .
+		 */
+		qid = rt2x00_get_field32(reg, TX_STA_FIFO_PID_QUEUE);
+		queue = rt2x00queue_get_tx_queue(rt2x00dev, qid);
+
+		if (unlikely(rt2x00queue_empty(queue))) {
+			rt2x00_dbg(rt2x00dev, "Got TX status for an empty queue %u, dropping\n",
+				   qid);
+			break;
+		}
+
+		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
+
+		if (unlikely(test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
+			     !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))) {
+			rt2x00_warn(rt2x00dev, "Data pending for entry %u in queue %u\n",
+				    entry->entry_idx, qid);
+			break;
+		}
+
+		match = rt2800_txdone_entry_check(entry, reg);
+		rt2800_txdone_entry(entry, reg, rt2800_drv_get_txwi(entry), match);
+	}
+}
+EXPORT_SYMBOL_GPL(rt2800_txdone);
+
+static inline bool rt2800_entry_txstatus_timeout(struct queue_entry *entry)
+{
+	bool tout;
+
+	if (!test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
+		return false;
+
+	tout = time_after(jiffies, entry->last_action + msecs_to_jiffies(500));
+	if (unlikely(tout))
+		rt2x00_dbg(entry->queue->rt2x00dev,
+			   "TX status timeout for entry %d in queue %d\n",
+			   entry->entry_idx, entry->queue->qid);
+	return tout;
+
+}
+
+bool rt2800_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
+{
+	struct data_queue *queue;
+	struct queue_entry *entry;
+
+	tx_queue_for_each(rt2x00dev, queue) {
+		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
+		if (rt2800_entry_txstatus_timeout(entry))
+			return true;
+	}
+	return false;
+}
+EXPORT_SYMBOL_GPL(rt2800_txstatus_timeout);
+
+void rt2800_txdone_nostatus(struct rt2x00_dev *rt2x00dev)
+{
+	struct data_queue *queue;
+	struct queue_entry *entry;
+
+	/*
+	 * Process any trailing TX status reports for IO failures,
+	 * we loop until we find the first non-IO error entry. This
+	 * can either be a frame which is free, is being uploaded,
+	 * or has completed the upload but didn't have an entry
+	 * in the TX_STAT_FIFO register yet.
+	 */
+	tx_queue_for_each(rt2x00dev, queue) {
+		while (!rt2x00queue_empty(queue)) {
+			entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
+
+			if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
+			    !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
+				break;
+
+			if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags) ||
+			    rt2800_entry_txstatus_timeout(entry))
+				rt2x00lib_txdone_noinfo(entry, TXDONE_FAILURE);
+			else
+				break;
+		}
+	}
+}
+EXPORT_SYMBOL_GPL(rt2800_txdone_nostatus);
+
 static unsigned int rt2800_hw_beacon_base(struct rt2x00_dev *rt2x00dev,
 					  unsigned int index)
 {
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h
index 51d9c2a932cc..0dff2c7b3010 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.h
@@ -195,6 +195,9 @@ void rt2800_process_rxwi(struct queue_entry *entry, struct rxdone_entry_desc *tx
 
 void rt2800_txdone_entry(struct queue_entry *entry, u32 status, __le32 *txwi,
 			 bool match);
+void rt2800_txdone(struct rt2x00_dev *rt2x00dev);
+void rt2800_txdone_nostatus(struct rt2x00_dev *rt2x00dev);
+bool rt2800_txstatus_timeout(struct rt2x00_dev *rt2x00dev);
 
 void rt2800_write_beacon(struct queue_entry *entry, struct txentry_desc *txdesc);
 void rt2800_clear_beacon(struct queue_entry *entry);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
index 98a7313fea4a..19eabf16147b 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
@@ -116,35 +116,6 @@ static bool rt2800usb_txstatus_pending(struct rt2x00_dev *rt2x00dev)
 	return false;
 }
 
-static inline bool rt2800usb_entry_txstatus_timeout(struct queue_entry *entry)
-{
-	bool tout;
-
-	if (!test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
-		return false;
-
-	tout = time_after(jiffies, entry->last_action + msecs_to_jiffies(500));
-	if (unlikely(tout))
-		rt2x00_dbg(entry->queue->rt2x00dev,
-			   "TX status timeout for entry %d in queue %d\n",
-			   entry->entry_idx, entry->queue->qid);
-	return tout;
-
-}
-
-static bool rt2800usb_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
-{
-	struct data_queue *queue;
-	struct queue_entry *entry;
-
-	tx_queue_for_each(rt2x00dev, queue) {
-		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
-		if (rt2800usb_entry_txstatus_timeout(entry))
-			return true;
-	}
-	return false;
-}
-
 #define TXSTATUS_READ_INTERVAL 1000000
 
 static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
@@ -171,7 +142,7 @@ static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
 	}
 
 	/* Check if there is any entry that timedout waiting on TX status */
-	if (rt2800usb_txstatus_timeout(rt2x00dev))
+	if (rt2800_txstatus_timeout(rt2x00dev))
 		queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
 
 	if (rt2800usb_txstatus_pending(rt2x00dev)) {
@@ -501,123 +472,17 @@ static int rt2800usb_get_tx_data_len(struct queue_entry *entry)
 /*
  * TX control handlers
  */
-static bool rt2800usb_txdone_entry_check(struct queue_entry *entry, u32 reg)
-{
-	__le32 *txwi;
-	u32 word;
-	int wcid, ack, pid;
-	int tx_wcid, tx_ack, tx_pid, is_agg;
-
-	/*
-	 * This frames has returned with an IO error,
-	 * so the status report is not intended for this
-	 * frame.
-	 */
-	if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
-		return false;
-
-	wcid	= rt2x00_get_field32(reg, TX_STA_FIFO_WCID);
-	ack	= rt2x00_get_field32(reg, TX_STA_FIFO_TX_ACK_REQUIRED);
-	pid	= rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE);
-	is_agg	= rt2x00_get_field32(reg, TX_STA_FIFO_TX_AGGRE);
-
-	/*
-	 * Validate if this TX status report is intended for
-	 * this entry by comparing the WCID/ACK/PID fields.
-	 */
-	txwi = rt2800usb_get_txwi(entry);
-
-	word = rt2x00_desc_read(txwi, 1);
-	tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
-	tx_ack  = rt2x00_get_field32(word, TXWI_W1_ACK);
-	tx_pid  = rt2x00_get_field32(word, TXWI_W1_PACKETID);
-
-	if (wcid != tx_wcid || ack != tx_ack || (!is_agg && pid != tx_pid)) {
-		rt2x00_dbg(entry->queue->rt2x00dev,
-			   "TX status report missed for queue %d entry %d\n",
-			   entry->queue->qid, entry->entry_idx);
-		return false;
-	}
-
-	return true;
-}
-
-static void rt2800usb_txdone(struct rt2x00_dev *rt2x00dev)
-{
-	struct data_queue *queue;
-	struct queue_entry *entry;
-	u32 reg;
-	u8 qid;
-	bool match;
-
-	while (kfifo_get(&rt2x00dev->txstatus_fifo, &reg)) {
-		/*
-		 * TX_STA_FIFO_PID_QUEUE is a 2-bit field, thus qid is
-		 * guaranteed to be one of the TX QIDs .
-		 */
-		qid = rt2x00_get_field32(reg, TX_STA_FIFO_PID_QUEUE);
-		queue = rt2x00queue_get_tx_queue(rt2x00dev, qid);
-
-		if (unlikely(rt2x00queue_empty(queue))) {
-			rt2x00_dbg(rt2x00dev, "Got TX status for an empty queue %u, dropping\n",
-				   qid);
-			break;
-		}
-
-		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
-
-		if (unlikely(test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
-			     !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))) {
-			rt2x00_warn(rt2x00dev, "Data pending for entry %u in queue %u\n",
-				    entry->entry_idx, qid);
-			break;
-		}
-
-		match = rt2800usb_txdone_entry_check(entry, reg);
-		rt2800_txdone_entry(entry, reg, rt2800usb_get_txwi(entry), match);
-	}
-}
-
-static void rt2800usb_txdone_nostatus(struct rt2x00_dev *rt2x00dev)
-{
-	struct data_queue *queue;
-	struct queue_entry *entry;
-
-	/*
-	 * Process any trailing TX status reports for IO failures,
-	 * we loop until we find the first non-IO error entry. This
-	 * can either be a frame which is free, is being uploaded,
-	 * or has completed the upload but didn't have an entry
-	 * in the TX_STAT_FIFO register yet.
-	 */
-	tx_queue_for_each(rt2x00dev, queue) {
-		while (!rt2x00queue_empty(queue)) {
-			entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
-
-			if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
-			    !test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
-				break;
-
-			if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags) ||
-			    rt2800usb_entry_txstatus_timeout(entry))
-				rt2x00lib_txdone_noinfo(entry, TXDONE_FAILURE);
-			else
-				break;
-		}
-	}
-}
-
 static void rt2800usb_work_txdone(struct work_struct *work)
 {
 	struct rt2x00_dev *rt2x00dev =
 	    container_of(work, struct rt2x00_dev, txdone_work);
 
 	while (!kfifo_is_empty(&rt2x00dev->txstatus_fifo) ||
-	       rt2800usb_txstatus_timeout(rt2x00dev)) {
+	       rt2800_txstatus_timeout(rt2x00dev)) {
 
-		rt2800usb_txdone(rt2x00dev);
+		rt2800_txdone(rt2x00dev);
 
-		rt2800usb_txdone_nostatus(rt2x00dev);
+		rt2800_txdone_nostatus(rt2x00dev);
 
 		/*
 		 * The hw may delay sending the packet after DMA complete
-- 
2.7.5

[PATCH 2/5] rt2800mmio: use txdone/txstatus routines from lib

[Stanislaw Gruszka]

Use usb txdone/txstatus routines (now in rt2800libc) for mmio devices.

Note this also change how we handle INT_SOURCE_CSR_TX_FIFO_STATUS
interrupt. Now it is disabled since IRQ routine till end of the txstatus
tasklet (the same behaviour like others interrupts). Reason to do not
disable this interrupt was not to miss any tx status from 16 entries
FIFO register. Now, since we check for tx status timeout, we can
allow to miss some tx statuses. However this will be improved in further
patch where I also implement read status FIFO register in the tasklet.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.c  | 180 +----------------------
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c |   1 +
 2 files changed, 9 insertions(+), 172 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
index e1a7ed7e4892..aa8449a5e8fe 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
@@ -175,161 +175,6 @@ static void rt2800mmio_wakeup(struct rt2x00_dev *rt2x00dev)
 	rt2800_config(rt2x00dev, &libconf, IEEE80211_CONF_CHANGE_PS);
 }
 
-static bool rt2800mmio_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);
-	word = rt2x00_desc_read(txwi, 1);
-	tx_wcid = rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
-
-	return (tx_wcid == wcid);
-}
-
-static bool rt2800mmio_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 (rt2800mmio_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 rt2800mmio_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 rt2800mmio_txdone_release_entries(struct queue_entry *entry,
-					      void *data)
-{
-	if (test_bit(ENTRY_DATA_STATUS_SET, &entry->flags)) {
-		rt2800_txdone_entry(entry, entry->status,
-				    rt2800mmio_get_txwi(entry), true);
-		return false;
-	}
-
-	/* No more frames to release */
-	return true;
-}
-
-static bool rt2800mmio_txdone(struct rt2x00_dev *rt2x00dev)
-{
-	struct data_queue *queue;
-	u32 status;
-	u8 qid;
-	int max_tx_done = 16;
-
-	while (kfifo_get(&rt2x00dev->txstatus_fifo, &status)) {
-		qid = rt2x00_get_field32(status, TX_STA_FIFO_PID_QUEUE);
-		if (unlikely(qid >= QID_RX)) {
-			/*
-			 * Unknown queue, this shouldn't happen. Just drop
-			 * this tx status.
-			 */
-			rt2x00_warn(rt2x00dev, "Got TX status report with unexpected pid %u, dropping\n",
-				    qid);
-			break;
-		}
-
-		queue = rt2x00queue_get_tx_queue(rt2x00dev, qid);
-		if (unlikely(queue == NULL)) {
-			/*
-			 * The queue is NULL, this shouldn't happen. Stop
-			 * processing here and drop the tx status
-			 */
-			rt2x00_warn(rt2x00dev, "Got TX status for an unavailable queue %u, dropping\n",
-				    qid);
-			break;
-		}
-
-		if (unlikely(rt2x00queue_empty(queue))) {
-			/*
-			 * The queue is empty. Stop processing here
-			 * and drop the tx status.
-			 */
-			rt2x00_warn(rt2x00dev, "Got TX status for an empty queue %u, dropping\n",
-				    qid);
-			break;
-		}
-
-		/*
-		 * Let's associate this tx status with the first
-		 * matching frame.
-		 */
-		if (!rt2x00queue_for_each_entry(queue, Q_INDEX_DONE,
-						Q_INDEX, &status,
-						rt2800mmio_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,
-							rt2800mmio_txdone_match_first)) {
-				rt2x00_warn(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,
-					   rt2800mmio_txdone_release_entries);
-
-		if (--max_tx_done == 0)
-			break;
-	}
-
-	return !max_tx_done;
-}
-
 static inline void rt2800mmio_enable_interrupt(struct rt2x00_dev *rt2x00dev,
 					       struct rt2x00_field32 irq_field)
 {
@@ -349,14 +194,14 @@ static inline void rt2800mmio_enable_interrupt(struct rt2x00_dev *rt2x00dev,
 void rt2800mmio_txstatus_tasklet(unsigned long data)
 {
 	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
-	if (rt2800mmio_txdone(rt2x00dev))
-		tasklet_schedule(&rt2x00dev->txstatus_tasklet);
 
-	/*
-	 * No need to enable the tx status interrupt here as we always
-	 * leave it enabled to minimize the possibility of a tx status
-	 * register overflow. See comment in interrupt handler.
-	 */
+	rt2800_txdone(rt2x00dev);
+
+	rt2800_txdone_nostatus(rt2x00dev);
+
+	if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+		rt2800mmio_enable_interrupt(rt2x00dev,
+					    INT_SOURCE_CSR_TX_FIFO_STATUS);
 }
 EXPORT_SYMBOL_GPL(rt2800mmio_txstatus_tasklet);
 
@@ -440,10 +285,6 @@ static void rt2800mmio_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
 	 * because we can schedule the tasklet multiple times (when the
 	 * interrupt fires again during tx status processing).
 	 *
-	 * Furthermore we don't disable the TX_FIFO_STATUS
-	 * interrupt here but leave it enabled so that the TX_STA_FIFO
-	 * can also be read while the tx status tasklet gets executed.
-	 *
 	 * Since we have only one producer and one consumer we don't
 	 * need to lock the kfifo.
 	 */
@@ -485,13 +326,8 @@ irqreturn_t rt2800mmio_interrupt(int irq, void *dev_instance)
 	 */
 	mask = ~reg;
 
-	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS)) {
+	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS))
 		rt2800mmio_txstatus_interrupt(rt2x00dev);
-		/*
-		 * Never disable the TX_FIFO_STATUS interrupt.
-		 */
-		rt2x00_set_field32(&mask, INT_MASK_CSR_TX_FIFO_STATUS, 1);
-	}
 
 	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_PRE_TBTT))
 		tasklet_hi_schedule(&rt2x00dev->pretbtt_tasklet);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 710e9641552e..6e8beb7ea350 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -113,6 +113,7 @@ int rt2x00queue_map_txskb(struct queue_entry *entry)
 		return -ENOMEM;
 
 	skbdesc->flags |= SKBDESC_DMA_MAPPED_TX;
+	rt2x00lib_dmadone(entry);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(rt2x00queue_map_txskb);
-- 
2.7.5

[PATCH 3/5] rt2x00: do not check for txstatus timeout every time on tasklet

[Stanislaw Gruszka]

Do not check for tx status timeout everytime we perform txstatus tasklet.
Perform check once per half a second.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c   | 7 +++++++
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.c  | 3 ++-
 drivers/net/wireless/ralink/rt2x00/rt2x00.h      | 2 ++
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 1 +
 4 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 9f2835729016..0c56c7dca55f 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1158,11 +1158,18 @@ bool rt2800_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
 	struct data_queue *queue;
 	struct queue_entry *entry;
 
+	if (time_before(jiffies,
+			rt2x00dev->last_nostatus_check + msecs_to_jiffies(500)))
+		return false;
+
+	rt2x00dev->last_nostatus_check = jiffies;
+
 	tx_queue_for_each(rt2x00dev, queue) {
 		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
 		if (rt2800_entry_txstatus_timeout(entry))
 			return true;
 	}
+
 	return false;
 }
 EXPORT_SYMBOL_GPL(rt2800_txstatus_timeout);
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
index aa8449a5e8fe..d0426314c2df 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
@@ -197,7 +197,8 @@ void rt2800mmio_txstatus_tasklet(unsigned long data)
 
 	rt2800_txdone(rt2x00dev);
 
-	rt2800_txdone_nostatus(rt2x00dev);
+	if (rt2800_txstatus_timeout(rt2x00dev))
+		rt2800_txdone_nostatus(rt2x00dev);
 
 	if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
 		rt2800mmio_enable_interrupt(rt2x00dev,
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index a279a4363bc1..af062cda4a23 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -980,6 +980,8 @@ struct rt2x00_dev {
 	 */
 	DECLARE_KFIFO_PTR(txstatus_fifo, u32);
 
+	unsigned long last_nostatus_check;
+
 	/*
 	 * Timer to ensure tx status reports are read (rt2800usb).
 	 */
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index 6e8beb7ea350..92ddc19e7bf7 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -1039,6 +1039,7 @@ void rt2x00queue_start_queues(struct rt2x00_dev *rt2x00dev)
 	 */
 	tx_queue_for_each(rt2x00dev, queue)
 		rt2x00queue_start_queue(queue);
+	rt2x00dev->last_nostatus_check = jiffies;
 
 	rt2x00queue_start_queue(rt2x00dev->rx);
 }
-- 
2.7.5

[PATCH 4/5] rt2x00: use different txstatus timeouts when flushing

[Stanislaw Gruszka]

Use different tx status timeouts for normal operation and when flushing.
This increase timeout to 2s for normal operation as when there are bad
radio conditions and frames are reposted many times device can not provide
the status for quite long. With new timeout we can still get valid status
on such bad conditions.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 31 +++++++++++++++++---------
 drivers/net/wireless/ralink/rt2x00/rt2x00.h    |  1 +
 drivers/net/wireless/ralink/rt2x00/rt2x00mac.c |  4 ++++
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 0c56c7dca55f..595cb9c90b81 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1137,36 +1137,47 @@ void rt2800_txdone(struct rt2x00_dev *rt2x00dev)
 }
 EXPORT_SYMBOL_GPL(rt2800_txdone);
 
-static inline bool rt2800_entry_txstatus_timeout(struct queue_entry *entry)
+static inline bool rt2800_entry_txstatus_timeout(struct rt2x00_dev *rt2x00dev,
+						 struct queue_entry *entry)
 {
-	bool tout;
+	bool ret;
+	unsigned long tout;
 
 	if (!test_bit(ENTRY_DATA_STATUS_PENDING, &entry->flags))
 		return false;
 
-	tout = time_after(jiffies, entry->last_action + msecs_to_jiffies(500));
-	if (unlikely(tout))
+	if (test_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags))
+		tout = msecs_to_jiffies(100);
+	else
+		tout = msecs_to_jiffies(2000);
+
+	ret = time_after(jiffies, entry->last_action + tout);
+	if (unlikely(ret))
 		rt2x00_dbg(entry->queue->rt2x00dev,
 			   "TX status timeout for entry %d in queue %d\n",
 			   entry->entry_idx, entry->queue->qid);
-	return tout;
-
+	return ret;
 }
 
 bool rt2800_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
 {
 	struct data_queue *queue;
 	struct queue_entry *entry;
+	unsigned long tout;
+
+	if (test_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags))
+		tout = msecs_to_jiffies(50);
+	else
+		tout = msecs_to_jiffies(1000);
 
-	if (time_before(jiffies,
-			rt2x00dev->last_nostatus_check + msecs_to_jiffies(500)))
+	if (time_before(jiffies, rt2x00dev->last_nostatus_check + tout))
 		return false;
 
 	rt2x00dev->last_nostatus_check = jiffies;
 
 	tx_queue_for_each(rt2x00dev, queue) {
 		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
-		if (rt2800_entry_txstatus_timeout(entry))
+		if (rt2800_entry_txstatus_timeout(rt2x00dev, entry))
 			return true;
 	}
 
@@ -1195,7 +1206,7 @@ void rt2800_txdone_nostatus(struct rt2x00_dev *rt2x00dev)
 				break;
 
 			if (test_bit(ENTRY_DATA_IO_FAILED, &entry->flags) ||
-			    rt2800_entry_txstatus_timeout(entry))
+			    rt2800_entry_txstatus_timeout(rt2x00dev, entry))
 				rt2x00lib_txdone_noinfo(entry, TXDONE_FAILURE);
 			else
 				break;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00.h b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
index af062cda4a23..4b1744e9fb78 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00.h
@@ -665,6 +665,7 @@ enum rt2x00_state_flags {
 	DEVICE_STATE_STARTED,
 	DEVICE_STATE_ENABLED_RADIO,
 	DEVICE_STATE_SCANNING,
+	DEVICE_STATE_FLUSHING,
 
 	/*
 	 * Driver configuration
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
index fa2fd64084ac..2825560e2424 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c
@@ -720,8 +720,12 @@ void rt2x00mac_flush(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
 	if (!test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags))
 		return;
 
+	set_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags);
+
 	tx_queue_for_each(rt2x00dev, queue)
 		rt2x00queue_flush_queue(queue, drop);
+
+	clear_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags);
 }
 EXPORT_SYMBOL_GPL(rt2x00mac_flush);
 
-- 
2.7.5

[PATCH 5/5] rt2800: flush and txstatus rework for rt2800mmio

[Stanislaw Gruszka]

Implement custom rt2800mmio flush routine and change txstatus
routine to read TX_STA_FIFO also in the tasklet.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2800lib.c  |  14 ++-
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.c | 118 ++++++++++++++++++------
 drivers/net/wireless/ralink/rt2x00/rt2800mmio.h |   1 +
 drivers/net/wireless/ralink/rt2x00/rt2800pci.c  |   2 +-
 4 files changed, 97 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
index 595cb9c90b81..9e7b8933d30c 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800lib.c
@@ -1147,7 +1147,7 @@ static inline bool rt2800_entry_txstatus_timeout(struct rt2x00_dev *rt2x00dev,
 		return false;
 
 	if (test_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags))
-		tout = msecs_to_jiffies(100);
+		tout = msecs_to_jiffies(50);
 	else
 		tout = msecs_to_jiffies(2000);
 
@@ -1163,15 +1163,13 @@ bool rt2800_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
 {
 	struct data_queue *queue;
 	struct queue_entry *entry;
-	unsigned long tout;
 
-	if (test_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags))
-		tout = msecs_to_jiffies(50);
-	else
-		tout = msecs_to_jiffies(1000);
+	if (!test_bit(DEVICE_STATE_FLUSHING, &rt2x00dev->flags)) {
+		unsigned long tout = msecs_to_jiffies(1000);
 
-	if (time_before(jiffies, rt2x00dev->last_nostatus_check + tout))
-		return false;
+		if (time_before(jiffies, rt2x00dev->last_nostatus_check + tout))
+			return false;
+	}
 
 	rt2x00dev->last_nostatus_check = jiffies;
 
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
index d0426314c2df..ddb88cfeace2 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.c
@@ -191,21 +191,6 @@ static inline void rt2800mmio_enable_interrupt(struct rt2x00_dev *rt2x00dev,
 	spin_unlock_irq(&rt2x00dev->irqmask_lock);
 }
 
-void rt2800mmio_txstatus_tasklet(unsigned long data)
-{
-	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
-
-	rt2800_txdone(rt2x00dev);
-
-	if (rt2800_txstatus_timeout(rt2x00dev))
-		rt2800_txdone_nostatus(rt2x00dev);
-
-	if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
-		rt2800mmio_enable_interrupt(rt2x00dev,
-					    INT_SOURCE_CSR_TX_FIFO_STATUS);
-}
-EXPORT_SYMBOL_GPL(rt2800mmio_txstatus_tasklet);
-
 void rt2800mmio_pretbtt_tasklet(unsigned long data)
 {
 	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
@@ -270,12 +255,26 @@ void rt2800mmio_autowake_tasklet(unsigned long data)
 }
 EXPORT_SYMBOL_GPL(rt2800mmio_autowake_tasklet);
 
-static void rt2800mmio_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
+static void rt2800mmio_txdone(struct rt2x00_dev *rt2x00dev)
+{
+	bool timeout = false;
+
+	while (!kfifo_is_empty(&rt2x00dev->txstatus_fifo) ||
+	       (timeout = rt2800_txstatus_timeout(rt2x00dev))) {
+
+		rt2800_txdone(rt2x00dev);
+
+		if (timeout)
+			rt2800_txdone_nostatus(rt2x00dev);
+	}
+}
+
+static bool rt2800mmio_fetch_txstatus(struct rt2x00_dev *rt2x00dev)
 {
 	u32 status;
-	int i;
+	bool more = false;
 
-	/*
+	/* FIXEME: rewrite this comment
 	 * The TX_FIFO_STATUS interrupt needs special care. We should
 	 * read TX_STA_FIFO but we should do it immediately as otherwise
 	 * the register can overflow and we would lose status reports.
@@ -286,25 +285,37 @@ static void rt2800mmio_txstatus_interrupt(struct rt2x00_dev *rt2x00dev)
 	 * because we can schedule the tasklet multiple times (when the
 	 * interrupt fires again during tx status processing).
 	 *
-	 * Since we have only one producer and one consumer we don't
+	 * txstatus tasklet is called with INT_SOURCE_CSR_TX_FIFO_STATUS
+	 * disabled so have only one producer and one consumer - we don't
 	 * need to lock the kfifo.
 	 */
-	for (i = 0; i < rt2x00dev->tx->limit; i++) {
+	while (!kfifo_is_full(&rt2x00dev->txstatus_fifo)) {
 		status = rt2x00mmio_register_read(rt2x00dev, TX_STA_FIFO);
-
 		if (!rt2x00_get_field32(status, TX_STA_FIFO_VALID))
 			break;
 
-		if (!kfifo_put(&rt2x00dev->txstatus_fifo, status)) {
-			rt2x00_warn(rt2x00dev, "TX status FIFO overrun, drop tx status report\n");
-			break;
-		}
+		kfifo_put(&rt2x00dev->txstatus_fifo, status);
+		more = true;
 	}
 
-	/* Schedule the tasklet for processing the tx status. */
-	tasklet_schedule(&rt2x00dev->txstatus_tasklet);
+	return more;
 }
 
+void rt2800mmio_txstatus_tasklet(unsigned long data)
+{
+	struct rt2x00_dev *rt2x00dev = (struct rt2x00_dev *)data;
+
+	do {
+		rt2800mmio_txdone(rt2x00dev);
+
+	} while (rt2800mmio_fetch_txstatus(rt2x00dev));
+
+	if (test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
+		rt2800mmio_enable_interrupt(rt2x00dev,
+					    INT_SOURCE_CSR_TX_FIFO_STATUS);
+}
+EXPORT_SYMBOL_GPL(rt2800mmio_txstatus_tasklet);
+
 irqreturn_t rt2800mmio_interrupt(int irq, void *dev_instance)
 {
 	struct rt2x00_dev *rt2x00dev = dev_instance;
@@ -327,8 +338,10 @@ irqreturn_t rt2800mmio_interrupt(int irq, void *dev_instance)
 	 */
 	mask = ~reg;
 
-	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS))
-		rt2800mmio_txstatus_interrupt(rt2x00dev);
+	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS)) {
+		rt2800mmio_fetch_txstatus(rt2x00dev);
+		tasklet_schedule(&rt2x00dev->txstatus_tasklet);
+	}
 
 	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_PRE_TBTT))
 		tasklet_hi_schedule(&rt2x00dev->pretbtt_tasklet);
@@ -453,6 +466,53 @@ void rt2800mmio_kick_queue(struct data_queue *queue)
 }
 EXPORT_SYMBOL_GPL(rt2800mmio_kick_queue);
 
+void rt2800mmio_flush_queue(struct data_queue *queue, bool drop)
+{
+	struct rt2x00_dev *rt2x00dev = queue->rt2x00dev;
+	bool tx_queue = false;
+	unsigned int i;
+
+	switch (queue->qid) {
+	case QID_AC_VO:
+	case QID_AC_VI:
+	case QID_AC_BE:
+	case QID_AC_BK:
+		tx_queue = true;
+		break;
+	case QID_RX:
+		break;
+	default:
+		return;
+	}
+
+	for (i = 0; i < 5; i++) {
+		/*
+		 * Check if the driver is already done, otherwise we
+		 * have to sleep a little while to give the driver/hw
+		 * the oppurtunity to complete interrupt process itself.
+		 */
+		if (rt2x00queue_empty(queue))
+			break;
+
+		/*
+		 * For TX queues schedule completion tasklet to catch
+		 * tx status timeouts, othewise just wait.
+		 */
+		if (tx_queue) {
+			tasklet_disable(&rt2x00dev->txstatus_tasklet);
+			rt2800mmio_txdone(rt2x00dev);
+			tasklet_enable(&rt2x00dev->txstatus_tasklet);
+		}
+
+		/*
+		 * Wait for a little while to give the driver
+		 * the oppurtunity to recover itself.
+		 */
+		msleep(50);
+	}
+}
+EXPORT_SYMBOL_GPL(rt2800mmio_flush_queue);
+
 void rt2800mmio_stop_queue(struct data_queue *queue)
 {
 	struct rt2x00_dev *rt2x00dev = queue->rt2x00dev;
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.h b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.h
index b63312ce3f27..3a513273f414 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800mmio.h
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800mmio.h
@@ -148,6 +148,7 @@ void rt2800mmio_toggle_irq(struct rt2x00_dev *rt2x00dev,
 /* Queue handlers */
 void rt2800mmio_start_queue(struct data_queue *queue);
 void rt2800mmio_kick_queue(struct data_queue *queue);
+void rt2800mmio_flush_queue(struct data_queue *queue, bool drop);
 void rt2800mmio_stop_queue(struct data_queue *queue);
 void rt2800mmio_queue_init(struct data_queue *queue);
 
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2800pci.c b/drivers/net/wireless/ralink/rt2x00/rt2800pci.c
index 71b1affc3885..0291441ac548 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800pci.c
@@ -364,7 +364,7 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
 	.start_queue		= rt2800mmio_start_queue,
 	.kick_queue		= rt2800mmio_kick_queue,
 	.stop_queue		= rt2800mmio_stop_queue,
-	.flush_queue		= rt2x00mmio_flush_queue,
+	.flush_queue		= rt2800mmio_flush_queue,
 	.write_tx_desc		= rt2800mmio_write_tx_desc,
 	.write_tx_data		= rt2800_write_tx_data,
 	.write_beacon		= rt2800_write_beacon,
-- 
2.7.5

Re: [PATCH 1/5] rt2800: move usb specific txdone/txstatus routines to rt2800lib

[Kalle Valo]

Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> In order to reuse usb txdone/txstatus routines for mmio, move them
> to common rt2800lib.c file.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

5 patches applied to wireless-drivers-next.git, thanks.

5c656c71b1bf rt2800: move usb specific txdone/txstatus routines to rt2800lib
0b0d556e0ebb rt2800mmio: use txdone/txstatus routines from lib
5022efb50f62 rt2x00: do not check for txstatus timeout every time on tasklet
adf26a356f13 rt2x00: use different txstatus timeouts when flushing
0240564430c0 rt2800: flush and txstatus rework for rt2800mmio

-- 
https://patchwork.kernel.org/patch/10615575/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Tomislav Požega]

Hi

As suspected this changeset causes throughput regression.

Below screenshots show iperf test from MS150N (RF5370) device connected to RT3070 adapter running AP mode:

This is with standard openwrt build without any rt2x00 changes:

[url=https://postimg.cc/BtYQLf6r][img]https://i.postimg.cc/BtYQLf6r/shot-2018-10-04_17-23-56.jpg[/img][/url]

And this printscreen show iperf test with your changes:

[url=https://postimg.cc/D8Sf1p48][img]https://i.postimg.cc/D8Sf1p48/shot-2018-10-04_17-42-09.jpg[/img][/url]

Atheros card connected to RT3070 iperf test difference was negligible (1Mbps or less) on bodhi system, but
it started to throw out reorder messages on my standard ubuntu after your changes:

[url=https://postimg.cc/SjJbP2SP][img]https://i.postimg.cc/SjJbP2SP/Screenshot.png[/img][/url]

My advice: stop sending low-quality patches and do some testing before submission.

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Stanislaw Gruszka]

(adding back removed CCs)

On Fri, Oct 05, 2018 at 01:51:42AM +0200, Tomislav Požega wrote:
> Hi

Hi.

> As suspected this changeset causes throughput regression.

You seems to have prejudice against my work :-)

> Below screenshots show iperf test from MS150N (RF5370) device connected to RT3070 adapter running AP mode:
> 
> This is with standard openwrt build without any rt2x00 changes:
> 
> [url=https://postimg.cc/BtYQLf6r][img]https://i.postimg.cc/BtYQLf6r/shot-2018-10-04_17-23-56.jpg[/img][/url]
> 
> And this printscreen show iperf test with your changes:
> 
> [url=https://postimg.cc/D8Sf1p48][img]https://i.postimg.cc/D8Sf1p48/shot-2018-10-04_17-42-09.jpg[/img][/url]

My experience is that performance between two rt2800 devices vary with
no apparent reason. There are two problems I know that maigh affect
performance at random (and I think there are also some other low level
problems that I'm not aware of that cause performance fluctuations).

First problem is that HW aggregate RATE_PROBE frames with other frames
at different rate, so we can not do rate probing properly for rate
control algorithm.

Second problem: we send BAR when we fail to send a frame and this might
have positive and negative effect, depend what remote hardware do when it
gets BAR. This seems to be problem when two rt2800 devices are connected
and not a problem if rt2800 is connected with ath or iwl devices.

> Atheros card connected to RT3070 iperf test difference was negligible (1Mbps or less) on bodhi system, but
> it started to throw out reorder messages on my standard ubuntu after your changes:
> 
> [url=https://postimg.cc/SjJbP2SP][img]https://i.postimg.cc/SjJbP2SP/Screenshot.png[/img][/url]

Ok, thats seems not right, I will try to reproduce this.

> My advice: stop sending low-quality patches and do some testing before submission.

My advice: stop being arrogant if you want to work with others.

Patches were tested on USB devices. At first I thought they broke
rt2800usb support:
https://bugzilla.kernel.org/show_bug.cgi?id=82751#c17
but then when I wanted to debug that, they start to work;
https://bugzilla.kernel.org/show_bug.cgi?id=82751#c33
I assumed that previous breakage was caused by some different
change not related with those patches. 

Anyway I would appreciate any additional testing of my rt2x00 patches
as well as code review, if anyone would like to do this.

Thanks
Stanislaw

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Stanislaw Gruszka]

On Fri, Oct 05, 2018 at 09:44:23AM +0200, Stanislaw Gruszka wrote:
> > Below screenshots show iperf test from MS150N (RF5370) device connected to RT3070 adapter running AP mode:
> > 
> > This is with standard openwrt build without any rt2x00 changes:
> > 
> > [url=https://postimg.cc/BtYQLf6r][img]https://i.postimg.cc/BtYQLf6r/shot-2018-10-04_17-23-56.jpg[/img][/url]
> > 
> > And this printscreen show iperf test with your changes:
> > 
> > [url=https://postimg.cc/D8Sf1p48][img]https://i.postimg.cc/D8Sf1p48/shot-2018-10-04_17-42-09.jpg[/img][/url]
> 
> My experience is that performance between two rt2800 devices vary with
> no apparent reason. There are two problems I know that maigh affect
> performance at random (and I think there are also some other low level
> problems that I'm not aware of that cause performance fluctuations).
> 
> First problem is that HW aggregate RATE_PROBE frames with other frames
> at different rate, so we can not do rate probing properly for rate
> control algorithm.
> 
> Second problem: we send BAR when we fail to send a frame and this might
> have positive and negative effect, depend what remote hardware do when it
> gets BAR. This seems to be problem when two rt2800 devices are connected
> and not a problem if rt2800 is connected with ath or iwl devices.

I have such results without patches:

[root@dhcp-27-155 ~]# iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 60040 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  1.67 MBytes  14.0 Mbits/sec    0   72.1 KBytes       
[  4]   1.00-2.00   sec  1.43 MBytes  12.0 Mbits/sec    0    123 KBytes       
[  4]   2.00-3.00   sec  1.93 MBytes  16.2 Mbits/sec    0    194 KBytes       
[  4]   3.00-4.00   sec  4.47 MBytes  37.5 Mbits/sec    0    328 KBytes       
[  4]   4.00-5.00   sec  3.23 MBytes  27.1 Mbits/sec    0    402 KBytes       
[  4]   5.00-6.00   sec  3.73 MBytes  31.3 Mbits/sec    0    464 KBytes       
[  4]   6.00-7.00   sec  4.72 MBytes  39.6 Mbits/sec    0    489 KBytes       
[  4]   7.00-8.00   sec  4.16 MBytes  34.9 Mbits/sec    0    516 KBytes       
[  4]   8.00-9.00   sec  3.91 MBytes  32.8 Mbits/sec    0    570 KBytes       
[  4]   9.00-10.00  sec  3.85 MBytes  32.3 Mbits/sec    0    570 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  33.1 MBytes  27.8 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  31.8 MBytes  26.7 Mbits/sec
receiver

iperf Done.
[root@dhcp-27-155 ~]# iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 60044 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  1.63 MBytes  13.7 Mbits/sec    0   69.3 KBytes       
[  4]   1.00-2.00   sec  1.49 MBytes  12.5 Mbits/sec    0    129 KBytes       
[  4]   2.00-3.00   sec  2.11 MBytes  17.7 Mbits/sec    0    208 KBytes       
[  4]   3.00-4.00   sec  4.41 MBytes  37.0 Mbits/sec    0    361 KBytes       
[  4]   4.00-5.00   sec  3.85 MBytes  32.3 Mbits/sec    0    478 KBytes       
[  4]   5.00-6.00   sec  4.41 MBytes  37.0 Mbits/sec    0    478 KBytes       
[  4]   6.00-7.00   sec  3.17 MBytes  26.6 Mbits/sec    0    502 KBytes       
[  4]   7.00-8.00   sec  3.73 MBytes  31.3 Mbits/sec    0    527 KBytes       
[  4]   8.00-9.00   sec  3.04 MBytes  25.5 Mbits/sec    0    551 KBytes       
[  4]   9.00-10.00  sec  4.10 MBytes  34.4 Mbits/sec    0    551 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  32.0 MBytes  26.8 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  31.0 MBytes  26.0 Mbits/sec
receiver

iperf Done.
[root@dhcp-27-155 ~]# iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 60048 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  1.44 MBytes  12.0 Mbits/sec    0   60.8 KBytes       
[  4]   1.00-2.00   sec  1.43 MBytes  12.0 Mbits/sec    0    113 KBytes       
[  4]   2.00-3.00   sec  2.61 MBytes  21.9 Mbits/sec    0    212 KBytes       
[  4]   3.00-4.00   sec  3.91 MBytes  32.8 Mbits/sec    0    349 KBytes       
[  4]   4.00-5.00   sec  4.04 MBytes  33.9 Mbits/sec    0    436 KBytes       
[  4]   5.00-6.00   sec  3.73 MBytes  31.3 Mbits/sec    0    494 KBytes       
[  4]   6.00-7.00   sec  3.73 MBytes  31.3 Mbits/sec    0    519 KBytes       
[  4]   7.00-8.00   sec  3.85 MBytes  32.3 Mbits/sec    0    560 KBytes       
[  4]   8.00-9.00   sec  4.16 MBytes  34.9 Mbits/sec    0    594 KBytes       
[  4]   9.00-10.00  sec  4.04 MBytes  33.9 Mbits/sec    0    600 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  32.9 MBytes  27.6 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  31.7 MBytes  26.6 Mbits/sec
receiver

iperf Done.



And with patches:

[stasiu@dhcp-27-155 ~]$ iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 59810 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  1.58 MBytes  13.3 Mbits/sec    0   72.1 KBytes       
[  4]   1.00-2.00   sec  1.68 MBytes  14.1 Mbits/sec    0    130 KBytes       
[  4]   2.00-3.00   sec  3.67 MBytes  30.8 Mbits/sec    0    279 KBytes       
[  4]   3.00-4.00   sec  6.21 MBytes  52.1 Mbits/sec    0    423 KBytes       
[  4]   4.00-5.00   sec  6.28 MBytes  52.6 Mbits/sec    0    495 KBytes       
[  4]   5.00-6.00   sec  6.28 MBytes  52.6 Mbits/sec    0    547 KBytes       
[  4]   6.00-7.00   sec  6.34 MBytes  53.2 Mbits/sec    0    547 KBytes       
[  4]   7.00-8.00   sec  6.34 MBytes  53.2 Mbits/sec    0    573 KBytes       
[  4]   8.00-9.00   sec  5.97 MBytes  50.0 Mbits/sec    0    604 KBytes       
[  4]   9.00-10.00  sec  5.90 MBytes  49.5 Mbits/sec    0    604 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  50.2 MBytes  42.1 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  49.1 MBytes  41.2 Mbits/sec
receiver

iperf Done.
[stasiu@dhcp-27-155 ~]$ iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 59814 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  3.73 MBytes  31.3 Mbits/sec    0    192 KBytes       
[  4]   1.00-2.00   sec  6.34 MBytes  53.2 Mbits/sec    0    379 KBytes       
[  4]   2.00-3.00   sec  7.02 MBytes  58.9 Mbits/sec    0    458 KBytes       
[  4]   3.00-4.00   sec  5.97 MBytes  50.0 Mbits/sec    0    501 KBytes       
[  4]   4.00-5.00   sec  5.90 MBytes  49.5 Mbits/sec    0    523 KBytes       
[  4]   5.00-6.00   sec  6.09 MBytes  51.1 Mbits/sec    0    550 KBytes       
[  4]   6.00-7.00   sec  6.28 MBytes  52.6 Mbits/sec    0    588 KBytes       
[  4]   7.00-8.00   sec  5.90 MBytes  49.5 Mbits/sec    0    588 KBytes       
[  4]   8.00-9.00   sec  6.28 MBytes  52.7 Mbits/sec    0    588 KBytes       
[  4]   9.00-10.00  sec  6.46 MBytes  54.2 Mbits/sec    0    617 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  60.0 MBytes  50.3 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  58.6 MBytes  49.2 Mbits/sec
receiver

iperf Done.
[stasiu@dhcp-27-155 ~]$ iperf3 -c 192.168.9.1
Connecting to host 192.168.9.1, port 5201
[  4] local 192.168.9.11 port 59818 connected to 192.168.9.1 port 5201
[ ID] Interval           Transfer     Bandwidth       Retr  Cwnd
[  4]   0.00-1.00   sec  2.29 MBytes  19.2 Mbits/sec    0    107 KBytes       
[  4]   1.00-2.00   sec  3.11 MBytes  26.1 Mbits/sec    0    238 KBytes       
[  4]   2.00-3.00   sec  6.71 MBytes  56.3 Mbits/sec    0    447 KBytes       
[  4]   3.00-4.00   sec  6.59 MBytes  55.3 Mbits/sec    0    484 KBytes       
[  4]   4.00-5.00   sec  6.28 MBytes  52.6 Mbits/sec    0    533 KBytes       
[  4]   5.00-6.00   sec  6.46 MBytes  54.2 Mbits/sec    0    559 KBytes       
[  4]   6.00-7.00   sec  4.72 MBytes  39.6 Mbits/sec    0    559 KBytes       
[  4]   7.00-8.00   sec  3.98 MBytes  33.4 Mbits/sec    0    587 KBytes       
[  4]   8.00-9.00   sec  3.36 MBytes  28.1 Mbits/sec    0    587 KBytes       
[  4]   9.00-10.00  sec  4.04 MBytes  33.9 Mbits/sec    0    587 KBytes       
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bandwidth       Retr
[  4]   0.00-10.00  sec  47.5 MBytes  39.9 Mbits/sec    0
sender
[  4]   0.00-10.00  sec  46.1 MBytes  38.7 Mbits/sec
receiver

iperf Done.

when connecting two rt2800usb devices one in AP mode second in STA mode.
Results with patches are somewhat better, but this is by accident.
Basically performance randomly vary on those testing.

Only 'flush' was chagned for USB, otherwise the code was just moved to
rt2800lib. So probability of regression there is low. Much more changes
were done for MMIO by those patches, so I would be more worry about
that. But this was tested by me and various other users. 

Thanks
Stanislaw

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Felix Fietkau]

On 2018-10-05 09:44, Stanislaw Gruszka wrote:
> (adding back removed CCs)
> 
> On Fri, Oct 05, 2018 at 01:51:42AM +0200, Tomislav Požega wrote:
>> Hi
> 
> Hi.
> 
>> As suspected this changeset causes throughput regression.
> 
> You seems to have prejudice against my work :-)
> 
>> Below screenshots show iperf test from MS150N (RF5370) device connected to RT3070 adapter running AP mode:
>> 
>> This is with standard openwrt build without any rt2x00 changes:
>> 
>> [url=https://postimg.cc/BtYQLf6r][img]https://i.postimg.cc/BtYQLf6r/shot-2018-10-04_17-23-56.jpg[/img][/url]
>> 
>> And this printscreen show iperf test with your changes:
>> 
>> [url=https://postimg.cc/D8Sf1p48][img]https://i.postimg.cc/D8Sf1p48/shot-2018-10-04_17-42-09.jpg[/img][/url]
> 
> My experience is that performance between two rt2800 devices vary with
> no apparent reason. There are two problems I know that maigh affect
> performance at random (and I think there are also some other low level
> problems that I'm not aware of that cause performance fluctuations).
> 
> First problem is that HW aggregate RATE_PROBE frames with other frames
> at different rate, so we can not do rate probing properly for rate
> control algorithm.
I believe this is fixable. On MT76x2, I was able to use the QSEL field
in the DMA descriptor to force the hw to send it out as a single frame.
It is normally set to 2, you can set it to 0 for frames with
IEEE80211_TX_CTL_RATE_CTRL_PROBE set.
Theoretically, the same should also work on RT2800 devices.

> Second problem: we send BAR when we fail to send a frame and this might
> have positive and negative effect, depend what remote hardware do when it
> gets BAR. This seems to be problem when two rt2800 devices are connected
> and not a problem if rt2800 is connected with ath or iwl devices.
On the receiver side, BAR is typically processed in software. mac80211
handles that, so I don't think there is a lot of room for chipset
specific behavior here.

- Felix

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Stanislaw Gruszka]

On Fri, Oct 05, 2018 at 12:05:56PM +0200, Felix Fietkau wrote:
> On 2018-10-05 09:44, Stanislaw Gruszka wrote:
> > My experience is that performance between two rt2800 devices vary with
> > no apparent reason. There are two problems I know that maigh affect
> > performance at random (and I think there are also some other low level
> > problems that I'm not aware of that cause performance fluctuations).
> > 
> > First problem is that HW aggregate RATE_PROBE frames with other frames
> > at different rate, so we can not do rate probing properly for rate
> > control algorithm.
> I believe this is fixable. On MT76x2, I was able to use the QSEL field
> in the DMA descriptor to force the hw to send it out as a single frame.
> It is normally set to 2, you can set it to 0 for frames with
> IEEE80211_TX_CTL_RATE_CTRL_PROBE set.
> Theoretically, the same should also work on RT2800 devices.

I created a patch that do this 
https://github.com/sgruszka/wireless-drivers-next/commit/846d205edd8c36d1b7828fee54bf4cf40bf8cb1a
but coused problems on some devices and I did not observed correct
behaviour - probe frames were still aggregated.

Perhaps patch has some bug (i.e. some registers have to be initialized
to make QSEL work) or maybe this will only work on some chipsets i.e.
MT7620 and QSEL should be used only conditionally on those chips.

> > Second problem: we send BAR when we fail to send a frame and this might
> > have positive and negative effect, depend what remote hardware do when it
> > gets BAR. This seems to be problem when two rt2800 devices are connected
> > and not a problem if rt2800 is connected with ath or iwl devices.
> On the receiver side, BAR is typically processed in software. mac80211
> handles that, so I don't think there is a lot of room for chipset
> specific behavior here.

This is not what I observed. For me looks like BA is send by FW/HW for
both AMPDU frames and for BAR. And behaviour differs. I have one older
device that send completely bogus SSN (seq num) in solicit BA frame
not correpond to BAR SSN or frames in AMPDU at all. 

Some devices send solicit BA SSN and then send BA with older SSNs,
for frames which should already be flushed.

And some devices send several BA with solicit SSN until catch up to
ack frame that has bigger sequence number than solicit SSN.

Regards 
Stanislaw 

Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework

[Tom Psyborg]

On 05/11/2018, Craig Matsuura <cmatsuura@vivint.com> wrote:
> Tom,
>
>
> I see my name and email in some urls but could not see what you are
> referring too?
>
>
> Craig
>
>
> Craig Matsuura • Technical Director, Embedded Software Architecture
>
> cmatsuura@vivint.com<mailto:cmatsuura@vivint.com> • P: 801.229.6005
>
>
>
> simply smarter • vivint.com
>
> 3401 N Ashton Blvd. Lehi, UT 84043
>
>
>
> [1497369905956_vivint-logo-orange.png]
>
> ________________________________
> From: linux-wireless-owner@vger.kernel.org
> <linux-wireless-owner@vger.kernel.org> on behalf of Tomislav Požega
> <pozega.tomislav@gmail.com>
> Sent: Thursday, October 4, 2018 5:51:42 PM
> To: sgruszka@redhat.com
> Cc: linux-wireless@vger.kernel.org
> Subject: Re: [PATCH 0/5] rt2800mmio txdone/interrupts/flush rework
>
> Hi
>
> As suspected this changeset causes throughput regression.
>
> Below screenshots show iperf test from MS150N (RF5370) device connected to
> RT3070 adapter running AP mode:
>
> This is with standard openwrt build without any rt2x00 changes:
>
> [url=https://postimg.cc/BtYQLf6r][img]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fi.postimg.cc%2FBtYQLf6r%2Fshot-2018-10-04_17-23-56.jpg%5B%2Fimg%5D%5B%2Furl&amp;data=01%7C01%7Ccmatsuura%40vivint.com%7C4b957a380bc24ebda1f708d62a546496%7C54cc98ca024a470185483741e3b8d59d%7C0&amp;sdata=Fsq5PtjbTx3vuXrNyicoVMVyk59dlXin0SecZTKCcOY%3D&amp;reserved=0]
>
> And this printscreen show iperf test with your changes:
>
> [url=https://postimg.cc/D8Sf1p48][img]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fi.postimg.cc%2FD8Sf1p48%2Fshot-2018-10-04_17-42-09.jpg%5B%2Fimg%5D%5B%2Furl&amp;data=01%7C01%7Ccmatsuura%40vivint.com%7C4b957a380bc24ebda1f708d62a546496%7C54cc98ca024a470185483741e3b8d59d%7C0&amp;sdata=w9eslvFB%2F13VBHGG7n48C2KH4ceyk1Acb5G3wEoPdEc%3D&amp;reserved=0]
>
> Atheros card connected to RT3070 iperf test difference was negligible (1Mbps
> or less) on bodhi system, but
> it started to throw out reorder messages on my standard ubuntu after your
> changes:
>
> [url=https://postimg.cc/SjJbP2SP][img]https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fi.postimg.cc%2FSjJbP2SP%2FScreenshot.png%5B%2Fimg%5D%5B%2Furl&amp;data=01%7C01%7Ccmatsuura%40vivint.com%7C4b957a380bc24ebda1f708d62a546496%7C54cc98ca024a470185483741e3b8d59d%7C0&amp;sdata=7AfQGKReywwEYrVZzYOxpLqbmg8kBdCbNE6Mj0%2BmGRM%3D&amp;reserved=0]
>
> My advice: stop sending low-quality patches and do some testing before
> submission.
>

Hi

No idea where did you got that message from, some safelink protection
seem to be added somewhere in the mailing process. You can check my
original mail here:
https://www.mail-archive.com/linux-wireless@vger.kernel.org/msg49652.html
. If interested in links you should open them manually as the URL
formating seems inappropriate.