From cd35a3912188d006f251fff1028d84e1d1649ca2 Mon Sep 17 00:00:00 2001 From: Ivo van Doorn Date: Mon, 30 Aug 2010 21:15:51 +0200 Subject: [PATCH] rt2x00: Cleanup rt2x00usb_watchdog_reset_tx rt2x00usb_watchdog_reset_tx performs the same task as rt2x00usb_kill_tx_queue, with the only difference is that it waits for all entries to be returned to the driver and for all frames the status has been reported to mac80211. We can easily split this task by calling rt2x00usb_kill_tx_queue, sleep for a short period and invoke the TX status reporting function. By adding the sleep() to the kill_entry we make sure that even during shutdown we guarentee the entry has been killed when the function returns. To make this work correctly the interrupt handlers have to be updated to prevent checking for the RADIO_ENABLED flag too early which prevents the ownership of the entry to be reset. Additionally a check for the DEVICE_PRESENT flag is not required but is nice to prevent race conditions when the device was unplugged. Additionally rather then calling rt2x00usb_work_txdone() for status reporting we let the driver perform the TX status reporting first. If this is not sufficient then rt2x00usb_work_txdone() will still be used to cleanup the mess. Signed-off-by: Ivo van Doorn Acked-by: Gertjan van Wingerde Signed-off-by: John W. Linville --- drivers/net/wireless/rt2x00/rt2x00usb.c | 66 +++++++++++++++---------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c index aec6440d364a..4c5ae3d45625 100644 --- a/drivers/net/wireless/rt2x00/rt2x00usb.c +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c @@ -208,8 +208,7 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb) struct queue_entry *entry = (struct queue_entry *)urb->context; struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev; - if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags) || - !__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)) + if (!__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)) return; /* @@ -227,7 +226,9 @@ static void rt2x00usb_interrupt_txdone(struct urb *urb) * Schedule the delayed work for reading the TX status * from the device. */ - ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->txdone_work); + if (test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags) && + test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags)) + ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->txdone_work); } static void rt2x00usb_kick_tx_entry(struct queue_entry *entry) @@ -279,6 +280,14 @@ static void rt2x00usb_kill_tx_entry(struct queue_entry *entry) if ((entry->queue->qid == QID_BEACON) && (test_bit(DRIVER_REQUIRE_BEACON_GUARD, &rt2x00dev->flags))) usb_kill_urb(bcn_priv->guardian_urb); + + /* + * We need a short delay here to wait for + * the URB to be canceled + */ + do { + udelay(100); + } while (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)); } void rt2x00usb_kill_tx_queue(struct data_queue *queue) @@ -290,8 +299,7 @@ EXPORT_SYMBOL_GPL(rt2x00usb_kill_tx_queue); static void rt2x00usb_watchdog_tx_dma(struct data_queue *queue) { - struct queue_entry *entry; - struct queue_entry_priv_usb *entry_priv; + struct rt2x00_dev *rt2x00dev = queue->rt2x00dev; unsigned short threshold = queue->threshold; WARNING(queue->rt2x00dev, "TX queue %d DMA timed out," @@ -305,28 +313,33 @@ static void rt2x00usb_watchdog_tx_dma(struct data_queue *queue) * queue from being enabled during the txdone handler. */ queue->threshold = queue->limit; - ieee80211_stop_queue(queue->rt2x00dev->hw, queue->qid); + ieee80211_stop_queue(rt2x00dev->hw, queue->qid); /* - * Reset all currently uploaded TX frames. + * Kill all entries in the queue, afterwards we need to + * wait a bit for all URBs to be cancelled. */ - while (!rt2x00queue_empty(queue)) { - entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE); - entry_priv = entry->priv_data; - usb_kill_urb(entry_priv->urb); + rt2x00usb_kill_tx_queue(queue); - /* - * We need a short delay here to wait for - * the URB to be canceled - */ - do { - udelay(100); - } while (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)); + /* + * In case that a driver has overriden the txdone_work + * function, we invoke the TX done through there. + */ + rt2x00dev->txdone_work.func(&rt2x00dev->txdone_work); - /* - * Invoke the TX done handler - */ - rt2x00usb_work_txdone_entry(entry); + /* + * Security measure: if the driver did override the + * txdone_work function, and the hardware did arrive + * in a state which causes it to malfunction, it is + * possible that the driver couldn't handle the txdone + * event correctly. So after giving the driver the + * chance to cleanup, we now force a cleanup of any + * leftovers. + */ + if (!rt2x00queue_empty(queue)) { + WARNING(queue->rt2x00dev, "TX queue %d DMA timed out," + " status handling failed, invoke hard reset", queue->qid); + rt2x00usb_work_txdone(&rt2x00dev->txdone_work); } /* @@ -334,7 +347,7 @@ static void rt2x00usb_watchdog_tx_dma(struct data_queue *queue) * queue again. */ queue->threshold = threshold; - ieee80211_wake_queue(queue->rt2x00dev->hw, queue->qid); + ieee80211_wake_queue(rt2x00dev->hw, queue->qid); } static void rt2x00usb_watchdog_tx_status(struct data_queue *queue) @@ -394,8 +407,7 @@ static void rt2x00usb_interrupt_rxdone(struct urb *urb) struct queue_entry *entry = (struct queue_entry *)urb->context; struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev; - if (!test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags) || - !__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)) + if (!__test_and_clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags)) return; /* @@ -415,7 +427,9 @@ static void rt2x00usb_interrupt_rxdone(struct urb *urb) * Schedule the delayed work for reading the RX status * from the device. */ - ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->rxdone_work); + if (test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags) && + test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags)) + ieee80211_queue_work(rt2x00dev->hw, &rt2x00dev->rxdone_work); } /*