From d44ecbcfbb5f1739956d2d9c0b7787ddd27226c2 Mon Sep 17 00:00:00 2001 From: Stefan Kolb Date: Fri, 27 May 2016 07:51:50 -0600 Subject: [PATCH] =?UTF-8?q?This=20is=20a=20fix=20to=20a=20problem=20in=20t?= =?UTF-8?q?he=20handling=20of=20the=20oneshot=20timer.=20Due=20to=20a=20wr?= =?UTF-8?q?ong=20assumption=20concerning=20the=20behavior=20directly=20aft?= =?UTF-8?q?er=20the=20start=20of=20the=20timer/counter=20the=20function=20?= =?UTF-8?q?sam=5Foneshot=5Fcancel(=E2=80=A6)=20calculates=20the=20wrong=20?= =?UTF-8?q?remaining=20time.=20The=20code=20assumes=20that=20the=20counter?= =?UTF-8?q?=20register=20is=20zero=20directly=20after=20the=20start=20of?= =?UTF-8?q?=20the=20timer,=20but=20this=20is=20not=20true.=20To=20start=20?= =?UTF-8?q?the=20time/counter=20a=20software=20trigger=20is=20invoked,=20t?= =?UTF-8?q?his=20trigger=20starts=20the=20timer/count=20and=20sets=20the?= =?UTF-8?q?=20counter=20register=20to=20zero,=20but=20the=20reset=20of=20t?= =?UTF-8?q?he=20counter=20register=20is=20not=20performed=20instantly.=20A?= =?UTF-8?q?ccording=20to=20the=20datasheet:=20=E2=80=9CThe=20counter=20can?= =?UTF-8?q?=20be=20reset=20by=20a=20trigger.=20In=20this=20case,=20the=20c?= =?UTF-8?q?ounter=20value=20passes=20to=20zero=20on=20the=20next=20valid?= =?UTF-8?q?=20edge=20of=20the=20selected=20clock.=E2=80=9D=20Thus=20the=20?= =?UTF-8?q?counter=20is=20set=20to=20zero=20between=200=20and=20USEC=5FPER?= =?UTF-8?q?=5FTICK=20microseconds=20after=20the=20clock=20was=20started.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In my fix I use the freerun count value to determine if at least one tick passed since the start of the timer and thus if the value of the oneshot counter is correct. I also tried to use the function up_timer_gettime(…) to achieve this but, at least if compiled with no optimization the problem vanishes without using the value of the function, the function call takes too long. Another problem treated in the fix is that if the oneshot timer/counter is canceled, we only know the remaining time with a precision of USEC_PER_TICK microseconds. This means the calculated remaining time is between 0 and USEC_PER_TICK microseconds too long. To fix this I subtract one tick if the calculated remaining time is greater than one tick and otherwise set the remaining time to zero. By doing so the measured times are much more precise as without it. --- arch/arm/src/samv7/Kconfig | 1 + arch/arm/src/samv7/sam_oneshot.c | 67 +++++++++++++++++++++++++------ arch/arm/src/samv7/sam_oneshot.h | 20 +++++++-- arch/arm/src/samv7/sam_tickless.c | 6 +-- 4 files changed, 75 insertions(+), 19 deletions(-) diff --git a/arch/arm/src/samv7/Kconfig b/arch/arm/src/samv7/Kconfig index 6cb41c03047..7a7d75c8408 100644 --- a/arch/arm/src/samv7/Kconfig +++ b/arch/arm/src/samv7/Kconfig @@ -1514,6 +1514,7 @@ endif # SAMV7_TC3 config SAMV7_ONESHOT bool "TC one-shot wrapper" + depends on SAMV7_FREERUN default n if !SCHED_TICKLESS default y if SCHED_TICKLESS ---help--- diff --git a/arch/arm/src/samv7/sam_oneshot.c b/arch/arm/src/samv7/sam_oneshot.c index 34fa89843e3..76c4ee1a41b 100644 --- a/arch/arm/src/samv7/sam_oneshot.c +++ b/arch/arm/src/samv7/sam_oneshot.c @@ -60,6 +60,7 @@ #include #include "sam_oneshot.h" +#include "sam_freerun.h" #ifdef CONFIG_SAMV7_ONESHOT @@ -124,10 +125,11 @@ static void sam_oneshot_handler(TC_HANDLE tch, void *arg, uint32_t sr) /* Forward the event, clearing out any vestiges */ - oneshot_handler = (oneshot_handler_t)oneshot->handler; - oneshot->handler = NULL; - oneshot_arg = (void *)oneshot->arg; - oneshot->arg = NULL; + oneshot_handler = (oneshot_handler_t)oneshot->handler; + oneshot->handler = NULL; + oneshot_arg = (void *)oneshot->arg; + oneshot->arg = NULL; + oneshot->start_count = 0; oneshot_handler(oneshot_arg); } @@ -224,10 +226,11 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan, * success. */ - oneshot->chan = chan; - oneshot->running = false; - oneshot->handler = NULL; - oneshot->arg = NULL; + oneshot->chan = chan; + oneshot->running = false; + oneshot->handler = NULL; + oneshot->arg = NULL; + oneshot->start_count = 0; return OK; } @@ -251,8 +254,8 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan, * ****************************************************************************/ -int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, - void *arg, const struct timespec *ts) +int sam_oneshot_start(struct sam_oneshot_s *oneshot, struct sam_freerun_s *freerun, + oneshot_handler_t handler, void *arg, const struct timespec *ts) { uint64_t usec; uint64_t regval; @@ -270,7 +273,7 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, /* Yes.. then cancel it */ tcvdbg("Already running... cancelling\n"); - (void)sam_oneshot_cancel(oneshot, NULL); + (void)sam_oneshot_cancel(oneshot, freerun, NULL); } /* Save the new handler and its argument */ @@ -309,6 +312,26 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, sam_tc_start(oneshot->tch); + /* The function sam_tc_start() starts the timer/counter by setting the + * bits TC_CCR_CLKEN and TC_CCR_SWTRG in the channel control register. + * The first one enables the timer/counter the latter performs an + * software trigger, which starts the clock and sets the counter + * register to zero. This reset is performed with the next valid edge + * of the selected clock. Thus it can take up USEC_PER_TICK microseconds + * until the counter register becomes zero. + * + * If the timer is canceled within this period the counter register holds + * the counter value for the last timer/counter run. To circumvent this + * the counter value of the freerun timer/counter is stored at each start + * of the oneshot timer/counter. + * + * The function up_timer_gettime() could also be used for this but it takes + * too long. If up_timer_gettime() is called within this function the problem + * vanishes at least if compiled with no optimisation. + */ + + oneshot->start_count = sam_tc_getcounter(freerun->tch); + /* Enable interrupts. We should get the callback when the interrupt * occurs. */ @@ -343,7 +366,8 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, * ****************************************************************************/ -int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts) +int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct sam_freerun_s *freerun, + struct timespec *ts) { irqstate_t flags; uint64_t usec; @@ -384,6 +408,17 @@ int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts) count = sam_tc_getcounter(oneshot->tch); rc = sam_tc_getregister(oneshot->tch, TC_REGC); + /* In the case the timer/counter was canceled very short after its start, + * the counter register can hold the wrong value (the value of the last + * run). To prevent this the counter value is set to zero if not at + * least on tick passed since the start of the timer/counter. + */ + + if (count > 0 && sam_tc_getcounter(freerun->tch) == oneshot->start_count) + { + count = 0; + } + /* Now we can disable the interrupt and stop the timer. */ sam_tc_attach(oneshot->tch, NULL, NULL, 0); @@ -432,6 +467,14 @@ int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts) usec = (((uint64_t)(rc - count)) * USEC_PER_SEC) / sam_tc_divfreq(oneshot->tch); + /* Each time the timer/counter is canceled the time calculated from + * the two registers (counter and REGC) is accurate up to an error + * between 0 and USEC_PER_TICK microseconds. To correct this error + * one tick which means USEC_PER_TICK microseconds are subtracted. + */ + + usec = usec > USEC_PER_TICK ? usec - USEC_PER_TICK : 0; + /* Return the time remaining in the correct form */ sec = usec / USEC_PER_SEC; diff --git a/arch/arm/src/samv7/sam_oneshot.h b/arch/arm/src/samv7/sam_oneshot.h index 62efde8047d..25fb414c144 100644 --- a/arch/arm/src/samv7/sam_oneshot.h +++ b/arch/arm/src/samv7/sam_oneshot.h @@ -46,7 +46,7 @@ #include #include "sam_tc.h" - +#include "sam_freerun.h" #ifdef CONFIG_SAMV7_ONESHOT /**************************************************************************** @@ -82,6 +82,11 @@ struct sam_oneshot_s volatile oneshot_handler_t handler; /* Oneshot expiration callback */ volatile void *arg; /* The argument that will accompany * the callback */ + volatile uint32_t start_count; /* Stores the value of the freerun counter, + * at each start of the onshot timer. Is neccesary + * to find out if the onshot counter was updated + * correctly at the time of the call to + * sam_oneshot_cancel or not. */ }; /**************************************************************************** @@ -157,6 +162,9 @@ int sam_oneshot_max_delay(struct sam_oneshot_s *oneshot, uint64_t *usec); * oneshot Caller allocated instance of the oneshot state structure. This * structure must have been previously initialized via a call to * sam_oneshot_initialize(); + * freerun Caller allocated instance of the freerun state structure. This + * structure must have been previously initialized via a call to + * sam_freerun_initialize(); * handler The function to call when when the oneshot timer expires. * arg An opaque argument that will accompany the callback. * ts Provides the duration of the one shot timer. @@ -167,8 +175,8 @@ int sam_oneshot_max_delay(struct sam_oneshot_s *oneshot, uint64_t *usec); * ****************************************************************************/ -int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, - void *arg, const struct timespec *ts); +int sam_oneshot_start(struct sam_oneshot_s *oneshot, struct sam_freerun_s *freerun, + oneshot_handler_t handler, void *arg, const struct timespec *ts); /**************************************************************************** * Name: sam_oneshot_cancel @@ -183,6 +191,9 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, * oneshot Caller allocated instance of the oneshot state structure. This * structure must have been previously initialized via a call to * sam_oneshot_initialize(); + * freerun Caller allocated instance of the freerun state structure. This + * structure must have been previously initialized via a call to + * sam_freerun_initialize(); * ts The location in which to return the time remaining on the * oneshot timer. A time of zero is returned if the timer is * not running. @@ -194,7 +205,8 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler, * ****************************************************************************/ -int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts); +int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct sam_freerun_s *freerun, + struct timespec *ts); #undef EXTERN #ifdef __cplusplus diff --git a/arch/arm/src/samv7/sam_tickless.c b/arch/arm/src/samv7/sam_tickless.c index 33cbc948c1b..3d1a7ad95ac 100644 --- a/arch/arm/src/samv7/sam_tickless.c +++ b/arch/arm/src/samv7/sam_tickless.c @@ -363,8 +363,8 @@ int up_timer_gettime(FAR struct timespec *ts) int up_timer_cancel(FAR struct timespec *ts) { - return ONESHOT_INITIALIZED(&g_tickless.oneshot) ? - sam_oneshot_cancel(&g_tickless.oneshot, ts) : + return ONESHOT_INITIALIZED(&g_tickless.oneshot) && FREERUN_INITIALIZED(&g_tickless.freerun) ? + sam_oneshot_cancel(&g_tickless.oneshot, &g_tickless.freerun, ts) : -EAGAIN; } @@ -396,7 +396,7 @@ int up_timer_cancel(FAR struct timespec *ts) int up_timer_start(FAR const struct timespec *ts) { return ONESHOT_INITIALIZED(&g_tickless.oneshot) ? - sam_oneshot_start(&g_tickless.oneshot, sam_oneshot_handler, NULL, ts) : + sam_oneshot_start(&g_tickless.oneshot, &g_tickless.freerun, sam_oneshot_handler, NULL, ts) : -EAGAIN; } #endif /* CONFIG_SCHED_TICKLESS */