Skip to contentSkip to conversation

timer problem with new ARM relocation feature

U-Boot New David Müller (ELSOFT AG) 5 participants 20 messages Oct 11, 2010 – Mar 5, 2012 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

Hello

The timer implementation for s3c24xx based SoC (arch/arm/cpu/arm920t/s3c24x0/timer.c) uses several global variables. With the newly introduced ARM relocation feature, this timer code does no longer work as expected.

What is the proposed fix/workaround for this?
Is moving these variables to the "global data pointer area" an acceptable solution?

Dave

#3 lore raw

PATCH 1/1 fix s3c2410_nand timing default values

In reply to David Müller (ELSOFT AG)

Hello

The attached patch fixes wrong timing default values and adds the possibility to specify board specific timing value in the board config file.

Dave

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: s3c2410_nand.patch
Url: http://lists.denx.de/pipermail/u-boot/attachments/20101129/a5353a77/attachment.asc

#4 lore raw

[U-Boot] fix s3c2410_nand timing default values

Quoted 7 lines · On Mon, Nov 29, 2010 at 05:49:19AM -0000…

Hello

The attached patch fixes wrong timing default values and adds the possibility to specify board specific timing value in the board config file.

Dave

Applied to u-boot-nand-flash.

In the future, please put anything you don't want in the git commit log (e.g. greetings) below a --- line.

-Scott

#2 lore raw

PATCH 1/1 fix s3c24x0 timer code

In reply to David Müller (ELSOFT AG)

Hi David,

Le 29/11/2010 16:33, "David M?ller (ELSOFT AG)" a ?crit :

Quoted 6 lines

Hello

The attached patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

Dave

Apparently this old patch never made it into the ARM tree... Can you rebase and resubmit?

Amicalement,

Signature · 1 line
Albert.
#5 lore raw

PATCH v2 1/1 ARM: fix s3c24x0 timer code

In reply to David Müller (ELSOFT AG)

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
 arch/arm/cpu/arm920t/s3c24x0/timer.c |   40 +++++++++++++++------------------
 arch/arm/include/asm/global_data.h   |    7 ++++++
 2 files changed, 25 insertions(+), 22 deletions(-)

Changes for V2:
- rebase to master

arch/arm/cpu/arm920t/s3c24x0/timer.c +18 -22
index 9571870..4efceac 100644
@@ -35,8 +35,7 @@
 #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)
@@ -46,9 +45,6 @@ static inline ulong READ_TIMER(void)
 	return readl(&timers->tcnto4) & 0xffff;
 }
 
-static ulong timestamp;
-static ulong lastdec;
-
 int timer_init(void)
 {
 	struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
@@ -57,27 +53,27 @@ int timer_init(void)
 	/* use PWM Timer 4 because it has no output */
 	/* prescaler for Timer 4 is 16 */
 	writel(0x0f00, &timers->tcfg0);
-	if (timer_load_val == 0) {
+	if (gd->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);
+		gd->timer_load_val = get_PCLK() / (2 * 16 * 100);
+		gd->timer_clk = get_PCLK() / (2 * 16);
 	}
 	/* load value for 10 ms timeout */
-	lastdec = timer_load_val;
-	writel(timer_load_val, &timers->tcntb4);
+	gd->lastdec = gd->timer_load_val;
+	writel(gd->timer_load_val, &timers->tcntb4);
 	/* auto load, manual update of timer 4 */
 	tmr = (readl(&timers->tcon) & ~0x0700000) | 0x0600000;
 	writel(tmr, &timers->tcon);
 	/* auto load, start timer 4 */
 	tmr = (tmr & ~0x0700000) | 0x0500000;
 	writel(tmr, &timers->tcon);
-	timestamp = 0;
+	gd->timestamp = 0;
 
-	return (0);
+	return 0;
 }
 
 /*
@@ -94,7 +90,7 @@ void __udelay (unsigned long usec)
 	ulong start = get_ticks();
 
 	tmo = usec / 1000;
-	tmo *= (timer_load_val * 100);
+	tmo *= (gd->timer_load_val * 100);
 	tmo /= 1000;
 
 	while ((ulong) (get_ticks() - start) < tmo)
@@ -105,7 +101,7 @@ ulong get_timer_masked(void)
 {
 	ulong tmr = get_ticks();
 
-	return tmr / (timer_clk / CONFIG_SYS_HZ);
+	return tmr / (gd->timer_clk / CONFIG_SYS_HZ);
 }
 
 void udelay_masked(unsigned long usec)
@@ -116,10 +112,10 @@ void udelay_masked(unsigned long usec)
 
 	if (usec >= 1000) {
 		tmo = usec / 1000;
-		tmo *= (timer_load_val * 100);
+		tmo *= (gd->timer_load_val * 100);
 		tmo /= 1000;
 	} else {
-		tmo = usec * (timer_load_val * 100);
+		tmo = usec * (gd->timer_load_val * 100);
 		tmo /= (1000 * 1000);
 	}
 
@@ -139,16 +135,16 @@ unsigned long long get_ticks(void)
 {
 	ulong now = READ_TIMER();
 
-	if (lastdec >= now) {
+	if (gd->lastdec >= now) {
 		/* normal mode */
