[SRU][F/raspi][PATCH] rtc: class: support hctosys from modular RTC drivers

Dimitri John Ledkov dimitri.ledkov at canonical.com
Tue May 4 17:23:33 UTC 2021


See previous discussion about this attached as emails.

On Tue, May 4, 2021 at 6:22 PM Dimitri John Ledkov
<dimitri.ledkov at canonical.com> wrote:
>
> Hi,
>
> This will introduce a regression in certain configurations as
> mentioned on https://github.com/systemd/systemd/issues/17737
>
> I had a tentative patch to kernel to only advance time in rtc_hctosys
> only if it is newer than the one returned by ktime_get_real_ts64.
>
> However that went nowhere.
>
> If this is cherry picked, imho we must change config
> CONFIG_RTC_HCTOSYS or at least set it to something sane because
> currently on raspi it is set to CONFIG_RTC_HCTOSYS_DEVICE="rtc0" which
> always points at the non-battery backed RTC from the SOC.
>
> Does above make any sense?
>
>
> On Mon, May 3, 2021 at 4:46 PM Juerg Haefliger
> <juerg.haefliger at canonical.com> wrote:
> >
> > From: Steve Muckle <smuckle at google.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1926911
> >
> > Due to distribution constraints it may not be possible to statically
> > compile the required RTC driver into the kernel.
> >
> > Expand RTC_HCTOSYS support to cover all RTC devices (statically compiled
> > or not) by checking at the end of RTC device registration whether the
> > time should be synced.
> >
> > Signed-off-by: Steve Muckle <smuckle at google.com>
> > Link: https://lore.kernel.org/r/20191106194625.116692-1-smuckle@google.com
> > Signed-off-by: Alexandre Belloni <alexandre.belloni at bootlin.com>
> > (cherry picked from commit f9b2a4d6a5f18e0aaf715206a056565c56889d9f)
> > Signed-off-by: Juerg Haefliger <juergh at canonical.com>
> > ---
> >  drivers/rtc/Kconfig   |  3 --
> >  drivers/rtc/Makefile  |  1 -
> >  drivers/rtc/class.c   | 61 ++++++++++++++++++++++++++++++++++++++
> >  drivers/rtc/hctosys.c | 69 -------------------------------------------
> >  4 files changed, 61 insertions(+), 73 deletions(-)
> >  delete mode 100644 drivers/rtc/hctosys.c
> >
> > diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> > index 778cb0e63055..d9daa6ec45ff 100644
> > --- a/drivers/rtc/Kconfig
> > +++ b/drivers/rtc/Kconfig
> > @@ -41,9 +41,6 @@ config RTC_HCTOSYS_DEVICE
> >           device should record time in UTC, since the kernel won't do
> >           timezone correction.
> >
> > -         The driver for this RTC device must be loaded before late_initcall
> > -         functions run, so it must usually be statically linked.
> > -
> >           This clock should be battery-backed, so that it reads the correct
> >           time when the system boots from a power-off state. Otherwise, your
> >           system will need an external clock source (like an NTP server).
> > diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> > index a20943eafaab..9918b0429383 100644
> > --- a/drivers/rtc/Makefile
> > +++ b/drivers/rtc/Makefile
> > @@ -6,7 +6,6 @@
> >  ccflags-$(CONFIG_RTC_DEBUG)    := -DDEBUG
> >
> >  obj-$(CONFIG_RTC_LIB)          += lib.o
> > -obj-$(CONFIG_RTC_HCTOSYS)      += hctosys.o
> >  obj-$(CONFIG_RTC_SYSTOHC)      += systohc.o
> >  obj-$(CONFIG_RTC_CLASS)                += rtc-core.o
> >  obj-$(CONFIG_RTC_MC146818_LIB) += rtc-mc146818-lib.o
> > diff --git a/drivers/rtc/class.c b/drivers/rtc/class.c
> > index 9458e6d6686a..8793b2b8cf9d 100644
> > --- a/drivers/rtc/class.c
> > +++ b/drivers/rtc/class.c
> > @@ -34,6 +34,62 @@ static void rtc_device_release(struct device *dev)
> >  #ifdef CONFIG_RTC_HCTOSYS_DEVICE
> >  /* Result of the last RTC to system clock attempt. */
> >  int rtc_hctosys_ret = -ENODEV;
> > +
> > +/* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
> > + * whether it stores the most close value or the value with partial
> > + * seconds truncated. However, it is important that we use it to store
> > + * the truncated value. This is because otherwise it is necessary,
> > + * in an rtc sync function, to read both xtime.tv_sec and
> > + * xtime.tv_nsec. On some processors (i.e. ARM), an atomic read
> > + * of >32bits is not possible. So storing the most close value would
> > + * slow down the sync API. So here we have the truncated value and
> > + * the best guess is to add 0.5s.
> > + */
> > +
> > +static int rtc_hctosys(void)
> > +{
> > +       int err = -ENODEV;
> > +       struct rtc_time tm;
> > +       struct timespec64 tv64 = {
> > +               .tv_nsec = NSEC_PER_SEC >> 1,
> > +       };
> > +       struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
> > +
> > +       if (!rtc) {
> > +               pr_info("unable to open rtc device (%s)\n",
> > +                       CONFIG_RTC_HCTOSYS_DEVICE);
> > +               goto err_open;
> > +       }
> > +
> > +       err = rtc_read_time(rtc, &tm);
> > +       if (err) {
> > +               dev_err(rtc->dev.parent,
> > +                       "hctosys: unable to read the hardware clock\n");
> > +               goto err_read;
> > +       }
> > +
> > +       tv64.tv_sec = rtc_tm_to_time64(&tm);
> > +
> > +#if BITS_PER_LONG == 32
> > +       if (tv64.tv_sec > INT_MAX) {
> > +               err = -ERANGE;
> > +               goto err_read;
> > +       }
> > +#endif
> > +
> > +       err = do_settimeofday64(&tv64);
> > +
> > +       dev_info(rtc->dev.parent, "setting system clock to %ptR UTC (%lld)\n",
> > +                &tm, (long long)tv64.tv_sec);
> > +
> > +err_read:
> > +       rtc_class_close(rtc);
> > +
> > +err_open:
> > +       rtc_hctosys_ret = err;
> > +
> > +       return err;
> > +}
> >  #endif
> >
> >  #if defined(CONFIG_PM_SLEEP) && defined(CONFIG_RTC_HCTOSYS_DEVICE)
> > @@ -375,6 +431,11 @@ int __rtc_register_device(struct module *owner, struct rtc_device *rtc)
> >         dev_info(rtc->dev.parent, "registered as %s\n",
> >                  dev_name(&rtc->dev));
> >
> > +#ifdef CONFIG_RTC_HCTOSYS_DEVICE
> > +       if (!strcmp(dev_name(&rtc->dev), CONFIG_RTC_HCTOSYS_DEVICE))
> > +               rtc_hctosys();
> > +#endif
> > +
> >         return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(__rtc_register_device);
> > diff --git a/drivers/rtc/hctosys.c b/drivers/rtc/hctosys.c
> > deleted file mode 100644
> > index a74d0d890600..000000000000
> > --- a/drivers/rtc/hctosys.c
> > +++ /dev/null
> > @@ -1,69 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > -/*
> > - * RTC subsystem, initialize system time on startup
> > - *
> > - * Copyright (C) 2005 Tower Technologies
> > - * Author: Alessandro Zummo <a.zummo at towertech.it>
> > - */
> > -
> > -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > -
> > -#include <linux/rtc.h>
> > -
> > -/* IMPORTANT: the RTC only stores whole seconds. It is arbitrary
> > - * whether it stores the most close value or the value with partial
> > - * seconds truncated. However, it is important that we use it to store
> > - * the truncated value. This is because otherwise it is necessary,
> > - * in an rtc sync function, to read both xtime.tv_sec and
> > - * xtime.tv_nsec. On some processors (i.e. ARM), an atomic read
> > - * of >32bits is not possible. So storing the most close value would
> > - * slow down the sync API. So here we have the truncated value and
> > - * the best guess is to add 0.5s.
> > - */
> > -
> > -static int __init rtc_hctosys(void)
> > -{
> > -       int err = -ENODEV;
> > -       struct rtc_time tm;
> > -       struct timespec64 tv64 = {
> > -               .tv_nsec = NSEC_PER_SEC >> 1,
> > -       };
> > -       struct rtc_device *rtc = rtc_class_open(CONFIG_RTC_HCTOSYS_DEVICE);
> > -
> > -       if (!rtc) {
> > -               pr_info("unable to open rtc device (%s)\n",
> > -                       CONFIG_RTC_HCTOSYS_DEVICE);
> > -               goto err_open;
> > -       }
> > -
> > -       err = rtc_read_time(rtc, &tm);
> > -       if (err) {
> > -               dev_err(rtc->dev.parent,
> > -                       "hctosys: unable to read the hardware clock\n");
> > -               goto err_read;
> > -       }
> > -
> > -       tv64.tv_sec = rtc_tm_to_time64(&tm);
> > -
> > -#if BITS_PER_LONG == 32
> > -       if (tv64.tv_sec > INT_MAX) {
> > -               err = -ERANGE;
> > -               goto err_read;
> > -       }
> > -#endif
> > -
> > -       err = do_settimeofday64(&tv64);
> > -
> > -       dev_info(rtc->dev.parent, "setting system clock to %ptR UTC (%lld)\n",
> > -                &tm, (long long)tv64.tv_sec);
> > -
> > -err_read:
> > -       rtc_class_close(rtc);
> > -
> > -err_open:
> > -       rtc_hctosys_ret = err;
> > -
> > -       return err;
> > -}
> > -
> > -late_initcall(rtc_hctosys);
> > --
> > 2.27.0
> >
> >
> > --
> > kernel-team mailing list
> > kernel-team at lists.ubuntu.com
> > https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
>
>
> --
> Regards,
>
> Dimitri.



