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 <sebastian.hu...@embedded-brains.de>
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 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..db0a7ee 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 void 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..33de269 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,21 @@ 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 Performs a simple timecounter tick.
  *
  * This is a special purpose tick function for simple timecounter to support
@@ -169,8 +186,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 d8f086d..1db8d65 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 bcopy(x, y, z) memcpy(y, x, z);
@@ -1384,7 +1386,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__ */
 
 	/*
@@ -1539,7 +1541,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__ */
 }
 
@@ -1964,14 +1966,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;
@@ -1998,7 +1998,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 )
-- 
1.8.4.5

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

Reply via email to