On Tue, Oct 16, 2018 at 5:23 PM Bart Van Assche <bvanassche@xxxxxxx> wrote:
>
> On Tue, 2018-10-16 at 09:54 +0200, Rafael J. Wysocki wrote:
> > drivers/acpi/osl.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > Index: linux-pm/drivers/acpi/osl.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/osl.c
> > +++ linux-pm/drivers/acpi/osl.c
> > @@ -623,7 +623,8 @@ void acpi_os_stall(u32 us)
> > */
> > u64 acpi_os_get_timer(void)
> > {
> > - u64 time_ns = ktime_to_ns(ktime_get());
> > + u64 time_ns = ktime_to_ns(unlikely(system_state == SYSTEM_SUSPEND) ?
> > + ktime_get_mono_fast_ns() : ktime_get());
> > do_div(time_ns, 100);
> > return time_ns;
> > }
>
> From the Documentation/core-api/timekeeping.rst section about
> ktime_get_mono_fast_ns(): "most drivers should never call them, since the
> time is allowed to jump under certain conditions".
Which doesn't apply, because the code in question runs on on CPU with
interrupts disabled.
> How about the following, also untested patch?
Well, if this is guaranteed to behave like a monotonic clock, then why not.
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index 8df9abfa947b..2fb7911b3a28 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -623,9 +623,8 @@ void acpi_os_stall(u32 us)
> */
> u64 acpi_os_get_timer(void)
> {
> - u64 time_ns = ktime_to_ns(ktime_get());
> - do_div(time_ns, 100);
> - return time_ns;
> + return (get_jiffies_64() - INITIAL_JIFFIES) *
> + (ACPI_100NSEC_PER_SEC / HZ);
> }
>
> acpi_status acpi_os_read_port(acpi_io_address port, u32 * value, u32 width)
>
> I think that there are already multiple functions in the kernel tree that
> subtract the initial value of the jiffies variable from that variable:
>
> $ git grep -nH ' - INITIAL_JIFFIES'
> arch/x86/kernel/tsc.c:228: return (jiffies_64 - INITIAL_JIFFIES) * (1000000000 / HZ);
> drivers/net/wan/hdlc_cisco.c:119: data->time = htonl((jiffies - INITIAL_JIFFIES) * (1000 / HZ));
> kernel/sched/clock.c:65: return (unsigned long long)(jiffies - INITIAL_JIFFIES)
> kernel/time/sched_clock.c:83: return (u64)(jiffies - INITIAL_JIFFIES);
> kernel/trace/trace_clock.c:72: return jiffies_64_to_clock_t(jiffies_64 - INITIAL_JIFFIES);
> net/ipv4/devinet.c:1570: return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
> net/ipv6/addrconf.c:104: return (cstamp - INITIAL_JIFFIES) * 100UL / HZ;
Yeah, but I'm not sure what Thomas (CCed) thinks about that.
Thanks,
Rafael
