[U-Boot-Users] [PATCH] [resend] mips: Support to set CFG_HZ to 1000, consistent with other architectures

[U-Boot-Users] [PATCH] [resend] mips: Support to set CFG_HZ to 1000, consistent with other architectures

[Jason McMullan]

All existing MIPS boards define CFG_HZ to be the number of ticks per second
for the timebase. However, CFG_HZ should be a constant 1000 for all boards.

This patch set CFG_HZ to be defined as 1000 for all MIPS boards, and uses
the new configuration value CFG_MIPS_CLOCK to calculate the timebase.

The MIPS time code now has a software maintained 64-bit counter, however
get_timer() must be called every few seconds for this counter to be fully
reliable.

Per-board patches to board config files are included.

Signed-off-by: Jason McMullan <mcmullan@netapp.com>
---
 include/configs/dbau1x00.h  |    3 ++-
 include/configs/gth2.h      |    3 ++-
 include/configs/incaip.h    |    3 ++-
 include/configs/pb1x00.h    |    3 ++-
 include/configs/purple.h    |    3 ++-
 include/configs/qemu-mips.h |    3 ++-
 include/configs/tb0229.h    |    3 ++-
 lib_mips/time.c             |   36 +++++++++++++++++++++++++++++++++++-
 8 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/include/configs/dbau1x00.h b/include/configs/dbau1x00.h
index b2f606f..b4cc0c2 100644
--- a/include/configs/dbau1x00.h
+++ b/include/configs/dbau1x00.h
@@ -148,7 +148,8 @@
 #error "Invalid CPU frequency - must be multiple of 12!"
 #endif
 
-#define CFG_HZ                  (CFG_MHZ * 1000000) /* FIXME causes overflow in net.c */
+#define CFG_MIPS_CLOCK		(CFG_MHZ * 1000000)
+#define CFG_HZ                  1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/gth2.h b/include/configs/gth2.h
index c2a50c1..656eccb 100644
--- a/include/configs/gth2.h
+++ b/include/configs/gth2.h
@@ -117,8 +117,9 @@
 #define CFG_BOOTPARAMS_LEN	128*1024
 
 #define CFG_MHZ			500
+#define CFG_MIPS_CLOCK		(CFG_MHZ * 1000000)
 
-#define CFG_HZ			(CFG_MHZ * 1000000) /* FIXME causes overflow in net.c */
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/incaip.h b/include/configs/incaip.h
index 5ca00b3..c0e26f2 100644
--- a/include/configs/incaip.h
+++ b/include/configs/incaip.h
@@ -118,7 +118,8 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			(incaip_get_cpuclk() / 2)
+#define CFG_MIPS_CLOCK		(incaip_get_cpuclk() / 2)
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000
 
diff --git a/include/configs/pb1x00.h b/include/configs/pb1x00.h
index 810e0f0..5cb5069 100644
--- a/include/configs/pb1x00.h
+++ b/include/configs/pb1x00.h
@@ -81,7 +81,8 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			396000000      /* FIXME causes overflow in net.c */
+#define CFG_MIPS_CLOCK		396000000
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/purple.h b/include/configs/purple.h
index 1be4e05..9fe98f2 100644
--- a/include/configs/purple.h
+++ b/include/configs/purple.h
@@ -114,7 +114,8 @@
 #define	CFG_PROMPT		"PURPLE # "	/* Monitor Command Prompt    */
 #define	CFG_CBSIZE		256		/* Console I/O Buffer Size   */
 #define	CFG_PBSIZE (CFG_CBSIZE+sizeof(CFG_PROMPT)+16)  /* Print Buffer Size */
-#define CFG_HZ			(CPU_CLOCK_RATE/2)
+#define CFG_MIPS_CLOCK		(CPU_CLOCK_RATE/2)
+#define CFG_HZ			1000
 #define	CFG_MAXARGS		16		/* max number of command args*/
 
 #define	CFG_LOAD_ADDR		0x80500000	/* default load address	*/
diff --git a/include/configs/qemu-mips.h b/include/configs/qemu-mips.h
index d6bcc8e..6889d7f 100644
--- a/include/configs/qemu-mips.h
+++ b/include/configs/qemu-mips.h
@@ -119,8 +119,9 @@
 #define CFG_BOOTPARAMS_LEN	128*1024
 
 #define CFG_MHZ			132
+#define CFG_MIPS_CLOCK		(132 * 1000000)
 
