Bug in LL_RTC_WaitForSynchro() function in stm32l1xx_ll_rtc.c?

[MSipo]

I think there is a bug in the function LL_RTC_WaitForSynchro(). It waits for the RTC_ISR_RSF to become 0 (on row 15). I think this is a bug. It should wait for the RTC_ISR_RSF to become 1 as in the old STM32 Standard Peripheral Libraries. My code sometimes gets stuck in this loop.

ErrorStatus LL_RTC_WaitForSynchro(RTC_TypeDef *RTCx)
{
 __IO uint32_t timeout = RTC_SYNCHRO_TIMEOUT;
 ErrorStatus status = SUCCESS;
 uint32_t tmp;
 
 /* Check the parameter */
 assert_param(IS_RTC_ALL_INSTANCE(RTCx));
 
 /* Clear RSF flag */
 LL_RTC_ClearFlag_RS(RTCx);
 
 /* Wait the registers to be synchronised */
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 while ((timeout != 0U) && (tmp != 0U))
 {
 if (LL_SYSTICK_IsActiveCounterFlag() == 1U)
 {
 timeout--;
 }
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 if (timeout == 0U)
 {
 status = ERROR;
 }
 }
 
 if (status != ERROR)
 {
 timeout = RTC_SYNCHRO_TIMEOUT;
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 while ((timeout != 0U) && (tmp != 1U))
 {
 if (LL_SYSTICK_IsActiveCounterFlag() == 1U)
 {
 timeout--;
 }
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 if (timeout == 0U)
 {
 status = ERROR;
 }
 }
 }
 
 return (status);
}

Compare it to the similar function RTC_WaitForSynchro() in STM32 Standard Peripheral Libraries (stm32l1xx_rtc.c). Here we wait for the RTC_ISR_RSF to become 1 (row 19).

ErrorStatus RTC_WaitForSynchro(void)
{
 __IO uint32_t synchrocounter = 0;
 ErrorStatus status = ERROR;
 uint32_t synchrostatus = 0x00;
 
 /* Disable the write protection for RTC registers */
 RTC->WPR = 0xCA;
 RTC->WPR = 0x53;
 
 /* Clear RSF flag */
 RTC->ISR &= (uint32_t)RTC_RSF_MASK;
 
 /* Wait the registers to be synchronised */
 do
 {
 synchrostatus = RTC->ISR & RTC_ISR_RSF;
 synchrocounter++; 
 } while((synchrocounter != SYNCHRO_TIMEOUT) && (synchrostatus == 0x00));
 
 if ((RTC->ISR & RTC_ISR_RSF) != RESET)
 {
 status = SUCCESS;
 }
 else
 {
 status = ERROR;
 }
 
 /* Enable the write protection for RTC registers */
 RTC->WPR = 0xFF;
 
 return (status);
}

[Amel NASRI]

Hello @Community member​ ,

The implementation is aligned with the specification. In fact,

1. the RSF flag is cleared first by software
2. then there is a check that it is 0
3. finally we wait for it to be set again by hardware

In RM0038 rev16, page 515 we say the following: "RSF must be cleared by software. The software must then wait until it is set again before reading the RTC_SSR, RTC_TR and RTC_DR registers."

And this is exactly what was implemented in LL_RTC_WaitForSynchro. So it is not a bug.

-Amel

[MSipo]

I think the number 2 "then there is a check that it is 0" is a bug. The RM0038 does not state this. You should only wait for it to go to 1. No check for 0 is needed. It is this that causes the bug for me sometimes. It will wait for ever. I think the code sometimes miss this zero as it is so short in time.

If you look at the implementation of HAL_RTC_WaitForSynchro() you see that it also don't wait for zero, it only waits for the bit to be 1.

So I still believe there is a bug in LL_RTC_WaitForSynchro().

HAL_StatusTypeDef HAL_RTC_WaitForSynchro(RTC_HandleTypeDef *hrtc)
{
 uint32_t tickstart;
 
#if defined(STM32L100xBA) || defined (STM32L151xBA) || defined (STM32L152xBA) || defined(STM32L100xC) || defined (STM32L151xC) || defined (STM32L152xC) || defined (STM32L162xC) || defined(STM32L151xCA) || defined (STM32L151xD) || defined (STM32L152xCA) || defined (STM32L152xD) || defined (STM32L162xCA) || defined (STM32L162xD) || defined(STM32L151xE) || defined(STM32L151xDX) || defined (STM32L152xE) || defined (STM32L152xDX) || defined (STM32L162xE) || defined (STM32L162xDX)
 /* If RTC_CR_BYPSHAD bit = 0, wait for synchro else this check is not needed */
 if ((hrtc->Instance->CR & RTC_CR_BYPSHAD) == RESET)
#endif /* STM32L100xBA || STM32L151xBA || STM32L152xBA || STM32L100xC || STM32L151xC || STM32L152xC || STM32L162xC || STM32L151xCA || STM32L151xD || STM32L152xCA || STM32L152xD || STM32L162xCA || STM32L162xD || STM32L151xE || STM32L151xDX || STM32L152xE || STM32L152xDX || STM32L162xE || STM32L162xDX */
 {
 /* Clear RSF flag */
 hrtc->Instance->ISR &= (uint32_t)RTC_RSF_MASK;
 
 tickstart = HAL_GetTick();
 
 /* Wait the registers to be synchronised */
 while ((hrtc->Instance->ISR & RTC_ISR_RSF) == 0U)
 {
 if ((HAL_GetTick() - tickstart) > RTC_TIMEOUT_VALUE)
 {
 return HAL_TIMEOUT;
 }
 }
 }
 
 return HAL_OK;
}

[Amel NASRI]

OK @Community member​ , I'll highlight your feedback to our development team and if there is really no need to wait for RSF bit to be cleared, the check needs to be removed as you suggest.

Do you confirm that there is no issue from your side if you comment this check:

 /* Wait the registers to be synchronised */
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 while ((timeout != 0U) && (tmp != 0U))
 {
 if (LL_SYSTICK_IsActiveCounterFlag() == 1U)
 {
 timeout--;
 }
 tmp = LL_RTC_IsActiveFlag_RS(RTCx);
 if (timeout == 0U)
 {
 status = ERROR;
 }
 }

-Amel

[waclawek.jan]

I still see this bug e.g. in https://github.com/STMicroelectronics/STM32CubeL4/blob/5e2d2a573164939fafe9b33de427884a4fe730cf/Drivers/STM32L4xx_HAL_Driver/Src/stm32l4xx_ll_rtc.c#L811

RSF is set asynchronously to APB clock from the RTC kernel clock domain, thus it may get set before the first test for zero, resulting in errorneous timeout or infinite loop.

JW

@Amel NASRI​ 

[Amel NASRI]

Hi @Community member​ ,

This was fixed in stm32l1xx_ll_rtc.c. An internal request is already there to track deployment on STM32L4.

Internal ticket number: 127004 (This is an internal tracking number and is not accessible or usable by customers).

-Amel

[waclawek.jan]

Hi Amel,

Thanks.

This is one of those things which highlights the need to work across all related STM32 families, both for documentation and code.

Jan

@Amel NASRI​