Currently CAC running flag in ath11k is not set if DFS radar is required in
secondary channel. This is due to the fact that only primary channel's DFS
state and CAC time is being checked before setting the flag.
Fix this issue by checking the DFS state of all the sub-channels inside a
channel definition and not just the primary channel.
Also, make minor change in debug prints to show the CAC time
Aditya Kumar Singh (3):
wifi: cfg80211: export DFS CAC time and usable state helper functions
wifi: ath11k: fix CAC running state during virtual interface start
wifi: ath11k: fix Tx power value during active CAC
---
v2: rebased on ToT
---
drivers/net/wireless/ath/ath11k/mac.c | 27 +++++++++++++++++++--------
include/net/cfg80211.h | 24 ++++++++++++++++++++++++
net/wireless/chan.c | 2 ++
net/wireless/core.h | 17 -----------------
4 files changed, 45 insertions(+), 25 deletions(-)
base-commit: 0263687f4441d5a5eab8074d56b4693c8f0acf85
--
2.17.1
cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
at least one channel in the chandef is in usable state or not. Also,
cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
time required for the given chandef.
Make these two functions visible to drivers by exporting their symbol
to global list of kernel symbols.
Lower level drivers can make use of these two functions to be aware
if CAC is required on the given chandef and for how long. For example
drivers which maintains the CAC state internally can make use of these.
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
include/net/cfg80211.h | 24 ++++++++++++++++++++++++
net/wireless/chan.c | 2 ++
net/wireless/core.h | 17 -----------------
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 3a4b684f89bf..92072a0f0ea6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -953,6 +953,30 @@ int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
const struct cfg80211_chan_def *chandef,
enum nl80211_iftype iftype);
+/**
+ * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable and we
+ * can/need start CAC on such channel
+ * @wiphy: the wiphy to validate against
+ * @chandef: the channel definition to check
+ *
+ * Return: true if all channels available and at least
+ * one channel requires CAC (NL80211_DFS_USABLE)
+ */
+bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
+ const struct cfg80211_chan_def *chandef);
+
+/**
+ * cfg80211_chandef_dfs_cac_time - get the DFS CAC time (in ms) for given
+ * channel definition
+ * @wiphy: the wiphy to validate against
+ * @chandef: the channel definition to check
+ *
+ * Returns: DFS CAC time (in ms) which applies for this channel definition
+ */
+unsigned int
+cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
+ const struct cfg80211_chan_def *chandef);
+
/**
* nl80211_send_chandef - sends the channel definition.
* @msg: the msg to send channel definition
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 0b7e81db383d..a78a6183d11e 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -666,6 +666,7 @@ bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
return (r1 + r2 > 0);
}
+EXPORT_SYMBOL(cfg80211_chandef_dfs_usable);
/*
* Checks if center frequency of chan falls with in the bandwidth
@@ -965,6 +966,7 @@ cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
return max(t1, t2);
}
+EXPORT_SYMBOL(cfg80211_chandef_dfs_cac_time);
static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
u32 center_freq, u32 bandwidth,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 507d184b8b40..cc4eb3906c0c 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -469,29 +469,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev);
extern struct work_struct cfg80211_disconnect_work;
-/**
- * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable
- * @wiphy: the wiphy to validate against
- * @chandef: the channel definition to check
- *
- * Checks if chandef is usable and we can/need start CAC on such channel.
- *
- * Return: true if all channels available and at least
- * one channel requires CAC (NL80211_DFS_USABLE)
- */
-bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
- const struct cfg80211_chan_def *chandef);
-
void cfg80211_set_dfs_state(struct wiphy *wiphy,
const struct cfg80211_chan_def *chandef,
enum nl80211_dfs_state dfs_state);
void cfg80211_dfs_channels_update_work(struct work_struct *work);
-unsigned int
-cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
- const struct cfg80211_chan_def *chandef);
-
void cfg80211_sched_dfs_chan_update(struct cfg80211_registered_device *rdev);
int
--
2.17.1
Tx power is fetched from firmware's pdev stats. However, during active
CAC, firmware does not fill the current Tx power and sends the max
initialised value filled during firmware init. If host sends this power
to user space, this is wrong since in certain situations, the Tx power
could be greater than the max allowed by the regulatory. Hence, host
should not be fetching the Tx power during an active CAC.
Fix this issue by returning Tx power as 0 during active CAC since it
is known that during CAC, there will be no transmission happening.
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.7.0.1-01744-QCAHKSWPL_SILICONZ-1
Fixes: 9a2aa68afe3d ("wifi: ath11k: add get_txpower mac ops")
Signed-off-by: Aditya Kumar Singh <[email protected]>
---
drivers/net/wireless/ath/ath11k/mac.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/wireless/ath/ath11k/mac.c b/drivers/net/wireless/ath/ath11k/mac.c
index 6b5f032197ff..a36698688f89 100644
--- a/drivers/net/wireless/ath/ath11k/mac.c
+++ b/drivers/net/wireless/ath/ath11k/mac.c
@@ -9045,6 +9045,14 @@ static int ath11k_mac_op_get_txpower(struct ieee80211_hw *hw,
if (ar->state != ATH11K_STATE_ON)
goto err_fallback;
+ /* Firmware doesn't provide Tx power during CAC hence no need to fetch
+ * the stats.
+ */
+ if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
+ mutex_unlock(&ar->conf_mutex);
+ return -EAGAIN;
+ }
+
req_param.pdev_id = ar->pdev->pdev_id;
req_param.stats_id = WMI_REQUEST_PDEV_STAT;
--
2.17.1
On Tue, 2023-09-12 at 10:48 +0530, Aditya Kumar Singh wrote:
> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
> at least one channel in the chandef is in usable state or not. Also,
> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
> time required for the given chandef.
>
Should we really export the time function just for a debug messages?
That seems like a waste of space?
johannes
On 9/13/23 14:58, Johannes Berg wrote:
> On Tue, 2023-09-12 at 10:48 +0530, Aditya Kumar Singh wrote:
>> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
>> at least one channel in the chandef is in usable state or not. Also,
>> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
>> time required for the given chandef.
>>
>
> Should we really export the time function just for a debug messages?
> That seems like a waste of space?
>
Yes absolutely. But actually as a follow up of this patch, we have
Background DFS patch in pipeline which requires us to send the CAC time
to firmware. So its actually needed there. And may be once we do this,
other drivers also may start using this?
On Wed, 2023-09-13 at 15:46 +0530, Aditya Kumar Singh wrote:
> On 9/13/23 14:58, Johannes Berg wrote:
> > On Tue, 2023-09-12 at 10:48 +0530, Aditya Kumar Singh wrote:
> > > cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
> > > at least one channel in the chandef is in usable state or not. Also,
> > > cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
> > > time required for the given chandef.
> > >
> >
> > Should we really export the time function just for a debug messages?
> > That seems like a waste of space?
> >
> Yes absolutely. But actually as a follow up of this patch, we have
> Background DFS patch in pipeline which requires us to send the CAC time
> to firmware. So its actually needed there. And may be once we do this,
> other drivers also may start using this?
OK, fair enough :)
johannes
On 9/11/2023 10:18 PM, Aditya Kumar Singh wrote:
> cfg80211 has cfg80211_chandef_dfs_usable() function to know whether
> at least one channel in the chandef is in usable state or not. Also,
> cfg80211_chandef_dfs_cac_time() function is there which tells the CAC
> time required for the given chandef.
>
> Make these two functions visible to drivers by exporting their symbol
> to global list of kernel symbols.
>
> Lower level drivers can make use of these two functions to be aware
> if CAC is required on the given chandef and for how long. For example
> drivers which maintains the CAC state internally can make use of these.
>
> Signed-off-by: Aditya Kumar Singh <[email protected]>
Reviewed-by: Jeff Johnson <[email protected]>
> ---
> include/net/cfg80211.h | 24 ++++++++++++++++++++++++
> net/wireless/chan.c | 2 ++
> net/wireless/core.h | 17 -----------------
> 3 files changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 3a4b684f89bf..92072a0f0ea6 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -953,6 +953,30 @@ int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> const struct cfg80211_chan_def *chandef,
> enum nl80211_iftype iftype);
>
> +/**
> + * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable and we
> + * can/need start CAC on such channel
> + * @wiphy: the wiphy to validate against
> + * @chandef: the channel definition to check
> + *
> + * Return: true if all channels available and at least
> + * one channel requires CAC (NL80211_DFS_USABLE)
> + */
> +bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
> + const struct cfg80211_chan_def *chandef);
> +
> +/**
> + * cfg80211_chandef_dfs_cac_time - get the DFS CAC time (in ms) for given
> + * channel definition
> + * @wiphy: the wiphy to validate against
> + * @chandef: the channel definition to check
> + *
> + * Returns: DFS CAC time (in ms) which applies for this channel definition
> + */
> +unsigned int
> +cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
> + const struct cfg80211_chan_def *chandef);
> +
> /**
> * nl80211_send_chandef - sends the channel definition.
> * @msg: the msg to send channel definition
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index 0b7e81db383d..a78a6183d11e 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -666,6 +666,7 @@ bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
>
> return (r1 + r2 > 0);
> }
> +EXPORT_SYMBOL(cfg80211_chandef_dfs_usable);
>
> /*
> * Checks if center frequency of chan falls with in the bandwidth
> @@ -965,6 +966,7 @@ cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
>
> return max(t1, t2);
> }
> +EXPORT_SYMBOL(cfg80211_chandef_dfs_cac_time);
>
> static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
> u32 center_freq, u32 bandwidth,
> diff --git a/net/wireless/core.h b/net/wireless/core.h
> index 507d184b8b40..cc4eb3906c0c 100644
> --- a/net/wireless/core.h
> +++ b/net/wireless/core.h
> @@ -469,29 +469,12 @@ int cfg80211_scan(struct cfg80211_registered_device *rdev);
>
> extern struct work_struct cfg80211_disconnect_work;
>
> -/**
> - * cfg80211_chandef_dfs_usable - checks if chandef is DFS usable
> - * @wiphy: the wiphy to validate against
> - * @chandef: the channel definition to check
> - *
> - * Checks if chandef is usable and we can/need start CAC on such channel.
> - *
> - * Return: true if all channels available and at least
> - * one channel requires CAC (NL80211_DFS_USABLE)
> - */
> -bool cfg80211_chandef_dfs_usable(struct wiphy *wiphy,
> - const struct cfg80211_chan_def *chandef);
> -
> void cfg80211_set_dfs_state(struct wiphy *wiphy,
> const struct cfg80211_chan_def *chandef,
> enum nl80211_dfs_state dfs_state);
>
> void cfg80211_dfs_channels_update_work(struct work_struct *work);
>
> -unsigned int
> -cfg80211_chandef_dfs_cac_time(struct wiphy *wiphy,
> - const struct cfg80211_chan_def *chandef);
> -
> void cfg80211_sched_dfs_chan_update(struct cfg80211_registered_device *rdev);
>
> int
Aditya Kumar Singh <[email protected]> wrote:
> Tx power is fetched from firmware's pdev stats. However, during active
> CAC, firmware does not fill the current Tx power and sends the max
> initialised value filled during firmware init. If host sends this power
> to user space, this is wrong since in certain situations, the Tx power
> could be greater than the max allowed by the regulatory. Hence, host
> should not be fetching the Tx power during an active CAC.
>
> Fix this issue by returning Tx power as 0 during active CAC since it
> is known that during CAC, there will be no transmission happening.
The returning as 0 doesn't seem to match the code. Should I change the sentence to:
"Fix this issue by returning -EAGAIN error so that the user space knows there's
no value available right now."
--
https://patchwork.kernel.org/project/linux-wireless/patch/[email protected]/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 10/3/23 20:01, Kalle Valo wrote:
> Aditya Kumar Singh <[email protected]> wrote:
>
>> Tx power is fetched from firmware's pdev stats. However, during active
>> CAC, firmware does not fill the current Tx power and sends the max
>> initialised value filled during firmware init. If host sends this power
>> to user space, this is wrong since in certain situations, the Tx power
>> could be greater than the max allowed by the regulatory. Hence, host
>> should not be fetching the Tx power during an active CAC.
>>
>> Fix this issue by returning Tx power as 0 during active CAC since it
>> is known that during CAC, there will be no transmission happening.
>
> The returning as 0 doesn't seem to match the code. Should I change the sentence to:
>
> "Fix this issue by returning -EAGAIN error so that the user space knows there's
> no value available right now."
Oops. Looks like only in commit message its still zero. Its changed to
return -EAGAIN in code.
+ if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
+ mutex_unlock(&ar->conf_mutex);
+ return -EAGAIN;
+ }
So could you just rectify while applying or should I resend?
Aditya Kumar Singh <[email protected]> writes:
> On 10/3/23 20:01, Kalle Valo wrote:
>> Aditya Kumar Singh <[email protected]> wrote:
>>
>>> Tx power is fetched from firmware's pdev stats. However, during active
>>> CAC, firmware does not fill the current Tx power and sends the max
>>> initialised value filled during firmware init. If host sends this power
>>> to user space, this is wrong since in certain situations, the Tx power
>>> could be greater than the max allowed by the regulatory. Hence, host
>>> should not be fetching the Tx power during an active CAC.
>>>
>>> Fix this issue by returning Tx power as 0 during active CAC since it
>>> is known that during CAC, there will be no transmission happening.
>> The returning as 0 doesn't seem to match the code. Should I change
>> the sentence to:
>> "Fix this issue by returning -EAGAIN error so that the user space
>> knows there's
>> no value available right now."
> Oops. Looks like only in commit message its still zero. Its changed to
> return -EAGAIN in code.
>
> + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
> + mutex_unlock(&ar->conf_mutex);
> + return -EAGAIN;
> + }
>
> So could you just rectify while applying or should I resend?
No need to resend because of this. I changed the commit message now to
this in the pending branch:
https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6eacc3b5a70ab3f92f9410839870edbb21c9d051
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 10/4/23 10:24, Kalle Valo wrote:
> Aditya Kumar Singh <[email protected]> writes:
>
>> On 10/3/23 20:01, Kalle Valo wrote:
>>> Aditya Kumar Singh <[email protected]> wrote:
>>>
>>>> Tx power is fetched from firmware's pdev stats. However, during active
>>>> CAC, firmware does not fill the current Tx power and sends the max
>>>> initialised value filled during firmware init. If host sends this power
>>>> to user space, this is wrong since in certain situations, the Tx power
>>>> could be greater than the max allowed by the regulatory. Hence, host
>>>> should not be fetching the Tx power during an active CAC.
>>>>
>>>> Fix this issue by returning Tx power as 0 during active CAC since it
>>>> is known that during CAC, there will be no transmission happening.
>>> The returning as 0 doesn't seem to match the code. Should I change
>>> the sentence to:
>>> "Fix this issue by returning -EAGAIN error so that the user space
>>> knows there's
>>> no value available right now."
>> Oops. Looks like only in commit message its still zero. Its changed to
>> return -EAGAIN in code.
>>
>> + if (test_bit(ATH11K_CAC_RUNNING, &ar->dev_flags)) {
>> + mutex_unlock(&ar->conf_mutex);
>> + return -EAGAIN;
>> + }
>>
>> So could you just rectify while applying or should I resend?
>
> No need to resend because of this. I changed the commit message now to
> this in the pending branch:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/ath.git/commit/?h=pending&id=6eacc3b5a70ab3f92f9410839870edbb21c9d051Sure, thanks!