[rt2x00-users] [PATCH] rt2x00 : rt2800pci tx and rx take 3 .

[Alban Browaeys]

Ivo van Doorn a écrit :
> Hi,
>
> Apologies for my late reply, but now I have some more time to better
> look into your patch. I have added Felix to the CC because he was also
> working on the rt2800pci driver and had some pending patches as well.
> So hopefully he can take a look at this patch as well to see if he has some
> additional ideas. :)
>
>   
>> This one add another hack but get the thing working against hostap
>> (which has header_length 26 instead of 24 for freebox wpa).
>> This was tested against hostap in wpa personal and open.
>> And also against freebox wpa (based on linux but internals not disclosed).
>>     
>
> So what is the exact status after this patch?
>   
As told to Benoit in reply . Everything works with those setups above :
I was able to stress the rx/tx by cloning
wireless-testing (which I needed to port this rt2x00 to as this was the
only tree close enough to rt2x00 and with
 i915 kms resume fix.).

Still I have not tested wep and I found a setup where it did not worked
(livebox in france with a wpa mode that
seems to be wpa+something - maybe wpa2 -  ... don't know yet this is a
thomson livebox and I saw the
configuration page a second).

>> I would really appreciate a better solution or an explanation as to why ALIGN_SIZE
>> was what it was (that is check for the position in memory of the header end).
>> This change make it a diff on a roundup of header length around 4.
>> Otherwise I always get 0 for align size be the header_length 24 or 26.
>>     
>
> Well for header_length of 24 you _should_ get the align size of 0. The payload
> should be aligned to a 4 byte boundary.
>
>   

