u-boot.lists.denx.de archive mirror
 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

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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).