-- 
Regards,

Dimitri.
-------------- next part --------------
An embedded message was scrubbed...
From: Alexandre Belloni <alexandre.belloni at bootlin.com>
Subject: Re: [PATCH] rtc: class: prevent hctosys moving realclock back in time
Date: Thu, 14 Jan 2021 22:15:07 +0100
Size: 11538
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210504/1b7619f6/attachment-0004.eml>
-------------- next part --------------
An embedded message was scrubbed...
From: Alexandre Belloni <alexandre.belloni at bootlin.com>
Subject: Re: [PATCH] rtc: class: prevent hctosys moving realclock back in time
Date: Thu, 14 Jan 2021 22:41:26 +0100
Size: 13094
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210504/1b7619f6/attachment-0005.eml>
-------------- next part --------------
An embedded message was scrubbed...
From: Dimitri John Ledkov <xnox at ubuntu.com>
Subject: Re: [PATCH] rtc: class: prevent hctosys moving realclock back in time
Date: Thu, 14 Jan 2021 21:36:03 +0000
Size: 12693
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210504/1b7619f6/attachment-0006.eml>
-------------- next part --------------
An embedded message was scrubbed...
From: Dimitri John Ledkov <xnox at ubuntu.com>
Subject: [PATCH] rtc: class: prevent hctosys moving realclock back in time
Date: Thu, 14 Jan 2021 20:21:19 +0000
Size: 3701
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20210504/1b7619f6/attachment-0007.eml>


More information about the kernel-team mailing list