Yes for 24 ... though for 26 I also get 0 in all tests. I expected to
get 2 (with the hack  providing a way to test if
data with header length of 26 where failing due to this l2pad behing 0
or anything else).
I let it in the patch as it was a working workaround not a perfect
solution. As you tell later in the mail there
are other alignement macro I should look at before understanding how to
get both of worlds (2 as align if header
length 26 while at the same time


>> By the way I check again in rt2860 and bHwRadio is true when gpio is (& 4) == 1 ... and I tried reverting the hack on rt2800pci_rfkill
>> with no working setup resulting (mac80211 always stop the interface asap when using the opposite value). So I ll need confirmation and
>> more inputs from another tester using rt2800pci to find out what should be done to get the interface staying up while at the same time
>> sending rfkill block when gpio is 1.
>>     
>
> From the legacy driver:
>
> 			// Read GPIO pin2 as Hardware controlled radio state
> 			RTMP_IO_READ32(pAd, GPIO_CTRL_CFG, &data);
> 			if ((data & 0x04) == 0)
> 			{
> 				pAd->StaCfg.bHwRadio = FALSE;
> 				pAd->StaCfg.bRadio = FALSE;
> //				RTMP_IO_WRITE32(pAd, PWR_PIN_CFG, 0x00001818);
> 				RTMP_SET_FLAG(pAd, fRTMP_ADAPTER_RADIO_OFF);
> 			}
>
> So my assumption about the rfkill is completely based on this code (and this behavior matches the rfkill behavior
> of all previous Ralink chipsets).
>   

Exactly . I agree if GPIO is 0 radio is off and if GPIO is 1 as in mlme
of rt2860sta radio is true :
                        // Read GPIO pin2 as Hardware controlled radio state
                        RTMP_IO_FORCE_READ32(pAd, GPIO_CTRL_CFG, &data);
                        if (data & 0x04)
                        {
                                pAd->StaCfg.bHwRadio = TRUE;
                        }
                        else
                        {
                                pAd->StaCfg.bHwRadio = FALSE;
                        }


So my issue is ... I don't understand if rt2800pci_rfkill should return
1 as my understanding of
void rt2x00mac_rfkill_poll(struct ieee80211_hw *hw)
{
        struct rt2x00_dev *rt2x00dev = hw->priv;
        bool blocked = !!rt2x00dev->ops->lib->rfkill_poll(rt2x00dev);

        wiphy_rfkill_set_hw_state(hw->wiphy, blocked);
}
is that 1 means blocked ....
And on eeepc be it with rt2860sta (I added printk in it too) or
rt2800pci ... GPIO was always 1.
Though as benoit showed by  his test this is wrong.
So I am back at my computer trying to understand why rfkill
wiphy_rfkill_set_hw_state to 1 means stop for me
(as shown by printk added to mac80211 layer) and why this means the
opposite for Benoit.
I ll find out ... only a matter of spending the time.


>> NB: I found out that if I load rt2860sta and unload it then load rt2800pci the tx  are not working (gpio is 1). Thus I though I found this test case to find out about rfkill but it turned out using the platform driver (eeepc_laptop) rfkill switch off then on everything is back to normal (tx are transmitted). One may have to unload rt2800pci and reload it to free the queue which are full though to see tx working. 
>>
>>
>> I still have not tested again without adding 1 to entry_idx and queue->qid in write_tx_desc . I first planned to do it today but found this
>> align size issue and it took me a while to find at least a hack to get an idea on what was at stake. I hope a well done answer can be found
>> for this peculiar issue. Because mine looks harsh and looks like it could break other drivers (I have not checked which ones are using txdesc l2pad.
>>     
>
> Perhaps the include/linux/kernel.h defines for ALIGN are interesting?
>
> #define ALIGN(x,a)				__ALIGN_MASK(x,(typeof(x))(a)-1)
> #define __ALIGN_MASK(x,mask)	(((x)+(mask))&~(mask))
> #define PTR_ALIGN(p, a)		((typeof(p))ALIGN((unsigned long)(p), (a)))
> #define IS_ALIGNED(x, a)		(((x) & ((typeof(x))(a) - 1)) == 0)
>
>   
>> 		rt2x00pci_register_write(rt2x00dev, TX_CTX_IDX(qidx), index);
>>  }
>>  
>>  static void rt2800pci_kill_tx_queue(struct rt2x00_dev *rt2x00dev,
>> @@ -2289,8 +2316,8 @@ static void rt2800pci_fill_rxdone(struct queue_entry *entry,
>>  				  struct rxdone_entry_desc *rxdesc)
>>  {
>>  	struct rt2x00_dev *rt2x00dev = entry->queue->rt2x00dev;
>> -	struct queue_entry_priv_pci *entry_priv = entry->priv_data;
>> -	__le32 *rxd = entry_priv->desc;
>> +	struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
>> +	__le32 *rxd = (__le32 *)skbdesc;
>>     
>
> This line looks bad. skbdesc points to entry->skb->cb, and you shouldn't cast the
> skb_frame_desc pointer to the descriptor because they mean completely different things.
>
>   
Ok I did it to be consistent with rt2800pci_fill_rxdone :
        struct skb_frame_desc *skbdesc = get_skb_frame_desc(entry->skb);
        __le32 *rxd = (__le32 *)skbdesc;
no more . I ll change and test if it modify the behaviour and report
(probably reverting to old code for those lines).

Thanks a lot. My primary target will be to clean the ALIGN_SIZE hack.
But this one looks like not that an easy
target. I ll have to translate :
rt2860sta sta/rtmp_data.c
STA_Build_ARalink_Frame_Header
       pHeaderBufPtr = &pTxBlk->HeaderBuf[TXINFO_SIZE + TXWI_SIZE];
        pHeader_802_11 = (HEADER_802_11 *) pHeaderBufPtr;

        // steal "order" bit to mark "aggregation"
        pHeader_802_11->FC.Order = 1;

        // skip common header
        pHeaderBufPtr += pTxBlk->MpduHeaderLen;

        if (TX_BLK_TEST_FLAG(pTxBlk, fTX_bWMM))
        {
                //
                // build QOS Control bytes
                //
                *pHeaderBufPtr = (pTxBlk->UserPriority & 0x0F);

                *(pHeaderBufPtr+1) = 0;
                pHeaderBufPtr +=2;
                pTxBlk->MpduHeaderLen += 2;
        }

        // padding at front of LLC header. LLC header should at 4-bytes
aligment.
        pTxBlk->HdrPadLen = (ULONG)pHeaderBufPtr;
        pHeaderBufPtr = (PCHAR)ROUND_UP(pHeaderBufPtr, 4);
        pTxBlk->HdrPadLen = (ULONG)(pHeaderBufPtr - pTxBlk->HdrPadLen);


(roughly the same in the 3 other frame functions).


Best regards
Alban