clocksource/drivers/arm_arch_timer: Add workaround for MediaTek MMIO timer

  From: Walter Chang <walter.chang@mediatek.com>

The MT69XX series SoCs have the incomplete implementation issue in the
mmio timer. Specifically, the hardware only implements the TVAL
functionality, but not the CVAL functionality. This hardware limitation
will cause set_next_event_mem() fail to set the actual expiration time
when writing a value to the CVAL. On these platforms, the mmio timer's
internal expiration time will still be judged as 0 (the value of TVAL),
resulting in the mmio timer not functioning as intended.

The workaround is to use TVAL in addition to CVAL for these affected
platforms.

Signed-off-by: Walter Chang <walter.chang@mediatek.com>
---
 Documentation/arm64/silicon-errata.rst |  4 ++++
 drivers/clocksource/Kconfig            |  9 ++++++++
 drivers/clocksource/arm_arch_timer.c   | 29 ++++++++++++++++++++++++++
 3 files changed, 42 insertions(+)
  

Il 17/04/23 11:06, walter.chang@mediatek.com ha scritto:
> From: Walter Chang <walter.chang@mediatek.com>
> 
> The MT69XX series SoCs have the incomplete implementation issue in the
> mmio timer. Specifically, the hardware only implements the TVAL
> functionality, but not the CVAL functionality. This hardware limitation
> will cause set_next_event_mem() fail to set the actual expiration time
> when writing a value to the CVAL. On these platforms, the mmio timer's
> internal expiration time will still be judged as 0 (the value of TVAL),
> resulting in the mmio timer not functioning as intended.
> 
> The workaround is to use TVAL in addition to CVAL for these affected
> platforms.
> 
> Signed-off-by: Walter Chang <walter.chang@mediatek.com>
> ---
>   Documentation/arm64/silicon-errata.rst |  4 ++++
>   drivers/clocksource/Kconfig            |  9 ++++++++
>   drivers/clocksource/arm_arch_timer.c   | 29 ++++++++++++++++++++++++++
>   3 files changed, 42 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> index ec5f889d7681..ca1893713a4c 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -209,3 +209,7 @@ stable kernels.
>   +----------------+-----------------+-----------------+-----------------------------+
>   | Fujitsu        | A64FX           | E#010001        | FUJITSU_ERRATUM_010001      |
>   +----------------+-----------------+-----------------+-----------------------------+
> +
> ++----------------+-----------------+-----------------+-----------------------------+
> +| MediaTek       | MT69XX series   | #690001         | MEDIATEK_ERRATUM_690001     |
> ++----------------+-----------------+-----------------+-----------------------------+
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 5fc8f0e7fb38..475356b8dbdc 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -368,6 +368,15 @@ config SUN50I_ERRATUM_UNKNOWN1
>   	  the Allwinner A64 SoC. The workaround will only be active if the
>   	  allwinner,erratum-unknown1 property is found in the timer node.
>   
> +config MEDIATEK_ERRATUM_690001
> +	bool "Workaround for MediaTek MT69XX erratum 690001"
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for incomplete implementation
> +	  in the MMIO timer on the MediaTek MT69XX SoCs. The workaround
> +	  will only be active if mediatek,erratum-690001 property is
> +	  found in the timer node.
> +
>   config ARM_GLOBAL_TIMER
>   	bool "Support for the ARM global timer" if COMPILE_TEST
>   	select TIMER_OF if OF
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index e09d4427f604..920570d57fc0 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -389,6 +389,10 @@ static u64 notrace sun50i_a64_read_cntvct_el0(void)
>   }
>   #endif
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +static bool arch_timer_mem_sne_use_tval __ro_after_init;

What about reusing part of the CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND logic and
adding something like CONFIG_ARM_ARCH_TIMER_MEM_WORKAROUND?

That would make you able to reuse the already existing infrastructure to parse
the device-tree and possibly more.

Regards,
Angelo

