[U-Boot] [PATCH] armv7m: Add SysTick timer driver

[Phil Edworthy]

Hi Vikas,

On 12 February 2017 20:53, Vikas MANOCHA wrote:
> > On Fri, Feb 03, 2017 at 02:48:40PM +0000, Phil Edworthy wrote:
> >
> > > The SysTick is a 24-bit down counter that is found on all ARM Cortex
> > > M3, M4, M7 devices and is always located at a fixed address.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edworthy at renesas.com>
> > > ---
> > >  arch/arm/cpu/armv7m/Makefile        |  2 +
> > >  arch/arm/cpu/armv7m/systick-timer.c | 91
> > > +++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 93 insertions(+)
> > >  create mode 100644 arch/arm/cpu/armv7m/systick-timer.c
> > >
> > > diff --git a/arch/arm/cpu/armv7m/Makefile
> > > b/arch/arm/cpu/armv7m/Makefile index aff60e8..e1a6c40 100644
> > > --- a/arch/arm/cpu/armv7m/Makefile
> > > +++ b/arch/arm/cpu/armv7m/Makefile
> > > @@ -7,3 +7,5 @@
> > >
> > >  extra-y := start.o
> > >  obj-y += cpu.o
> > > +
> > > +obj-$(CONFIG_SYS_ARCH_TIMER) += systick-timer.o
> > > diff --git a/arch/arm/cpu/armv7m/systick-timer.c
> > > b/arch/arm/cpu/armv7m/systick-timer.c
> > > new file mode 100644
> > > index 0000000..6ccc2fb
> > > --- /dev/null
> > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > @@ -0,0 +1,91 @@
> > > +/*
> > > + * ARM Cortex M3/M4/M7 SysTick timer driver
> > > + * (C) Copyright 2017 Renesas Electronics Europe Ltd
> > > + *
> > > + * Based on arch/arm/mach-stm32/stm32f1/timer.c
> > > + * (C) Copyright 2015
> > > + * Kamil Lulko, <kamil.lulko at gmail.com>
> > > + *
> > > + * Copyright 2015 ATS Advanced Telematics Systems GmbH
> > > + * Copyright 2015 Konsulko Group, Matt Porter <mporter at konsulko.com>
> > > + *
> > > + * SPDX-License-Identifier:     GPL-2.0+
> > > + */
> > > +
> > > +#include <common.h>
> > > +#include <asm/io.h>
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +/* SysTick Base Address - fixed for all Cortex M3, M4 and M7 devices */
> > > +#define SYSTICK_BASE		0xE000E010
> > > +
> > > +struct cm3_systick {
> > > +	uint32_t ctrl;
> > > +	uint32_t reload;
> > > +	uint32_t val;
> 
> This register is current timer value, write of any value clears it.
> Rename to cur_val would avoid any confusion.
Ok, though I'm not sure that cur_val is any clearer :)

> > > +	uint32_t calibration;
> > > +};
> > > +
> > > +#define TIMER_LOAD_VAL		0x00FFFFFF
> > > +#define SYSTICK_CTRL_EN		BIT(0)
> > > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > > +#define SYSTICK_CTRL_CPU_CLK	BIT(2)
> > > +
> > > +/* read the 24-bit timer */
> > > +static ulong read_timer(void)
> > > +{
> > > +	struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > +	/* The timer counts down, therefore convert to an incrementing timer */
> > > +	return TIMER_LOAD_VAL - readl(&systick->val); }
> 
> use mask to be sure of 24bit timer counter value.
Why? It's a 24-bit timer, the other bits are always zero.
 
> > > +
> > > +int timer_init(void)
> > > +{
> > > +	struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +
> > > +	writel(TIMER_LOAD_VAL, &systick->reload);
> > > +	writel(TIMER_LOAD_VAL, &systick->val);
> 
> Writing load value gives the impression that it is load register, in fact it is only
> clearing countflag.
Ok

> > > +
> > > +#ifdef CONFIG_ARMCORTEXM3_SYSTICK_CPU
> 
> Cortex M3 systick has only cpu clock source ?
No, it can have cpu clock or another clock external to the M3 core.
In our case we use the external clock because it is much slower than the
cpu clock, making the timer usable without interrupts. I can't imagine
anyone using the cpu clock unless they add code to handle the interrupt.
I'll add a comment to that effect.

Thanks
Phil

> Cheers,
> Vikas
> 
> > > +	/* Use CPU clock, no interrupts */
> > > +	writel(SYSTICK_CTRL_EN | SYSTICK_CTRL_CPU_CLK, &systick->ctrl);
> > > +#else
> > > +	/* Use external clock, no interrupts */
> > > +	writel(SYSTICK_CTRL_EN, &systick->ctrl); #endif
> > > +
> > > +	gd->arch.tbl = 0;
> > > +	gd->arch.tbu = 0;
> > > +	gd->arch.lastinc = read_timer();
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/* return milli-seconds timer value */ ulong get_timer(ulong base) {
> > > +	unsigned long long t = get_ticks() * 1000;
> > > +
> > > +	return (ulong)((t / CONFIG_SYS_HZ_CLOCK)) - base; }
> > > +
> > > +unsigned long long get_ticks(void)
> > > +{
> > > +	u32 now = read_timer();
> > > +
> > > +	if (now >= gd->arch.lastinc)
> > > +		gd->arch.tbl += (now - gd->arch.lastinc);
> > > +	else
> > > +		gd->arch.tbl += (TIMER_LOAD_VAL - gd->arch.lastinc) + now;
> > > +
> > > +	gd->arch.lastinc = now;
> > > +
> > > +	return gd->arch.tbl;
> > > +}
> > > +
> > > +ulong get_tbclk(void)
> > > +{
> > > +	return CONFIG_SYS_HZ_CLOCK;
> > > +}
> >
> > And (cc'ing maintainers) we could use this in place of arch/arm/mach-
> stm32/*/timer.c I assume, yes?  Thanks!
> >
> > --
> > Tom