[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Minkyu Kang]

Use the global data instead of bss variable, replace as follow.
timer_load_val -> timer_rate_hz
timestamp -> timer_reset_value
lastdec -> lastinc

Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
Signed-off-by: Darius Augulis <augulis.darius@gmail.com>
cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/cpu/arm1176/s3c64xx/timer.c |   40 +++++++++++++--------------------
 1 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
index 9768319..c1280f2 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,18 @@ 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 */
-		timers->TCFG1 = (timers->TCFG1 & ~0xf0000) | 0x20000;
-	}
+	gd->timer_rate_hz = 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->lastinc = timers->TCNTB4 = gd->timer_rate_hz;
 	/* 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->timer_reset_value = 0;
 
 	return 0;
 }
@@ -113,16 +105,16 @@ unsigned long long get_ticks(void)
 {
 	ulong now = read_timer();
 
-	if (lastdec >= now) {
+	if (gd->lastinc >= now) {
 		/* normal mode */
-		timestamp += lastdec - now;
+		gd->timer_reset_value += gd->lastinc - now;
 	} else {
 		/* we have an overflow ... */
-		timestamp += lastdec + timer_load_val - now;
+		gd->timer_reset_value += gd->lastinc + gd->timer_rate_hz - now;
 	}
-	lastdec = now;
+	gd->lastinc = now;
 
-	return timestamp;
+	return gd->timer_reset_value;
 }
 
 /*
@@ -132,14 +124,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_rate_hz / 100);
 }
 
 void reset_timer_masked(void)
 {
 	/* reset time */
-	lastdec = read_timer();
-	timestamp = 0;
+	gd->lastinc = read_timer();
+	gd->timer_reset_value = 0;
 }
 
 void reset_timer(void)
@@ -150,7 +142,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_rate_hz / (100 * CONFIG_SYS_HZ)));
 	return res;
 }
 
@@ -161,7 +153,7 @@ ulong get_timer(ulong base)
 
 void set_timer(ulong t)
 {
-	timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
+	gd->timer_reset_value = t * (gd->timer_rate_hz / (100 * CONFIG_SYS_HZ));
 }
 
 void __udelay(unsigned long usec)
@@ -170,7 +162,7 @@ void __udelay(unsigned long usec)
 	ulong tmo;
 
 	tmo = (usec + 9) / 10;
-	tmp = get_ticks() + tmo;	/* get current timestamp */
+	tmp = get_ticks() + tmo;	/* get current timer_reset_value */
 
 	while (get_ticks() < tmp)/* loop till event */
 		 /*NOP*/;
-- 
1.7.1

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[seedshope]

On 01/04/2011 04:14 PM, Minkyu Kang wrote:
> Use the global data instead of bss variable, replace as follow.
> timer_load_val ->  timer_rate_hz
> timestamp ->  timer_reset_value
> lastdec ->  lastinc
I have already test the patch on s3c6410 . It is work perfect.

