Re: [PATCH 0/2] Expand oscillator stop flag (OSF) validity check to ds1341

[Rodolfo Giometti <giometti@enneenne.com>]

On 01/07/25 00:54, Tyler Hicks wrote:
> [Adding Rodolfo Giometti]
> 
> On 2025-06-11 11:14:14, Meagan Lloyd wrote:
>> We would like to use CONFIG_RTC_HCTOSYS to sync a supercapacitor-backed
>> DS1342 RTC to the kernel time early in boot. An obstacle is that the
>> sync in rtc_hctosys() is unconditional as long as rtc_read_time()
>> succeeds and in some power loss situations, our RTC comes up with either
>> an unpredictable future time or the default 01/01/00 from the datasheet.
>> Syncing a future time, followed by an NTP sync would not be desired as
>> it would result in a backwards time jump. The sync feature is useful in
>> boot scenarios where power is maintained so syncing only when the RTC
>> data is valid would allow us to make use of the feature.
>>
>> The DS1342 has the oscillator stop flag (OSF) which is a status flag
>> indicating that the oscillator stopped for a period of time. It can be
>> set due to power loss. Some chip types in the ds1307 driver already use
>> the OSF to determine whether .read_time should provide valid data or
>> return -EINVAL. This patch series expands that handling to the ds1341
>> chip type (DS1341 and DS1342 share a datasheet).
>>
>> These changes enable us to make use of CONFIG_RTC_HCTOSYS as they
>> prevent the invalid time from getting synced to the kernel time. It will
>> also prevent userspace programs from getting the invalid time as the fix
>> cuts it off at the source - the .read_time function.
> 
> These two patches look good to me, although I'm not an expert in RTC drivers.
> I've reviewed the DS1341/DS1342 datasheet and the approach that Meagan has
> taken makes sense to me given our (Meagan and I work together) desire to use
> CONFIG_RTC_HCTOSYS and the need to avoid syncing from an invalid RTC state.
> 
> I've added Rodolfo because he first added the logic to clear the Oscillator
> Stop Flag, during driver initialization, way back in 2007 with v2.6.23 commit
> be5f59f4b67f ("rtc-ds1307: oscillator restart for ds13{37,38,39,40}") and may
> have additional context to provide.
> 
> Alexandre and Rodolfo, does this approach make sense to you? If not, do you
> have any other suggestions on how to make CONFIG_RTC_HCTOSYS work with this
> driver? Thanks!

They look good to me. You can add my Acked-by line to all of them:

     Acked-by: Rodolfo Giometti <giometti@enneenne.com>

Rodolfo

> Tyler
> 
>>
>> Meagan Lloyd (2):
>>    rtc: ds1307: remove clear of oscillator stop flag (OSF) in probe
>>    rtc: ds1307: handle oscillator stop flag (OSF) for ds1341
>>
>>   drivers/rtc/rtc-ds1307.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>>
>> base-commit: 19272b37aa4f83ca52bdf9c16d5d81bdd1354494
>> -- 
>> 2.49.0
>>

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming