All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dirk Behme <dirk.behme@googlemail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] arm925t: Fix CONFIG_SYS_HZ to 1000
Date: Mon, 20 Apr 2009 20:27:34 +0200	[thread overview]
Message-ID: <49ECBE96.1040600@googlemail.com> (raw)
In-Reply-To: <20090420190738.GA10292@localhost.localdomain>

Dear Ladis,

ah, and some remarks on the patch itself ;)

Ladislav Michl wrote:
> Let CONFIG_SYS_HZ to have value of 1000 effectively fixing all users of
> get_timer.
> 
> Signed-off-by: Ladislav Michl <ladis@linux-mips.org>
> 
> diff --git a/cpu/arm925t/interrupts.c b/cpu/arm925t/interrupts.c
> index e5c77f7..a22be66 100644
> --- a/cpu/arm925t/interrupts.c
> +++ b/cpu/arm925t/interrupts.c
...
> -#define TIMER_LOAD_VAL 0xffffffff
> +#define TIMER_LOAD_VAL	0xffffffff
> +#define TIMER_CLOCK	(CONFIG_SYS_CLK_FREQ / (2 << CONFIG_SYS_PTV))

Just to get an idea of the math:

CONFIG_SYS_CLK_FREQ is 12000000 (12MHz)? This is divided by 256, so 
TIMER_CLOCK is 46875Hz? A free running 32-bit count down timer is used 
starting@0xffffffff? Underflow (0) is reached after ~91626s == 
~25hours with this?

Please correct if something is wrong ;)

> -/* delay x useconds AND preserve advance timestamp value */
> +/* delay usec microseconds preserving timestamp value */

Hmm, 'usec microseconds' sounds somehow confusing?

>  void udelay (unsigned long usec)
>  {
...
> +	int32_t tmo = usec * (TIMER_CLOCK / 1000) / 1000;
> +	uint32_t now, last = __raw_readl(CONFIG_SYS_TIMERBASE + READ_TIM);

The first '1000' should be CONFIG_SYS_HZ? I.e.

(TIMER_CLOCK / CONFIG_SYS_HZ) / 1000;

?

In my udelay patch, I use

+	tmo = usec * (TIMER_CLOCK / CONFIG_SYS_HZ);
+	tmo /= 1000;

 From some printf debugging, for OMAP3 there was a difference doing it 
in one or two lines. If I remember correctly due to integer vs 
floating point math and rounding. What do you think?

Running OMAP3 counter with 1.625MHz, max udelay resolution is ~1.62us. 
If you run with 46875Hz, you have max udelay resolution of ~22us?

> +	while (tmo > 0) {
> +		now = __raw_readl(CONFIG_SYS_TIMERBASE + READ_TIM);
> +		if (last < now) /* count down timer underflow */
> +			tmo -= TIMER_LOAD_VAL - now + last;
> +		else
> +			tmo -= last - now;
> +		last = now;

I will think about this, I always need some time for this clock math ;)

In contrast to OMAP3 your timer here counts down, right? So while 
OMAP1 has to deal with underflow, OMAP3 will need overflow handling, 
right?

Best regards

Dirk

  parent reply	other threads:[~2009-04-20 18:27 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-20 19:07 [U-Boot] [PATCH] arm925t: Fix CONFIG_SYS_HZ to 1000 Ladislav Michl
2009-04-20 17:38 ` Dirk Behme
2009-04-20 22:32   ` Ladislav Michl
2009-04-20 18:27 ` Dirk Behme [this message]
2009-04-20 22:31   ` Ladislav Michl
2009-04-21 15:38     ` Dirk Behme
2009-04-21 23:13       ` Ladislav Michl

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=49ECBE96.1040600@googlemail.com \
    --to=dirk.behme@googlemail.com \
    --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.