non atomic read of RTC registers

Working libraries, libraries being ported and related hardware
electrobling
Posts: 23
Joined: Fri Jan 26, 2018 9:28 pm

non atomic read of RTC registers

Post by 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:

Code: Select all

/**
 * @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;
}
It really looks to me like this is flawed. The comment refers to prescaler values, but it appears to me that it would always be a problem. The only solution that I can think of, involves turning interrupts off and polling the RTC_DIV low register for a change, and then immediately grabbing the time. That would impose a maximum delay of 1/32768 seconds using the LSE. I'd like to know what people think.

stevestrong
Posts: 2059
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany
Contact:

Re: non atomic read of RTC registers

Post by stevestrong » Fri Feb 09, 2018 9:24 am

One could read consecutively until the last two readings are identical.

electrobling
Posts: 23
Joined: Fri Jan 26, 2018 9:28 pm

Re: non atomic read of RTC registers

Post by 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
Posts: 696
Joined: Sat Nov 07, 2015 9:09 am

Re: non atomic read of RTC registers

Post by 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
Posts: 23
Joined: Fri Jan 26, 2018 9:28 pm

Re: non atomic read of RTC registers

Post by 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.

User avatar
RogerClark
Posts: 7683
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: non atomic read of RTC registers

Post by 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
Posts: 23
Joined: Fri Jan 26, 2018 9:28 pm

Re: non atomic read of RTC registers

Post by 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.

User avatar
RogerClark
Posts: 7683
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: non atomic read of RTC registers

Post by RogerClark » Fri Feb 09, 2018 9:39 pm

Thanks

electrobling
Posts: 23
Joined: Fri Jan 26, 2018 9:28 pm

Re: non atomic read of RTC registers

Post by 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:

Code: Select all

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);
}
Another is a modification to rtc_get_count():

Code: Select all

/**
 * @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 hi1, hi2;
	uint32 lo1, lo2;
	rtc_clear_sync();
	rtc_wait_sync();
	rtc_wait_finished();
	do {
		hi1 = RTC->regs->CNTH & 0xffff;
		lo1 = RTC->regs->CNTL & 0xffff;
		hi2 = RTC->regs->CNTH & 0xffff;
		lo2 = RTC->regs->CNTL & 0xffff;
	} while (hi1 != hi2 or lo1 != lo2);
	return (hi2 << 16) | lo2;
}
I have a concern about the latter. Would compiler optimization cause the same read of RTC->regs->CNTH & 0xffff to be assigned to both hi1 and hi2? If so, is there some kind of inline pragma that can disable it? Also, what are your opinions about one method vs. the other? I should also mention that I think there are a few features missing from the library, that I would like to add along with the changes I need to make in order to make calibration work.

dannyf
Posts: 228
Joined: Wed May 11, 2016 4:29 pm

Re: non atomic read of RTC registers

Post by 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:

Code: Select all

  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;

Post Reply