[PATCH 0/2] ath9k: spectral scan on HT40 channels

[PATCH 0/2] ath9k: spectral scan on HT40 channels

[Lorenzo Bianconi]

This couple of patches add spectral scan capability on HT40 channels for
ath9k. In particular:
*[PATCH 1/2]: add nf parameter to ath9k_hw_getchan_noise() to compute noise
floor for ext chains
*[PATCH 2/2]: parse FFT samples on HT40 channels

Spectral scan has been tested using Simon Wunderlich's userspace program
(https://github.com/simonwunderlich/FFT_eval) on HT20 channels and using a Qt-Qwt
program (https://github.com/LorenzoBianconi/ath_spectral) on HT40 channels

Regards,
Lorenzo

Lorenzo Bianconi (2):
  ath9k: add noise floor parameter to ath9k_hw_getchan_noise
  ath9k: add HT40 spectral scan capability

 drivers/net/wireless/ath/ath9k/ath9k.h |  28 +++++++
 drivers/net/wireless/ath/ath9k/calib.c |  10 +--
 drivers/net/wireless/ath/ath9k/calib.h |   3 +-
 drivers/net/wireless/ath/ath9k/hw.c    |   2 +-
 drivers/net/wireless/ath/ath9k/link.c  |   3 +-
 drivers/net/wireless/ath/ath9k/recv.c  | 144 ++++++++++++++++++++++++---------
 6 files changed, 144 insertions(+), 46 deletions(-)

-- 
1.8.1.2

[PATCH 1/2] ath9k: add noise floor parameter to ath9k_hw_getchan_noise

[Lorenzo Bianconi]

Add nf parameter to ath9k_hw_getchan_noise() in order to compute NF for EXT
chains with the same scale of noise floor calculated on CTL chains.
ath9k_hw_getchan_noise() will be used in ath_process_fft() for spectral scan on
HT40 channels

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 drivers/net/wireless/ath/ath9k/calib.c | 10 +++++-----
 drivers/net/wireless/ath/ath9k/calib.h |  3 ++-
 drivers/net/wireless/ath/ath9k/hw.c    |  2 +-
 drivers/net/wireless/ath/ath9k/link.c  |  3 ++-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/calib.c b/drivers/net/wireless/ath/ath9k/calib.c
index d438a03..698d91a 100644
--- a/drivers/net/wireless/ath/ath9k/calib.c
+++ b/drivers/net/wireless/ath/ath9k/calib.c
@@ -63,13 +63,13 @@ static s16 ath9k_hw_get_default_nf(struct ath_hw *ah,
 	return ath9k_hw_get_nf_limits(ah, chan)->nominal;
 }
 
-s16 ath9k_hw_getchan_noise(struct ath_hw *ah, struct ath9k_channel *chan)
+s16 ath9k_hw_getchan_noise(struct ath_hw *ah, struct ath9k_channel *chan,
+			   s16 nf)
 {
 	s8 noise = ATH_DEFAULT_NOISE_FLOOR;
 
-	if (chan && chan->noisefloor) {
-		s8 delta = chan->noisefloor -
-			   ATH9K_NF_CAL_NOISE_THRESH -
+	if (nf) {
+		s8 delta = nf - ATH9K_NF_CAL_NOISE_THRESH -
 			   ath9k_hw_get_default_nf(ah, chan);
 		if (delta > 0)
 			noise += delta;
@@ -394,7 +394,7 @@ bool ath9k_hw_getnf(struct ath_hw *ah, struct ath9k_channel *chan)
 	clear_bit(NFCAL_PENDING, &caldata->cal_flags);
 	ath9k_hw_update_nfcal_hist_buffer(ah, caldata, nfarray);
 	chan->noisefloor = h[0].privNF;
-	ah->noise = ath9k_hw_getchan_noise(ah, chan);
+	ah->noise = ath9k_hw_getchan_noise(ah, chan, chan->noisefloor);
 	return true;
 }
 EXPORT_SYMBOL(ath9k_hw_getnf);
diff --git a/drivers/net/wireless/ath/ath9k/calib.h b/drivers/net/wireless/ath/ath9k/calib.h
index 3d70b8c..b8ed95e 100644
--- a/drivers/net/wireless/ath/ath9k/calib.h
+++ b/drivers/net/wireless/ath/ath9k/calib.h
@@ -116,7 +116,8 @@ void ath9k_init_nfcal_hist_buffer(struct ath_hw *ah,
 void ath9k_hw_bstuck_nfcal(struct ath_hw *ah);
 void ath9k_hw_reset_calibration(struct ath_hw *ah,
 				struct ath9k_cal_list *currCal);
-s16 ath9k_hw_getchan_noise(struct ath_hw *ah, struct ath9k_channel *chan);
+s16 ath9k_hw_getchan_noise(struct ath_hw *ah, struct ath9k_channel *chan,
+			   s16 nf);
 
 
 #endif /* CALIB_H */
diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c
index f11e838..79b766f 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -1907,7 +1907,7 @@ int ath9k_hw_reset(struct ath_hw *ah, struct ath9k_channel *chan,
 	} else if (caldata) {
 		clear_bit(PAPRD_PACKET_SENT, &caldata->cal_flags);
 	}
-	ah->noise = ath9k_hw_getchan_noise(ah, chan);
+	ah->noise = ath9k_hw_getchan_noise(ah, chan, chan->noisefloor);
 
 	if (fastcc) {
 		r = ath9k_hw_do_fastcc(ah, chan);
diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c
index 84a6064..b797519 100644
--- a/drivers/net/wireless/ath/ath9k/link.c
+++ b/drivers/net/wireless/ath/ath9k/link.c
@@ -518,7 +518,8 @@ void ath_update_survey_nf(struct ath_softc *sc, int channel)
 
 	if (chan->noisefloor) {
 		survey->filled |= SURVEY_INFO_NOISE_DBM;
-		survey->noise = ath9k_hw_getchan_noise(ah, chan);
+		survey->noise = ath9k_hw_getchan_noise(ah, chan,
+						       chan->noisefloor);
 	}
 }
 
-- 
1.8.1.2

[PATCH 2/2] ath9k: add HT40 spectral scan capability

[Lorenzo Bianconi]

Add spectral scan feature on HT40 channels for ath9k. This patch extends
previous capability added by Simon Wunderlich

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi83@gmail.com>
---
 drivers/net/wireless/ath/ath9k/ath9k.h |  28 +++++++
 drivers/net/wireless/ath/ath9k/recv.c  | 144 ++++++++++++++++++++++++---------
 2 files changed, 134 insertions(+), 38 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 8878f2d..62d2812 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -879,6 +879,7 @@ static inline u8 spectral_bitmap_weight(u8 *bins)
  */
 enum ath_fft_sample_type {
 	ATH_FFT_SAMPLE_HT20 = 1,
+	ATH_FFT_SAMPLE_HT20_40,
 };
 
 struct fft_sample_tlv {
@@ -905,6 +906,33 @@ struct fft_sample_ht20 {
 	u8 data[SPECTRAL_HT20_NUM_BINS];
 } __packed;
 
+struct fft_sample_ht20_40 {
+	struct fft_sample_tlv tlv;
+
+	u8 max_exp;
+
+	__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 data[SPECTRAL_HT20_40_NUM_BINS];
+} __packed;
+
 void ath9k_tasklet(unsigned long data);
 int ath_cabq_update(struct ath_softc *);
 
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index ab9e3a8..b49fd13 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -972,14 +972,14 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
 {
 #ifdef CONFIG_ATH9K_DEBUGFS
 	struct ath_hw *ah = sc->sc_ah;
-	u8 bins[SPECTRAL_HT20_NUM_BINS];
-	u8 *vdata = (u8 *)hdr;
-	struct fft_sample_ht20 fft_sample;
+	u8 num_bins, *bins, *sample, *vdata = (u8 *)hdr;
+	struct fft_sample_ht20 fft_sample_20;
+	struct fft_sample_ht20_40 fft_sample_40;
 	struct ath_radar_info *radar_info;
-	struct ath_ht20_mag_info *mag_info;
 	int len = rs->rs_datalen;
 	int dc_pos;
-	u16 length, max_magnitude;
+	u16 fft_len, length, freq = ah->curchan->chan->center_freq;
+	enum nl80211_channel_type chan_type;
 
 	/* AR9280 and before report via ATH9K_PHYERR_RADAR, AR93xx and newer
 	 * via ATH9K_PHYERR_SPECTRAL. Haven't seen ATH9K_PHYERR_FALSE_RADAR_EXT
@@ -997,45 +997,44 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
 	if (!(radar_info->pulse_bw_info & SPECTRAL_SCAN_BITMASK))
 		return 0;
 
-	/* Variation in the data length is possible and will be fixed later.
-	 * Note that we only support HT20 for now.
-	 *
-	 * TODO: add HT20_40 support as well.
-	 */
-	if ((len > SPECTRAL_HT20_TOTAL_DATA_LEN + 2) ||
-	    (len < SPECTRAL_HT20_TOTAL_DATA_LEN - 1))
-		return 1;
-
-	fft_sample.tlv.type = ATH_FFT_SAMPLE_HT20;
-	length = sizeof(fft_sample) - sizeof(fft_sample.tlv);
-	fft_sample.tlv.length = __cpu_to_be16(length);
+	chan_type = cfg80211_get_chandef_type(&sc->hw->conf.chandef);
+	if ((chan_type == NL80211_CHAN_HT40MINUS) ||
+	    (chan_type == NL80211_CHAN_HT40PLUS)) {
+		fft_len = SPECTRAL_HT20_40_TOTAL_DATA_LEN;
+		num_bins = SPECTRAL_HT20_40_NUM_BINS;
+		bins = (u8 *) fft_sample_40.data;
+	} else {
+		fft_len = SPECTRAL_HT20_TOTAL_DATA_LEN;
+		num_bins = SPECTRAL_HT20_NUM_BINS;
+		bins = (u8 *) fft_sample_20.data;
+	}
 
-	fft_sample.freq = __cpu_to_be16(ah->curchan->chan->center_freq);
-	fft_sample.rssi = fix_rssi_inv_only(rs->rs_rssi_ctl0);
-	fft_sample.noise = ah->noise;
+	/* Variation in the data length is possible and will be fixed later */
+	if ((len > fft_len + 2) || (len < fft_len - 1))
+		return 1;
 
-	switch (len - SPECTRAL_HT20_TOTAL_DATA_LEN) {
+	switch (len - fft_len) {
 	case 0:
 		/* length correct, nothing to do. */
-		memcpy(bins, vdata, SPECTRAL_HT20_NUM_BINS);
+		memcpy(bins, vdata, num_bins);
 		break;
 	case -1:
 		/* first byte missing, duplicate it. */
-		memcpy(&bins[1], vdata, SPECTRAL_HT20_NUM_BINS - 1);
+		memcpy(&bins[1], vdata, num_bins - 1);
 		bins[0] = vdata[0];
 		break;
 	case 2:
 		/* MAC added 2 extra bytes at bin 30 and 32, remove them. */
 		memcpy(bins, vdata, 30);
 		bins[30] = vdata[31];
-		memcpy(&bins[31], &vdata[33], SPECTRAL_HT20_NUM_BINS - 31);
+		memcpy(&bins[31], &vdata[33], num_bins - 31);
 		break;
 	case 1:
 		/* MAC added 2 extra bytes AND first byte is missing. */
 		bins[0] = vdata[0];
-		memcpy(&bins[0], vdata, 30);
+		memcpy(&bins[1], vdata, 30);
 		bins[31] = vdata[31];
-		memcpy(&bins[32], &vdata[33], SPECTRAL_HT20_NUM_BINS - 32);
+		memcpy(&bins[32], &vdata[33], num_bins - 32);
 		break;
 	default:
 		return 1;
@@ -1044,23 +1043,92 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
 	/* DC value (value in the middle) is the blind spot of the spectral
 	 * sample and invalid, interpolate it.
 	 */
-	dc_pos = SPECTRAL_HT20_NUM_BINS / 2;
+	dc_pos = num_bins / 2;
 	bins[dc_pos] = (bins[dc_pos + 1] + bins[dc_pos - 1]) / 2;
 
-	/* mag data is at the end of the frame, in front of radar_info */
-	mag_info = ((struct ath_ht20_mag_info *)radar_info) - 1;
+	if ((chan_type == NL80211_CHAN_HT40MINUS) ||
+	    (chan_type == NL80211_CHAN_HT40PLUS)) {
+		s8 lower_rssi, upper_rssi;
+		s16 ext_nf;
+		u8 lower_max_index, upper_max_index;
+		u8 lower_bitmap_w, upper_bitmap_w;
+		u16 lower_mag, upper_mag;
+		struct ath9k_hw_cal_data *caldata = ah->caldata;
+		struct ath_ht20_40_mag_info *mag_info;
+
+		if (caldata)
+			ext_nf = ath9k_hw_getchan_noise(ah, ah->curchan,
+					caldata->nfCalHist[3].privNF);
+		else
+			ext_nf = ATH_DEFAULT_NOISE_FLOOR;
+
+		length = sizeof(fft_sample_40) - sizeof(struct fft_sample_tlv);
+		fft_sample_40.tlv.type = ATH_FFT_SAMPLE_HT20_40;
+		fft_sample_40.tlv.length = __cpu_to_be16(length);
+		fft_sample_40.freq = __cpu_to_be16(freq);
 
-	/* copy raw bins without scaling them */
-	memcpy(fft_sample.data, bins, SPECTRAL_HT20_NUM_BINS);
-	fft_sample.max_exp = mag_info->max_exp & 0xf;
+		if (chan_type == NL80211_CHAN_HT40PLUS) {
+			lower_rssi = fix_rssi_inv_only(rs->rs_rssi_ctl0);
+			upper_rssi = fix_rssi_inv_only(rs->rs_rssi_ext0);
+
+			fft_sample_40.lower_noise = ah->noise;
+			fft_sample_40.upper_noise = ext_nf;
+		} else {
+			lower_rssi = fix_rssi_inv_only(rs->rs_rssi_ext0);
+			upper_rssi = fix_rssi_inv_only(rs->rs_rssi_ctl0);
 
-	max_magnitude = spectral_max_magnitude(mag_info->all_bins);
-	fft_sample.max_magnitude = __cpu_to_be16(max_magnitude);
-	fft_sample.max_index = spectral_max_index(mag_info->all_bins);
-	fft_sample.bitmap_weight = spectral_bitmap_weight(mag_info->all_bins);
-	fft_sample.tsf = __cpu_to_be64(tsf);
+			fft_sample_40.lower_noise = ext_nf;
+			fft_sample_40.upper_noise = ah->noise;
+		}
+		fft_sample_40.lower_rssi = lower_rssi;
+		fft_sample_40.upper_rssi = upper_rssi;
+
+		mag_info = ((struct ath_ht20_40_mag_info *)radar_info) - 1;
+		lower_mag = spectral_max_magnitude(mag_info->lower_bins);
+		upper_mag = spectral_max_magnitude(mag_info->upper_bins);
+		fft_sample_40.lower_max_magnitude = __cpu_to_be16(lower_mag);
+		fft_sample_40.upper_max_magnitude = __cpu_to_be16(upper_mag);
+		lower_max_index = spectral_max_index(mag_info->lower_bins);
+		upper_max_index = spectral_max_index(mag_info->upper_bins);
+		fft_sample_40.lower_max_index = lower_max_index;
+		fft_sample_40.upper_max_index = upper_max_index;
+		lower_bitmap_w = spectral_bitmap_weight(mag_info->lower_bins);
+		upper_bitmap_w = spectral_bitmap_weight(mag_info->upper_bins);
+		fft_sample_40.lower_bitmap_weight = lower_bitmap_w;
+		fft_sample_40.upper_bitmap_weight = upper_bitmap_w;
+		fft_sample_40.max_exp = mag_info->max_exp & 0xf;
+
+		fft_sample_40.tsf = __cpu_to_be64(tsf);
+
+		sample = (u8 *) &fft_sample_40;
+	} else {
+		u8 max_index, bitmap_w;
+		u16 magnitude;
+		struct ath_ht20_mag_info *mag_info;
+
+		length = sizeof(fft_sample_20) - sizeof(struct fft_sample_tlv);
+		fft_sample_20.tlv.type = ATH_FFT_SAMPLE_HT20;
+		fft_sample_20.tlv.length = __cpu_to_be16(length);
+		fft_sample_20.freq = __cpu_to_be16(freq);
+
+		fft_sample_20.rssi = fix_rssi_inv_only(rs->rs_rssi_ctl0);
+		fft_sample_20.noise = ah->noise;
+
+		mag_info = ((struct ath_ht20_mag_info *)radar_info) - 1;
+		magnitude = spectral_max_magnitude(mag_info->all_bins);
+		fft_sample_20.max_magnitude = __cpu_to_be16(magnitude);
+		max_index = spectral_max_index(mag_info->all_bins);
+		fft_sample_20.max_index = max_index;
+		bitmap_w = spectral_bitmap_weight(mag_info->all_bins);
+		fft_sample_20.bitmap_weight = bitmap_w;
+		fft_sample_20.max_exp = mag_info->max_exp & 0xf;
+
+		fft_sample_20.tsf = __cpu_to_be64(tsf);
+
+		sample = (u8 *) &fft_sample_20;
+	}
 
-	ath_debug_send_fft_sample(sc, &fft_sample.tlv);
+	ath_debug_send_fft_sample(sc, (struct fft_sample_tlv *) sample);
 	return 1;
 #else
 	return 0;
-- 
1.8.1.2

Re: [PATCH 2/2] ath9k: add HT40 spectral scan capability

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 4226 bytes --]

Hello Lorenzo,

I've reviewed and tested your patch, this looks good! The old format still works, and
for a HT40+ channel I get your new format. Please find a few minor comments inline,
and feel free to add my Tested-by/Reviewed-by in the next round.

BTW, i failed to compile your UI[1]. I'm not familiar with QT, maybe you can add some
installation document (or Makefile or whatever)? :)

[1] https://github.com/LorenzoBianconi/ath_spectral

On Sat, Oct 05, 2013 at 11:16:09PM +0200, Lorenzo Bianconi wrote:
> @@ -905,6 +906,33 @@ struct fft_sample_ht20 {
>  	u8 data[SPECTRAL_HT20_NUM_BINS];
>  } __packed;
>  
> +struct fft_sample_ht20_40 {
> +	struct fft_sample_tlv tlv;
> +
> +	u8 max_exp;
> +
> +	__be16 freq;

What does the frequency tell us? This is just the frequency of the primary
channel, isn't it? You could either:
 1) make this the center frequency of both channels (e.g. for 2412 HT40+ this would be 2422)
 2) add a field to indicate HT40+ or HT40- or
 3) use lower_freq and upper_freq

Option 2 would have the advantage that the channel type is clear, not sure if this matters. :)

> +
> +	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 data[SPECTRAL_HT20_40_NUM_BINS];
> +} __packed;
> +
>  void ath9k_tasklet(unsigned long data);
>  int ath_cabq_update(struct ath_softc *);
>  
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index ab9e3a8..b49fd13 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -972,14 +972,14 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
>  {
>  #ifdef CONFIG_ATH9K_DEBUGFS
>  	struct ath_hw *ah = sc->sc_ah;
> -	u8 bins[SPECTRAL_HT20_NUM_BINS];
> -	u8 *vdata = (u8 *)hdr;
> -	struct fft_sample_ht20 fft_sample;
> +	u8 num_bins, *bins, *sample, *vdata = (u8 *)hdr;
> +	struct fft_sample_ht20 fft_sample_20;
> +	struct fft_sample_ht20_40 fft_sample_40;
>  	struct ath_radar_info *radar_info;
> -	struct ath_ht20_mag_info *mag_info;
>  	int len = rs->rs_datalen;
>  	int dc_pos;
> -	u16 length, max_magnitude;
> +	u16 fft_len, length, freq = ah->curchan->chan->center_freq;
> +	enum nl80211_channel_type chan_type;
>  
>  	/* AR9280 and before report via ATH9K_PHYERR_RADAR, AR93xx and newer
>  	 * via ATH9K_PHYERR_SPECTRAL. Haven't seen ATH9K_PHYERR_FALSE_RADAR_EXT
> @@ -997,45 +997,44 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
>  	if (!(radar_info->pulse_bw_info & SPECTRAL_SCAN_BITMASK))
>  		return 0;
>  
> -	/* Variation in the data length is possible and will be fixed later.
> -	 * Note that we only support HT20 for now.
> -	 *
> -	 * TODO: add HT20_40 support as well.
> -	 */
> -	if ((len > SPECTRAL_HT20_TOTAL_DATA_LEN + 2) ||
> -	    (len < SPECTRAL_HT20_TOTAL_DATA_LEN - 1))
> -		return 1;
> -
> -	fft_sample.tlv.type = ATH_FFT_SAMPLE_HT20;
> -	length = sizeof(fft_sample) - sizeof(fft_sample.tlv);
> -	fft_sample.tlv.length = __cpu_to_be16(length);
> +	chan_type = cfg80211_get_chandef_type(&sc->hw->conf.chandef);
> +	if ((chan_type == NL80211_CHAN_HT40MINUS) ||
> +	    (chan_type == NL80211_CHAN_HT40PLUS)) {
> +		fft_len = SPECTRAL_HT20_40_TOTAL_DATA_LEN;
> +		num_bins = SPECTRAL_HT20_40_NUM_BINS;
> +		bins = (u8 *) fft_sample_40.data;

I get a few checkpatch errors here, like:

0002-ath9k-add-HT40-spectral-scan-capability.patch
CHECK: No space is necessary after a cast
#101: FILE: drivers/net/wireless/ath/ath9k/recv.c:1005:
+		num_bins = SPECTRAL_HT20_40_NUM_BINS;
+		bins = (u8 *) fft_sample_40.data;

CHECK: No space is necessary after a cast
#105: FILE: drivers/net/wireless/ath/ath9k/recv.c:1009:
+		num_bins = SPECTRAL_HT20_NUM_BINS;
+		bins = (u8 *) fft_sample_20.data;

There are a few more, please check using ./scripts/checkpatch.pl --strict.

(I think you can ignore the camelcase errors though).

Thanks,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCH 2/2] ath9k: add HT40 spectral scan capability

[Lorenzo Bianconi]

Hi Simon,

> Hello Lorenzo,
>
> I've reviewed and tested your patch, this looks good! The old format still works, and
> for a HT40+ channel I get your new format. Please find a few minor comments inline,
> and feel free to add my Tested-by/Reviewed-by in the next round.
>

Perfect, I will send a new patchset with changes you suggested me

> BTW, i failed to compile your UI[1]. I'm not familiar with QT, maybe you can add some
> installation document (or Makefile or whatever)? :)

Sorry, I forgot to commit main.c :)
You should install Qwt libraries (http://qwt.sourceforge.net/) since
they are not compiled statically into the program and then run qmake
and make. I tested UI with Qwt-6.1.0 and Qt-5.0.1
Anyway, I will write a README

>
> [1] https://github.com/LorenzoBianconi/ath_spectral
>
> On Sat, Oct 05, 2013 at 11:16:09PM +0200, Lorenzo Bianconi wrote:
>> @@ -905,6 +906,33 @@ struct fft_sample_ht20 {
>>       u8 data[SPECTRAL_HT20_NUM_BINS];
>>  } __packed;
>>
>> +struct fft_sample_ht20_40 {
>> +     struct fft_sample_tlv tlv;
>> +
>> +     u8 max_exp;
>> +
>> +     __be16 freq;
>
> What does the frequency tell us? This is just the frequency of the primary
> channel, isn't it? You could either:
>  1) make this the center frequency of both channels (e.g. for 2412 HT40+ this would be 2422)
>  2) add a field to indicate HT40+ or HT40- or
>  3) use lower_freq and upper_freq
>
> Option 2 would have the advantage that the channel type is clear, not sure if this matters. :)
>

