* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
@ 2010-11-01 20:49 Darius Augulis
2010-11-12 3:10 ` Minkyu Kang
0 siblings, 1 reply; 8+ messages in thread
From: Darius Augulis @ 2010-11-01 20:49 UTC (permalink / raw)
To: u-boot
S3C64XX timer uses static variables which
are initialized before relocation and used after it.
Move them to global data structure.
Signed-off-by: Darius Augulis <augulis.darius@gmail.com>
---
arch/arm/cpu/arm1176/s3c64xx/timer.c | 36 ++++++++++++++--------------------
arch/arm/include/asm/global_data.h | 6 ++++++
2 files changed, 21 insertions(+), 21 deletions(-)
diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
index 9768319..4e420cb 100644
--- a/arch/arm/cpu/arm1176/s3c64xx/timer.c
+++ b/arch/arm/cpu/arm1176/s3c64xx/timer.c
@@ -43,7 +43,7 @@
#include <asm/arch/s3c6400.h>
#include <div64.h>
-static ulong timer_load_val;
+DECLARE_GLOBAL_DATA_PTR;
#define PRESCALER 167
@@ -60,12 +60,6 @@ static inline ulong read_timer(void)
return timers->TCNTO4;
}
-/* Internal tick units */
-/* Last decremneter snapshot */
-static unsigned long lastdec;
-/* Monotonic incrementing timer */
-static unsigned long long timestamp;
-
int timer_init(void)
{
s3c64xx_timers *const timers = s3c64xx_get_base_timers();
@@ -83,20 +77,20 @@ int timer_init(void)
* the prescaler automatically for other PCLK frequencies.
*/
timers->TCFG0 = PRESCALER << 8;
- if (timer_load_val == 0) {
- timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
+ if (gd->timer_load_val == 0) {
+ gd->timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
timers->TCFG1 = (timers->TCFG1 & ~0xf0000) | 0x20000;
}
/* load value for 10 ms timeout */
- lastdec = timers->TCNTB4 = timer_load_val;
+ gd->lastdec = timers->TCNTB4 = gd->timer_load_val;
/* auto load, manual update of Timer 4 */
timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO |
TCON_4_UPDATE;
/* auto load, start Timer 4 */
timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
- timestamp = 0;
+ gd->timestamp = 0;
return 0;
}
@@ -113,16 +107,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;
}
/*
@@ -132,14 +126,14 @@ unsigned long long get_ticks(void)
ulong get_tbclk(void)
{
/* We overrun in 100s */
- return (ulong)(timer_load_val / 100);
+ return (ulong)(gd->timer_load_val / 100);
}
void reset_timer_masked(void)
{
/* reset time */
- lastdec = read_timer();
- timestamp = 0;
+ gd->lastdec = read_timer();
+ gd->timestamp = 0;
}
void reset_timer(void)
@@ -150,7 +144,7 @@ void reset_timer(void)
ulong get_timer_masked(void)
{
unsigned long long res = get_ticks();
- do_div (res, (timer_load_val / (100 * CONFIG_SYS_HZ)));
+ do_div (res, (gd->timer_load_val / (100 * CONFIG_SYS_HZ)));
return res;
}
@@ -161,7 +155,7 @@ ulong get_timer(ulong base)
void set_timer(ulong t)
{
- timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
+ gd->timestamp = t * (gd->timer_load_val / (100 * CONFIG_SYS_HZ));
}
void __udelay(unsigned long usec)
diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
index ada3fbb..69fb7b6 100644
--- a/arch/arm/include/asm/global_data.h
+++ b/arch/arm/include/asm/global_data.h
@@ -61,6 +61,12 @@ typedef struct global_data {
unsigned long tbu;
unsigned long long timer_reset_value;
#endif
+#ifdef CONFIG_S3C64XX
+ /* "static data" needed by s3c64xx's timer.c */
+ unsigned long timer_load_val;
+ unsigned long lastdec;
+ unsigned long long timestamp;
+#endif
unsigned long relocaddr; /* Start address of U-Boot in RAM */
phys_size_t ram_size; /* RAM size */
unsigned long mon_len; /* monitor len */
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-01 20:49 [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation Darius Augulis
@ 2010-11-12 3:10 ` Minkyu Kang
2010-11-12 18:18 ` Darius Augulis
0 siblings, 1 reply; 8+ messages in thread
From: Minkyu Kang @ 2010-11-12 3:10 UTC (permalink / raw)
To: u-boot
Dear Darius Augulis,
On 2 November 2010 05:49, Darius Augulis <augulis.darius@gmail.com> wrote:
> S3C64XX timer uses static variables which
> are initialized before relocation and used after it.
> Move them to global data structure.
>
> Signed-off-by: Darius Augulis <augulis.darius@gmail.com>
> ---
> ?arch/arm/cpu/arm1176/s3c64xx/timer.c | ? 36 ++++++++++++++--------------------
> ?arch/arm/include/asm/global_data.h ? | ? ?6 ++++++
> ?2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
> index 9768319..4e420cb 100644
> --- a/arch/arm/cpu/arm1176/s3c64xx/timer.c
> +++ b/arch/arm/cpu/arm1176/s3c64xx/timer.c
> @@ -43,7 +43,7 @@
> ?#include <asm/arch/s3c6400.h>
> ?#include <div64.h>
>
> -static ulong timer_load_val;
> +DECLARE_GLOBAL_DATA_PTR;
>
> ?#define PRESCALER ? ? ?167
>
> @@ -60,12 +60,6 @@ static inline ulong read_timer(void)
> ? ? ? ?return timers->TCNTO4;
> ?}
>
> -/* Internal tick units */
> -/* Last decremneter snapshot */
> -static unsigned long lastdec;
> -/* Monotonic incrementing timer */
> -static unsigned long long timestamp;
> -
> ?int timer_init(void)
> ?{
> ? ? ? ?s3c64xx_timers *const timers = s3c64xx_get_base_timers();
> @@ -83,20 +77,20 @@ int timer_init(void)
> ? ? ? ? * the prescaler automatically for other PCLK frequencies.
> ? ? ? ? */
> ? ? ? ?timers->TCFG0 = PRESCALER << 8;
> - ? ? ? if (timer_load_val == 0) {
> - ? ? ? ? ? ? ? timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
> + ? ? ? if (gd->timer_load_val == 0) {
> + ? ? ? ? ? ? ? gd->timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
> ? ? ? ? ? ? ? ?timers->TCFG1 = (timers->TCFG1 & ~0xf0000) | 0x20000;
> ? ? ? ?}
>
> ? ? ? ?/* load value for 10 ms timeout */
> - ? ? ? lastdec = timers->TCNTB4 = timer_load_val;
> + ? ? ? gd->lastdec = timers->TCNTB4 = gd->timer_load_val;
> ? ? ? ?/* auto load, manual update of Timer 4 */
> ? ? ? ?timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO |
> ? ? ? ? ? ? ? ?TCON_4_UPDATE;
>
> ? ? ? ?/* auto load, start Timer 4 */
> ? ? ? ?timers->TCON = (timers->TCON & ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
> - ? ? ? timestamp = 0;
> + ? ? ? gd->timestamp = 0;
>
> ? ? ? ?return 0;
> ?}
> @@ -113,16 +107,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;
> ?}
>
> ?/*
> @@ -132,14 +126,14 @@ unsigned long long get_ticks(void)
> ?ulong get_tbclk(void)
> ?{
> ? ? ? ?/* We overrun in 100s */
> - ? ? ? return (ulong)(timer_load_val / 100);
> + ? ? ? return (ulong)(gd->timer_load_val / 100);
> ?}
>
> ?void reset_timer_masked(void)
> ?{
> ? ? ? ?/* reset time */
> - ? ? ? lastdec = read_timer();
> - ? ? ? timestamp = 0;
> + ? ? ? gd->lastdec = read_timer();
> + ? ? ? gd->timestamp = 0;
> ?}
>
> ?void reset_timer(void)
> @@ -150,7 +144,7 @@ void reset_timer(void)
> ?ulong get_timer_masked(void)
> ?{
> ? ? ? ?unsigned long long res = get_ticks();
> - ? ? ? do_div (res, (timer_load_val / (100 * CONFIG_SYS_HZ)));
> + ? ? ? do_div (res, (gd->timer_load_val / (100 * CONFIG_SYS_HZ)));
> ? ? ? ?return res;
> ?}
>
> @@ -161,7 +155,7 @@ ulong get_timer(ulong base)
>
> ?void set_timer(ulong t)
> ?{
> - ? ? ? timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
> + ? ? ? gd->timestamp = t * (gd->timer_load_val / (100 * CONFIG_SYS_HZ));
> ?}
>
> ?void __udelay(unsigned long usec)
> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
> index ada3fbb..69fb7b6 100644
> --- a/arch/arm/include/asm/global_data.h
> +++ b/arch/arm/include/asm/global_data.h
> @@ -61,6 +61,12 @@ typedef ? ? ?struct ?global_data {
> ? ? ? ?unsigned long ? tbu;
> ? ? ? ?unsigned long long ? ? ?timer_reset_value;
> ?#endif
> +#ifdef CONFIG_S3C64XX
> + ? ? ? /* "static data" needed by s3c64xx's timer.c */
> + ? ? ? unsigned long ? ? ? ? ? timer_load_val;
> + ? ? ? unsigned long ? ? ? ? ? lastdec;
> + ? ? ? unsigned long long ? ? ?timestamp;
> +#endif
> ? ? ? ?unsigned long ? relocaddr; ? ? ?/* Start address of U-Boot in RAM */
> ? ? ? ?phys_size_t ? ? ram_size; ? ? ? /* RAM size */
> ? ? ? ?unsigned long ? mon_len; ? ? ? ?/* monitor len */
>
I think it's not good way.
We just need to fix the timer_init function.
Because we can use the static variables after relocation.
I'll post the timer patch for s5p series.
Please refer it.
Thanks
Minkyu Kang
--
from. prom.
www.promsoft.net
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-12 3:10 ` Minkyu Kang
@ 2010-11-12 18:18 ` Darius Augulis
2010-11-12 19:25 ` Albert ARIBAUD
0 siblings, 1 reply; 8+ messages in thread
From: Darius Augulis @ 2010-11-12 18:18 UTC (permalink / raw)
To: u-boot
Dear Minkyu Kang,
On 11/12/2010 05:10 AM, Minkyu Kang wrote:
> Dear Darius Augulis,
>
> On 2 November 2010 05:49, Darius Augulis<augulis.darius@gmail.com> wrote:
>> S3C64XX timer uses static variables which
>> are initialized before relocation and used after it.
>> Move them to global data structure.
>>
>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>> ---
>> arch/arm/cpu/arm1176/s3c64xx/timer.c | 36 ++++++++++++++--------------------
>> arch/arm/include/asm/global_data.h | 6 ++++++
>> 2 files changed, 21 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>> index 9768319..4e420cb 100644
>> --- a/arch/arm/cpu/arm1176/s3c64xx/timer.c
>> +++ b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>> @@ -43,7 +43,7 @@
>> #include<asm/arch/s3c6400.h>
>> #include<div64.h>
>>
>> -static ulong timer_load_val;
>> +DECLARE_GLOBAL_DATA_PTR;
>>
>> #define PRESCALER 167
>>
>> @@ -60,12 +60,6 @@ static inline ulong read_timer(void)
>> return timers->TCNTO4;
>> }
>>
>> -/* Internal tick units */
>> -/* Last decremneter snapshot */
>> -static unsigned long lastdec;
>> -/* Monotonic incrementing timer */
>> -static unsigned long long timestamp;
>> -
>> int timer_init(void)
>> {
>> s3c64xx_timers *const timers = s3c64xx_get_base_timers();
>> @@ -83,20 +77,20 @@ int timer_init(void)
>> * the prescaler automatically for other PCLK frequencies.
>> */
>> timers->TCFG0 = PRESCALER<< 8;
>> - if (timer_load_val == 0) {
>> - timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>> + if (gd->timer_load_val == 0) {
>> + gd->timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>> timers->TCFG1 = (timers->TCFG1& ~0xf0000) | 0x20000;
>> }
>>
>> /* load value for 10 ms timeout */
>> - lastdec = timers->TCNTB4 = timer_load_val;
>> + gd->lastdec = timers->TCNTB4 = gd->timer_load_val;
>> /* auto load, manual update of Timer 4 */
>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO |
>> TCON_4_UPDATE;
>>
>> /* auto load, start Timer 4 */
>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
>> - timestamp = 0;
>> + gd->timestamp = 0;
>>
>> return 0;
>> }
>> @@ -113,16 +107,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;
>> }
>>
>> /*
>> @@ -132,14 +126,14 @@ unsigned long long get_ticks(void)
>> ulong get_tbclk(void)
>> {
>> /* We overrun in 100s */
>> - return (ulong)(timer_load_val / 100);
>> + return (ulong)(gd->timer_load_val / 100);
>> }
>>
>> void reset_timer_masked(void)
>> {
>> /* reset time */
>> - lastdec = read_timer();
>> - timestamp = 0;
>> + gd->lastdec = read_timer();
>> + gd->timestamp = 0;
>> }
>>
>> void reset_timer(void)
>> @@ -150,7 +144,7 @@ void reset_timer(void)
>> ulong get_timer_masked(void)
>> {
>> unsigned long long res = get_ticks();
>> - do_div (res, (timer_load_val / (100 * CONFIG_SYS_HZ)));
>> + do_div (res, (gd->timer_load_val / (100 * CONFIG_SYS_HZ)));
>> return res;
>> }
>>
>> @@ -161,7 +155,7 @@ ulong get_timer(ulong base)
>>
>> void set_timer(ulong t)
>> {
>> - timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
>> + gd->timestamp = t * (gd->timer_load_val / (100 * CONFIG_SYS_HZ));
>> }
>>
>> void __udelay(unsigned long usec)
>> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
>> index ada3fbb..69fb7b6 100644
>> --- a/arch/arm/include/asm/global_data.h
>> +++ b/arch/arm/include/asm/global_data.h
>> @@ -61,6 +61,12 @@ typedef struct global_data {
>> unsigned long tbu;
>> unsigned long long timer_reset_value;
>> #endif
>> +#ifdef CONFIG_S3C64XX
>> + /* "static data" needed by s3c64xx's timer.c */
>> + unsigned long timer_load_val;
>> + unsigned long lastdec;
>> + unsigned long long timestamp;
>> +#endif
>> unsigned long relocaddr; /* Start address of U-Boot in RAM */
>> phys_size_t ram_size; /* RAM size */
>> unsigned long mon_len; /* monitor len */
>>
>
> I think it's not good way.
> We just need to fix the timer_init function.
> Because we can use the static variables after relocation.
at least it's working one and not new - at91 does it the same way. But
please send your patch, if it's better solution then I will be happy to
have it instead of mine.
Please remember, that these static variables are used in almost all
timer functions and timer is initialised before relocation. So you
should not change them during relocation to have correct ones after it.
Looking forward for your solution.
regards,
Darius.
>
> I'll post the timer patch for s5p series.
> Please refer it.
>
> Thanks
> Minkyu Kang
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-12 18:18 ` Darius Augulis
@ 2010-11-12 19:25 ` Albert ARIBAUD
2010-11-13 16:56 ` Darius Augulis
0 siblings, 1 reply; 8+ messages in thread
From: Albert ARIBAUD @ 2010-11-12 19:25 UTC (permalink / raw)
To: u-boot
Le 12/11/2010 19:18, Darius Augulis a ?crit :
> Dear Minkyu Kang,
>
> On 11/12/2010 05:10 AM, Minkyu Kang wrote:
>> Dear Darius Augulis,
>>
>> On 2 November 2010 05:49, Darius Augulis<augulis.darius@gmail.com> wrote:
>>> S3C64XX timer uses static variables which
>>> are initialized before relocation and used after it.
>>> Move them to global data structure.
>>>
>>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>>> ---
>>> arch/arm/cpu/arm1176/s3c64xx/timer.c | 36 ++++++++++++++--------------------
>>> arch/arm/include/asm/global_data.h | 6 ++++++
>>> 2 files changed, 21 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>> index 9768319..4e420cb 100644
>>> --- a/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>> +++ b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>> @@ -43,7 +43,7 @@
>>> #include<asm/arch/s3c6400.h>
>>> #include<div64.h>
>>>
>>> -static ulong timer_load_val;
>>> +DECLARE_GLOBAL_DATA_PTR;
>>>
>>> #define PRESCALER 167
>>>
>>> @@ -60,12 +60,6 @@ static inline ulong read_timer(void)
>>> return timers->TCNTO4;
>>> }
>>>
>>> -/* Internal tick units */
>>> -/* Last decremneter snapshot */
>>> -static unsigned long lastdec;
>>> -/* Monotonic incrementing timer */
>>> -static unsigned long long timestamp;
>>> -
>>> int timer_init(void)
>>> {
>>> s3c64xx_timers *const timers = s3c64xx_get_base_timers();
>>> @@ -83,20 +77,20 @@ int timer_init(void)
>>> * the prescaler automatically for other PCLK frequencies.
>>> */
>>> timers->TCFG0 = PRESCALER<< 8;
>>> - if (timer_load_val == 0) {
>>> - timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>>> + if (gd->timer_load_val == 0) {
>>> + gd->timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>>> timers->TCFG1 = (timers->TCFG1& ~0xf0000) | 0x20000;
>>> }
>>>
>>> /* load value for 10 ms timeout */
>>> - lastdec = timers->TCNTB4 = timer_load_val;
>>> + gd->lastdec = timers->TCNTB4 = gd->timer_load_val;
>>> /* auto load, manual update of Timer 4 */
>>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO |
>>> TCON_4_UPDATE;
>>>
>>> /* auto load, start Timer 4 */
>>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
>>> - timestamp = 0;
>>> + gd->timestamp = 0;
>>>
>>> return 0;
>>> }
>>> @@ -113,16 +107,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;
>>> }
>>>
>>> /*
>>> @@ -132,14 +126,14 @@ unsigned long long get_ticks(void)
>>> ulong get_tbclk(void)
>>> {
>>> /* We overrun in 100s */
>>> - return (ulong)(timer_load_val / 100);
>>> + return (ulong)(gd->timer_load_val / 100);
>>> }
>>>
>>> void reset_timer_masked(void)
>>> {
>>> /* reset time */
>>> - lastdec = read_timer();
>>> - timestamp = 0;
>>> + gd->lastdec = read_timer();
>>> + gd->timestamp = 0;
>>> }
>>>
>>> void reset_timer(void)
>>> @@ -150,7 +144,7 @@ void reset_timer(void)
>>> ulong get_timer_masked(void)
>>> {
>>> unsigned long long res = get_ticks();
>>> - do_div (res, (timer_load_val / (100 * CONFIG_SYS_HZ)));
>>> + do_div (res, (gd->timer_load_val / (100 * CONFIG_SYS_HZ)));
>>> return res;
>>> }
>>>
>>> @@ -161,7 +155,7 @@ ulong get_timer(ulong base)
>>>
>>> void set_timer(ulong t)
>>> {
>>> - timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
>>> + gd->timestamp = t * (gd->timer_load_val / (100 * CONFIG_SYS_HZ));
>>> }
>>>
>>> void __udelay(unsigned long usec)
>>> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
>>> index ada3fbb..69fb7b6 100644
>>> --- a/arch/arm/include/asm/global_data.h
>>> +++ b/arch/arm/include/asm/global_data.h
>>> @@ -61,6 +61,12 @@ typedef struct global_data {
>>> unsigned long tbu;
>>> unsigned long long timer_reset_value;
>>> #endif
>>> +#ifdef CONFIG_S3C64XX
>>> + /* "static data" needed by s3c64xx's timer.c */
>>> + unsigned long timer_load_val;
>>> + unsigned long lastdec;
>>> + unsigned long long timestamp;
>>> +#endif
>>> unsigned long relocaddr; /* Start address of U-Boot in RAM */
>>> phys_size_t ram_size; /* RAM size */
>>> unsigned long mon_len; /* monitor len */
>>>
>>
>> I think it's not good way.
>> We just need to fix the timer_init function.
>> Because we can use the static variables after relocation.
>
> at least it's working one and not new - at91 does it the same way. But
> please send your patch, if it's better solution then I will be happy to
> have it instead of mine.
> Please remember, that these static variables are used in almost all
> timer functions and timer is initialised before relocation. So you
> should not change them during relocation to have correct ones after it.
> Looking forward for your solution.
>
> regards,
> Darius.
FWIW, there already is a solution based on statics and post-relocation
initialization for orion5x. The principle there is that the timer is not
used before calling board_init_r, so we don't need initializing
timestamp before relocation.
I'm not saying that's how it should be done, mind; it's just how I did
it then.
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-12 19:25 ` Albert ARIBAUD
@ 2010-11-13 16:56 ` Darius Augulis
2010-11-13 18:13 ` Wolfgang Denk
0 siblings, 1 reply; 8+ messages in thread
From: Darius Augulis @ 2010-11-13 16:56 UTC (permalink / raw)
To: u-boot
Dear Albert,
On 11/12/2010 09:25 PM, Albert ARIBAUD wrote:
> Le 12/11/2010 19:18, Darius Augulis a ?crit :
>> Dear Minkyu Kang,
>>
>> On 11/12/2010 05:10 AM, Minkyu Kang wrote:
>>> Dear Darius Augulis,
>>>
>>> On 2 November 2010 05:49, Darius Augulis<augulis.darius@gmail.com> wrote:
>>>> S3C64XX timer uses static variables which
>>>> are initialized before relocation and used after it.
>>>> Move them to global data structure.
>>>>
>>>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>>>> ---
>>>> arch/arm/cpu/arm1176/s3c64xx/timer.c | 36 ++++++++++++++--------------------
>>>> arch/arm/include/asm/global_data.h | 6 ++++++
>>>> 2 files changed, 21 insertions(+), 21 deletions(-)
>>>>
>>>> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>>> index 9768319..4e420cb 100644
>>>> --- a/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>>> +++ b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>>>> @@ -43,7 +43,7 @@
>>>> #include<asm/arch/s3c6400.h>
>>>> #include<div64.h>
>>>>
>>>> -static ulong timer_load_val;
>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>
>>>> #define PRESCALER 167
>>>>
>>>> @@ -60,12 +60,6 @@ static inline ulong read_timer(void)
>>>> return timers->TCNTO4;
>>>> }
>>>>
>>>> -/* Internal tick units */
>>>> -/* Last decremneter snapshot */
>>>> -static unsigned long lastdec;
>>>> -/* Monotonic incrementing timer */
>>>> -static unsigned long long timestamp;
>>>> -
>>>> int timer_init(void)
>>>> {
>>>> s3c64xx_timers *const timers = s3c64xx_get_base_timers();
>>>> @@ -83,20 +77,20 @@ int timer_init(void)
>>>> * the prescaler automatically for other PCLK frequencies.
>>>> */
>>>> timers->TCFG0 = PRESCALER<< 8;
>>>> - if (timer_load_val == 0) {
>>>> - timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>>>> + if (gd->timer_load_val == 0) {
>>>> + gd->timer_load_val = get_PCLK() / PRESCALER * (100 / 4); /* 100s */
>>>> timers->TCFG1 = (timers->TCFG1& ~0xf0000) | 0x20000;
>>>> }
>>>>
>>>> /* load value for 10 ms timeout */
>>>> - lastdec = timers->TCNTB4 = timer_load_val;
>>>> + gd->lastdec = timers->TCNTB4 = gd->timer_load_val;
>>>> /* auto load, manual update of Timer 4 */
>>>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO |
>>>> TCON_4_UPDATE;
>>>>
>>>> /* auto load, start Timer 4 */
>>>> timers->TCON = (timers->TCON& ~0x00700000) | TCON_4_AUTO | COUNT_4_ON;
>>>> - timestamp = 0;
>>>> + gd->timestamp = 0;
>>>>
>>>> return 0;
>>>> }
>>>> @@ -113,16 +107,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;
>>>> }
>>>>
>>>> /*
>>>> @@ -132,14 +126,14 @@ unsigned long long get_ticks(void)
>>>> ulong get_tbclk(void)
>>>> {
>>>> /* We overrun in 100s */
>>>> - return (ulong)(timer_load_val / 100);
>>>> + return (ulong)(gd->timer_load_val / 100);
>>>> }
>>>>
>>>> void reset_timer_masked(void)
>>>> {
>>>> /* reset time */
>>>> - lastdec = read_timer();
>>>> - timestamp = 0;
>>>> + gd->lastdec = read_timer();
>>>> + gd->timestamp = 0;
>>>> }
>>>>
>>>> void reset_timer(void)
>>>> @@ -150,7 +144,7 @@ void reset_timer(void)
>>>> ulong get_timer_masked(void)
>>>> {
>>>> unsigned long long res = get_ticks();
>>>> - do_div (res, (timer_load_val / (100 * CONFIG_SYS_HZ)));
>>>> + do_div (res, (gd->timer_load_val / (100 * CONFIG_SYS_HZ)));
>>>> return res;
>>>> }
>>>>
>>>> @@ -161,7 +155,7 @@ ulong get_timer(ulong base)
>>>>
>>>> void set_timer(ulong t)
>>>> {
>>>> - timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
>>>> + gd->timestamp = t * (gd->timer_load_val / (100 * CONFIG_SYS_HZ));
>>>> }
>>>>
>>>> void __udelay(unsigned long usec)
>>>> diff --git a/arch/arm/include/asm/global_data.h b/arch/arm/include/asm/global_data.h
>>>> index ada3fbb..69fb7b6 100644
>>>> --- a/arch/arm/include/asm/global_data.h
>>>> +++ b/arch/arm/include/asm/global_data.h
>>>> @@ -61,6 +61,12 @@ typedef struct global_data {
>>>> unsigned long tbu;
>>>> unsigned long long timer_reset_value;
>>>> #endif
>>>> +#ifdef CONFIG_S3C64XX
>>>> + /* "static data" needed by s3c64xx's timer.c */
>>>> + unsigned long timer_load_val;
>>>> + unsigned long lastdec;
>>>> + unsigned long long timestamp;
>>>> +#endif
>>>> unsigned long relocaddr; /* Start address of U-Boot in RAM */
>>>> phys_size_t ram_size; /* RAM size */
>>>> unsigned long mon_len; /* monitor len */
>>>>
>>>
>>> I think it's not good way.
>>> We just need to fix the timer_init function.
>>> Because we can use the static variables after relocation.
>>
>> at least it's working one and not new - at91 does it the same way. But
>> please send your patch, if it's better solution then I will be happy to
>> have it instead of mine.
>> Please remember, that these static variables are used in almost all
>> timer functions and timer is initialised before relocation. So you
>> should not change them during relocation to have correct ones after it.
>> Looking forward for your solution.
>>
>> regards,
>> Darius.
>
> FWIW, there already is a solution based on statics and post-relocation
> initialization for orion5x. The principle there is that the timer is not
> used before calling board_init_r, so we don't need initializing
> timestamp before relocation.
in this case it seems like timer must be initialised after relocation.
But I don't know if it could be correct for all ARM architectures. Maybe
some of them use timer before relocation.
Global data is good place to store important static variables because
they are valid before and after relocation and it could be common for
all architectures. Since we have automatic size calculation of global
data structure there should not be a problem to add several additional
bytes specific to every CPU.
That's why I did like this and this is my opinion about this.
Darius.
>
> I'm not saying that's how it should be done, mind; it's just how I did
> it then.
>
> Amicalement,
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-13 16:56 ` Darius Augulis
@ 2010-11-13 18:13 ` Wolfgang Denk
2010-11-13 18:32 ` Albert ARIBAUD
2010-11-22 18:30 ` Darius Augulis
0 siblings, 2 replies; 8+ messages in thread
From: Wolfgang Denk @ 2010-11-13 18:13 UTC (permalink / raw)
To: u-boot
Dear Darius Augulis,
In message <4CDEC32B.7050309@gmail.com> you wrote:
>
> > FWIW, there already is a solution based on statics and post-relocation
> > initialization for orion5x. The principle there is that the timer is not
> > used before calling board_init_r, so we don't need initializing
> > timestamp before relocation.
STOP!! I don't think we want this.
> in this case it seems like timer must be initialised after relocation.
> But I don't know if it could be correct for all ARM architectures. Maybe
> some of them use timer before relocation.
Indeed. Drivers and other code may want to implement timeouts and the
like, and need at least basic timer services like udelay() and such.
> Global data is good place to store important static variables because
> they are valid before and after relocation and it could be common for
> all architectures. Since we have automatic size calculation of global
> data structure there should not be a problem to add several additional
> bytes specific to every CPU.
We should keep the gd as small as possible, but adinng one or two
integers here is indeed probably the best approach.
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
Always leave room to add an explanation if it doesn't work out.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-13 18:13 ` Wolfgang Denk
@ 2010-11-13 18:32 ` Albert ARIBAUD
2010-11-22 18:30 ` Darius Augulis
1 sibling, 0 replies; 8+ messages in thread
From: Albert ARIBAUD @ 2010-11-13 18:32 UTC (permalink / raw)
To: u-boot
Le 13/11/2010 19:13, Wolfgang Denk a ?crit :
> Dear Darius Augulis,
>
> In message<4CDEC32B.7050309@gmail.com> you wrote:
>>
>>> FWIW, there already is a solution based on statics and post-relocation
>>> initialization for orion5x. The principle there is that the timer is not
>>> used before calling board_init_r, so we don't need initializing
>>> timestamp before relocation.
>
> STOP!! I don't think we want this.
Just goes to prove a patch can get in mainline master and not get caught
for being naughy, see commit d778a2fb. :)
As I said before, I am ok with changing this to use a field of gd.
Amicalement,
--
Albert.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation
2010-11-13 18:13 ` Wolfgang Denk
2010-11-13 18:32 ` Albert ARIBAUD
@ 2010-11-22 18:30 ` Darius Augulis
1 sibling, 0 replies; 8+ messages in thread
From: Darius Augulis @ 2010-11-22 18:30 UTC (permalink / raw)
To: u-boot
Hi Wolfgang,
On 11/13/2010 08:13 PM, Wolfgang Denk wrote:
> Dear Darius Augulis,
>
> In message<4CDEC32B.7050309@gmail.com> you wrote:
>>
>>> FWIW, there already is a solution based on statics and post-relocation
>>> initialization for orion5x. The principle there is that the timer is not
>>> used before calling board_init_r, so we don't need initializing
>>> timestamp before relocation.
>
> STOP!! I don't think we want this.
>
>> in this case it seems like timer must be initialised after relocation.
>> But I don't know if it could be correct for all ARM architectures. Maybe
>> some of them use timer before relocation.
>
> Indeed. Drivers and other code may want to implement timeouts and the
> like, and need at least basic timer services like udelay() and such.
>
>> Global data is good place to store important static variables because
>> they are valid before and after relocation and it could be common for
>> all architectures. Since we have automatic size calculation of global
>> data structure there should not be a problem to add several additional
>> bytes specific to every CPU.
>
> We should keep the gd as small as possible, but adinng one or two
> integers here is indeed probably the best approach.
could you please merge it or point me what is missing?
thanks,
Darius.
>
>
> Best regards,
>
> Wolfgang Denk
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-11-22 18:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-01 20:49 [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation Darius Augulis
2010-11-12 3:10 ` Minkyu Kang
2010-11-12 18:18 ` Darius Augulis
2010-11-12 19:25 ` Albert ARIBAUD
2010-11-13 16:56 ` Darius Augulis
2010-11-13 18:13 ` Wolfgang Denk
2010-11-13 18:32 ` Albert ARIBAUD
2010-11-22 18:30 ` Darius Augulis
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox