Re: [PATCH 1/2] clocksource/drivers/exynos_mct: Prioritise Arm arch timer on arm64

[Chanwoo Choi]

Hi,

On 6/9/21 12:43 AM, Will Deacon wrote:
> All arm64 CPUs feature an architected timer, which offers a relatively
> low-latency interface to a per-cpu clocksource and timer. For the most
> part, using this interface is a no-brainer, with the exception of SoCs
> where it cannot be used to wake up from deep idle state (i.e.
> CLOCK_EVT_FEAT_C3STOP is set).
> 
> On the contrary, the Exynos MCT is extremely slow to access yet can be
> used as a wakeup source. In preparation for using the Exynos MCT as a
> potential wakeup timer for the Arm architected timer, reduce its ratings
> so that the architected timer is preferred.
> 
> This effectively reverts the decision made in 6282edb72bed
> ("clocksource/drivers/exynos_mct: Increase priority over ARM arch timer")
> for arm64, as the reasoning for the original change was to work around
> a 32-bit SoC design.
> 
> Cc: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> Cc: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Cc: Chanwoo Choi <cw00.choi@xxxxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Signed-off-by: Will Deacon <will@xxxxxxxxxx>
> ---
>  drivers/clocksource/exynos_mct.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clocksource/exynos_mct.c b/drivers/clocksource/exynos_mct.c
> index fabad79baafc..804d3e01c8f4 100644
> --- a/drivers/clocksource/exynos_mct.c
> +++ b/drivers/clocksource/exynos_mct.c
> @@ -51,6 +51,15 @@
>  
>  #define TICK_BASE_CNT	1
>  
> +#ifdef CONFIG_ARM
> +/* Use values higher than ARM arch timer. See 6282edb72bed. */
> +#define MCT_CLKSOURCE_RATING		450
> +#define MCT_CLKEVENTS_RATING		500
> +#else
> +#define MCT_CLKSOURCE_RATING		350
> +#define MCT_CLKEVENTS_RATING		350
> +#endif
> +
>  enum {
>  	MCT_INT_SPI,
>  	MCT_INT_PPI
> @@ -206,7 +215,7 @@ static void exynos4_frc_resume(struct clocksource *cs)
>  
>  static struct clocksource mct_frc = {
>  	.name		= "mct-frc",
> -	.rating		= 450,	/* use value higher than ARM arch timer */
> +	.rating		= MCT_CLKSOURCE_RATING,
>  	.read		= exynos4_frc_read,
>  	.mask		= CLOCKSOURCE_MASK(32),
>  	.flags		= CLOCK_SOURCE_IS_CONTINUOUS,
> @@ -457,7 +466,7 @@ static int exynos4_mct_starting_cpu(unsigned int cpu)
>  	evt->set_state_oneshot_stopped = set_state_shutdown;
>  	evt->tick_resume = set_state_shutdown;
>  	evt->features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
> -	evt->rating = 500;	/* use value higher than ARM arch timer */
> +	evt->rating = MCT_CLKEVENTS_RATING,
>  
>  	exynos4_mct_write(TICK_BASE_CNT, mevt->base + MCT_L_TCNTB_OFFSET);
>  
> 

I'm not sure that exynos mct is working without problem
such as the case of 6282edb72bed.
As described on On ,6282edb72bed the arch timer on exynos SoC
depends on Exynos MCT device. the arch timer is not able to work
without Exynos MCT because of using the common module.

-- 
Best Regards,
Chanwoo Choi
Samsung Electronics