[rt2x00-users] [RFC 5/6] rt2x00: Cleanup USB RX index counting

[Helmut Schaa]

Am Dienstag 23 November 2010 schrieb Ivo Van Doorn:
> >> >> ---
> >> >>  drivers/net/wireless/rt2x00/rt2x00.h      |    1 +
> >> >>  drivers/net/wireless/rt2x00/rt2x00dev.c   |   11 ++++++++---
> >> >>  drivers/net/wireless/rt2x00/rt2x00queue.c |    5 +----
> >> >>  drivers/net/wireless/rt2x00/rt2x00usb.c   |    2 ++
> >> >>  4 files changed, 12 insertions(+), 7 deletions(-)
> >> >>
> >> >> diff --git a/drivers/net/wireless/rt2x00/rt2x00.h b/drivers/net/wireless/rt2x00/rt2x00.h
> >> >> index 520e736..556b744 100644
> >> >> --- a/drivers/net/wireless/rt2x00/rt2x00.h
> >> >> +++ b/drivers/net/wireless/rt2x00/rt2x00.h
> >> >> @@ -1170,6 +1170,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_dmastart(struct queue_entry *entry);
> >> >>  void rt2x00lib_dmadone(struct queue_entry *entry);
> >> >>  void rt2x00lib_txdone(struct queue_entry *entry,
> >> >>                     struct txdone_entry_desc *txdesc);
> >> >> diff --git a/drivers/net/wireless/rt2x00/rt2x00dev.c b/drivers/net/wireless/rt2x00/rt2x00dev.c
> >> >> index 01c8415..f3ae949 100644
> >> >> --- a/drivers/net/wireless/rt2x00/rt2x00dev.c
> >> >> +++ b/drivers/net/wireless/rt2x00/rt2x00dev.c
> >> >> @@ -226,6 +226,13 @@ void rt2x00lib_pretbtt(struct rt2x00_dev *rt2x00dev)
> >> >>  }
> >> >>  EXPORT_SYMBOL_GPL(rt2x00lib_pretbtt);
> >> >>
> >> >> +void rt2x00lib_dmastart(struct queue_entry *entry)
> >> >> +{
> >> >> +     set_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
> >> >> +     rt2x00queue_index_inc(entry->queue, Q_INDEX);
> >> >> +}
> >> >> +EXPORT_SYMBOL_GPL(rt2x00lib_dmastart);
> >> >> +
> >> >>  void rt2x00lib_dmadone(struct queue_entry *entry)
> >> >>  {
> >> >>       clear_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags);
> >> >> @@ -552,10 +559,8 @@ submit_entry:
> >> >>       entry->flags = 0;
> >> >>       rt2x00queue_index_inc(entry->queue, Q_INDEX_DONE);
> >> >>       if (test_bit(DEVICE_STATE_PRESENT, &rt2x00dev->flags) &&
> >> >> -         test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags)) {
> >> >> +         test_bit(DEVICE_STATE_ENABLED_RADIO, &rt2x00dev->flags))
> >> >>               rt2x00dev->ops->lib->clear_entry(entry);
> >> >> -             rt2x00queue_index_inc(entry->queue, Q_INDEX);
> >> >> -     }
> >> >>  }
> >> >
> >> > Seems like this might be the culprit? Why do you remove this index increment?
> >> > rt2x00lib_dmastart is only used by rt2x00usb right now.
> >>
> >> Because having the rt2x00queue_index_inc is causing problems in rt2x00usb.
> >> clear_entry will transfer the entry to the device again, but this
> >> could fail. It also
> >> was a problem for the "rt2x00queue_init_queues" where rt2x00lib was
> >> simply looping 128
> >> times to get the queue index on the right position while that wasn't
> >> necessarily the
> >> right position (depending on the rt2x00usb state). So it was much
> >> cleaner to introduce
> >> the rt2x00lib_dmastart() function.
> >> This actually did solve part of the race conditions and queue
> >> corruption inside rt2x00usb
> >> which caused complete breakage.
> >>
> >> I have a bit of a problem with testing the PCI drivers, but any help
> >> would be appreciated
> >> to update the PCI drivers with this rt2x00lib_dmastart() unction. :)
> >
> > Hmm, dmastart is only meant for RX, right?
> 
> Yeah, because RX is the only case where the index handling is
> a bit tricky.
> 
> > Will look into this soon.

This will fix RX for PCI devices and you patch applied. Please incorporate
these changes into your patch.

Since we get the rx interrupt after DMA is done already (AFAIK) we can simply
add dmastart dmadone fake calls to make it work again.

Maybe we should make dmastart and dmadone inline functions and move them
to rt2x00queue.h?

Thanks,
Helmut

---
Signed-off-by: Helmut Schaa <helmut.schaa at googlemail.com>
---
 drivers/net/wireless/rt2x00/rt2x00pci.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c b/drivers/net/wireless/rt2x00/rt2x00pci.c
index 868ca19..28e6ff1 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -82,6 +82,13 @@ void rt2x00pci_rxdone(struct rt2x00_dev *rt2x00dev)
 		skbdesc->desc_len = entry->queue->desc_size;
 
 		/*
+		 * DMA is already done, notify rt2x00lib that
+		 * it finished successfully.
+		 */
+		rt2x00lib_dmastart(entry);
+		rt2x00lib_dmadone(entry);
+
+		/*
 		 * Send the frame to rt2x00lib for further processing.
 		 */
 		rt2x00lib_rxdone(entry);