Skip to contentSkip to conversation

mips: Support to set CFG_HZ to 1000, consistent with other architectures

PATCH RESEND New Jason McMullan 5 participants 14 messages May 20, 2008 – May 31, 2008

lore
Share & tools
lore

Jason McMullan

author
#0 lore raw

PATCH RESEND 1/1 mips: Support to set CFG_HZ to 1000, consistent with other architectures

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 <[email protected]>
---
 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(-)
include/configs/dbau1x00.h +2 -1
index b2f606f..b4cc0c2 100644
@@ -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 */
include/configs/gth2.h +2 -1
index c2a50c1..656eccb 100644
@@ -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 */
include/configs/incaip.h +2 -1
index 5ca00b3..c0e26f2 100644
@@ -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
include/configs/pb1x00.h +2 -1
index 810e0f0..5cb5069 100644
@@ -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 */
include/configs/purple.h +2 -1
index 1be4e05..9fe98f2 100644
@@ -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	*/
include/configs/qemu-mips.h +2 -1
index d6bcc8e..6889d7f 100644
@@ -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 */
include/configs/tb0229.h +2 -1
index dadf5d3..82206d5 100644
@@ -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
lib_mips/time.c +35 -1
index cd8dc72..4807f1a 100644
@@ -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*/;
 }
Signature · 1 line
1.5.4.3
#1 lore raw

PATCH RESEND 1/1 mips: Support to set CFG_HZ to 1000, consistent with other architectures

Hi Jason,

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

Quoted 8 lines · Jason McMullan
---
 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.

Quoted 12 lines
lib_mips/time.c +4 -0
index cd8dc72..4807f1a 100644
@@ -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

lib_mips/time.c line 39

Quoted 10 lines
lib_mips/time.c +9 -0
+ * 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.

lib_mips/time.c line 46

Quoted 6 lines
lib_mips/time.c +2 -0
+/* 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.

Quoted 11 lines
lib_mips/time.c +6 -0
+/* 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) :);
+

lib_mips/time.c line 78

Quoted 10 lines
lib_mips/time.c +8 -0
+	/* 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.

lib_mips/time.c void udelay (unsigned long usec) · line 115

Quoted 9 lines
lib_mips/time.c +1 -1
@@ -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

#2 lore raw

PATCH RESEND 1/1 mips: Support to set CFG_HZ to 1000, consistent with other architectures

Quoted 3 lines · Shinya Kuribayashi

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

#3 lore raw

PATCH 1/3 lib_mips/time.c: Replace CP0 access functions with existing macros

Quoted 8 lines · Scott Wood
Quoted 3 lines · Shinya Kuribayashi

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 <[email protected]>

---

 lib_mips/time.c |   35 ++++++++---------------------------
 1 files changed, 8 insertions(+), 27 deletions(-)
lib_mips/time.c +8 -27
index cd8dc72..f03f023 100644
@@ -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);
 }
 
 /*
#4 lore raw

PATCH 2/3 lib_mips/time.c: Fix udelay

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 <[email protected]>

---

 lib_mips/time.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
lib_mips/time.c +4 -4
index f03f023..154d792 100644
@@ -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*/;
 }
#5 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

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 <[email protected]>

---

 lib_mips/time.c |   44 ++++++++++++++++++++++++++++++++++++++------
 1 files changed, 38 insertions(+), 6 deletions(-)
lib_mips/time.c +38 -6
index 154d792..4b47e41 100644
@@ -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);
 }
 
 /*
#6 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 5 lines · In message <[email protected]> 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

Signature · 5 lines
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
#7 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 4 lines · Wolfgang Denk

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

#8 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 7 lines · In message <[email protected]> you wrote:
Quoted 4 lines · Wolfgang Denk

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

Signature · 3 lines
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
Quoted 1 line

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 <[email protected]>

#9 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 4 lines · Wolfgang Denk
Quoted 1 line

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:

lib_mips/time.c +6 -10
index f6678e1..d4754e3 100644
@@ -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.

Quoted 3 lines

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

#10 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 3 lines · In message <[email protected]> you wrote:

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

...

lib_mips/time.c line 30

Quoted 2 lines
lib_mips/time.c +1 -1
-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?

Quoted 1 line

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

Signature · 5 lines
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 <[email protected]>
#11 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 8 lines · Wolfgang Denk
Quoted 2 lines · In message <[email protected]> you wrote:

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

...

lib_mips/time.c line 30

Quoted 2 lines
lib_mips/time.c +1 -1
-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.

Quoted 4 lines
Quoted 1 line

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.

Quoted 1 line

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

#12 lore raw

PATCH 3/3 lib_mips/time.c: Fix improper use of CFG_HZ and timer routines

Quoted 4 lines
Quoted 1 line

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/

#13 lore raw

PATCH v2 3/3 lib_mips/time.c: Fix CP0 count register usage and timer routines

In reply to 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 <[email protected]>

---

 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(-)
include/configs/dbau1x00.h +3 -1
index b2f606f..45ff1e7 100644
@@ -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 */
include/configs/gth2.h +3 -1
index c2a50c1..23618db 100644
@@ -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 */
include/configs/incaip.h +3 -1
index 5ca00b3..2e4ee66 100644
@@ -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
include/configs/pb1x00.h +3 -1
index 810e0f0..181cd11 100644
@@ -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 */
include/configs/purple.h +2 -1
index 1be4e05..ef92637 100644
@@ -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	*/
include/configs/qemu-mips.h +3 -1
index d6bcc8e..3dfd218 100644
@@ -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 */
include/configs/tb0229.h +3 -1
index dadf5d3..fc2357d 100644
@@ -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
lib_mips/time.c +25 -6
index fe36530..1e92789 100644
@@ -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);
 }
 
 /*