| Message ID | 4D22D6F7.3050609@samsung.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Minkyu Kang |
| Headers | show |
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*/;
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*/; >
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
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
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
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
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
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@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
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*/;