-#define CFG_HZ			(CFG_MHZ * 1000000)
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000	/* Cached addr */
 
diff --git a/include/configs/tb0229.h b/include/configs/tb0229.h
index dadf5d3..82206d5 100644
--- a/include/configs/tb0229.h
+++ b/include/configs/tb0229.h
@@ -122,7 +122,8 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			(CPU_TCLOCK_RATE/4)
+#define CFG_MIPS_CLOCK		(CPU_TCLOCK_RATE/4)
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000
 
diff --git a/lib_mips/time.c b/lib_mips/time.c
index cd8dc72..4807f1a 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -23,23 +23,57 @@
 
 #include <common.h>
 
+/* CFG_MIPS_CLOCK is the number of ticks per second of the MIPS C0 Clock timer.
+ *
+ * For most implementations, this is the same as the CPU speed in HZ
+ * divided by 2. Some embedded MIPS implementations may use a /4
+ * or /1 divider, so see your CPU reference manual for specific details.
+ */
+#ifndef CFG_MIPS_CLOCK
+#error CFG_MIPS_CLOCK must be set in the board configuration file
+#endif
 
+static struct {
+	uint32_t lo;
+	uint32_t hi;
+} mips_ticks;	/* Last number of ticks seen */
+
+/* Input is in CFG_HZ ticks */
 static inline void mips_compare_set(u32 v)
 {
+	v *= (CFG_MIPS_CLOCK / CFG_HZ);
 	asm volatile ("mtc0 %0, $11" : : "r" (v));
 }
 
+/* Input is in CFG_HZ ticks */
 static inline void mips_count_set(u32 v)
 {
+	v *= (CFG_MIPS_CLOCK / CFG_HZ);
+	mips_ticks.lo = v;
+	mips_ticks.hi = 0;
 	asm volatile ("mtc0 %0, $9" : : "r" (v));
 }
 
 
+/* Returns CFG_HZ ticks 
+ *
+ * NOTE: This must be called at least once every
+ *       few seconds to be reliable.
+ */
 static inline u32 mips_count_get(void)
 {
 	u32 count;
 
 	asm volatile ("mfc0 %0, $9" : "=r" (count) :);
+
+	/* Handle 32-bit timer overflow */
+	if (count < mips_ticks.lo) {
+		mips_ticks.hi++;
+	}
+	mips_ticks.lo = count;
+	count =(mips_ticks.lo / (CFG_MIPS_CLOCK / CFG_HZ)) +
+	       (mips_ticks.hi * (0x100000000ULL / (CFG_MIPS_CLOCK / CFG_HZ)));
+
 	return count;
 }
 
@@ -75,7 +109,7 @@ void udelay (unsigned long usec)
 	ulong tmo;
 	ulong start = get_timer(0);
 
-	tmo = usec * (CFG_HZ / 1000000);
+	tmo = usec * CFG_HZ / 1000;
 	while ((ulong)((mips_count_get() - start)) < tmo)
 		/*NOP*/;
 }
-- 
1.5.4.3

[U-Boot-Users] [PATCH] [resend] mips: Support to set CFG_HZ to 1000, consistent with other architectures

[Shinya Kuribayashi]

Hi Jason,

I don't look closely yet, but find some comments below:

Jason McMullan wrote:
> ---
>  include/configs/dbau1x00.h  |    3 ++-
>  include/configs/gth2.h      |    3 ++-
>  include/configs/incaip.h    |    3 ++-
>  include/configs/pb1x00.h    |    3 ++-
>  include/configs/purple.h    |    3 ++-
>  include/configs/qemu-mips.h |    3 ++-
>  include/configs/tb0229.h    |    3 ++-

Ok.

> diff --git a/lib_mips/time.c b/lib_mips/time.c
> index cd8dc72..4807f1a 100644
> --- a/lib_mips/time.c
> +++ b/lib_mips/time.c
> @@ -23,23 +23,57 @@
>  
>  #include <common.h>
>  
> +/* CFG_MIPS_CLOCK is the number of ticks per second of the MIPS C0 Clock timer.
> + *
> + * For most implementations, this is the same as the CPU speed in HZ
> + * divided by 2. Some embedded MIPS implementations may use a /4
> + * or /1 divider, so see your CPU reference manual for specific details.
> + */
> +#ifndef CFG_MIPS_CLOCK
> +#error CFG_MIPS_CLOCK must be set in the board configuration file
> +#endif
>  
> +static struct {
> +	uint32_t lo;
> +	uint32_t hi;
> +} mips_ticks;	/* Last number of ticks seen */

