ath9k-devel.lists.ath9k.org archive mirror
 help / color / mirror / Atom feed
* [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently
@ 2016-11-21 14:04 Benjamin Berg
  2016-11-21 14:16 ` Michal Kazior
  2016-11-21 14:41 ` Zefir Kurtisi
  0 siblings, 2 replies; 5+ messages in thread
From: Benjamin Berg @ 2016-11-21 14:04 UTC (permalink / raw)
  To: ath9k-devel

In the case that a spectral scan is enabled the PHY errors sent by the
hardware as part of the scanning might trigger the radar detection and
channels might be marked as 'unusable' incorrectly. This patch fixes
the issue by preventing the spectral scan to be enabled if DFS is used
and only analysing the PHY errors for DFS if radar detection is enabled.

Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fit.fraunhofer.de>
Signed-off-by: Benjamin Berg <benjamin@sipsolutions.net>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 drivers/net/wireless/ath/ath9k/common-spectral.c | 23 +++++++++++++++++++----
 drivers/net/wireless/ath/ath9k/main.c            |  6 ++++++
 drivers/net/wireless/ath/ath9k/recv.c            |  7 +++++--
 3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/common-spectral.c b/drivers/net/wireless/ath/ath9k/common-spectral.c
index e2512d5..8444d6d 100644
--- a/drivers/net/wireless/ath/ath9k/common-spectral.c
+++ b/drivers/net/wireless/ath/ath9k/common-spectral.c
@@ -802,16 +802,27 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 					size_t count, loff_t *ppos)
 {
 	struct ath_spec_scan_priv *spec_priv = file->private_data;
+	struct ath_hw *ah = spec_priv->ah;
+	struct ath_softc *sc = ah->hw->priv;
 	struct ath_common *common = ath9k_hw_common(spec_priv->ah);
 	char buf[32];
 	ssize_t len;
+	ssize_t result = count;
 
 	if (IS_ENABLED(CONFIG_ATH9K_TX99))
 		return -EOPNOTSUPP;
 
+	mutex_lock(&sc->mutex);
+	if (ah->hw->conf.radar_enabled) {
+		result = -EINVAL;
+		goto unlock;
+	}
+
 	len = min(count, sizeof(buf) - 1);
-	if (copy_from_user(buf, user_buf, len))
-		return -EFAULT;
+	if (copy_from_user(buf, user_buf, len)) {
+		result = -EFAULT;
+		goto unlock;
+	}
 
 	buf[len] = '\0';
 
@@ -830,10 +841,14 @@ static ssize_t write_file_spec_scan_ctl(struct file *file,
 		ath9k_cmn_spectral_scan_config(common, spec_priv, SPECTRAL_DISABLED);
 		ath_dbg(common, CONFIG, "spectral scan: disabled\n");
 	} else {
-		return -EINVAL;
+		result = -EINVAL;
+		goto unlock;
 	}
 
-	return count;
+unlock:
+	mutex_unlock(&sc->mutex);
+
+	return result;
 }
 
 static const struct file_operations fops_spec_scan_ctl = {
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index e9f32b5..62b86fb 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1459,6 +1459,12 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed)
 	if (!ath9k_is_chanctx_enabled() && (changed & IEEE80211_CONF_CHANGE_CHANNEL)) {
 		ctx->offchannel = !!(conf->flags & IEEE80211_CONF_OFFCHANNEL);
 		ath_chanctx_set_channel(sc, ctx, &hw->conf.chandef);
+
+		/* We need to ensure that spectral scan is disabled. */
+		if (conf->radar_enabled &&
+		    sc->spec_priv.spectral_mode != SPECTRAL_DISABLED)
+			ath9k_cmn_spectral_scan_config(common, &sc->spec_priv,
+						       SPECTRAL_DISABLED);
 	}
 
 	mutex_unlock(&sc->mutex);
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..ce34add 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -867,8 +867,11 @@ static int ath9k_rx_skb_preprocess(struct ath_softc *sc,
 	 * can be dropped.
 	 */
 	if (rx_stats->rs_status & ATH9K_RXERR_PHY) {
-		ath9k_dfs_process_phyerr(sc, hdr, rx_stats, rx_status->mactime);
-		if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats, rx_status->mactime))
+		if (hw->conf.radar_enabled)
+			ath9k_dfs_process_phyerr(sc, hdr, rx_stats,
+						 rx_status->mactime);
+		else if (ath_cmn_process_fft(&sc->spec_priv, hdr, rx_stats,
+					     rx_status->mactime))
 			RX_STAT_INC(rx_spectral);
 
 		return -EINVAL;