> +#endif
> +
>   #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
>   DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *, timer_unstable_counter_workaround);
>   EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
> @@ -783,6 +787,19 @@ static __always_inline void set_next_event_mem(const int access, unsigned long e
>   		cnt = arch_counter_get_cnt_mem(timer, CNTPCT_LO);
>   
>   	arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt, clk);
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	if (arch_timer_mem_sne_use_tval) {
> +		/* Due to the incomplete implementation of mmio timer on
> +		 * specific MediaTek platforms, CVAL has not been implemented.
> +		 * Therefore, the workaround is to use TVAL in addition to
> +		 * CVAL.
> +		 */
> +		if (access == ARCH_TIMER_MEM_VIRT_ACCESS)
> +			writel_relaxed(evt, timer->base + 0x38);
> +		else
> +			writel_relaxed(evt, timer->base + 0x28);
> +	}
> +#endif
>   	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>   }
>   
> @@ -878,7 +895,16 @@ static void __arch_timer_setup(unsigned type,
>   				arch_timer_set_next_event_phys_mem;
>   		}
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +		if (arch_timer_mem_sne_use_tval) {
> +			pr_info("Enabling mediatek,erratum-690001 for mmio timer\n");
> +			max_delta = CLOCKSOURCE_MASK(31);
> +		} else {
> +			max_delta = CLOCKSOURCE_MASK(56);
> +		}
> +#else
>   		max_delta = CLOCKSOURCE_MASK(56);
> +#endif
>   	}
>   
>   	clk->set_state_shutdown(clk);
> @@ -1591,6 +1617,9 @@ static int __init arch_timer_mem_of_init(struct device_node *np)
>   		frame->valid = true;
>   	}
>   
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	arch_timer_mem_sne_use_tval = of_property_read_bool(np, "mediatek,erratum-690001");
> +#endif
>   	frame = arch_timer_mem_find_best_frame(timer_mem);
>   	if (!frame) {
>   		pr_err("Unable to find a suitable frame in timer @ %pa\n",
  

On Mon, 17 Apr 2023 11:01:15 +0100,
AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote:
> 
> Il 17/04/23 11:06, walter.chang@mediatek.com ha scritto:
> > From: Walter Chang <walter.chang@mediatek.com>
> > 
> > The MT69XX series SoCs have the incomplete implementation issue in the
> > mmio timer. Specifically, the hardware only implements the TVAL
> > functionality, but not the CVAL functionality. This hardware limitation
> > will cause set_next_event_mem() fail to set the actual expiration time
> > when writing a value to the CVAL. On these platforms, the mmio timer's
> > internal expiration time will still be judged as 0 (the value of TVAL),
> > resulting in the mmio timer not functioning as intended.
> > 
> > The workaround is to use TVAL in addition to CVAL for these affected
> > platforms.
> > 
> > Signed-off-by: Walter Chang <walter.chang@mediatek.com>
> > ---
> >   Documentation/arm64/silicon-errata.rst |  4 ++++
> >   drivers/clocksource/Kconfig            |  9 ++++++++
> >   drivers/clocksource/arm_arch_timer.c   | 29 ++++++++++++++++++++++++++
> >   3 files changed, 42 insertions(+)
> > 
> > diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst
> > index ec5f889d7681..ca1893713a4c 100644
> > --- a/Documentation/arm64/silicon-errata.rst
> > +++ b/Documentation/arm64/silicon-errata.rst
> > @@ -209,3 +209,7 @@ stable kernels.
> >   +----------------+-----------------+-----------------+-----------------------------+
> >   | Fujitsu        | A64FX           | E#010001        | FUJITSU_ERRATUM_010001      |
> >   +----------------+-----------------+-----------------+-----------------------------+
> > +
> > ++----------------+-----------------+-----------------+-----------------------------+
> > +| MediaTek       | MT69XX series   | #690001         | MEDIATEK_ERRATUM_690001     |
> > ++----------------+-----------------+-----------------+-----------------------------+
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 5fc8f0e7fb38..475356b8dbdc 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -368,6 +368,15 @@ config SUN50I_ERRATUM_UNKNOWN1
> >   	  the Allwinner A64 SoC. The workaround will only be active if the
> >   	  allwinner,erratum-unknown1 property is found in the timer node.
> >   +config MEDIATEK_ERRATUM_690001
> > +	bool "Workaround for MediaTek MT69XX erratum 690001"
> > +	depends on ARM_ARCH_TIMER && ARM64
> > +	help
> > +	  This option enables a workaround for incomplete implementation
> > +	  in the MMIO timer on the MediaTek MT69XX SoCs. The workaround
> > +	  will only be active if mediatek,erratum-690001 property is
> > +	  found in the timer node.
> > +
> >   config ARM_GLOBAL_TIMER
> >   	bool "Support for the ARM global timer" if COMPILE_TEST
> >   	select TIMER_OF if OF
> > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> > index e09d4427f604..920570d57fc0 100644
> > --- a/drivers/clocksource/arm_arch_timer.c
> > +++ b/drivers/clocksource/arm_arch_timer.c
> > @@ -389,6 +389,10 @@ static u64 notrace sun50i_a64_read_cntvct_el0(void)
> >   }
> >   #endif
> >   +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> > +static bool arch_timer_mem_sne_use_tval __ro_after_init;
> 
> What about reusing part of the CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND logic and
> adding something like CONFIG_ARM_ARCH_TIMER_MEM_WORKAROUND?

I don't think there is *any* need for the out-of-line stuff on any of
the memory-mapped accesses, if only because they are not inlined the
first place, and that there is no performance requirements around this
timer (it should hardly ever be used as the sched clock, for example).

Despite being part of the same driver, the sysreg and MMIO timers are
vastly different and keeping them together has only been a nuisance.
Someone should throw an axe at this stuff.

	M.
  

On Mon, 2023-04-17 at 17:06 +0800, walter.chang@mediatek.com wrote:
> From: Walter Chang <walter.chang@mediatek.com>
> 
> The MT69XX series SoCs have the incomplete implementation issue in
> the
> mmio timer. Specifically, the hardware only implements the TVAL
> functionality, but not the CVAL functionality. This hardware
> limitation
> will cause set_next_event_mem() fail to set the actual expiration
> time
> when writing a value to the CVAL. On these platforms, the mmio
> timer's
> internal expiration time will still be judged as 0 (the value of
> TVAL),
> resulting in the mmio timer not functioning as intended.
> 
> The workaround is to use TVAL in addition to CVAL for these affected
> platforms.
> 
> Signed-off-by: Walter Chang <walter.chang@mediatek.com>

Reviewed-by: Chun-Hung Wu <chun-hung.wu@mediatek.com>

> ---
>  Documentation/arm64/silicon-errata.rst |  4 ++++
>  drivers/clocksource/Kconfig            |  9 ++++++++
>  drivers/clocksource/arm_arch_timer.c   | 29
> ++++++++++++++++++++++++++
>  3 files changed, 42 insertions(+)
> 
> diff --git a/Documentation/arm64/silicon-errata.rst
> b/Documentation/arm64/silicon-errata.rst
> index ec5f889d7681..ca1893713a4c 100644
> --- a/Documentation/arm64/silicon-errata.rst
> +++ b/Documentation/arm64/silicon-errata.rst
> @@ -209,3 +209,7 @@ stable kernels.
>  +----------------+-----------------+-----------------+------------
> -----------------+
>  | Fujitsu        | A64FX           | E#010001        |
> FUJITSU_ERRATUM_010001      |
>  +----------------+-----------------+-----------------+------------
> -----------------+
> +
> ++----------------+-----------------+-----------------+------------
> -----------------+
> +| MediaTek       | MT69XX series   | #690001         |
> MEDIATEK_ERRATUM_690001     |
> ++----------------+-----------------+-----------------+------------
> -----------------+
> diff --git a/drivers/clocksource/Kconfig
> b/drivers/clocksource/Kconfig
> index 5fc8f0e7fb38..475356b8dbdc 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -368,6 +368,15 @@ config SUN50I_ERRATUM_UNKNOWN1
>  	  the Allwinner A64 SoC. The workaround will only be active if
> the
>  	  allwinner,erratum-unknown1 property is found in the timer
> node.
>  
> +config MEDIATEK_ERRATUM_690001
> +	bool "Workaround for MediaTek MT69XX erratum 690001"
> +	depends on ARM_ARCH_TIMER && ARM64
> +	help
> +	  This option enables a workaround for incomplete
> implementation
> +	  in the MMIO timer on the MediaTek MT69XX SoCs. The workaround
> +	  will only be active if mediatek,erratum-690001 property is
> +	  found in the timer node.
> +
>  config ARM_GLOBAL_TIMER
>  	bool "Support for the ARM global timer" if COMPILE_TEST
>  	select TIMER_OF if OF
> diff --git a/drivers/clocksource/arm_arch_timer.c
> b/drivers/clocksource/arm_arch_timer.c
> index e09d4427f604..920570d57fc0 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -389,6 +389,10 @@ static u64 notrace
> sun50i_a64_read_cntvct_el0(void)
>  }
>  #endif
>  
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +static bool arch_timer_mem_sne_use_tval __ro_after_init;
> +#endif
> +
>  #ifdef CONFIG_ARM_ARCH_TIMER_OOL_WORKAROUND
>  DEFINE_PER_CPU(const struct arch_timer_erratum_workaround *,
> timer_unstable_counter_workaround);
>  EXPORT_SYMBOL_GPL(timer_unstable_counter_workaround);
> @@ -783,6 +787,19 @@ static __always_inline void
> set_next_event_mem(const int access, unsigned long e
>  		cnt = arch_counter_get_cnt_mem(timer, CNTPCT_LO);
>  
>  	arch_timer_reg_write(access, ARCH_TIMER_REG_CVAL, evt + cnt,
> clk);
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	if (arch_timer_mem_sne_use_tval) {
> +		/* Due to the incomplete implementation of mmio timer
> on
> +		 * specific MediaTek platforms, CVAL has not been
> implemented.
> +		 * Therefore, the workaround is to use TVAL in addition
> to
> +		 * CVAL.
> +		 */
> +		if (access == ARCH_TIMER_MEM_VIRT_ACCESS)
> +			writel_relaxed(evt, timer->base + 0x38);
> +		else
> +			writel_relaxed(evt, timer->base + 0x28);
> +	}
> +#endif
>  	arch_timer_reg_write(access, ARCH_TIMER_REG_CTRL, ctrl, clk);
>  }
>  
> @@ -878,7 +895,16 @@ static void __arch_timer_setup(unsigned type,
>  				arch_timer_set_next_event_phys_mem;
>  		}
>  
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +		if (arch_timer_mem_sne_use_tval) {
> +			pr_info("Enabling mediatek,erratum-690001 for
> mmio timer\n");
> +			max_delta = CLOCKSOURCE_MASK(31);
> +		} else {
> +			max_delta = CLOCKSOURCE_MASK(56);
> +		}
> +#else
>  		max_delta = CLOCKSOURCE_MASK(56);
> +#endif
>  	}
>  
>  	clk->set_state_shutdown(clk);
> @@ -1591,6 +1617,9 @@ static int __init arch_timer_mem_of_init(struct
> device_node *np)
>  		frame->valid = true;
>  	}
>  
> +#ifdef CONFIG_MEDIATEK_ERRATUM_690001
> +	arch_timer_mem_sne_use_tval = of_property_read_bool(np,
> "mediatek,erratum-690001");
> +#endif
>  	frame = arch_timer_mem_find_best_frame(timer_mem);
>  	if (!frame) {
>  		pr_err("Unable to find a suitable frame in timer @
> %pa\n",