[U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem
From: Andrew Ruder andrew.ruder@elecsyscorp.com
Hey all,
Was tracking down an issue on a PXA270 based board where a long y-modem transfer would frequently deadlock the board in the middle of the transfer. Unfortunately I felt like I had opened pandora's box by looking around at other ports.
Very few ports were 32-bit rollover safe! Most looked like they would just skip a __udelay on roll-over. These are fixes for the couple I noticed that would potentially deadlock if __udelay crossed the 32-bit boundary. And some are so complicated that I didn't waste time looking at them.
get_ticks() which returns a unsigned long long had a variety of implementations. Some returned a number that wrapped at 32 bits which worked with some implementations of __udelay. Others returned a number that wraps at a fraction of 32-bits which would hit the roll-over issues at an unexpected place and fail even more often.
There is a lot of opportunity here for tons of code removal by moving more ports to the common code. Here are some notes that I took while going through some that might save someone some time. This was a very cursory look so my apologies if I have missed something.
** u-boot/arch/arm/cpu/arm1136/mx31/timer.c:98:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay() depends on 64-bit value - HANGS ** u-boot/arch/arm/cpu/arm1136/mx35/timer.c:75:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay() depends on 64-bit value - HANGS ** u-boot/arch/arm/cpu/arm1176/bcm2835/timer.c:37:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay() is safe ** u-boot/arch/arm/cpu/arm1176/tnetv107x/timer.c:67:20:unsigned long long get_ticks(void) get_ticks() isn't even a 32-bit number __udelay() is safe ** u-boot/arch/arm/cpu/arm920t/a320/timer.c:74:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay() is safe ** u-boot/arch/arm/cpu/arm920t/at91/timer.c:115:20:unsigned long long get_ticks(void) get_ticks() isn't even a 32-bit number __udelay() is safe ** u-boot/arch/arm/cpu/arm920t/ep93xx/timer.c:58:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay() is safe until 64-bit rollover ** u-boot/arch/arm/cpu/arm920t/imx/timer.c:70:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay is safe ** u-boot/arch/arm/cpu/arm920t/s3c24x0/timer.c:110:20:unsigned long long get_ticks(void) get_ticks() is complicated. tbu abused for something else __udelay might be safe? It handles 32-bit rollover correctly if get_ticks() is really a 32-bit number ** u-boot/arch/arm/cpu/arm926ejs/armada100/timer.c:182:20:unsigned long long get_ticks(void) get_ticks() isn't a 32-bit number __udelay completely doesn't handle roll-over ** u-boot/arch/arm/cpu/arm926ejs/at91/timer.c:75:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay handles roll-over looks perfect ** u-boot/arch/arm/cpu/arm926ejs/davinci/timer.c:56:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number chooses to ignore roll-over ** u-boot/arch/arm/cpu/arm926ejs/kirkwood/timer.c:145:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number no idea, too complicated ** u-boot/arch/arm/cpu/arm926ejs/mb86r0x/timer.c:65:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay doesn't handle roll-over ** u-boot/arch/arm/cpu/arm926ejs/lpc32xx/timer.c:74:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay is safe (doesn't handle roll-over - doesn't need to) ** u-boot/arch/arm/cpu/arm926ejs/mx27/timer.c:111:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay broken ** u-boot/arch/arm/cpu/arm926ejs/mxs/timer.c:81:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay complicated ** u-boot/arch/arm/cpu/arm926ejs/nomadik/timer.c:63:20:unsigned long long get_ticks(void) get_ticks() isn't a 32-bit number __udelay complicated ** u-boot/arch/arm/cpu/arm926ejs/omap/timer.c:140:20:unsigned long long get_ticks(void) get_ticks() isn't a 32-bit number __udelay looks good ** u-boot/arch/arm/cpu/arm926ejs/orion5x/timer.c:159:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay complicated ** u-boot/arch/arm/cpu/arm926ejs/pantheon/timer.c:189:20:unsigned long long get_ticks(void) get_ticks() doesn't return a 32-bit number __udelay doesn't handle roll-over ** u-boot/arch/arm/cpu/arm926ejs/spear/timer.c:111:20:unsigned long long get_ticks(void) get_ticks() doesn't return a 32-bit number __udelay looks good ** u-boot/arch/arm/cpu/armv7/arch_timer.c:24:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay doesn't handle 64-bit roll-over ** u-boot/arch/arm/cpu/armv7/at91/timer.c:78:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay looks good looks perfect ** u-boot/arch/arm/cpu/armv7/omap-common/timer.c:96:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay complicated but looks good ** u-boot/arch/arm/cpu/armv7/rmobile/timer.c:77:20:unsigned long long get_ticks(void) get_ticks() returns a 64-bit number __udelay looks good ** u-boot/arch/arm/cpu/armv7/s5p-common/timer.c:120:20:unsigned long long get_ticks(void) get_ticks() doesn't return a 32-bit number __udelay looks good ** u-boot/arch/arm/cpu/armv7/sunxi/timer.c:101:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay off by 1 on roll-over ** u-boot/arch/arm/cpu/armv7/vf610/timer.c:49:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay looks good ** u-boot/arch/arm/cpu/armv7/u8500/timer.c:123:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay off by 1 on rollover ** u-boot/arch/arm/cpu/armv7/zynq/timer.c:154:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay complicated ** u-boot/arch/arm/cpu/pxa/timer.c:45:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay depends on 64-bit - HANGS ** u-boot/arch/arm/cpu/sa1100/timer.c:58:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay looks good ** u-boot/arch/arm/imx-common/timer.c:74:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay doesn't handle 64-bit roll-over ** u-boot/arch/avr32/cpu/interrupts.c:36:20:unsigned long long get_ticks(void) get_ticks() returns 64-bit number __udelay() correctly handles overflow ** u-boot/arch/blackfin/cpu/interrupts.c:39:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay() looks ok i think (pretty complicated) ** u-boot/arch/m68k/lib/time.c:179:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay() looks ok ** u-boot/arch/microblaze/cpu/timer.c:80:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay() looks ok. ** u-boot/arch/mips/cpu/mips32/time.c:58:20:unsigned long long get_ticks(void) get_ticks() returns a 32-bit number __udelay() complicated ** u-boot/arch/mips/cpu/mips64/time.c:58:20:unsigned long long get_ticks(void) get_ticks() might return 64-bit __udelay() complicated ** u-boot/arch/nds32/cpu/n1213/ag101/timer.c:173:20:unsigned long long get_ticks(void) get_ticks() returns 32-bit number __udelay() off-by-1 on roll-over ** u-boot/arch/nds32/cpu/n1213/ag102/timer.c:173:20:unsigned long long get_ticks(void) see above - same code
Andrew Ruder (3): arm: pxa: use common timer functions arm: mx31: use common timer functions arm: mx35: use common timer functions
arch/arm/cpu/arm1136/mx31/timer.c | 104 +----------------------------- arch/arm/cpu/arm1136/mx35/timer.c | 83 ------------------------ arch/arm/cpu/pxa/timer.c | 69 +------------------- arch/arm/include/asm/arch-mx31/imx-regs.h | 10 +++ arch/arm/include/asm/arch-mx35/imx-regs.h | 12 ++++ include/configs/pxa-common.h | 13 ++++ 6 files changed, 38 insertions(+), 253 deletions(-)
Cc: Marek Vasut marex@denx.de
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves pxa to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) pxa timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locked the processor. Rather than patch the specific pxa issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de ---
32-bit rollover occurs every 22 minutes so even a long y-modem transfer was enough to hit this issue fairly regularly. This has been tested.
arch/arm/cpu/pxa/timer.c | 69 +------------------------------------------- include/configs/pxa-common.h | 13 +++++++++ 2 files changed, 14 insertions(+), 68 deletions(-)
diff --git a/arch/arm/cpu/pxa/timer.c b/arch/arm/cpu/pxa/timer.c index c4717de..11fefd5 100644 --- a/arch/arm/cpu/pxa/timer.c +++ b/arch/arm/cpu/pxa/timer.c @@ -6,80 +6,13 @@ * SPDX-License-Identifier: GPL-2.0+ */
-#include <asm/arch/pxa-regs.h> #include <asm/io.h> #include <common.h> -#include <div64.h>
DECLARE_GLOBAL_DATA_PTR;
-#define TIMER_LOAD_VAL 0xffffffff - -#define timestamp (gd->arch.tbl) -#define lastinc (gd->arch.lastinc) - -#if defined(CONFIG_CPU_PXA27X) || defined(CONFIG_CPU_MONAHANS) -#define TIMER_FREQ_HZ 3250000 -#elif defined(CONFIG_CPU_PXA25X) -#define TIMER_FREQ_HZ 3686400 -#else -#error "Timer frequency unknown - please config PXA CPU type" -#endif - -static unsigned long long tick_to_time(unsigned long long tick) -{ - return lldiv(tick * CONFIG_SYS_HZ, TIMER_FREQ_HZ); -} - -static unsigned long long us_to_tick(unsigned long long us) -{ - return lldiv(us * TIMER_FREQ_HZ, 1000000); -} - int timer_init(void) { - writel(0, OSCR); + writel(0, CONFIG_SYS_TIMER_COUNTER); return 0; } - -unsigned long long get_ticks(void) -{ - /* Current tick value */ - uint32_t now = readl(OSCR); - - if (now >= lastinc) { - /* - * Normal mode (non roll) - * Move stamp forward with absolute diff ticks - */ - timestamp += (now - lastinc); - } else { - /* We have rollover of incrementer */ - timestamp += (TIMER_LOAD_VAL - lastinc) + now; - } - - lastinc = now; - return timestamp; -} - -ulong get_timer(ulong base) -{ - return tick_to_time(get_ticks()) - base; -} - -void __udelay(unsigned long usec) -{ - unsigned long long tmp; - ulong tmo; - - tmo = us_to_tick(usec); - tmp = get_ticks() + tmo; /* get current timestamp */ - - while (get_ticks() < tmp) /* loop till event */ - /*NOP*/; -} - -ulong get_tbclk(void) -{ - return TIMER_FREQ_HZ; -} diff --git a/include/configs/pxa-common.h b/include/configs/pxa-common.h index f0ecc34..8da37a3 100644 --- a/include/configs/pxa-common.h +++ b/include/configs/pxa-common.h @@ -43,4 +43,17 @@ #define CONFIG_USB_STORAGE #endif
+/* + * Generic timer support + */ +#if defined(CONFIG_CPU_PXA27X) || defined(CONFIG_CPU_MONAHANS) +#define CONFIG_SYS_TIMER_RATE 3250000 +#elif defined(CONFIG_CPU_PXA25X) +#define CONFIG_SYS_TIMER_RATE 3686400 +#else +#error "Timer frequency unknown - please config PXA CPU type" +#endif + +#define CONFIG_SYS_TIMER_COUNTER 0x40A00010 /* OSCR */ + #endif /* __CONFIG_PXA_COMMON_H__ */
On Tuesday, August 12, 2014 at 04:25:59 PM, andrew.ruder@elecsyscorp.com wrote:
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves pxa to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) pxa timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locked the processor. Rather than patch the specific pxa issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de
32-bit rollover occurs every 22 minutes so even a long y-modem transfer was enough to hit this issue fairly regularly. This has been tested.
arch/arm/cpu/pxa/timer.c | 69 +------------------------------------------- include/configs/pxa-common.h | 13 +++++++++ 2 files changed, 14 insertions(+), 68 deletions(-)
Acked-by: Marek Vasut marex@denx.de
+CC Albert.
Albert , can you please pick this one up ?
Best regards, Marek Vasut
On Tue, Aug 12, 2014 at 09:25:59AM -0500, Andrew Ruder wrote:
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves pxa to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) pxa timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locked the processor. Rather than patch the specific pxa issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de Acked-by: Marek Vasut marex@denx.de
As it stands today this breaks building a number of PXA boards so it needs to be picked up and re-worked / build tested, thanks!
On 08/30/2014 10:13 AM, Tom Rini wrote:
As it stands today this breaks building a number of PXA boards so it needs to be picked up and re-worked / build tested, thanks!
I'm still planning on getting back to this and taking a look. At the time I was on a slightly older version of U-Boot (2014.07-rc3 I think) and hadn't yet figured out what was going on with all of the Kconfig changes so mostly just made sure it applied cleanly on HEAD. My apologies!
- Andy
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves mx31 to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) mx31 timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locks the processor. Rather than patch the specific mx31 issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de Cc: Linus Walleij linus.walleij@linaro.org Cc: Wolfgang Denk wd@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Helmut Raiger helmut.raiger@hale.at ---
This patch has been COMPILE tested only. The situation isn't quite as bad on mx31 as 32-bit rollover occurs in about a day and a half.
Before patch: Configuring for qong board... 456 -rw-r--r-- 1 andy andy 463236 Aug 12 08:36 u-boot.bin
After patch: Configuring for qong board... 456 -rw-r--r-- 1 andy andy 463248 Aug 12 08:38 u-boot.bin
arch/arm/cpu/arm1136/mx31/timer.c | 104 +----------------------------- arch/arm/include/asm/arch-mx31/imx-regs.h | 10 +++ 2 files changed, 12 insertions(+), 102 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx31/timer.c b/arch/arm/cpu/arm1136/mx31/timer.c index f111242..3a81ce4 100644 --- a/arch/arm/cpu/arm1136/mx31/timer.c +++ b/arch/arm/cpu/arm1136/mx31/timer.c @@ -7,9 +7,6 @@
#include <common.h> #include <asm/arch/imx-regs.h> -#include <asm/arch/clock.h> -#include <div64.h> -#include <watchdog.h> #include <asm/io.h>
#define TIMER_BASE 0x53f90000 /* General purpose timer 1 */ @@ -28,57 +25,6 @@
DECLARE_GLOBAL_DATA_PTR;
-/* - * "time" is measured in 1 / CONFIG_SYS_HZ seconds, - * "tick" is internal timer period - */ - -#ifdef CONFIG_MX31_TIMER_HIGH_PRECISION -/* ~0.4% error - measured with stop-watch on 100s boot-delay */ -static inline unsigned long long tick_to_time(unsigned long long tick) -{ - tick *= CONFIG_SYS_HZ; - do_div(tick, MXC_CLK32); - return tick; -} - -static inline unsigned long long time_to_tick(unsigned long long time) -{ - time *= MXC_CLK32; - do_div(time, CONFIG_SYS_HZ); - return time; -} - -static inline unsigned long long us_to_tick(unsigned long long us) -{ - us = us * MXC_CLK32 + 999999; - do_div(us, 1000000); - return us; -} -#else -/* ~2% error */ -#define TICK_PER_TIME ((MXC_CLK32 + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ) -#define US_PER_TICK (1000000 / MXC_CLK32) - -static inline unsigned long long tick_to_time(unsigned long long tick) -{ - do_div(tick, TICK_PER_TIME); - return tick; -} - -static inline unsigned long long time_to_tick(unsigned long long time) -{ - return time * TICK_PER_TIME; -} - -static inline unsigned long long us_to_tick(unsigned long long us) -{ - us += US_PER_TICK - 1; - do_div(us, US_PER_TICK); - return us; -} -#endif - /* The 32768Hz 32-bit timer overruns in 131072 seconds */ int timer_init(void) { @@ -95,53 +41,7 @@ int timer_init(void) return 0; }
-unsigned long long get_ticks(void) -{ - ulong now = GPTCNT; /* current tick value */ - - if (now >= gd->arch.lastinc) /* normal mode (non roll) */ - /* move stamp forward with absolut diff ticks */ - gd->arch.tbl += (now - gd->arch.lastinc); - else /* we have rollover of incrementer */ - gd->arch.tbl += (0xFFFFFFFF - gd->arch.lastinc) + now; - gd->arch.lastinc = now; - return gd->arch.tbl; -} - -ulong get_timer_masked(void) -{ - /* - * get_ticks() returns a long long (64 bit), it wraps in - * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~ - * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in - * 5 * 10^6 days - long enough. - */ - return tick_to_time(get_ticks()); -} - -ulong get_timer(ulong base) -{ - return get_timer_masked() - base; -} - -/* delay x useconds AND preserve advance timestamp value */ -void __udelay(unsigned long usec) -{ - unsigned long long tmp; - ulong tmo; - - tmo = us_to_tick(usec); - tmp = get_ticks() + tmo; /* get current timestamp */ - - while (get_ticks() < tmp) /* loop till event */ - /*NOP*/; -} - -/* - * This function is derived from PowerPC code (timebase clock frequency). - * On ARM it returns the number of timer ticks per second. - */ -ulong get_tbclk(void) +unsigned long timer_read_counter(void) { - return MXC_CLK32; + return GPTCNT; } diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h index f23350e..71ebd24 100644 --- a/arch/arm/include/asm/arch-mx31/imx-regs.h +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h @@ -909,9 +909,19 @@ struct esdc_regs { #define MXC_CSPIPERIOD_32KHZ (1 << 15) #define MAX_SPI_BYTES 4
+ #define MXC_SPI_BASE_ADDRESSES \ 0x43fa4000, \ 0x50010000, \ 0x53f84000,
+/* + * Generic timer support + */ +#ifdef CONFIG_MX31_CLK32 +#define CONFIG_SYS_TIMER_RATE CONFIG_MX31_CLK32 +#else +#define CONFIG_SYS_TIMER_RATE 32768 +#endif + #endif /* __ASM_ARCH_MX31_IMX_REGS_H */
On Tuesday, August 12, 2014 at 04:26:00 PM, andrew.ruder@elecsyscorp.com wrote:
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves mx31 to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) mx31 timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locks the processor. Rather than patch the specific mx31 issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de Cc: Linus Walleij linus.walleij@linaro.org Cc: Wolfgang Denk wd@denx.de Cc: Fabio Estevam fabio.estevam@freescale.com Cc: Helmut Raiger helmut.raiger@hale.at
+CC Stefano
This patch has been COMPILE tested only. The situation isn't quite as bad on mx31 as 32-bit rollover occurs in about a day and a half.
Before patch: Configuring for qong board... 456 -rw-r--r-- 1 andy andy 463236 Aug 12 08:36 u-boot.bin
After patch: Configuring for qong board... 456 -rw-r--r-- 1 andy andy 463248 Aug 12 08:38 u-boot.bin
arch/arm/cpu/arm1136/mx31/timer.c | 104 +----------------------------- arch/arm/include/asm/arch-mx31/imx-regs.h | 10 +++ 2 files changed, 12 insertions(+), 102 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx31/timer.c b/arch/arm/cpu/arm1136/mx31/timer.c index f111242..3a81ce4 100644 --- a/arch/arm/cpu/arm1136/mx31/timer.c +++ b/arch/arm/cpu/arm1136/mx31/timer.c @@ -7,9 +7,6 @@
#include <common.h> #include <asm/arch/imx-regs.h> -#include <asm/arch/clock.h> -#include <div64.h> -#include <watchdog.h> #include <asm/io.h>
#define TIMER_BASE 0x53f90000 /* General purpose timer 1 */ @@ -28,57 +25,6 @@
DECLARE_GLOBAL_DATA_PTR;
-/*
- "time" is measured in 1 / CONFIG_SYS_HZ seconds,
- "tick" is internal timer period
- */
-#ifdef CONFIG_MX31_TIMER_HIGH_PRECISION -/* ~0.4% error - measured with stop-watch on 100s boot-delay */ -static inline unsigned long long tick_to_time(unsigned long long tick) -{
- tick *= CONFIG_SYS_HZ;
- do_div(tick, MXC_CLK32);
- return tick;
-}
-static inline unsigned long long time_to_tick(unsigned long long time) -{
- time *= MXC_CLK32;
- do_div(time, CONFIG_SYS_HZ);
- return time;
-}
-static inline unsigned long long us_to_tick(unsigned long long us) -{
- us = us * MXC_CLK32 + 999999;
- do_div(us, 1000000);
- return us;
-} -#else -/* ~2% error */ -#define TICK_PER_TIME ((MXC_CLK32 + CONFIG_SYS_HZ / 2) / CONFIG_SYS_HZ) -#define US_PER_TICK (1000000 / MXC_CLK32)
-static inline unsigned long long tick_to_time(unsigned long long tick) -{
- do_div(tick, TICK_PER_TIME);
- return tick;
-}
-static inline unsigned long long time_to_tick(unsigned long long time) -{
- return time * TICK_PER_TIME;
-}
-static inline unsigned long long us_to_tick(unsigned long long us) -{
- us += US_PER_TICK - 1;
- do_div(us, US_PER_TICK);
- return us;
-} -#endif
/* The 32768Hz 32-bit timer overruns in 131072 seconds */ int timer_init(void) { @@ -95,53 +41,7 @@ int timer_init(void) return 0; }
-unsigned long long get_ticks(void) -{
- ulong now = GPTCNT; /* current tick value */
- if (now >= gd->arch.lastinc) /* normal mode (non roll) */
/* move stamp forward with absolut diff ticks */gd->arch.tbl += (now - gd->arch.lastinc);- else /* we have rollover of incrementer */
gd->arch.tbl += (0xFFFFFFFF - gd->arch.lastinc) + now;- gd->arch.lastinc = now;
- return gd->arch.tbl;
-}
-ulong get_timer_masked(void) -{
- /*
* get_ticks() returns a long long (64 bit), it wraps in* 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~* 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in* 5 * 10^6 days - long enough.*/- return tick_to_time(get_ticks());
-}
-ulong get_timer(ulong base) -{
- return get_timer_masked() - base;
-}
-/* delay x useconds AND preserve advance timestamp value */ -void __udelay(unsigned long usec) -{
- unsigned long long tmp;
- ulong tmo;
- tmo = us_to_tick(usec);
- tmp = get_ticks() + tmo; /* get current timestamp */
- while (get_ticks() < tmp) /* loop till event */
/*NOP*/;-}
-/*
- This function is derived from PowerPC code (timebase clock frequency).
- On ARM it returns the number of timer ticks per second.
- */
-ulong get_tbclk(void) +unsigned long timer_read_counter(void) {
- return MXC_CLK32;
- return GPTCNT;
} diff --git a/arch/arm/include/asm/arch-mx31/imx-regs.h b/arch/arm/include/asm/arch-mx31/imx-regs.h index f23350e..71ebd24 100644 --- a/arch/arm/include/asm/arch-mx31/imx-regs.h +++ b/arch/arm/include/asm/arch-mx31/imx-regs.h @@ -909,9 +909,19 @@ struct esdc_regs { #define MXC_CSPIPERIOD_32KHZ (1 << 15) #define MAX_SPI_BYTES 4
#define MXC_SPI_BASE_ADDRESSES \ 0x43fa4000, \ 0x50010000, \ 0x53f84000,
+/*
- Generic timer support
- */
+#ifdef CONFIG_MX31_CLK32 +#define CONFIG_SYS_TIMER_RATE CONFIG_MX31_CLK32 +#else +#define CONFIG_SYS_TIMER_RATE 32768 +#endif
#endif /* __ASM_ARCH_MX31_IMX_REGS_H */
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves mx35 to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) mx35 timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locks the processor. Rather than patch the specific mx35 issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de Cc: Stefano Babic sbabic@denx.de ---
This patch has been COMPILE tested only. The situation isn't quite as bad on mx35 as 32-bit rollover occurs in about a day and a half.
Before patch: Configuring for woodburn board... 308 -rw-r--r-- 1 andy andy 314232 Aug 12 08:36 u-boot.bin
After patch: Configuring for woodburn board... 308 -rw-r--r-- 1 andy andy 314208 Aug 12 08:37 u-boot.bin
arch/arm/cpu/arm1136/mx35/timer.c | 83 ------------------------------- arch/arm/include/asm/arch-mx35/imx-regs.h | 12 +++++ 2 files changed, 12 insertions(+), 83 deletions(-)
diff --git a/arch/arm/cpu/arm1136/mx35/timer.c b/arch/arm/cpu/arm1136/mx35/timer.c index cc6166f..4edf533 100644 --- a/arch/arm/cpu/arm1136/mx35/timer.c +++ b/arch/arm/cpu/arm1136/mx35/timer.c @@ -9,16 +9,11 @@
#include <common.h> #include <asm/io.h> -#include <div64.h> #include <asm/arch/imx-regs.h> #include <asm/arch/crm_regs.h> -#include <asm/arch/clock.h>
DECLARE_GLOBAL_DATA_PTR;
-#define timestamp (gd->arch.tbl) -#define lastinc (gd->arch.lastinc) - /* General purpose timers bitfields */ #define GPTCR_SWR (1<<15) /* Software reset */ #define GPTCR_FRR (1<<9) /* Freerun / restart */ @@ -26,27 +21,6 @@ DECLARE_GLOBAL_DATA_PTR; #define GPTCR_TEN (1) /* Timer enable */
/* - * "time" is measured in 1 / CONFIG_SYS_HZ seconds, - * "tick" is internal timer period - */ -/* ~0.4% error - measured with stop-watch on 100s boot-delay */ -static inline unsigned long long tick_to_time(unsigned long long tick) -{ - tick *= CONFIG_SYS_HZ; - do_div(tick, MXC_CLK32); - - return tick; -} - -static inline unsigned long long us_to_tick(unsigned long long us) -{ - us = us * MXC_CLK32 + 999999; - do_div(us, 1000000); - - return us; -} - -/* * nothing really to do with interrupts, just starts up a counter. * The 32KHz 32-bit timer overruns in 134217 seconds */ @@ -71,60 +45,3 @@ int timer_init(void)
return 0; } - -unsigned long long get_ticks(void) -{ - struct gpt_regs *gpt = (struct gpt_regs *)GPT1_BASE_ADDR; - ulong now = readl(&gpt->counter); /* current tick value */ - - if (now >= lastinc) { - /* - * normal mode (non roll) - * move stamp forward with absolut diff ticks - */ - timestamp += (now - lastinc); - } else { - /* we have rollover of incrementer */ - timestamp += (0xFFFFFFFF - lastinc) + now; - } - lastinc = now; - return timestamp; -} - -ulong get_timer_masked(void) -{ - /* - * get_ticks() returns a long long (64 bit), it wraps in - * 2^64 / MXC_CLK32 = 2^64 / 2^15 = 2^49 ~ 5 * 10^14 (s) ~ - * 5 * 10^9 days... and get_ticks() * CONFIG_SYS_HZ wraps in - * 5 * 10^6 days - long enough. - */ - return tick_to_time(get_ticks()); -} - -ulong get_timer(ulong base) -{ - return get_timer_masked() - base; -} - -/* delay x useconds AND preserve advance timstamp value */ -void __udelay(unsigned long usec) -{ - unsigned long long tmp; - ulong tmo; - - tmo = us_to_tick(usec); - tmp = get_ticks() + tmo; /* get current timestamp */ - - while (get_ticks() < tmp) /* loop till event */ - /*NOP*/; -} - -/* - * This function is derived from PowerPC code (timebase clock frequency). - * On ARM it returns the number of timer ticks per second. - */ -ulong get_tbclk(void) -{ - return MXC_CLK32; -} diff --git a/arch/arm/include/asm/arch-mx35/imx-regs.h b/arch/arm/include/asm/arch-mx35/imx-regs.h index b530029..28a47ed 100644 --- a/arch/arm/include/asm/arch-mx35/imx-regs.h +++ b/arch/arm/include/asm/arch-mx35/imx-regs.h @@ -372,4 +372,16 @@ struct aips_regs { #define CCM_RCSR_NF_16BIT_SEL (1 << 14)
#endif + +/* + * Generic timer support + */ +#ifdef CONFIG_MX35_CLK32 +#define CONFIG_SYS_TIMER_RATE CONFIG_MX35_CLK32 +#else +#define CONFIG_SYS_TIMER_RATE 32768 +#endif + +#define CONFIG_SYS_TIMER_COUNTER (GPT1_BASE_ADDR+36) + #endif /* __ASM_ARCH_MX35_H */
Hi Andrew,
On 12/08/2014 16:26, andrew.ruder@elecsyscorp.com wrote:
From: Andrew Ruder andrew.ruder@elecsyscorp.com
This patch moves mx35 to the common timer functions added in commit
8dfafdd - Introduce common timer functions <Rob Herring>
The (removed) mx35 timer code (specifically __udelay()) could deadlock at the 32-bit boundary of get_ticks(). get_ticks() returned a 32-bit value cast up to a 64-bit value. If get_ticks() + tmo in __udelay() crossed the 32-bit boundary, the while condition became unconditionally true and locks the processor. Rather than patch the specific mx35 issues, simply move everything over to the common code.
Signed-off-by: Andrew Ruder andrew.ruder@elecsyscorp.com Cc: Marek Vasut marex@denx.de Cc: Stefano Babic sbabic@denx.de
This patch has been COMPILE tested only. The situation isn't quite as bad on mx35 as 32-bit rollover occurs in about a day and a half.
I have applied both patches for MX31 and MX35, even if I had no time to make myself a test. I hope in this way there is more testers before release.
Applied to u-boot-imx, thanks !
Best regards, Stefano Babic
participants (5)
-
Andrew Ruder -
andrew.ruder＠elecsyscorp.com
-
Marek Vasut -
Stefano Babic -
Tom Rini