diff mbox

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

Message ID 4D22D59B.7030902@samsung.com
State Changes Requested
Delegated to: Minkyu Kang
Headers show

Commit Message

Minkyu Kang Jan. 4, 2011, 8:08 a.m. UTC 
Use the global data instead of bss variable, replace as follow.
count_value -> timer_rate_hz
timestamp -> timer_reset_value
lastdec -> lastinc

Signed-off-by: Minkyu Kang <mk7.kang@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/cpu/armv7/s5p-common/timer.c |   48 +++++++++++++++------------------
 1 files changed, 22 insertions(+), 26 deletions(-)

Comments

Wolfgang Denk Jan. 9, 2011, 4:14 p.m. UTC | #1 
Dear Minkyu Kang,

In message <4D22D59B.7030902@samsung.com> you wrote:
> Use the global data instead of bss variable, replace as follow.
> count_value -> timer_rate_hz
> timestamp -> timer_reset_value
> lastdec -> lastinc

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

Best regards,

Wolfgang Denk
Minkyu Kang Jan. 10, 2011, 1:46 a.m. UTC | #2 
Dear Wolfgang Denk,

On 10 January 2011 01:14, Wolfgang Denk <wd@denx.de> wrote:
> Dear Minkyu Kang,
>
> In message <4D22D59B.7030902@samsung.com> you wrote:
>> Use the global data instead of bss variable, replace as follow.
>> count_value -> timer_rate_hz
>> timestamp -> timer_reset_value
>> lastdec -> lastinc
>
> I object against using variables wich have somewhat self-explanatory
> names for different purposes.
>

Yes, I know.
If so, do I have to add new variables at global_data?
Maybe that is not good way too.
How you think?

One more question,
what is the purpose of tbl and tbu?

Thanks
Minkyu Kang
Wolfgang Denk Jan. 17, 2011, 9:46 p.m. UTC | #3 
Dear Minkyu Kang,

In message <AANLkTikoZFrd9SYLg4v3AoqRFEd9bM0jpQQOi+Qzy6eg@mail.gmail.com> you wrote:
> 
> > I object against using variables wich have somewhat self-explanatory
> > names for different purposes.
> 
> Yes, I know.
> If so, do I have to add new variables at global_data?
> Maybe that is not good way too.
> How you think?

I asked this several times before: ther eis currently ongoing efforts
for aunified clock handling for ARM in the Linux kernel.  Maybe we can
benefit from this in any way?

> One more question,
> what is the purpose of tbl and tbu?

tbl (time base, lower part) and tbu (time base, upper part) are the
software representation of two 32 bit hardware registers available in
Power Architecture systems, where they form a free-runnign 64 bit
counter that starts automatically as soon as the CPU comes out of
reeset.

There have been attempts to re-write the (sometimes somewhat obscure)
timer implementations on ARM based on such a model.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/arch/arm/cpu/armv7/s5p-common/timer.c b/arch/arm/cpu/armv7/s5p-common/timer.c
index 651fd5d..0a1883f 100644
--- a/arch/arm/cpu/armv7/s5p-common/timer.c
+++ b/arch/arm/cpu/armv7/s5p-common/timer.c
@@ -28,6 +28,8 @@ 
 #include <asm/arch/pwm.h>
 #include <asm/arch/clk.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define PRESCALER_1		(16 - 1)	/* prescaler of timer 2, 3, 4 */
 #define MUX_DIV_2		1		/* 1/2 period */
 #define MUX_DIV_4		2		/* 1/4 period */
@@ -37,12 +39,6 @@ 
 
 #define TCON_TIMER4_SHIFT	20
 
