[rt2x00-users] [PATCH 01/12] rt2x00: Fix rt2800pci use of WCID & PID field in TXWI descriptors

[Ivo van Doorn]

On Saturday 29 August 2009, Benoit PAPILLAULT wrote:
> We now WCID as before (ie with the crypto key index). The process done
> in rt2800pci_txdone() has been changed to only process the last
> descriptor of the specified queue. We use WCID/ACK/PID fields for
> checking the TX_STA_FIFO content (it's not perfect though).
> 
> In fact, if PID is 0, TX_STA_FIFO interrupt never happen so we need to
> avoid this value (by adding 1).
> 
> Signed-off-by: Benoit PAPILLAULT <benoit.papillault at free.fr>
> ---
>  drivers/net/wireless/rt2x00/rt2800pci.c |   97 +++++++++++++++----------------
>  1 files changed, 47 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/net/wireless/rt2x00/rt2800pci.c b/drivers/net/wireless/rt2x00/rt2800pci.c
> index 5780c36..5c9272f 100644
> --- a/drivers/net/wireless/rt2x00/rt2800pci.c
> +++ b/drivers/net/wireless/rt2x00/rt2800pci.c
> @@ -2131,7 +2131,7 @@ static void rt2800pci_write_tx_desc(struct rt2x00_dev *rt2x00dev,
>  	/*
>  	 * Initialize TX Info descriptor
>  	 */
> -	rt2x00_desc_read(txwi, 0, &word);
> +	word = 0;

Please remove this change (and similar changes below).

>  	/*
>  	 * Initialize TX descriptor
>  	 */
> -	rt2x00_desc_read(txd, 0, &word);
> +
> +	/* 
> +	 * The buffers pointed by (SD_PTR0, SD_LEN0) and (SD_PTR1, SD_LEN1)
> +	 * must contains a TXWI structure + 802.11 header + padding + 802.11
> +	 * data. We choose to have (SD_PTR0, SD_LEN0) only contains TXWI and
> +	 * (SD_PTR1, SD_LEN1) contains 802.11 header + padding + 802.11
> +	 * data. It means that LAST_SEC0 is always 0.
> +	 */

This comment is already present in rt2x00.git.... Are you sure you updated the
rt2x00.git repository correctly?

Below patch doesn't even apply to rt2x00.git because it was changed with your
previous patchset. Also I hade comments/questions on these changes during your
last patch series.

> @@ -2410,75 +2419,63 @@ static void rt2800pci_txdone(struct rt2x00_dev *rt2x00dev)
>  {
>  	struct data_queue *queue;
>  	struct queue_entry *entry;
> -	struct queue_entry *entry_done;
> -	struct queue_entry_priv_pci *entry_priv;
> +	__le32 *txwi;
>  	struct txdone_entry_desc txdesc;
>  	u32 word;
>  	u32 reg;
> -	u32 old_reg;
> -	unsigned int type;
> -	unsigned int index;
>  	u16 mcs, real_mcs;
> +	int i;
> +	int wcid, ack, pid, tx_wcid, tx_ack, tx_pid;
>  
>  	/*
> -	 * During each loop we will compare the freshly read
> -	 * TX_STA_FIFO register value with the value read from
> -	 * the previous loop. If the 2 values are equal then
> -	 * we should stop processing because the chance it
> -	 * quite big that the device has been unplugged and
> -	 * we risk going into an endless loop.
> +	 * To avoid an endlees loop, we only read the TX_STA_FIFO register up
> +	 * to 256 times (this is enought to get all values from the FIFO). In
> +	 * normal situation, the loop is terminated when we reach a value with
> +	 * TX_STA_FIFO_VALID bit is 0.
>  	 */
> -	old_reg = 0;
>  
> -	while (1) {
> +	for (i=0; i<256; i++) {
>  		rt2x00pci_register_read(rt2x00dev, TX_STA_FIFO, &reg);
> +
>  		if (!rt2x00_get_field32(reg, TX_STA_FIFO_VALID))
>  			break;
>  
> -		if (old_reg == reg)
> -			break;
> -		old_reg = reg;
> +		wcid	= rt2x00_get_field32(reg, TX_STA_FIFO_WCID);
> +		ack	= rt2x00_get_field32(reg, TX_STA_FIFO_TX_ACK_REQUIRED);
> +		pid	= rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE);
>  
>  		/*
>  		 * Skip this entry when it contains an invalid
>  		 * queue identication number.
>  		 */
> -		type = rt2x00_get_field32(reg, TX_STA_FIFO_PID_TYPE) - 1;
> -		if (type >= QID_RX)
> +		if (pid < 1)
>  			continue;
>  
> -		queue = rt2x00queue_get_queue(rt2x00dev, type);
> +		queue = rt2x00queue_get_queue(rt2x00dev, pid - 1);
>  		if (unlikely(!queue))
>  			continue;
>  
>  		/*
> -		 * Skip this entry when it contains an invalid
> -		 * index number.
> +		 * Inside each queue, we process each entry in a chronological
> +		 * order. We first check that the queue is not empty.
>  		 */
> -		index = rt2x00_get_field32(reg, TX_STA_FIFO_WCID) - 1;
> -		if (unlikely(index >= queue->limit))
> +		if (queue->length == 0) {
>  			continue;
> +		}
> +		entry = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> +
> +		/* Check if we got a match by looking at WCID/ACK/PID
> +		 * fields */
> +		txwi = (__le32 *)(entry->skb->data - 
> +				  rt2x00dev->hw->extra_tx_headroom);
> +
> +		rt2x00_desc_read(txwi, 1, &word);
> +		tx_wcid	= rt2x00_get_field32(word, TXWI_W1_WIRELESS_CLI_ID);
> +		tx_ack	= rt2x00_get_field32(word, TXWI_W1_ACK);
> +		tx_pid	= rt2x00_get_field32(word, TXWI_W1_PACKETID);
>  
> -		entry = &queue->entries[index];
> -		entry_priv = entry->priv_data;
> -		rt2x00_desc_read((__le32 *)entry->skb->data, 0, &word);
> -
> -		entry_done = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> -		while (entry != entry_done) {
> -			/*
> -			 * Catch up.
> -			 * Just report any entries we missed as failed.
> -			 */
> -			WARNING(rt2x00dev,
> -				"TX status report missed for entry %d\n",
> -				entry_done->entry_idx);
> -
> -			txdesc.flags = 0;
> -			__set_bit(TXDONE_UNKNOWN, &txdesc.flags);
> -			txdesc.retry = 0;
> -
> -			rt2x00lib_txdone(entry_done, &txdesc);
> -			entry_done = rt2x00queue_get_entry(queue, Q_INDEX_DONE);
> +		if ((wcid != tx_wcid) || (ack != tx_ack) || (pid != tx_pid)) {
> +			WARNING(rt2x00dev, "invalid TX_STA_FIFO content\n");
>  		}
>  
>  		/*