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

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

[Manikandan Pillai]

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@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 +-
 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);
 }
 
 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;
+	return readl(&timer_base->tcrr) >> 5;	/* current tick value */
 }
 
 /* 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
 
 /*-----------------------------------------------------------------------
  * Stack sizes
diff --git a/include/configs/omap3_evm.h b/include/configs/omap3_evm.h
index f4498a9..59305b8 100644
--- a/include/configs/omap3_evm.h
+++ b/include/configs/omap3_evm.h
@@ -226,7 +226,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
 
 /*-----------------------------------------------------------------------
  * Stack sizes
diff --git a/include/configs/omap3_overo.h b/include/configs/omap3_overo.h
index dee0417..7bacb5b 100644
--- a/include/configs/omap3_overo.h
+++ b/include/configs/omap3_overo.h
@@ -220,7 +220,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
 
 /*-----------------------------------------------------------------------
  * Stack sizes
diff --git a/include/configs/omap3_pandora.h b/include/configs/omap3_pandora.h
index 00c0374..0ee1602 100644
--- a/include/configs/omap3_pandora.h
+++ b/include/configs/omap3_pandora.h
@@ -222,7 +222,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
 
 /*-----------------------------------------------------------------------
  * Stack sizes
-- 
1.5.6

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

[Dirk Behme]

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@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?

>  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.

>  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?

> +	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

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

[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@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

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

[Wolfgang Denk]

Dear "Pillai, Manikandan",

In message <19F8576C6E063C45BE387C64729E73940427CBF3D2@dbde02.ent.ti.com> you wrote:
>
> > > -	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 ???

What makes you sure it is NOT needed?

> > 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.

Argh... We don't need realy high precision, but I think we should
attempt to not worse than +/- 2% or so.  THis implementation is not
acceptable.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"I can call spirits from the vasty deep."
"Why so can I, or so can any man; but will they come when you do call
for them?"          - Shakespeare, 1 King Henry IV, Act III, Scene I.