Skip to content

Commit 0c7b04b

Browse files
jefftenneyjefftenney
committed
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.
1 parent bac101c commit 0c7b04b

File tree

1 file changed

+36
-27
lines changed

1 file changed

+36
-27
lines changed

‎portable/GCC/ARM_CM4F/port.c

Lines changed: 36 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,13 @@
5252
#define portNVIC_SYSTICK_LOAD_REG ( * ( ( volatile uint32_t * ) 0xe000e014 ) )
5353
#define portNVIC_SYSTICK_CURRENT_VALUE_REG ( * ( ( volatile uint32_t * ) 0xe000e018 ) )
5454
#define portNVIC_SYSPRI2_REG ( * ( ( volatile uint32_t * ) 0xe000ed20 ) )
55+
#define portNVIC_ICSR_REG ( * ( ( volatile uint32_t * ) 0xe000ed04 ) )
5556
/* ...then bits in the registers. */
5657
#define portNVIC_SYSTICK_INT_BIT ( 1UL << 1UL )
5758
#define portNVIC_SYSTICK_ENABLE_BIT ( 1UL << 0UL )
5859
#define portNVIC_SYSTICK_COUNT_FLAG_BIT ( 1UL << 16UL )
5960
#define portNVIC_PENDSVCLEAR_BIT ( 1UL << 27UL )
61+
#define portNVIC_PEND_SYSTICK_SET_BIT ( 1UL << 26UL )
6062
#define portNVIC_PEND_SYSTICK_CLEAR_BIT ( 1UL << 25UL )
6163

6264
/* Constants used to detect a Cortex-M7 r0p1 core, which should use the ARM_CM7
@@ -518,21 +520,6 @@ void xPortSysTickHandler( void )
518520
xExpectedIdleTime = xMaximumPossibleSuppressedTicks;
519521
}
520522

521-
/* Stop the SysTick momentarily. The time the SysTick is stopped for
522-
is accounted for as best it can be, but using the tickless mode will
523-
inevitably result in some tiny drift of the time maintained by the
524-
kernel with respect to calendar time. */
525-
portNVIC_SYSTICK_CTRL_REG &= ~portNVIC_SYSTICK_ENABLE_BIT;
526-
527-
/* Calculate the reload value required to wait xExpectedIdleTime
528-
tick periods. -1 is used because this code will execute part way
529-
through one of the tick periods. */
530-
ulReloadValue = portNVIC_SYSTICK_CURRENT_VALUE_REG + ( ulTimerCountsForOneTick * ( xExpectedIdleTime - 1UL ) );
531-
if( ulReloadValue > ulStoppedTimerCompensation )
532-
{
533-
ulReloadValue -= ulStoppedTimerCompensation;
534-
}
535-
536523
/* Enter a critical section but don't use the taskENTER_CRITICAL()
537524
method as that will mask interrupts that should exit sleep mode. */
538525
__asm volatile( "cpsid i" ::: "memory" );
@@ -543,23 +530,44 @@ void xPortSysTickHandler( void )
543530
to be unsuspended then abandon the low power entry. */
544531
if( eTaskConfirmSleepModeStatus() == eAbortSleep )
545532
{
546-
/* Restart from whatever is left in the count register to complete
547-
this tick period. */
548-
portNVIC_SYSTICK_LOAD_REG = portNVIC_SYSTICK_CURRENT_VALUE_REG;
549-
550-
/* Restart SysTick. */
551-
portNVIC_SYSTICK_CTRL_REG |= portNVIC_SYSTICK_ENABLE_BIT;
552-
553-
/* Reset the reload register to the value required for normal tick
554-
periods. */
555-
portNVIC_SYSTICK_LOAD_REG = ulTimerCountsForOneTick - 1UL;
556-
557533
/* Re-enable interrupts - see comments above the cpsid instruction()
558534
above. */
559535
__asm volatile( "cpsie i" ::: "memory" );
560536
}
561537
else
562538
{
539+
/* Stop the SysTick momentarily. The time the SysTick is stopped for
540+
is accounted for as best it can be, but using the tickless mode will
541+
inevitably result in some tiny drift of the time maintained by the
542+
kernel with respect to calendar time. */
543+
portNVIC_SYSTICK_CTRL_REG = ( portNVIC_SYSTICK_CLK_BIT_SETTING | portNVIC_SYSTICK_INT_BIT );
544+
545+
/* Calculate the reload value required to wait xExpectedIdleTime
546+
tick periods. -1 is used because this code normally executes part
547+
way through the first tick period. But if the SysTick IRQ is now
548+
pending, then clear the IRQ, suppressing the first tick, and correct
549+
the reload value to reflect that the second tick period is already
550+
underway. The expected idle time is always at least two ticks. */
551+
ulReloadValue = portNVIC_SYSTICK_CURRENT_VALUE_REG + ( ulTimerCountsForOneTick * (xExpectedIdleTime - 1UL) );
552+
if( ( portNVIC_ICSR_REG & portNVIC_PEND_SYSTICK_SET_BIT ) != 0 )
553+
{
554+
portNVIC_ICSR_REG = portNVIC_PEND_SYSTICK_CLEAR_BIT;
555+
556+
/* Skip the correction described above if SysTick happened to
557+
stop on zero. In that case, there are still
558+
ulTimerCountsForOneTick ticks left in the second tick period,
559+
not zero, so the value in portNVIC_SYSTICK_CURRENT_VALUE_REG
560+
already includes the correction normally done here. */
561+
if( portNVIC_SYSTICK_CURRENT_VALUE_REG != 0 )
562+
{
563+
ulReloadValue -= ulTimerCountsForOneTick;
564+
}
565+
}
566+
if( ulReloadValue > ulStoppedTimerCompensation )
567+
{
568+
ulReloadValue -= ulStoppedTimerCompensation;
569+
}
570+
563571
/* Set the new reload value. */
564572
portNVIC_SYSTICK_LOAD_REG = ulReloadValue;
565573

@@ -626,7 +634,8 @@ void xPortSysTickHandler( void )
626634

627635
/* Don't allow a tiny value, or values that have somehow
628636
underflowed because the post sleep hook did something
629-
that took too long. */
637+
that took too long or because the SysTick current-value register
638+
is zero. */
630639
if( ( ulCalculatedLoadValue < ulStoppedTimerCompensation ) || ( ulCalculatedLoadValue > ulTimerCountsForOneTick ) )
631640
{
632641
ulCalculatedLoadValue = ( ulTimerCountsForOneTick - 1UL );

0 commit comments

Comments
 (0)