Kalle Valo <kvalo@xxxxxxxxxxxxxx> writes:
> Venkateswara Naralasetty <vnaralas@xxxxxxxxxxxxxx> writes:
>
>> AP power save where AP goes to power save mode when no stations associate
>> to it and come out of power save when any station associate to AP.
>>
>> This AP power save capability can be used to save power with the drawback
>> of reduced range or delayed discovery of the AP
>>
>> This patch also porvides user configuration to enable/disable
>> this feature using vendor command. This feature is disabled by default.
>>
>> Tested-on: IPQ8074 WLAN.HK.2.1.0.1-01228-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Venkateswara Naralasetty <vnaralas@xxxxxxxxxxxxxx>
>
> [...]
>
>> +static int ath11k_vendor_set_wifi_config(struct wiphy *wihpy,
>
> s/wihpy/wiphy/
>
>> + struct wireless_dev *wdev,
>> + const void *data,
>> + int data_len)
>> +{
>> + struct ieee80211_vif *vif;
>> + struct ath11k_vif *arvif;
>> + struct ath11k *ar;
>> + struct nlattr *tb[QCA_WLAN_VENDOR_ATTR_CONFIG_MAX + 1];
>> + int ret = 0;
>> +
>> + if (!wdev)
>> + return -EINVAL;
>> +
>> + vif = wdev_to_ieee80211_vif(wdev);
>> + if (!vif)
>> + return -EINVAL;
>> +
>> + arvif = (struct ath11k_vif *)vif->drv_priv;
>> + if (!arvif)
>> + return -EINVAL;
>> +
>> + ar = arvif->ar;
>> +
>> + mutex_lock(&ar->conf_mutex);
>> +
>> + ret = nla_parse(tb, QCA_WLAN_VENDOR_ATTR_CONFIG_MAX, data, data_len,
>> + ath11k_vendor_set_wifi_config_policy, NULL);
>> + if (ret) {
>> + ath11k_warn(ar->ab, "invalid set wifi config policy attribute\n");
>> + goto exit;
>> + }
>> +
>> + ar->ap_ps_enabled = nla_get_flag(tb[QCA_WLAN_VENDOR_ATTR_CONFIG_GTX]);
>> + ret = ath11k_mac_ap_ps_recalc(ar);
>> + if (ret) {
>> + ath11k_warn(ar->ab, "failed to send ap ps ret %d\n", ret);
>> + goto exit;
>> + }
>> +
>> +exit:
>> + mutex_unlock(&ar->conf_mutex);
>> + return ret;
>> +}
>
> Something which I find awkward here is that this is per pdev (=all
> vdevs), even though the vendor command is per vif. So if you change the
> config on one vif, all other vifs will change as well. And there's no
> way to check if the state from user space as there's only a set command
> and no equivalent get command.
Actually the problem comes here:
+static struct wiphy_vendor_command ath11k_vendor_commands[] = {
+ {
+ .info.vendor_id = QCA_NL80211_VENDOR_ID,
+ .info.subcmd = QCA_NL80211_VENDOR_SUBCMD_SET_WIFI_CONFIGURATION,
+ .flags = WIPHY_VENDOR_CMD_NEED_WDEV |
+ WIPHY_VENDOR_CMD_NEED_RUNNING,
+ .doit = ath11k_vendor_set_wifi_config,
+ .policy = ath11k_vendor_set_wifi_config_policy,
+ .maxattr = QCA_WLAN_VENDOR_ATTR_CONFIG_MAX
+ }
If it's per pdev you shouldn't set WIPHY_VENDOR_CMD_NEED_WDEV.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
