clock_settime: don't block when up_rtc_settime is called

[michallenc]

Summary

Setting the current time in RTC may be a blocking operation (driver needs to wait for oscillator stabilization after reset and so on). This may cause the unwanted effect of clock_settime blocking the code execution for a considerable amount of time.

The solution is to plan a low priority work that takes care of setting the time in RTC and let clock_settime continue. We don't have to check if the work is available, just cancel it if there is a new time set request.

This is used only if CONFIG_SCHED_LPWORK is enabled. I was thinking about putting this under a special separate config option, but it seems like something you always want if you have LPWORK - but feel free to suggest otherwise, I am not against creating an option solely for this.

Impact

clock_settime now shouldn't block the code execution.

Testing

We encountered this with mcp794xx RTC - the controller waits until the oscillator is started, which may take tens of milliseconds (we measured up to ~40 ms on oscilloscope delay). This caused issue in a customer's custom protocol implementation - we receive the current time, set it and response with status message - but there were timeouts in the communication if time was set.

The issue no longer occurs if RTC set time is done in a separate work thread as clock_settime doesn't block.

[linguini1]

Seems fine! Would this cause issues with getting the clock time after the set though? Where the caller might expect the time to be set correctly once the caller returns?

Can this issue be fixed by performing the call in a worker thread at the application level, instead of in the kernel?

[michallenc]

Seems fine! Would this cause issues with getting the clock time after the set though? Where the caller might expect the time to be set correctly once the caller returns?

Can this issue be fixed by performing the call in a worker thread at the application level, instead of in the kernel?

You still get the correct time after the set, because the read with clock_gettime is performed from system clock, not from RTC. The user won't have the confirmation the write to RTC was done, but he can't rely on it now neither as we don't handle possible error from up_rtc_settime - we can't, it would violate POSIX interface of clock_settime. So even now successful clock_settime doesn't mean RTC was written.

The entire RTC clock setting is unfortunately not standardized and platform specific. Linux for example has a specific IOCTL or you can configure periodic writes to RTC if I remember it correctly, but it doesn't change it on clock_settime.

It can be fixed at the application level, but that is fixing the kernel problem from the app instead of fixing it in the kernel. And I think this is kernel issue because clock_settime shouldn't block. I am ok with adding new config option that would not make the proposed behavior default but only if user selects it if you prefer.

[linguini1]

You still get the correct time after the set, because the read with clock_gettime is performed from system clock, not from RTC. The user won't have the confirmation the write to RTC was done, but he can't rely on it now neither as we don't handle possible error from up_rtc_settime - we can't, it would violate POSIX interface of clock_settime. So even now successful clock_settime doesn't mean RTC was written.

The entire RTC clock setting is unfortunately not standardized and platform specific. Linux for example has a specific IOCTL or you can configure periodic writes to RTC if I remember it correctly, but it doesn't change it on clock_settime.

It can be fixed at the application level, but that is fixing the kernel problem from the app instead of fixing it in the kernel. And I think this is kernel issue because clock_settime shouldn't block. I am ok with adding new config option that would not make the proposed behavior default but only if user selects it if you prefer.

This explanation makes a lot of sense, I think that fixing in the kernel and having it as default behavior is fine!

[cederom]

Thank you @michallenc :-)

[xiaoxiang781216]

but in a busy system, lp work mayn't schedule for while, how to handle the time drift. @michallenc
if caller really care the blocking, it's better let caller call clock_settime in the background thread.

[michallenc]

True, but blocking for 30 ms is also not correct. The user can choose higher priority for lp work if the system is really busy and preempt other not so important tasks. Most RTCs store time in seconds, so the write would have to be significantly delayed to show any time drift.

I don't really see other good solution here, except from removing RTC set from clock_settime and having a separate API like Linux.

[xiaoxiang781216]

True, but blocking for 30 ms is also not correct. The user can choose higher priority for lp work if the system is really busy and preempt other not so important tasks. Most RTCs store time in seconds, so the write would have to be significantly delayed to show any time drift.

it's depends on hardware, nuttx even has the code to support the high res RTC(CONFIG_RTC_HIRES).

I don't really see other good solution here, except from removing RTC set from clock_settime and having a separate API like Linux.

you can update RTC through /dev/rtcx.

BT, timespec has two fields, the second clock_settime may corrupt the setting in the work thread.

[michallenc]

you can update RTC through /dev/rtcx.

That's what I would prefer in a long term, but that would mean removing up_rtc_settime from clock_settime, which would break many applications that rely on this behavior. And we would have to rewrite some RTC drivers to actually register a device driver.

[xiaoxiang781216]

you can update RTC through /dev/rtcx.

That's what I would prefer in a long term, but that would mean removing up_rtc_settime from clock_settime, which would break many applications that rely on this behavior.

And we would have to rewrite some RTC drivers to actually register a device driver.

you can utilize arch_rtc which could simplify the code a lot:
https://github.com/apache/nuttx/blob/master/drivers/timers/arch_rtc.c