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

Thu Feb 16 16:52:44 UTC 2017

Hi Phil,

> -----Original Message-----
> From: Phil Edworthy [mailto:phil.edworthy at renesas.com]
> Sent: Wednesday, February 15, 2017 11:22 PM
> To: Vikas MANOCHA <vikas.manocha at st.com>; Albert Aribaud <albert.u.boot at aribaud.net>
> Cc: Tom Rini <trini at konsulko.com>; Kamil Lulko <kamil.lulko at gmail.com>; Michael Kurz <michi.kurz at gmail.com>; u-
> boot at lists.denx.de
> Subject: RE: [PATCH v2] armv7m: Add SysTick timer driver
> 
> Hi Vikas,
> 
> On 15 February 2017 23:50, Vikas MANOCHA wrote:
> > Hi Phil,
> >
> > > -----Original Message-----
> > > From: Phil Edworthy [mailto:phil.edworthy at renesas.com]
> > > Sent: Monday, February 13, 2017 11:48 PM
> > > To: Albert Aribaud <albert.u.boot at aribaud.net>
> > > Cc: Tom Rini <trini at konsulko.com>; Vikas MANOCHA
> > <vikas.manocha at st.com>; Kamil Lulko <kamil.lulko at gmail.com>; Michael
> > > Kurz <michi.kurz at gmail.com>; u-boot at lists.denx.de; Phil Edworthy
> > <phil.edworthy at renesas.com>
> > > Subject: [PATCH v2] armv7m: Add SysTick timer driver
> > >
> > > 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>
> > > ---
> > > v2:
> > >  - Variables & constant renamed.
> > >  - Use the calibration reg to determine if we use the cpu or ref clk
> > >  - Use the calibration reg to get the clk rate, unless specified
> > > ---
> > >  arch/arm/cpu/armv7m/Makefile        |   2 +
> > >  arch/arm/cpu/armv7m/systick-timer.c | 107
> > ++++++++++++++++++++++++++++++++++++
> > >  2 files changed, 109 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..62308f9
> > > --- /dev/null
> > > +++ b/arch/arm/cpu/armv7m/systick-timer.c
> > > @@ -0,0 +1,107 @@
> > > +/*
> > > + * 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+
> > > + *
> > > + * The SysTick timer is a 24-bit count down timer. The clock can be
> > > +either the
> > > + * CPU clock or a reference clock. Since the timer will wrap around
> > > +very quickly
> > > + * when using the CPU clock, and we do not handle the timer
> > > +interrupts, it is
> > > + * expected that this driver is only ever used with a slow reference clock.
> > > + */
> > > +
> > > +#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_val;
> > > +	uint32_t current_val;
> > > +	uint32_t calibration;
> > > +};
> > > +
> > > +#define TIMER_MAX_VAL		0x00FFFFFF
> > > +#define SYSTICK_CTRL_EN		BIT(0)
> > > +/* Clock source: 0 = Ref clock, 1 = CPU clock */
> > > +#define SYSTICK_CTRL_CPU_CLK	BIT(2)
> > > +#define SYSTICK_CAL_NOREF	BIT(31)
> > > +#define SYSTICK_CAL_SKEW	BIT(30)
> > > +#define SYSTICK_CAL_TENMS_MASK	0x00FFFFFF
> > > +
> > > +/* 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_MAX_VAL - readl(&systick->current_val); }
> > > +
> > > +int timer_init(void)
> > > +{
> > > +	struct cm3_systick *systick = (struct cm3_systick *)SYSTICK_BASE;
> > > +	u32 cal;
> > > +
> > > +	writel(TIMER_MAX_VAL, &systick->reload_val);
> > > +	/* Any write to current_val reg clears it to 0 */
> > > +	writel(0, &systick->current_val);
> > > +
> > > +	cal = readl(&systick->calibration);
> > > +	if (cal & SYSTICK_CAL_NOREF)
> >
> > Good.
> >
> > > +		/* 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);
> > > +
> > > +#if defined(CONFIG_SYS_HZ_CLOCK)
> > > +	gd->arch.timer_rate_hz = CONFIG_SYS_HZ_CLOCK; #else
> > > +	gd->arch.timer_rate_hz = (cal & SYSTICK_CAL_TENMS_MASK) * 100;
> >
> > This value is implementation dependent, it can't replace
> > CONFIG_SYS_HZ_CLOCK.
> >
> > Let me explain a bit,
> > Here the assumption is  calibration value is always 1ms at any systick
> > clock frequency which is not true. Different arm vendors write
> > different values in this register to have timer value for 1 or 10 ms
> > at one particular clock frequency, on top of it this particular
> > frequency is also different for all vendors.
> 
> Hmm, I think it's vendor defined because the clock rate can be different from different vendors. The field is named TENMS for a
> reason and the ARM description says that it is the ticks for 10ms.
> 
> However, I also realise that sometimes this field is wrong, that is why I put:
> > > +#if defined(CONFIG_SYS_HZ_CLOCK)
> This allows anyone to define the clock rate in their system, if the CAL_TENMS field does not match the ticks needed for 10ms.

Ok, let's keep it but add sufficient info about it in the comment here & in the commit message.

Cheers,
Vikas

> 
> > Otherwise,
> > Reviewed-by: Vikas MANOCHA <vikas.manocha at st.com>
> Thanks!
> Phil
> 
> > Cheers,
> > Vikas
> >
> > > #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 / gd->arch.timer_rate_hz)) - 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_MAX_VAL - gd->arch.lastinc) + now;
> > > +
> > > +	gd->arch.lastinc = now;
> > > +
> > > +	return gd->arch.tbl;
> > > +}
> > > +
> > > +ulong get_tbclk(void)
> > > +{
> > > +	return gd->arch.timer_rate_hz;
> > > +}
> > > --
> > > 2.7.4