Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <0ca46228311ec615947e199def9fed62d70c1f07.1524118799.git.baolin.wang@linaro.org>
References: <0ca46228311ec615947e199def9fed62d70c1f07.1524118799.git.baolin.wang@linaro.org>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 20 Apr 2018 17:22:49 +0200
Message-ID: <CAK8P3a15stj5Yurd7rM7278cYPtM6kufs=QL2GihCOxRUCHRSQ@mail.gmail.com>
Subject: Re: [PATCH] m68k: Remove read_persistent_clock()
To:     Baolin Wang <baolin.wang@linaro.org>
Cc:     Geert Uytterhoeven <geert@linux-m68k.org>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Mark Brown <broonie@kernel.org>,
        linux-m68k@lists.linux-m68k.org,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, Apr 19, 2018 at 8:22 AM, Baolin Wang <baolin.wang@linaro.org> wrote:
> The read_persistent_clock() uses a timespec, which is not year 2038 safe
> on 32bit systems. Moreover on m68k architecture, we have implemented generic
> RTC drivers that can be used to compensate the system suspend time. So
> we can remove the obsolete read_persistent_clock().
>
> Signed-off-by: Baolin Wang <baolin.wang@linaro.org>

I'm not sure about this one: it's still possible to turn off
CONFIG_RTC_DRV_GENERIC
on M68KCLASSIC, and in that case we still need a read_persistent_clock64()
implementation. Or we use your patch but make CONFIG_RTC_DRV_GENERIC
mandatory.

See below for a version I did a while ago (but never submitted as I got
distracted).

       Arnd

commit 13ddf5a33a195e9b7a7a6ed10481363b5259c1d4
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Thu Jan 25 15:30:50 2018 +0100

    m68k: use read_persistent_clock64 consistently

    We have two ways of getting the current time from a platform at boot
    or during suspend: either using read_persistent_clock() or the rtc
    class operation. We never need both, so I'm hiding the
    read_persistent_clock variant when the generic RTC is enabled.

    Since read_persistent_clock() and mktime() are deprecated because of
    the y2038 overflow of time_t, we should use the time64_t based
    replacements here.

    Finally, the dependency on CONFIG_ARCH_USES_GETTIMEOFFSET looks
    completely bogus in this case, so let's remove that. It was
    added in commit b13b3f51ff7b ("m68k: fix inclusion of
    arch_gettimeoffset for non-MMU 68k classic CPU types") to deal
    with arch_gettimeoffset(), which has since been removed from
    this file and is unrelated to the RTC functions.

    The rtc accessors are only used by classic machines, while
    coldfire uses proper RTC drivers, so we can put the old
    ifdef back around both functions.

    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/m68k/kernel/time.c b/arch/m68k/kernel/time.c
index 97dd4e26f234..edcf3855d8b0 100644
--- a/arch/m68k/kernel/time.c
+++ b/arch/m68k/kernel/time.c
@@ -71,7 +71,9 @@ static irqreturn_t timer_interrupt(int irq, void *dummy)
        return IRQ_HANDLED;
 }

-void read_persistent_clock(struct timespec *ts)
+#ifdef CONFIG_M68KCLASSIC
+#if !IS_BUILTIN(CONFIG_RTC_DRV_GENERIC)
+void read_persistent_clock64(struct timespec64 *ts)
 {
        struct rtc_time time;
        ts->tv_sec = 0;
@@ -82,12 +84,13 @@ void read_persistent_clock(struct timespec *ts)

                if ((time.tm_year += 1900) < 1970)
                        time.tm_year += 100;
-               ts->tv_sec = mktime(time.tm_year, time.tm_mon, time.tm_mday,
+               ts->tv_sec = mktime64(time.tm_year, time.tm_mon, time.tm_mday,
                                      time.tm_hour, time.tm_min, time.tm_sec);
        }
 }
+#endif

-#if defined(CONFIG_ARCH_USES_GETTIMEOFFSET) &&
IS_ENABLED(CONFIG_RTC_DRV_GENERIC)
+#if IS_ENABLED(CONFIG_RTC_DRV_GENERIC)
 static int rtc_generic_get_time(struct device *dev, struct rtc_time *tm)
 {
        mach_hwclk(0, tm);
@@ -145,8 +148,8 @@ static int __init rtc_init(void)
 }

 module_init(rtc_init);
-
-#endif /* CONFIG_ARCH_USES_GETTIMEOFFSET */
+#endif /* CONFIG_RTC_DRV_GENERIC */
+#endif /* CONFIG M68KCLASSIC */

 void __init time_init(void)
 {