Thanks,
seedshope
> Signed-off-by: Minkyu Kang<mk7.kang@samsung.com>
> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
> cc: Heiko Schocher<hs@denx.de>
> ---
>   arch/arm/cpu/arm1176/s3c64xx/timer.c |   40 +++++++++++++--------------------
>   1 files changed, 16 insertions(+), 24 deletions(-)
>
> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c b/arch/arm/cpu/arm1176/s3c64xx/timer.c
> index 9768319..c1280f2 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,18 @@ 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 */
> -		timers->TCFG1 = (timers->TCFG1&  ~0xf0000) | 0x20000;
> -	}
> +	gd->timer_rate_hz = 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->lastinc = timers->TCNTB4 = gd->timer_rate_hz;
>   	/* 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->timer_reset_value = 0;
>
>   	return 0;
>   }
> @@ -113,16 +105,16 @@ unsigned long long get_ticks(void)
>   {
>   	ulong now = read_timer();
>
> -	if (lastdec>= now) {
> +	if (gd->lastinc>= now) {
>   		/* normal mode */
> -		timestamp += lastdec - now;
> +		gd->timer_reset_value += gd->lastinc - now;
>   	} else {
>   		/* we have an overflow ... */
> -		timestamp += lastdec + timer_load_val - now;
> +		gd->timer_reset_value += gd->lastinc + gd->timer_rate_hz - now;
>   	}
> -	lastdec = now;
> +	gd->lastinc = now;
>
> -	return timestamp;
> +	return gd->timer_reset_value;
>   }
>
>   /*
> @@ -132,14 +124,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_rate_hz / 100);
>   }
>
>   void reset_timer_masked(void)
>   {
>   	/* reset time */
> -	lastdec = read_timer();
> -	timestamp = 0;
> +	gd->lastinc = read_timer();
> +	gd->timer_reset_value = 0;
>   }
>
>   void reset_timer(void)
> @@ -150,7 +142,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_rate_hz / (100 * CONFIG_SYS_HZ)));
>   	return res;
>   }
>
> @@ -161,7 +153,7 @@ ulong get_timer(ulong base)
>
>   void set_timer(ulong t)
>   {
> -	timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
> +	gd->timer_reset_value = t * (gd->timer_rate_hz / (100 * CONFIG_SYS_HZ));
>   }
>
>   void __udelay(unsigned long usec)
> @@ -170,7 +162,7 @@ void __udelay(unsigned long usec)
>   	ulong tmo;
>
>   	tmo = (usec + 9) / 10;
> -	tmp = get_ticks() + tmo;	/* get current timestamp */
> +	tmp = get_ticks() + tmo;	/* get current timer_reset_value */
>
>   	while (get_ticks()<  tmp)/* loop till event */
>   		 /*NOP*/;

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[seedshope]

On 01/04/2011 10:46 PM, seedshope wrote:
> On 01/04/2011 04:14 PM, Minkyu Kang wrote:
>> Use the global data instead of bss variable, replace as follow.
>> timer_load_val ->  timer_rate_hz
>> timestamp ->  timer_reset_value
>> lastdec ->  lastinc
> I have already test the patch on s3c6410 . It is work perfect.
Before I am not use the patch, When I use the command "nand read",
The some information will generate. as following:
raise: Signal # 8 caught

I also trace the timer.c, But I can't resolve it. As if after the first 
initial timer,
I guess the bss will be clear.

Now It is ok after patch this patch.

Thanks,
seedshope
>
> Thanks,
> seedshope
>> Signed-off-by: Minkyu Kang<mk7.kang@samsung.com>
>> Signed-off-by: Darius Augulis<augulis.darius@gmail.com>
>> cc: Heiko Schocher<hs@denx.de>
>> ---
>>   arch/arm/cpu/arm1176/s3c64xx/timer.c |   40 
>> +++++++++++++--------------------
>>   1 files changed, 16 insertions(+), 24 deletions(-)
>>
>> diff --git a/arch/arm/cpu/arm1176/s3c64xx/timer.c 
>> b/arch/arm/cpu/arm1176/s3c64xx/timer.c
>> index 9768319..c1280f2 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,18 @@ 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 */
>> -        timers->TCFG1 = (timers->TCFG1&  ~0xf0000) | 0x20000;
>> -    }
>> +    gd->timer_rate_hz = 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->lastinc = timers->TCNTB4 = gd->timer_rate_hz;
>>       /* 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->timer_reset_value = 0;
>>
>>       return 0;
>>   }
>> @@ -113,16 +105,16 @@ unsigned long long get_ticks(void)
>>   {
>>       ulong now = read_timer();
>>
>> -    if (lastdec>= now) {
>> +    if (gd->lastinc>= now) {
>>           /* normal mode */
>> -        timestamp += lastdec - now;
>> +        gd->timer_reset_value += gd->lastinc - now;
>>       } else {
>>           /* we have an overflow ... */
>> -        timestamp += lastdec + timer_load_val - now;
>> +        gd->timer_reset_value += gd->lastinc + gd->timer_rate_hz - now;
>>       }
>> -    lastdec = now;
>> +    gd->lastinc = now;
>>
>> -    return timestamp;
>> +    return gd->timer_reset_value;
>>   }
>>
>>   /*
>> @@ -132,14 +124,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_rate_hz / 100);
>>   }
>>
>>   void reset_timer_masked(void)
>>   {
>>       /* reset time */
>> -    lastdec = read_timer();
>> -    timestamp = 0;
>> +    gd->lastinc = read_timer();
>> +    gd->timer_reset_value = 0;
>>   }
>>
>>   void reset_timer(void)
>> @@ -150,7 +142,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_rate_hz / (100 * CONFIG_SYS_HZ)));
>>       return res;
>>   }
>>
>> @@ -161,7 +153,7 @@ ulong get_timer(ulong base)
>>
>>   void set_timer(ulong t)
>>   {
>> -    timestamp = t * (timer_load_val / (100 * CONFIG_SYS_HZ));
>> +    gd->timer_reset_value = t * (gd->timer_rate_hz / (100 * 
>> CONFIG_SYS_HZ));
>>   }
>>
>>   void __udelay(unsigned long usec)
>> @@ -170,7 +162,7 @@ void __udelay(unsigned long usec)
>>       ulong tmo;
>>
>>       tmo = (usec + 9) / 10;
>> -    tmp = get_ticks() + tmo;    /* get current timestamp */
>> +    tmp = get_ticks() + tmo;    /* get current timer_reset_value */
>>
>>       while (get_ticks()<  tmp)/* loop till event */
>>            /*NOP*/;
>

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Minkyu Kang]

