[rt2x00-users] [PATCHv2] rt2x00: rework tx status handling in rt2800pci

Helmut Schaa helmut.schaa at googlemail.com
Fri Sep 10 09:28:09 UTC 2010


This patch changes the way tx status reports are handled by rt2800pci.
Previously rt2800pci would sometimes lose tx status reports as the
TX_STA_FIFO register is a fifo of 16 entries that can overflow in case
we don't read it often/fast enough. Since interrupts are disabled in the
device during the execution of the interrupt thread it happend sometimes
under high network and CPU load that processing took too long and a few
tx status reports were dropped by the hw.

To fix this issue the TX_STA_FIFO register is read directly in the
interrupt handler and stored in a kfifo which is large enough to hold
all status reports of all used tx queues.

To process the status reports a new tasklet txstatus_tasklet is used.
Using the already used interrupt thread is not possible since we don't
want to disable the TX_FIFO_STATUS interrupt while processing them and
it is not possible to schedule the interrupt thread multiple times for
execution. A tasklet instead can be scheduled multiple times which
allows to leave the TX_FIFO_STATUS interrupt enabled while a previously
scheduled tasklet is still executing.

In short: All other interrupts are handled in the interrupt thread as
before. Only the TX_FIFO_STATUS interrupt is partly handled in the
interrupt handler and finished in the according tasklet.

One drawback of this patch is that it duplicates some code from
rt2800lib. However, that can be cleaned up in the future once the
rt2800usb and rt2800pci tx status handling converge more.

Using this patch on a Ralink RT3052 embedded board gives me a reliable
wireless connection even under high CPU and network load. I've
transferred several gigabytes without any queue lockups.

Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
---

Changes since v1:
* Fix compiler warnings regarding kfifo handling
* Correctly handle different interrupts in the same handler invocation
  (this doesn't happen on the SoC board, but this approach looks safer)

 drivers/net/wireless/rt2x00/rt2800pci.c |  209 ++++++++++++++++++++++++++++--
 drivers/net/wireless/rt2x00/rt2x00.h    |   11 ++
 drivers/net/wireless/rt2x00/rt2x00dev.c |   10 ++
 3 files changed, 216 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
index 005ee15..3256a33 100644
--- a/drivers/net/wireless/rt2x00/rt2800pci.c
+++ b/drivers/net/wireless/rt2x00/rt2800pci.c
@@ -660,6 +660,114 @@ static void rt2800pci_wakeup(struct rt2x00_dev *rt2x00dev)
 	rt2800_config(rt2x00dev, &libconf, IEEE80211_CONF_CHANGE_PS);
 }
 
