* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation @ 2010-11-01 20:49 Darius Augulis 2010-11-12 3:10 ` Minkyu Kang 0 siblings, 1 reply; 8+ messages in thread From: Darius Augulis @ 2010-11-01 20:49 UTC (permalink / raw) To: u-boot 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@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 */ ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-01 20:49 [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation Darius Augulis @ 2010-11-12 3:10 ` Minkyu Kang 2010-11-12 18:18 ` Darius Augulis 0 siblings, 1 reply; 8+ messages in thread From: Minkyu Kang @ 2010-11-12 3:10 UTC (permalink / raw) To: u-boot Dear Darius Augulis, On 2 November 2010 05:49, Darius Augulis <augulis.darius@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@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. I'll post the timer patch for s5p series. Please refer it. Thanks Minkyu Kang -- from. prom. www.promsoft.net ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-12 3:10 ` Minkyu Kang @ 2010-11-12 18:18 ` Darius Augulis 2010-11-12 19:25 ` Albert ARIBAUD 0 siblings, 1 reply; 8+ messages in thread From: Darius Augulis @ 2010-11-12 18:18 UTC (permalink / raw) To: u-boot 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@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@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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-12 18:18 ` Darius Augulis @ 2010-11-12 19:25 ` Albert ARIBAUD 2010-11-13 16:56 ` Darius Augulis 0 siblings, 1 reply; 8+ messages in thread From: Albert ARIBAUD @ 2010-11-12 19:25 UTC (permalink / raw) To: u-boot Le 12/11/2010 19:18, Darius Augulis a ?crit : > 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@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@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. FWIW, there already is a solution based on statics and post-relocation initialization for orion5x. The principle there is that the timer is not used before calling board_init_r, so we don't need initializing timestamp before relocation. I'm not saying that's how it should be done, mind; it's just how I did it then. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-12 19:25 ` Albert ARIBAUD @ 2010-11-13 16:56 ` Darius Augulis 2010-11-13 18:13 ` Wolfgang Denk 0 siblings, 1 reply; 8+ messages in thread From: Darius Augulis @ 2010-11-13 16:56 UTC (permalink / raw) To: u-boot Dear Albert, On 11/12/2010 09:25 PM, Albert ARIBAUD wrote: > Le 12/11/2010 19:18, Darius Augulis a ?crit : >> 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@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@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. > > FWIW, there already is a solution based on statics and post-relocation > initialization for orion5x. The principle there is that the timer is not > used before calling board_init_r, so we don't need initializing > timestamp before relocation. in this case it seems like timer must be initialised after relocation. But I don't know if it could be correct for all ARM architectures. Maybe some of them use timer before relocation. Global data is good place to store important static variables because they are valid before and after relocation and it could be common for all architectures. Since we have automatic size calculation of global data structure there should not be a problem to add several additional bytes specific to every CPU. That's why I did like this and this is my opinion about this. Darius. > > I'm not saying that's how it should be done, mind; it's just how I did > it then. > > Amicalement, ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-13 16:56 ` Darius Augulis @ 2010-11-13 18:13 ` Wolfgang Denk 2010-11-13 18:32 ` Albert ARIBAUD 2010-11-22 18:30 ` Darius Augulis 0 siblings, 2 replies; 8+ messages in thread From: Wolfgang Denk @ 2010-11-13 18:13 UTC (permalink / raw) To: u-boot Dear Darius Augulis, In message <4CDEC32B.7050309@gmail.com> you wrote: > > > FWIW, there already is a solution based on statics and post-relocation > > initialization for orion5x. The principle there is that the timer is not > > used before calling board_init_r, so we don't need initializing > > timestamp before relocation. STOP!! I don't think we want this. > in this case it seems like timer must be initialised after relocation. > But I don't know if it could be correct for all ARM architectures. Maybe > some of them use timer before relocation. Indeed. Drivers and other code may want to implement timeouts and the like, and need at least basic timer services like udelay() and such. > Global data is good place to store important static variables because > they are valid before and after relocation and it could be common for > all architectures. Since we have automatic size calculation of global > data structure there should not be a problem to add several additional > bytes specific to every CPU. We should keep the gd as small as possible, but adinng one or two integers here is indeed probably the best approach. 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 Always leave room to add an explanation if it doesn't work out. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-13 18:13 ` Wolfgang Denk @ 2010-11-13 18:32 ` Albert ARIBAUD 2010-11-22 18:30 ` Darius Augulis 1 sibling, 0 replies; 8+ messages in thread From: Albert ARIBAUD @ 2010-11-13 18:32 UTC (permalink / raw) To: u-boot Le 13/11/2010 19:13, Wolfgang Denk a ?crit : > Dear Darius Augulis, > > In message<4CDEC32B.7050309@gmail.com> you wrote: >> >>> FWIW, there already is a solution based on statics and post-relocation >>> initialization for orion5x. The principle there is that the timer is not >>> used before calling board_init_r, so we don't need initializing >>> timestamp before relocation. > > STOP!! I don't think we want this. Just goes to prove a patch can get in mainline master and not get caught for being naughy, see commit d778a2fb. :) As I said before, I am ok with changing this to use a field of gd. Amicalement, -- Albert. ^ permalink raw reply [flat|nested] 8+ messages in thread
* [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation 2010-11-13 18:13 ` Wolfgang Denk 2010-11-13 18:32 ` Albert ARIBAUD @ 2010-11-22 18:30 ` Darius Augulis 1 sibling, 0 replies; 8+ messages in thread From: Darius Augulis @ 2010-11-22 18:30 UTC (permalink / raw) To: u-boot Hi Wolfgang, On 11/13/2010 08:13 PM, Wolfgang Denk wrote: > Dear Darius Augulis, > > In message<4CDEC32B.7050309@gmail.com> you wrote: >> >>> FWIW, there already is a solution based on statics and post-relocation >>> initialization for orion5x. The principle there is that the timer is not >>> used before calling board_init_r, so we don't need initializing >>> timestamp before relocation. > > STOP!! I don't think we want this. > >> in this case it seems like timer must be initialised after relocation. >> But I don't know if it could be correct for all ARM architectures. Maybe >> some of them use timer before relocation. > > Indeed. Drivers and other code may want to implement timeouts and the > like, and need at least basic timer services like udelay() and such. > >> Global data is good place to store important static variables because >> they are valid before and after relocation and it could be common for >> all architectures. Since we have automatic size calculation of global >> data structure there should not be a problem to add several additional >> bytes specific to every CPU. > > We should keep the gd as small as possible, but adinng one or two > integers here is indeed probably the best approach. could you please merge it or point me what is missing? thanks, Darius. > > > Best regards, > > Wolfgang Denk > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-11-22 18:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-01 20:49 [U-Boot] [PATCH] ARM: S3C64XX: fix timer broken by relocation Darius Augulis 2010-11-12 3:10 ` Minkyu Kang 2010-11-12 18:18 ` Darius Augulis 2010-11-12 19:25 ` Albert ARIBAUD 2010-11-13 16:56 ` Darius Augulis 2010-11-13 18:13 ` Wolfgang Denk 2010-11-13 18:32 ` Albert ARIBAUD 2010-11-22 18:30 ` Darius Augulis
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox