subject:"Problem with system time in lpc176x bsp"

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Marcos Diaz

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( _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( _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(_context);
+_Timecounter_Tick_simple(
+  interval,
+  (*tc->tc_get_timecount)(tc),
+  _context
+);

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Joel Sherrill

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.d...@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( _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( _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

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Marcos Díaz

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  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.d...@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( _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( _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() \
>>

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Joel Sherrill

On Mon, Jan 11, 2016 at 12:28 PM, Marcos Díaz <
marcos.d...@tallertechnologies.com> wrote:

> 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.
>
>
I would prefer not to ship BSPs with bugs but fixing them comes with risks.
So I would lean to the same fix on the master and 4.11 branch fixing it
where it needs to be fixed.

What BSPs does this change impact?

--joel


>
> On Mon, Jan 11, 2016 at 3:16 PM, Joel Sherrill  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.d...@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( _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( _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(
>>> +

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Marcos Díaz

I made a fast search:
These BSPs use *rtems_timecounter_simple_downcounter*:
arm/shared/armv7m
m68k/mcf52235
m68k/mcf5225x
m68k/mcf5329
m68k/uC5282
powerpc/mpc55xxevb
sparc/erc32
sparc/leon2
sparc/leon3
sparc/shared

These use *rtems_timecounter_simple_upcounter*:
powerpc/mpc55xxevb
arm/lpc22xx

We should search a little more for the other changes but at least these
BSPs will be affected. Maybe we should wait Sebastian's opinion, since he
suggested the change in timecounter's signatures.

On Mon, Jan 11, 2016 at 3:44 PM, Joel Sherrill  wrote:

>
>
> On Mon, Jan 11, 2016 at 12:28 PM, Marcos Díaz <
> marcos.d...@tallertechnologies.com> wrote:
>
>> 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.
>>
>>
> I would prefer not to ship BSPs with bugs but fixing them comes with risks.
> So I would lean to the same fix on the master and 4.11 branch fixing it
> where it needs to be fixed.
>
> What BSPs does this change impact?
>
> --joel
>
>
>>
>> On Mon, Jan 11, 2016 at 3:16 PM, Joel Sherrill  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.d...@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( _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( _context );
 +  return is_pending_local;
  }

  static

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Sebastian Huber

Hello,

I already created a ticket which blocks the 4.11 release for this bug:

https://devel.rtems.org/ticket/2502

I sent a new version of the patch to one of your previous threads some days ago:

https://lists.rtems.org/pipermail/devel/2016-January/013289.html

There is still a bug in it, since we must disable interrupts in the is pending 
function, but the general approach should work.

- Am 11. Jan 2016 um 20:05 schrieb Marcos Díaz 
marcos.d...@tallertechnologies.com:

> I made a fast search:
> These BSPs use *rtems_timecounter_simple_downcounter*:
> arm/shared/armv7m
> m68k/mcf52235
> m68k/mcf5225x
> m68k/mcf5329
> m68k/uC5282
> powerpc/mpc55xxevb
> sparc/erc32
> sparc/leon2
> sparc/leon3
> sparc/shared
> 
> These use *rtems_timecounter_simple_upcounter*:
> powerpc/mpc55xxevb
> arm/lpc22xx
> 
> We should search a little more for the other changes but at least these
> BSPs will be affected. Maybe we should wait Sebastian's opinion, since he
> suggested the change in timecounter's signatures.
> 
> On Mon, Jan 11, 2016 at 3:44 PM, Joel Sherrill  wrote:
> 
>>
>>
>> On Mon, Jan 11, 2016 at 12:28 PM, Marcos Díaz <
>> marcos.d...@tallertechnologies.com> wrote:
>>
>>> 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.
>>>
>>>
>> I would prefer not to ship BSPs with bugs but fixing them comes with risks.
>> So I would lean to the same fix on the master and 4.11 branch fixing it
>> where it needs to be fixed.
>>
>> What BSPs does this change impact?
>>
>> --joel
>>
>>
>>>
>>> On Mon, Jan 11, 2016 at 3:16 PM, Joel Sherrill  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.d...@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

Re: Problem with system time in lpc176x bsp

2016-01-11 Thread Marcos Díaz

Well, we could still use the patch you sent with the protection in is
pending, but I think this will break the BSPs I mentioned, since you
changed timecounter functions signatures. Am I right?

On Mon, Jan 11, 2016 at 4:18 PM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> Hello,
>
> I already created a ticket which blocks the 4.11 release for this bug:
>
> https://devel.rtems.org/ticket/2502
>
> I sent a new version of the patch to one of your previous threads some
> days ago:
>
> https://lists.rtems.org/pipermail/devel/2016-January/013289.html
>
> There is still a bug in it, since we must disable interrupts in the is
> pending function, but the general approach should work.
>
> - Am 11. Jan 2016 um 20:05 schrieb Marcos Díaz
> marcos.d...@tallertechnologies.com:
>
> > I made a fast search:
> > These BSPs use *rtems_timecounter_simple_downcounter*:
> > arm/shared/armv7m
> > m68k/mcf52235
> > m68k/mcf5225x
> > m68k/mcf5329
> > m68k/uC5282
> > powerpc/mpc55xxevb
> > sparc/erc32
> > sparc/leon2
> > sparc/leon3
> > sparc/shared
> >
> > These use *rtems_timecounter_simple_upcounter*:
> > powerpc/mpc55xxevb
> > arm/lpc22xx
> >
> > We should search a little more for the other changes but at least these
> > BSPs will be affected. Maybe we should wait Sebastian's opinion, since he
> > suggested the change in timecounter's signatures.
> >
> > On Mon, Jan 11, 2016 at 3:44 PM, Joel Sherrill  wrote:
> >
> >>
> >>
> >> On Mon, Jan 11, 2016 at 12:28 PM, Marcos Díaz <
> >> marcos.d...@tallertechnologies.com> wrote:
> >>
> >>> 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.
> >>>
> >>>
> >> I would prefer not to ship BSPs with bugs but fixing them comes with
> risks.
> >> So I would lean to the same fix on the master and 4.11 branch fixing it
> >> where it needs to be fixed.
> >>
> >> What BSPs does this change impact?
> >>
> >> --joel
> >>
> >>
> >>>
> >>> On Mon, Jan 11, 2016 at 3:16 PM, Joel Sherrill  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.d...@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
> > +++

Re: Problem with system time in lpc176x bsp

2016-01-04 Thread Daniel Gutson

On Mon, Jan 4, 2016 at 2:04 PM, Joel Sherrill  wrote:

>
>
> On Mon, Jan 4, 2016 at 3:48 AM, Sebastian Huber <
> sebastian.hu...@embedded-brains.de> wrote:
>
>>
>>
>> On 23/12/15 22:22, Marcos Díaz wrote:
>>
>>> That patch didn't work,
>>> one reason is that it has a mistake:
>>> in kern_tc.c you defined this macro:
>>>
>>> #define _Timecounter_Release(lock_context) \
>>> + *_ISR_lock_ISR_disable_and_acquire*(&_Timecounter_Lock, lock_context)
>>>
>>> I think you meant:
>>>
>>> *_ISR_lock_Release_and_ISR_enable*(&_Timecounter_Lock, lock_context)
>>>
>>
>> Its strange, that this didn't lead to failures in Qemu.
>>
>>
> Qemu doesn't do cycle or instruction level simulation.
>

I'm not sure if I fully understand this statement, but FWIW qemu can do
per-instruction emulation, in fact I fixed one 7 years ago
https://lists.nongnu.org/archive/html/qemu-devel/2009-08/msg01153.html


> It does blocks of instructions between potential control flow points.
> Perhaps this is enough to make real hardware and the simulation behave
> differently in this case.
>
> For sure, it reduces the potential race conditions showing up in SMP
> applications since it is alternating blocks of instructions on each
> simulated core.
>
>
>>
>>> The other reason is that as I said, the driver checks for the flag
>>> PENDSTSET of the ICSR register, and I think this approach is wrong, because
>>> that flag goes down when the tick interrupt is attended. I used the
>>> solution I proposed before, but I'm still seeing some errors. I'll let you
>>> know what I find.
>>>
>>
>> Would you mind testing the attached second version of the patch?
>>
>> --
>> Sebastian Huber, embedded brains GmbH
>>
>> Address : Dornierstr. 4, D-82178 Puchheim, Germany
>> Phone   : +49 89 189 47 41-16
>> Fax : +49 89 189 47 41-09
>> E-Mail  : sebastian.hu...@embedded-brains.de
>> PGP : Public key available on request.
>>
>> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>>
>>
>> ___
>> devel mailing list
>> devel@rtems.org
>> http://lists.rtems.org/mailman/listinfo/devel
>>
>
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>



-- 

Daniel F. Gutson
Chief Engineering Officer, SPD

San Lorenzo 47, 3rd Floor, Office 5
Córdoba, Argentina

Phone:   +54 351 4217888 / +54 351 4218211
Skype:dgutson
LinkedIn: http://ar.linkedin.com/in/danielgutson
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

2016-01-04 Thread Sebastian Huber




On 23/12/15 22:22, Marcos Díaz wrote:

That patch didn't work,
one reason is that it has a mistake:
in kern_tc.c you defined this macro:

#define _Timecounter_Release(lock_context) \
+ *_ISR_lock_ISR_disable_and_acquire*(&_Timecounter_Lock, lock_context)

I think you meant:

*_ISR_lock_Release_and_ISR_enable*(&_Timecounter_Lock, lock_context)


Its strange, that this didn't lead to failures in Qemu.



The other reason is that as I said, the driver checks for the flag 
PENDSTSET of the ICSR register, and I think this approach is wrong, 
because that flag goes down when the tick interrupt is attended. I 
used the solution I proposed before, but I'm still seeing some errors. 
I'll let you know what I find.


Would you mind testing the attached second version of the patch?

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

>From fc52281c3c434f66d5765e9282d3c1a98e0b8447 Mon Sep 17 00:00:00 2001
From: Sebastian Huber 
Date: Wed, 23 Dec 2015 07:29:47 +0100
Subject: [PATCH v2] score: Fix simple timecounter support

Close #2502.
---
 .../arm/shared/armv7m/clock/armv7m-clock-config.c  | 45 +++---
 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 | 27 -
 cpukit/score/src/kern_tc.c | 16 
 doc/bsp_howto/clock.t  | 13 ++-
 7 files changed, 125 insertions(+), 33 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..f5bd9ac 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
@@ -23,7 +23,12 @@
 /* This is defined in clockdrv_shell.h */
 static void Clock_isr(void *arg);
 
-static rtems_timecounter_simple _ARMV7M_TC;
+typedef struct {
+  rtems_timecounter_simple base;
+  bool countflag;
+} ARMV7M_Timecounter;
+
+static ARMV7M_Timecounter _ARMV7M_TC;
 
 static uint32_t _ARMV7M_TC_get(rtems_timecounter_simple *tc)
 {
@@ -32,11 +37,19 @@ static uint32_t _ARMV7M_TC_get(rtems_timecounter_simple *tc)
   return systick->cvr;
 }
 
-static bool _ARMV7M_TC_is_pending(rtems_timecounter_simple *tc)
+static bool _ARMV7M_TC_is_pending(rtems_timecounter_simple *base)
 {
-  volatile ARMV7M_SCB *scb = _ARMV7M_SCB;
+  ARMV7M_Timecounter *tc = (ARMV7M_Timecounter *) base;
+  bool countflag = tc->countflag;
+
+  if (!countflag) {
+volatile ARMV7M_Systick *systick = _ARMV7M_Systick;
 
-  return ((scb->icsr & ARMV7M_SCB_ICSR_PENDSTSET) != 0);
+countflag = ((systick->csr & ARMV7M_SYSTICK_CSR_COUNTFLAG) != 0);
+tc->countflag = countflag;
+  }
+
+  return countflag;
 }
 
 static uint32_t _ARMV7M_TC_get_timecount(struct timecounter *tc)
@@ -48,19 +61,26 @@ 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 *base)
 {
+  ARMV7M_Timecounter *tc = (ARMV7M_Timecounter *) base;
   volatile ARMV7M_Systick *systick = _ARMV7M_Systick;
 
+  tc->countflag = false;
+
   /* Clear COUNTFLAG */
   systick->csr;
 }
 
+static void _ARMV7M_TC_tick(void)
+{
+  rtems_timecounter_simple_downcounter_tick(
+&_ARMV7M_TC.base,
+_ARMV7M_TC_get,
+_ARMV7M_TC_at_tick
+  );
+}
+
 static void _ARMV7M_Systick_handler(void)
 {
   _ARMV7M_Interrupt_service_enter();
@@ -95,7 +115,7 @@ static void _ARMV7M_Systick_initialize(void)
 | ARMV7M_SYSTICK_CSR_CLKSOURCE;
 
   rtems_timecounter_simple_install(
-&_ARMV7M_TC,
+&_ARMV7M_TC.base,
 freq,
 interval,
 _ARMV7M_TC_get_timecount
@@ -111,9 +131,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

Re: Problem with system time in lpc176x bsp

2016-01-04 Thread Joel Sherrill

On Mon, Jan 4, 2016 at 3:48 AM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

>
>
> On 23/12/15 22:22, Marcos Díaz wrote:
>
>> That patch didn't work,
>> one reason is that it has a mistake:
>> in kern_tc.c you defined this macro:
>>
>> #define _Timecounter_Release(lock_context) \
>> + *_ISR_lock_ISR_disable_and_acquire*(&_Timecounter_Lock, lock_context)
>>
>> I think you meant:
>>
>> *_ISR_lock_Release_and_ISR_enable*(&_Timecounter_Lock, lock_context)
>>
>
> Its strange, that this didn't lead to failures in Qemu.
>
>
Qemu doesn't do cycle or instruction level simulation. It does blocks of
instructions between potential control flow points. Perhaps this is enough
to make real hardware and the simulation behave differently in this case.

For sure, it reduces the potential race conditions showing up in SMP
applications since it is alternating blocks of instructions on each
simulated core.


>
>> The other reason is that as I said, the driver checks for the flag
>> PENDSTSET of the ICSR register, and I think this approach is wrong, because
>> that flag goes down when the tick interrupt is attended. I used the
>> solution I proposed before, but I'm still seeing some errors. I'll let you
>> know what I find.
>>
>
> Would you mind testing the attached second version of the patch?
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax : +49 89 189 47 41-09
> E-Mail  : sebastian.hu...@embedded-brains.de
> PGP : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
>
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
>
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

2015-12-23 Thread Marcos Díaz

That patch didn't work,
one reason is that it has a mistake:
in kern_tc.c you defined this macro:

#define _Timecounter_Release(lock_context) \
+  *_ISR_lock_ISR_disable_and_acquire*(&_Timecounter_Lock, lock_context)

I think you meant:

 *_ISR_lock_Release_and_ISR_enable*(&_Timecounter_Lock, lock_context)

The other reason is that as I said, the driver checks for the flag PENDSTSET
of the ICSR register, and I think this approach is wrong, because that flag
goes down when the tick interrupt is attended. I used the solution I
proposed before, but I'm still seeing some errors. I'll let you know what I
find.

On Wed, Dec 23, 2015 at 3:53 AM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> Hello Marcos,
>
> yes, nested interrupts are a problem for the current implementation. Its a
> general problem of the RTEMS testsuite that we cannot test nested
> interrupts. I think we have to fix this at some time.
>
> Could you please test the attached patch?
>
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax : +49 89 189 47 41-09
> E-Mail  : sebastian.hu...@embedded-brains.de
> PGP : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
>


-- 

__




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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

2015-12-22 Thread Marcos Díaz

Hi, thanks for answering, but spnsext01 test works ok.
After a while I noticed the problem:
The ticker interrupt has lower priority than any irq. When I entered the
ticker:
 The bit PENDSTSET of the ICSR register that was used to check whether the
sysclk had a pending interrupt or not went to 0,
But since the ticker has lower priority, my GPIO interrupt preempted the
ticker interrupt before updating the binuptime, and asked for the time,
when checking for that bit inside rtems_timecounter_simple_downcounter_get,
it was 0, but the tick time wasn't added correctly to binuptime and the
timer had already reset. So there is the difference I noticed from time to
time.

The solution I propose include two things: One is to put the interrupt
fence in Clock_isr function before calling Clock_driver_support_at_tick, or
make another Clock_isr function, that "atomically" clears the pending flag
(systick->csr) and updates the binuptime.

The other is to change _ARMV7M_TC_is_pending to not check for the mentioned
bit. instead we should check  for the systick->csr register, but since
that register clears after each read we should "help" that flag with
another bool:

static bool is_count_pending = false;
static bool _ARMV7M_TC_is_pending(rtems_timecounter_simple *tc)
{

  volatile ARMV7M_Systick *systick = _ARMV7M_Systick;

  if (!is_count_pending)
  {
is_count_pending = (systick->csr & ARMV7M_SYSTICK_CSR_COUNTFLAG) != 0;
  }

  return is_count_pending;
}

And in the function in charge of clearing that bit we should do this:

static void _ARMV7M_Systick_at_tick(void)
{
  volatile ARMV7M_Systick *systick = _ARMV7M_Systick;

  /* Clear COUNTFLAG */
  systick->csr;

  is_count_pending = false;
}

With this solution I can have any order in priorities between the
interrupts, and it works.
If you approve this I will submit the patch.
I suggest checking for similar problems in other bsps.
Besides this I have another question: What is the best to set for the
ticker priority? It should have a higher/same/lower priority than the other
interrupts?

On Mon, Dec 21, 2015 at 4:39 AM, Sebastian Huber <
sebastian.hu...@embedded-brains.de> wrote:

> Hello,
>
> works the spnsext01 test on your target?
>
> --
> Sebastian Huber, embedded brains GmbH
>
> Address : Dornierstr. 4, D-82178 Puchheim, Germany
> Phone   : +49 89 189 47 41-16
> Fax : +49 89 189 47 41-09
> E-Mail  : sebastian.hu...@embedded-brains.de
> PGP : Public key available on request.
>
> Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
>
>

-- 

__

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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

2015-12-22 Thread Sebastian Huber


Hello Marcos,

yes, nested interrupts are a problem for the current implementation. Its 
a general problem of the RTEMS testsuite that we cannot test nested 
interrupts. I think we have to fix this at some time.


Could you please test the attached patch?

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

>From a963c17ed32f9cc6f423531be4534cf9f21d774b Mon Sep 17 00:00:00 2001
From: Sebastian Huber 
Date: Wed, 23 Dec 2015 07:29:47 +0100
Subject: [PATCH] score: Fix simple timecounter support

Close #2502.
---
 .../arm/shared/armv7m/clock/armv7m-clock-config.c  | 19 ++--
 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 | 27 +++--
 cpukit/score/src/kern_tc.c | 16 +-
 doc/bsp_howto/clock.t  | 13 +++-
 7 files changed, 104 insertions(+), 28 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..d58749c 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
@@ -48,12 +48,7 @@ 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;
 
@@ -61,6 +56,15 @@ static void _ARMV7M_Systick_at_tick(void)
   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 +115,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(_context);
+_Timecounter_Tick_simple(
+  interval,
+  (*tc->tc_get_timecount)(tc),
+  _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( _context );
   _Timecounter_Tick_simple(
 rtems_configuration_get_microseconds_per_tick(),
-0
+0,
+_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

Re: Problem with system time in lpc176x bsp

2015-12-20 Thread Sebastian Huber


Hello,

works the spnsext01 test on your target?

--
Sebastian Huber, embedded brains GmbH

Address : Dornierstr. 4, D-82178 Puchheim, Germany
Phone   : +49 89 189 47 41-16
Fax : +49 89 189 47 41-09
E-Mail  : sebastian.hu...@embedded-brains.de
PGP : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Problem with system time in lpc176x bsp

2015-12-18 Thread Marcos Díaz

Hi, i'm using c++ chrono to measure a steady signal periods (With GPIO
interrupts), And I see that periodically I have errors of +1ms and then of
-1ms. (I discarded GPIOP interrupts problem) I tried using rtems API's
rtems_clock_get_uptime_nanoseconds
and then I saw the same errors.
I'm suspecting of some problem related with interrupts and the updating of
bintime uptime in kern_tc.c related to the simple downcounter ticker this
bsp has.
Has anybody seen something like this in any other bsp?
Thanks!

-- 

__




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
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

2015-12-18 Thread Nick Withers

On Fri, 2015-12-18 at 16:18 -0300, Marcos Díaz wrote:
> Hi, i'm using c++ chrono to measure a steady signal periods (With
> GPIO
> interrupts), And I see that periodically I have errors of +1ms and
> then of
> -1ms. (I discarded GPIOP interrupts problem) I tried using rtems
> API's
> rtems_clock_get_uptime_nanoseconds
> and then I saw the same errors.
> I'm suspecting of some problem related with interrupts and the
> updating of
> bintime uptime in kern_tc.c related to the simple downcounter ticker
> this
> bsp has.
> Has anybody seen something like this in any other bsp?

Yep, this sounds familiar...

See https://devel.rtems.org/ticket/2230 (pre-timecounter changes) and 
https://devel.rtems.org/ticket/2356 (post-).
-- 
Nick Withers

Embedded Systems Programmer
Department of Nuclear Physics, Research School of Physics and Engineering
The Australian National University (CRICOS: 00120C)
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

Problem with system time in lpc176x bsp

Re: Problem with system time in lpc176x bsp

16 matches

Site Navigation

Mail list logo

Footer information