Yes, it is control channel. I prefer option 2 too. I will add a
channel type filed into fft_sample_ht20_40 structure.

>> +
>> +     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 data[SPECTRAL_HT20_40_NUM_BINS];
>> +} __packed;
>> +
>>  void ath9k_tasklet(unsigned long data);
>>  int ath_cabq_update(struct ath_softc *);
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
>> index ab9e3a8..b49fd13 100644
>> --- a/drivers/net/wireless/ath/ath9k/recv.c
>> +++ b/drivers/net/wireless/ath/ath9k/recv.c
>> @@ -972,14 +972,14 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
>>  {
>>  #ifdef CONFIG_ATH9K_DEBUGFS
>>       struct ath_hw *ah = sc->sc_ah;
>> -     u8 bins[SPECTRAL_HT20_NUM_BINS];
>> -     u8 *vdata = (u8 *)hdr;
>> -     struct fft_sample_ht20 fft_sample;
>> +     u8 num_bins, *bins, *sample, *vdata = (u8 *)hdr;
>> +     struct fft_sample_ht20 fft_sample_20;
>> +     struct fft_sample_ht20_40 fft_sample_40;
>>       struct ath_radar_info *radar_info;
>> -     struct ath_ht20_mag_info *mag_info;
>>       int len = rs->rs_datalen;
>>       int dc_pos;
>> -     u16 length, max_magnitude;
>> +     u16 fft_len, length, freq = ah->curchan->chan->center_freq;
>> +     enum nl80211_channel_type chan_type;
>>
>>       /* AR9280 and before report via ATH9K_PHYERR_RADAR, AR93xx and newer
>>        * via ATH9K_PHYERR_SPECTRAL. Haven't seen ATH9K_PHYERR_FALSE_RADAR_EXT
>> @@ -997,45 +997,44 @@ static int ath_process_fft(struct ath_softc *sc, struct ieee80211_hdr *hdr,
>>       if (!(radar_info->pulse_bw_info & SPECTRAL_SCAN_BITMASK))
>>               return 0;
>>
>> -     /* Variation in the data length is possible and will be fixed later.
>> -      * Note that we only support HT20 for now.
>> -      *
>> -      * TODO: add HT20_40 support as well.
>> -      */
>> -     if ((len > SPECTRAL_HT20_TOTAL_DATA_LEN + 2) ||
>> -         (len < SPECTRAL_HT20_TOTAL_DATA_LEN - 1))
>> -             return 1;
>> -
>> -     fft_sample.tlv.type = ATH_FFT_SAMPLE_HT20;
>> -     length = sizeof(fft_sample) - sizeof(fft_sample.tlv);
>> -     fft_sample.tlv.length = __cpu_to_be16(length);
>> +     chan_type = cfg80211_get_chandef_type(&sc->hw->conf.chandef);
>> +     if ((chan_type == NL80211_CHAN_HT40MINUS) ||
>> +         (chan_type == NL80211_CHAN_HT40PLUS)) {
>> +             fft_len = SPECTRAL_HT20_40_TOTAL_DATA_LEN;
>> +             num_bins = SPECTRAL_HT20_40_NUM_BINS;
>> +             bins = (u8 *) fft_sample_40.data;
>
> I get a few checkpatch errors here, like:
>
> 0002-ath9k-add-HT40-spectral-scan-capability.patch
> CHECK: No space is necessary after a cast
> #101: FILE: drivers/net/wireless/ath/ath9k/recv.c:1005:
> +               num_bins = SPECTRAL_HT20_40_NUM_BINS;
> +               bins = (u8 *) fft_sample_40.data;
>
> CHECK: No space is necessary after a cast
> #105: FILE: drivers/net/wireless/ath/ath9k/recv.c:1009:
> +               num_bins = SPECTRAL_HT20_NUM_BINS;
> +               bins = (u8 *) fft_sample_20.data;
>
> There are a few more, please check using ./scripts/checkpatch.pl --strict.
>
> (I think you can ignore the camelcase errors though).
>
> Thanks,
>         Simon

Fixed

Regards
Lorenzo



-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep

Re: [PATCH 2/2] ath9k: add HT40 spectral scan capability

[Luis R. Rodriguez]

On Fri, Oct 11, 2013 at 01:33:49PM +0200, Lorenzo Bianconi wrote:
> Hi Simon,
> 
> > Hello Lorenzo,
> >
> > I've reviewed and tested your patch, this looks good! The old format still works, and
> > for a HT40+ channel I get your new format. Please find a few minor comments inline,
> > and feel free to add my Tested-by/Reviewed-by in the next round.
> >
> 
> Perfect, I will send a new patchset with changes you suggested me
> 
> > BTW, i failed to compile your UI[1]. I'm not familiar with QT, maybe you can add some
> > installation document (or Makefile or whatever)? :)
> 
> Sorry, I forgot to commit main.c :)
> You should install Qwt libraries (http://qwt.sourceforge.net/) since
> they are not compiled statically into the program and then run qmake
> and make. I tested UI with Qwt-6.1.0 and Qt-5.0.1
> Anyway, I will write a README

Please also extend the ath9k wiki with informatoin
referncing your project and feel free to also use the
wiki to even document your project as this is very ath9k
specific.

  Luis