From: Sergei Shtylyov <sshtylyov@ru•mvista.com>
To: Tony Breeds <tony@bakeyournoodle•com>
Cc: linuxppc-dev@ozlabs•org, Thomas Gleixner <tglx@linutronix•de>,
Paul Mackerras <paulus@samba•org>,
Realtime Kernel <linux-rt-users@vger•kernel.org>
Subject: Re: [PATCH v2 1/4] Implement {read,update}_persistent_clock.
Date: Wed, 17 Oct 2007 19:34:07 +0400 [thread overview]
Message-ID: <47162B6F.40903@ru.mvista.com> (raw)
In-Reply-To: <1190345162.620000.305760830507.qpush@thor>
Hello.
Tony Breeds wrote:
> With these functions implemented we cooperate better with the generic
> timekeeping code. This obsoletes the need for the timer sysdev as a bonus.
Aha, I'm seeing it's not merged to mainline yet! And this can't be merged
to -rt patch either, beucase -rt alsready has read_persistent_clock()
implemented since around 2.6.18-rt...
> Signed-off-by: Tony Breeds <tony@bakeyournoodle•com>
> ---
> Patch set updated to powerpc/for-2.6.24
> * Compile tested for arch/powerpc/configs/*_defconfig
> * Booted on pSeries, iSeries, Cell and PS3
> arch/powerpc/Kconfig | 3 +
> arch/powerpc/kernel/time.c | 85 ++++++++++-------------------------
> arch/powerpc/sysdev/Makefile | 5 --
> arch/powerpc/sysdev/timer.c | 81 ---------------------------------
> 4 files changed, 29 insertions(+), 145 deletions(-)
>
> Index: working/arch/powerpc/Kconfig
> ===================================================================
> --- working.orig/arch/powerpc/Kconfig
> +++ working/arch/powerpc/Kconfig
> @@ -21,6 +21,9 @@ config MMU
> bool
> default y
>
> +config GENERIC_CMOS_UPDATE
> + def_bool y
> +
> config GENERIC_HARDIRQS
> bool
> default y
> Index: working/arch/powerpc/kernel/time.c
> ===================================================================
> --- working.orig/arch/powerpc/kernel/time.c
> +++ working/arch/powerpc/kernel/time.c
> @@ -881,12 +837,35 @@ void __init generic_calibrate_decr(void)
> #endif
> }
>
> -unsigned long get_boot_time(void)
> +int update_persistent_clock(struct timespec now)
> {
> struct rtc_time tm;
>
> - if (ppc_md.get_boot_time)
> - return ppc_md.get_boot_time();
> + if (!ppc_md.set_rtc_time)
> + return 0;
> +
> + to_tm(now.tv_sec + 1 + timezone_offset, &tm);
> + tm.tm_year -= 1900;
> + tm.tm_mon -= 1;
> +
> + return ppc_md.set_rtc_time(&tm);
> +}
> +
> +unsigned long read_persistent_clock(void)
> +{
> + struct rtc_time tm;
> + static int first = 1;
> +
> + /* XXX this is a litle fragile but will work okay in the short term */
> + if (first) {
> + first = 0;
> + if (ppc_md.time_init)
> + timezone_offset = ppc_md.time_init();
> +
> + /* get_boot_time() isn't guaranteed to be safe to call late */
> + if (ppc_md.get_boot_time)
> + return ppc_md.get_boot_time() -timezone_offset;
> + }
This looks incomplete.
> @@ -898,14 +877,10 @@ unsigned long get_boot_time(void)
> void __init time_init(void)
> {
> unsigned long flags;
> - unsigned long tm = 0;
> struct div_result res;
> u64 scale, x;
> unsigned shift;
>
> - if (ppc_md.time_init != NULL)
> - timezone_offset = ppc_md.time_init();
> -
> if (__USE_RTC()) {
> /* 601 processor: dec counts down by 128 every 128ns */
> ppc_tb_freq = 1000000000;
> @@ -980,19 +955,14 @@ void __init time_init(void)
> /* Save the current timebase to pretty up CONFIG_PRINTK_TIME */
> boot_tb = get_tb_or_rtc();
>
> - tm = get_boot_time();
> -
> write_seqlock_irqsave(&xtime_lock, flags);
Is there any sense of grabbing xtime_lock in time_init() when you've
implemented read_persistent_clock()?
>
> /* If platform provided a timezone (pmac), we correct the time */
> if (timezone_offset) {
> sys_tz.tz_minuteswest = -timezone_offset / 60;
> sys_tz.tz_dsttime = 0;
> - tm -= timezone_offset;
> }
>
> - xtime.tv_sec = tm;
> - xtime.tv_nsec = 0;
Huh? The 'xtime' should now be set by the *generic* timekeeping code using
read_persistent_clock() -- or else what's the point to implement the function? :-/
> @@ -1010,9 +980,6 @@ void __init time_init(void)
>
> time_freq = 0;
>
> - last_rtc_update = xtime.tv_sec;
> - set_normalized_timespec(&wall_to_monotonic,
> - -xtime.tv_sec, -xtime.tv_nsec);
> write_sequnlock_irqrestore(&xtime_lock, flags);
That 'xtime_lock' grabbing should be gone with this patch, and is not. NAK.
WBR, Sergei
next prev parent reply other threads:[~2007-10-17 15:33 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2007-09-21 3:26 [PATCH v2 1/4] Implement {read,update}_persistent_clock Tony Breeds
2007-09-21 3:26 ` [PATCH v2 2/4] Implement generic time of day clocksource for powerpc machines Tony Breeds
2007-09-21 4:05 ` Daniel Walker
2007-09-21 4:59 ` Paul Mackerras
2007-09-21 6:43 ` David Gibson
2007-09-21 4:52 ` Stephen Rothwell
2007-09-21 21:35 ` Tony Breeds
2007-09-21 21:35 ` [PATCH v3 " Tony Breeds
2007-10-03 0:48 ` Paul Mackerras
2007-10-03 4:00 ` Thomas Gleixner
2007-09-21 3:26 ` [PATCH v2 3/4] Implement clockevents driver for powerpc Tony Breeds
2007-10-15 17:40 ` Sergei Shtylyov
2007-10-15 18:33 ` Sergei Shtylyov
2007-10-15 23:44 ` Paul Mackerras
2007-10-17 14:29 ` Sergei Shtylyov
2007-10-18 0:51 ` Paul Mackerras
2007-10-18 15:11 ` Sergei Shtylyov
2007-10-19 1:53 ` Paul Mackerras
2007-10-19 12:11 ` Sergei Shtylyov
2007-10-19 12:36 ` Paul Mackerras
2007-10-19 13:35 ` Sergei Shtylyov
2007-10-24 12:07 ` Sergei Shtylyov
2007-10-24 23:55 ` Paul Mackerras
2007-10-17 14:34 ` Sergei Shtylyov
2007-10-18 0:36 ` Paul Mackerras
2007-10-18 14:48 ` Sergei Shtylyov
2007-10-19 0:14 ` Paul Mackerras
2007-10-19 9:22 ` Gabriel Paubert
2007-10-19 11:22 ` Paul Mackerras
2007-10-19 11:49 ` Sergei Shtylyov
2007-10-19 12:24 ` Paul Mackerras
2007-09-21 3:26 ` [PATCH v2 4/4] Enable tickless idle and high res timers " Tony Breeds
2007-09-26 19:04 ` [PATCH v2 1/4] Implement {read,update}_persistent_clock Steven Rostedt
2007-09-26 19:39 ` Sergei Shtylyov
2007-09-26 19:44 ` Steven Rostedt
2007-09-26 19:58 ` Thomas Gleixner
2007-10-15 18:05 ` Sergei Shtylyov
2007-10-15 23:46 ` Paul Mackerras
2007-10-16 1:19 ` Benjamin Herrenschmidt
2007-10-17 12:45 ` Sergei Shtylyov
2007-09-27 1:59 ` Benjamin Herrenschmidt
2007-10-15 18:07 ` Sergei Shtylyov
2007-10-15 23:02 ` Benjamin Herrenschmidt
2007-10-17 15:34 ` Sergei Shtylyov [this message]
2007-10-18 14:18 ` Sergei Shtylyov
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=47162B6F.40903@ru.mvista.com \
--to=sshtylyov@ru$(echo .)mvista.com \
--cc=linux-rt-users@vger$(echo .)kernel.org \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=paulus@samba$(echo .)org \
--cc=tglx@linutronix$(echo .)de \
--cc=tony@bakeyournoodle$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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