From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S940482AbdEZLKl (ORCPT ); Fri, 26 May 2017 07:10:41 -0400 Received: from foss.arm.com ([217.140.101.70]:59164 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S940211AbdEZLKj (ORCPT ); Fri, 26 May 2017 07:10:39 -0400 Date: Fri, 26 May 2017 12:09:59 +0100 From: Mark Rutland To: Bogdan Mirea Cc: linux-kernel@vger.kernel.org, john.stultz@linaro.org, tglx@linutronix.de, ore@pengutronix.de, kernel@pengutronix.de Subject: Re: [PATCH v3] Added "Preserve Boot Time Support" Message-ID: <20170526110959.GB21770@leverpostej> References: <1495188983-47002-1-git-send-email-Bogdan-Stefan_mirea@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1495188983-47002-1-git-send-email-Bogdan-Stefan_mirea@mentor.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, May 19, 2017 at 01:16:23PM +0300, Bogdan Mirea wrote: > This option enables Boot Time Preservation between Bootloader and > Linux Kernel. It is based on the idea that the Bootloader (or any > other early firmware) will start the HW Timer and Linux Kernel will > count the time starting with the cycles elapsed since timer start. > > The sched_clock part is preserving boottime for kmsg which should be in > sync with system uptime. The system uptime part is driver specific and I > updated the arm_arch_timer with an arch_timer_setsystime() function > which will call do_settimeofday64() with the values read from arch timer > counter. > > This way both kmsg and uptime will be in sync, otherwise inconsistencies > will appear between the two. > > The "preserve_boot_time" parameter should be appended to kernel cmdline > from bootloader for kernel acknowledgment that the timer is running in > bootloader. > > Signed-off-by: Bogdan Mirea > --- > drivers/clocksource/arm_arch_timer.c | 33 +++++++++++++++++++++++++++++++++ In future, *please* use get_maintainer.pl, and ensure all relevant maintainers are on Cc. e.g. [mark@leverpostej:~/src/linux]% ./scripts/get_maintainer.pl -f drivers/clocksource/arm_arch_timer.c Mark Rutland (maintainer:ARM ARCHITECTED TIMER DRIVER) Marc Zyngier (maintainer:ARM ARCHITECTED TIMER DRIVER) Daniel Lezcano (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS) Thomas Gleixner (supporter:CLOCKSOURCE, CLOCKEVENT DRIVERS) linux-arm-kernel@lists.infradead.org (moderated list:ARM ARCHITECTED TIMER DRIVER) linux-kernel@vger.kernel.org (open list:CLOCKSOURCE, CLOCKEVENT DRIVERS) As I was not Cc'd, I only spotted this by chance. [...] > diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c > index 5152b38..95699cd 100644 > --- a/drivers/clocksource/arm_arch_timer.c > +++ b/drivers/clocksource/arm_arch_timer.c > @@ -475,6 +475,35 @@ struct timecounter *arch_timer_get_timecounter(void) > return &timecounter; > } > > +#ifdef CONFIG_BOOT_TIME_PRESERVE > +/* > + * Set the real system time(including the time spent in bootloader) > + * based on the timer counter. > + */ > + > +#ifndef BOOT_TIME_PRESERVE_CMDLINE > + #define BOOT_TIME_PRESERVE_CMDLINE "preserve_boot_time" > +#endif > +void arch_timer_setsystime(void) > +{ > + static struct timespec64 boot_ts; > + static cycles_t cycles; > + unsigned long long nsecs; > + > + if (!strstr(boot_command_line, BOOT_TIME_PRESERVE_CMDLINE)) > + return; > + > + cycles = arch_timer_read_counter() ? arch_timer_read_counter() : 0; > + > + nsecs = clocksource_cyc2ns(cycles, clocksource_counter.mult, > + clocksource_counter.shift); > + timespec64_add_ns(&boot_ts, nsecs); > + > + if (do_settimeofday64(&boot_ts)) > + pr_warn("arch_timer: unable to set systime\n"); > +} > +#endif /* CONFIG_BOOT_TIME_PRESERVE */ > + > static void __init arch_counter_register(unsigned type) > { > u64 start_count; > @@ -504,6 +533,10 @@ static void __init arch_counter_register(unsigned type) > > /* 56 bits minimum, so we assume worst case rollover */ > sched_clock_register(arch_timer_read_counter, 56, arch_timer_rate); > +#ifdef CONFIG_BOOT_TIME_PRESERVE > + /* Set systime */ > + arch_timer_setsystime(); > +#endif /* CONFIG_BOOT_TIME_PRESERVE */ > } Regardless of what this is trying to achieve, this does not belong in the arch timer driver, as Thomas has already said. Thanks, Mark.