[U-Boot] timer problem with new ARM relocation feature

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

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

Hi David,
Le 29/11/2010 16:33, "David Müller (ELSOFT AG)" a écrit :
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,

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

On Mon, Nov 29, 2010 at 05:49:19AM -0000, =?utf-8?q?David_M=C3=BCller_=28ELSOFT_AG=29_=3Cd=2Emueller=40elsoft?==?utf-8?q?=2Ech=3E?= wrote:
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 Signed-off-by: David Mueller d.mueller@elsoft.ch
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

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Mueller d.mueller@elsoft.ch
--- 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
diff --git a/arch/arm/cpu/arm920t/s3c24x0/timer.c b/arch/arm/cpu/arm920t/s3c24x0/timer.c index 9571870..4efceac 100644 --- a/arch/arm/cpu/arm920t/s3c24x0/timer.c +++ b/arch/arm/cpu/arm920t/s3c24x0/timer.c @@ -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) || \ diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index c3ff789..02420d8 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -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 */

Dear David Müller,
Am 08.12.2011 13:23, schrieb David Müller:
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Mueller d.mueller@elsoft.ch
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>
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index c3ff789..02420d8 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -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

Hi Andreas,
Le 08/12/2011 13:58, Andreas Bießmann a écrit :
Dear David Müller,
Am 08.12.2011 13:23, schrieb David Müller:
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Muellerd.mueller@elsoft.ch
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>
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h index c3ff789..02420d8 100644 --- a/arch/arm/include/asm/global_data.h +++ b/arch/arm/include/asm/global_data.h @@ -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?
Sorry for being late.
Yes, I would tend to favor as common a timer code as possible.
best regards
Andreas Bießmann
Amicalement,

This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Mueller d.mueller@elsoft.ch
--- 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
diff --git a/arch/arm/cpu/arm920t/s3c24x0/timer.c b/arch/arm/cpu/arm920t/s3c24x0/timer.c index 9571870..d8668be 100644 --- a/arch/arm/cpu/arm920t/s3c24x0/timer.c +++ b/arch/arm/cpu/arm920t/s3c24x0/timer.c @@ -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; }
/*

Hello
Any news?

Hello
Any news?

Dear David Müller,
On 22.12.2011 12:16, David Müller wrote:
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Mueller d.mueller@elsoft.ch
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
seems really better to me.
- simplify code by removing support for obsoleted SMDK2400 / SBC2410X boards
<snip>
best regards
Andreas Bießmann

Le 22/12/2011 12:16, David Müller a écrit :
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Muellerd.mueller@elsoft.ch
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,

On 19 February 2012 20:59, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Le 22/12/2011 12:16, David Müller a écrit :
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Muellerd.mueller@elsoft.ch
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?
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.

Dear David Müller,
On 21 February 2012 10:24, Minkyu Kang promsoft@gmail.com wrote:
On 19 February 2012 20:59, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Le 22/12/2011 12:16, David Müller a écrit :
This patch fixes the s3c24x0 timer code to work with the ARM relocation feature.
Signed-off-by: David Muellerd.mueller@elsoft.ch
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?
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.

Signed-off-by: David Mueller d.mueller@elsoft.ch
--- include/configs/VCMA9.h | 3 +-- include/configs/smdk2410.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/configs/VCMA9.h b/include/configs/VCMA9.h index a370c15..fa00a04 100644 --- a/include/configs/VCMA9.h +++ b/include/configs/VCMA9.h @@ -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 */ diff --git a/include/configs/smdk2410.h b/include/configs/smdk2410.h index 77c0a08..877e7ca 100644 --- a/include/configs/smdk2410.h +++ b/include/configs/smdk2410.h @@ -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 */

Hello
Any news?

On 4 February 2012 04:43, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Hi David,
Le 09/01/2012 08:23, "David Müller (ELSOFT AG)" a écrit :
Hello
Any news?
Adding Minkyu.
Amicalement,
Albert.
Acked-by: Minkyu Kang mk7.kang@samsung.com
Thanks Minkyu Kang.

Hi David,
Le 22/12/2011 12:19, David Müller a écrit :
Signed-off-by: David Muellerd.mueller@elsoft.ch
include/configs/VCMA9.h | 3 +-- include/configs/smdk2410.h | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/include/configs/VCMA9.h b/include/configs/VCMA9.h index a370c15..fa00a04 100644 --- a/include/configs/VCMA9.h +++ b/include/configs/VCMA9.h @@ -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 */ diff --git a/include/configs/smdk2410.h b/include/configs/smdk2410.h index 77c0a08..877e7ca 100644 --- a/include/configs/smdk2410.h +++ b/include/configs/smdk2410.h @@ -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,
participants (6)
-
"David Müller (ELSOFT AG)"
-
Albert ARIBAUD
-
Andreas Bießmann
-
David Müller
-
Minkyu Kang
-
Scott Wood