Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Peter Oh writes: >> Sure, every software change can cause regressions. But the thing is that >> this isn't an optional, ath10k has to have this to be able to continue >> using DFS channels. > > Kalle, you said you don't know which exact FCC requirement this is for > in another email thread. Then how come you're sure this is not an > optional To me this is a mandatory ath10k feature because if the host driver does not have this DFS channels won't work anymore. > and this approach is right one? That's something you need to ask the firmware team who designed and implemented this. This discussion is going circles, it would be a lot easier if you could clearly say your point. But if your point is just "I don't like this" then there's not much, if any, I can do about that. -- Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Sure, every software change can cause regressions. But the thing is that this isn't an optional, ath10k has to have this to be able to continue using DFS channels. Kalle, you said you don't know which exact FCC requirement this is for in another email thread. Then how come you're sure this is not an optional and this approach is right one? Peter
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Adrian Chadd writes: > On Mon, 14 May 2018 at 11:25, Kalle Valo wrote: > >> Adrian Chadd writes: > >> > May we have a little more information about how this is supposed to > work? >> > >> > It looks like we're supposed to send the information about the matched >> > radar pattern back to the firmware for confirmation? What's the intended >> > behaviour from the firmware? Will the firmware have a hard-coded set of >> > patterns we have to answer in/by? > >> That's really an implementation detail inside the firmware and from >> ath10k point of view we don't care what checks the firmware has, we just >> provide all the necessary information. The checks in firmware might even >> change in later releases. > >> > I ask (like Peter, we work together) because we've had to tweak this >> > behaviour a little to actually pass FCC / ETSI DFS certification. My >> > general concern is that this'll cause a lot of false detects on boards > that >> > haven't had things tweaked for the given board. As far as I'm aware the > DFS >> > parameters are still hard-coded into the firmware image so if you have > to >> > change those you're SOL without the relevant NDAs - this makes running > the >> > open source DFS stuff a little tricksy on vendor boards. > >> This shouldn't cause more false detections, the pattern detection from >> ath.ko is still used as before. The firmware will just disable DFS >> altogether if it thinks ath10k is not compliant. > > > Heh, well the fun one for production for us is "ok, so what's > non-compliant" ? > > Eg - if it's 1 out of 100 that we don't hit the explicit timing > requirements because of the rest of the linux kernel (eg someone holds a > spinlock more than they should) then I'd prefer that we got a notification > that something happened so we can fix it. Otherwise in the field it'll just > be "hey, our stuff stopped working" because whatever the firmware > expectations are aren't being met. > > Again, we're OK because we can at least inspect what's going on, but not > everyone doing ath10k development/deployment will be so lucky :( Sure, every software change can cause regressions. But the thing is that this isn't an optional, ath10k has to have this to be able to continue using DFS channels. -- Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
On Mon, 14 May 2018 at 11:25, Kalle Valo wrote: > Adrian Chadd writes: > > May we have a little more information about how this is supposed to work? > > > > It looks like we're supposed to send the information about the matched > > radar pattern back to the firmware for confirmation? What's the intended > > behaviour from the firmware? Will the firmware have a hard-coded set of > > patterns we have to answer in/by? > That's really an implementation detail inside the firmware and from > ath10k point of view we don't care what checks the firmware has, we just > provide all the necessary information. The checks in firmware might even > change in later releases. > > I ask (like Peter, we work together) because we've had to tweak this > > behaviour a little to actually pass FCC / ETSI DFS certification. My > > general concern is that this'll cause a lot of false detects on boards that > > haven't had things tweaked for the given board. As far as I'm aware the DFS > > parameters are still hard-coded into the firmware image so if you have to > > change those you're SOL without the relevant NDAs - this makes running the > > open source DFS stuff a little tricksy on vendor boards. > This shouldn't cause more false detections, the pattern detection from > ath.ko is still used as before. The firmware will just disable DFS > altogether if it thinks ath10k is not compliant. Heh, well the fun one for production for us is "ok, so what's non-compliant" ? Eg - if it's 1 out of 100 that we don't hit the explicit timing requirements because of the rest of the linux kernel (eg someone holds a spinlock more than they should) then I'd prefer that we got a notification that something happened so we can fix it. Otherwise in the field it'll just be "hey, our stuff stopped working" because whatever the firmware expectations are aren't being met. Again, we're OK because we can at least inspect what's going on, but not everyone doing ath10k development/deployment will be so lucky :( -adrian
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Which regulatory enforcement are you talking to? Are you talking about such "prevent users from changing regulatory domain" or "don't provide a manner to change regulatory domain or channel list" ? If not, can you share the section of document? Sorry, I don't have any references about that. This question is for "what the patch is trying to solve" and "to comply FCC regulatory rule" is not the right answer. It should be answered by "FCC regulatory rule section xx.xx.xx". So we all can double check if this patch addresses "the root cause of the requirement" and "if it's the best approach or not" Thanks, Peter
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Adrian Chadd writes: > May we have a little more information about how this is supposed to work? > > It looks like we're supposed to send the information about the matched > radar pattern back to the firmware for confirmation? What's the intended > behaviour from the firmware? Will the firmware have a hard-coded set of > patterns we have to answer in/by? That's really an implementation detail inside the firmware and from ath10k point of view we don't care what checks the firmware has, we just provide all the necessary information. The checks in firmware might even change in later releases. > I ask (like Peter, we work together) because we've had to tweak this > behaviour a little to actually pass FCC / ETSI DFS certification. My > general concern is that this'll cause a lot of false detects on boards that > haven't had things tweaked for the given board. As far as I'm aware the DFS > parameters are still hard-coded into the firmware image so if you have to > change those you're SOL without the relevant NDAs - this makes running the > open source DFS stuff a little tricksy on vendor boards. This shouldn't cause more false detections, the pattern detection from ath.ko is still used as before. The firmware will just disable DFS altogether if it thinks ath10k is not compliant. -- Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Peter Oh writes:
> On 05/02/2018 04:27 AM, Kalle Valo wrote:
>> Peter Oh writes:
>>
>>> On 04/30/2018 10:45 AM, Sriram R wrote:
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when using
FCC regulatory region.
>>> What's the main reason you introduce this feature?
>>> What are you trying to solve with this change?
>> Otherwise one cannot use DFS channels on FCC regions with a firmware
>> from 10.4-3.6 branch.
>
> It's a big change and none of us knows until I asked and you answered.
I was hoping that would be clear from the commit log but I guess Sriram
could include my sentence above to enhance it?
@@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k
*ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
- if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL))
{
+ if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
"dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
- ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
- ATH10K_DFS_STAT_INC(ar, radar_detected);
+ if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
+ ar->dfs_detector->region == NL80211_DFS_FCC) {
>>>
>>> I feel risky that host drivers have no way to control this new feature
>>> and totally rely on FW feature mask. We should have a host drivers'
>>> feature mask such as module param and set it false (don't use) by
>>> default until it proves safe to use.
>>
>> This is for regulatory enforcement so it's not possible to disable the
>> feature from host.
>
> Which regulatory enforcement are you talking to? Are you talking about
> such "prevent users from changing regulatory domain" or "don't provide
> a manner to change regulatory domain or channel list" ? If not, can
> you share the section of document?
Sorry, I don't have any references about that.
> In addition to that, How do you make sure if FW side DFS radar
> detection algorithm has no defects and it always categorizes phy
> errors to correct radar type? I'm asking it, because it's known that
> DFS detector in ath module does sometimes detect radar patterns as
> wrong radar type in both of ath10k and ath9k. I couldn't think FW side
> detector has no such defects at all. It's like blind patch to open
> source community, since no one can review the implementation in FW
> even though no one can guarantee its level of integrity.
It's not about replacing the DFS detector in ath.ko, it's about adding
extra checks in firmware for regulatory compliance. So at least in
theory DFS detection should still work same as before, just with an
extra roundtrip via firmware.
--
Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Sriram R writes: > In the 10.4-3.6 firmware branch there's a new DFS Host confirmation > feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag. > > This new features enables the ath10k host to send information to the > firmware on the specifications of detected radar type. This allows the > firmware to validate if the host's radar pattern detector unit is > operational and check if the radar information shared by host matches > the radar pulses sent as phy error events from firmware. If the check > fails the firmware won't allow use of DFS channels on AP mode when using > FCC regulatory region. > > Supported Chipsets : QCA9984/QCA9888/QCA4019 > Firmware Version : 10.4-3.6-00104 > > Signed-off-by: Sriram R This introduces a new checkpatch warning: drivers/net/wireless/ath/ath10k/wmi.c:3753: Please use a blank line after function/struct/union/enum declarations -- Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
On 2018-05-01 01:20, Peter Oh wrote:
On 04/30/2018 10:45 AM, Sriram R wrote:
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT
flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when
using
FCC regulatory region.
What's the main reason you introduce this feature?
What are you trying to solve with this change?
+#define ATH10K_WMI_DFS_CONF_TIMEOUT_HZ (HZ / 2)
+static void ath10k_radar_confirmation_work(struct work_struct
*work)
+{
+ struct ath10k *ar = container_of(work, struct ath10k,
+radar_confirmation_work);
+ struct ath10k_radar_found_info radar_info;
+ int ret, time_left;
+
+ reinit_completion(&ar->wmi.radar_confirm);
+
+ spin_lock_bh(&ar->data_lock);
+ memcpy(&radar_info, &ar->last_radar_info, sizeof(radar_info));
+ spin_unlock_bh(&ar->data_lock);
+
+ ret = ath10k_wmi_report_radar_found(ar, &radar_info);
+ if (ret) {
+ ath10k_warn(ar, "failed to send radar found %d\n", ret);
+ goto wait_complete;
+ }
+
+ time_left = wait_for_completion_timeout(&ar->wmi.radar_confirm,
+ ATH10K_WMI_DFS_CONF_TIMEOUT_HZ);
It looks wrong to me in terms of timeout value.
Typical channel closing time in FCC domain is 200ms (excluding control
signals), but you're waiting for 500ms for response from FW.
Right Peter, We haven't hit this limit during our testing. On an average
we
received completion event within few milliseconds ,say less than
10-15ms. I'll correct
this timeout in my next patch revision.
@@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct
ath10k *ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
- if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL)) {
+ if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
"dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
- ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
- ATH10K_DFS_STAT_INC(ar, radar_detected);
+ if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map))
&&
+ ar->dfs_detector->region == NL80211_DFS_FCC) {
I feel risky that host drivers have no way to control this new feature
and totally rely on FW feature mask. We should have a host drivers'
feature mask such as module param and set it false (don't use) by
default until it proves safe to use.
+static void
+ath10k_wmi_event_dfs_status_check(struct ath10k *ar, struct sk_buff
*skb)
+{
+ struct wmi_dfs_status_ev_arg status_arg = {};
+ int ret;
+
+ ret = ath10k_wmi_pull_dfs_status(ar, skb, &status_arg);
+
+ if (ret) {
+ ath10k_warn(ar, "failed to parse dfs status event: %d\n", ret);
+ return;
+ }
+
+ ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
+ "dfs status event received from fw: %d\n",
+ status_arg.status);
+
+ /* Even in case of radar detection failure we follow the same
+* behaviour as if radar is detected i.e to switch to a different
+* channel.
+*/
+ if (status_arg.status == WMI_HW_RADAR_DETECTED ||
+ status_arg.status == WMI_RADAR_DETECTION_FAIL)
+ ath10k_radar_detected(ar);
+ complete(&ar->wmi.radar_confirm);
+}
What is typical average duration from
wait_for_completion_timeout(&ar->wmi.radar_confirm) to this completion
called ?
We didn't see this taking more than 20ms to reach this completion.
As mentioned above I'll change the timeout value in the next patch.
Thanks,
Sriram.R
Thanks,
Peter
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Am 02.05.2018 um 13:27 schrieb Kalle Valo:
Peter Oh writes:
On 04/30/2018 10:45 AM, Sriram R wrote:
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when using
FCC regulatory region.
What's the main reason you introduce this feature?
What are you trying to solve with this change?
Otherwise one cannot use DFS channels on FCC regions with a firmware
from 10.4-3.6 branch.
@@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k *ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
-if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL))
{
+ if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
"dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
- ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
- ATH10K_DFS_STAT_INC(ar, radar_detected);
+ if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
+ ar->dfs_detector->region == NL80211_DFS_FCC) {
I feel risky that host drivers have no way to control this new feature
and totally rely on FW feature mask. We should have a host drivers'
feature mask such as module param and set it false (don't use) by
default until it proves safe to use.
This is for regulatory enforcement so it's not possible to disable the
feature from host.
mmh from what i see it is possible to disable it from host since the
firmware seem to be sensitive on the regdomain set by the host
so if host sets anything else than a FCC regdomain, it will still work
in the old way. so its more a pseudo enforcement. dont know if this is
usefull and it also will it make impossible to fix radar detection
issues for certification as mentioned by peter and adrian.
Sebastian
--
Mit freundlichen Grüssen / Regards
Sebastian Gottschall / CTO
NewMedia-NET GmbH - DD-WRT
Firmensitz: Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottsch...@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Hi, May we have a little more information about how this is supposed to work? It looks like we're supposed to send the information about the matched radar pattern back to the firmware for confirmation? What's the intended behaviour from the firmware? Will the firmware have a hard-coded set of patterns we have to answer in/by? I ask (like Peter, we work together) because we've had to tweak this behaviour a little to actually pass FCC / ETSI DFS certification. My general concern is that this'll cause a lot of false detects on boards that haven't had things tweaked for the given board. As far as I'm aware the DFS parameters are still hard-coded into the firmware image so if you have to change those you're SOL without the relevant NDAs - this makes running the open source DFS stuff a little tricksy on vendor boards. -adrian
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
On 05/02/2018 04:27 AM, Kalle Valo wrote:
Peter Oh writes:
On 04/30/2018 10:45 AM, Sriram R wrote:
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when using
FCC regulatory region.
What's the main reason you introduce this feature?
What are you trying to solve with this change?
Otherwise one cannot use DFS channels on FCC regions with a firmware
from 10.4-3.6 branch.
It's a big change and none of us knows until I asked and you answered.
@@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k *ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
-if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL))
{
+ if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
"dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
- ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
- ATH10K_DFS_STAT_INC(ar, radar_detected);
+ if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
+ ar->dfs_detector->region == NL80211_DFS_FCC) {
I feel risky that host drivers have no way to control this new feature
and totally rely on FW feature mask. We should have a host drivers'
feature mask such as module param and set it false (don't use) by
default until it proves safe to use.
This is for regulatory enforcement so it's not possible to disable the
feature from host.
Which regulatory enforcement are you talking to?
Are you talking about such "prevent users from changing regulatory
domain" or "don't provide a manner to change regulatory domain or
channel list" ?
If not, can you share the section of document?
In addition to that, How do you make sure if FW side DFS radar detection
algorithm has no defects and it always categorizes phy errors to correct
radar type?
I'm asking it, because it's known that DFS detector in ath module does
sometimes detect radar patterns as wrong radar type in both of ath10k
and ath9k.
I couldn't think FW side detector has no such defects at all.
It's like blind patch to open source community, since no one can review
the implementation in FW even though no one can guarantee its level of
integrity.
Peter
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
Peter Oh writes:
> On 04/30/2018 10:45 AM, Sriram R wrote:
>> In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
>> feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
>>
>> This new features enables the ath10k host to send information to the
>> firmware on the specifications of detected radar type. This allows the
>> firmware to validate if the host's radar pattern detector unit is
>> operational and check if the radar information shared by host matches
>> the radar pulses sent as phy error events from firmware. If the check
>> fails the firmware won't allow use of DFS channels on AP mode when using
>> FCC regulatory region.
>
> What's the main reason you introduce this feature?
> What are you trying to solve with this change?
Otherwise one cannot use DFS channels on FCC regions with a firmware
from 10.4-3.6 branch.
>> @@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k
>> *ar,
>> ATH10K_DFS_STAT_INC(ar, pulses_detected);
>> - if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL))
>> {
>> +if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
>> ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
>> "dfs no pulse pattern detected, yet\n");
>> return;
>> }
>> -radar_detected:
>> -ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
>> -ATH10K_DFS_STAT_INC(ar, radar_detected);
>> +if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
>> +ar->dfs_detector->region == NL80211_DFS_FCC) {
>
> I feel risky that host drivers have no way to control this new feature
> and totally rely on FW feature mask. We should have a host drivers'
> feature mask such as module param and set it false (don't use) by
> default until it proves safe to use.
This is for regulatory enforcement so it's not possible to disable the
feature from host.
--
Kalle Valo
Re: [PATCH 2/2] ath10k: DFS Host Confirmation
On 04/30/2018 10:45 AM, Sriram R wrote:
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when using
FCC regulatory region.
What's the main reason you introduce this feature?
What are you trying to solve with this change?
+#define ATH10K_WMI_DFS_CONF_TIMEOUT_HZ (HZ / 2)
+static void ath10k_radar_confirmation_work(struct work_struct *work)
+{
+ struct ath10k *ar = container_of(work, struct ath10k,
+radar_confirmation_work);
+ struct ath10k_radar_found_info radar_info;
+ int ret, time_left;
+
+ reinit_completion(&ar->wmi.radar_confirm);
+
+ spin_lock_bh(&ar->data_lock);
+ memcpy(&radar_info, &ar->last_radar_info, sizeof(radar_info));
+ spin_unlock_bh(&ar->data_lock);
+
+ ret = ath10k_wmi_report_radar_found(ar, &radar_info);
+ if (ret) {
+ ath10k_warn(ar, "failed to send radar found %d\n", ret);
+ goto wait_complete;
+ }
+
+ time_left = wait_for_completion_timeout(&ar->wmi.radar_confirm,
+ ATH10K_WMI_DFS_CONF_TIMEOUT_HZ);
It looks wrong to me in terms of timeout value.
Typical channel closing time in FCC domain is 200ms (excluding control
signals), but you're waiting for 500ms for response from FW.
@@ -3765,25 +3834,46 @@ static void ath10k_dfs_radar_report(struct ath10k *ar,
ATH10K_DFS_STAT_INC(ar, pulses_detected);
- if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, NULL)) {
+ if (!ar->dfs_detector->add_pulse(ar->dfs_detector, &pe, &rs)) {
ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
"dfs no pulse pattern detected, yet\n");
return;
}
-radar_detected:
- ath10k_dbg(ar, ATH10K_DBG_REGULATORY, "dfs radar detected\n");
- ATH10K_DFS_STAT_INC(ar, radar_detected);
+ if ((test_bit(WMI_SERVICE_HOST_DFS_CHECK_SUPPORT, ar->wmi.svc_map)) &&
+ ar->dfs_detector->region == NL80211_DFS_FCC) {
I feel risky that host drivers have no way to control this new feature
and totally rely on FW feature mask. We should have a host drivers'
feature mask such as module param and set it false (don't use) by
default until it proves safe to use.
+static void
+ath10k_wmi_event_dfs_status_check(struct ath10k *ar, struct sk_buff *skb)
+{
+ struct wmi_dfs_status_ev_arg status_arg = {};
+ int ret;
+
+ ret = ath10k_wmi_pull_dfs_status(ar, skb, &status_arg);
+
+ if (ret) {
+ ath10k_warn(ar, "failed to parse dfs status event: %d\n", ret);
+ return;
+ }
+
+ ath10k_dbg(ar, ATH10K_DBG_REGULATORY,
+ "dfs status event received from fw: %d\n",
+ status_arg.status);
+
+ /* Even in case of radar detection failure we follow the same
+* behaviour as if radar is detected i.e to switch to a different
+* channel.
+*/
+ if (status_arg.status == WMI_HW_RADAR_DETECTED ||
+ status_arg.status == WMI_RADAR_DETECTION_FAIL)
+ ath10k_radar_detected(ar);
+ complete(&ar->wmi.radar_confirm);
+}
What is typical average duration from
wait_for_completion_timeout(&ar->wmi.radar_confirm) to this completion
called ?
Thanks,
Peter
[PATCH 2/2] ath10k: DFS Host Confirmation
In the 10.4-3.6 firmware branch there's a new DFS Host confirmation
feature which is advertised using WMI_SERVICE_HOST_DFS_CHECK_SUPPORT flag.
This new features enables the ath10k host to send information to the
firmware on the specifications of detected radar type. This allows the
firmware to validate if the host's radar pattern detector unit is
operational and check if the radar information shared by host matches
the radar pulses sent as phy error events from firmware. If the check
fails the firmware won't allow use of DFS channels on AP mode when using
FCC regulatory region.
Supported Chipsets : QCA9984/QCA9888/QCA4019
Firmware Version : 10.4-3.6-00104
Signed-off-by: Sriram R
---
drivers/net/wireless/ath/ath10k/core.h| 21
drivers/net/wireless/ath/ath10k/mac.c | 12 ++
drivers/net/wireless/ath/ath10k/wmi-ops.h | 32 ++
drivers/net/wireless/ath/ath10k/wmi.c | 185 --
drivers/net/wireless/ath/ath10k/wmi.h | 32 ++
5 files changed, 272 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index e4ac8f2..c95774e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -175,6 +175,7 @@ struct ath10k_wmi {
struct completion service_ready;
struct completion unified_ready;
struct completion barrier;
+ struct completion radar_confirm;
wait_queue_head_t tx_credits_wq;
DECLARE_BITMAP(svc_map, WMI_SERVICE_MAX);
struct wmi_cmd_map *cmd;
@@ -377,6 +378,21 @@ struct ath10k_dfs_stats {
u32 radar_detected;
};
+enum ath10k_radar_confirmation_state {
+ ATH10K_RADAR_CONFIRMATION_IDLE = 0,
+ ATH10K_RADAR_CONFIRMATION_INPROGRESS,
+ ATH10K_RADAR_CONFIRMATION_STOPPED,
+};
+
+struct ath10k_radar_found_info {
+ u32 pri_min;
+ u32 pri_max;
+ u32 width_min;
+ u32 width_max;
+ u32 sidx_min;
+ u32 sidx_max;
+};
+
#define ATH10K_MAX_NUM_PEER_IDS (1 << 11) /* htt rx_desc limit */
struct ath10k_peer {
@@ -1109,6 +1125,11 @@ struct ath10k {
u32 sta_tid_stats_mask;
+ /* protected by data_lock */
+ enum ath10k_radar_confirmation_state radar_conf_state;
+ struct ath10k_radar_found_info last_radar_info;
+ struct work_struct radar_confirmation_work;
+
/* must be last */
u8 drv_priv[0] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index 3d7119a..7463432 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3217,6 +3217,15 @@ static void ath10k_reg_notifier(struct wiphy *wiphy,
ar->hw->wiphy->bands[NL80211_BAND_5GHZ]);
}
+static void ath10k_stop_radar_confirmation(struct ath10k *ar)
+{
+ spin_lock_bh(&ar->data_lock);
+ ar->radar_conf_state = ATH10K_RADAR_CONFIRMATION_STOPPED;
+ spin_unlock_bh(&ar->data_lock);
+
+ cancel_work_sync(&ar->radar_confirmation_work);
+}
+
/***/
/* TX handlers */
/***/
@@ -4333,6 +4342,7 @@ void ath10k_halt(struct ath10k *ar)
ath10k_scan_finish(ar);
ath10k_peer_cleanup_all(ar);
+ ath10k_stop_radar_confirmation(ar);
ath10k_core_stop(ar);
ath10k_hif_power_down(ar);
@@ -4758,6 +4768,8 @@ static int ath10k_start(struct ieee80211_hw *hw)
ath10k_spectral_start(ar);
ath10k_thermal_set_throttling(ar);
+ ar->radar_conf_state = ATH10K_RADAR_CONFIRMATION_IDLE;
+
mutex_unlock(&ar->conf_mutex);
return 0;
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index e37d16b..5ecce04 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -55,6 +55,8 @@ struct wmi_ops {
struct wmi_wow_ev_arg *arg);
int (*pull_echo_ev)(struct ath10k *ar, struct sk_buff *skb,
struct wmi_echo_ev_arg *arg);
+ int (*pull_dfs_status_ev)(struct ath10k *ar, struct sk_buff *skb,
+ struct wmi_dfs_status_ev_arg *arg);
int (*pull_svc_avail)(struct ath10k *ar, struct sk_buff *skb,
struct wmi_svc_avail_ev_arg *arg);
@@ -188,6 +190,9 @@ struct wmi_ops {
const struct
wmi_tdls_peer_update_cmd_arg *arg,
const struct
wmi_tdls_peer_capab_arg *cap,
const struct wmi_channel_arg
*chan);
+ struct sk_buff *(*gen_radar_found)
+ (struct ath10k *ar,
+const struct ath10k_radar_found_info *arg);
struct sk_buff *(*gen_adaptive_qcs)(struct ath10k *ar, bool enable);
struct sk_buff *(*ge
