[rt2x00-users] [PATCH 5/5] rt2x00: Correctly identify MIMO_PS values

[Ivo Van Doorn]

On Mon, Aug 23, 2010 at 2:49 PM, Helmut Schaa
<helmut.schaa at googlemail.com> wrote:
> Am Monday 23 August 2010 schrieb Ivo van Doorn:
>> When sending frames, we should not use the STA capabilities
>> to determine which MIMO PS settings should be applied. Instead
>> we should read the configuration parameters from mac80211.
>> Also the legacy drivers indicate that the MIMO PS static mode,
>> have a maximum rate of MCS7.
>>
>> Signed-off-by: Ivo van Doorn <IvDoorn at gmail.com>
>> ---
>>  drivers/net/wireless/rt2x00/rt2x00ht.c |   15 +++++++++------
>>  1 files changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rt2x00/rt2x00ht.c b/drivers/net/wireless/rt2x00/rt2x00ht.c
>> index ad3c7ff..2e5d90e 100644
>> --- a/drivers/net/wireless/rt2x00/rt2x00ht.c
>> +++ b/drivers/net/wireless/rt2x00/rt2x00ht.c
>> @@ -33,6 +33,7 @@ void rt2x00ht_create_tx_descriptor(struct queue_entry *entry,
>>                                  struct txentry_desc *txdesc,
>>                                  const struct rt2x00_rate *hwrate)
>>  {
>> +     struct ieee80211_hw *hw= entry->queue->rt2x00dev->hw;
>>       struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(entry->skb);
>>       struct ieee80211_tx_rate *txrate = &tx_info->control.rates[0];
>>       struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)entry->skb->data;
>> @@ -57,13 +58,15 @@ void rt2x00ht_create_tx_descriptor(struct queue_entry *entry,
>>
>>               /*
>>                * MIMO PS should be set to 1 for STA's using dynamic SM PS
>> -              * when using more then one tx stream (>MCS7).
>> +              * when using more then one tx stream (>MCS7). For Static
>> +              * SM PS does now allow MCS rates higher then 7.
>>                */
>> -             if (tx_info->control.sta && txdesc->mcs > 7 &&
>> -                 (tx_info->control.sta->ht_cap.cap &
>> -                  (WLAN_HT_CAP_SM_PS_DYNAMIC <<
>> -                   IEEE80211_HT_CAP_SM_PS_SHIFT)))
>> -                     __set_bit(ENTRY_TXD_HT_MIMO_PS, &txdesc->flags);
>> +             if (txdesc->mcs > 7) {
>> +                     if (hw->conf.smps_mode == IEEE80211_SMPS_DYNAMIC)
>> +                             __set_bit(ENTRY_TXD_HT_MIMO_PS, &txdesc->flags);
>> +                     else if (hw->conf.smps_mode == IEEE80211_SMPS_STATIC)
>> +                             txdesc->mcs = 7;
>> +             }
>>       } else {
>>               txdesc->mcs = rt2x00_get_rate_mcs(hwrate->mcs);
>>               if (txrate->flags & IEEE80211_TX_RC_USE_SHORT_PREAMBLE)
>>
>
> I agree with the mcs > 7 check for enabling MIMO_PS as it doesn't make sense to
> enable it for MCS <= 7 rates. However, that could also be checked in mac80211.

True, I just copied the checks exactly from the legacy driver.

> The check for IEEE80211_SMPS_STATIC should also be accounted for in mac80211.
> As long as we are in static smps mode mac80211 shouldn't try to send with mcs
> rates requiring more chains then the enabled ones.
>
> The check for IEEE80211_SMPS_DYNAMIC doesn't make sense I'd say. The previous
> check was correct according to the spec sheet: "MMPS: 1: the remote peer is in
> dynamic MIMO-PS mode".

Correct, but mac80211 sends any value of ieee80211_smps_mode, also in the legacy
driver it works with a similar enumeration and uses it to translate to
the MMPS setting.
So I disagree that mac80211 must send a 1 or 0, since that only
applies to rt2800
and not for all wireless hardware. So it is the task of rt2x00lib to
translate it into something
rt2800 would accept. Which is checking the different ieee80211_smps_mode values.

> So this bit should be set to 1 when the _remote peer_ is in dynamic MIMO PS
> mode. The remote peer (be it the AP or a STA) is represented by
> tx_info->control.sta and if the peer is in dynamic SMPS mode can be checked
> with
>
> tx_info->control.sta->ht_cap.cap &> (WLAN_HT_CAP_SM_PS_DYNAMIC << IEEE80211_HT_CAP_SM_PS_SHIFT)
>
> So the current code should be correct.

I am not saying the current code is not correct (although it didn't
work because rt2800 didn't
advertise support for WLAN_HT_CAP_SM_PS to mac80211). But mac80211 does send the
requested MIMO PS state to the driver, which means that mac80211 bases
its value from
the STA interface but perhaps has other reasons to enable (or disable)
it. Overall listening
to mac80211 configuration should preceed any setting from the STA
interface and the
driver interpretation of the STA capabilities (because _that_ is the
task of mac80211).

Ivo