[RFC 0/2] ath10k spectral scan support

[RFC 0/2] ath10k spectral scan support

[Simon Wunderlich]

This is the first patchset iteration for review to implement the ath10k
spectral scan feature. The patchset is still experimental, but we were
able to detect our analogue 5.8 GHz TV repeater in the sample data. :)

As for ath9k, a proof of concept data plotter has been implemented, and
is currently available for ath10k in its own branch of FFT_eval [1].

There are a few open questions, and any help or pointers would be
appreciated:

 * so far, we could only enable endless mode (samples are sent
   endlessly). To prevent relayfs buffers overflowing because they are
   not read from userspace, it would be great to set a "count" to limit
   the number of samples sent after a trigger. That was possible in
   ath9k as well. However, if we set the "scan_count" parameter in the
   WMI command to something else than 0 (default), we don't get any
   samples at all. Is there any other configuration which needs to be
   done to get a limited number of samples?
 * So far, we always get 64 bins of fft data in the samples, even if we
   are in HT40 or VHT80 mode. That doesn't seem right, as there should
   be more bins for wider channels (e.g. 128 or 256 bins). Is there any
   parameter to change in the configuration to get the output from
   multiple channels / higher bandwidth?

Kudos to Kalle and Michal for their help to get this started. :)

Cheers,
     Simon

[1] https://github.com/simonwunderlich/FFT_eval/tree/ath10k_wip

Sven Eckelmann (2):
  ath: Move spectral debugfs structs to shared header
  ath10k: add spectral scan feature

 drivers/net/wireless/ath/ath10k/Kconfig    |   1 +
 drivers/net/wireless/ath/ath10k/Makefile   |   3 +-
 drivers/net/wireless/ath/ath10k/core.h     |   6 +
 drivers/net/wireless/ath/ath10k/debug.c    |   3 +
 drivers/net/wireless/ath/ath10k/spectral.c | 368 +++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/spectral.h |  66 ++++++
 drivers/net/wireless/ath/ath10k/wmi.c      |  98 +++++++-
 drivers/net/wireless/ath/ath10k/wmi.h      |  80 +++++++
 drivers/net/wireless/ath/ath9k/spectral.h  |  71 +-----
 drivers/net/wireless/ath/spectral_common.h | 108 +++++++++
 10 files changed, 733 insertions(+), 71 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/spectral.c
 create mode 100644 drivers/net/wireless/ath/ath10k/spectral.h
 create mode 100644 drivers/net/wireless/ath/spectral_common.h

-- 
1.9.1

[RFC 1/2] ath: Move spectral debugfs structs to shared header

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

The ath9k and ath10k will share the definitions of the debugfs spectral
structures and enums. Having them in the same place helps to avoid conflicts.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 drivers/net/wireless/ath/ath9k/spectral.h  | 71 +-----------------------
 drivers/net/wireless/ath/spectral_common.h | 88 ++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+), 69 deletions(-)
 create mode 100644 drivers/net/wireless/ath/spectral_common.h

diff --git a/drivers/net/wireless/ath/ath9k/spectral.h b/drivers/net/wireless/ath/ath9k/spectral.h
index ead6341..7b410c6 100644
--- a/drivers/net/wireless/ath/ath9k/spectral.h
+++ b/drivers/net/wireless/ath/ath9k/spectral.h
@@ -17,6 +17,8 @@
 #ifndef SPECTRAL_H
 #define SPECTRAL_H
 
+#include "../spectral_common.h"
+
 /* enum spectral_mode:
  *
  * @SPECTRAL_DISABLED: spectral mode is disabled
@@ -54,8 +56,6 @@ struct ath_ht20_mag_info {
 	u8 max_exp;
 } __packed;
 
-#define SPECTRAL_HT20_NUM_BINS		56
-
 /* WARNING: don't actually use this struct! MAC may vary the amount of
  * data by -1/+2. This struct is for reference only.
  */
@@ -83,8 +83,6 @@ struct ath_ht20_40_mag_info {
 	u8 max_exp;
 } __packed;
 
-#define SPECTRAL_HT20_40_NUM_BINS		128
-
 /* WARNING: don't actually use this struct! MAC may vary the amount of
  * data. This struct is for reference only.
  */
@@ -125,71 +123,6 @@ static inline u8 spectral_bitmap_weight(u8 *bins)
 	return bins[0] & 0x3f;
 }
 
-/* FFT sample format given to userspace via debugfs.
- *
- * Please keep the type/length at the front position and change
- * other fields after adding another sample type
- *
- * TODO: this might need rework when switching to nl80211-based
- * interface.
- */
-enum ath_fft_sample_type {
-	ATH_FFT_SAMPLE_HT20 = 1,
-	ATH_FFT_SAMPLE_HT20_40,
-};
-
-struct fft_sample_tlv {
-	u8 type;	/* see ath_fft_sample */
-	__be16 length;
-	/* type dependent data follows */
-} __packed;
-
-struct fft_sample_ht20 {
-	struct fft_sample_tlv tlv;
-
-	u8 max_exp;
-
-	__be16 freq;
-	s8 rssi;
-	s8 noise;
-
-	__be16 max_magnitude;
-	u8 max_index;
-	u8 bitmap_weight;
-
-	__be64 tsf;
-
-	u8 data[SPECTRAL_HT20_NUM_BINS];
-} __packed;
-
-struct fft_sample_ht20_40 {
-	struct fft_sample_tlv tlv;
-
-	u8 channel_type;
-	__be16 freq;
-
-	s8 lower_rssi;
-	s8 upper_rssi;
-
-	__be64 tsf;
-
-	s8 lower_noise;
-	s8 upper_noise;
-
-	__be16 lower_max_magnitude;
-	__be16 upper_max_magnitude;
-
-	u8 lower_max_index;
-	u8 upper_max_index;
-
-	u8 lower_bitmap_weight;
-	u8 upper_bitmap_weight;
-
-	u8 max_exp;
-
-	u8 data[SPECTRAL_HT20_40_NUM_BINS];
-} __packed;
-
 void ath9k_spectral_init_debug(struct ath_softc *sc);
 void ath9k_spectral_deinit_debug(struct ath_softc *sc);
 
diff --git a/drivers/net/wireless/ath/spectral_common.h b/drivers/net/wireless/ath/spectral_common.h
new file mode 100644
index 0000000..b9ab722
--- /dev/null
+++ b/drivers/net/wireless/ath/spectral_common.h
@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2013 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef SPECTRAL_COMMON_H
+#define SPECTRAL_COMMON_H
+
+#define SPECTRAL_HT20_NUM_BINS		56
+#define SPECTRAL_HT20_40_NUM_BINS		128
+
+/* FFT sample format given to userspace via debugfs.
+ *
+ * Please keep the type/length at the front position and change
+ * other fields after adding another sample type
+ *
+ * TODO: this might need rework when switching to nl80211-based
+ * interface.
+ */
+enum ath_fft_sample_type {
+	ATH_FFT_SAMPLE_HT20 = 1,
+	ATH_FFT_SAMPLE_HT20_40,
+};
+
+struct fft_sample_tlv {
+	u8 type;	/* see ath_fft_sample */
+	__be16 length;
+	/* type dependent data follows */
+} __packed;
+
+struct fft_sample_ht20 {
+	struct fft_sample_tlv tlv;
+
+	u8 max_exp;
+
+	__be16 freq;
+	s8 rssi;
+	s8 noise;
+
+	__be16 max_magnitude;
+	u8 max_index;
+	u8 bitmap_weight;
+
+	__be64 tsf;
+
+	u8 data[SPECTRAL_HT20_NUM_BINS];
+} __packed;
+
+struct fft_sample_ht20_40 {
+	struct fft_sample_tlv tlv;
+
+	u8 channel_type;
+	__be16 freq;
+
+	s8 lower_rssi;
+	s8 upper_rssi;
+
+	__be64 tsf;
+
+	s8 lower_noise;
+	s8 upper_noise;
+
+	__be16 lower_max_magnitude;
+	__be16 upper_max_magnitude;
+
+	u8 lower_max_index;
+	u8 upper_max_index;
+
+	u8 lower_bitmap_weight;
+	u8 upper_bitmap_weight;
+
+	u8 max_exp;
+
+	u8 data[SPECTRAL_HT20_40_NUM_BINS];
+} __packed;
+
+#endif /* SPECTRAL_COMMON_H */
-- 
1.9.1

