Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH] rtc: adapt allowed RTC update error | Date | Wed, 02 Dec 2020 20:21:00 +0100 |
| |
On Wed, Dec 02 2020 at 12:27, Jason Gunthorpe wrote: > On Wed, Dec 02, 2020 at 02:44:53PM +0100, Thomas Gleixner wrote: >> if (IS_ENABLED(CONFIG_GENERIC_CMOS_UPDATE) || >> IS_ENABLED(CONFIG_RTC_SYSTOHC)) >> - queue_delayed_work(system_power_efficient_wq, &sync_work, 0); >> + queue_work(system_power_efficient_wq, &sync_work); > > As Miroslav noted, probably the right thing to do here is to reset the > hrtimer and remove the sync_work? I think this code was to expedite an > RTC sync when NTP fixes the clock on boot.
This has two purposes:
1) Initiating the update on boot once ntp is synced.
2) Reinitiating the sync after ntp lost sync and the work did not reschedule itself because it observed !ntp_synced().
In both cases it's highly unlikely that the write actually happens when the work is queued because do_adjtimex() would have to be exactly around the valid update window. So it will not write immediately. It will run through at least one retry.
I don't think the timer should be canceled if the ntp_synced() state did not change. Otherwise every do_adtimex() call will cancel/restart it, which does not make sense. Lemme stare at it some more.
> IIRC this was made somewhat messy due to the dual path with rtclib and > old cmos.
:)
> It would be very nice to kill the cmos path, things are better > now.. But PPC still has a long way to go: > > arch/powerpc/platforms/52xx/efika.c: .set_rtc_time = rtas_set_rtc_time, > arch/powerpc/platforms/8xx/mpc86xads_setup.c: .set_rtc_time = mpc8xx_set_rtc_time, > arch/powerpc/platforms/8xx/tqm8xx_setup.c: .set_rtc_time = mpc8xx_set_rtc_time, > arch/powerpc/platforms/cell/setup.c: .set_rtc_time = rtas_set_rtc_time, > arch/powerpc/platforms/chrp/setup.c: ppc_md.set_rtc_time = rtas_set_rtc_time; > arch/powerpc/platforms/chrp/setup.c: .set_rtc_time = chrp_set_rtc_time, > arch/powerpc/platforms/maple/setup.c: .set_rtc_time = maple_set_rtc_time, > arch/powerpc/platforms/powermac/setup.c: .set_rtc_time = pmac_set_rtc_time, > arch/powerpc/platforms/pseries/setup.c: .set_rtc_time = rtas_set_rtc_time, > > Also x86 needs a touch, it already has RTC lib, no idea why it also > provides this old path too
Because nobody had the stomach and/or cycles to touch it :)
> I wonder if the cmos path could be killed off under the dead HW > principle?
Unfortunately that code path is not that dead on x86. You need to fix all the (ab)users first. :)
Thanks,
tglx
| |