non atomic read of RTC registers

electrobling
Fri Feb 09, 2018 5:12 am
I’m trying to get ahead of things while I wait for my hardware to arrive. The RTC is a very important thing for me, I would rather not depend on external RTC modules so I’m looking forward to using the on chip RTC hardware. I have some ideas for implementing the software frequency calibration that is provided on chip that I will present later in a separate thread when everything is ready. To that end, I have been digging into the Maple RTC library, and found what seems to be a disturbing problem. The upper and lower 16 bits of the current time are read separately, while the clock is running. That means that there can be times when they are not synchronized, and will produce an incorrect time. That is because the counter value can change between reads of the upper and lower 16 bits. Here is the code:
/**
* @brief Returns the rtc's counter (i.e. time/date) value.
*
* This value is likely to be inaccurate if the counter is running
* with a low prescaler.
*/
uint32 rtc_get_count() {
uint32 h, l;
rtc_clear_sync();
rtc_wait_sync();
rtc_wait_finished();
h = RTC->regs->CNTH & 0xffff;
l = RTC->regs->CNTL & 0xffff;
return (h << 16) | l;
}

stevestrong
Fri Feb 09, 2018 9:24 am
One could read consecutively until the last two readings are identical.

electrobling
Fri Feb 09, 2018 1:10 pm
Yes, that is smarter. I think it is better to do this action at the library level than at the application level. So I will implement it and submit a pull request when I am able to do testing.

racemaniac
Fri Feb 09, 2018 2:49 pm
Was wondering of STM didn’t realize it themselves, apparantly they did:
RSF: Registers synchronized flag
This bit is set by hardware at each time the RTC_CNT and RTC_DIV registers are updated
and cleared by software. Before any read operation after an APB1 reset or an APB1 clock
stop, this bit must be cleared by software, and the user application must wait until it is set to
be sure that the RTC_CNT, RTC_ALR or RTC_PRL registers are synchronized.
0: Registers not yet synchronized.
1: Registers synchronized.

So you can clear a bit that the RTC will set whenever it updates its registers, and can use that to make sure you only read right after a write to those registers.

Haven’t read much further into it, but if it’s possible to get an interrupt from the rtc after an update to the registers, that might also be useful to address this (if you can handle the interrupt fast enough)

*edit*
I’m also wondering how well the proposed repetitive read solution would work. It works perfectly if you assume the write is atomic, but is that guaranteed?


electrobling
Fri Feb 09, 2018 6:37 pm
Thanks for the heads up. I also thought ST would not let something like this go without a solution. It sounds like the guarantee comes from checking the RSF flag, which is cleaner than comparing the registers anyway. So I will look into it tonight. The code is new to me, maybe the existing function already checks it somewhere.

RogerClark
Fri Feb 09, 2018 8:34 pm
If you make an improvement to the existing code, could you post it, or create. PR on GitHub, so that the master code can be updated

electrobling
Fri Feb 09, 2018 9:20 pm
If my investigation leads me to change things, I will do that for sure. Here and as a PR. I need an atomic read, not only of the RTC_CNT, but also of the RTC_DIV, for the RTC calibration procedure which I am preparing. Details of that project will follow once I have successfully reached the first test checkpoint.

RogerClark
Fri Feb 09, 2018 9:39 pm
Thanks

electrobling
Sat Feb 10, 2018 6:37 pm
Okay, I did some digging last night. In my opinion, my original suspicions are confirmed. My take on section 18.3.3 Reading RTC registers in the user manual, is that the RSF bit indicates synchronization between the RTC registers and APB1 bus, not necessarily synch between different RTC registers. In all honesty, I calculated the probability of a miss, and that it is likely to happen once in 35,000 years. :) That might make the problem seem rather academic. It also means, that testing is impossible. Any confidence in it has to come from confidence in the algorithm.

However, for the sake of science, I came up with two solutions that I would feel confident of. Both are implementations of a double read and compare. I feel that I need some guidance with this, for one thing, this is helping me get familiar with the mechanics of the software so I can move on to step two, the calibration.

One is to modify the class, leaving the utility function rtc_get_count() unchanged:
time_t RTClock::getTime() {
time_t t1, t2;
do {
t1 = rtc_get_count();
t2 = rtc_get_count();
} while (t1 != t2);
return t2;
}

void RTClock::getTime(tm_t & tm_ptr) {
time_t res = getTime();
breakTime(res, tm_ptr);
}


dannyf
Sat Feb 10, 2018 8:05 pm
It really looks to me like this is flawed

you are correct. the code is wrong.

the typical strategy is a double read:

uint32_t msw, lsw; //most and least significant word
do {
msw = CNTH; //read the high word
lsw = CNTL; //read the low word
} while (msw ^ CNTH); //keep reading until msw:lsw are atomic
return (msw << 16) | lsw;


electrobling
Sat Feb 10, 2018 9:02 pm
Thanks, Danny. That is a neat abbreviation too! I think I also see that it is not necessary to also check the low word. I blame the enormous snowstorm that has engulfed my city, and has affected my brain somehow. I submitted a pull request.

Leave a Reply

Your email address will not be published. Required fields are marked *