+static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
+{
+	struct data_queue *queue;
+	struct queue_entry *entry;
+	u32 status, word;
+	__le32 *txwi;
+	struct txdone_entry_desc txdesc;
+	u8 qid;
+	u16 mcs, real_mcs;
+
+	while (!kfifo_is_empty(&rt2x00dev->txstatus_fifo)) {
+		/*
+		 * Just peek the status report and verify if before
+		 * removing it from the FIFO.
+		 */
+		if (kfifo_out_peek(&rt2x00dev->txstatus_fifo, &status,
+				   sizeof(status), 0) != sizeof(status)) {
+			WARN_ON(1);
+			break;
+		}
+
+		qid = rt2x00_get_field32(status, TX_STA_FIFO_PID_TYPE) - 1;
+		if (qid >= QID_RX) {
+			/*
+			 * Unknown queue, this shouldn't happen. Just drop
+			 * this tx status.
+			 */
+			WARN_ON(kfifo_out(&rt2x00dev->txstatus_fifo, &status,
+				  sizeof(status)) != sizeof(status));
+			break;
+		}
+
+		queue = rt2x00queue_get_queue(rt2x00dev, qid);
+		if (unlikely(queue == NULL)) {
+			/*
+			 * The queue is NULL, this shouldn't happen. Stop
+			 * processing here without dropping the tx status
+			 * report.
+			 */
+			break;
+		}
+
+		if (rt2x00queue_empty(queue)) {
+			/*
+			 * The queue is empty. Stop processing here without
+			 * dropping the tx status report.
+			 */
+			break;
+		}
+
+		/* Now remove the tx status from the FIFO */
+		if (kfifo_out(&rt2x00dev->txstatus_fifo, &status,
+			      sizeof(status)) != sizeof(status)) {
+			WARN_ON(1);
+			break;
+		}
+
+		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
+
+		/*
+		 * Obtain the status about this packet.
+		 */
+		txdesc.flags = 0;
+		txwi = rt2800_drv_get_txwi(entry);
+		rt2x00_desc_read(txwi, 0, &word);
+		mcs = rt2x00_get_field32(word, TXWI_W0_MCS);
+		real_mcs = rt2x00_get_field32(status, TX_STA_FIFO_MCS);
+
+		/*
+		 * Ralink has a retry mechanism using a global fallback
+		 * table. We setup this fallback table to try the immediate
+		 * lower rate for all rates. In the TX_STA_FIFO, the MCS field
+		 * always contains the MCS used for the last transmission, be
+		 * it successful or not.
+		 */
+		if (rt2x00_get_field32(status, TX_STA_FIFO_TX_SUCCESS)) {
+			/*
+			 * Transmission succeeded. The number of retries is
+			 * mcs - real_mcs
+			 */
+			__set_bit(TXDONE_SUCCESS, &txdesc.flags);
+			txdesc.retry = ((mcs > real_mcs) ? mcs - real_mcs : 0);
+		} else {
+			/*
+			 * Transmission failed. The number of retries is
+			 * always 7 in this case (for a total number of 8
+			 * frames sent).
+			 */
+			__set_bit(TXDONE_FAILURE, &txdesc.flags);
+			txdesc.retry = rt2x00dev->long_retry;
+		}
+
+		/*
+		 * the frame was retried at least once
+		 * -> hw used fallback rates
+		 */
+		if (txdesc.retry)
+			__set_bit(TXDONE_FALLBACK, &txdesc.flags);
+
+		rt2x00lib_txdone(entry, &txdesc);
+	}
+}
+
+static void rt2800pci_txstatus_tasklet(unsigned long data)
+{
+	rt2800pci_txdone((struct rt2x00_dev *)data);
+}
+
 static irqreturn_t rt2800pci_interrupt_thread(int irq, void *dev_instance)
 {
 	struct rt2x00_dev *rt2x00dev = dev_instance;
@@ -684,13 +792,7 @@ static irqreturn_t rt2800pci_interrupt_thread(int irq, void *dev_instance)
 		rt2x00pci_rxdone(rt2x00dev);
 
 	/*
-	 * 4 - Tx done interrupt.
-	 */
-	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS))
-		rt2800_txdone(rt2x00dev);
-
-	/*
-	 * 5 - Auto wakeup interrupt.
+	 * 4 - Auto wakeup interrupt.
 	 */
 	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AUTO_WAKEUP))
 		rt2800pci_wakeup(rt2x00dev);
@@ -705,7 +807,9 @@ static irqreturn_t rt2800pci_interrupt_thread(int irq, void *dev_instance)
 static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
 {
 	struct rt2x00_dev *rt2x00dev = dev_instance;
-	u32 reg;
+	u32 reg, status;
+	int i;
+	irqreturn_t ret = IRQ_HANDLED;
 
 	/* Read status and ACK all interrupts */
 	rt2800_register_read(rt2x00dev, INT_SOURCE_CSR, &reg);
@@ -717,15 +821,78 @@ static irqreturn_t rt2800pci_interrupt(int irq, void *dev_instance)
 	if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
 		return IRQ_HANDLED;
 
-	/* Store irqvalue for use in the interrupt thread. */
-	rt2x00dev->irqvalue[0] = reg;
+	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_TX_FIFO_STATUS)) {
+		/*
+		 * 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.
+		 *
+		 * Hence, read the TX_STA_FIFO register and copy all tx status
+		 * reports into a kernel FIFO which is handled in the txstatus
+		 * tasklet. We use a tasklet to process the tx status reports
+		 * 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 interrupt thread gets executed.
+		 *
+		 * Since we have only one producer and one consumer we don't
+		 * need to lock the kfifo.
+		 */
+		for (i = 0; i < TX_ENTRIES; i++) {
+			rt2800_register_read(rt2x00dev, TX_STA_FIFO, &status);
+
+			if (!rt2x00_get_field32(status, TX_STA_FIFO_VALID))
+				break;
+
+			if (kfifo_is_full(&rt2x00dev->txstatus_fifo)) {
+				WARNING(rt2x00dev, "TX status FIFO overrun,"
+					" drop tx status report.\n");
+				break;
+			}
+
+			if (kfifo_in(&rt2x00dev->txstatus_fifo, &status,
+				     sizeof(status)) != sizeof(status)) {
+				WARNING(rt2x00dev, "TX status FIFO overrun,"
+					"drop tx status report.\n");
+				break;
+			}
+		}
+
+		/* Schedule the tasklet for processing the tx status. */
+		tasklet_schedule(&rt2x00dev->txstatus_tasklet);
+	}
 
