[U-Boot] [PATCH 1/1] The GP timer is fixed for 1msec and CONFIG_SYS_HZ to 1000.

[Pillai, Manikandan]

Pls find my comments inlined.

> -----Original Message-----
> From: Dirk Behme [mailto:dirk.behme at googlemail.com]
> Sent: Tuesday, March 17, 2009 12:15 AM
> To: Pillai, Manikandan
> Cc: u-boot at lists.denx.de
> Subject: Re: [PATCH 1/1] The GP timer is fixed for 1msec and CONFIG_SYS_HZ to
> 1000.
> 
> Dear Mani,
> 
> some comments below.
> 
> I will have a look to this soon. Maybe I will come with a patch then,
> too.
> 
> Manikandan Pillai wrote:
> > The header files for other OAMP3 boards have also been changed
> > for CONFIG_SYS_HZ to be 1000.
> >
> > Signed-off-by: Manikandan Pillai <mani.pillai at ti.com>
> > ---
> >  cpu/arm_cortexa8/omap3/interrupts.c |   12 +++---------
> >  include/configs/omap3_beagle.h      |    2 +-
> >  include/configs/omap3_evm.h         |    2 +-
> >  include/configs/omap3_overo.h       |    2 +-
> >  include/configs/omap3_pandora.h     |    2 +-
> 
> zoom1 missing?
[Pillai, Manikandan] OK. I have tested only on OMAP3 EVM since I do not have
any of the other boards.

> 
> >  5 files changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/cpu/arm_cortexa8/omap3/interrupts.c
> b/cpu/arm_cortexa8/omap3/interrupts.c
> > index 9e9817d..d16f7bc 100644
> > --- a/cpu/arm_cortexa8/omap3/interrupts.c
> > +++ b/cpu/arm_cortexa8/omap3/interrupts.c
> > @@ -234,19 +234,13 @@ void reset_timer_masked(void)
> >  	/* reset time, capture current incrementer value time */
> >  	lastinc = readl(&timer_base->tcrr);
> >  	timestamp = 0;		/* start "advancing" time stamp from 0 */
> > +	/* reset the timer count  */
> > +	writel(0x1, &timer_base->ttgr);
> 
> Not sure if this is necessary (correct?) here.
[Pillai, Manikandan] Writing into this register clears the register count.
The reset_timer() functions needs to clear the count in this register.
> 
> >  ulong get_timer_masked(void)
> >  {
> > -	ulong now = readl(&timer_base->tcrr); /* current tick value */
> > -
> > -	if (now >= lastinc)	/* normal mode (non roll) */
> > -		/* move stamp fordward with absoulte diff ticks */
> > -		timestamp += (now - lastinc);
> > -	else	/* we have rollover of incrementer */
> > -		timestamp += (0xFFFFFFFF - lastinc) + now;
> > -	lastinc = now;
> > -	return timestamp;
> 
> As already mentioned in
> 
> http://lists.denx.de/pipermail/u-boot/2009-March/048842.html
> 
> I don't understand why you remove the overflow handling here?
[Pillai, Manikandan] Is overflow handling really required here ???

> 
> > +	return readl(&timer_base->tcrr) >> 5;	/* current tick value */
> 
> Hmm, here you shift by 5. In your first patch (linked above) you had 6.
> 
> Let's have a look to the clocking used here: If I'm right, the timer
> runs with 13MHz. Then we divide by 256 (2^(7+1)), this results in
> 50781.25Hz. Is this correct?
> 
> With '>> 6' this will result in 793.457Hz, with '>> 5' we will get
> 1586.914Hz. Correct?
> 
> Both doesn't fit
[Pillai, Manikandan] I knew it doesn't give the correct values. This was tested
using sleep() function. Since 5 gives a > time than 1 second, I picked up 5 here.
> 
> CONFIG_SYS_HZ			1000
> 
> ?
> 
> At the moment I think the best way to go is to have a look how DaVinci
> is dealing with this. The only difference I see at the moment between
> OMAP3 and DaVinci concept is that DaVinci timer runs with 27MHz.
> 
> >  /* waits specified delay value and resets timestamp */
> > diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h
> > index 9057606..346df43 100644
> > --- a/include/configs/omap3_beagle.h
> > +++ b/include/configs/omap3_beagle.h
> > @@ -227,7 +227,7 @@
> >
> >  #define CONFIG_SYS_TIMERBASE		(OMAP34XX_GPT2)
> >  #define CONFIG_SYS_PVT			V_PVT	/* 2^(pvt+1) */
> > -#define CONFIG_SYS_HZ			((V_SCLK) / (2 << CONFIG_SYS_PVT))
> > +#define CONFIG_SYS_HZ			1000
> 
> Looking at this in the config files and the comments around it, it
> seems to me that we should do some more clean up here. E.g. update the
> comment above these macros to OMAP3 and replace V_PVT directly with 7.
> 
> Dirk