2021-08-30 11:26 PM
I am referring to the bug whereby the shadowing of the date/time registers is very occassionally broken if the sync prescaler was read beforehand.
That reading order is exactly what the ST HAL functions do. See below.
HAL_RTC_GetTime reads the SSR and the time.
HAL_RTC_GetDate reads the date.
I have been doing tests and reading the RTC at 5Hz it fails once every few hours; then my code below reads the whole lot again.
int getrtc (struct tm * mytime)
{
RTC_TimeTypeDef sTime = {0};
RTC_DateTypeDef sDate = {0};
int rtcret1=0;
int rtcret2=0;
RTC_Lock();
// Implement RTC errata sheet (SSR needs reading before and after date/time and compared, until same)
uint32_t last_subsec;
volatile uint32_t errorcnt=0; // for doing a breakpoint
do
{
last_subsec = (uint32_t)(hrtc.Instance->SSR);
// Always read both, even if 1st returns an error
rtcret1 = HAL_RTC_GetTime(&hrtc, &sTime, RTC_FORMAT_BIN);
rtcret2 = HAL_RTC_GetDate(&hrtc, &sDate, RTC_FORMAT_BIN);
errorcnt++;
}
while (last_subsec != sTime.SubSeconds);
RTC_Unlock();
if ( (rtcret1!=0) || (rtcret2!=0))
return 2;
// Convert the 32F4 RTC API to the K&R standard C clock structure "tm"
// Comments in setrtc above
// No error checking - would be meaningless
mytime->tm_year = sDate.Year+100;
mytime->tm_mon = sDate.Month-1;
mytime->tm_mday = sDate.Date;
mytime->tm_wday = sDate.WeekDay; // assume the RTC maintained the day of week correctly
if (sDate.WeekDay==7) mytime->tm_wday = 0;
mytime->tm_hour = sTime.Hours;
mytime->tm_min = sTime.Minutes;
mytime->tm_sec = sTime.Seconds;
// Load subseconds into a global variable, as microseconds
// There were problems with this when using the SHIFTR method to advance the RTC...
g_SubSeconds = ((RTC_SYNC_PREDIV-sTime.SubSeconds)*1000000L)/(RTC_SYNC_PREDIV+1);
// Calculate day of year from the date, because that doesn't come from the RTC.
mytime->tm_yday = day_of_year(mytime->tm_mday, mytime->tm_mon, mytime->tm_year);
// Return -1 for daylight saving, as per K&R
mytime->tm_isdst = -1;
return 0;
}
2021-09-01 09:20 PM
Your writeup looks good (without compiling the code and checking it all).
You won't see the fault after a few mins. It happens (of the order of) once in every 10k RTC accesses, at 5Hz.
And if doing this workaround, there is no need to enable shadowing at all, is there?
Also worth pointing out that STM's own code triggers this issue. Most people will just use HAL_RTC_GetTime and HAL_RTC_GetDate, and the first of those reads the SSR! The vast majority of users won't need subseconds but they are still getting this bug activated free of charge... Why didn't STM fix this?
In my system there is no way the RTOS will go away for 1 sec or anything like that. This isn't winNT, which in any case is not fully pre-emptive (no version of Windows is AFAIK, not even the "embedded" versions) and an errant process at the right privilege level can just hang it up. That's why bugs in things like display drivers are famous for crashing PCs.
2021-09-02 01:15 AM
> You won't see the fault after a few mins. It happens (of the order of) once in every 10k RTC accesses, at 5Hz.
That code uses unusual setup, with RTC clock = HSE/32=250kHz while APB clock is HSI=8MHz. This brings the RTC-registers-update-to-APB-clock ratio down to 64, from the usual cca 20000; given the problem's description says it happens when the locking read is initiated one APB clock before the registers update, this should increase the probability it happens in the 20000/64-times. Also, I deliberately attempted to make the reads random (although admittedly in a sloppy way), whereas your code may have inadvertent synchronicities (e.g. from the RTOS tick). Also, the unusually low prescalers I use there increase the rate of RTC update (7800 times), further decreasing the time within which the problem should exhibit itself.
> And if doing this workaround, there is no need to enable shadowing at all, is there?
The difference is in the 2 APB clock waitstates per non-shadowed register read. In your application that won't matter.
> Why didn't STM fix this?
As I've said, I am not interested in Cube. If you want discuss this with ST, start a new thread clearly stating the issue and mark it (using "Topic") as Cube_FW. Or contact ST directly.
JW
2021-09-03 12:00 AM
You often state you are not interested in Cube :grinning_face: but the reality is that probably most new users will start with the HAL functions, and the get time one always reads the SSR and thus triggers this issue.
What I meant by "fixing" is fixing the silicon. The register locking is absolutely key in any RTC, otherwise you have to read the whole thing twice and compare. ST obviously know about this - they issued the errata after all!
2021-11-02 11:14 AM
/**
* @brief Read the RTC_SSR value
* As described in the reference manual, the RTC_SSR shall be read twice to ensure
* reliability of the value
* @param None
* @retval SSR value read
*/
static uint32_t ReadRtcSsrValue(void)
{
uint32_t first_read;
uint32_t second_read;
first_read = (uint32_t)(READ_BIT(RTC->SSR, RTC_SSR_SS));
second_read = (uint32_t)(READ_BIT(RTC->SSR, RTC_SSR_SS));
while(first_read != second_read)
{
first_read = second_read;
second_read = (uint32_t)(READ_BIT(RTC->SSR, RTC_SSR_SS));
}
return second_read;
}
2021-11-02 12:13 PM
There's no point to read *only* subseconds multiple times. The problem is in the atomicity of reading SSR+TR+DR.
JW
2022-02-13 06:56 AM
2022-02-14 01:13 PM
In the end, I used the solution with BYPSHAD=1 because it doesn't have a problem with locking registers described in errata and doesn't have to wait an unreasonable amount of time (60 µs).
I tested this code 500,000x / 1sec for several tens of hours without any problem.
void ReadDateTime(void)
{
volatile u32 subsec, timereg, datereg;
volatile u32 subsec2, timereg2, datereg2;
do
{
datereg = RTC->DR;
timereg = RTC->TR;
subsec = RTC->SSR;
datereg2 = RTC->DR;
timereg2 = RTC->TR;
subsec2 = RTC->SSR;
} while (datereg != datereg2 ||
timereg != timereg2 ||
subsec != subsec2
);
... process subsec,timereg & datereg ...
2022-02-14 05:42 PM
So, according to the errata, my idea only works with BYPSHAD=0. I updated the original comment accordingly.
But, because of BYPSHAD=0, there seems to be another problem with the version with subseconds:
uint32_t rTR1 = RTC->TR;
uint32_t rDR1 = RTC->DR;
// Should we clear the RTC_ISR_RSF and wait for it to be set again here?
uint32_t rSSR = RTC->SSR;
uint32_t rTR2 = RTC->TR;
uint32_t rDR2 = RTC->DR;
If that is the case, then indeed my idea becomes not so cool for the SSR+TR+DR scenario. Any comments?
2022-02-15 12:51 AM
According to my and @Community member tests the algorithm which works with BYPSHAD=0 is
volatile uint32_t drreg1, trreg1, ssrreg1;
volatile uint32_t drreg2, trreg2, ssrreg2;
do
{
// Unlock RTC registers
RTC->WPR = 0xCA;
RTC->WPR = 0x53;
// Clear the RTC_ISR_RSF
RTC->ISR &= (uint32_t)~RTC_ISR_RSF;
// Lock RTC registers
RTC->WPR = 0xFF;
// Wait for RSF
while ((RTC->ISR AND RTC_ISR_RSF) == 0);
ssrreg1 = RTC->SSR;
trreg1 = RTC->TR;
drreg1 = RTC->DR;
// Unlock RTC registers
RTC->WPR = 0xCA;
RTC->WPR = 0x53;
// Clear the RTC_ISR_RSF
RTC->ISR &= (uint32_t)~RTC_ISR_RSF;
// Lock RTC registers
RTC->WPR = 0xFF;
// Wait for RSF
while ((RTC->ISR AND RTC_ISR_RSF) == 0);
ssrreg2 = RTC->SSR;
trreg2 = RTC->TR;
drreg2 = RTC->DR;
} while (ssrreg1 != ssrreg2);
Some notes:
2022-02-15 06:30 AM
@Piranha ,
as much as I like your method, as it avoids the loop; unfortunately, with BYPSHAD=0,
> // Should we clear the RTC_ISR_RSF and wait for it to be set again here?
appears to be a necessity... as
sec:22 subsec: 1
sec:22 subsec: 0
sec:22 subsec: 255 <-- Error
sec:23 subsec: 255
sec:23 subsec: 254
has been observed in the wild (on a 'F429; and SSR was read before TR and DR, it's just the printout which swaps them), when omitting waiting for RSF. It means, without waiting for RSF, SSR being read as 255 does not guarantee that TR will be read as incremented.
Now there still might be some nuances which would allow your method to be used safely in either BYPSHAD setting, but without knowing the intimate internal details I am afraid it would be a long and painful process to get to its end.
JW