-		timestamp += lastdec - now;
+		gd->timestamp += gd->lastdec - now;
 	} else {
 		/* we have an overflow ... */
-		timestamp += lastdec + timer_load_val - now;
+		gd->timestamp += gd->lastdec + gd->timer_load_val - now;
 	}
-	lastdec = now;
+	gd->lastdec = now;
 
-	return timestamp;
+	return gd->timestamp;
 }
 
 /*
@@ -160,7 +156,7 @@ ulong get_tbclk(void)
 	ulong tbclk;
 
 #if defined(CONFIG_SMDK2400)
-	tbclk = timer_load_val * 100;
+	tbclk = gd->timer_load_val * 100;
 #elif defined(CONFIG_SBC2410X) || \
       defined(CONFIG_SMDK2410) || \
 	defined(CONFIG_S3C2440) || \
arch/arm/include/asm/global_data.h +7 -0
index c3ff789..02420d8 100644
@@ -67,6 +67,13 @@ typedef	struct	global_data {
 #ifdef CONFIG_IXP425
 	unsigned long	timestamp;
 #endif
+#ifdef CONFIG_S3C24X0
+	/* "static data" needed by s3c24x0 timer.c */
+	unsigned long	timer_load_val;
+	unsigned long	timer_clk;
+	unsigned long	timestamp;
+	unsigned long	lastdec;
+#endif
 	unsigned long	relocaddr;	/* Start address of U-Boot in RAM */
 	phys_size_t	ram_size;	/* RAM size */
 	unsigned long	mon_len;	/* monitor len */
Signature · 1 line
1.7.4.4
#6 lore raw

PATCH v2 1/1 ARM: fix s3c24x0 timer code

Dear David M?ller,

Am 08.12.2011 13:23, schrieb David M?ller:

Quoted 13 lines

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
 arch/arm/cpu/arm920t/s3c24x0/timer.c |   40 +++++++++++++++------------------
 arch/arm/include/asm/global_data.h   |    7 ++++++
 2 files changed, 25 insertions(+), 22 deletions(-)

Changes for V2:
- rebase to master

<snip>

Quoted 5 lines
arch/arm/include/asm/global_data.h +0 -0
index c3ff789..02420d8 100644

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