Dear seedshope,

On 4 January 2011 23:56, seedshope <bocui107@gmail.com> wrote:
> On 01/04/2011 10:46 PM, seedshope wrote:
>> On 01/04/2011 04:14 PM, Minkyu Kang wrote:
>>> Use the global data instead of bss variable, replace as follow.
>>> timer_load_val -> ?timer_rate_hz
>>> timestamp -> ?timer_reset_value
>>> lastdec -> ?lastinc
>> I have already test the patch on s3c6410 . It is work perfect.
> Before I am not use the patch, When I use the command "nand read",
> The some information will generate. as following:
> raise: Signal # 8 caught
>
> I also trace the timer.c, But I can't resolve it. As if after the first
> initial timer,
> I guess the bss will be clear.
>
> Now It is ok after patch this patch.
>
> Thanks,
> seedshope
>>

Thanks for testing.

Did you test with latest version?
Currently, SMDK6400 can't build because didn't rework for relocation.
If you are OK, could you please fix it?
I appreciate your help.

Minkyu Kang
-- 
from. prom.
www.promsoft.net

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[seedshope]

On 01/05/2011 03:04 PM, Minkyu Kang wrote:
> Dear seedshope,
>
> On 4 January 2011 23:56, seedshope<bocui107@gmail.com>  wrote:
>> On 01/04/2011 10:46 PM, seedshope wrote:
>>> On 01/04/2011 04:14 PM, Minkyu Kang wrote:
>>>> Use the global data instead of bss variable, replace as follow.
>>>> timer_load_val ->   timer_rate_hz
>>>> timestamp ->   timer_reset_value
>>>> lastdec ->   lastinc
>>> I have already test the patch on s3c6410 . It is work perfect.
>> Before I am not use the patch, When I use the command "nand read",
>> The some information will generate. as following:
>> raise: Signal # 8 caught
>>
>> I also trace the timer.c, But I can't resolve it. As if after the first
>> initial timer,
>> I guess the bss will be clear.
>>
>> Now It is ok after patch this patch.
>>
>> Thanks,
>> seedshope
> Thanks for testing.
>
> Did you test with latest version?
Yes,
> Currently, SMDK6400 can't build because didn't rework for relocation.
> If you are OK, could you please fix it?
Ok, recently, I do the patch for SMD6400 and SMDK6410,
I will send request review in the weekend?

Thank?
seedshope
> I appreciate your help.
>
> Minkyu Kang

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Minkyu Kang]

Dear seedshope,