-static unsigned long count_value;
-
-/* Internal tick units */
-static unsigned long long timestamp;	/* Monotonic incrementing timer */
-static unsigned long lastdec;		/* Last decremneter snapshot */
-
 /* macro to read the 16 bit timer */
 static inline struct s5p_timer *s5p_get_base_timer(void)
 {
@@ -65,16 +61,16 @@  int timer_init(void)
 	writel((PRESCALER_1 & 0xff) << 8, &timer->tcfg0);
 	writel((MUX_DIV_2 & 0xf) << MUX4_DIV_SHIFT, &timer->tcfg1);
 
-	/* count_value = 2085937.5(HZ) (per 1 sec)*/
-	count_value = get_pwm_clk() / ((PRESCALER_1 + 1) *
+	/* timer_rate_hz = 2085937.5(HZ) (per 1 sec)*/
+	gd->timer_rate_hz = get_pwm_clk() / ((PRESCALER_1 + 1) *
 			(MUX_DIV_2 + 1));
 
-	/* count_value / 100 = 20859.375(HZ) (per 10 msec) */
-	count_value = count_value / 100;
+	/* timer_rate_hz / 100 = 20859.375(HZ) (per 10 msec) */
+	gd->timer_rate_hz = gd->timer_rate_hz / 100;
 
 	/* set count value */
-	writel(count_value, &timer->tcntb4);
-	lastdec = count_value;
+	writel(gd->timer_rate_hz, &timer->tcntb4);
+	gd->lastinc = gd->timer_rate_hz;
 
 	val = (readl(&timer->tcon) & ~(0x07 << TCON_TIMER4_SHIFT)) |
 		TCON4_AUTO_RELOAD;
@@ -85,7 +81,7 @@  int timer_init(void)
 	/* start PWM timer 4 */
 	writel(val | TCON4_START, &timer->tcon);
 
-	timestamp = 0;
+	gd->timer_reset_value = 0;
 
 	return 0;
 }
@@ -105,7 +101,7 @@  unsigned long get_timer(unsigned long base)
 
 void set_timer(unsigned long t)
 {
-	timestamp = t;
+	gd->timer_reset_value = t;
 }
 
 /* delay x useconds */
@@ -114,7 +110,7 @@  void __udelay(unsigned long usec)
 	struct s5p_timer *const timer = s5p_get_base_timer();
 	unsigned long tmo, tmp;
 
-	count_value = readl(&timer->tcntb4);
+	gd->timer_rate_hz = readl(&timer->tcntb4);
 
 	if (usec >= 1000) {
 		/*
@@ -125,19 +121,19 @@  void __udelay(unsigned long usec)
 		 * 3. finish normalize.
 		 */
 		tmo = usec / 1000;
-		tmo *= (CONFIG_SYS_HZ * count_value / 10);
+		tmo *= (CONFIG_SYS_HZ * gd->timer_rate_hz / 10);
 		tmo /= 1000;
 	} else {
 		/* else small number, don't kill it prior to HZ multiply */
-		tmo = usec * CONFIG_SYS_HZ * count_value / 10;
+		tmo = usec * CONFIG_SYS_HZ * gd->timer_rate_hz / 10;
 		tmo /= (1000 * 1000);
 	}
 
-	/* get current timestamp */
+	/* get current timer_reset_value */
 	tmp = get_timer(0);
 
 	/* if setting this fordward will roll time stamp */
-	/* reset "advancing" timestamp to 0, set lastdec value */
+	/* reset "advancing" timer_reset_value to 0, set lastinc value */
 	/* else, set advancing stamp wake up time */
 	if ((tmo + tmp + 1) < tmp)
 		reset_timer_masked();
@@ -154,8 +150,8 @@  void reset_timer_masked(void)
 	struct s5p_timer *const timer = s5p_get_base_timer();
 
 	/* reset time */
-	lastdec = readl(&timer->tcnto4);
-	timestamp = 0;
+	gd->lastinc = readl(&timer->tcnto4);
+	gd->timer_reset_value = 0;
 }
 
 unsigned long get_timer_masked(void)
@@ -163,14 +159,14 @@  unsigned long get_timer_masked(void)
 	struct s5p_timer *const timer = s5p_get_base_timer();
 	unsigned long now = readl(&timer->tcnto4);
 
-	if (lastdec >= now)
-		timestamp += lastdec - now;
+	if (gd->lastinc >= now)
+		gd->timer_reset_value += gd->lastinc - now;
 	else
-		timestamp += lastdec + count_value - now;
+		gd->timer_reset_value += gd->lastinc + gd->timer_rate_hz - now;
 
-	lastdec = now;
+	gd->lastinc = now;
 
-	return timestamp;
+	return gd->timer_reset_value;
 }
 
 /*