See below.

> +/* Input is in CFG_HZ ticks */
>  static inline void mips_compare_set(u32 v)
>  {
> +	v *= (CFG_MIPS_CLOCK / CFG_HZ);
>  	asm volatile ("mtc0 %0, $11" : : "r" (v));
>  }

I tend to remove these mips_{count,compare}_{get,set} functions because
they can be (and should be) replaced {read,write}_32bit_ cp0_register,
IMO.

And I want to handle (CFG_MIPS_CLOCK/CFG_HZ) stuffs or something like
that at udelay() side.

> +/* Returns CFG_HZ ticks 
> + *
> + * NOTE: This must be called at least once every
> + *       few seconds to be reliable.
> + */
>  static inline u32 mips_count_get(void)
>  {
>  	u32 count;
>  
>  	asm volatile ("mfc0 %0, $9" : "=r" (count) :);
> +
> +	/* Handle 32-bit timer overflow */
> +	if (count < mips_ticks.lo) {
> +		mips_ticks.hi++;
> +	}
> +	mips_ticks.lo = count;
> +	count =(mips_ticks.lo / (CFG_MIPS_CLOCK / CFG_HZ)) +
> +	       (mips_ticks.hi * (0x100000000ULL / (CFG_MIPS_CLOCK / CFG_HZ)));
> +
>  	return count;
>  }

I disagree with having this structure. Basic strategy for MIPS COUNT/
COMPARE handling is, let them overflow (os should I say wrap-around) as
they are. All we need is the Delta, not the numbers of overflows.

> @@ -75,7 +109,7 @@ void udelay (unsigned long usec)
>  	ulong tmo;
>  	ulong start = get_timer(0);
>  
> -	tmo = usec * (CFG_HZ / 1000000);
> +	tmo = usec * CFG_HZ / 1000;
>  	while ((ulong)((mips_count_get() - start)) < tmo)
>  		/*NOP*/;
>  }

Again, what is needed is `the Delta' between CP0.COUNT(Previous) and
CP0.COUNT(Current). This can be always achieved by

    delta = COUNT(Curr) - COUNT(Prev)

regardless of COUNT value. Even if `(u32)0x00010000 - (u32)0xFFFFFFFF',
it works.

I'll look around until this weekend. Sorry for inconvinience, and thank
you for working on this.

  Shinya

[U-Boot-Users] [PATCH] [resend] mips: Support to set CFG_HZ to 1000, consistent with other architectures

[Scott Wood]

On Wed, May 21, 2008 at 12:53:01PM +0900, Shinya Kuribayashi wrote:
> I disagree with having this structure. Basic strategy for MIPS COUNT/
> COMPARE handling is, let them overflow (os should I say wrap-around) as
> they are. All we need is the Delta, not the numbers of overflows.

You *do* need the full, non-overflowing counter if you want to provide
the 32-bit millisecond clock that u-boot wants.  Read the recent
discussion on CFG_HZ that led to this patch.

-Scott

[U-Boot-Users] [PATCH 1/3][MIPS] lib_mips/time.c: Replace CP0 access functions with existing macros

[Shinya Kuribayashi]

Scott Wood wrote:
> On Wed, May 21, 2008 at 12:53:01PM +0900, Shinya Kuribayashi wrote:
>> I disagree with having this structure. Basic strategy for MIPS COUNT/
>> COMPARE handling is, let them overflow (os should I say wrap-around) as
>> they are. All we need is the Delta, not the numbers of overflows.
> 
> You *do* need the full, non-overflowing counter if you want to provide
> the 32-bit millisecond clock that u-boot wants.  Read the recent
> discussion on CFG_HZ that led to this patch.

Ok, here's my proposal. Conceptual patches for MIPS timer routines.
There might be still room for improvement, but I'd like to see something
like thease.

Just build tested. Any comments are appriciated.


  Shinya

---

[MIPS] lib_mips/time.c: Replace CP0 access functions with existing macros

We already have many pre-defined CP0 access macros in <asm/mipsregs.h>.
This patch replaces mips_{compare,count}_set and mips_count_get with
existing macros.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 lib_mips/time.c |   35 ++++++++---------------------------
 1 files changed, 8 insertions(+), 27 deletions(-)


