From: Jens Scharsig <js_at_ng@scharsoft.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk
Date: Sun, 08 Aug 2010 12:52:15 +0200 [thread overview]
Message-ID: <4C5E8C5F.6000001@scharsoft.de> (raw)
In-Reply-To: <4C5DA0FD.5020709@emk-elektronik.de>
Dear Reinhard
> I think the timer.c is more broken that just that return value:
>
You are right, but the patch fixes only the single small problem.
> example 1:
> /*
> * timer without interrupts
> */
> unsigned long long get_ticks(void)
> {
> at91_pit_t *pit = (at91_pit_t *) AT91_PIT_BASE;
>
> ulong now = readl(&pit->piir);
>
> 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;
> }
>
> observation: timestamp, lastinc, now are all ulong.
> timestamp += (0xFFFFFFFF - lastinc) + now;
> is the same as
> timestamp += (now - lastinc) - 1;
> so in case of a "rollover" timestamp is incremented just by one less.
> I think the if is superfluous. ulong will handle the rollover automatically
But this is only true, if we are using ulong variables. I think the idea behind
this code is use unsigned long long for variables. (especially timestamp)
>
> example 2:
> void __udelay(unsigned long usec)
> {
> unsigned long long tmp;
> ulong tmo;
>
> tmo = usec_to_tick(usec);
> tmp = get_ticks() + tmo; /* get current timestamp */
>
> while (get_ticks() < tmp) /* loop till event */
> /*NOP*/;
> }
>
> observation: tmp being unsigned long long, get_ticks returning
> the unsigned long timestamp, tmp being a sum of two ulongs,
> the while might NEVER end. In practice that is very unlikely,
> however the code should be corrected.
Right, but this isn't only a problem of AT91 arch. Is should be fixed global.
The code is correct, if get_ticks returns real long long values.
>
> I was going to rework that timer sooner or later to address all
> those issues, but you are welcome to go ahead, too. Just we
> should avoid double work :)
I have no time enough at the moment to do that.
>
> Greetings, Reinhard
Best regards
Jens Scharsig
next prev parent reply other threads:[~2010-08-08 10:52 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-08-07 17:49 [U-Boot] [PATCH V3] AT91 Fix: return value of get_tbclk Jens Scharsig
2010-08-07 18:07 ` Reinhard Meyer
2010-08-08 10:52 ` Jens Scharsig [this message]
2010-08-09 6:34 ` Alexander Stein
2010-08-18 7:17 ` Xu, Hong
2010-08-30 6:52 ` Alexander Stein
2010-08-31 7:36 ` Reinhard Meyer
2010-08-31 16:22 ` Jens Scharsig
2010-09-01 7:30 ` Reinhard Meyer
2010-09-01 17:37 ` Jens Scharsig
2010-09-02 1:55 ` Xu, Hong
2010-09-02 6:58 ` Andreas Bießmann
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=4C5E8C5F.6000001@scharsoft.de \
--to=js_at_ng@scharsoft.de \
--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.