Re: [PATCH] RFC: arm64: arch_timer: Fix timer inconsistency test for A64

[Ondřej Jirman]

Hello Roman,

On Tue, Sep 29, 2020 at 02:13:47PM +0300, Roman Stratiienko wrote:
> Fixes linux_kselftest:timers_inconsistency-check_arm_64
> 
> Test logs without the fix:
> '''
> binary returned non-zero. Exit code: 1, stderr: , stdout:
> Consistent CLOCK_REALTIME
> 1601335525:467086804
> 1601335525:467087554
> 1601335525:467088345
> 1601335525:467089095
> 1601335525:467089887
> 1601335525:467090637
> 1601335525:467091429
> 1601335525:467092179
> 1601335525:467092929
> 1601335525:467093720
> 1601335525:467094470
> 1601335525:467095262
> 1601335525:467096012
> 1601335525:467096804
> --------------------
> 1601335525:467097554
> 1601335525:467077012
> --------------------
> 1601335525:467099095
> 1601335525:467099845
> 1601335525:467100637
> 1601335525:467101387
> 1601335525:467102179
> 1601335525:467102929
> '''

Can you reproduce the issue with a fixed CPU frequency. I suspect the root
cause is around CPU frequency scaling code on A64, and timer jumps happen when
the kernel is changing CPU frequency.

I fixed a similar issue on H3 SoC just by changing the CPU frequency scaling
code, without having to touch the timer readout code.

https://megous.com/git/linux/commit/?h=ths-5.9&id=51ff1a6d80126f678efca42555f93efa611f50c4

regards,
	o.

> Signed-off-by: Roman Stratiienko <r.stratiienko@xxxxxxxxx>
> CC: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> CC: linux-kernel@xxxxxxxxxxxxxxx
> CC: linux-sunxi@xxxxxxxxxxxxxxxx
> CC: megous@xxxxxxxxxx
> ---
>  drivers/clocksource/arm_arch_timer.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 6c3e841801461..d50aa43cb654b 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -346,16 +346,17 @@ static u64 notrace arm64_858921_read_cntvct_el0(void)
>   * number of CPU cycles in 3 consecutive 24 MHz counter periods.
>   */
>  #define __sun50i_a64_read_reg(reg) ({					\
> -	u64 _val;							\
> +	u64 _val1, _val2;						\
>  	int _retries = 150;						\
>  									\
>  	do {								\
> -		_val = read_sysreg(reg);				\
> +		_val1 = read_sysreg(reg);				\
> +		_val2 = read_sysreg(reg);				\
>  		_retries--;						\
> -	} while (((_val + 1) & GENMASK(9, 0)) <= 1 && _retries);	\
> +	} while (((_val2 - _val1) > 0x10) && _retries);			\
>  									\
>  	WARN_ON_ONCE(!_retries);					\
> -	_val;								\
> +	_val2;								\
>  })
>  
>  static u64 notrace sun50i_a64_read_cntpct_el0(void)
> -- 
> 2.25.1
>