Update s3c24x0 timer implementation

Since the .rel.text section is required by the relocation code, I
assume that .bss global variables cannot be used until after relocation?

After studying several other timer.c files I developed the following patch which uses the global data struct to store the global variables.
I also restructured some of the code based on structure of the other
timer.c files. I have confirmed it works correctly on the SBC I have.

Quoted 3 lines

?Since the .rel.text section is required by the relocation code, I assume that .bss global variables cannot be used until after relocation?

Why do you have to make such assumptions? ?That's documented behaviour. ?Didn't you RTFM?

Dear Wolfgang Denk,

Thank you for your response.

Quoted 6 lines

Quoted 3 lines

Since the .rel.text section is required by the relocation code, I
assume that .bss global variables cannot be used until after relocation?

Why do you have to make such assumptions? That's documented behaviour. Didn't you RTFM?

I thought I had done a thorough search previously but after receiving your response I managed to find some of the details outlined in the README file.

I have attached an updated patch below which hopefully addresses the other issues you highlighted.

Kind Regards,

Mark Norman

The s3c24x0 timer has been updated to use the global_data struct. Restructured code based on other timer.c files.
Updated comments and several parameters.

Signed-off-by Mark Norman <[email protected]>

---
 arch/arm/cpu/arm920t/s3c24x0/timer.c |  155
+++++++++++++-------------------- arch/arm/include/asm/global_data.h   |  
 4 +
 2 files changed, 65 insertions(+), 94 deletions(-)

arch/arm/cpu/arm920t/s3c24x0/timer.c +10 -15

index 9571870..5695c62 100644
@@ -35,142 +35,109 @@
 #include <asm/io.h>
 #include <asm/arch/s3c24x0_cpu.h>
 
-int timer_load_val = 0;
-static ulong timer_clk;
+DECLARE_GLOBAL_DATA_PTR;
 
-/* macro to read the 16 bit timer */
-static inline ulong READ_TIMER(void)
+/* Read the 16 bit timer */
+static inline ulong read_timer(void)
 {
 	struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
 
 	return readl(&timers->tcnto4) & 0xffff;
 }
 
