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

Helmut Schaa helmut.schaa at googlemail.com
Sat Jul 31 21:04:03 AEST 2010


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.

As this still might improve rt2800usb performance it's still far from
a reliable tx status reporting. At least on rt2800pci when the tx status
reports don't match the real frames the rate control algortihms often end up
using a lower rate then possible.

Don't get me wrong, I don't have any objections against this patch series
to get merged (as I guess you did some tests and noticed better performance
with rt2800usb, right ;) ? ) but wanted to point out a few things I've noticed
while working on the tx status reporting in rt2800pci.

But at least this makes rt2800usb and rt2800pci to suffer from the same issue
and we might be able to fix it for both drivers in one shot ;)

I didn't review the whole series yet but I might find some time to do so soon,
Helmut




More information about the users mailing list