On 6 January 2011 00:57, seedshope <bocui107@gmail.com> wrote:
> On 01/05/2011 03:04 PM, Minkyu Kang wrote:
>>
>> Dear seedshope,
>>
>> On 4 January 2011 23:56, seedshope<bocui107@gmail.com> ?wrote:
>>>
>>> On 01/04/2011 10:46 PM, seedshope wrote:
>>>>
>>>> On 01/04/2011 04:14 PM, Minkyu Kang wrote:
>>>>>
>>>>> Use the global data instead of bss variable, replace as follow.
>>>>> timer_load_val -> ? timer_rate_hz
>>>>> timestamp -> ? timer_reset_value
>>>>> lastdec -> ? lastinc
>>>>
>>>> I have already test the patch on s3c6410 . It is work perfect.
>>>
>>> Before I am not use the patch, When I use the command "nand read",
>>> The some information will generate. as following:
>>> raise: Signal # 8 caught
>>>
>>> I also trace the timer.c, But I can't resolve it. As if after the first
>>> initial timer,
>>> I guess the bss will be clear.
>>>
>>> Now It is ok after patch this patch.
>>>
>>> Thanks,
>>> seedshope
>>
>> Thanks for testing.
>>
>> Did you test with latest version?
>
> Yes,
>>
>> Currently, SMDK6400 can't build because didn't rework for relocation.
>> If you are OK, could you please fix it?
>
> Ok, recently, I do the patch for SMD6400 and SMDK6410,
> I will send request review in the weekend?
>

Thanks! :)

Minkyu Kang
-- 
from. prom.
www.promsoft.net

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Reinhard Meyer]

Dear concerned,
>>>>>> Use the global data instead of bss variable, replace as follow.
>>>>>> timer_load_val ->     timer_rate_hz
>>>>>> timestamp ->     timer_reset_value

I am not too happy about this "misuse" of gd->variables making them
"misnomers" and the code harder to read.

timer_rate_hz is supposed to hold the rate at which the high
speed timer increments.

timer_reset_value supposedly holds the high speed timer's value when
timer_reset() is called.

There was a discussion started about this a while ago, but came to no
conclusion...

Best Regards,
Reinhard

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Wolfgang Denk]

Dear Reinhard Meyer,

In message <4D2582F6.2040909@emk-elektronik.de> you wrote:
>
> >>>>>> timer_load_val ->     timer_rate_hz
> >>>>>> timestamp ->     timer_reset_value
> 
> I am not too happy about this "misuse" of gd->variables making them
> "misnomers" and the code harder to read.

I'm not happy with this as well, and I actually will not accept the
patch because of that.

> timer_rate_hz is supposed to hold the rate at which the high
> speed timer increments.
> 
> timer_reset_value supposedly holds the high speed timer's value when
> timer_reset() is called.
> 
> There was a discussion started about this a while ago, but came to no
> conclusion...

I suggested this before:

------- Forwarded Message

Date:    Thu, 16 Dec 2010 15:12:41 +0100
From:    Wolfgang Denk <wd@denx.de>
To:      Reinhard Meyer <u-boot@emk-elektronik.de>
cc:      U-Boot user list <u-boot@lists.denx.de>, hs at denx.de
Subject: Re: [U-Boot] arm926ejs, timer:

...
Maybe we should lean back and have a look at what Linux is doing in
this area?

The recent patch series "64-bit sched_clock" on the lak ML seems to
fit pretty well :-)

See http://thread.gmane.org/gmane.linux.ports.arm.kernel/99740


Can we copy from Linux?
...

------- End of Forwarded Message


Unfortunately it seems nobody had time or resources to have a look
yet.

I think we should fix this for real now, and not continue to implement
one SoC specific version or workaround after the other.

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
If you hear an onion ring, answer it.

[U-Boot] [PATCH] S3C64XX: timer: replace bss variable by gd

[Wolfgang Denk]

Dear Minkyu Kang,

In message <4D22D6F7.3050609@samsung.com> you wrote:
> Use the global data instead of bss variable, replace as follow.
> timer_load_val -> timer_rate_hz
> timestamp -> timer_reset_value

I object against using variables wich have somewhat self-explanatory
names for different purposes.

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
I program, therefore I am.