Problem with system time in lpc176x bsp
Marcos Díaz
marcos.diaz at tallertechnologies.com
Mon Jan 11 18:28:05 UTC 2016
I will issue a ticket. But I noticed that in my patch I include changes to
common code that Sebastian suggested, and this will break any other BSP
that uses rtems timecounter simple downcounter or upcounter, since these
function's signatures changed. Should we update all BSPs? Or make changes
more locally to our BSP? Meanwhile please do not apply the patch I sent.
On Mon, Jan 11, 2016 at 3:16 PM, Joel Sherrill <joel at rtems.org> wrote:
> At this point, a ticket is needed for anything applied to 4.11 that is not
> release
> mechanics related.
>
> --joel
>
> On Mon, Jan 11, 2016 at 11:47 AM, Marcos Diaz <
> marcos.diaz at tallertechnologies.com> wrote:
>
>> 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
>>
>> _______________________________________________
>> devel mailing list
>> devel at rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>>
>
>
--
______________________________
<http://www.tallertechnologies.com>
Marcos Díaz
Software Engineer
San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina
Phone: +54 351 4217888 / +54 351 4218211/ +54 351 7617452
Skype: markdiaz22
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20160111/a50f45cd/attachment-0001.html>
More information about the devel
mailing list