Patchwork [U-Boot,v2,1/2] arm: a320 timer: move static data to global_data struct

login
register
mail settings
Submitter Po-Yu Chuang
Date Dec. 20, 2010, 9 a.m.
Message ID <1292835616-1679-1-git-send-email-ratbert.chuang@gmail.com>
Download mbox | patch
Permalink /patch/76177/
State Changes Requested
Delegated to: Albert ARIBAUD
Headers show

Comments

Po-Yu Chuang - Dec. 20, 2010, 9 a.m.
From: Po-Yu Chuang <ratbert@faraday-tech.com>

timer.c used static data and are called before relocation.
Move all static variables into global_data structure. Also cleanup
timer.c from unused stubs and make it truly use 64 bit tick values.

Based on Reinhard Meyer <u-boot@emk-elektronik.de>'s patch
5dca710a3d7703e41da0e9894f2d71f9e25bea6b

Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
---
v2:
rebase
use gd->timer_rate_hz.
remove unused global variable timestamp and lastdec.
move register bases to local variables

 arch/arm/cpu/arm920t/a320/timer.c |  147 ++++++++++++++-----------------------
 1 files changed, 56 insertions(+), 91 deletions(-)
Po-Yu Chuang - Jan. 17, 2011, 2:56 a.m.
Dear Wolfgang,

On Mon, Dec 20, 2010 at 5:00 PM, Po-Yu Chuang <ratbert.chuang@gmail.com> wrote:
> From: Po-Yu Chuang <ratbert@faraday-tech.com>
>
> timer.c used static data and are called before relocation.
> Move all static variables into global_data structure. Also cleanup
> timer.c from unused stubs and make it truly use 64 bit tick values.
>
> Based on Reinhard Meyer <u-boot@emk-elektronik.de>'s patch
> 5dca710a3d7703e41da0e9894f2d71f9e25bea6b
>
> Signed-off-by: Po-Yu Chuang <ratbert@faraday-tech.com>
> ---
> v2:
> rebase
> use gd->timer_rate_hz.
> remove unused global variable timestamp and lastdec.
> move register bases to local variables
>
>  arch/arm/cpu/arm920t/a320/timer.c |  147 ++++++++++++++-----------------------
>  1 files changed, 56 insertions(+), 91 deletions(-)

Would you please check this patch series?
Or should I rebase and resubmit?

best regards,
Po-Yu Chuang
Wolfgang Denk - Jan. 18, 2011, 9:45 p.m.
Dear Po-Yu Chuang,

In message <AANLkTint_7+g+Fp2cv5qJb3kQtNbF15Y0MM7_s6HByCU@mail.gmail.com> you wrote:
> 
> > From: Po-Yu Chuang <ratbert@faraday-tech.com>
> >
> > timer.c used static data and are called before relocation.
> > Move all static variables into global_data structure. Also cleanup
> > timer.c from unused stubs and make it truly use 64 bit tick values.
...
> Would you please check this patch series?
> Or should I rebase and resubmit?

This is mostly in the responsibility of Albert, the ARM custodian.

Best regards,

Wolfgang Denk
Po-Yu Chuang - Jan. 19, 2011, 1:56 a.m.
Dear Wolfgang,

On Wed, Jan 19, 2011 at 5:45 AM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Po-Yu Chuang,
> In message <AANLkTint_7+g+Fp2cv5qJb3kQtNbF15Y0MM7_s6HByCU@mail.gmail.com> you wrote:
>> Would you please check this patch series?
>> Or should I rebase and resubmit?
>
> This is mostly in the responsibility of Albert, the ARM custodian.

Sorry, I didn't noticed that we have new custodian now.

