[PATCH] ath10k: handle cycle count wrap around

[Michal Kazior]

On 14 May 2015 at 03:56, Srinivasa Duvvuri <sduvvuri at google.com> wrote:
> When the cycle counts part of wmi_ch_info_ev_arg reach max value of
> 0xffffffff The HW/FW right shifts all the counters by 1 . The cycle
> count will be reset to 0x7fffffff and starts counting from 0x7fffffff.
> At the same time all the other counters will also have their values
> right shifted by 1 (divided by 2). There is no way to handle this odd
> wrap around. Detect and ignore the wrap around case.
> This change has 2 parts.
>
>  1) Ignore all the cycle counts , if the cycle count field wraps around.
>  2) With HW frequency of 88MHz the cycle count wraps around every 24
> seconds. the survey_last_cycle_count saved will become stale with in
> 24 seconds. Reset it to 0 at every scan start.

So what does it fix from a user perspective?


>
> Signed-off-by: Srinivasa Duvvuri <sduvvuri at chromium.org>
>
> diff --git a/drivers/net/wireless-3.18/ath/ath10k/wmi.c
> b/drivers/net/wireless-3.18/ath/ath10k/wmi.c
> index 720b6eb..09616bb 100644
> --- a/drivers/net/wireless-3.18/ath/ath10k/wmi.c
> +++ b/drivers/net/wireless-3.18/ath/ath10k/wmi.c

This doesn't look like an upstream kernel patch. Rebase it for
github.com/kvalo/ath, please.


> @@ -1158,6 +1158,8 @@
>   complete(&ar->scan.started);
>   break;
>   }
> + /* reset the last cycle count as the last cycle count is stale from
> last scan. */
> + ar->survey_last_cycle_count = 0;
>  }
>
>  static void ath10k_wmi_event_scan_start_failed(struct ath10k *ar)
> @@ -1716,21 +1718,36 @@
>   goto exit;
>   }
>
> - if (cmd_flags & WMI_CHAN_INFO_FLAG_COMPLETE) {
> + if (cmd_flags & WMI_CHAN_INFO_FLAG_COMPLETE && ar->survey_last_cycle_count) {

Channel info events come in pairs: without and with FLAG_COMPLETE
respectively. Thus it is pointless to reset
`ar->survey_last_cycle_count` and it's pointless to use it in this
condition.


>   /* During scanning chan info is reported twice for each
> - * visited channel. The reported cycle count is global
> + * 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->channel_time = WMI_CHAN_INFO_MSEC(cycle_count);
> - survey->channel_time_rx = WMI_CHAN_INFO_MSEC(rx_clear_count);
> +
> + if (cycle_count  <  ar->survey_last_cycle_count) {
> + /*
> +  * Ignore the wrapped ar ust_cycnd data. When the total cycle count
> +  * wraps around, FW/HW will right shift all the counts
> +  * (including total cycle counts) by 1.The cycle counts wrap around
> +  * every 24 seconds. To handle this wrap around behavior,
> +  * we need the value of each counter at the time of wrap around which
> +  * FW does not provide. For now ignore the data.
> +  */

If you really think about it, this can be calculated.

Channel visit shouldn't be longer than 24s, not even 12s. It's mostly
20-100ms for hw_scan and up to 5s in offchannel (which is rare
anyway).

Hence you can detect the wrap around AND calculate the actual time difference.

Your patch seems like it's whitespace-damaged. Please use git send-email.

Also fix styling issues, please - you have typo in the comment, modify
a comment line needlessly, exceed 80 chars in line and use multiple
spaces between operator `<`.


Michał