diff --git a/lib_mips/time.c b/lib_mips/time.c
index cd8dc72..f03f023 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -22,26 +22,7 @@
  */
 
 #include <common.h>
-
-
-static inline void mips_compare_set(u32 v)
-{
-	asm volatile ("mtc0 %0, $11" : : "r" (v));
-}
-
-static inline void mips_count_set(u32 v)
-{
-	asm volatile ("mtc0 %0, $9" : : "r" (v));
-}
-
-
-static inline u32 mips_count_get(void)
-{
-	u32 count;
-
-	asm volatile ("mfc0 %0, $9" : "=r" (count) :);
-	return count;
-}
+#include <asm/mipsregs.h>
 
 /*
  * timer without interrupts
@@ -49,25 +30,25 @@ static inline u32 mips_count_get(void)
 
 int timer_init(void)
 {
-	mips_compare_set(0);
-	mips_count_set(0);
+	write_32bit_cp0_register(CP0_COMPARE, 0);
+	write_32bit_cp0_register(CP0_COUNT, 0);
 
 	return 0;
 }
 
 void reset_timer(void)
 {
-	mips_count_set(0);
+	write_32bit_cp0_register(CP0_COUNT, 0);
 }
 
 ulong get_timer(ulong base)
 {
-	return mips_count_get() - base;
+	return read_32bit_cp0_register(CP0_COUNT) - base;
 }
 
 void set_timer(ulong t)
 {
-	mips_count_set(t);
+	write_32bit_cp0_register(CP0_COUNT, t);
 }
 
 void udelay (unsigned long usec)
@@ -76,7 +57,7 @@ void udelay (unsigned long usec)
 	ulong start = get_timer(0);
 
 	tmo = usec * (CFG_HZ / 1000000);
-	while ((ulong)((mips_count_get() - start)) < tmo)
+	while ((ulong)((read_32bit_cp0_register(CP0_COUNT) - start)) < tmo)
 		/*NOP*/;
 }
 
@@ -86,7 +67,7 @@ void udelay (unsigned long usec)
  */
 unsigned long long get_ticks(void)
 {
-	return mips_count_get();
+	return read_32bit_cp0_register(CP0_COUNT);
 }
 
 /*

[U-Boot-Users] [PATCH 2/3][MIPS] lib_mips/time.c: Fix udelay

[Shinya Kuribayashi]

What we have to do is just to wait for given micro-seconds. No need to
take into account current time, get_timer and CFG_HZ.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 lib_mips/time.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)


diff --git a/lib_mips/time.c b/lib_mips/time.c
index f03f023..154d792 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -51,13 +51,13 @@ void set_timer(ulong t)
 	write_32bit_cp0_register(CP0_COUNT, t);
 }
 
-void udelay (unsigned long usec)
+void udelay(unsigned long usec)
 {
 	ulong tmo;
-	ulong start = get_timer(0);
+	unsigned int start = read_32bit_cp0_register(CP0_COUNT);
 
-	tmo = usec * (CFG_HZ / 1000000);
-	while ((ulong)((read_32bit_cp0_register(CP0_COUNT) - start)) < tmo)
+	tmo = start + (usec * (CONFIG_MIPS_TIMER_FREQ / 1000000));
+	while ((ulong)(tmo - read_32bit_cp0_register(CP0_COUNT)) < 0x7fffffff)
 		/*NOP*/;
 }
 

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Shinya Kuribayashi]

MIPS port has two problems in timer routines. One is now we assume CFG_HZ
== CP0 counter frequency, but this is wrong. CFG_HZ has to be 1000 in the
U-Boot system.

The other is we don't have a proper time management counter like timestamp
other ARCHs have. We need the 32-bit millisecond clock that U-Boot wants.

This patch adds 3 global variables; timestamp, cycles_per_jiffy, expirelo.
timestamp is a 32-bit non-overflowing CFG_HZ counter. cycles_per_jiffy is
calculated counter cycles in a CFG_HZ. And expirelo holds the count value
for next CPU timer expiration.

With these variables, fix each functions. Notably,

* timer_init: Initialize cycles_per_jiffy, timestamp, and expirelo. Note
  that we don't have to initialize CP0 count/compare registers here. They
  have been already cleared on the system reset. Leave them as they are.

* get_timer: Calculate how many timestamps have been passed, then return
  (relative) timestamp. I'm afraid we might suffer from a big catch up
  loop if this function is called after a long delay.