Thanks,
Po-Yu Chuang
Albert ARIBAUD - Jan. 24, 2011, 9:18 p.m.
Le 20/12/2010 10:00, Po-Yu Chuang a écrit :
> From: Po-Yu Chuang<ratbert@faraday-tech.com>
>
> timer.c used static data and are called before relocation.
> Move all static variables into global_data structure. Also cleanup
> timer.c from unused stubs and make it truly use 64 bit tick values.
>
> Based on Reinhard Meyer<u-boot@emk-elektronik.de>'s patch
> 5dca710a3d7703e41da0e9894f2d71f9e25bea6b
>
> Signed-off-by: Po-Yu Chuang<ratbert@faraday-tech.com>
> ---
> v2:
> rebase
> use gd->timer_rate_hz.
> remove unused global variable timestamp and lastdec.
> move register bases to local variables
>
>   arch/arm/cpu/arm920t/a320/timer.c |  147 ++++++++++++++-----------------------
>   1 files changed, 56 insertions(+), 91 deletions(-)
>
> diff --git a/arch/arm/cpu/arm920t/a320/timer.c b/arch/arm/cpu/arm920t/a320/timer.c
> index d2e316f..5af94c2 100644
> --- a/arch/arm/cpu/arm920t/a320/timer.c
> +++ b/arch/arm/cpu/arm920t/a320/timer.c
> @@ -18,21 +18,36 @@
>    */
>
>   #include<common.h>
> +#include<div64.h>
>   #include<asm/io.h>
>   #include<asm/arch/ftpmu010.h>
>   #include<asm/arch/fttmr010.h>
>
> -static ulong timestamp;
> -static ulong lastdec;
> -
> -static struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
> -static struct ftpmu010 *pmu = (struct ftpmu010 *)CONFIG_FTPMU010_BASE;
> +DECLARE_GLOBAL_DATA_PTR;
>
>   #define TIMER_CLOCK	32768
>   #define TIMER_LOAD_VAL	0xffffffff
>
> +static inline unsigned long long tick_to_time(unsigned long long tick)
> +{
> +	tick *= CONFIG_SYS_HZ;
> +	do_div(tick, gd->timer_rate_hz);
> +
> +	return tick;
> +}
> +
> +static inline unsigned long long usec_to_tick(unsigned long long usec)
> +{
> +	usec *= gd->timer_rate_hz;
> +	do_div(usec, 1000000);
> +
> +	return usec;
> +}
> +
>   int timer_init(void)
>   {
> +	struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
> +	struct ftpmu010 *pmu = (struct ftpmu010 *)CONFIG_FTPMU010_BASE;
>   	unsigned int oscc;
>   	unsigned int cr;
>
> @@ -76,118 +91,68 @@ int timer_init(void)
>   	cr |= FTTMR010_TM3_ENABLE;
>   	writel(cr,&tmr->cr);
>
> -	/* init the timestamp and lastdec value */
> -	reset_timer_masked();
> +	gd->timer_rate_hz = TIMER_CLOCK;
> +	gd->tbu = gd->tbl = 0;
>
>   	return 0;
>   }
>
>   /*
> - * timer without interrupts
> - */
> -
> -/*
> - * reset time
> + * Get the current 64 bit timer tick count
>    */
> -void reset_timer_masked(void)
> +unsigned long long get_ticks(void)
>   {
> -	/* capure current decrementer value time */
> -	lastdec = readl(&tmr->timer3_counter) / (TIMER_CLOCK / CONFIG_SYS_HZ);
> -	timestamp = 0;		/* start "advancing" time stamp from 0 */
> -
> -	debug("%s(): lastdec = %lx\n", __func__, lastdec);
> -}
> +	struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
> +	ulong now = TIMER_LOAD_VAL - readl(&tmr->timer3_counter);
>
> -void reset_timer(void)
> -{
>   	debug("%s()\n", __func__);
> -	reset_timer_masked();
> -}
>
> -/*
> - * return timer ticks
> - */
> -ulong get_timer_masked(void)
> -{
> -	/* current tick value */
> -	ulong now = readl(&tmr->timer3_counter) / (TIMER_CLOCK / CONFIG_SYS_HZ);
> -
> -	debug("%s(): now = %lx, lastdec = %lx\n", __func__, now, lastdec);
> -
> -	if (lastdec>= now) {
> -		/*
> -		 * normal mode (non roll)
> -		 * move stamp fordward with absoulte diff ticks
> -		 */
> -		timestamp += lastdec - now;
> -	} else {
> -		/*
> -		 * we have overflow of the count down timer
> -		 *
> -		 * nts = ts + ld + (TLV - now)
> -		 * ts=old stamp, ld=time that passed before passing through -1
> -		 * (TLV-now) amount of time after passing though -1
> -		 * nts = new "advancing time stamp"...it could also roll and
> -		 * cause problems.
> -		 */
> -		timestamp += lastdec + TIMER_LOAD_VAL - now;
> -	}
> -
> -	lastdec = now;
> -
> -	debug("%s() returns %lx\n", __func__, timestamp);
> -
> -	return timestamp;
> -}
> -
> -/*
> - * return difference between timer ticks and base
> - */
> -ulong get_timer(ulong base)
> -{
> -	debug("%s(%lx)\n", __func__, base);
> -	return get_timer_masked() - base;
> -}
> -
> -void set_timer(ulong t)
> -{
> -	debug("%s(%lx)\n", __func__, t);
> -	timestamp = t;
> +	/* increment tbu if tbl has rolled over */
> +	if (now<  gd->tbl)
> +		gd->tbu++;
> +	gd->tbl = now;
> +	return (((unsigned long long)gd->tbu)<<  32) | gd->tbl;
>   }
>
>   /* delay x useconds AND preserve advance timestamp value */
>   void __udelay(unsigned long usec)
>   {
> -	long tmo = usec * (TIMER_CLOCK / 1000) / 1000;
> -	unsigned long now, last = readl(&tmr->timer3_counter);
> -
> -	debug("%s(%lu)\n", __func__, usec);
> -	while (tmo>  0) {
> -		now = readl(&tmr->timer3_counter);
> -		if (now>  last) /* count down timer overflow */
> -			tmo -= TIMER_LOAD_VAL + last - now;
> -		else
> -			tmo -= last - now;
> -		last = now;
> -	}
> +	unsigned long long tmp;
> +	ulong tmo;
> +
> +	tmo = usec_to_tick(usec);
> +	tmp = get_ticks() + tmo;	/* get current timestamp */
> +
> +	while (get_ticks()<  tmp)	/* loop till event */
> +		;
>   }
>
>   /*
> - * This function is derived from PowerPC code (read timebase as long long).
> - * On ARM it just returns the timer value.
> + * reset_timer() and get_timer(base) are a pair of functions that are used by
> + * some timeout/sleep mechanisms in u-boot.
> + *
> + * reset_timer() marks the current time as epoch and
> + * get_timer(base) works relative to that epoch.
> + *
> + * The time is used in CONFIG_SYS_HZ units!
>    */
> -unsigned long long get_ticks(void)
> +void reset_timer(void)
>   {
>   	debug("%s()\n", __func__);
> -	return get_timer(0);
> +	gd->timer_reset_value = get_ticks();
> +}
> +
> +ulong get_timer(ulong base)
> +{
> +	debug("%s(%lx)\n", __func__, base);
> +	return tick_to_time(get_ticks() - gd->timer_reset_value) - base;
>   }
>
>   /*
> - * This function is derived from PowerPC code (timebase clock frequency).
> - * On ARM it returns the number of timer ticks per second.
> + * Return the number of timer ticks per second.
>    */
>   ulong get_tbclk(void)
>   {
>   	debug("%s()\n", __func__);
> -	return CONFIG_SYS_HZ;
> +	return gd->timer_rate_hz;
>   }

