Fix tickless idle when stopping systick on zero...

...and don't stop SysTick at all in the eAbortSleep case. Prior to this commit, if vPortSuppressTicksAndSleep() happens to stop the SysTick on zero, then after tickless idle ends, xTickCount advances one full tick more than the time that actually elapsed as measured by the SysTick. See "bug 1" in this forum post: https://forums.freertos.org/t/ultasknotifytake-timeout-accuracy/9629/40 SysTick ------- The SysTick is the hardware timer that provides the OS tick interrupt in the official ports for Cortex M. SysTick starts counting down from the value stored in its reload register. When SysTick reaches zero, it requests an interrupt. On the next SysTick clock cycle, it loads the counter again from the reload register. To get periodic interrupts every N SysTick clock cycles, the reload register must be N - 1. Bug Example ----------- - Idle task calls vPortSuppressTicksAndSleep(xExpectedIdleTime = 2). [Doesn't have to be "2" -- could be any number.] - vPortSuppressTicksAndSleep() stops SysTick, and the current-count register happens to stop on zero. - SysTick ISR executes, setting xPendedTicks = 1 - vPortSuppressTicksAndSleep() masks interrupts and calls eTaskConfirmSleepModeStatus() which confirms the sleep operation. *** - vPortSuppressTicksAndSleep() configures SysTick for 1 full tick (xExpectedIdleTime - 1) plus the current-count register (which is 0) - One tick period elapses in sleep. - SysTick wakes CPU, ISR executes and increments xPendedTicks to 2. - vPortSuppressTicksAndSleep() calls vTaskStepTick(1), then returns. - Idle task resumes scheduler, which increments xTickCount twice (for xPendedTicks = 2) In the end, two ticks elapsed as measured by SysTick, but the code increments xTickCount three times. The root cause is that the code assumes the SysTick current-count register always contains the number of SysTick counts remaining in the current tick period. However, when the current-count register is zero, there are ulTimerCountsForOneTick counts remaining, not zero. This error is not the kind of time slippage normally associated with tickless idle. *** Note that a recent commit e1b98f0 results in eAbortSleep in this case, due to xPendedTicks != 0. That commit does mostly resolve this bug without specifically mentioning it, and without this commit. But that resolution allows the code in port.c not to directly address the special case of stopping SysTick on zero in any code or comments. That commit also generates additional instances of eAbortSleep, and a second purpose of this commit is to optimize how vPortSuppressTicksAndSleep() behaves for eAbortSleep, as noted below. This commit also includes an optimization to avoid stopping the SysTick when eTaskConfirmSleepModeStatus() returns eAbortSleep. This optimization belongs with this fix because the method of handling the SysTick being stopped on zero changes with this optimization.

Author: jefftenney

portable/GCC/ARM_CM4F/port.c

@@ -52,11 +52,13 @@
 #define portNVIC_SYSTICK_LOAD_REG( * ( ( volatile uint32_t * ) 0xe000e014 ) )
 #define portNVIC_SYSTICK_CURRENT_VALUE_REG( * ( ( volatile uint32_t * ) 0xe000e018 ) )
 #define portNVIC_SYSPRI2_REG( * ( ( volatile uint32_t * ) 0xe000ed20 ) )
+#define portNVIC_ICSR_REG( * ( ( volatile uint32_t * ) 0xe000ed04 ) )
 /* ...then bits in the registers. */
 #define portNVIC_SYSTICK_INT_BIT( 1UL << 1UL )
 #define portNVIC_SYSTICK_ENABLE_BIT( 1UL << 0UL )
 #define portNVIC_SYSTICK_COUNT_FLAG_BIT( 1UL << 16UL )
 #define portNVIC_PENDSVCLEAR_BIT	( 1UL << 27UL )