* get_ticks: Return the current timestamp, that is get_timer(0).

Most parts are from good old Linux v2.6.16 kernel.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 lib_mips/time.c |   44 ++++++++++++++++++++++++++++++++++++++------
 1 files changed, 38 insertions(+), 6 deletions(-)


diff --git a/lib_mips/time.c b/lib_mips/time.c
index 154d792..4b47e41 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -24,31 +24,63 @@
 #include <common.h>
 #include <asm/mipsregs.h>
 
+static unsigned long timestamp;
+
+/* how many counter cycles in a jiffy */
+static unsigned long cycles_per_jiffy;
+
+/* expirelo is the count value for next CPU timer interrupt */
+static unsigned int expirelo;
+
 /*
  * timer without interrupts
  */
 
 int timer_init(void)
 {
-	write_32bit_cp0_register(CP0_COMPARE, 0);
-	write_32bit_cp0_register(CP0_COUNT, 0);
+	/* Calculate cache parameters. */
+	cycles_per_jiffy = (CONFIG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ;
+
+	/* Report the high precision timer rate for a reference. */
+	printf("Using %u.%03u MHz high precision timer.\n",
+	       ((CONFIG_MIPS_TIMER_FREQ + 500) / 1000) / 1000,
+	       ((CONFIG_MIPS_TIMER_FREQ + 500) / 1000) % 1000);
+
+	/* Set up the timer for the first expiration. */
+	timestamp = 0;
+	expirelo = read_32bit_cp0_register(CP0_COUNT) + cycles_per_jiffy;
 
 	return 0;
 }
 
 void reset_timer(void)
 {
-	write_32bit_cp0_register(CP0_COUNT, 0);
+	timestamp = 0;
+	expirelo = read_32bit_cp0_register(CP0_COUNT) + cycles_per_jiffy;
 }
 
 ulong get_timer(ulong base)
 {
-	return read_32bit_cp0_register(CP0_COUNT) - base;
+	unsigned int count;
+
+	/* Check to see if we have missed any timestamps. */
+	count = read_32bit_cp0_register(CP0_COUNT);
+	while ((count - expirelo) < 0x7fffffff) {
+		/*
+		 * FIXME: We might suffer from a big catch up loop
+		 * if called after a long delay.
+		 */
+		expirelo += cycles_per_jiffy;
+		timestamp++;
+	}
+
+	return (timestamp - base);
 }
 
 void set_timer(ulong t)
 {
-	write_32bit_cp0_register(CP0_COUNT, t);
+	timestamp = t;
+	expirelo = read_32bit_cp0_register(CP0_COUNT) + cycles_per_jiffy;
 }
 
 void udelay(unsigned long usec)
@@ -67,7 +99,7 @@ void udelay(unsigned long usec)
  */
 unsigned long long get_ticks(void)
 {
-	return read_32bit_cp0_register(CP0_COUNT);
+	return get_timer(0);
 }
 
 /*

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Wolfgang Denk]

In message <483811E4.3070209@ruby.dti.ne.jp> you wrote:
> 
> This patch adds 3 global variables; timestamp, cycles_per_jiffy, expirelo.
> timestamp is a 32-bit non-overflowing CFG_HZ counter. cycles_per_jiffy is
> calculated counter cycles in a CFG_HZ. And expirelo holds the count value
> for next CPU timer expiration.

Be careful - keep in mind that variables in the data segment will  be
read-only  before  relocation  to  RAM,  and variables in BSS will be
uninitialized *and* get overwritten  with  zero  when  the  BSS  gets
initialized.

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
If a group of N persons implements a COBOL compiler,  there  will  be
N-1 passes. Someone in the group has to be the manager. - T. Cheatham

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Shinya Kuribayashi]

Wolfgang Denk wrote:
> Be careful - keep in mind that variables in the data segment will  be
> read-only  before  relocation  to  RAM,  and variables in BSS will be
> uninitialized *and* get overwritten  with  zero  when  the  BSS  gets
> initialized.

Sure. We'll probably need to move timer_init to board_init_r at the end.

  Shinya

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Wolfgang Denk]

In message <4838C92D.7020700@ruby.dti.ne.jp> you wrote:
> Wolfgang Denk wrote:
> > Be careful - keep in mind that variables in the data segment will  be
> > read-only  before  relocation  to  RAM,  and variables in BSS will be
> > uninitialized *and* get overwritten  with  zero  when  the  BSS  gets
> > initialized.
> 
> Sure. We'll probably need to move timer_init to board_init_r at the end.

I'm not sure if this works. We use udelay() and friends all over the
place - a long time before that.

The timer implementation should have no such restrictions as being
available only after relocation. Timers / delays are such a basic
feature that they should be available everywhere.

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
>  Is there a way to determine Yesterday's date using Unix utilities?
         echo "what is yesterday's date?" | /bin/mail root
         -- Randal L. Schwartz in <ukbuh2y982.fsf@julie.teleport.com>

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Shinya Kuribayashi]

Wolfgang Denk wrote:
>> Sure. We'll probably need to move timer_init to board_init_r at the end.
> 
> I'm not sure if this works. We use udelay() and friends all over the
> place - a long time before that.

Hmm.

My udelay is already global-variable-free. And as for cycles_per_jiffy
and expirelo, we could make it work like this:

diff --git a/lib_mips/time.c b/lib_mips/time.c
index f6678e1..d4754e3 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -27,10 +27,7 @@
 static unsigned long timestamp;
 
 /* how many counter cycles in a jiffy */
-static unsigned long cycles_per_jiffy;
-
-/* expirelo is the count value for next CPU timer interrupt */
-static unsigned int expirelo;
+static unsigned long cycles_per_jiffy = (CONFIG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ;
 
 /*
  * timer without interrupts
@@ -38,9 +35,6 @@ static unsigned int expirelo;
 
 int timer_init(void)
 {
-	/* Calculate cache parameters. */
-	cycles_per_jiffy = (CONFIG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ;
-
 	/* Report the high precision timer rate for a reference. */
 	printf("Using %u.%03u MHz high precision timer.\n",
 	       ((CONFIG_MIPS_TIMER_FREQ + 500) / 1000) / 1000,
@@ -48,7 +42,7 @@ int timer_init(void)
 
 	/* Set up the timer for the first expiration. */
 	timestamp = 0;
-	expirelo = read_c0_count() + cycles_per_jiffy;
+	write_c0_compare(read_c0_count() + cycles_per_jiffy);
 
 	return 0;
 }
@@ -56,12 +50,13 @@ int timer_init(void)
 void reset_timer(void)
 {
 	timestamp = 0;
-	expirelo = read_c0_count() + cycles_per_jiffy;
+	write_c0_compare(read_c0_count() + cycles_per_jiffy);
 }
 
 ulong get_timer(ulong base)
 {
 	unsigned int count;
+	unsigned int expirelo = read_c0_compare();
 
 	/* Check to see if we have missed any timestamps. */
 	count = read_c0_count();
@@ -73,6 +68,7 @@ ulong get_timer(ulong base)
 		expirelo += cycles_per_jiffy;
 		timestamp++;
 	}
+	write_c0_compare(expirelo);
 
 	return (timestamp - base);
 }
@@ -80,7 +76,7 @@ ulong get_timer(ulong base)
 void set_timer(ulong t)
 {
 	timestamp = t;
-	expirelo = read_c0_count() + cycles_per_jiffy;
+	write_c0_compare(read_c0_count() + cycles_per_jiffy);
 }
 
 void udelay(unsigned long usec)
_


But I have no clue about timestamp.


> The timer implementation should have no such restrictions as being
> available only after relocation. Timers / delays are such a basic
> feature that they should be available everywhere.

I'll remember that.


  Shinya

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Wolfgang Denk]

In message <48396D8C.8050402@ruby.dti.ne.jp> you wrote:
>
> My udelay is already global-variable-free. And as for cycles_per_jiffy
> and expirelo, we could make it work like this:
...
> -static unsigned int expirelo;
> +static unsigned long cycles_per_jiffy = (CONFIG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ;

If it's a constant anyway - then why do we need a variable for it?

> But I have no clue about timestamp.

Maybe there is a register that is (1) common to all or at least most
of the supported processors we could use for that? If not, then we
probably have to use the initial data structure for this.

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 think it's a new feature. Don't tell anyone it was an accident. :-)
  -- Larry Wall on s/foo/bar/eieio in <10911@jpl-devvax.JPL.NASA.GOV>

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Shinya Kuribayashi]

