[PATCH v3 1/2] ath10k: handle cycle counter wraparound

[PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Michal Kazior]

When QCA988X cycle counter HW register wraps
around it resets to 0x7fffffff instead of 0. All
other cycle counter related registers are divided
by 2 so they never wraparound themselves. QCA61X4
has a uniform CC and it wraparounds in a regular
fashion though.

Worst case wraparound time is approx 24 seconds
(2**31 / 88MHz). Since scan channel visit times
are max 5 seconds (offchannel case) it is
guaranteed there's been at most 1 wraparound and
it is possible to compute survey data.

This fixes some occasional incorrect survey data
on QCA988X as some channels (depending on how/when
scan/offchannel requests were requested) would
have approx 24 sec active time which wasn't
actually the case.

This should help make hostapd ACS more reliable.

Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v3:
     * split into 2 separate patches
     * don't ignore all wraparound data: scans can be computed
     * apply the shifted wraparound fix only for QCA988X
     * improve commit log

 drivers/net/wireless/ath/ath10k/core.c | 15 +++++++++++++++
 drivers/net/wireless/ath/ath10k/core.h | 12 ++++++++++++
 drivers/net/wireless/ath/ath10k/wmi.c  | 14 ++++++++++++--
 3 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index bcccae19325d..0cc2122d6921 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -48,6 +48,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.name = "qca988x hw2.0",
 		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
+		.has_shifted_cc_wraparound = true,
 		.fw = {
 			.dir = QCA988X_HW_2_0_FW_DIR,
 			.fw = QCA988X_HW_2_0_FW_FILE,
@@ -102,6 +103,20 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 	},
 };
 
