[rt2x00-users] [PATCH 4/4] rt2x00: Implement TX status reporting for rt2800usb

[Helmut Schaa]

Am Samstag 31 Juli 2010 schrieb Ivo Van Doorn:
> On Sat, Jul 31, 2010 at 1:04 PM, Helmut Schaa
> <helmut.schaa at googlemail.com> wrote:
> > Hi Ivo,
> >
> > Am Samstag 31 Juli 2010 schrieb Ivo van Doorn:
> >> The TX_STA_FIFO register which is used for per-frame TX frame
> >> status reporting is also valid on rt2800usb. We can move the
> >> rt2800pci_txdone function into rt2800lib where it can also
> >> be used by rt2800usb.
> >>
> >> rt2800usb needs to overwrite the txdone work handler to
> >> a different function.
> >>
> >> Both rt2800usb as rt2800_txdone need to take into account
> >> that IO failures can occur while uploading the URB, which
> >> means that when obtaining the new entry the IO status must
> >> be checked.
> >>
> >> Signed-off-by: Ivo van Doorn <IvDoorn at gmail.com>
> >
> > [...]
> >
> >> +static void rt2800usb_work_txdone(struct work_struct *work)
> >> +{
> >> +     struct rt2x00_dev *rt2x00dev =
> >> +         container_of(work, struct rt2x00_dev, txdone_work);
> >> +     struct data_queue *queue;
> >> +     struct queue_entry *entry;
> >> +
> >> +     rt2800_txdone(rt2x00dev);
> >> +
> >> +     tx_queue_for_each(rt2x00dev, queue) {
> >> +             while (!rt2x00queue_empty(queue)) {
> >> +                     entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> >> +
> >> +                     if (test_bit(ENTRY_OWNER_DEVICE_DATA, &entry->flags) ||
> >> +                         !test_bit(ENTRY_DATA_IO_FAILED, &entry->flags))
> >> +                             break;
> >> +
> >> +                     rt2x00lib_txdone_noinfo(entry, TXDONE_FAILURE);
> >> +             }
> >> +     }
> >> +}
> >
> > Wouldn't that mess up the tx status reporting in some cases? Assume the
> > following situation:
> >
> > Frames 1-3 are submitted to the hw, frames 1 and 2 are sent out immediately
> > but frame 3 has to wait due to a high contention on the air for example. So,
> > when rt2800usb_work_txdone is run the TX_STA_FIFO contains two entries for
> > frames 1 and 2 but none for frame 3. The tx status for 1 and 2 is reported
> > correctly to mac80211 whereas frame 3 is reported with a default status (which
> > is ok). However. once frame 3 is really sent out the hw will push its real
> > tx status into TX_STA_FIFO. Sending frame 4 now will match the tx status of
> > frame 3 to frame 4 whereas the tx status of frame 4 will be kept in the FIFO
> > till the next frame is sent, resulting in an incorrect tx status <-> frame
> > association.
> 
> This situation wouldn't occur. If 3 frames are submitted to the hardware,
> each of them reports back if the upload succeeded. If that is the case
> the flag ENTRY_OWNER_DEVICE_DATA is cleared. If the URB failed,
> then ENTRY_DATA_IO_FAILED is set, otherwise the driver will wait for
> the TX_STA_FIFO report.
> 
> Lets assume your scenario, 3 frames were uploaded succesfully,
> but TX_STA_FIFO contains 2 entries. Then only the status of 2 entries
> are reported to mac80211. After the call to rt2800_txdone() rt2800usb,
> needs to check if any URB has come back with a failed report (which
> means that ENTRY_DATA_IO_FAILED is set), if that is not the case
> (in your example frame 3), then nothing will happen until the next
> frame is sucessfully uploaded and the rt2800usb_work_txdone
> handler runs again.
> 
> The handler works in such a way that when it ends it has moved the
> INDEX_DONE pointer to the last entry which was has either not
> completed the upload yet, or to the entry for which no status was
> reported yet.

Ah, yeah, you're right, I didn't look close enough. Thanks for clarification.

> I didn't notice better performance, as with the old situation rt2800usb always
> reports success. Thus the rate control never switched down when the performance
> decreased. I can now walk away from my AP, and the rate is lowered accordingly.

Ok, but that's also a good thing ;)

Helmut