At the moment I won't apply changes to timer code which are more than 
simple fixes, because we are in a discussion to normalize the use of 
timers in U-boot; depending on the outcome, this patch could need to be 
reversed in the end.

Meanwhile, a patch with only the fixes to the use of static variables 
remains welcome.

Amicalement,

Patch

diff --git a/arch/arm/cpu/arm920t/a320/timer.c b/arch/arm/cpu/arm920t/a320/timer.c
index d2e316f..5af94c2 100644
--- a/arch/arm/cpu/arm920t/a320/timer.c
+++ b/arch/arm/cpu/arm920t/a320/timer.c
@@ -18,21 +18,36 @@ 
  */
 
 #include <common.h>
+#include <div64.h>
 #include <asm/io.h>
 #include <asm/arch/ftpmu010.h>
 #include <asm/arch/fttmr010.h>
 
-static ulong timestamp;
-static ulong lastdec;
-
-static struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
-static struct ftpmu010 *pmu = (struct ftpmu010 *)CONFIG_FTPMU010_BASE;
+DECLARE_GLOBAL_DATA_PTR;
 
 #define TIMER_CLOCK	32768
 #define TIMER_LOAD_VAL	0xffffffff
 
+static inline unsigned long long tick_to_time(unsigned long long tick)
+{
+	tick *= CONFIG_SYS_HZ;
+	do_div(tick, gd->timer_rate_hz);
+
+	return tick;
+}
+
+static inline unsigned long long usec_to_tick(unsigned long long usec)
+{
+	usec *= gd->timer_rate_hz;
+	do_div(usec, 1000000);
+
+	return usec;
+}
+
 int timer_init(void)
 {
+	struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
+	struct ftpmu010 *pmu = (struct ftpmu010 *)CONFIG_FTPMU010_BASE;
 	unsigned int oscc;
 	unsigned int cr;
 
@@ -76,118 +91,68 @@  int timer_init(void)
 	cr |= FTTMR010_TM3_ENABLE;
 	writel(cr, &tmr->cr);
 
-	/* init the timestamp and lastdec value */
-	reset_timer_masked();
+	gd->timer_rate_hz = TIMER_CLOCK;
+	gd->tbu = gd->tbl = 0;
 
 	return 0;
 }
 
 /*
- * timer without interrupts
- */
-
-/*
- * reset time
+ * Get the current 64 bit timer tick count
  */