+#define portNVIC_PEND_SYSTICK_SET_BIT( 1UL << 26UL )
 #define portNVIC_PEND_SYSTICK_CLEAR_BIT( 1UL << 25UL )
 
 /* Constants used to detect a Cortex-M7 r0p1 core, which should use the ARM_CM7
@@ -518,21 +520,6 @@ void xPortSysTickHandler( void )
 xExpectedIdleTime = xMaximumPossibleSuppressedTicks;
 }
 
-/* Stop the SysTick momentarily. The time the SysTick is stopped for
-is accounted for as best it can be, but using the tickless mode will
-inevitably result in some tiny drift of the time maintained by the
-kernel with respect to calendar time. */
-portNVIC_SYSTICK_CTRL_REG &= ~portNVIC_SYSTICK_ENABLE_BIT;
-
-/* Calculate the reload value required to wait xExpectedIdleTime
-tick periods. -1 is used because this code will execute part way
-through one of the tick periods. */
-ulReloadValue = portNVIC_SYSTICK_CURRENT_VALUE_REG + ( ulTimerCountsForOneTick * ( xExpectedIdleTime - 1UL ) );
-if( ulReloadValue > ulStoppedTimerCompensation )
-{
-ulReloadValue -= ulStoppedTimerCompensation;
-}
-
 /* Enter a critical section but don't use the taskENTER_CRITICAL()
 method as that will mask interrupts that should exit sleep mode. */
 __asm volatile( "cpsid i" ::: "memory" );
@@ -543,23 +530,44 @@ void xPortSysTickHandler( void )
 to be unsuspended then abandon the low power entry. */
 if( eTaskConfirmSleepModeStatus() == eAbortSleep )
 {
-/* Restart from whatever is left in the count register to complete
-this tick period. */
-portNVIC_SYSTICK_LOAD_REG = portNVIC_SYSTICK_CURRENT_VALUE_REG;
-
-/* Restart SysTick. */
-portNVIC_SYSTICK_CTRL_REG |= portNVIC_SYSTICK_ENABLE_BIT;
-
-/* Reset the reload register to the value required for normal tick
-periods. */
-portNVIC_SYSTICK_LOAD_REG = ulTimerCountsForOneTick - 1UL;
-
 /* Re-enable interrupts - see comments above the cpsid instruction()
 above. */
 __asm volatile( "cpsie i" ::: "memory" );
 }
 else
 {
+/* Stop the SysTick momentarily. The time the SysTick is stopped for
+is accounted for as best it can be, but using the tickless mode will
+inevitably result in some tiny drift of the time maintained by the
+kernel with respect to calendar time. */
+portNVIC_SYSTICK_CTRL_REG = ( portNVIC_SYSTICK_CLK_BIT_SETTING | portNVIC_SYSTICK_INT_BIT );
+
+/* Calculate the reload value required to wait xExpectedIdleTime
+tick periods. -1 is used because this code normally executes part
+way through the first tick period. But if the SysTick IRQ is now
+pending, then clear the IRQ, suppressing the first tick, and correct
+the reload value to reflect that the second tick period is already
+underway. The expected idle time is always at least two ticks. */
+ulReloadValue = portNVIC_SYSTICK_CURRENT_VALUE_REG + ( ulTimerCountsForOneTick * (xExpectedIdleTime - 1UL) );
+if( ( portNVIC_ICSR_REG & portNVIC_PEND_SYSTICK_SET_BIT ) != 0 )
+{
+portNVIC_ICSR_REG = portNVIC_PEND_SYSTICK_CLEAR_BIT;
+
+/* Skip the correction described above if SysTick happened to
+stop on zero. In that case, there are still
+ulTimerCountsForOneTick ticks left in the second tick period,
+not zero, so the value in portNVIC_SYSTICK_CURRENT_VALUE_REG
+already includes the correction normally done here. */
+if( portNVIC_SYSTICK_CURRENT_VALUE_REG != 0 )
+{
+ulReloadValue -= ulTimerCountsForOneTick;
+}
+}
+if( ulReloadValue > ulStoppedTimerCompensation )
+{
+ulReloadValue -= ulStoppedTimerCompensation;
+}
+
 /* Set the new reload value. */
 portNVIC_SYSTICK_LOAD_REG = ulReloadValue;
 
@@ -626,7 +634,8 @@ void xPortSysTickHandler( void )
 
 /* Don't allow a tiny value, or values that have somehow
 underflowed because the post sleep hook did something
-that took too long. */
+that took too long or because the SysTick current-value register
+is zero. */
 if( ( ulCalculatedLoadValue < ulStoppedTimerCompensation ) || ( ulCalculatedLoadValue > ulTimerCountsForOneTick ) )
 {
 ulCalculatedLoadValue = ( ulTimerCountsForOneTick - 1UL );