+void ath10k_core_get_cc_delta(struct ath10k *ar,
+			      u32 *cc_delta, u32 *rcc_delta,
+			      u32 cc, u32 rcc,
+			      u32 cc_prev, u32 rcc_prev)
+{
+	if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
+		cc_prev -= 0x7fffffff;
+		rcc *= 2;
+	}
+
+	*cc_delta = cc - cc_prev;
+	*rcc_delta = rcc - rcc_prev;
+}
+
 static void ath10k_send_suspend_complete(struct ath10k *ar)
 {
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot suspend complete\n");
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 827b3d79ed0c..fb403972b1c5 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -572,6 +572,13 @@ struct ath10k {
 		u32 patch_load_addr;
 		int uart_pin;
 
+		/* This is true if given HW chip has a quirky Cycle Counter
+		 * wraparound which resets to 0x7fffffff instead of 0. All
+		 * other CC related counters (e.g. Rx Clear Count) are divided
+		 * by 2 so they never wraparound themselves.
+		 */
+		bool has_shifted_cc_wraparound;
+
 		struct ath10k_hw_params_fw {
 			const char *dir;
 			const char *fw;
@@ -742,4 +749,9 @@ void ath10k_core_stop(struct ath10k *ar);
 int ath10k_core_register(struct ath10k *ar, u32 chip_id);
 void ath10k_core_unregister(struct ath10k *ar);
 
+void ath10k_core_get_cc_delta(struct ath10k *ar,
+			      u32 *cc_delta, u32 *rcc_delta,
+			      u32 cc, u32 rcc,
+			      u32 cc_prev, u32 rcc_prev);
+
 #endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 0fabe689179c..52ed48b7e5f9 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1640,8 +1640,18 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 		 * visited channel. The reported cycle count is global
 		 * and per-channel cycle count must be calculated */
 
-		cycle_count -= ar->survey_last_cycle_count;
-		rx_clear_count -= ar->survey_last_rx_clear_count;
+		/* Worst case wraparound time is approx 24 seconds (2**31 /
+		 * 88MHz). Since scan channel visit times are max 5 seconds
+		 * (offchannel case) it is guaranteed there's been at most 1
+		 * wraparound and it is possible to compute survey data.
+		 */
+		ath10k_core_get_cc_delta(ar,
+					 &cycle_count,
+					 &rx_clear_count,
+					 cycle_count,
+					 rx_clear_count,
+					 ar->survey_last_cycle_count,
+					 ar->survey_last_rx_clear_count);
 
 		survey = &ar->survey[idx];
 		survey->time = WMI_CHAN_INFO_MSEC(cycle_count);
-- 
2.1.4

[PATCH v3 2/2] ath10k: fix inconsistent survey reports

[Michal Kazior]

In some cases some channel survey data was
reported incorrect.

Channel info events were expected to come in pairs
without and with COMPLETE flag set respectively
for each channel visit during scan.

However there are deviations from this rule:

 * last scan chan info and first (next) scan chan
   info both have COMPLETE flag set. This was
   either programmed with the intent of providing BSS
   cycle count info or this is an artefact of
   firmware scan state machine. Either way this is
   useless due to short wraparound time and no
   overflow counters,

 * some channels are completed twice, e.g. I've
   seen chan info for freq 2462 come twice many
   times during scan. This may be some artefact of
   firmware scan state machine, e.g. during band
   switching.

Data provided in either of the above is now
ignored and not reported to avoid confusing the
user.

Survey dumps now include only data gathered during
scan channel visits that can be computed
correctly.

This should help make hostapd ACS more reliable.

Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v3:
     * result of patch split up
     * rename the var so it matches more tightly with it's purpose
     * improve commit log

 drivers/net/wireless/ath/ath10k/core.h |  8 ++++++++
 drivers/net/wireless/ath/ath10k/wmi.c  | 16 ++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index fb403972b1c5..d2978292b26c 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -696,6 +696,14 @@ struct ath10k {
 	u32 survey_last_cycle_count;
 	struct survey_info survey[ATH10K_NUM_CHANS];
 
+	/* Channel info events are expected to come in pairs without and with
+	 * COMPLETE flag set respectively for each channel visit during scan.
+	 *
+	 * However there are deviations from this rule. This flag is used to
+	 * avoid reporting garbage data.
+	 */
+	bool ch_info_can_report_cc;
+
 	struct dfs_pattern_detector *dfs_detector;
 
 	unsigned long tx_paused; /* see ATH10K_TX_PAUSE_ */
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 52ed48b7e5f9..7485cc517802 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1636,10 +1636,6 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 	}
 
 	if (cmd_flags & WMI_CHAN_INFO_FLAG_COMPLETE) {
-		/* During scanning chan info is reported twice for each
-		 * visited channel. The reported cycle count is global
-		 * and per-channel cycle count must be calculated */
-
 		/* Worst case wraparound time is approx 24 seconds (2**31 /
 		 * 88MHz). Since scan channel visit times are max 5 seconds
 		 * (offchannel case) it is guaranteed there's been at most 1
@@ -1660,6 +1656,18 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 		survey->filled = SURVEY_INFO_TIME |
 				 SURVEY_INFO_TIME_BUSY |
 				 SURVEY_INFO_NOISE_DBM;
+
+		if (!ar->ch_info_can_report_cc) {
+			/* Survey times can't be relied upon now but noise
+			 * floor is still correct though.
+			 */
+			survey->filled &= ~(SURVEY_INFO_TIME |
+					    SURVEY_INFO_TIME_BUSY);
+		}
+
+		ar->ch_info_can_report_cc = false;
+	} else {
+		ar->ch_info_can_report_cc = true;
 	}
 
 	ar->survey_last_rx_clear_count = rx_clear_count;
-- 
2.1.4

Re: [PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> When QCA988X cycle counter HW register wraps
> around it resets to 0x7fffffff instead of 0. All
> other cycle counter related registers are divided
> by 2 so they never wraparound themselves. QCA61X4
> has a uniform CC and it wraparounds in a regular
> fashion though.
>
> Worst case wraparound time is approx 24 seconds
> (2**31 / 88MHz). Since scan channel visit times
> are max 5 seconds (offchannel case) it is
> guaranteed there's been at most 1 wraparound and
> it is possible to compute survey data.
>
> This fixes some occasional incorrect survey data
> on QCA988X as some channels (depending on how/when
> scan/offchannel requests were requested) would
> have approx 24 sec active time which wasn't
> actually the case.
>
> This should help make hostapd ACS more reliable.
>
> Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

[...]

> +void ath10k_core_get_cc_delta(struct ath10k *ar,
> +			      u32 *cc_delta, u32 *rcc_delta,
> +			      u32 cc, u32 rcc,
> +			      u32 cc_prev, u32 rcc_prev)
> +{
> +	if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
> +		cc_prev -= 0x7fffffff;
> +		rcc *= 2;
> +	}
> +
> +	*cc_delta = cc - cc_prev;
> +	*rcc_delta = rcc - rcc_prev;
> +}

Why do you have this function in core.c? Why not in wmi.c?


-- 
Kalle Valo

Re: [PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Michal Kazior]

On 22 May 2015 at 13:36, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> When QCA988X cycle counter HW register wraps
>> around it resets to 0x7fffffff instead of 0. All
>> other cycle counter related registers are divided
>> by 2 so they never wraparound themselves. QCA61X4
>> has a uniform CC and it wraparounds in a regular
>> fashion though.
>>
>> Worst case wraparound time is approx 24 seconds
>> (2**31 / 88MHz). Since scan channel visit times
>> are max 5 seconds (offchannel case) it is
>> guaranteed there's been at most 1 wraparound and
>> it is possible to compute survey data.
>>
>> This fixes some occasional incorrect survey data
>> on QCA988X as some channels (depending on how/when
>> scan/offchannel requests were requested) would
>> have approx 24 sec active time which wasn't
>> actually the case.
>>
>> This should help make hostapd ACS more reliable.
>>
>> Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> [...]
>
>> +void ath10k_core_get_cc_delta(struct ath10k *ar,
>> +                           u32 *cc_delta, u32 *rcc_delta,
>> +                           u32 cc, u32 rcc,
>> +                           u32 cc_prev, u32 rcc_prev)
>> +{
>> +     if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
>> +             cc_prev -= 0x7fffffff;
>> +             rcc *= 2;
>> +     }
>> +
>> +     *cc_delta = cc - cc_prev;
>> +     *rcc_delta = rcc - rcc_prev;
>> +}
>
> Why do you have this function in core.c? Why not in wmi.c?

I don't consider this a part of WMI protocol per se. It's a logic
which happens to be used with values delivered via WMI but is chip
specific otherwise. For what it's worth we could be reading CC
registers, e.g. directly via MMIO.


Michał

Re: [PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Michal Kazior]

On 22 May 2015 at 13:56, Michal Kazior <michal.kazior@tieto.com> wrote:
> On 22 May 2015 at 13:36, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
[...]
>>> +void ath10k_core_get_cc_delta(struct ath10k *ar,
>>> +                           u32 *cc_delta, u32 *rcc_delta,
>>> +                           u32 cc, u32 rcc,
>>> +                           u32 cc_prev, u32 rcc_prev)
>>> +{
>>> +     if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
>>> +             cc_prev -= 0x7fffffff;
>>> +             rcc *= 2;
>>> +     }
>>> +
>>> +     *cc_delta = cc - cc_prev;
>>> +     *rcc_delta = rcc - rcc_prev;
>>> +}
>>
>> Why do you have this function in core.c? Why not in wmi.c?
>
> I don't consider this a part of WMI protocol per se. It's a logic
> which happens to be used with values delivered via WMI but is chip
> specific otherwise. For what it's worth we could be reading CC
> registers, e.g. directly via MMIO.

