From: Lee Jones <lee.jones@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over
Date: Wed, 21 Nov 2012 10:02:28 +0000 [thread overview]
Message-ID: <20121121100228.GJ28265@gmail.com> (raw)
In-Reply-To: <20121120181453.568D620009C@gemini.denx.de>
> > If we attempt to take a 32bit timer value and multiply it by a
> > significant number, the core will not be able to handle it. This
> > gives the illusion that the timer is rolling over, when in fact
> > this is not the case. If we ensure the division in this instance
> > is carried out before the multiplication, the issue vanishes.
>
> Are you sure this is a good idea?
Not now I'm not. :)
> > --- a/arch/arm/cpu/armv7/u8500/timer.c
> > +++ b/arch/arm/cpu/armv7/u8500/timer.c
> > @@ -70,7 +70,7 @@ struct u8500_mtu {
> > * The MTU is clocked at 133 MHz by default. (V1 and later)
> > */
> > #define TIMER_CLOCK (133 * 1000 * 1000 / 16)
> > -#define COUNT_TO_USEC(x) ((x) * 16 / 133)
> > +#define COUNT_TO_USEC(x) ((x) / 133 * 16)
>
> Before the change, the result is useful for all values of x in the
> interval from 0 through UINT_MAX/16 = 268435455 = 268 seconds, i. e.
> for all values that make sense to be measured in microseconds.
We're actually discussing this elsewhere. I don't have
permission to paste the other side of the conversation, but
I can show you my calculations:
> The original implementation is fine until we reach 32.28 seconds, which
> as you predicted is 0x1000_0000:
> 0x10000000 * PRESCALER) / (CLOCK_SPEED_133_MHZ)
> (268435456 * 16 ) / (133 * 1000 * 1000) == 32.28 seconds
> If we spend >30 seconds at the u-boot commandline, which is not
> unreasonable by any stretch, then the kernel assumes responsibility for
> the remaining of the time spent at the prompt, which is obviously not
> acceptable for this usecase.
> I doubt x will never be < 133, as that will be 16 micro-seconds
> post timer-start or role-over. Do we really need that degree of
> resolution?
Am assuming by your response that I'm wrong somewhere, and the
resolution loss will be >16us?
> After the change, the result is changed to the worse for all low
> values of X, especially for the ranges from 16 through 132.
>
> You lose accuracy here, and win nothing.
>
> This makes no sense to me.
>
> If you need a bigger number range, then use proper arithmetics. It';s
> available, just use it.
Would you be able to lend a hand here, as I'm no mathematician?
What are the correct arithmetics?
Kind regards,
Lee
--
Lee Jones
Linaro ST-Ericsson Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
next prev parent reply other threads:[~2012-11-21 10:02 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-11-20 14:33 [U-Boot] [PATCH 0/8] Adding boottime support Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 1/8] u8500: Correct unnecessary mathematical roll-over Lee Jones
2012-11-20 18:14 ` Wolfgang Denk
2012-11-21 10:02 ` Lee Jones [this message]
2012-11-21 13:51 ` Wolfgang Denk
2012-11-21 14:54 ` Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 2/8] u8500: Add utimer support Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 3/8] boottime: Add core boottime measurement support Lee Jones
2012-11-20 18:20 ` Wolfgang Denk
2012-11-21 9:50 ` Lee Jones
2012-11-21 13:44 ` Wolfgang Denk
2012-11-21 15:03 ` Lee Jones
2012-11-21 16:14 ` Wolfgang Denk
2012-11-21 17:26 ` Lee Jones
2012-11-21 19:04 ` Wolfgang Denk
2012-11-26 6:08 ` Simon Glass
2012-11-26 9:00 ` Lee Jones
2012-11-26 19:57 ` Simon Glass
2012-11-27 8:55 ` Lee Jones
2012-11-27 13:46 ` Wolfgang Denk
2012-11-27 14:28 ` Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 4/8] boottime: Apply some key boottime tags into common code Lee Jones
2012-11-20 18:22 ` Wolfgang Denk
2012-11-21 9:36 ` Lee Jones
2012-11-21 13:40 ` Wolfgang Denk
2012-11-21 15:07 ` Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 5/8] arm: Add boottime support for the ARM architecture Lee Jones
2012-11-20 15:11 ` Otavio Salvador
2012-11-20 15:52 ` Lee Jones
2012-11-20 18:24 ` Wolfgang Denk
2012-11-21 9:17 ` Lee Jones
2012-11-21 9:30 ` Wolfgang Denk
2012-11-21 10:13 ` Lee Jones
2012-11-21 13:58 ` Wolfgang Denk
2012-11-21 14:39 ` Lee Jones
2012-11-21 16:05 ` Wolfgang Denk
2012-11-21 17:48 ` Lee Jones
2012-11-21 19:18 ` Wolfgang Denk
2012-11-22 10:14 ` Lee Jones
2012-11-22 13:04 ` Wolfgang Denk
2012-11-22 16:08 ` Lee Jones
2012-11-22 17:40 ` Wolfgang Denk
2012-11-23 10:08 ` Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 6/8] arm: Add some boottime tags into prime booting locations Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 7/8] href: Enable boottime functionality Lee Jones
2012-11-20 14:33 ` [U-Boot] [PATCH 8/8] snowball: " Lee Jones
2012-11-20 18:08 ` [U-Boot] [PATCH 0/8] Adding boottime support Wolfgang Denk
2012-11-21 10:03 ` Lee Jones
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121121100228.GJ28265@gmail.com \
--to=lee.jones@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox