On 13 February 2014 15:29, Kalle Valo <kvalo@xxxxxxxxxxxxxxxx> wrote:
> Michal Kazior <michal.kazior@xxxxxxxxx> 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@xxxxxxxxx>
>
> [...]
>
>> +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ł
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