Now that I think about it this would fit into hw.c as well.


Michał

Re: [PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 22 May 2015 at 13:56, Michal Kazior <michal.kazior@tieto.com> wrote:
>> On 22 May 2015 at 13:36, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>>> Michal Kazior <michal.kazior@tieto.com> writes:
> [...]
>>>> +void ath10k_core_get_cc_delta(struct ath10k *ar,
>>>> +                           u32 *cc_delta, u32 *rcc_delta,
>>>> +                           u32 cc, u32 rcc,
>>>> +                           u32 cc_prev, u32 rcc_prev)
>>>> +{
>>>> +     if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
>>>> +             cc_prev -= 0x7fffffff;
>>>> +             rcc *= 2;
>>>> +     }
>>>> +
>>>> +     *cc_delta = cc - cc_prev;
>>>> +     *rcc_delta = rcc - rcc_prev;
>>>> +}
>>>
>>> Why do you have this function in core.c? Why not in wmi.c?
>>
>> I don't consider this a part of WMI protocol per se. It's a logic
>> which happens to be used with values delivered via WMI but is chip
>> specific otherwise. For what it's worth we could be reading CC
>> registers, e.g. directly via MMIO.
>
> Now that I think about it this would fit into hw.c as well.

Yeah, hw.c would be much better. And we could start adding more similar
hw adaptation code there in the future.

-- 
Kalle Valo

Re: [PATCH v3 1/2] ath10k: handle cycle counter wraparound

[Michal Kazior]