[RFC 2/2] ath10k: add spectral scan feature

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

Adds the spectral scan feature for ath10k. The spectral scan is triggered by
configuring a mode through a debugfs control file. Samples can be gathered via
another relay debugfs file.

Essentially, to try it out:

ip link dev wlan0 set up
echo background > /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan_ctl
iw dev wlan0 scan
echo disable > /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan_ctl
cat /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan0 > samples

This feature is still experimental.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
---
 drivers/net/wireless/ath/ath10k/Kconfig    |   1 +
 drivers/net/wireless/ath/ath10k/Makefile   |   3 +-
 drivers/net/wireless/ath/ath10k/core.h     |   6 +
 drivers/net/wireless/ath/ath10k/debug.c    |   3 +
 drivers/net/wireless/ath/ath10k/spectral.c | 368 +++++++++++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/spectral.h |  66 ++++++
 drivers/net/wireless/ath/ath10k/wmi.c      |  98 +++++++-
 drivers/net/wireless/ath/ath10k/wmi.h      |  80 +++++++
 drivers/net/wireless/ath/spectral_common.h |  20 ++
 9 files changed, 643 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/wireless/ath/ath10k/spectral.c
 create mode 100644 drivers/net/wireless/ath/ath10k/spectral.h

diff --git a/drivers/net/wireless/ath/ath10k/Kconfig b/drivers/net/wireless/ath/ath10k/Kconfig
index a6f5285..1053bb5 100644
--- a/drivers/net/wireless/ath/ath10k/Kconfig
+++ b/drivers/net/wireless/ath/ath10k/Kconfig
@@ -25,6 +25,7 @@ config ATH10K_DEBUG
 config ATH10K_DEBUGFS
 	bool "Atheros ath10k debugfs support"
 	depends on ATH10K
+	select RELAY
 	---help---
 	  Enabled debugfs support
 
diff --git a/drivers/net/wireless/ath/ath10k/Makefile b/drivers/net/wireless/ath/ath10k/Makefile
index a4179f4..2a1e9a3 100644
--- a/drivers/net/wireless/ath/ath10k/Makefile
+++ b/drivers/net/wireless/ath/ath10k/Makefile
@@ -8,7 +8,8 @@ ath10k_core-y += mac.o \
 		 htt_tx.o \
 		 txrx.o \
 		 wmi.o \
-		 bmi.o
+		 bmi.o \
+		 spectral.o
 
 ath10k_core-$(CONFIG_ATH10K_TRACING) += trace.o
 
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 83a5fa9..4af7f59 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -31,6 +31,7 @@
 #include "../ath.h"
 #include "../regd.h"
 #include "../dfs_pattern_detector.h"
+#include "spectral.h"
 
 #define MS(_v, _f) (((_v) & _f##_MASK) >> _f##_LSB)
 #define SM(_v, _f) (((_v) << _f##_LSB) & _f##_MASK)
@@ -494,6 +495,11 @@ struct ath10k {
 #ifdef CONFIG_ATH10K_DEBUGFS
 	struct ath10k_debug debug;
 #endif
+
+	/* relay(fs) channel for spectral scan */
+	struct rchan *rfs_chan_spec_scan;
+	enum spectral_mode spectral_mode;
+	struct ath_spec_scan spec_config;
 };
 
 struct ath10k *ath10k_core_create(void *hif_priv, struct device *dev,
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 3030158..42cb448 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -957,12 +957,15 @@ int ath10k_debug_create(struct ath10k *ar)
 				    &fops_dfs_stats);
 	}
 
+	ath10k_spectral_init_debug(ar);
+
 	return 0;
 }
 
 void ath10k_debug_destroy(struct ath10k *ar)
 {
 	cancel_delayed_work_sync(&ar->debug.htt_stats_dwork);
+	ath10k_spectral_deinit_debug(ar);
 }
 
 #endif /* CONFIG_ATH10K_DEBUGFS */
