Problem with system time in lpc176x bsp
Marcos Diaz
marcos.diaz at tallertechnologies.com
Mon Jan 11 17:47:01 UTC 2016
Hi Sebastian,
we did some more testing and found out what's causing the problem. Based on what
I posted at https://lists.rtems.org/pipermail/devel/2015-December/013235.html,
the problem arises when the ticker interrupt occurs while a task is executing
one of the instructions that make up the following line:
is_count_pending = (systick->csr & ARMV7M_SYSTICK_CSR_COUNTFLAG) != 0;
which compiling with -O3 are:
ldr.w r3, [r9]
ubfx r5, r3, #16, #1
strb.w r5, [r8, #64]
If the SysTick counts to 0 and the ldr executes before the interrupt occurs,
R3 will have the CSR.COUNTFLAG bit set. Even though _ARMV7M_TC_at_tick will
clear the is_count_pending boolean, the task will still see it as true.
The solution we found is to simply disable interrupts while reading that flag.
We also use a local variable inside _ARMV7M_TC_is_pending instead of directly
returning the current value of the is_count_pending global, just in case.
Here's the patch that fixes it; this applies to the 4.11 branch.
Do we have to open a new thread for it to be applied to the mainline, or will
this suffice? Should we open a ticket for 4.11?
---
.../arm/shared/armv7m/clock/armv7m-clock-config.c | 38 +++++++++++++++-------
c/src/lib/libbsp/shared/clockdrv_shell.h | 16 ++++++++-
cpukit/rtems/src/clocktick.c | 6 +++-
cpukit/sapi/include/rtems/timecounter.h | 35 ++++++++++++++++----
cpukit/score/include/rtems/score/timecounter.h | 37 +++++++++++++++++++--
cpukit/score/src/kern_tc.c | 16 ++++-----
doc/bsp_howto/clock.t | 13 +++++++-
7 files changed, 130 insertions(+), 31 deletions(-)
diff --git a/c/src/lib/libbsp/arm/shared/armv7m/clock/armv7m-clock-config.c b/c/src/lib/libbsp/arm/shared/armv7m/clock/armv7m-clock-config.c
index e78684c..53bd7cf 100644
--- a/c/src/lib/libbsp/arm/shared/armv7m/clock/armv7m-clock-config.c
+++ b/c/src/lib/libbsp/arm/shared/armv7m/clock/armv7m-clock-config.c
@@ -25,6 +25,8 @@ static void Clock_isr(void *arg);
static rtems_timecounter_simple _ARMV7M_TC;
+static bool is_count_pending = false;
+
static uint32_t _ARMV7M_TC_get(rtems_timecounter_simple *tc)
{
volatile ARMV7M_Systick *systick = _ARMV7M_Systick;
@@ -34,9 +36,20 @@ static uint32_t _ARMV7M_TC_get(rtems_timecounter_simple *tc)
static bool _ARMV7M_TC_is_pending(rtems_timecounter_simple *tc)
{
- volatile ARMV7M_SCB *scb = _ARMV7M_SCB;
+ volatile ARMV7M_Systick *systick = _ARMV7M_Systick;
+ ISR_lock_Context lock_context;
+
+ _Timecounter_Acquire( &lock_context );
+ /* We use this bool since the hardware flag is reset each time it is read. */
+ if (!is_count_pending)
+ {
+ is_count_pending = ((systick->csr & ARMV7M_SYSTICK_CSR_COUNTFLAG) != 0 );
+ }
- return ((scb->icsr & ARMV7M_SCB_ICSR_PENDSTSET) != 0);
+ const bool is_pending_local = is_count_pending;
+
+ _Timecounter_Release( &lock_context );
+ return is_pending_local;
}
static uint32_t _ARMV7M_TC_get_timecount(struct timecounter *tc)
@@ -48,19 +61,23 @@ static uint32_t _ARMV7M_TC_get_timecount(struct timecounter *tc)
);
}
-static void _ARMV7M_TC_tick(void)
-{
- rtems_timecounter_simple_downcounter_tick(&_ARMV7M_TC, _ARMV7M_TC_get);
-}
-
-static void _ARMV7M_Systick_at_tick(void)
+static void _ARMV7M_TC_at_tick(rtems_timecounter_simple *tc)
{
volatile ARMV7M_Systick *systick = _ARMV7M_Systick;
-
+ is_count_pending = false;
/* Clear COUNTFLAG */
systick->csr;
}
+static void _ARMV7M_TC_tick(void)
+{
+ rtems_timecounter_simple_downcounter_tick(
+ &_ARMV7M_TC,
+ _ARMV7M_TC_get,
+ _ARMV7M_TC_at_tick
+ );
+}
+
static void _ARMV7M_Systick_handler(void)
{
_ARMV7M_Interrupt_service_enter();
@@ -111,9 +128,6 @@ static void _ARMV7M_Systick_cleanup(void)
#define Clock_driver_timecounter_tick() _ARMV7M_TC_tick()
-#define Clock_driver_support_at_tick() \
- _ARMV7M_Systick_at_tick()
-
#define Clock_driver_support_initialize_hardware() \
_ARMV7M_Systick_initialize()
diff --git a/c/src/lib/libbsp/shared/clockdrv_shell.h b/c/src/lib/libbsp/shared/clockdrv_shell.h
index d546fb8..96b962f 100644
--- a/c/src/lib/libbsp/shared/clockdrv_shell.h
+++ b/c/src/lib/libbsp/shared/clockdrv_shell.h
@@ -44,6 +44,13 @@
#define Clock_driver_support_find_timer()
#endif
+/**
+ * @brief Do nothing by default.
+ */
+#ifndef Clock_driver_support_at_tick
+ #define Clock_driver_support_at_tick()
+#endif
+
/*
* A specialized clock driver may use for example rtems_timecounter_tick_simple()
* instead of the default.
@@ -108,7 +115,14 @@ rtems_isr Clock_isr(
&& _Thread_Executing->Start.entry_point
== (Thread_Entry) rtems_configuration_get_idle_task()
) {
- _Timecounter_Tick_simple(interval, (*tc->tc_get_timecount)(tc));
+ ISR_lock_Context lock_context;
+
+ _Timecounter_Acquire(&lock_context);
+ _Timecounter_Tick_simple(
+ interval,
+ (*tc->tc_get_timecount)(tc),
+ &lock_context
+ );
}
Clock_driver_support_at_tick();
diff --git a/cpukit/rtems/src/clocktick.c b/cpukit/rtems/src/clocktick.c
index e2cd35f..a6bf26d 100644
--- a/cpukit/rtems/src/clocktick.c
+++ b/cpukit/rtems/src/clocktick.c
@@ -23,9 +23,13 @@
rtems_status_code rtems_clock_tick( void )
{
+ ISR_lock_Context lock_context;
+
+ _Timecounter_Acquire( &lock_context );
_Timecounter_Tick_simple(
rtems_configuration_get_microseconds_per_tick(),
- 0
+ 0,
+ &lock_context
);
return RTEMS_SUCCESSFUL;
diff --git a/cpukit/sapi/include/rtems/timecounter.h b/cpukit/sapi/include/rtems/timecounter.h
index 04bc534..06b3973 100644
--- a/cpukit/sapi/include/rtems/timecounter.h
+++ b/cpukit/sapi/include/rtems/timecounter.h
@@ -94,6 +94,13 @@ typedef struct {
} rtems_timecounter_simple;
/**
+ * @brief At tick handling done under protection of the timecounter lock.
+ */
+typedef uint32_t rtems_timecounter_simple_at_tick(
+ rtems_timecounter_simple *tc
+);
+
+/**
* @brief Returns the current value of a simple timecounter.
*/
typedef uint32_t rtems_timecounter_simple_get(
@@ -199,20 +206,28 @@ RTEMS_INLINE_ROUTINE uint32_t rtems_timecounter_simple_scale(
*
* @param[in] tc The simple timecounter.
* @param[in] get The method to get the value of the simple timecounter.
+ * @param[in] at_tick The method to perform work under timecounter lock
+ * protection at this tick, e.g. clear a pending flag.
*/
RTEMS_INLINE_ROUTINE void rtems_timecounter_simple_downcounter_tick(
- rtems_timecounter_simple *tc,
- rtems_timecounter_simple_get get
+ rtems_timecounter_simple *tc,
+ rtems_timecounter_simple_get get,
+ rtems_timecounter_simple_at_tick at_tick
)
{
+ ISR_lock_Context lock_context;
uint32_t current;
+ _Timecounter_Acquire( &lock_context );
+
+ ( *at_tick )( tc );
+
current = rtems_timecounter_simple_scale(
tc,
tc->real_interval - ( *get )( tc )
);
- _Timecounter_Tick_simple( tc->binary_interval, current );
+ _Timecounter_Tick_simple( tc->binary_interval, current, &lock_context );
}
/**
@@ -220,17 +235,25 @@ RTEMS_INLINE_ROUTINE void rtems_timecounter_simple_downcounter_tick(
*
* @param[in] tc The simple timecounter.
* @param[in] get The method to get the value of the simple timecounter.
+ * @param[in] at_tick The method to perform work under timecounter lock
+ * protection at this tick, e.g. clear a pending flag.
*/
RTEMS_INLINE_ROUTINE void rtems_timecounter_simple_upcounter_tick(
- rtems_timecounter_simple *tc,
- rtems_timecounter_simple_get get
+ rtems_timecounter_simple *tc,
+ rtems_timecounter_simple_get get,
+ rtems_timecounter_simple_at_tick at_tick
)
{
+ ISR_lock_Context lock_context;
uint32_t current;
+ _Timecounter_Acquire( &lock_context );
+
+ ( *at_tick )( tc );
+
current = rtems_timecounter_simple_scale( tc, ( *get )( tc ) );
- _Timecounter_Tick_simple( tc->binary_interval, current );
+ _Timecounter_Tick_simple( tc->binary_interval, current, &lock_context );
}
/**
diff --git a/cpukit/score/include/rtems/score/timecounter.h b/cpukit/score/include/rtems/score/timecounter.h
index 0d17cc7..0e611b5 100644
--- a/cpukit/score/include/rtems/score/timecounter.h
+++ b/cpukit/score/include/rtems/score/timecounter.h
@@ -26,6 +26,8 @@
#include <sys/time.h>
#include <sys/timetc.h>
+#include <rtems/score/isrlock.h>
+
#ifdef __cplusplus
extern "C" {
#endif /* __cplusplus */
@@ -161,6 +163,31 @@ void _Timecounter_Install( struct timecounter *tc );
void _Timecounter_Tick( void );
/**
+ * @brief Lock to protect the timecounter mechanic.
+ */
+ISR_LOCK_DECLARE( extern, _Timecounter_Lock )
+
+/**
+ * @brief Acquires the timecounter lock.
+ *
+ * @param[in] lock_context The lock context.
+ *
+ * See _Timecounter_Tick_simple().
+ */
+#define _Timecounter_Acquire( lock_context ) \
+ _ISR_lock_ISR_disable_and_acquire( &_Timecounter_Lock, lock_context )
+
+/**
+ * @brief Releases the timecounter lock.
+ *
+ * @param[in] lock_context The lock context.
+ *
+ * See _Timecounter_Tick_simple().
+ */
+#define _Timecounter_Release(lock_context) \
+ _ISR_lock_Release_and_ISR_enable(&_Timecounter_Lock, lock_context)
+
+/**
* @brief Performs a simple timecounter tick.
*
* This is a special purpose tick function for simple timecounter to support
@@ -169,8 +196,14 @@ void _Timecounter_Tick( void );
* @param[in] delta The time in timecounter ticks elapsed since the last call
* to _Timecounter_Tick_simple().
* @param[in] offset The current value of the timecounter.
- */
-void _Timecounter_Tick_simple( uint32_t delta, uint32_t offset );
+ * @param[in] lock_context The lock context of the corresponding
+ * _Timecounter_Acquire().
+ */
+void _Timecounter_Tick_simple(
+ uint32_t delta,
+ uint32_t offset,
+ ISR_lock_Context *lock_context
+);
/**
* @brief The wall clock time in seconds.
diff --git a/cpukit/score/src/kern_tc.c b/cpukit/score/src/kern_tc.c
index f6a1136..4257533 100644
--- a/cpukit/score/src/kern_tc.c
+++ b/cpukit/score/src/kern_tc.c
@@ -64,7 +64,9 @@ __FBSDID("$FreeBSD r284178 2015-06-09T11:49:56Z$");
#ifdef __rtems__
#include <limits.h>
#include <rtems.h>
-ISR_LOCK_DEFINE(static, _Timecounter_Lock, "Timecounter");
+ISR_LOCK_DEFINE(, _Timecounter_Lock, "Timecounter")
+#define _Timecounter_Release(lock_context) \
+ _ISR_lock_Release_and_ISR_enable(&_Timecounter_Lock, lock_context)
#define hz rtems_clock_get_ticks_per_second()
#define printf(...)
#define log(...)
@@ -1383,7 +1385,7 @@ tc_windup(void)
#ifdef __rtems__
ISR_lock_Context lock_context;
- _ISR_lock_ISR_disable_and_acquire(&_Timecounter_Lock, &lock_context);
+ _Timecounter_Acquire(&lock_context);
#endif /* __rtems__ */
/*
@@ -1538,7 +1540,7 @@ tc_windup(void)
timekeep_push_vdso();
#endif /* __rtems__ */
#ifdef __rtems__
- _ISR_lock_Release_and_ISR_enable(&_Timecounter_Lock, &lock_context);
+ _Timecounter_Release(&lock_context);
#endif /* __rtems__ */
}
@@ -1963,14 +1965,12 @@ _Timecounter_Tick(void)
}
#ifdef __rtems__
void
-_Timecounter_Tick_simple(uint32_t delta, uint32_t offset)
+_Timecounter_Tick_simple(uint32_t delta, uint32_t offset,
+ ISR_lock_Context *lock_context)
{
struct bintime bt;
struct timehands *th;
uint32_t ogen;
- ISR_lock_Context lock_context;
-
- _ISR_lock_ISR_disable_and_acquire(&_Timecounter_Lock, &lock_context);
th = timehands;
ogen = th->th_generation;
@@ -1997,7 +1997,7 @@ _Timecounter_Tick_simple(uint32_t delta, uint32_t offset)
time_second = th->th_microtime.tv_sec;
time_uptime = th->th_offset.sec;
- _ISR_lock_Release_and_ISR_enable(&_Timecounter_Lock, &lock_context);
+ _Timecounter_Release(lock_context);
_Watchdog_Tick();
}
diff --git a/doc/bsp_howto/clock.t b/doc/bsp_howto/clock.t
index f58b898..2891bb2 100644
--- a/doc/bsp_howto/clock.t
+++ b/doc/bsp_howto/clock.t
@@ -89,6 +89,13 @@ static uint32_t some_tc_get( rtems_timecounter_simple *tc )
return some.counter;
@}
+static void some_tc_at_tick( rtems_timecounter_simple *tc )
+@{
+ /*
+ * Do work necessary at the clock tick interrupt, e.g. clear a pending flag.
+ */
+@}
+
static bool some_tc_is_pending( rtems_timecounter_simple *tc )
@{
return some.is_pending;
@@ -105,7 +112,11 @@ static uint32_t some_tc_get_timecount( struct timecounter *tc )
static void some_tc_tick( void )
@{
- rtems_timecounter_simple_downcounter_tick( &some_tc, some_tc_get );
+ rtems_timecounter_simple_downcounter_tick(
+ &some_tc,
+ some_tc_get,
+ some_tc_at_tick
+ );
@}
static void some_support_initialize_hardware( void )
--
2.7.0
More information about the devel
mailing list