On 22 May 2015 at 19:11, Srinivasa Duvvuri <sduvvuri@google.com> wrote:
> Michal,
>    Sorry I was out sick last few days. Thanks for splitting and creating 2
> patches.
>    The math for total cycle counts makes sense but the math to handle the
> rx_clear_count
>    does not seem right to me. For the total cycle count we know that the
> count changes
>    from 0xffffffff to 0x7fffffff. So, we clearly know the wrap around point
> . But for
>    rcc( rx_clear_count)  we do not know when this count is halfed (wrapped).
>
>    Lets say rcc_prev  is the previous clear cycle count, rcc is the current
> clear cycle count
>    and rcc_wrap is a point at which the the total cycle count is wrapped
> from 0xfffffffff to 0x7fffffff .
>    At that point the rcc_wrap resets to rcc_wrap/2.
>
>     Here is how counts wrap around.
>
>    cc_prev  ............0xffffffff    -> 0x7fffffff ................ cc
>    rcc_prev ............rcc_wrap ->  rcc_wrap/2 ........... rcc.
>
>    The right math to calculate rcc_delta would be  (rcc_wrap - rcc_prev) +
> (rcc - rcc_wrap/2).
>    To do the math,  we need the rcc_wrap which we do not have.
>    the patch seems to simply do  2*rcc - rcc_prev . Unless I am missing
> something,
>    this does not seem right to me.

Ah, you're right. I guess I'll need to discard the data like you
originally did upon wraparound detection.


Michał

[PATCH v4 1/3] ath10k: move cycle_count macro

[Michal Kazior]