-static ulong timestamp;
-static ulong lastdec;
-
 int timer_init(void)
 {
+	/*
+	 * PWM Timer 4 is used because it has no output.
+	 * Prescaler is hard fixed at 250, divider at 2.
+	 * This generates a Timer clock frequency of 100kHz (@PCLK=50MHz) and
+	 * therefore 10us timer ticks.
+	 */
+	const ulong prescaler = 250;
 	struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
 	ulong tmr;
 
-	/* use PWM Timer 4 because it has no output */
-	/* prescaler for Timer 4 is 16 */
-	writel(0x0f00, &timers->tcfg0);
-	if (timer_load_val == 0) {
-		/*
-		 * for 10 ms clock period @ PCLK with 4 bit divider = 1/2
-		 * (default) and prescaler = 16. Should be 10390
-		 * @33.25MHz and 15625 @ 50 MHz

arch/arm/cpu/arm920t/s3c24x0/timer.c +27 -35

 }
 
 /*
- * timer without interrupts
+ * Get the number of ticks (in CONFIG_SYS_HZ resolution)
  */
-ulong get_timer(ulong base)
+unsigned long long get_ticks(void)
 {
-	return get_timer_masked() - base;
+	return get_timer(0);
 }
 
-void __udelay (unsigned long usec)
+unsigned long get_timer_raw(void)
 {
-	ulong tmo;
-	ulong start = get_ticks();
+	ulong now = read_timer();
 
-	tmo = usec / 1000;
-	tmo *= (timer_load_val * 100);
-	tmo /= 1000;
+	if (gd->lastdec >= now) {
+		/* normal mode */
+		gd->timestamp += gd->lastdec - now;
+	} else {
+		/* we have an overflow ... */
+		gd->timestamp += gd->lastdec + gd->timer_reset_value - now;
+	}
+	gd->lastdec = now;
 
-	while ((ulong) (get_ticks() - start) < tmo)
-		/*NOP*/;
+	return gd->timestamp;
 }
 
-ulong get_timer_masked(void)
+/*
+ * This function is derived from PowerPC code (timebase clock frequency).
+ * On ARM it returns the number of timer ticks per second.
+ */
+ulong get_tbclk(void)
 {
-	ulong tmr = get_ticks();
-
-	return tmr / (timer_clk / CONFIG_SYS_HZ);
+	return CONFIG_SYS_HZ;
 }
 
-void udelay_masked(unsigned long usec)
+ulong get_timer_masked(void)
 {
-	ulong tmo;
-	ulong endtime;
-	signed long diff;
-
-	if (usec >= 1000) {
-		tmo = usec / 1000;
-		tmo *= (timer_load_val * 100);
-		tmo /= 1000;
-	} else {
-		tmo = usec * (timer_load_val * 100);
-		tmo /= (1000 * 1000);
-	}
+	unsigned long tmr = get_timer_raw();
 
-	endtime = get_ticks() + tmo;
+	return (tmr * CONFIG_SYS_HZ) / gd->timer_rate_hz;
+}
 
-	do {
-		ulong now = get_ticks();
-		diff = endtime - now;
-	} while (diff >= 0);
+ulong get_timer(ulong base)
+{
+	return get_timer_masked() - base;
 }
 
-/*
- * This function is derived from PowerPC code (read timebase as long

long). - * On ARM it just returns the timer value.
- */

arch/arm/cpu/arm920t/s3c24x0/timer.c +8 -32

-unsigned long long get_ticks(void)
+void __udelay(unsigned long usec)
 {
-	ulong now = READ_TIMER();
+	unsigned long tmp;
+	unsigned long tmo;
 
-	if (lastdec >= now) {
-		/* normal mode */
-		timestamp += lastdec - now;
-	} else {
-		/* we have an overflow ... */
-		timestamp += lastdec + timer_load_val - now;
-	}
-	lastdec = now;
+	/* convert usec to ticks. */
+	tmo = ((gd->timer_rate_hz / 1000) * usec) / 1000;
 
-	return timestamp;
-}
+	tmp = get_timer_raw() + tmo;	/* get current timestamp */
 
-/*
- * This function is derived from PowerPC code (timebase clock frequency).
- * On ARM it returns the number of timer ticks per second.
- */
-ulong get_tbclk(void)
-{
-	ulong tbclk;
-
-#if defined(CONFIG_SMDK2400)
-	tbclk = timer_load_val * 100;
-#elif defined(CONFIG_SBC2410X) || \
-      defined(CONFIG_SMDK2410) || \
-	defined(CONFIG_S3C2440) || \
-      defined(CONFIG_VCMA9)
-	tbclk = CONFIG_SYS_HZ;
-#else
-#	error "tbclk not configured"
-#endif
-
-	return tbclk;
+	while (get_timer_raw() < tmp)   /* loop till event */
+		/*NOP*/;
 }
 
 /*

arch/arm/include/asm/global_data.h +0 -0

index fac98d5..b836915 100644

Quoted 3 lines

I have attached an updated patch below which hopefully addresses the other issues you highlighted.

Hehe, you should RTFM ;-) http://www.denx.de/wiki/U-Boot/Patches

Generally, use git send-email to send the patch.

Quoted 62 lines

The s3c24x0 timer has been updated to use the global_data struct. Restructured code based on other timer.c files.
Updated comments and several parameters.

Signed-off-by Mark Norman <[email protected]>

---
?arch/arm/cpu/arm920t/s3c24x0/timer.c | ?155 +++++++++++++-------------------- arch/arm/include/asm/global_data.h ? | ?4 +
?2 files changed, 65 insertions(+), 94 deletions(-)

arch/arm/cpu/arm920t/s3c24x0/timer.c +0 -0

index 9571870..5695c62 100644
@@ -35,142 +35,109 @@

?#include <asm/io.h>
?#include <asm/arch/s3c24x0_cpu.h>

arch/arm/cpu/arm920t/s3c24x0/timer.c +3 -4

-int timer_load_val = 0;
-static ulong timer_clk;
+DECLARE_GLOBAL_DATA_PTR;
 
-/* macro to read the 16 bit timer */
-static inline ulong READ_TIMER(void)
+/* Read the 16 bit timer */
+static inline ulong read_timer(void)

?{
? ? ? struct s3c24x0_timers *timers = s3c24x0_get_base_timers();

? ? ? return readl(&timers->tcnto4) & 0xffff;
?}

arch/arm/cpu/arm920t/s3c24x0/timer.c +0 -3

-static ulong timestamp;
-static ulong lastdec;
-

?int timer_init(void)
?{
+ ? ? /*
+ ? ? ?* PWM Timer 4 is used because it has no output.
+ ? ? ?* Prescaler is hard fixed at 250, divider at 2.
+ ? ? ?* This generates a Timer clock frequency of 100kHz (@PCLK=50MHz) and
+ ? ? ?* therefore 10us timer ticks.
+ ? ? ?*/
+ ? ? const ulong prescaler = 250;
? ? ? struct s3c24x0_timers *timers = s3c24x0_get_base_timers(); ? ? ? ulong tmr;

- ? ? /* use PWM Timer 4 because it has no output */
- ? ? /* prescaler for Timer 4 is 16 */
- ? ? writel(0x0f00, &timers->tcfg0);
- ? ? if (timer_load_val == 0) {
- ? ? ? ? ? ? /*
- ? ? ? ? ? ? ?* for 10 ms clock period @ PCLK with 4 bit divider = 1/2
- ? ? ? ? ? ? ?* (default) and prescaler = 16. Should be 10390
- ? ? ? ? ? ? ?* @33.25MHz and 15625 @ 50 MHz
- ? ? ? ? ? ? ?*/
- ? ? ? ? ? ? timer_load_val = get_PCLK() / (2 * 16 * 100);
- ? ? ? ? ? ? timer_clk = get_PCLK() / (2 * 16);
- ? ? }

Quoted 6 lines

- ? ? /* load value for 10 ms timeout */
- ? ? lastdec = timer_load_val;
- ? ? writel(timer_load_val, &timers->tcntb4);
- ? ? /* auto load, manual update of timer 4 */
+ ? ? /* Set prescaler for Timer 4 */
+ ? ? writel((prescaler-1) << 8, &timers->tcfg0);

Can we get rid of the magic numbers please?

Quoted 4 lines

+
+ ? ? /* Calculate timer freq, approx 100kHz @ PCLK=50MHz. */
+ ? ? gd->timer_rate_hz = get_PCLK() / (2 * prescaler);

Can get_PCLK() be renamed to be lower-case only?

Quoted 7 lines

arch/arm/include/asm/global_data.h +0 -0

index fac98d5..b836915 100644
@@ -67,6 +67,10 @@ typedef ? ?struct ?global_data {

?#ifdef CONFIG_IXP425

arch/arm/include/asm/global_data.h typedef struct global_data { · line 73

Quoted 6 lines

? ? ? unsigned long ? timestamp;
?#endif

arch/arm/include/asm/global_data.h +4 -0

+#ifdef CONFIG_S3C24X0
+ ? ? unsigned long ? lastdec;
+ ? ? unsigned long ? timestamp;
+#endif

I wonder about this change ...

Dear Wolfgang Denk,

Thank you for your response.

Quoted 7 lines

Quoted 3 lines

=A0Since the .rel.text section is required by the relocation code, I assume that .bss global variables cannot be used until after relocation?

Why do you have to make such assumptions? =A0That's documented behaviour. =A0Didn't you RTFM?

I thought I had done a thorough search previously but after receiving your response I managed to find some of the details outlined in the README file.

I have attached an updated patch below which hopefully addresses the other issues you highlighted.

Kind Regards,

Mark Norman

The s3c24x0 timer has been updated to use the global_data struct. Restructured code based on other timer.c files.
Updated comments and several parameters.

Signed-off-by Mark Norman <[email protected]>

---