[U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation

[Darius Augulis]

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 at 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 at 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