Messages in this thread | | | Subject | Re: [PATCH] m68k: Remove read_persistent_clock() | From | Greg Ungerer <> | Date | Mon, 23 Apr 2018 22:44:28 +1000 |
| |
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 >
| |