-- 
2.10.2

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently
  2016-11-21 14:04 [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently Benjamin Berg
@ 2016-11-21 14:16 ` Michal Kazior
  2016-11-21 14:41 ` Zefir Kurtisi
  1 sibling, 0 replies; 5+ messages in thread
From: Michal Kazior @ 2016-11-21 14:16 UTC (permalink / raw)
  To: ath9k-devel

On 21 November 2016 at 15:04, Benjamin Berg <benjamin@sipsolutions.net> wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.

According to the comment in ath_cmn_process_fft() this doesn't seem to
be necessary for all chips:

 515         /* AR9280 and before report via ATH9K_PHYERR_RADAR,
AR93xx and newer
 516          * via ATH9K_PHYERR_SPECTRAL. Haven't seen
ATH9K_PHYERR_FALSE_RADAR_EXT
 517          * yet, but this is supposed to be possible as well.
 518          */
 519         if (rs->rs_phyerr != ATH9K_PHYERR_RADAR &&
 520             rs->rs_phyerr != ATH9K_PHYERR_FALSE_RADAR_EXT &&
 521             rs->rs_phyerr != ATH9K_PHYERR_SPECTRAL)
 522                 return 0;


Micha?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently
  2016-11-21 14:04 [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently Benjamin Berg
  2016-11-21 14:16 ` Michal Kazior
@ 2016-11-21 14:41 ` Zefir Kurtisi
  2016-11-21 15:10   ` Michal Kazior
  1 sibling, 1 reply; 5+ messages in thread
From: Zefir Kurtisi @ 2016-11-21 14:41 UTC (permalink / raw)
  To: ath9k-devel

On 11/21/2016 03:04 PM, Benjamin Berg wrote:
> In the case that a spectral scan is enabled the PHY errors sent by the
> hardware as part of the scanning might trigger the radar detection and
> channels might be marked as 'unusable' incorrectly. This patch fixes
> the issue by preventing the spectral scan to be enabled if DFS is used
> and only analysing the PHY errors for DFS if radar detection is enabled.
> 
> [...]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently
  2016-11-21 14:41 ` Zefir Kurtisi
@ 2016-11-21 15:10   ` Michal Kazior
  2016-11-21 16:16     ` Zefir Kurtisi
  0 siblings, 1 reply; 5+ messages in thread
From: Michal Kazior @ 2016-11-21 15:10 UTC (permalink / raw)
  To: ath9k-devel

On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>> In the case that a spectral scan is enabled the PHY errors sent by the
>> hardware as part of the scanning might trigger the radar detection and
>> channels might be marked as 'unusable' incorrectly. This patch fixes
>> the issue by preventing the spectral scan to be enabled if DFS is used
>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>
>> [...]
>
> From the relevant source code portion in channel.c:ath_set_channel()
>
> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
> 81          * scanning and radar detection can not be used concurrently.
> 82          */
> 83         if (hw->conf.radar_enabled) {
> 84                 u32 rxfilter;
> 85
> 86                 rxfilter = ath9k_hw_getrxfilter(ah);
> 87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
> 88                                 ATH9K_RX_FILTER_PHYERR;
> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
> 91                         chan->center_freq);
> 92         } else {
> 93                 /* perform spectral scan if requested. */
> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
> 95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
> 97         }
>
> it seems that spectral can't ever be activated while operating on a DFS channel.
>
> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
> into the pattern detector, you just need to ensure that they depend on
> hw->conf.radar_enabled. A patch like the attached one should be enough.

Good point. I guess set_channel could be oversimplified as well. I
mean, it makes sense to consider radar and spectral mutually exclusive
if they use the same phyerr code. However some chips actually seem (as
per the comment I mentioned) to distinguish the two so I don't know if
the "mutually exclusive" is true for all chips per se. Just thinking
out loud.

I also wonder if calling ieee80211_radar_detect() should have any
effect if there are no radar operated interfaces in the first place?


Micha?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently
  2016-11-21 15:10   ` Michal Kazior
@ 2016-11-21 16:16     ` Zefir Kurtisi
  0 siblings, 0 replies; 5+ messages in thread
From: Zefir Kurtisi @ 2016-11-21 16:16 UTC (permalink / raw)
  To: ath9k-devel

On 11/21/2016 04:10 PM, Michal Kazior wrote:
> On 21 November 2016 at 15:41, Zefir Kurtisi <zefir.kurtisi@neratec.com> wrote:
>> On 11/21/2016 03:04 PM, Benjamin Berg wrote:
>>> In the case that a spectral scan is enabled the PHY errors sent by the
>>> hardware as part of the scanning might trigger the radar detection and
>>> channels might be marked as 'unusable' incorrectly. This patch fixes
>>> the issue by preventing the spectral scan to be enabled if DFS is used
>>> and only analysing the PHY errors for DFS if radar detection is enabled.
>>>
>>> [...]
>>
>> From the relevant source code portion in channel.c:ath_set_channel()
>>
>> 80         /* Enable radar pulse detection if on a DFS channel. Spectral
>> 81          * scanning and radar detection can not be used concurrently.
>> 82          */
>> 83         if (hw->conf.radar_enabled) {
>> 84                 u32 rxfilter;
>> 85
>> 86                 rxfilter = ath9k_hw_getrxfilter(ah);
>> 87                 rxfilter |= ATH9K_RX_FILTER_PHYRADAR |
>> 88                                 ATH9K_RX_FILTER_PHYERR;
>> 89                 ath9k_hw_setrxfilter(ah, rxfilter);
>> 90                 ath_dbg(common, DFS, "DFS enabled at freq %d\n",
>> 91                         chan->center_freq);
>> 92         } else {
>> 93                 /* perform spectral scan if requested. */
>> 94                 if (test_bit(ATH_OP_SCANNING, &common->op_flags) &&
>> 95                         sc->spec_priv.spectral_mode == SPECTRAL_CHANSCAN)
>> 96                         ath9k_cmn_spectral_scan_trigger(common, &sc->spec_priv);
>> 97         }
>>
>> it seems that spectral can't ever be activated while operating on a DFS channel.
>>
>> If you need to catch the opposite case, i.e. prevent feeding pseudo-radar pulses
>> into the pattern detector, you just need to ensure that they depend on
>> hw->conf.radar_enabled. A patch like the attached one should be enough.
> 
> Good point. I guess set_channel could be oversimplified as well. I
> mean, it makes sense to consider radar and spectral mutually exclusive
> if they use the same phyerr code. However some chips actually seem (as
> per the comment I mentioned) to distinguish the two so I don't know if
> the "mutually exclusive" is true for all chips per se. Just thinking
> out loud.
> 
You're right, blindly feeding PHYERR frames to spectral when spectral is not
enabled is as wrong as feeding them to the dfs-detector when not on a radar
channel. Therefore, the patch I proposed should be extended to make calling
ath_cmn_process_fft() depending on
a) not operating on radar channel, and
b) spectral scan is enabled

Updated patch attached as proposal.

> I also wonder if calling ieee80211_radar_detect() should have any
> effect if there are no radar operated interfaces in the first place?
> 
True, calling ieee80211_radar_detect() for a non-DFS channel is (or should be)
ignored by mac. But feeding the dfs-detector with invalid pulses causes quite some
computational overhead - dropping them if no radar detection is required is a good
thing to do.


Cheers,
Zefir




-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ath9k-feed-DFS-detector-only-if-operating-on-radar-c.patch
Type: text/x-patch
Size: 0 bytes
Desc: not available
Url : http://lists.ath9k.org/pipermail/ath9k-devel/attachments/20161121/ab36ab4e/attachment-0001.bin 

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2016-11-21 16:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-21 14:04 [ath9k-devel] [PATCH] ath9k: Prevent radar detection and spectral scan to be used concurrently Benjamin Berg
2016-11-21 14:16 ` Michal Kazior
2016-11-21 14:41 ` Zefir Kurtisi
2016-11-21 15:10   ` Michal Kazior
2016-11-21 16:16     ` Zefir Kurtisi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).