All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem
@ 2014-08-12 14:25 andrew.ruder at elecsyscorp.com
  2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: andrew.ruder at elecsyscorp.com @ 2014-08-12 14:25 UTC (permalink / raw)
  To: u-boot

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>

-- 
2.0.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions
  2014-08-12 14:25 [U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem andrew.ruder at elecsyscorp.com
@ 2014-08-12 14:25 ` andrew.ruder at elecsyscorp.com
  2014-08-12 18:20   ` Marek Vasut
  2014-08-30 15:13   ` [U-Boot] [U-Boot,1/3] " Tom Rini
  2014-08-12 14:26 ` [U-Boot] [PATCH 2/3] arm: mx31: " andrew.ruder at elecsyscorp.com
  2014-08-12 14:26 ` [U-Boot] [PATCH 3/3] arm: mx35: " andrew.ruder at elecsyscorp.com
  2 siblings, 2 replies; 9+ messages in thread
From: andrew.ruder at elecsyscorp.com @ 2014-08-12 14:25 UTC (permalink / raw)
  To: u-boot

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__ */
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] arm: mx31: use common timer functions
  2014-08-12 14:25 [U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem andrew.ruder at elecsyscorp.com
  2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
@ 2014-08-12 14:26 ` andrew.ruder at elecsyscorp.com
  2014-08-12 18:21   ` Marek Vasut
  2014-08-12 14:26 ` [U-Boot] [PATCH 3/3] arm: mx35: " andrew.ruder at elecsyscorp.com
  2 siblings, 1 reply; 9+ messages in thread
From: andrew.ruder at elecsyscorp.com @ 2014-08-12 14:26 UTC (permalink / raw)
  To: u-boot

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 */
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/3] arm: mx35: use common timer functions
  2014-08-12 14:25 [U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem andrew.ruder at elecsyscorp.com
  2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
  2014-08-12 14:26 ` [U-Boot] [PATCH 2/3] arm: mx31: " andrew.ruder at elecsyscorp.com
@ 2014-08-12 14:26 ` andrew.ruder at elecsyscorp.com
  2014-09-16 10:56   ` Stefano Babic
  2 siblings, 1 reply; 9+ messages in thread
From: andrew.ruder at elecsyscorp.com @ 2014-08-12 14:26 UTC (permalink / raw)
  To: u-boot

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 */
-- 
2.0.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions
  2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
@ 2014-08-12 18:20   ` Marek Vasut
  2014-08-30 15:13   ` [U-Boot] [U-Boot,1/3] " Tom Rini
  1 sibling, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2014-08-12 18:20 UTC (permalink / raw)
  To: u-boot

On Tuesday, August 12, 2014 at 04:25:59 PM, andrew.ruder at 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 2/3] arm: mx31: use common timer functions
  2014-08-12 14:26 ` [U-Boot] [PATCH 2/3] arm: mx31: " andrew.ruder at elecsyscorp.com
@ 2014-08-12 18:21   ` Marek Vasut
  0 siblings, 0 replies; 9+ messages in thread
From: Marek Vasut @ 2014-08-12 18:21 UTC (permalink / raw)
  To: u-boot

On Tuesday, August 12, 2014 at 04:26:00 PM, andrew.ruder at 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 */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [U-Boot,1/3] arm: pxa: use common timer functions
  2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
  2014-08-12 18:20   ` Marek Vasut
@ 2014-08-30 15:13   ` Tom Rini
  2014-09-16 14:44     ` Andrew Ruder
  1 sibling, 1 reply; 9+ messages in thread
From: Tom Rini @ 2014-08-30 15:13 UTC (permalink / raw)
  To: u-boot

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140830/7d7877e9/attachment.pgp>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [PATCH 3/3] arm: mx35: use common timer functions
  2014-08-12 14:26 ` [U-Boot] [PATCH 3/3] arm: mx35: " andrew.ruder at elecsyscorp.com
@ 2014-09-16 10:56   ` Stefano Babic
  0 siblings, 0 replies; 9+ messages in thread
From: Stefano Babic @ 2014-09-16 10:56 UTC (permalink / raw)
  To: u-boot

Hi Andrew,


On 12/08/2014 16:26, andrew.ruder at 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


-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [U-Boot] [U-Boot,1/3] arm: pxa: use common timer functions
  2014-08-30 15:13   ` [U-Boot] [U-Boot,1/3] " Tom Rini
@ 2014-09-16 14:44     ` Andrew Ruder
  0 siblings, 0 replies; 9+ messages in thread
From: Andrew Ruder @ 2014-09-16 14:44 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-09-16 14:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-12 14:25 [U-Boot] [PATCH 0/3] Initial cleanup of arm port timer subsystem andrew.ruder at elecsyscorp.com
2014-08-12 14:25 ` [U-Boot] [PATCH 1/3] arm: pxa: use common timer functions andrew.ruder at elecsyscorp.com
2014-08-12 18:20   ` Marek Vasut
2014-08-30 15:13   ` [U-Boot] [U-Boot,1/3] " Tom Rini
2014-09-16 14:44     ` Andrew Ruder
2014-08-12 14:26 ` [U-Boot] [PATCH 2/3] arm: mx31: " andrew.ruder at elecsyscorp.com
2014-08-12 18:21   ` Marek Vasut
2014-08-12 14:26 ` [U-Boot] [PATCH 3/3] arm: mx35: " andrew.ruder at elecsyscorp.com
2014-09-16 10:56   ` Stefano Babic

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.