Wolfgang Denk wrote:
> In message <48396D8C.8050402@ruby.dti.ne.jp> you wrote:
>> My udelay is already global-variable-free. And as for cycles_per_jiffy
>> and expirelo, we could make it work like this:
> ...
>> -static unsigned int expirelo;
>> +static unsigned long cycles_per_jiffy = (CONFIG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ;
> 
> If it's a constant anyway - then why do we need a variable for it?

Of course, we don't. That was for easy review, will fix.

>> But I have no clue about timestamp.
> 
> Maybe there is a register that is (1) common to all or at least most
> of the supported processors we could use for that? If not, then we

I'm afraid there's not such register left.

> probably have to use the initial data structure for this.

Looked around board_init_r, and it seems there is no {set,get}_timer
user there. I'm going to leave timestamp as it is at this moment.


  Shinya

[U-Boot-Users] [PATCH 3/3][MIPS] lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

[Shinya Kuribayashi]

>> probably have to use the initial data structure for this.
> 
> Looked around board_init_r, and it seems there is no {set,get}_timer
> user there. I'm going to leave timestamp as it is at this moment.

s/board_init_r/board_init_f/

[U-Boot-Users] [PATCH 3/3 v2][MIPS] lib_mips/time.c: Fix CP0 count register usage and timer routines

[Shinya Kuribayashi]

MIPS port has two problems in timer routines. One is now we assume CFG_HZ
equals to CP0 counter frequency, but this is wrong. CFG_HZ has to be 1000
in the U-Boot system.

The other is we don't have a proper time management counter like timestamp
other ARCHs have. We need the 32-bit millisecond clock counter.

This patch introduces timestamp and CYCLES_PER_JIFFY. timestamp is a
32-bit non-overflowing CFG_HZ counter, and CYCLES_PER_JIFFY is the number
of calculated CP0 counter cycles in a CFG_HZ.

STRATEGY:

* Fix improper CFG_HZ value to have 1000

* Use CFG_MIPS_TIMER_FREQ for timer counter frequency, instead.

* timer_init: initialize timestamp and set up the first timer expiration.
  Note that we don't need to initialize CP0 count/compare registers here
  as they have been already zeroed out on the system reset. Leave them as
  they are.

* get_timer: calculate how many timestamps have been passed, then return
  base-relative timestamp. Make sure we can easily count missed timestamps
  regardless of CP0 count/compare value.

* get_ticks: return the current timestamp, that is get_timer(0).

Most parts are from good old Linux v2.6.16 kernel.

v2:
- Remove FIXME comments as they turned out to be trivial.
- Use CP0 compare register as a global variable for expirelo.
- Kill a global variable 'cycles_per_jiffy'. Use #define CYCLES_PER_JIFFY
  instead.

Signed-off-by: Shinya Kuribayashi <skuribay@ruby.dti.ne.jp>
---

 include/configs/dbau1x00.h  |    4 +++-
 include/configs/gth2.h      |    4 +++-
 include/configs/incaip.h    |    4 +++-
 include/configs/pb1x00.h    |    4 +++-
 include/configs/purple.h    |    3 ++-
 include/configs/qemu-mips.h |    4 +++-
 include/configs/tb0229.h    |    4 +++-
 lib_mips/time.c             |   31 +++++++++++++++++++++++++------
 8 files changed, 45 insertions(+), 13 deletions(-)


diff --git a/include/configs/dbau1x00.h b/include/configs/dbau1x00.h
index b2f606f..45ff1e7 100644
--- a/include/configs/dbau1x00.h
+++ b/include/configs/dbau1x00.h
@@ -148,7 +148,9 @@
 #error "Invalid CPU frequency - must be multiple of 12!"
 #endif
 
-#define CFG_HZ                  (CFG_MHZ * 1000000) /* FIXME causes overflow in net.c */
+#define CFG_MIPS_TIMER_FREQ	(CFG_MHZ * 1000000)
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/gth2.h b/include/configs/gth2.h
index c2a50c1..23618db 100644
--- a/include/configs/gth2.h
+++ b/include/configs/gth2.h
@@ -118,7 +118,9 @@
 
 #define CFG_MHZ			500
 
-#define CFG_HZ			(CFG_MHZ * 1000000) /* FIXME causes overflow in net.c */
+#define CFG_MIPS_TIMER_FREQ	(CFG_MHZ * 1000000)
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/incaip.h b/include/configs/incaip.h
index 5ca00b3..2e4ee66 100644
--- a/include/configs/incaip.h
+++ b/include/configs/incaip.h
@@ -118,7 +118,9 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			(incaip_get_cpuclk() / 2)
+#define CFG_MIPS_TIMER_FREQ	(incaip_get_cpuclk() / 2)
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000
 
diff --git a/include/configs/pb1x00.h b/include/configs/pb1x00.h
index 810e0f0..181cd11 100644
--- a/include/configs/pb1x00.h
+++ b/include/configs/pb1x00.h
@@ -81,7 +81,9 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			396000000      /* FIXME causes overflow in net.c */
+#define CFG_MIPS_TIMER_FREQ	396000000
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000     /* Cached addr */
 
diff --git a/include/configs/purple.h b/include/configs/purple.h
index 1be4e05..ef92637 100644
--- a/include/configs/purple.h
+++ b/include/configs/purple.h
@@ -114,7 +114,8 @@
 #define	CFG_PROMPT		"PURPLE # "	/* Monitor Command Prompt    */
 #define	CFG_CBSIZE		256		/* Console I/O Buffer Size   */
 #define	CFG_PBSIZE (CFG_CBSIZE+sizeof(CFG_PROMPT)+16)  /* Print Buffer Size */
-#define CFG_HZ			(CPU_CLOCK_RATE/2)
+#define CFG_MIPS_TIMER_FREQ	(CPU_CLOCK_RATE/2)
+#define CFG_HZ			1000
 #define	CFG_MAXARGS		16		/* max number of command args*/
 
 #define	CFG_LOAD_ADDR		0x80500000	/* default load address	*/
diff --git a/include/configs/qemu-mips.h b/include/configs/qemu-mips.h
index d6bcc8e..3dfd218 100644
--- a/include/configs/qemu-mips.h
+++ b/include/configs/qemu-mips.h
@@ -120,7 +120,9 @@
 
 #define CFG_MHZ			132
 
-#define CFG_HZ			(CFG_MHZ * 1000000)
+#define CFG_MIPS_TIMER_FREQ	(CFG_MHZ * 1000000)
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000	/* Cached addr */
 
diff --git a/include/configs/tb0229.h b/include/configs/tb0229.h
index dadf5d3..fc2357d 100644
--- a/include/configs/tb0229.h
+++ b/include/configs/tb0229.h
@@ -122,7 +122,9 @@
 
 #define CFG_BOOTPARAMS_LEN	128*1024
 
-#define CFG_HZ			(CPU_TCLOCK_RATE/4)
+#define CFG_MIPS_TIMER_FREQ	(CPU_TCLOCK_RATE/4)
+
+#define CFG_HZ			1000
 
 #define CFG_SDRAM_BASE		0x80000000
 
diff --git a/lib_mips/time.c b/lib_mips/time.c
index fe36530..1e92789 100644
--- a/lib_mips/time.c
+++ b/lib_mips/time.c
@@ -24,31 +24,50 @@
 #include <common.h>
 #include <asm/mipsregs.h>
 
+static unsigned long timestamp;
+
+/* how many counter cycles in a jiffy */
+#define CYCLES_PER_JIFFY	(CFG_MIPS_TIMER_FREQ + CFG_HZ / 2) / CFG_HZ
+
 /*
  * timer without interrupts
  */
 
 int timer_init(void)
 {
-	write_c0_compare(0);
-	write_c0_count(0);
+	/* Set up the timer for the first expiration. */
+	timestamp = 0;
+	write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY);
 
 	return 0;
 }
 
 void reset_timer(void)
 {
-	write_c0_count(0);
+	timestamp = 0;
+	write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY);
 }
 
 ulong get_timer(ulong base)
 {
-	return read_c0_count() - base;
+	unsigned int count;
+	unsigned int expirelo = read_c0_compare();
+
+	/* Check to see if we have missed any timestamps. */
+	count = read_c0_count();
+	while ((count - expirelo) < 0x7fffffff) {
+		expirelo += CYCLES_PER_JIFFY;
+		timestamp++;
+	}
+	write_c0_compare(expirelo);
+
+	return (timestamp - base);
 }
 
 void set_timer(ulong t)
 {
-	write_c0_count(t);
+	timestamp = t;
+	write_c0_compare(read_c0_count() + CYCLES_PER_JIFFY);
 }
 
 void udelay(unsigned long usec)
@@ -66,7 +85,7 @@ void udelay(unsigned long usec)
  */
 unsigned long long get_ticks(void)
 {
-	return read_c0_count();
+	return get_timer(0);
 }
 
 /*