diff --git a/drivers/net/wireless/ath/ath10k/spectral.c b/drivers/net/wireless/ath/ath10k/spectral.c
new file mode 100644
index 0000000..20f7037
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/spectral.c
@@ -0,0 +1,368 @@
+/*
+ * Copyright (c) 2013 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include <linux/relay.h>
+#include "core.h"
+
+static void ath_debug_send_fft_sample(struct ath10k *ar,
+				      struct fft_sample_tlv *fft_sample_tlv)
+{
+	int length;
+
+	if (!ar->rfs_chan_spec_scan)
+		return;
+
+	length = __be16_to_cpu(fft_sample_tlv->length) +
+		 sizeof(*fft_sample_tlv);
+	relay_write(ar->rfs_chan_spec_scan, fft_sample_tlv, length);
+}
+
+int ath10k_process_fft(struct ath10k *ar,
+		       struct wmi_single_phyerr_rx_event *event,
+		       struct phyerr_fft_report *fftr, size_t bin_len, u64 tsf)
+{
+	struct fft_sample_ath10k_ht20 fft_sample;
+	u32 reg0, reg1;
+	u16 peak_mag;
+	u8 *bins;
+	u16 freq1;
+	u16 freq2;
+	u16 total_gain_db;
+	u16 base_pwr_db;
+	u8 chain_idx;
+	u32 nf_list1;
+	u32 nf_list2;
+	u16 length;
+
+	if (bin_len != SPECTRAL_ATH10K_HT20_NUM_BINS)
+		return 0;
+
+	reg0 = __le32_to_cpu(fftr->reg0);
+	reg1 = __le32_to_cpu(fftr->reg1);
+
+	length = sizeof(fft_sample) - sizeof(struct fft_sample_tlv);
+	fft_sample.tlv.type = ATH_FFT_SAMPLE_ATH10K_HT20;
+	fft_sample.tlv.length = __cpu_to_be16(length);
+
+	fft_sample.mhz = event->hdr.chan_width_mhz;
+	fft_sample.relpwr_db = MS(reg1, SEARCH_FFT_REPORT_REG1_RELPWR_DB);
+	fft_sample.avgpwr_db = MS(reg1, SEARCH_FFT_REPORT_REG1_AVGPWR_DB);
+
+	peak_mag = MS(reg1, SEARCH_FFT_REPORT_REG1_PEAK_MAG);
+	fft_sample.max_magnitude = __cpu_to_be16(peak_mag);
+	fft_sample.max_index = MS(reg0, SEARCH_FFT_REPORT_REG0_PEAK_SIDX);
+	fft_sample.rssi = event->hdr.rssi_combined;
+
+	total_gain_db = MS(reg0, SEARCH_FFT_REPORT_REG0_TOTAL_GAIN_DB);
+	base_pwr_db = MS(reg0, SEARCH_FFT_REPORT_REG0_BASE_PWR_DB);
+	fft_sample.total_gain_db = __cpu_to_be16(total_gain_db);
+	fft_sample.base_pwr_db = __cpu_to_be16(base_pwr_db);
+
+	freq1 = __le16_to_cpu(event->hdr.freq1);
+	freq2 = __le16_to_cpu(event->hdr.freq2);
+	fft_sample.freq1 = __cpu_to_be16(freq1);
+	fft_sample.freq2 = __cpu_to_be16(freq2);
+
+	nf_list1 = __le32_to_cpu(event->hdr.nf_list_1);
+	nf_list2 = __le32_to_cpu(event->hdr.nf_list_2);
+	chain_idx = MS(reg0, SEARCH_FFT_REPORT_REG0_FFT_CHN_IDX);
+	switch (chain_idx) {
+	case 0:
+		fft_sample.noise = __cpu_to_be16(nf_list1 & 0xffffu);
+		break;
+	case 1:
+		fft_sample.noise = __cpu_to_be16((nf_list1 >> 16) & 0xffffu);
+		break;
+	case 2:
+		fft_sample.noise = __cpu_to_be16(nf_list2 & 0xffffu);
+		break;
+	case 3:
+		fft_sample.noise = __cpu_to_be16((nf_list2 >> 16) & 0xffffu);
+		break;
+	}
+
+	fft_sample.tsf = __cpu_to_be64(tsf);
+
+	bins = (u8 *)fftr;
+	bins += sizeof(*fftr);
+	memcpy(fft_sample.data, bins, SPECTRAL_ATH10K_HT20_NUM_BINS);
+
+	ath_debug_send_fft_sample(ar, &fft_sample.tlv);
+
+	return 1;
+}
+
+static int ath_get_spectral_vdevid(struct ath10k *ar)
+{
+	struct ath10k_vif *arvif;
+
+	if (list_empty(&ar->arvifs))
+		return -ENODEV;
+
+	arvif = list_first_entry(&ar->arvifs, typeof(*arvif), list);
+	return arvif->vdev_id;
+}
+
+int ath10k_spectral_scan_trigger(struct ath10k *ar)
+{
+	int ret;
+	int vdev_id;
+
+	vdev_id = ath_get_spectral_vdevid(ar);
+	if (vdev_id < 0)
+		return vdev_id;
+
+	if (ar->spectral_mode == SPECTRAL_DISABLED)
+		return 0;
+
+	ret = ath10k_vdev_spectral_enable(ar, vdev_id,
+					  WMI_SPECTRAL_TRIGGER_CMD_CLEAR,
+					  WMI_SPECTRAL_ENABLE_CMD_ENABLE);
+	if (ret < 0)
+		return ret;
+
+	ret = ath10k_vdev_spectral_enable(ar, vdev_id,
+					  WMI_SPECTRAL_TRIGGER_CMD_TRIGGER,
+					  WMI_SPECTRAL_ENABLE_CMD_ENABLE);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int ath10k_spectral_scan_config(struct ath10k *ar, enum spectral_mode mode)
+{
+	int count, ret;
+	struct wmi_vdev_spectral_configure_arg arg;
+	int vdev_id;
+
+	vdev_id = ath_get_spectral_vdevid(ar);
+	if (vdev_id < 0)
+		return vdev_id;
+
+	ar->spectral_mode = mode;
+
+	ath10k_vdev_spectral_enable(ar, vdev_id,
+				    WMI_SPECTRAL_TRIGGER_CMD_CLEAR,
+				    WMI_SPECTRAL_ENABLE_CMD_DISABLE);
+	if (mode == SPECTRAL_DISABLED)
+		return 0;
+
+	if (mode == SPECTRAL_BACKGROUND)
+		count = WMI_SPECTRAL_COUNT_DEFAULT;
+	else
+		count = max_t(u8, 1, ar->spec_config.count);
+
+	arg.vdev_id              = vdev_id;
+	arg.scan_count           = count;
+	arg.scan_period          = WMI_SPECTRAL_PERIOD_DEFAULT;
+	arg.scan_priority        = WMI_SPECTRAL_PRIORITY_DEFAULT;
+	arg.scan_fft_size        = WMI_SPECTRAL_FFT_SIZE_DEFAULT;
+	arg.scan_gc_ena          = WMI_SPECTRAL_GC_ENA_DEFAULT;
+	arg.scan_restart_ena     = WMI_SPECTRAL_RESTART_ENA_DEFAULT;
+	arg.scan_noise_floor_ref = WMI_SPECTRAL_NOISE_FLOOR_REF_DEFAULT;
+	arg.scan_init_delay      = WMI_SPECTRAL_INIT_DELAY_DEFAULT;
+	arg.scan_nb_tone_thr     = WMI_SPECTRAL_NB_TONE_THR_DEFAULT;
+	arg.scan_str_bin_thr     = WMI_SPECTRAL_STR_BIN_THR_DEFAULT;
+	arg.scan_wb_rpt_mode     = WMI_SPECTRAL_WB_RPT_MODE_DEFAULT;
+	arg.scan_rssi_rpt_mode   = WMI_SPECTRAL_RSSI_RPT_MODE_DEFAULT;
+	arg.scan_rssi_thr        = WMI_SPECTRAL_RSSI_THR_DEFAULT;
+	arg.scan_pwr_format      = WMI_SPECTRAL_PWR_FORMAT_DEFAULT;
+	arg.scan_rpt_mode        = WMI_SPECTRAL_RPT_MODE_DEFAULT;
+	arg.scan_bin_scale       = WMI_SPECTRAL_BIN_SCALE_DEFAULT;
+	arg.scan_dBm_adj         = WMI_SPECTRAL_DBM_ADJ_DEFAULT;
+	arg.scan_chn_mask        = WMI_SPECTRAL_CHN_MASK_DEFAULT;
+
+	ret = ath10k_vdev_spectral_configure(ar, &arg);
+	if (ret < 0)
+		return ret;
+
+	if (mode == SPECTRAL_BACKGROUND)
+		ath10k_spectral_scan_trigger(ar);
+
+	return 0;
+}
+
+/*********************/
+/* spectral_scan_ctl */
+/*********************/
+
+static ssize_t read_file_spec_scan_ctl(struct file *file, char __user *user_buf,
+				       size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char *mode = "";
+	unsigned int len;
+
+	switch (ar->spectral_mode) {
+	case SPECTRAL_DISABLED:
+		mode = "disable";
+		break;
+	case SPECTRAL_BACKGROUND:
+		mode = "background";
+		break;
+	case SPECTRAL_MANUAL:
+		mode = "manual";
+		break;
+	}
+
+	len = strlen(mode);
+	return simple_read_from_buffer(user_buf, count, ppos, mode, len);
+}
+
+static ssize_t write_file_spec_scan_ctl(struct file *file,
+					const char __user *user_buf,
+					size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char buf[32];
+	ssize_t len;
+
+	len = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, len))
+		return -EFAULT;
+
+	buf[len] = '\0';
+
+	if (strncmp("trigger", buf, 7) == 0) {
+		if (ar->spectral_mode == SPECTRAL_MANUAL)
+			ath10k_spectral_scan_trigger(ar);
+	} else if (strncmp("background", buf, 9) == 0) {
+		ath10k_spectral_scan_config(ar, SPECTRAL_BACKGROUND);
+	} else if (strncmp("manual", buf, 6) == 0) {
+		ath10k_spectral_scan_config(ar, SPECTRAL_MANUAL);
+	} else if (strncmp("disable", buf, 7) == 0) {
+		ath10k_spectral_scan_config(ar, SPECTRAL_DISABLED);
+	} else {
+		return -EINVAL;
+	}
+
+	return count;
+}
+
+static const struct file_operations fops_spec_scan_ctl = {
+	.read = read_file_spec_scan_ctl,
+	.write = write_file_spec_scan_ctl,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
+/******************/
+/* spectral_count */
+/******************/
+
+static ssize_t read_file_spectral_count(struct file *file,
+					char __user *user_buf,
+					size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	char buf[32];
+	unsigned int len;
+
+	len = sprintf(buf, "%d\n", ar->spec_config.count);
+	return simple_read_from_buffer(user_buf, count, ppos, buf, len);
+}
+
+static ssize_t write_file_spectral_count(struct file *file,
+					 const char __user *user_buf,
+					 size_t count, loff_t *ppos)
+{
+	struct ath10k *ar = file->private_data;
+	unsigned long val;
+	char buf[32];
+	ssize_t len;
+
+	len = min(count, sizeof(buf) - 1);
+	if (copy_from_user(buf, user_buf, len))
+		return -EFAULT;
+
+	buf[len] = '\0';
+	if (kstrtoul(buf, 0, &val))
+		return -EINVAL;
+
+	if (val < 0 || val > 255)
+		return -EINVAL;
+
+	ar->spec_config.count = val;
+	return count;
+}
+
+static const struct file_operations fops_spectral_count = {
+	.read = read_file_spectral_count,
+	.write = write_file_spectral_count,
+	.open = simple_open,
+	.owner = THIS_MODULE,
+	.llseek = default_llseek,
+};
+
+/*******************/
+/* Relay interface */
+/*******************/
+
+static struct dentry *create_buf_file_handler(const char *filename,
+					      struct dentry *parent,
+					      umode_t mode,
+					      struct rchan_buf *buf,
+					      int *is_global)
+{
+	struct dentry *buf_file;
+
+	buf_file = debugfs_create_file(filename, mode, parent, buf,
+				       &relay_file_operations);
+	*is_global = 1;
+	return buf_file;
+}
+
+static int remove_buf_file_handler(struct dentry *dentry)
+{
+	debugfs_remove(dentry);
+
+	return 0;
+}
+
+static struct rchan_callbacks rfs_spec_scan_cb = {
+	.create_buf_file = create_buf_file_handler,
+	.remove_buf_file = remove_buf_file_handler,
+};
+
+/*********************/
+/* Debug Init/Deinit */
+/*********************/
+
+void ath10k_spectral_deinit_debug(struct ath10k *ar)
+{
+	if (config_enabled(CONFIG_ATH10K_DEBUGFS) && ar->rfs_chan_spec_scan) {
+		relay_close(ar->rfs_chan_spec_scan);
+		ar->rfs_chan_spec_scan = NULL;
+	}
+}
+
+void ath10k_spectral_init_debug(struct ath10k *ar)
+{
+	ar->rfs_chan_spec_scan = relay_open("spectral_scan",
+					    ar->debug.debugfs_phy,
+					    1024, 256, &rfs_spec_scan_cb,
+					    NULL);
+	debugfs_create_file("spectral_scan_ctl",
+			    S_IRUSR | S_IWUSR,
+			    ar->debug.debugfs_phy, ar,
+			    &fops_spec_scan_ctl);
+	debugfs_create_file("spectral_count",
+			    S_IRUSR | S_IWUSR,
+			    ar->debug.debugfs_phy, ar,
+			    &fops_spectral_count);
+}
diff --git a/drivers/net/wireless/ath/ath10k/spectral.h b/drivers/net/wireless/ath/ath10k/spectral.h
new file mode 100644
index 0000000..288672e
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/spectral.h
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2013 Qualcomm Atheros, Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#ifndef SPECTRAL_H
+#define SPECTRAL_H
+
+#include "../spectral_common.h"
+
+/**
+ * struct ath_spec_scan - parameters for Atheros spectral scan
+ *
+ * @count: number of scan results requested for manual mode
+ */
+struct ath_spec_scan {
+	u8 count;
+};
+
+/* enum spectral_mode:
+ *
+ * @SPECTRAL_DISABLED: spectral mode is disabled
+ * @SPECTRAL_BACKGROUND: hardware sends samples when it is not busy with
+ *	something else.
+ * @SPECTRAL_MANUAL: spectral scan is enabled, triggering for samples
+ *	is performed manually.
+ */
+enum spectral_mode {
+	SPECTRAL_DISABLED = 0,
+	SPECTRAL_BACKGROUND,
+	SPECTRAL_MANUAL,
+};
+
+void ath10k_spectral_init_debug(struct ath10k *ar);
+void ath10k_spectral_deinit_debug(struct ath10k *ar);
+
+int ath10k_spectral_scan_trigger(struct ath10k *ar);
+int ath10k_spectral_scan_config(struct ath10k *ar,
+				enum spectral_mode spectral_mode);
+
+#ifdef CONFIG_ATH10K_DEBUGFS
+int ath10k_process_fft(struct ath10k *ar,
+		       struct wmi_single_phyerr_rx_event *event,
+		       struct phyerr_fft_report *fftr, size_t bin_len, u64 tsf);
+#else
+static inline int
+ath10k_process_fft(struct ath10k *ar,
+		   struct wmi_single_phyerr_rx_event *event,
+		   struct phyerr_fft_report *fftr, size_t bin_len, u64 tsf)
+{
+	return 0;
+}
+#endif /* CONFIG_ATH10K_DEBUGFS */
+
+#endif /* SPECTRAL_H */
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 6f83cae..7ad9c71 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1659,7 +1659,47 @@ static void ath10k_wmi_event_spectral_scan(struct ath10k *ar,
 				struct wmi_single_phyerr_rx_event *event,
 				u64 tsf)
 {
-	ath10k_dbg(ATH10K_DBG_WMI, "wmi event spectral scan\n");
+	int buf_len, tlv_len, res, i = 0;
+	struct phyerr_tlv *tlv;
+	u8 *tlv_buf;
+	struct phyerr_fft_report *fftr;
+	size_t fftr_len;
+
+	buf_len = __le32_to_cpu(event->hdr.buf_len);
+
+	while (i < buf_len) {
+		if (i + sizeof(*tlv) > buf_len) {
+			ath10k_warn("too short buf for tlv header (%d)\n", i);
+			return;
+		}
+
+		tlv = (struct phyerr_tlv *)&event->bufp[i];
+		tlv_len = __le16_to_cpu(tlv->len);
+		tlv_buf = &event->bufp[i + sizeof(*tlv)];
+
+		if (i + sizeof(*tlv) + tlv_len > buf_len) {
+			ath10k_warn("too short buf for tlv (%d)\n", i);
+			return;
+		}
+
+		switch (tlv->tag) {
+		case PHYERR_TLV_TAG_SEARCH_FFT_REPORT:
+			if (sizeof(*fftr) > tlv_len) {
+				ath10k_warn("too short fft report (%d)\n", i);
+				return;
+			}
+
+			fftr_len = tlv_len - sizeof(*fftr);
+			fftr = (struct phyerr_fft_report *)tlv_buf;
+			res = ath10k_process_fft(ar, event, fftr, fftr_len,
+						 tsf);
+			if (res)
+				return;
+			break;
+		}
+
+		i += sizeof(*tlv) + tlv_len;
+	}
 }
 
 static void ath10k_wmi_event_phyerr(struct ath10k *ar, struct sk_buff *skb)
@@ -3236,6 +3276,62 @@ int ath10k_wmi_vdev_install_key(struct ath10k *ar,
 				   ar->wmi.cmd->vdev_install_key_cmdid);
 }
 