-void reset_timer_masked(void)
+unsigned long long get_ticks(void)
 {
-	/* capure current decrementer value time */
-	lastdec = readl(&tmr->timer3_counter) / (TIMER_CLOCK / CONFIG_SYS_HZ);
-	timestamp = 0;		/* start "advancing" time stamp from 0 */
-
-	debug("%s(): lastdec = %lx\n", __func__, lastdec);
-}
+	struct fttmr010 *tmr = (struct fttmr010 *)CONFIG_FTTMR010_BASE;
+	ulong now = TIMER_LOAD_VAL - readl(&tmr->timer3_counter);
 
-void reset_timer(void)
-{
 	debug("%s()\n", __func__);
-	reset_timer_masked();
-}
 
-/*
- * return timer ticks
- */
-ulong get_timer_masked(void)
-{
-	/* current tick value */
-	ulong now = readl(&tmr->timer3_counter) / (TIMER_CLOCK / CONFIG_SYS_HZ);
-
-	debug("%s(): now = %lx, lastdec = %lx\n", __func__, now, lastdec);
-
-	if (lastdec >= now) {
-		/*
-		 * normal mode (non roll)
-		 * move stamp fordward with absoulte diff ticks
-		 */
-		timestamp += lastdec - now;
-	} else {
-		/*
-		 * we have overflow of the count down timer
-		 *
-		 * nts = ts + ld + (TLV - now)
-		 * ts=old stamp, ld=time that passed before passing through -1
-		 * (TLV-now) amount of time after passing though -1
-		 * nts = new "advancing time stamp"...it could also roll and
-		 * cause problems.
-		 */
-		timestamp += lastdec + TIMER_LOAD_VAL - now;
-	}
-
-	lastdec = now;
-
-	debug("%s() returns %lx\n", __func__, timestamp);
-
-	return timestamp;
-}
-
-/*
- * return difference between timer ticks and base
- */
-ulong get_timer(ulong base)
-{
-	debug("%s(%lx)\n", __func__, base);
-	return get_timer_masked() - base;
-}
-
-void set_timer(ulong t)
-{
-	debug("%s(%lx)\n", __func__, t);
-	timestamp = t;
+	/* increment tbu if tbl has rolled over */
+	if (now < gd->tbl)
+		gd->tbu++;
+	gd->tbl = now;
+	return (((unsigned long long)gd->tbu) << 32) | gd->tbl;
 }
 
 /* delay x useconds AND preserve advance timestamp value */
 void __udelay(unsigned long usec)
 {
-	long tmo = usec * (TIMER_CLOCK / 1000) / 1000;
-	unsigned long now, last = readl(&tmr->timer3_counter);
-
-	debug("%s(%lu)\n", __func__, usec);
-	while (tmo > 0) {
-		now = readl(&tmr->timer3_counter);
-		if (now > last) /* count down timer overflow */
-			tmo -= TIMER_LOAD_VAL + last - now;
-		else
-			tmo -= last - now;
-		last = now;
-	}
+	unsigned long long tmp;
+	ulong tmo;
+
+	tmo = usec_to_tick(usec);
+	tmp = get_ticks() + tmo;	/* get current timestamp */
+
+	while (get_ticks() < tmp)	/* loop till event */
+		;
 }
 
 /*
- * This function is derived from PowerPC code (read timebase as long long).
- * On ARM it just returns the timer value.
+ * reset_timer() and get_timer(base) are a pair of functions that are used by
+ * some timeout/sleep mechanisms in u-boot.
+ *
+ * reset_timer() marks the current time as epoch and
+ * get_timer(base) works relative to that epoch.
+ *
+ * The time is used in CONFIG_SYS_HZ units!
  */
-unsigned long long get_ticks(void)
+void reset_timer(void)
 {
 	debug("%s()\n", __func__);
-	return get_timer(0);
+	gd->timer_reset_value = get_ticks();
+}
+
+ulong get_timer(ulong base)
+{
+	debug("%s(%lx)\n", __func__, base);
+	return tick_to_time(get_ticks() - gd->timer_reset_value) - base;
 }
 
 /*
- * This function is derived from PowerPC code (timebase clock frequency).
- * On ARM it returns the number of timer ticks per second.
+ * Return the number of timer ticks per second.
  */
 ulong get_tbclk(void)
 {
 	debug("%s()\n", __func__);
-	return CONFIG_SYS_HZ;
+	return gd->timer_rate_hz;
 }