* [PATCH 1/2] ath10k: enable ANI by default @ 2015-03-18 18:01 Ashok Raj Nagarajan 2015-03-18 18:01 ` [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs Ashok Raj Nagarajan 2015-03-19 8:45 ` [PATCH 1/2] ath10k: enable ANI by default Kalle Valo 0 siblings, 2 replies; 7+ messages in thread From: Ashok Raj Nagarajan @ 2015-03-18 18:01 UTC (permalink / raw) To: kvalo; +Cc: linux-wireless, Ashok Raj Nagarajan ANI is currently not enabled by default. Enable this feature by default. Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> --- drivers/net/wireless/ath/ath10k/mac.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 0f39af7..380d4b1 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -2896,6 +2896,14 @@ static int ath10k_start(struct ieee80211_hw *hw) goto err_core_stop; } + ret = ath10k_wmi_pdev_set_param(ar, + ar->wmi.pdev_param->ani_enable, 1); + if (ret) { + ath10k_warn(ar, "failed to enable ani by default: %d\n", + ret); + goto err_core_stop; + } + ar->num_started_vdevs = 0; ath10k_regd_update(ar); -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs 2015-03-18 18:01 [PATCH 1/2] ath10k: enable ANI by default Ashok Raj Nagarajan @ 2015-03-18 18:01 ` Ashok Raj Nagarajan 2015-03-19 8:52 ` Kalle Valo 2015-03-19 8:45 ` [PATCH 1/2] ath10k: enable ANI by default Kalle Valo 1 sibling, 1 reply; 7+ messages in thread From: Ashok Raj Nagarajan @ 2015-03-18 18:01 UTC (permalink / raw) To: kvalo; +Cc: linux-wireless, Ashok Raj Nagarajan Now that ANI is enabled by default, allow user to disable or enable ANI feature from debugfs Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> --- 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; 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); + + 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"); +exit: + mutex_unlock(&ar->conf_mutex); + + return ret; +} + +static ssize_t ath10k_read_ani_enable(struct file *file, char __user *user_buf, + size_t count, loff_t *ppos) +{ + struct ath10k *ar = file->private_data; + int len = 0; + char buf[32]; + + len = scnprintf(buf, sizeof(buf) - len, "%d\n", + ar->ani_enabled); + + return simple_read_from_buffer(user_buf, count, ppos, buf, len); +} + +static const struct file_operations fops_ani_enable = { + .read = ath10k_read_ani_enable, + .write = ath10k_write_ani_enable, + .open = simple_open, + .owner = THIS_MODULE, + .llseek = default_llseek, +}; + static const struct file_operations fops_cal_data = { .open = ath10k_debug_cal_data_open, .read = ath10k_debug_cal_data_read, @@ -2068,6 +2123,9 @@ int ath10k_debug_register(struct ath10k *ar) debugfs_create_file("cal_data", S_IRUSR, ar->debug.debugfs_phy, ar, &fops_cal_data); + debugfs_create_file("ani_enable", S_IRUSR | S_IWUSR, + ar->debug.debugfs_phy, ar, &fops_ani_enable); + debugfs_create_file("nf_cal_period", S_IRUSR | S_IWUSR, ar->debug.debugfs_phy, ar, &fops_nf_cal_period); diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 380d4b1..456aa3c 100644 --- 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; } + ar->ani_enabled = 1; ar->num_started_vdevs = 0; ath10k_regd_update(ar); -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs 2015-03-18 18:01 ` [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs Ashok Raj Nagarajan @ 2015-03-19 8:52 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2015-03-19 8:52 UTC (permalink / raw) To: Ashok Raj Nagarajan; +Cc: linux-wireless, ath10k Adding also ath10k list. Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> 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@qti.qualcomm.com> > --- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs @ 2015-03-19 8:52 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2015-03-19 8:52 UTC (permalink / raw) To: Ashok Raj Nagarajan; +Cc: linux-wireless, ath10k Adding also ath10k list. Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> 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@qti.qualcomm.com> > --- > 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 _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ath10k: enable ANI by default 2015-03-18 18:01 [PATCH 1/2] ath10k: enable ANI by default Ashok Raj Nagarajan 2015-03-18 18:01 ` [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs Ashok Raj Nagarajan @ 2015-03-19 8:45 ` Kalle Valo 2015-03-19 8:47 ` Kalle Valo 1 sibling, 1 reply; 7+ messages in thread From: Kalle Valo @ 2015-03-19 8:45 UTC (permalink / raw) To: Ashok Raj Nagarajan; +Cc: linux-wireless Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> writes: > ANI is currently not enabled by default. Enable this feature by default. > > Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> You did not send this to ath10k list (and CC linux-wireless). Check the instructions here: https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches Also the commit log doesn't tell anything. What is ANI and why should it be enabled? What bug does this fix (if any)? How will the user see the difference after this patch is applied? As a rule of thumb, the commit log should tell any engineer (even one who is not familiar with ath10k) how the behaviour changes after the patch is applied. Think of your target group being distro maintainers, ath10k users, kernel subsystem maintainers etc. No company internal jargon or anything like that, write in plain english so that everyone understand. -- Kalle Valo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ath10k: enable ANI by default 2015-03-19 8:45 ` [PATCH 1/2] ath10k: enable ANI by default Kalle Valo @ 2015-03-19 8:47 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2015-03-19 8:47 UTC (permalink / raw) To: Ashok Raj Nagarajan; +Cc: linux-wireless, ath10k Kalle Valo <kvalo@qca.qualcomm.com> writes: > Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> writes: > >> ANI is currently not enabled by default. Enable this feature by default. >> >> Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> > > You did not send this to ath10k list (and CC linux-wireless). Check the > instructions here: > > https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches > > Also the commit log doesn't tell anything. What is ANI and why should it > be enabled? What bug does this fix (if any)? How will the user see the > difference after this patch is applied? > > As a rule of thumb, the commit log should tell any engineer (even one > who is not familiar with ath10k) how the behaviour changes after the > patch is applied. Think of your target group being distro maintainers, > ath10k users, kernel subsystem maintainers etc. No company internal > jargon or anything like that, write in plain english so that everyone > understand. I also forgot to CC ath10k list. -- Kalle Valo ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] ath10k: enable ANI by default @ 2015-03-19 8:47 ` Kalle Valo 0 siblings, 0 replies; 7+ messages in thread From: Kalle Valo @ 2015-03-19 8:47 UTC (permalink / raw) To: Ashok Raj Nagarajan; +Cc: linux-wireless, ath10k Kalle Valo <kvalo@qca.qualcomm.com> writes: > Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> writes: > >> ANI is currently not enabled by default. Enable this feature by default. >> >> Signed-off-by: Ashok Raj Nagarajan <arnagara@qti.qualcomm.com> > > You did not send this to ath10k list (and CC linux-wireless). Check the > instructions here: > > https://wireless.wiki.kernel.org/en/users/drivers/ath10k/sources#submitting_patches > > Also the commit log doesn't tell anything. What is ANI and why should it > be enabled? What bug does this fix (if any)? How will the user see the > difference after this patch is applied? > > As a rule of thumb, the commit log should tell any engineer (even one > who is not familiar with ath10k) how the behaviour changes after the > patch is applied. Think of your target group being distro maintainers, > ath10k users, kernel subsystem maintainers etc. No company internal > jargon or anything like that, write in plain english so that everyone > understand. I also forgot to CC ath10k list. -- Kalle Valo _______________________________________________ ath10k mailing list ath10k@lists.infradead.org http://lists.infradead.org/mailman/listinfo/ath10k ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-03-19 8:52 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-03-18 18:01 [PATCH 1/2] ath10k: enable ANI by default Ashok Raj Nagarajan 2015-03-18 18:01 ` [PATCH 2/2] ath10k: allow user to toggle ani_enable via debugfs Ashok Raj Nagarajan 2015-03-19 8:52 ` Kalle Valo 2015-03-19 8:52 ` Kalle Valo 2015-03-19 8:45 ` [PATCH 1/2] ath10k: enable ANI by default Kalle Valo 2015-03-19 8:47 ` Kalle Valo 2015-03-19 8:47 ` Kalle Valo
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.