[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1705212251570.3023@nanos>
Date: Sun, 21 May 2017 23:36:21 +0200 (CEST)
From: Thomas Gleixner <tglx@...utronix.de>
To: Bogdan Mirea <Bogdan-Stefan_mirea@...tor.com>
cc: linux-kernel@...r.kernel.org, john.stultz@...aro.org,
ore@...gutronix.de, kernel@...gutronix.de
Subject: Re: [PATCH v3] Added "Preserve Boot Time Support"
On Fri, 19 May 2017, Bogdan Mirea wrote:
> +#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;
This adds a arch_timer specific command line option. Why is this arch_timer
specific? So if any other platform wants to gain this feature then we end
up copying that mess to every single timer implementation? Certainly NOT!
> +
> + 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");
What the heck is this? What has boot_ts to do with do_settimeofday()?
Exactly nothing. settimeofday() modifies CLOCK_REALTIME and if the platform
has an early accessible RTC, you hereby wreckaged wall_time. If the RTC
readout comes later then CLOCK_REALTIME is overwritten. So what is this
supposed to do?
It has absolutely nothing to do with CLOCK_BOOTTIME. /proc/uptime is based
on CLOCK_BOOTTIME, which is the CLOCK_MONOTONIC time since system boot. The
difference between CLOCK_MONOTONIC and CLOCK_BOOTTIME is that
CLOCK_MONOTONIC does not advance during suspend, but CLOCK_BOOTTIME takes
the suspended time into account.
I have not the faintest idea how you can claim that this patch actually
does what it is supposed to do. It's simply crap and CANNOT work at all.
Thanks,
tglx
Powered by blists - more mailing lists