Adding also ath10k list.
Ashok Raj Nagarajan <arnagara@xxxxxxxxxxxxxxxx> writes:
> Now that ANI is enabled by default, allow user to disable or enable ANI feature
> from debugfs
>
> Signed-off-by: Ashok Raj Nagarajan <arnagara@xxxxxxxxxxxxxxxx>
> ---
> drivers/net/wireless/ath/ath10k/core.h | 1 +
> drivers/net/wireless/ath/ath10k/debug.c | 58 +++++++++++++++++++++++++++++++++
> drivers/net/wireless/ath/ath10k/mac.c | 1 +
> 3 files changed, 60 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> index 7cba781..1359d2c 100644
> --- a/drivers/net/wireless/ath/ath10k/core.h
> +++ b/drivers/net/wireless/ath/ath10k/core.h
> @@ -510,6 +510,7 @@ struct ath10k {
> u32 ht_cap_info;
> u32 vht_cap_info;
> u32 num_rf_chains;
> + bool ani_enabled;
Please document the locking, "/* protected by conf_mutex */" or
something like that.
>
> DECLARE_BITMAP(fw_features, ATH10K_FW_FEATURE_COUNT);
>
> diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> index 301081d..95f2a29 100644
> --- a/drivers/net/wireless/ath/ath10k/debug.c
> +++ b/drivers/net/wireless/ath/ath10k/debug.c
> @@ -1708,6 +1708,61 @@ static int ath10k_debug_cal_data_release(struct inode *inode,
> return 0;
> }
>
> +static ssize_t ath10k_write_ani_enable(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct ath10k *ar = file->private_data;
> + int ret;
> + u8 enable;
> +
> + if (kstrtou8_from_user(user_buf, count, 0, &enable))
> + return -EINVAL;
> +
> + if (ar->ani_enabled == enable)
> + return count;
> +
> + mutex_lock(&ar->conf_mutex);
Hmm, you don't protect ar->ani_enabled? Isn't that racy?
> +
> + ret = ath10k_wmi_pdev_set_param(ar, ar->wmi.pdev_param->ani_enable,
> + enable);
> + if (ret) {
> + ath10k_warn(ar, "ani_enable failed from debugfs: %d\n", ret);
> + goto exit;
> + }
> + ar->ani_enabled = enable;
> +
> + ret = count;
> +
> + ath10k_dbg(ar, ATH10K_DBG_WMI, "ANI is %s\n",
> + enable ? "enabled" : "disabled");
WMI debug level should not be used here. Is this debug really needed? I
would assume there's a debug print in ath10k_wmi_pdev_set_param().
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -2903,6 +2903,7 @@ static int ath10k_start(struct ieee80211_hw *hw)
> ret);
> goto err_core_stop;
> }
Empty line here.
> + ar->ani_enabled = 1;
ar->ani_enabled = true;
--
Kalle Valo
--
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
