lkml.org 
[lkml]   [2018]   [Apr]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] m68k: Remove read_persistent_clock()
From
Date
On 23/04/18 21:47, Arnd Bergmann wrote:
> On Mon, Apr 23, 2018 at 11:47 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> Hi Arnd,
>>
>> On Mon, Apr 23, 2018 at 11:28 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> On Mon, Apr 23, 2018 at 11:07 AM, Geert Uytterhoeven
>>> <geert@linux-m68k.org> wrote:
>>>> On Fri, Apr 20, 2018 at 5:22 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>>> 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.
>>>>
>>>> Typically (in the defconfigs) CONFIG_RTC_DRV_GENERIC is either "m",
>>>> or not set.
>>>>
>>>> And in both cases, a platform-specific RTC class driver may or may not be
>>>> builtin or loaded. We have them for some Amigas (RTC_DRV_MSM6242 or
>>>> RTC_DRV_RP5C01).
>>>>
>>>> I've never been an expert of timekeeping code...
>>>
>>> Some extra background on this:
>>>
>>> read_persistent_clock64/read_persistent_clock is primarily needed when you
>>> have a real time source that is better than the one exposed in the RTC class
>>> driver. For platforms doing suspend/resume, the timekeeping code will
>>> first try calling read_persistent_clock64() but fall back to the RTC
>>> if that fails.
>>> On only a few architectures like m68k, read_persistent_clock64() falls
>>> back to reading the RTC hardware, which means it will still work even if
>>> the RTC_DRV_GENERIC driver is not loaded. Removing that code will
>>> still work correctly as long as the generic RTC driver does get loaded
>>> before suspend. On platforms without suspend/resume, none of this matters.
>>
>> M68k-with-MMU[*] does not support suspend/resume.
>>
>> [*] Probably the PM dependency should be updated for Coldfire with MMU?
>
> Ok, so for the suspend case on m68k, can can totally ignore
> read_persistent_clock64(): classic doesn't have suspend and
> coldfire doesn't implement mach_hwclock() callbacks, right?

Yep, that is right, no ColdFire platforms use it.

Regards
Greg


> For reference, this is the set of machines implementing mach_hwclk:
>
> arch/m68k/68000/m68328.c: mach_hwclk = m68328_hwclk;
> arch/m68k/68000/m68EZ328.c: mach_hwclk = m68328_hwclk;
> arch/m68k/68000/m68VZ328.c: mach_hwclk = m68328_hwclk;
> arch/m68k/apollo/config.c: mach_hwclk = dn_dummy_hwclk; /* */
> arch/m68k/atari/config.c: mach_hwclk = atari_tt_hwclk;
> arch/m68k/atari/config.c: mach_hwclk = atari_mste_hwclk;
> arch/m68k/bvme6000/config.c: mach_hwclk = bvme6000_hwclk;
> arch/m68k/hp300/config.c: mach_hwclk = hp300_hwclk;
> arch/m68k/mac/config.c: mach_hwclk = mac_hwclk;
> arch/m68k/mvme147/config.c: mach_hwclk = mvme147_hwclk;
> arch/m68k/mvme16x/config.c: mach_hwclk = mvme16x_hwclk;
> arch/m68k/q40/config.c: mach_hwclk = q40_hwclk;
> arch/m68k/sun3/config.c: mach_hwclk = sun3_hwclk;
> arch/m68k/sun3x/config.c: mach_hwclk = sun3x_hwclk;
>
>>> The other user of read_persistent_clock() is to set the initial time at boot.
>>> This again can be done by the RTC subsystem if the correct RTC driver
>>> is built-in and CONFIG_RTC_HCTOSYS is set. Alexandre is planning
>>> to remove that option and instead force early user space to load the
>>> RTC driver and then sync it manually using the sysfs knob for it.
>>>
>>> If you have ancient user space that doesn't do this, you might still
>>> rely on read_persistent_clock() to set the boot time.
>>
>> Yeah, we have some ancient userspace (old ramdisk from just after the
>> a.out to ELF switch ;-), which worked fine last time I tried ;-)
>>
>> But this should not be considered a dependency, as most people will
>> run e.g. Debian/ports, which I assume is a modern userspace.
>
> Ok. In that case, a possible cleanup for the old time handling
> could be to move each of the hwclk implementations over to use
> their own RTC driver instead of a mach_hwclk function with
> generic_rtc.
>
> It seems that the mach_set_ss and nach_set_mmss
> callbacks can simply get removed already, they are
> never called. Similarly, the mach_get_rtc_pll() and
> mach_get_rtc_pll() callbacks are only used on q40, and
> could be removed after changing the q40 over to using
> its own RTC driver, or registering the ioctl operation itself.
>
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

\
 
 \ /
  Last update: 2018-04-23 14:45    [W:0.283 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site