Quoted 10 lines
arch/arm/include/asm/global_data.h +7 -0
@@ -67,6 +67,13 @@ typedef	struct	global_data {
 #ifdef CONFIG_IXP425
 	unsigned long	timestamp;
 #endif
+#ifdef CONFIG_S3C24X0
+	/* "static data" needed by s3c24x0 timer.c */
+	unsigned long	timer_load_val;
+	unsigned long	timer_clk;
+	unsigned long	timestamp;
+	unsigned long	lastdec;
+#endif

I tend to NAK this. There are currently values defined ('#ifdef CONFIG_ARM') which should work out for s3c24x0 timer too. E.g. your 'lastdec' corresponds to the 'lastinc' there, timestamp could be timer_reset_value (but maybe we should include the 'timestamp' defined by CONFIG_IXP425 into the CONFIG_ARM?), timer_clk could be timer_rate_hz ...
I guess the arm920t/at91 timer is not that much different to yours (by means of logic), maybe have a look for that driver?

Albert, how do you think about this?

best regards

Andreas Bie?mann

#7 lore raw

PATCH v2 1/1 ARM: fix s3c24x0 timer code

Hi Andreas,

Le 08/12/2011 13:58, Andreas Bie?mann a ?crit :

Quoted 44 lines

Dear David M?ller,

Am 08.12.2011 13:23, schrieb David M?ller:

Quoted 13 lines

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
  arch/arm/cpu/arm920t/s3c24x0/timer.c |   40 +++++++++++++++------------------
  arch/arm/include/asm/global_data.h   |    7 ++++++
  2 files changed, 25 insertions(+), 22 deletions(-)

Changes for V2:
- rebase to master

<snip>

Quoted 5 lines
arch/arm/include/asm/global_data.h +0 -0
index c3ff789..02420d8 100644

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

Quoted 10 lines
arch/arm/include/asm/global_data.h +7 -0
@@ -67,6 +67,13 @@ typedef	struct	global_data {
  #ifdef CONFIG_IXP425
  	unsigned long	timestamp;
  #endif
+#ifdef CONFIG_S3C24X0
+	/* "static data" needed by s3c24x0 timer.c */
+	unsigned long	timer_load_val;
+	unsigned long	timer_clk;
+	unsigned long	timestamp;
+	unsigned long	lastdec;
+#endif

I tend to NAK this. There are currently values defined ('#ifdef CONFIG_ARM') which should work out for s3c24x0 timer too. E.g. your 'lastdec' corresponds to the 'lastinc' there, timestamp could be timer_reset_value (but maybe we should include the 'timestamp' defined by CONFIG_IXP425 into the CONFIG_ARM?), timer_clk could be timer_rate_hz ...
I guess the arm920t/at91 timer is not that much different to yours (by means of logic), maybe have a look for that driver?

Quoted 1 line

Albert, how do you think about this?

Sorry for being late.

Yes, I would tend to favor as common a timer code as possible.

Quoted 3 lines

best regards

Andreas Bie?mann

Amicalement,

Signature · 1 line
Albert.
#8 lore raw

PATCH v3 1/1 ARM: fix s3c2410 timer code

In reply to David Müller (ELSOFT AG)

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
 arch/arm/cpu/arm920t/s3c24x0/timer.c |   64 ++++++++++-----------------------
 1 files changed, 20 insertions(+), 44 deletions(-)

Changes for V2:
- rebase to master

Changes for V3:
- do not introduce new fields in the global_data structure but use the existing ones
- simplify code by removing support for obsoleted SMDK2400 / SBC2410X boards

arch/arm/cpu/arm920t/s3c24x0/timer.c +20 -44
index 9571870..d8668be 100644
@@ -35,19 +35,7 @@
 #include <asm/io.h>
 #include <asm/arch/s3c24x0_cpu.h>
 
-int timer_load_val = 0;
-static ulong timer_clk;
-
-/* macro to 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;
+DECLARE_GLOBAL_DATA_PTR;
 
 int timer_init(void)
 {
@@ -57,27 +45,27 @@ int timer_init(void)
 	/* use PWM Timer 4 because it has no output */
 	/* prescaler for Timer 4 is 16 */
 	writel(0x0f00, &timers->tcfg0);
-	if (timer_load_val == 0) {
+	if (gd->tbu == 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);
+		gd->tbu = get_PCLK() / (2 * 16 * 100);
+		gd->timer_rate_hz = get_PCLK() / (2 * 16);
 	}
 	/* load value for 10 ms timeout */
-	lastdec = timer_load_val;
-	writel(timer_load_val, &timers->tcntb4);
+	writel(gd->tbu, &timers->tcntb4);
 	/* auto load, manual update of timer 4 */
 	tmr = (readl(&timers->tcon) & ~0x0700000) | 0x0600000;
 	writel(tmr, &timers->tcon);
 	/* auto load, start timer 4 */
 	tmr = (tmr & ~0x0700000) | 0x0500000;
 	writel(tmr, &timers->tcon);
-	timestamp = 0;
+	gd->lastinc = 0;
+	gd->tbl = 0;
 
-	return (0);
+	return 0;
 }
 
 /*
@@ -94,7 +82,7 @@ void __udelay (unsigned long usec)
 	ulong start = get_ticks();
 
 	tmo = usec / 1000;
-	tmo *= (timer_load_val * 100);
+	tmo *= (gd->tbu * 100);
 	tmo /= 1000;
 
 	while ((ulong) (get_ticks() - start) < tmo)
@@ -105,7 +93,7 @@ ulong get_timer_masked(void)
 {
 	ulong tmr = get_ticks();
 
-	return tmr / (timer_clk / CONFIG_SYS_HZ);
+	return tmr / (gd->timer_rate_hz / CONFIG_SYS_HZ);
 }
 
 void udelay_masked(unsigned long usec)
@@ -116,10 +104,10 @@ void udelay_masked(unsigned long usec)
 
 	if (usec >= 1000) {
 		tmo = usec / 1000;
-		tmo *= (timer_load_val * 100);
+		tmo *= (gd->tbu * 100);
 		tmo /= 1000;
 	} else {
-		tmo = usec * (timer_load_val * 100);
+		tmo = usec * (gd->tbu * 100);
 		tmo /= (1000 * 1000);
 	}
 
@@ -137,18 +125,19 @@ void udelay_masked(unsigned long usec)
  */
 unsigned long long get_ticks(void)
 {
-	ulong now = READ_TIMER();
+	struct s3c24x0_timers *timers = s3c24x0_get_base_timers();
+	ulong now = readl(&timers->tcnto4) & 0xffff;
 
-	if (lastdec >= now) {
+	if (gd->lastinc >= now) {
 		/* normal mode */
-		timestamp += lastdec - now;
+		gd->tbl += gd->lastinc - now;
 	} else {
 		/* we have an overflow ... */
-		timestamp += lastdec + timer_load_val - now;
+		gd->tbl += gd->lastinc + gd->tbu - now;
 	}
-	lastdec = now;
+	gd->lastinc = now;
 
-	return timestamp;
+	return gd->tbl;
 }
 
 /*
@@ -157,20 +146,7 @@ unsigned long long get_ticks(void)
  */
 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;
+	return CONFIG_SYS_HZ;
 }
 
 /*
Signature · 1 line
1.7.4.4
#15 lore raw

PATCH 1/1 ARM: activate DISPLAY_CPUINFO for VCMA9 + SMDK2410 board

In reply to David Müller (ELSOFT AG)
---
 include/configs/VCMA9.h    |    3 +--
 include/configs/smdk2410.h |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)
include/configs/VCMA9.h +1 -2
index a370c15..fa00a04 100644
@@ -169,8 +169,7 @@
 /* Boot Argument Buffer Size */
 #define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
 
-/* to be activated as soon as s3c24x0 has print_cpuinfo support */
-/*#define CONFIG_DISPLAY_CPUINFO*/			/* Display cpu info */
+#define CONFIG_DISPLAY_CPUINFO				/* Display cpu info */
 #define CONFIG_DISPLAY_BOARDINFO			/* Display board info */
 
 #define CONFIG_SYS_MEMTEST_START	0x30000000	/* memtest works on */
include/configs/smdk2410.h +1 -2
index 77c0a08..877e7ca 100644
@@ -135,8 +135,7 @@
 #define CONFIG_SYS_MAXARGS	16
 #define CONFIG_SYS_BARGSIZE	CONFIG_SYS_CBSIZE
 
-/* may be activated as soon as s3c24x0 has print_cpuinfo support */
-/*#define CONFIG_DISPLAY_CPUINFO*/		/* Display cpu info */
+#define CONFIG_DISPLAY_CPUINFO				/* Display cpu info */
 
 #define CONFIG_SYS_MEMTEST_START	0x30000000	/* memtest works on */
 #define CONFIG_SYS_MEMTEST_END		0x33F00000	/* 63 MB in DRAM */
Signature · 1 line
1.7.4.4
#11 lore raw

PATCH v3 1/1 ARM: fix s3c2410 timer code

In reply to David Müller

Dear David M?ller,

Quoted 9 lines · David M?ller

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
 arch/arm/cpu/arm920t/s3c24x0/timer.c |   64 ++++++++++-----------------------
 1 files changed, 20 insertions(+), 44 deletions(-)
Quoted 6 lines

Changes for V2:
- rebase to master

Changes for V3:
- do not introduce new fields in the global_data structure but use the existing ones

seems really better to me.

Quoted 2 lines

- simplify code by removing support for obsoleted SMDK2400 / SBC2410X boards

<snip>

best regards

Andreas Bie?mann

#17 lore raw

[U-Boot] [PING] Re: [PATCH] ARM: activate DISPLAY_CPUINFO for VCMA9 + SMDK2410 board

In reply to David Müller (ELSOFT AG)

Hi David,

Le 09/01/2012 08:23, "David M?ller (ELSOFT AG)" a ?crit :

Quoted 3 lines

Hello

Any news?

Adding Minkyu.

Amicalement,

Signature · 1 line
Albert.
Acked-by
#18 lore raw

[U-Boot] [PING] Re: [PATCH] ARM: activate DISPLAY_CPUINFO for VCMA9 + SMDK2410 board

Quoted 6 lines · Albert ARIBAUD

Hi David,

Le 09/01/2012 08:23, "David M?ller (ELSOFT AG)" a ?crit :

Quoted 3 lines

Hello

Quoted 9 lines
Quoted 1 line

Any news?

Adding Minkyu.

Amicalement,
--
Albert.

Thanks
Minkyu Kang.

Signature · 2 lines
from. prom.
www.promsoft.net
#19 lore raw

PATCH 1/1 ARM: activate DISPLAY_CPUINFO for VCMA9 + SMDK2410 board

In reply to David Müller

Hi David,

Le 22/12/2011 12:19, David M?ller a ?crit :

Quoted 26 lines
---
  include/configs/VCMA9.h    |    3 +--
  include/configs/smdk2410.h |    3 +--
  2 files changed, 2 insertions(+), 4 deletions(-)
include/configs/VCMA9.h +1 -2
index a370c15..fa00a04 100644
@@ -169,8 +169,7 @@
  /* Boot Argument Buffer Size */
  #define CONFIG_SYS_BARGSIZE		CONFIG_SYS_CBSIZE
 
-/* to be activated as soon as s3c24x0 has print_cpuinfo support */
-/*#define CONFIG_DISPLAY_CPUINFO*/			/* Display cpu info */
+#define CONFIG_DISPLAY_CPUINFO				/* Display cpu info */
  #define CONFIG_DISPLAY_BOARDINFO			/* Display board info */
 
  #define CONFIG_SYS_MEMTEST_START	0x30000000	/* memtest works on */
include/configs/smdk2410.h +0 -0
index 77c0a08..877e7ca 100644

include/configs/smdk2410.h line 141

Quoted 9 lines
include/configs/smdk2410.h +1 -2
@@ -135,8 +135,7 @@
  #define CONFIG_SYS_MAXARGS	16
  #define CONFIG_SYS_BARGSIZE	CONFIG_SYS_CBSIZE
 
-/* may be activated as soon as s3c24x0 has print_cpuinfo support */
-/*#define CONFIG_DISPLAY_CPUINFO*/		/* Display cpu info */
+#define CONFIG_DISPLAY_CPUINFO				/* Display cpu info */
 
  #define CONFIG_SYS_MEMTEST_START	0x30000000	/* memtest works on */
  #define CONFIG_SYS_MEMTEST_END		0x33F00000	/* 63 MB in DRAM */

Applied to u-boot-arm/master, thanks!

Amicalement,

Signature · 1 line
Albert.
#12 lore raw

PATCH v3 1/1 ARM: fix s3c2410 timer code

In reply to David Müller

Le 22/12/2011 12:16, David M?ller a ?crit :

Quoted 8 lines

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
  arch/arm/cpu/arm920t/s3c24x0/timer.c |   64 ++++++++++-----------------------
  1 files changed, 20 insertions(+), 44 deletions(-)

Minkyu,

Shouldn't this go through your tree, or at least get your Ack?

Amicalement,

Signature · 1 line
Albert.
#13 lore raw

PATCH v3 1/1 ARM: fix s3c2410 timer code

Quoted 13 lines · Albert ARIBAUD

Le 22/12/2011 12:16, David M?ller a ?crit :

Quoted 9 lines

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
?arch/arm/cpu/arm920t/s3c24x0/timer.c | ? 64
++++++++++-----------------------
?1 files changed, 20 insertions(+), 44 deletions(-)

Quoted 4 lines

Minkyu,

Shouldn't this go through your tree, or at least get your Ack?

I'm going to apply this patch at u-boot-samsung. Please wait few days, cuz I have network problem in the office.

David,
Are there other pending patches?
If so, please let me know.

Thanks :)
Minkyu Kang.

Signature · 2 lines
from. prom.
www.promsoft.net
#14 lore raw

PATCH v3 1/1 ARM: fix s3c2410 timer code

Dear David M?ller,

Quoted 19 lines · Minkyu Kang
Quoted 13 lines · Albert ARIBAUD

Le 22/12/2011 12:16, David M?ller a ?crit :

Quoted 9 lines

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.

---
?arch/arm/cpu/arm920t/s3c24x0/timer.c | ? 64
++++++++++-----------------------
?1 files changed, 20 insertions(+), 44 deletions(-)

Quoted 4 lines

Minkyu,

Shouldn't this go through your tree, or at least get your Ack?

Quoted 7 lines

I'm going to apply this patch at u-boot-samsung. Please wait few days, cuz I have network problem in the office.

David,
Are there other pending patches?
If so, please let me know.

applied to u-boot-samsung.

Thanks,
Minkyu Kang.

Signature · 2 lines
from. prom.
www.promsoft.net