[rt2x00-users] [RFC] rt2x00: Implement tx queue watchdog for rt2800pci
Helmut Schaa
helmut.schaa at googlemail.com
Tue Aug 10 14:32:59 UTC 2010
Am Dienstag 10 August 2010 schrieb Ivo Van Doorn:
> Hi,
>
> On Tue, Aug 10, 2010 at 3:54 PM, Helmut Schaa
> <helmut.schaa at googlemail.com> wrote:
> > On rt2800pci it can happen that a tx status report gets lost which would
> > mean that a tx queue entry remains unfreed. However, the device also
> > also generates DMA done interrupts when it finishes a DMA transfer.
> > Hence, introduce a new tx queue index called Q_INDEX_DMA_DONE which
> > always points to the next tx queue entry that wasn't transferred to the
> > hardware yet.
> >
> > The rt2800pci_watchdog function checks if a frame was queued for
> > transmission but the according tx status wasn't reported within 100ms.
> > If that condition appears all remaining tx entries till the
> > Q_INDEX_DMA_DONE index will get freed.
> >
> > Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
> > ---
>
> Just a quick glance, I will review more in depth later today.
Ok, great. Thanks Ivo.
> > drivers/net/wireless/rt2x00/rt2800pci.c | 92 +++++++++++++++++++++++++++++
> > drivers/net/wireless/rt2x00/rt2x00.h | 1 +
> > drivers/net/wireless/rt2x00/rt2x00debug.c | 6 +-
> > drivers/net/wireless/rt2x00/rt2x00dev.c | 6 ++
> > drivers/net/wireless/rt2x00/rt2x00queue.h | 1 +
> > 5 files changed, 104 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> > index 4390f2b..fbab54e 100644
> > --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> > +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> > @@ -665,6 +665,59 @@ static void rt2800pci_kill_tx_queue(struct rt2x00_dev *rt2x00dev,
> > rt2800_register_write(rt2x00dev, WPDMA_RST_IDX, reg);
> > }
> >
> > +static void rt2800pci_watchdog_reset_tx(struct data_queue *queue)
> > +{
> > + unsigned short threshold = queue->threshold;
> > + struct queue_entry *entry;
> > + struct txdone_entry_desc txdesc;
> > +
> > + WARNING(queue->rt2x00dev, "TX queue %d timed out, invoke reset",
> > + queue->qid);
> > +
> > + /*
> > + * Temporarily disable the TX queue, this will force mac80211
> > + * to use the other queues until this queue has been restored.
> > + *
> > + * Set the queue threshold to the queue limit. This prevents the
> > + * queue from being enabled during the txdone handler.
> > + */
> > + queue->threshold = queue->limit;
> > + ieee80211_stop_queue(queue->rt2x00dev->hw, queue->qid);
> > +
> > + /*
> > + * Reset all TX frames that were successfully uploaded to the hw
> > + * but didn't get a tx status.
> > + */
> > + while (queue->index[Q_INDEX_DONE] != queue->index[Q_INDEX_DMA_DONE]) {
>
> Can't we wrap this in an inline function in rt2x00queue.h
> I prefer to keep the index variable internal to rt2x00queue.
> Perhaps something like empty_dma or whatever.
Yeah, I'm going to change that as soon as I find a nice name for it ;)
> > + entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> > +
> > + txdesc.flags = 0;
> > + __set_bit(TXDONE_UNKNOWN, &txdesc.flags);
> > + txdesc.retry = 0;
> > +
> > + rt2x00lib_txdone(entry, &txdesc);
> > + }
> > +
> > + /*
> > + * The queue has been reset, and mac80211 is allowed to use the
> > + * queue again.
> > + */
> > + queue->threshold = threshold;
> > + ieee80211_wake_queue(queue->rt2x00dev->hw, queue->qid);
> > +}
> > +
> > +static void rt2800pci_watchdog(struct rt2x00_dev *rt2x00dev)
> > +{
> > + struct data_queue *queue;
> > +
> > + tx_queue_for_each(rt2x00dev, queue) {
> > + if (time_after(jiffies, queue->last_index + (HZ / 10)) &&
>
> Perhaps something like rt2x00queue_timeout(), do we really need
> jiffies here instead of another time_t variable in the queue (last_index_done)?
There was something with rt2x00queue_timeout. Not sure, I'll recheck ...
> > + queue->index[Q_INDEX_DONE] !=
> > + queue->index[Q_INDEX_DMA_DONE])
>
> Same function as above in your while loop.
ACK
> > + rt2800pci_watchdog_reset_tx(queue);
> > + }
> > +}
> > +
> > /*
> > * RX control handlers
> > */
> > @@ -724,6 +777,24 @@ static void rt2800pci_fill_rxdone(struct queue_entry *entry,
> > /*
> > * Interrupt functions.
> > */
> > +static void rt2800pci_dmadone(struct rt2x00_dev *rt2x00dev,
> > + enum data_queue_qid qid)
> > +{
> > + struct data_queue *queue = rt2x00queue_get_queue(rt2x00dev, qid);
> > + struct queue_entry *entry;
> > + u32 reg;
> > +
> > + /*
> > + * Move the DMA_DONE index until the first not-yet-transferred frame
> > + */
> > + rt2800_register_read(rt2x00dev, TX_DTX_IDX(qid), ®);
> > +
> > + while (queue->index[Q_INDEX_DMA_DONE] != reg) {
> > + entry = rt2x00queue_get_entry(queue, Q_INDEX_DMA_DONE);
> > + rt2x00lib_dmadone(entry);
> > + }
> > +}
> > +
> > static void rt2800pci_wakeup(struct rt2x00_dev *rt2x00dev)
> > {
> > struct ieee80211_conf conf = { .flags = 0 };
> > @@ -762,6 +833,21 @@ static irqreturn_t rt2800pci_interrupt_thread(int irq, void *dev_instance)
> > rt2800_txdone(rt2x00dev);
> >
> > /*
> > + * 4 - DMA done interrupt.
> > + */
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AC0_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_AC_BE);
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AC1_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_AC_BK);
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AC2_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_AC_VI);
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AC3_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_AC_VO);
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_HCCA_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_HCCA);
> > + if (rt2x00_get_field32(reg, INT_SOURCE_CSR_MGMT_DMA_DONE))
> > + rt2800pci_dmadone(rt2x00dev, QID_MGMT);
> > + /*
> > * 5 - Auto wakeup interrupt.
> > */
> > if (rt2x00_get_field32(reg, INT_SOURCE_CSR_AUTO_WAKEUP))
> > @@ -854,6 +940,11 @@ static int rt2800pci_probe_hw(struct rt2x00_dev *rt2x00dev)
> > __set_bit(DRIVER_SUPPORT_PRE_TBTT_INTERRUPT, &rt2x00dev->flags);
> >
> > /*
> > + * rt2800 supports a tx queue watchdog
> > + */
> > + __set_bit(DRIVER_SUPPORT_WATCHDOG, &rt2x00dev->flags);
> > +
> > + /*
> > * This device requires firmware.
> > */
> > if (!rt2x00_is_soc(rt2x00dev))
> > @@ -922,6 +1013,7 @@ static const struct rt2x00lib_ops rt2800pci_rt2x00_ops = {
> > .link_stats = rt2800_link_stats,
> > .reset_tuner = rt2800_reset_tuner,
> > .link_tuner = rt2800_link_tuner,
> > + .watchdog = rt2800pci_watchdog,
> > .write_tx_desc = rt2800pci_write_tx_desc,
> > .write_tx_data = rt2800_write_tx_data,
> > .write_beacon = rt2800_write_beacon,
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> > index 8c65244..9839c10 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00.h
> > +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> > @@ -1072,6 +1072,7 @@ static inline void rt2x00debug_dump_frame(struct rt2x00_dev *rt2x00dev,
> > */
> > void rt2x00lib_beacondone(struct rt2x00_dev *rt2x00dev);
> > void rt2x00lib_pretbtt(struct rt2x00_dev *rt2x00dev);
> > +void rt2x00lib_dmadone(struct queue_entry *entry);
> > void rt2x00lib_txdone(struct queue_entry *entry,
> > struct txdone_entry_desc *txdesc);
> > void rt2x00lib_txdone_noinfo(struct queue_entry *entry, u32 status);
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00debug.c b/drivers/net/wireless/rt2x00/rt2x00debug.c
> > index b0498e7..eb82aa6 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00debug.c
> > +++ b/drivers/net/wireless/rt2x00/rt2x00debug.c
> > @@ -338,14 +338,16 @@ static ssize_t rt2x00debug_read_queue_stats(struct file *file,
> > return -ENOMEM;
> >
> > temp = data +
> > - sprintf(data, "qid\tcount\tlimit\tlength\tindex\tdone\tcrypto\n");
> > + sprintf(data, "qid\tcount\tlimit\tlength\tindex\tdmadone\tdone\tcrypto\n");
> >
> > queue_for_each(intf->rt2x00dev, queue) {
> > spin_lock_irqsave(&queue->lock, irqflags);
> >
> > - temp += sprintf(temp, "%d\t%d\t%d\t%d\t%d\t%d\t%d\n", queue->qid,
> > + temp += sprintf(temp, "%d\t%d\t%d\t%d\t%d\t%d\t%d\t%d\n",
> > + queue->qid,
> > queue->count, queue->limit, queue->length,
> > queue->index[Q_INDEX],
> > + queue->index[Q_INDEX_DMA_DONE],
> > queue->index[Q_INDEX_DONE],
> > queue->index[Q_INDEX_CRYPTO]);
> >
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> > index e692608..4e42d12 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> > +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> > @@ -251,6 +251,12 @@ void rt2x00lib_pretbtt(struct rt2x00_dev *rt2x00dev)
> > }
> > EXPORT_SYMBOL_GPL(rt2x00lib_pretbtt);
> >
> > +void rt2x00lib_dmadone(struct queue_entry *entry)
> > +{
> > + rt2x00queue_index_inc(entry->queue, Q_INDEX_DMA_DONE);
> > +}
> > +EXPORT_SYMBOL_GPL(rt2x00lib_dmadone);
> > +
> > void rt2x00lib_txdone(struct queue_entry *entry,
> > struct txdone_entry_desc *txdesc)
> > {
> > diff --git a/drivers/net/wireless/rt2x00/rt2x00queue.h b/drivers/net/wireless/rt2x00/rt2x00queue.h
> > index 2d3bf84..eef961f 100644
> > --- a/drivers/net/wireless/rt2x00/rt2x00queue.h
> > +++ b/drivers/net/wireless/rt2x00/rt2x00queue.h
> > @@ -415,6 +415,7 @@ struct queue_entry {
> > */
> > enum queue_index {
> > Q_INDEX,
> > + Q_INDEX_DMA_DONE,
> > Q_INDEX_DONE,
> > Q_INDEX_CRYPTO,
> > Q_INDEX_MAX,
> > --
> > 1.7.1
> >
> >
>
More information about the users
mailing list