From mboxrd@z Thu Jan  1 00:00:00 1970
From: Mark Rutland <mark.rutland@arm.com>
Date: Fri, 20 Mar 2015 18:21:36 +0000
Subject: [U-Boot] [PATCH v2 04/28] armv8/ls2085a: Fix generic timer
	clock source
In-Reply-To: <550C5D11.4010704@freescale.com>
References: <1426797262-20850-1-git-send-email-yorksun@freescale.com>
 <20150320173413.GA14290@leverpostej>
 <550C5D11.4010704@freescale.com>
Message-ID: <20150320182136.GA24455@leverpostej>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

> >> +int timer_init(void)
> >> +{
> >> +	u32 __iomem *cntcr = (u32 *)CONFIG_SYS_FSL_TIMER_ADDR;
> >> +	u32 __iomem *cltbenr = (u32 *)CONFIG_SYS_FSL_PMU_CLTBENR;
> >> +#ifdef COUNTER_FREQUENCY_REAL
> >> +	unsigned long cntfrq = COUNTER_FREQUENCY_REAL;
> >> +
> >> +	/* Update with accurate clock frequency */
> >> +	asm volatile("msr cntfrq_el0, %0" : : "r" (cntfrq) : "memory");
> > 
> > The commit message says that this can only be determined at runtime, but
> > this looks like we're writing a compile-time static value.
> > 
> 
> The macro COUNTER_FREQUENCY_REA is (CONFIG_SYS_CLK_FREQ/4), where
> CONFIG_SYS_CLK_FREQ is a function call get_board_sys_clk().

Ah, that sounds fine to me then.

> >> +	__real_cntfrq = cntfrq;	/* update for secondary cores */
> > 
> > Do we need anything in the way or barriers and/or cache flushing to
> > ensure that this is visible to the secondary CPUs? Or is the MMU off at
> > this point?
> 
> It is flushed before booting secondary cores. But I am relying on the trick of
> enabling cache on flash. It may not be as reliable if someone decide to disable
> the cache to begin with. I will move the code to somewhere safe in next version.

Ok.

> >> +	.global __real_cntfrq
> >> +__real_cntfrq:
> >> +	.quad 0x17d7840		/* 25MHz */
> > 
> > I think this would be better as COUNTER_FREQUENCY, so as to avoid
> > duplicating the value.
> 
> Good idea. Will fix in next version.

Great!

Mark.