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

[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.

Thanks.

Ivo