Re: [PATCH] rtc: adapt allowed RTC update error

[Alexandre Belloni]

On 03/12/2020 22:05:09+0100, Thomas Gleixner wrote:
> 2) I2C/SPI ...
> 
>    tsched t0                 t1                     t2
>           transfer(newsec)   RTC update (newsec)    RTC increments seconds
> 
>    Lets assume that ttransfer = t1 - t0 is known.

Note that ttransfer is one of the reason why setting set_offset_nsec
from the RTC driver is not a good idea. The same RTC may be on busses
with different rates and there is no way to know that. I think that was
one of my objections at the time.

ttransfer is not a function of the RTC model but rather of how it is
integrated in the system.

> 
>    tinc is the same as above = t2 - t1
> 
>    Again, lets assume that the event is accurate for now and ignore the fuzz
>    logic, i.e. tsched == t0
> 
>    So tsched has to be ttot = t2 - t0 _before_ wallclock reaches t2 and
>    increments seconds.
> 

I had a rough week and I'm probably not awake enough to follow
completely but is that thinking correct?

For the mc146818, you have t1 - t0 which is probably negligible and t2 -
T& == 500 ms

For most of the other RTCs, you have t1 - t0 is somewhat important,
probably around 100 to 150µs and t2 - t1 is 1s. I would think that what
is needed is tsched has to be t1-t0 before wallclock reaches t1. In that
case t2 doesn't matter, it will always be 1s after t1.

>    In this case newsec = t1.tv_sec = (t0 + ttransfer).tv_sec
> 
>    So now the fuzz logic for this is:
> 
>       newtime = t0 + ttransfer;
> 
>       if (newtime.tv_nsec < FUZZ)
>           newsec = newtime.tv_sec;
>       else if (newtime.tv_nsec > NSEC_PER_SEC - FUZZ)
>           newsec = newtime.tv_sec + 1;
>       else
>           goto fail;
> 
>    Again the first condition handles the case where t1 >= tsched and the
>    second one where t1 < tsched.
> 
> So now we have two options to fix this:
> 
>    1) Use a negative sync_offset for devices which need #1 above
>       (rtc_cmos & similar)
> 
>       That requires setting tsched to t2 - abs(sync_offset)
> 
>    2) Use always a positive sync_offset and a flag which tells
>       rtc_tv_nsec_ok() whether it needs to add or subtract.
> 
> #1 is good enough. All it takes is a comment at the timer start code why
> abs() is required.
> 
> Let me hack that up along with the hrtimer muck.
> 
> Thanks,
> 
>         tglx

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com