The macro isn't WMI specific. Instead it is
related to hardware chip so move the macro
accordingly. While at it document the magic value.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    I've had to move more survey logic in "ath10k:
    handle cycle counter wraparound" into hw.c hence
    this patch.
    
    v4:
     * add this patch

 drivers/net/wireless/ath/ath10k/hw.h  | 3 +++
 drivers/net/wireless/ath/ath10k/wmi.c | 4 ++--
 drivers/net/wireless/ath/ath10k/wmi.h | 1 -
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 89e09cbeac19..372f0b8c96f5 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -449,6 +449,9 @@ enum ath10k_hw_rate_cck {
 #define SCRATCH_3_ADDRESS			ar->regs->scratch_3_address
 #define CPU_INTR_ADDRESS			0x0010
 
+/* Cycle counters are running at 88MHz */
+#define CCNT_TO_MSEC(x) ((x) / 88000)
+
 /* Firmware indications to the Host via SCRATCH_3 register. */
 #define FW_INDICATOR_ADDRESS	(SOC_CORE_BASE_ADDRESS + SCRATCH_3_ADDRESS)
 #define FW_IND_EVENT_PENDING			1
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 0fabe689179c..43caabf5b025 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1644,8 +1644,8 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 		rx_clear_count -= ar->survey_last_rx_clear_count;
 
 		survey = &ar->survey[idx];
-		survey->time = WMI_CHAN_INFO_MSEC(cycle_count);
-		survey->time_busy = WMI_CHAN_INFO_MSEC(rx_clear_count);
+		survey->time = CCNT_TO_MSEC(cycle_count);
+		survey->time_busy = CCNT_TO_MSEC(rx_clear_count);
 		survey->noise = noise_floor;
 		survey->filled = SURVEY_INFO_TIME |
 				 SURVEY_INFO_TIME_BUSY |
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index cad72ae76253..cf44a3d080a3 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -4665,7 +4665,6 @@ struct wmi_peer_sta_kickout_event {
 } __packed;
 
 #define WMI_CHAN_INFO_FLAG_COMPLETE BIT(0)
-#define WMI_CHAN_INFO_MSEC(x) ((x) / 88000)
 
 /* Beacon filter wmi command info */
 #define BCN_FLT_MAX_SUPPORTED_IES	256
-- 
2.1.4

[PATCH v4 2/3] ath10k: handle cycle counter wraparound

[Michal Kazior]

When QCA988X cycle counter HW register wraps
around it resets to 0x7fffffff instead of 0. All
other cycle counter related registers are divided
by 2 so they never wraparound themselves. QCA61X4
has a uniform CC and it wraparounds in a regular
fashion though.

Worst case wraparound time is approx 24 seconds
(2**31 / 88MHz). Since scan channel visit times
are max 5 seconds (offchannel case) it is
guaranteed there's been at most 1 wraparound and
it is possible to compute survey active time
value. It is, however, impossible to determine the
point at which Rx Clear Count has been divided by
two so it is not reported upon wraparound.

This fixes some occasional incorrect survey data
on QCA988X as some channels (depending on how/when
scan/offchannel requests were requested) would
have approx 24 sec active time which wasn't
actually the case.

This should improve hostapd ACS a little bit.

Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v4:
     * use hw.c for cc computation logic [Kalle]
     * drop rx_clear_count upon wraparound [Srinivasa]
    
    v3:
     * split into 2 separate patches
     * don't ignore all wraparound data: scans can be computed
     * apply the shifted wraparound fix only for QCA988X
     * improve commit log

 drivers/net/wireless/ath/ath10k/core.c |  1 +
 drivers/net/wireless/ath/ath10k/core.h |  7 +++++++
 drivers/net/wireless/ath/ath10k/hw.c   | 21 +++++++++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath10k/wmi.c  | 17 +++++++++--------
 5 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index bcccae19325d..684d460d79b7 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -48,6 +48,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.name = "qca988x hw2.0",
 		.patch_load_addr = QCA988X_HW_2_0_PATCH_LOAD_ADDR,
 		.uart_pin = 7,
+		.has_shifted_cc_wraparound = true,
 		.fw = {
 			.dir = QCA988X_HW_2_0_FW_DIR,
 			.fw = QCA988X_HW_2_0_FW_FILE,
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 827b3d79ed0c..729b366e2a18 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -572,6 +572,13 @@ struct ath10k {
 		u32 patch_load_addr;
 		int uart_pin;
 
+		/* This is true if given HW chip has a quirky Cycle Counter
+		 * wraparound which resets to 0x7fffffff instead of 0. All
+		 * other CC related counters (e.g. Rx Clear Count) are divided
+		 * by 2 so they never wraparound themselves.
+		 */
+		bool has_shifted_cc_wraparound;
+
 		struct ath10k_hw_params_fw {
 			const char *dir;
 			const char *fw;
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 839a8791fb9e..5997f00afe3b 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -15,6 +15,7 @@
  */
 
 #include <linux/types.h>
+#include "core.h"
 #include "hw.h"
 
 const struct ath10k_hw_regs qca988x_regs = {
@@ -56,3 +57,23 @@ const struct ath10k_hw_regs qca6174_regs = {
 	.soc_chip_id_address			= 0x000f0,
 	.scratch_3_address			= 0x0028,
 };
+
+void ath10k_hw_fill_survey_time(struct ath10k *ar, struct survey_info *survey,
+				u32 cc, u32 rcc, u32 cc_prev, u32 rcc_prev)
+{
+	u32 cc_fix = 0;
+
+	survey->filled |= SURVEY_INFO_TIME |
+			  SURVEY_INFO_TIME_BUSY;
+
+	if (ar->hw_params.has_shifted_cc_wraparound && cc < cc_prev) {
+		cc_fix = 0x7fffffff;
+		survey->filled &= ~SURVEY_INFO_TIME_BUSY;
+	}
+
+	cc -= cc_prev - cc_fix;
+	rcc -= rcc_prev;
+
+	survey->time = CCNT_TO_MSEC(cc);
+	survey->time_busy = CCNT_TO_MSEC(rcc);
+}
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 372f0b8c96f5..85cca29375fe 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -169,6 +169,9 @@ struct ath10k_hw_regs {
 extern const struct ath10k_hw_regs qca988x_regs;
 extern const struct ath10k_hw_regs qca6174_regs;
 
+void ath10k_hw_fill_survey_time(struct ath10k *ar, struct survey_info *survey,
+				u32 cc, u32 rcc, u32 cc_prev, u32 rcc_prev);
+
 #define QCA_REV_988X(ar) ((ar)->hw_rev == ATH10K_HW_QCA988X)
 #define QCA_REV_6174(ar) ((ar)->hw_rev == ATH10K_HW_QCA6174)
 
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 43caabf5b025..70e6efa2c071 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -27,6 +27,7 @@
 #include "testmode.h"
 #include "wmi-ops.h"
 #include "p2p.h"
+#include "hw.h"
 
 /* MAIN WMI cmd track */
 static struct wmi_cmd_map wmi_cmd_map = {
@@ -1640,16 +1641,16 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 		 * visited channel. The reported cycle count is global
 		 * and per-channel cycle count must be calculated */
 
-		cycle_count -= ar->survey_last_cycle_count;
-		rx_clear_count -= ar->survey_last_rx_clear_count;
-
 		survey = &ar->survey[idx];
-		survey->time = CCNT_TO_MSEC(cycle_count);
-		survey->time_busy = CCNT_TO_MSEC(rx_clear_count);
 		survey->noise = noise_floor;
-		survey->filled = SURVEY_INFO_TIME |
-				 SURVEY_INFO_TIME_BUSY |
-				 SURVEY_INFO_NOISE_DBM;
+		survey->filled = SURVEY_INFO_NOISE_DBM;
+
+		ath10k_hw_fill_survey_time(ar,
+					   survey,
+					   cycle_count,
+					   rx_clear_count,
+					   ar->survey_last_cycle_count,
+					   ar->survey_last_rx_clear_count);
 	}
 
 	ar->survey_last_rx_clear_count = rx_clear_count;
-- 
2.1.4

[PATCH v4 3/3] ath10k: fix inconsistent survey reports

[Michal Kazior]

In some cases some channel survey data was
reported incorrect.

Channel info events were expected to come in pairs
without and with COMPLETE flag set respectively
for each channel visit during scan.

The known deviation from this is rule for last
scan chan info and first (next) scan chan info
both have COMPLETE flag set. This was either
programmed with the intent of providing BSS cycle
count info or this is an artefact of firmware scan
state machine. Either way this is useless due to
short wraparound time, wraparound quirks and no
overflow notification.

Survey dumps now include only data gathered during
scan channel visits that can be computed
correctly.

This should improve hostapd ACS a little bit.

Reported-by: Srinivasa Duvvuri <sduvvuri@chromium.org>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---

Notes:
    v4:
     * rebased
     * dont overwrite survey data needlessly
     * rename var
     * update commit log
    
    v3:
     * result of patch split up
     * rename the var so it matches more tightly with it's purpose
     * improve commit log

 drivers/net/wireless/ath/ath10k/core.h |  8 ++++++++
 drivers/net/wireless/ath/ath10k/wmi.c  | 26 ++++++++++++++------------
 2 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index 729b366e2a18..2d28cfae5c94 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -696,6 +696,14 @@ struct ath10k {
 	u32 survey_last_cycle_count;
 	struct survey_info survey[ATH10K_NUM_CHANS];
 
+	/* Channel info events are expected to come in pairs without and with
+	 * COMPLETE flag set respectively for each channel visit during scan.
+	 *
+	 * However there are deviations from this rule. This flag is used to
+	 * avoid reporting garbage data.
+	 */
+	bool ch_info_can_report_survey;
+
 	struct dfs_pattern_detector *dfs_detector;
 
 	unsigned long tx_paused; /* see ATH10K_TX_PAUSE_ */
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 70e6efa2c071..77220b0f0e89 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -1637,20 +1637,22 @@ void ath10k_wmi_event_chan_info(struct ath10k *ar, struct sk_buff *skb)
 	}
 
 	if (cmd_flags & WMI_CHAN_INFO_FLAG_COMPLETE) {
-		/* During scanning chan info is reported twice for each
-		 * visited channel. The reported cycle count is global
-		 * and per-channel cycle count must be calculated */
+		if (ar->ch_info_can_report_survey) {
+			survey = &ar->survey[idx];
+			survey->noise = noise_floor;
+			survey->filled = SURVEY_INFO_NOISE_DBM;
 
-		survey = &ar->survey[idx];
-		survey->noise = noise_floor;
-		survey->filled = SURVEY_INFO_NOISE_DBM;
+			ath10k_hw_fill_survey_time(ar,
+						   survey,
+						   cycle_count,
+						   rx_clear_count,
+						   ar->survey_last_cycle_count,
+						   ar->survey_last_rx_clear_count);
+		}
 
-		ath10k_hw_fill_survey_time(ar,
-					   survey,
-					   cycle_count,
-					   rx_clear_count,
-					   ar->survey_last_cycle_count,
-					   ar->survey_last_rx_clear_count);
+		ar->ch_info_can_report_survey = false;
+	} else {
+		ar->ch_info_can_report_survey = true;
 	}
 
 	ar->survey_last_rx_clear_count = rx_clear_count;
-- 
2.1.4

Re: [PATCH v4 1/3] ath10k: move cycle_count macro

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> The macro isn't WMI specific. Instead it is
> related to hardware chip so move the macro
> accordingly. While at it document the magic value.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Thanks, all three applied.

-- 
Kalle Valo