[PATCH] ath10k: fix SMPS support

[Michal Kazior]

On 13 February 2014 15:29, Kalle Valo <kvalo at qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior at tieto.com> writes:
>
>> Firmware ignores SMPS flags in peer assoc command.
>>
>> For SMPS to work it is necessary to set peer
>> parameter after peer assoc command so that tx
>> chainmask is setup properly.
>>
>> This should fix packet loss and improve throughput
>> with stations that have SMPS enabled upon
>> association.
>>
>> Signed-off-by: Michal Kazior <michal.kazior at tieto.com>
>
> [...]
>
>> +static const u32 ath10k_smps_map[] = {
>> +     [WLAN_HT_CAP_SM_PS_STATIC] = WMI_PEER_SMPS_STATIC,
>> +     [WLAN_HT_CAP_SM_PS_DYNAMIC] = WMI_PEER_SMPS_DYNAMIC,
>> +     [WLAN_HT_CAP_SM_PS_INVALID] = WMI_PEER_SMPS_PS_NONE,
>> +     [WLAN_HT_CAP_SM_PS_DISABLED] = WMI_PEER_SMPS_PS_NONE,
>> +};
>> +
>> +static int ath10k_setup_peer_smps(struct ath10k *ar, struct ath10k_vif *arvif,
>> +                               const u8 *addr,
>> +                               const struct ieee80211_sta_ht_cap *ht_cap)
>> +{
>> +     int smps;
>> +
>> +     if (!ht_cap->ht_supported)
>> +             return 0;
>> +
>> +     smps = ht_cap->cap & IEEE80211_HT_CAP_SM_PS;
>> +     smps >>= IEEE80211_HT_CAP_SM_PS_SHIFT;
>> +
>> +     if (smps >= ARRAY_SIZE(ath10k_smps_map))
>> +             return -EINVAL;
>> +
>> +     return ath10k_wmi_peer_set_param(ar, arvif->vdev_id, addr,
>> +                                      WMI_PEER_SMPS_STATE,
>> +                                      ath10k_smps_map[smps]);
>> +}
>
> ath10k_smps_map looks overkill (and fragile), wouldn't a switch
> statement be simpler?

The map shouldn't really ever change since it depends on the 11n spec.
I just prefer it this way as it's shorter and easier to grasp. I can
change it to a switch() if you insist.


>> @@ -1370,6 +1385,7 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
>>  {
>>       struct ath10k *ar = hw->priv;
>>       struct ath10k_vif *arvif = ath10k_vif_to_arvif(vif);
>> +     struct ieee80211_sta_ht_cap ht_cap;
>>       struct wmi_peer_assoc_complete_arg peer_arg;
>>       struct ieee80211_sta *ap_sta;
>>       int ret;
>> @@ -1386,6 +1402,8 @@ static void ath10k_bss_assoc(struct ieee80211_hw *hw,
>>               return;
>>       }
>>
>> +     ht_cap = ap_sta->ht_cap;
>
> Why do you copy ht_cap? I can't figure out the reason.

It is used by ath10k_setup_peer_smps() which might sleep as it sends
wmi command. This means we have to leave rcu section and must not
touch ap_sta pointer anymore.


Michał