All of lore.kernel.org
 help / color / mirror / Atom feed
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

  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 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.