-	/* Disable interrupts, will be enabled again in the interrupt thread. */
-	rt2x00dev->ops->lib->set_device_state(rt2x00dev,
-					      STATE_RADIO_IRQ_OFF_ISR);
+	if (rt2x00_get_field32(reg, INT_SOURCE_CSR_PRE_TBTT) ||
+	    rt2x00_get_field32(reg, INT_SOURCE_CSR_TBTT) ||
+	    rt2x00_get_field32(reg, INT_SOURCE_CSR_RX_DONE) ||
+	    rt2x00_get_field32(reg, INT_SOURCE_CSR_AUTO_WAKEUP)) {
+		/*
+		 * All other interrupts are handled in the interrupt thread.
+		 * Store irqvalue for use in the interrupt thread.
+		 */
+		rt2x00dev->irqvalue[0] = reg;
 
+		/*
+		 * Disable interrupts, will be enabled again in the
+		 * interrupt thread.
+		*/
+		rt2x00dev->ops->lib->set_device_state(rt2x00dev,
+						      STATE_RADIO_IRQ_OFF_ISR);
 
-	return IRQ_WAKE_THREAD;
+		/*
+		 * Leave the TX_FIFO_STATUS interrupt enabled to not lose any
+		 * tx status reports.
+		 */
+		rt2800_register_read(rt2x00dev, INT_MASK_CSR, &reg);
+		rt2x00_set_field32(&reg, INT_MASK_CSR_TX_FIFO_STATUS, 1);
+		rt2800_register_write(rt2x00dev, INT_MASK_CSR, reg);
+
+		ret = IRQ_WAKE_THREAD;
+	}
+
+	return ret;
 }
 
 /*
@@ -797,6 +964,20 @@ static int rt2800pci_probe_hw(struct rt2x00_dev *rt2x00dev)
 	 */
 	rt2x00dev->rssi_offset = DEFAULT_RSSI_OFFSET;
 
+	/*
+	 * Allocate txstatus fifo and tasklet, we use a size of 512 for the
+	 * kfifo which is big enough to store 512/4=128 tx status reports.
+	 * In the worst case (tx status for all tx queues gets reported before
+	 * we've got a chance to handle them) 24*4=384 tx status reports need
+	 * to be cached.
+	 */
+	retval = kfifo_alloc(&rt2x00dev->txstatus_fifo, 512, GFP_KERNEL);
+	if (retval)
+		return retval;
+
+	tasklet_init(&rt2x00dev->txstatus_tasklet, rt2800pci_txstatus_tasklet,
+		     (unsigned long)rt2x00dev);
+
 	return 0;
 }
 
diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
index 7832a59..9cc20b0 100644
--- a/drivers/net/wireless/rt2x00/rt2x00.h
+++ b/drivers/net/wireless/rt2x00/rt2x00.h
@@ -36,6 +36,7 @@
 #include <linux/mutex.h>
 #include <linux/etherdevice.h>
 #include <linux/input-polldev.h>
+#include <linux/kfifo.h>
 
 #include <net/mac80211.h>
 
@@ -884,6 +885,16 @@ struct rt2x00_dev {
 	 * and interrupt thread routine.
 	 */
 	u32 irqvalue[2];
+
+	/*
+	 * FIFO for storing tx status reports between isr and tasklet.
+	 */
+	struct kfifo txstatus_fifo;
+
+	/*
+	 * Tasklet for processing tx status reports (rt2800pci).
+	 */
+	struct tasklet_struct txstatus_tasklet;
 };
 
 /*
diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
index 053fdd3..1847653 100644
--- a/drivers/net/wireless/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
@@ -1028,6 +1028,16 @@ void rt2x00lib_remove_dev(struct rt2x00_dev *rt2x00dev)
 	cancel_work_sync(&rt2x00dev->txdone_work);
 
 	/*
+	 * Free the tx status fifo.
+	 */
+	kfifo_free(&rt2x00dev->txstatus_fifo);
+
+	/*
+	 * Kill the tx status tasklet.
+	 */
+	tasklet_kill(&rt2x00dev->txstatus_tasklet);
+
+	/*
 	 * Uninitialize device.
 	 */
 	rt2x00lib_uninitialize(rt2x00dev);
-- 
1.7.1




More information about the users mailing list