[U-Boot] [PATCH resend] armv7/ltimer: Add support for local timer on armv7 cpus

[Armando Visconti]

Ciao Vipin,

Yes, I agree about the need to have the generic
local_timer support in u-boot.

Internally I was not able to give comment about this
part. So, see my comments now.

On 12/06/2012 10:22 AM, Vipin KUMAR wrote:
> Certain ARMV7 cpus eg. CortexA9 contains a local and a global timer within the
> CPU core itself.  This patch adds generic support for local timer.
>
> Signed-off-by: Vipin Kumar<vipin.kumar at st.com>
> ---
>   arch/arm/cpu/armv7/Makefile       |  11 ++-
>   arch/arm/cpu/armv7/ca9_ltimer.c   | 152 ++++++++++++++++++++++++++++++++++++++
>   arch/arm/include/asm/ca9_ltimer.h |  40 ++++++++++
>   3 files changed, 199 insertions(+), 4 deletions(-)
>   create mode 100644 arch/arm/cpu/armv7/ca9_ltimer.c
>   create mode 100644 arch/arm/include/asm/ca9_ltimer.h
>
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index 4fdbee4..3ef01f6 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -27,15 +27,18 @@ LIB	= $(obj)lib$(CPU).o
>
>   START	:= start.o
>
> -COBJS	+= cache_v7.o
> +COBJS-y	+= cache_v7.o
>
> -COBJS	+= cpu.o
> -COBJS	+= syslib.o
> +COBJS-y	+= cpu.o
> +COBJS-y	+= syslib.o
> +COBJS-$(CONFIG_ARMV7_CA9LTIMER) += ca9_ltimer.o
>

Is it really necessary to have the 'ca9' prefix here?
I think it  would be better to stay more generic here,
like: 'CONFIG_ARMV7_LTIMER' and 'ltimer.o'.

In linux as well is kept generic, even across architectures...

If accepted, apply globally...

>   ifneq ($(CONFIG_AM33XX)$(CONFIG_OMAP44XX)$(CONFIG_OMAP54XX)$(CONFIG_TEGRA20),)
> -SOBJS	+= lowlevel_init.o
> +SOBJS-y	+= lowlevel_init.o
>   endif
>
> +COBJS	:= $(sort $(COBJS-y))
> +SOBJS	:= $(sort $(SOBJS-y))
>   SRCS	:= $(START:.o=.S) $(COBJS:.o=.c)
>   OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
>   START	:= $(addprefix $(obj),$(START))
> diff --git a/arch/arm/cpu/armv7/ca9_ltimer.c b/arch/arm/cpu/armv7/ca9_ltimer.c
> new file mode 100644
> index 0000000..cbf1552
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/ca9_ltimer.c
> @@ -0,0 +1,152 @@
> +/*
> + * (C) Copyright 2012
> + * Vipin Kumar, ST Micoelectronics, vipin.kumar at st.com.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include<common.h>
> +#include<asm/io.h>
> +#include<asm/ca9_ltimer.h>
> +#include<asm/arch/hardware.h>
> +
> +#define READ_TIMER()	readl(&ca9_timer_p->count)
> +
> +static struct ca9_timer_regs *const ca9_timer_p =
> +	(struct ca9_timer_regs *)CONFIG_ARMV7_LTIMER_BASE;
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define timestamp	gd->tbl
> +#define lastdec		gd->lastinc
> +#define tickshz		gd->timer_rate_hz
> +#define ticksper10usec	gd->tbu
> +
> +int timer_init(void)
> +{
> +	u32 prescaler, timertickshz;
> +	/*
> +	 * Genrally, CortexA9 MPUs are operating from 500MHz to 1500MHz which
> +	 * means that CA9 local timer clock would be in the range of 250 MHz to
> +	 * 750MHz.
> +	 * Try to find a prescaler which can perfectly divide the local timer
> +	 * clock. Take prescaler as 200 if nothing is found
> +	 */
> +	for (prescaler = 255; prescaler>  1; prescaler--) {
> +		if (CONFIG_ARMV7_LTMR_CLK ==
> +				(CONFIG_ARMV7_LTMR_CLK / prescaler) * prescaler)
> +			break;
> +	}
> +
> +	if (prescaler == 1)
> +		prescaler = 200;

