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

Helmut Schaa helmut.schaa at googlemail.com
Mon Aug 23 13:43:07 UTC 2010


Am Monday 23 August 2010 schrieb Ivo Van Doorn:
> On Mon, Aug 23, 2010 at 3:18 PM, Helmut Schaa
> <helmut.schaa at googlemail.com> wrote:
> > Am Monday 23 August 2010 schrieb 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.
> >
> > Yeah, but that value should only be configured via the config callback not via
> > the tx callback.
> >
> >> 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).
> >
> > We don't need to advertise that to mac80211 in order to send frames to a
> > dynamic SMPS enabled station. Setting one of the WLAN_HT_CAP_SM_PS flags
> > only tells mac80211 that we are able to turn ourself into different SMPS
> > modes. Sending frames to dynamic SMPS stations is independant.
> >
> >> 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).
> >
> > But we should use the SMPS mode configured via the config callback as our own
> > SMPS mode (and we don't have code for that yet) while the MIMO_PS flag in the
> > TXWI should only be set when the remote peer is in dynamic SMPS mode.
> 
> Ok, well lets just drop the patches then. Legacy driver claims support
> and does nothing
> special with the values other then using it to transmit frames. So I
> don't know how to adjust
> the patch to make it acceptable.

Fine with me fow now ;) I also had another look at the SMPS code in the legacy
drivers but it seems as if the device is never set into SMPS mode at all. Not
sure what would be needed to support it!?

Helmut



More information about the users mailing list