+int ath10k_vdev_spectral_configure(struct ath10k *ar,
+			      const struct wmi_vdev_spectral_configure_arg *arg)
+{
+	struct wmi_vdev_spectral_configure_cmd *cmd;
+	struct sk_buff *skb;
+	__le32 cmdid;
+
+	skb = ath10k_wmi_alloc_skb(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct wmi_vdev_spectral_configure_cmd *)skb->data;
+	cmd->vdev_id              = __cpu_to_le32(arg->vdev_id);
+	cmd->scan_count           = __cpu_to_le32(arg->scan_count);
+	cmd->scan_period          = __cpu_to_le32(arg->scan_period);
+	cmd->scan_priority        = __cpu_to_le32(arg->scan_priority);
+	cmd->scan_fft_size        = __cpu_to_le32(arg->scan_fft_size);
+	cmd->scan_gc_ena          = __cpu_to_le32(arg->scan_gc_ena);
+	cmd->scan_restart_ena     = __cpu_to_le32(arg->scan_restart_ena);
+	cmd->scan_noise_floor_ref = __cpu_to_le32(arg->scan_noise_floor_ref);
+	cmd->scan_init_delay      = __cpu_to_le32(arg->scan_init_delay);
+	cmd->scan_nb_tone_thr     = __cpu_to_le32(arg->scan_nb_tone_thr);
+	cmd->scan_str_bin_thr     = __cpu_to_le32(arg->scan_str_bin_thr);
+	cmd->scan_wb_rpt_mode     = __cpu_to_le32(arg->scan_wb_rpt_mode);
+	cmd->scan_rssi_rpt_mode   = __cpu_to_le32(arg->scan_rssi_rpt_mode);
+	cmd->scan_rssi_thr        = __cpu_to_le32(arg->scan_rssi_thr);
+	cmd->scan_pwr_format      = __cpu_to_le32(arg->scan_pwr_format);
+	cmd->scan_rpt_mode        = __cpu_to_le32(arg->scan_rpt_mode);
+	cmd->scan_bin_scale       = __cpu_to_le32(arg->scan_bin_scale);
+	cmd->scan_dBm_adj         = __cpu_to_le32(arg->scan_dBm_adj);
+	cmd->scan_chn_mask        = __cpu_to_le32(arg->scan_chn_mask);
+
+	cmdid = ar->wmi.cmd->vdev_spectral_scan_configure_cmdid;
+	return ath10k_wmi_cmd_send(ar, skb, cmdid);
+}
+
+int ath10k_vdev_spectral_enable(struct ath10k *ar, u32 vdev_id, u32 trigger,
+				u32 enable)
+{
+	struct wmi_vdev_spectral_enable_cmd *cmd;
+	struct sk_buff *skb;
+	__le32 cmdid;
+
+	skb = ath10k_wmi_alloc_skb(sizeof(*cmd));
+	if (!skb)
+		return -ENOMEM;
+
+	cmd = (struct wmi_vdev_spectral_enable_cmd *)skb->data;
+	cmd->vdev_id     = __cpu_to_le32(vdev_id);
+	cmd->trigger_cmd = __cpu_to_le32(trigger);
+	cmd->enable_cmd  = __cpu_to_le32(enable);
+
+	cmdid = ar->wmi.cmd->vdev_spectral_scan_enable_cmdid;
+	return ath10k_wmi_cmd_send(ar, skb, cmdid);
+}
+
 int ath10k_wmi_peer_create(struct ath10k *ar, u32 vdev_id,
 			   const u8 peer_addr[ETH_ALEN])
 {
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index e93df2c..1020851 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -2067,6 +2067,7 @@ struct wmi_comb_phyerr_rx_event {
 #define PHYERR_TLV_SIG				0xBB
 #define PHYERR_TLV_TAG_SEARCH_FFT_REPORT	0xFB
 #define PHYERR_TLV_TAG_RADAR_PULSE_SUMMARY	0xF8
+#define PHYERR_TLV_TAG_SPECTRAL_SUMMARY_REPORT	0xF9
 
 struct phyerr_radar_report {
 	__le32 reg0; /* RADAR_REPORT_REG0_* */
@@ -3444,6 +3445,81 @@ struct wmi_vdev_simple_event {
 /* unsupported VDEV combination */
 #define WMI_INIFIED_VDEV_START_RESPONSE_NOT_SUPPORTED	0x2
 
+struct wmi_vdev_spectral_configure_cmd {
+	__le32 vdev_id;
+	__le32 scan_count;
+	__le32 scan_period;
+	__le32 scan_priority;
+	__le32 scan_fft_size;
+	__le32 scan_gc_ena;
+	__le32 scan_restart_ena;
+	__le32 scan_noise_floor_ref;
+	__le32 scan_init_delay;
+	__le32 scan_nb_tone_thr;
+	__le32 scan_str_bin_thr;
+	__le32 scan_wb_rpt_mode;
+	__le32 scan_rssi_rpt_mode;
+	__le32 scan_rssi_thr;
+	__le32 scan_pwr_format;
+	__le32 scan_rpt_mode;
+	__le32 scan_bin_scale;
+	__le32 scan_dBm_adj;
+	__le32 scan_chn_mask;
+} __packed;
+
+struct wmi_vdev_spectral_configure_arg {
+	u32 vdev_id;
+	u32 scan_count;
+	u32 scan_period;
+	u32 scan_priority;
+	u32 scan_fft_size;
+	u32 scan_gc_ena;
+	u32 scan_restart_ena;
+	u32 scan_noise_floor_ref;
+	u32 scan_init_delay;
+	u32 scan_nb_tone_thr;
+	u32 scan_str_bin_thr;
+	u32 scan_wb_rpt_mode;
+	u32 scan_rssi_rpt_mode;
+	u32 scan_rssi_thr;
+	u32 scan_pwr_format;
+	u32 scan_rpt_mode;
+	u32 scan_bin_scale;
+	u32 scan_dBm_adj;
+	u32 scan_chn_mask;
+};
+
+#define WMI_SPECTRAL_ENABLE_DEFAULT              0
+#define WMI_SPECTRAL_COUNT_DEFAULT               0
+#define WMI_SPECTRAL_PERIOD_DEFAULT             35
+#define WMI_SPECTRAL_PRIORITY_DEFAULT            1
+#define WMI_SPECTRAL_FFT_SIZE_DEFAULT            7
+#define WMI_SPECTRAL_GC_ENA_DEFAULT              1
+#define WMI_SPECTRAL_RESTART_ENA_DEFAULT         0
+#define WMI_SPECTRAL_NOISE_FLOOR_REF_DEFAULT   -96
+#define WMI_SPECTRAL_INIT_DELAY_DEFAULT         80
+#define WMI_SPECTRAL_NB_TONE_THR_DEFAULT        12
+#define WMI_SPECTRAL_STR_BIN_THR_DEFAULT         8
+#define WMI_SPECTRAL_WB_RPT_MODE_DEFAULT         0
+#define WMI_SPECTRAL_RSSI_RPT_MODE_DEFAULT       0
+#define WMI_SPECTRAL_RSSI_THR_DEFAULT         0xf0
+#define WMI_SPECTRAL_PWR_FORMAT_DEFAULT          0
+#define WMI_SPECTRAL_RPT_MODE_DEFAULT            2
+#define WMI_SPECTRAL_BIN_SCALE_DEFAULT           1
+#define WMI_SPECTRAL_DBM_ADJ_DEFAULT             1
+#define WMI_SPECTRAL_CHN_MASK_DEFAULT            1
+
+struct wmi_vdev_spectral_enable_cmd {
+	__le32 vdev_id;
+	__le32 trigger_cmd;
+	__le32 enable_cmd;
+} __packed;
+
+#define WMI_SPECTRAL_TRIGGER_CMD_TRIGGER  1
+#define WMI_SPECTRAL_TRIGGER_CMD_CLEAR    2
+#define WMI_SPECTRAL_ENABLE_CMD_ENABLE    1
+#define WMI_SPECTRAL_ENABLE_CMD_DISABLE   2
+
 /* Beacon processing related command and event structures */
 struct wmi_bcn_tx_hdr {
 	__le32 vdev_id;
@@ -4290,6 +4366,10 @@ int ath10k_wmi_vdev_set_param(struct ath10k *ar, u32 vdev_id,
 			      u32 param_id, u32 param_value);
 int ath10k_wmi_vdev_install_key(struct ath10k *ar,
 				const struct wmi_vdev_install_key_arg *arg);
+int ath10k_vdev_spectral_configure(struct ath10k *ar,
+			     const struct wmi_vdev_spectral_configure_arg *arg);
+int ath10k_vdev_spectral_enable(struct ath10k *ar, u32 vdev_id, u32 trigger,
+				u32 enable);
 int ath10k_wmi_peer_create(struct ath10k *ar, u32 vdev_id,
 		    const u8 peer_addr[ETH_ALEN]);
 int ath10k_wmi_peer_delete(struct ath10k *ar, u32 vdev_id,
diff --git a/drivers/net/wireless/ath/spectral_common.h b/drivers/net/wireless/ath/spectral_common.h
index b9ab722..fa01b9f 100644
--- a/drivers/net/wireless/ath/spectral_common.h
+++ b/drivers/net/wireless/ath/spectral_common.h
@@ -19,6 +19,7 @@
 
 #define SPECTRAL_HT20_NUM_BINS		56
 #define SPECTRAL_HT20_40_NUM_BINS		128
+#define SPECTRAL_ATH10K_HT20_NUM_BINS		64
 
 /* FFT sample format given to userspace via debugfs.
  *
@@ -31,6 +32,7 @@
 enum ath_fft_sample_type {
 	ATH_FFT_SAMPLE_HT20 = 1,
 	ATH_FFT_SAMPLE_HT20_40,
+	ATH_FFT_SAMPLE_ATH10K_HT20,
 };
 
 struct fft_sample_tlv {
@@ -85,4 +87,22 @@ struct fft_sample_ht20_40 {
 	u8 data[SPECTRAL_HT20_40_NUM_BINS];
 } __packed;
 
+struct fft_sample_ath10k_ht20 {
+	struct fft_sample_tlv tlv;
+	u8 mhz;
+	__be16 freq1;
+	__be16 freq2;
+	__be16 noise;
+	__be16 max_magnitude;
+	__be16 total_gain_db;
+	__be16 base_pwr_db;
+	__be64 tsf;
+	s8 max_index;
+	u8 rssi;
+	u8 relpwr_db;
+	u8 avgpwr_db;
+
+	u8 data[SPECTRAL_ATH10K_HT20_NUM_BINS];
+} __packed;
+
 #endif /* SPECTRAL_COMMON_H */
-- 
1.9.1

Re: [RFC 2/2] ath10k: add spectral scan feature

[Michal Kazior]

On 15 July 2014 18:32, Simon Wunderlich <sw@simonwunderlich.de> wrote:
> From: Sven Eckelmann <sven@narfation.org>
>
> Adds the spectral scan feature for ath10k. The spectral scan is triggered by
> configuring a mode through a debugfs control file. Samples can be gathered via
> another relay debugfs file.
>
> Essentially, to try it out:
>
> ip link dev wlan0 set up
> echo background > /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan_ctl
> iw dev wlan0 scan
> echo disable > /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan_ctl
> cat /sys/kernel/debug/ieee80211/phy0/ath10k/spectral_scan0 > samples
>
> This feature is still experimental.
>
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
> Signed-off-by: Mathias Kretschmer <mathias.kretschmer@fokus.fraunhofer.de>
> ---
>  drivers/net/wireless/ath/ath10k/Kconfig    |   1 +
>  drivers/net/wireless/ath/ath10k/Makefile   |   3 +-
>  drivers/net/wireless/ath/ath10k/core.h     |   6 +
>  drivers/net/wireless/ath/ath10k/debug.c    |   3 +
>  drivers/net/wireless/ath/ath10k/spectral.c | 368 +++++++++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/spectral.h |  66 ++++++
>  drivers/net/wireless/ath/ath10k/wmi.c      |  98 +++++++-
>  drivers/net/wireless/ath/ath10k/wmi.h      |  80 +++++++
>  drivers/net/wireless/ath/spectral_common.h |  20 ++
>  9 files changed, 643 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/wireless/ath/ath10k/spectral.c
>  create mode 100644 drivers/net/wireless/ath/ath10k/spectral.h
>
[...]
> +static void ath_debug_send_fft_sample(struct ath10k *ar,
> +                                     struct fft_sample_tlv *fft_sample_tlv)
> +{

Why ath_debug_... instead of ath10k_debug_.. ?


[...]
> +static int ath_get_spectral_vdevid(struct ath10k *ar)

ath_get_..? Shouldn't this be ath10k_get?


> +{
> +       struct ath10k_vif *arvif;
> +
> +       if (list_empty(&ar->arvifs))
> +               return -ENODEV;
> +
> +       arvif = list_first_entry(&ar->arvifs, typeof(*arvif), list);
> +       return arvif->vdev_id;
> +}

No locks? Access to arvifs must be protected with conf_mutex.

What happens when an interface is removed without spectral scan being stopped?


> +int ath10k_spectral_scan_trigger(struct ath10k *ar)
> +{
> +       int ret;
> +       int vdev_id;
> +
> +       vdev_id = ath_get_spectral_vdevid(ar);
> +       if (vdev_id < 0)
> +               return vdev_id;
> +
> +       if (ar->spectral_mode == SPECTRAL_DISABLED)
> +               return 0;
> +
> +       ret = ath10k_vdev_spectral_enable(ar, vdev_id,
> +                                         WMI_SPECTRAL_TRIGGER_CMD_CLEAR,
> +                                         WMI_SPECTRAL_ENABLE_CMD_ENABLE);
> +       if (ret < 0)
> +               return ret;
> +
> +       ret = ath10k_vdev_spectral_enable(ar, vdev_id,
> +                                         WMI_SPECTRAL_TRIGGER_CMD_TRIGGER,
> +                                         WMI_SPECTRAL_ENABLE_CMD_ENABLE);
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +int ath10k_spectral_scan_config(struct ath10k *ar, enum spectral_mode mode)
> +{
> +       int count, ret;

We tend to use `res` in ath10k.


> +       struct wmi_vdev_spectral_configure_arg arg;
> +       int vdev_id;
> +
> +       vdev_id = ath_get_spectral_vdevid(ar);
> +       if (vdev_id < 0)
> +               return vdev_id;
> +
> +       ar->spectral_mode = mode;
> +
> +       ath10k_vdev_spectral_enable(ar, vdev_id,
> +                                   WMI_SPECTRAL_TRIGGER_CMD_CLEAR,
> +                                   WMI_SPECTRAL_ENABLE_CMD_DISABLE);
> +       if (mode == SPECTRAL_DISABLED)
> +               return 0;
> +
> +       if (mode == SPECTRAL_BACKGROUND)
> +               count = WMI_SPECTRAL_COUNT_DEFAULT;
> +       else
> +               count = max_t(u8, 1, ar->spec_config.count);
> +
> +       arg.vdev_id              = vdev_id;
> +       arg.scan_count           = count;
> +       arg.scan_period          = WMI_SPECTRAL_PERIOD_DEFAULT;
> +       arg.scan_priority        = WMI_SPECTRAL_PRIORITY_DEFAULT;
> +       arg.scan_fft_size        = WMI_SPECTRAL_FFT_SIZE_DEFAULT;
> +       arg.scan_gc_ena          = WMI_SPECTRAL_GC_ENA_DEFAULT;
> +       arg.scan_restart_ena     = WMI_SPECTRAL_RESTART_ENA_DEFAULT;
> +       arg.scan_noise_floor_ref = WMI_SPECTRAL_NOISE_FLOOR_REF_DEFAULT;
> +       arg.scan_init_delay      = WMI_SPECTRAL_INIT_DELAY_DEFAULT;
> +       arg.scan_nb_tone_thr     = WMI_SPECTRAL_NB_TONE_THR_DEFAULT;
> +       arg.scan_str_bin_thr     = WMI_SPECTRAL_STR_BIN_THR_DEFAULT;
> +       arg.scan_wb_rpt_mode     = WMI_SPECTRAL_WB_RPT_MODE_DEFAULT;
> +       arg.scan_rssi_rpt_mode   = WMI_SPECTRAL_RSSI_RPT_MODE_DEFAULT;
> +       arg.scan_rssi_thr        = WMI_SPECTRAL_RSSI_THR_DEFAULT;
> +       arg.scan_pwr_format      = WMI_SPECTRAL_PWR_FORMAT_DEFAULT;
> +       arg.scan_rpt_mode        = WMI_SPECTRAL_RPT_MODE_DEFAULT;
> +       arg.scan_bin_scale       = WMI_SPECTRAL_BIN_SCALE_DEFAULT;
> +       arg.scan_dBm_adj         = WMI_SPECTRAL_DBM_ADJ_DEFAULT;
> +       arg.scan_chn_mask        = WMI_SPECTRAL_CHN_MASK_DEFAULT;
> +
> +       ret = ath10k_vdev_spectral_configure(ar, &arg);
> +       if (ret < 0)

+ ath10k_warn("failed to configure spectral scan: %d\n", ret);


> +               return ret;
> +
> +       if (mode == SPECTRAL_BACKGROUND)
> +               ath10k_spectral_scan_trigger(ar);

Why isn't ath10k_spectral_scan_trigger() return value not checked?
Also +ath10k_warn.


[...]
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> index 6f83cae..7ad9c71 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> @@ -1659,7 +1659,47 @@ static void ath10k_wmi_event_spectral_scan(struct ath10k *ar,
>                                 struct wmi_single_phyerr_rx_event *event,
>                                 u64 tsf)
>  {
> -       ath10k_dbg(ATH10K_DBG_WMI, "wmi event spectral scan\n");
> +       int buf_len, tlv_len, res, i = 0;
> +       struct phyerr_tlv *tlv;
> +       u8 *tlv_buf;
> +       struct phyerr_fft_report *fftr;
> +       size_t fftr_len;
> +
> +       buf_len = __le32_to_cpu(event->hdr.buf_len);
> +
> +       while (i < buf_len) {
> +               if (i + sizeof(*tlv) > buf_len) {
> +                       ath10k_warn("too short buf for tlv header (%d)\n", i);

"failed to parse phyerr tlv header at byte %d\n", i


> +                       return;
> +               }
> +
> +               tlv = (struct phyerr_tlv *)&event->bufp[i];
> +               tlv_len = __le16_to_cpu(tlv->len);
> +               tlv_buf = &event->bufp[i + sizeof(*tlv)];
> +
> +               if (i + sizeof(*tlv) + tlv_len > buf_len) {
> +                       ath10k_warn("too short buf for tlv (%d)\n", i);

"failed to parse phyerr tlv payload at byte %d\n", i


> +                       return;
> +               }
> +
> +               switch (tlv->tag) {
> +               case PHYERR_TLV_TAG_SEARCH_FFT_REPORT:
> +                       if (sizeof(*fftr) > tlv_len) {
> +                               ath10k_warn("too short fft report (%d)\n", i);

"failed to parse fft report at byte %d\n", i


> +                               return;
> +                       }
> +
> +                       fftr_len = tlv_len - sizeof(*fftr);
> +                       fftr = (struct phyerr_fft_report *)tlv_buf;
> +                       res = ath10k_process_fft(ar, event, fftr, fftr_len,
> +                                                tsf);
> +                       if (res)

+ ath10k_warn("failed to process fft report: %d\n", res)


[...]
> +int ath10k_vdev_spectral_configure(struct ath10k *ar,
> +                             const struct wmi_vdev_spectral_configure_arg *arg)

ath10k_wmi_vdev_spectral_configure


[...]
> +int ath10k_vdev_spectral_enable(struct ath10k *ar, u32 vdev_id, u32 trigger,
> +                               u32 enable)

ath10k_wmi_vdev_spectral_enable


[...]
> +struct wmi_vdev_spectral_configure_arg {
> +       u32 vdev_id;
> +       u32 scan_count;
> +       u32 scan_period;
> +       u32 scan_priority;
> +       u32 scan_fft_size;
> +       u32 scan_gc_ena;
> +       u32 scan_restart_ena;
> +       u32 scan_noise_floor_ref;
> +       u32 scan_init_delay;
> +       u32 scan_nb_tone_thr;
> +       u32 scan_str_bin_thr;
> +       u32 scan_wb_rpt_mode;
> +       u32 scan_rssi_rpt_mode;
> +       u32 scan_rssi_thr;
> +       u32 scan_pwr_format;
> +       u32 scan_rpt_mode;
> +       u32 scan_bin_scale;
> +       u32 scan_dBm_adj;
> +       u32 scan_chn_mask;

Some stuff isn't self-explanatory. I'd love to see some of these
fields documented to account for possible values (wb_rpt_mode?) and
units (init_delay?).


[...]
> +struct fft_sample_ath10k_ht20 {
> +       struct fft_sample_tlv tlv;
> +       u8 mhz;

A more suitable name would be "chan_width_mhz" I guess?


> +       __be16 freq1;
> +       __be16 freq2;
> +       __be16 noise;
> +       __be16 max_magnitude;
> +       __be16 total_gain_db;
> +       __be16 base_pwr_db;
> +       __be64 tsf;
> +       s8 max_index;
> +       u8 rssi;
> +       u8 relpwr_db;
> +       u8 avgpwr_db;
> +
> +       u8 data[SPECTRAL_ATH10K_HT20_NUM_BINS];

And as pointed out the size/count of bins is most likely not
bandwidth-dependant. It's just a matter of resolution configured with
fft_size + bin_scale. Maybe this should be a variable-sized array?


Generally this seems to be missing locking (does debugfs guarantee
serialized read/write calls?).


Michał

Re: [RFC 2/2] ath10k: add spectral scan feature

[Sven Eckelmann]

On Wednesday 16 July 2014 11:30:12 Michal Kazior wrote:
> > +static void ath_debug_send_fft_sample(struct ath10k *ar,
> > +                                     struct fft_sample_tlv
> > *fft_sample_tlv) +{
> 
> Why ath_debug_... instead of ath10k_debug_.. ?
> 
> 
> [...]

Will be modified.

> 
> > +static int ath_get_spectral_vdevid(struct ath10k *ar)
> 
> ath_get_..? Shouldn't this be ath10k_get?
> 
> > +{
> > +       struct ath10k_vif *arvif;
> > +
> > +       if (list_empty(&ar->arvifs))
> > +               return -ENODEV;
> > +
> > +       arvif = list_first_entry(&ar->arvifs, typeof(*arvif), list);
> > +       return arvif->vdev_id;
> > +}
> 
> No locks? Access to arvifs must be protected with conf_mutex.

Will be handled by the caller and lockdep_assert will be added to this 
function

> 
> What happens when an interface is removed without spectral scan being
> stopped?

This is a question for the QCA firmware guys.

> > +int ath10k_spectral_scan_config(struct ath10k *ar, enum spectral_mode
> > mode) +{
> > +       int count, ret;
> 
> We tend to use `res` in ath10k.

Thanks, will be changed. But actually, I found a lot of ret stuff in the 
ath10k code.

[...]
> + ath10k_warn("failed to configure spectral scan: %d\n", ret);
[...]
> "failed to parse phyerr tlv header at byte %d\n", i
[...]
> "failed to parse phyerr tlv payload at byte %d\n", i
[...]
> "failed to parse fft report at byte %d\n", i
[...]
> + ath10k_warn("failed to process fft report: %d\n", res)

Ok, warning messages will be modified.

> > +int ath10k_vdev_spectral_configure(struct ath10k *ar,
> > +                             const struct wmi_vdev_spectral_configure_arg
> > *arg)
> ath10k_wmi_vdev_spectral_configure
[...]
> > +int ath10k_vdev_spectral_enable(struct ath10k *ar, u32 vdev_id, u32
> > trigger, +                               u32 enable)
> 
> ath10k_wmi_vdev_spectral_enable

Ok, will be renamed.

> [...]
> > +struct wmi_vdev_spectral_configure_arg {
> > +       u32 vdev_id;
> > +       u32 scan_count;
> > +       u32 scan_period;
> > +       u32 scan_priority;
> > +       u32 scan_fft_size;
> > +       u32 scan_gc_ena;
> > +       u32 scan_restart_ena;
> > +       u32 scan_noise_floor_ref;
> > +       u32 scan_init_delay;
> > +       u32 scan_nb_tone_thr;
> > +       u32 scan_str_bin_thr;
> > +       u32 scan_wb_rpt_mode;
> > +       u32 scan_rssi_rpt_mode;
> > +       u32 scan_rssi_thr;
> > +       u32 scan_pwr_format;
> > +       u32 scan_rpt_mode;
> > +       u32 scan_bin_scale;
> > +       u32 scan_dBm_adj;
> > +       u32 scan_chn_mask;
> 
> Some stuff isn't self-explanatory. I'd love to see some of these
> fields documented to account for possible values (wb_rpt_mode?) and
> units (init_delay?).

Sorry, I got no documentation to anything. So I cannot provide one to you. 
Maybe you can get it easier than me.

> [...]
> 
> > +struct fft_sample_ath10k_ht20 {
> > +       struct fft_sample_tlv tlv;
> > +       u8 mhz;
> 
> A more suitable name would be "chan_width_mhz" I guess?

Ok,

> > +       __be16 freq1;
> > +       __be16 freq2;
> > +       __be16 noise;
> > +       __be16 max_magnitude;
> > +       __be16 total_gain_db;
> > +       __be16 base_pwr_db;
> > +       __be64 tsf;
> > +       s8 max_index;
> > +       u8 rssi;
> > +       u8 relpwr_db;
> > +       u8 avgpwr_db;
> > +
> > +       u8 data[SPECTRAL_ATH10K_HT20_NUM_BINS];
> 
> And as pointed out the size/count of bins is most likely not
> bandwidth-dependant. It's just a matter of resolution configured with
> fft_size + bin_scale. Maybe this should be a variable-sized array?

Ok, bin_scale didn't make a difference in my tests (I've just repeated them). 
But 2 ** (fft_size - 1) seems to be relevant. We have to change the format 
accordingly.

But currently we don't see any change in the bandwidth (20MHz/40MHz) for the 
different bin numbers. This has to be checked later in detail.

Kind regards,
	Sven

Re: [RFC 2/2] ath10k: add spectral scan feature

[Michal Kazior]

On 16 July 2014 16:35, Sven Eckelmann <sven@narfation.org> wrote:
> On Wednesday 16 July 2014 11:30:12 Michal Kazior wrote:
[...]
>>
>> What happens when an interface is removed without spectral scan being
>> stopped?
>
> This is a question for the QCA firmware guys.

Not really. My problems is this:
 - have 2 interfaces (vdev 0 and vdev 1)
 - start spectral (using vdev 0 which happens to be the first one)
 - remove 1 interface (vdev 0)

Now spectral scan on vdev 0 is not stopped and if you request to stop
it you effectively call spectral commands with vdev 1 (the other
interface which is the only one left). Yuck. I don't even want to know
how firmware (or different versions and branches of it) handle this
clumsiness.


>> > +int ath10k_spectral_scan_config(struct ath10k *ar, enum spectral_mode
>> > mode) +{
>> > +       int count, ret;
>>
>> We tend to use `res` in ath10k.
>
> Thanks, will be changed. But actually, I found a lot of ret stuff in the
> ath10k code.

Ah, sorry. You're right. I had a brain fart. `ret` is good.


[...]
>> > +       __be16 freq1;
>> > +       __be16 freq2;
>> > +       __be16 noise;
>> > +       __be16 max_magnitude;
>> > +       __be16 total_gain_db;
>> > +       __be16 base_pwr_db;
>> > +       __be64 tsf;
>> > +       s8 max_index;
>> > +       u8 rssi;
>> > +       u8 relpwr_db;
>> > +       u8 avgpwr_db;
>> > +
>> > +       u8 data[SPECTRAL_ATH10K_HT20_NUM_BINS];
>>
>> And as pointed out the size/count of bins is most likely not
>> bandwidth-dependant. It's just a matter of resolution configured with
>> fft_size + bin_scale. Maybe this should be a variable-sized array?
>
> Ok, bin_scale didn't make a difference in my tests (I've just repeated them).
> But 2 ** (fft_size - 1) seems to be relevant. We have to change the format
> accordingly.
>
> But currently we don't see any change in the bandwidth (20MHz/40MHz) for the
> different bin numbers. This has to be checked later in detail.

Yes. Apparently number of bins does *not* change with bandwidth but in
that case I'm guessing bin data does refer to the given bandwidth
which implies different bandwidths end up with different bin data
*resolution*.


Michał

Re: [RFC 0/2] ath10k spectral scan support

[Michal Kazior]

On 15 July 2014 18:32, Simon Wunderlich <sw@simonwunderlich.de> wrote:
> This is the first patchset iteration for review to implement the ath10k
> spectral scan feature. The patchset is still experimental, but we were
> able to detect our analogue 5.8 GHz TV repeater in the sample data. :)

Awesome!


[...]
>  * So far, we always get 64 bins of fft data in the samples, even if we
>    are in HT40 or VHT80 mode. That doesn't seem right, as there should
>    be more bins for wider channels (e.g. 128 or 256 bins). Is there any
>    parameter to change in the configuration to get the output from
>    multiple channels / higher bandwidth?

Hmm.. I suspect this may be because you use scan command. Scanning
uses channel list which is programmed with legacy modes a and g
(implying 20Mhz). Perhaps that's the limiting factor now. Using other
(wider) modes breaks scan from what I remember but it might be worth
checking out if it impacts fft bin sizes.


Michał

Re: [RFC 0/2] ath10k spectral scan support

[Sven Eckelmann]

On Wednesday 16 July 2014 08:24:58 Michal Kazior wrote:
> >  * So far, we always get 64 bins of fft data in the samples, even if we
> >  
> >    are in HT40 or VHT80 mode. That doesn't seem right, as there should
> >    be more bins for wider channels (e.g. 128 or 256 bins). Is there any
> >    parameter to change in the configuration to get the output from
> >    multiple channels / higher bandwidth?
> 
> Hmm.. I suspect this may be because you use scan command. Scanning
> uses channel list which is programmed with legacy modes a and g
> (implying 20Mhz). Perhaps that's the limiting factor now. Using other
> (wider) modes breaks scan from what I remember but it might be worth
> checking out if it impacts fft bin sizes.

I did the HT80 and HT40 stuff on a single channel in the spectral "background" 
mode. They reports from the firmware (10.1) still only had 64 bins. The HT80 
stuff wasn't tested with the "main" firmware because it refused to switch to 
HT80 mode as AP/Sta.

Kind regards,
	Sven

Re: [RFC 0/2] ath10k spectral scan support

[Simon Wunderlich]

> On Wednesday 16 July 2014 08:24:58 Michal Kazior wrote:
> > >  * So far, we always get 64 bins of fft data in the samples, even if we
> > >  
> > >    are in HT40 or VHT80 mode. That doesn't seem right, as there should
> > >    be more bins for wider channels (e.g. 128 or 256 bins). Is there any
> > >    parameter to change in the configuration to get the output from
> > >    multiple channels / higher bandwidth?
> > 
> > Hmm.. I suspect this may be because you use scan command. Scanning
> > uses channel list which is programmed with legacy modes a and g
> > (implying 20Mhz). Perhaps that's the limiting factor now. Using other
> > (wider) modes breaks scan from what I remember but it might be worth
> > checking out if it impacts fft bin sizes.
> 
> I did the HT80 and HT40 stuff on a single channel in the spectral
> "background" mode. They reports from the firmware (10.1) still only had 64
> bins. The HT80 stuff wasn't tested with the "main" firmware because it
> refused to switch to HT80 mode as AP/Sta.

Just to clarify this a little more, we tried AP (running hostapd) as well a 
STA in 40 and 80 MHz modes. We always got 64 bins returned.

Cheers,
    Simon

Re: [RFC 0/2] ath10k spectral scan support

[Michal Kazior]

On 16 July 2014 10:38, Simon Wunderlich <sw@simonwunderlich.de> wrote:
>
>> On Wednesday 16 July 2014 08:24:58 Michal Kazior wrote:
>> > >  * So far, we always get 64 bins of fft data in the samples, even if we
>> > >
>> > >    are in HT40 or VHT80 mode. That doesn't seem right, as there should
>> > >    be more bins for wider channels (e.g. 128 or 256 bins). Is there any
>> > >    parameter to change in the configuration to get the output from
>> > >    multiple channels / higher bandwidth?
>> >
>> > Hmm.. I suspect this may be because you use scan command. Scanning
>> > uses channel list which is programmed with legacy modes a and g
>> > (implying 20Mhz). Perhaps that's the limiting factor now. Using other
>> > (wider) modes breaks scan from what I remember but it might be worth
>> > checking out if it impacts fft bin sizes.
>>
>> I did the HT80 and HT40 stuff on a single channel in the spectral
>> "background" mode. They reports from the firmware (10.1) still only had 64
>> bins. The HT80 stuff wasn't tested with the "main" firmware because it
>> refused to switch to HT80 mode as AP/Sta.
>
> Just to clarify this a little more, we tried AP (running hostapd) as well a
> STA in 40 and 80 MHz modes. We always got 64 bins returned.

It seems the number of bins is 2^ (fft_size - bin_scale) = 2^(7-1) =
64. Changing fft_size to 9 yields 256 bins.

Perhaps fft reports are (implicitly) relative to the mode the hw is in
at a given time and if you want a reasonable bin resolution for, e.g.
VHT80 you need to increase fft_size?


Michał

Re: [RFC 0/2] ath10k spectral scan support

[Janusz Dziedzic]

On 15 July 2014 18:32, Simon Wunderlich <sw@simonwunderlich.de> wrote:
> This is the first patchset iteration for review to implement the ath10k
> spectral scan feature. The patchset is still experimental, but we were
> able to detect our analogue 5.8 GHz TV repeater in the sample data. :)
>
> As for ath9k, a proof of concept data plotter has been implemented, and
> is currently available for ath10k in its own branch of FFT_eval [1].
>
Tested this with fft_eval, seems works quite good.
There is small problem with scan timeout, when spectral scan activated
and channels with high load checked, I often hit scan timeout.
Seems we should increase scan timeout in ath10k_start_scan() when
spectral activated.

BR
Janusz