Where the default '200' prescaler selection come from?
Shouldn't it be a configurable option (i.e. CONFIG_)?
Or passed as an argument to this function?

> +	timertickshz = CONFIG_ARMV7_LTMR_CLK / prescaler;
> +	ticksper10usec = timertickshz / (100 * 1000);
> +	tickshz = timertickshz / CONFIG_SYS_HZ;
> +
> +	/* disable timers */
> +	writel(((prescaler - 1)<<  8) | AUTO_RELOAD,&ca9_timer_p->control);
> +

Why can't single-shot be selectable?
Shouldn't it be passed as an argument to timer_init()?

> +	/* load value for free running */
> +	writel(FREE_RUNNING,&ca9_timer_p->load);
> +
> +	/* auto reload, start timer */
> +	setbits_le32(&ca9_timer_p->control, TIMER_ENABLE);
> +
> +	reset_timer_masked();
> +
> +	return 0;
> +}
> +
> +/*
> + * timer without interrupts
> + */
> +
> +void reset_timer(void)
> +{
> +	reset_timer_masked();
> +}
> +
> +ulong get_timer(ulong base)
> +{
> +	return (get_timer_masked() / tickshz) - base;
> +}
> +
> +void set_timer(ulong t)
> +{
> +	timestamp = t;
> +}
> +
> +void __udelay(unsigned long usec)
> +{
> +	ulong tmo;
> +	ulong start = get_timer_masked();
> +	ulong rndoff;
> +
> +	rndoff = (usec % 10) ? 1 : 0;
> +	tmo = ((usec / 10) + rndoff) * ticksper10usec;
> +
> +	while ((ulong) (get_timer_masked() - start)<  tmo);
> +}
> +
> +void reset_timer_masked(void)
> +{
> +	/* reset time */
> +	lastdec = READ_TIMER();
> +	timestamp = 0;
> +}
> +
> +ulong get_timer_masked(void)
> +{
> +	ulong now = READ_TIMER();
> +
> +	if (now<= lastdec) {
> +		/* normal mode */
> +		timestamp += lastdec - now;
> +	} else {
> +		/* we have an overflow ... */
> +		timestamp += lastdec + FREE_RUNNING - now;
> +	}
> +	lastdec = now;
> +
> +	return timestamp;
> +}
> +
> +void udelay_masked(unsigned long usec)
> +{
> +	return udelay(usec);
> +}
> +
> +/*
> + * This function is derived from PowerPC code (read timebase as long long).
> + * On ARM it just returns the timer value.
> + */
> +unsigned long long get_ticks(void)
> +{
> +	return get_timer(0);
> +}
> +
> +/*
> + * This function is derived from PowerPC code (timebase clock frequency).
> + * On ARM it returns the number of timer ticks per second.
> + */
> +ulong get_tbclk(void)
> +{
> +	return CONFIG_SYS_HZ;
> +}
> diff --git a/arch/arm/include/asm/ca9_ltimer.h b/arch/arm/include/asm/ca9_ltimer.h
> new file mode 100644
> index 0000000..8833853
> --- /dev/null
> +++ b/arch/arm/include/asm/ca9_ltimer.h
> @@ -0,0 +1,40 @@
> +/*
> + * (C) Copyright 2012
> + * Vipin Kumar, ST Micoelectronics, vipin.kumar at st.com.
> + *
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#ifndef __ARCH_ARM_CA9TIMER_H
> +#define __ARCH_ARM_CA9TIMER_H
> +
> +struct ca9_timer_regs {
> +	u32 load;
> +	u32 count;
> +	u32 control;
> +};
> +
> +/* control related definitions */
> +#define AUTO_RELOAD		(1<<  1)
> +#define TIMER_ENABLE		(1<<  0)
> +
> +/* load related definitions */
> +#define FREE_RUNNING		(0xFFFFFFFF)
> +
> +#endif

Seems good otherwise.
Reveiewed-by:armando.visconti at st.com