SAMA5 Tickless: Corrects some logic errors with timer/counter frequency

8 files changed, 163 insertions, 76 deletions

diff --git a/nuttx/arch/arm/src/sama5/chip/sam_tc.h b/nuttx/arch/arm/src/sama5/chip/sam_tc.h
index 46943023e..25d75ff13 100644
--- a/nuttx/arch/arm/src/sama5/chip/sam_tc.h
+++ b/nuttx/arch/arm/src/sama5/chip/sam_tc.h
@@ -414,6 +414,7 @@
 
 #define TC_CMR_TCCLKS_SHIFT      (0)       /* Bits 0-2: Clock Selection */
 #define TC_CMR_TCCLKS_MASK       (7 << TC_CMR_TCCLKS_SHIFT)
+#  define TC_CMR_TCCLKS(n)       ((uint32_t)(n) << TC_CMR_TCCLKS_SHIFT)
 #  define TC_CMR_TCCLKS_TCLK1    (0 << TC_CMR_TCCLKS_SHIFT) /* TIMER_CLOCK1 Clock selected */
 #  define TC_CMR_TCCLKS_TCLK2    (1 << TC_CMR_TCCLKS_SHIFT) /* TIMER_CLOCK2 Clock selected */
 #  define TC_CMR_TCCLKS_TCLK3    (2 << TC_CMR_TCCLKS_SHIFT) /* TIMER_CLOCK3 Clock selected */
diff --git a/nuttx/arch/arm/src/sama5/sam_adc.c b/nuttx/arch/arm/src/sama5/sam_adc.c
index 90e8e8ddb..34abbe7a0 100644
--- a/nuttx/arch/arm/src/sama5/sam_adc.c
+++ b/nuttx/arch/arm/src/sama5/sam_adc.c
@@ -1229,7 +1229,6 @@ static int sam_adc_ioctl(struct adc_dev_s *dev, int cmd, unsigned long arg)
 static int sam_adc_settimer(struct sam_adc_s *priv, uint32_t frequency,
                             int channel)
 {
-  uint32_t ftc;
   uint32_t div;
   uint32_t tcclks;
   uint32_t mode;
@@ -1281,7 +1280,7 @@ static int sam_adc_settimer(struct sam_adc_s *priv, uint32_t frequency,
    * frequency.
    */
 
-  regval = sam_tc_frequency() / fdiv;
+  regval = sam_tc_infreq() / fdiv;
 
   /* Set up TC_RA and TC_RC.  The frequency is determined by RA and RC:  TIOA is
    * cleared on RA match; TIOA is set on RC match.
diff --git a/nuttx/arch/arm/src/sama5/sam_freerun.c b/nuttx/arch/arm/src/sama5/sam_freerun.c
index f49bed2f5..94cd7cb9d 100644
--- a/nuttx/arch/arm/src/sama5/sam_freerun.c
+++ b/nuttx/arch/arm/src/sama5/sam_freerun.c
@@ -196,9 +196,9 @@ int sam_freerun_initialize(struct sam_freerun_s *freerun, int chan,
    * success.
    */
 
-  freerun->chan       = chan;
-  freerun->running    = false;
-  freerun->resolution = resolution;
+  freerun->chan     = chan;
+  freerun->running  = false;
+  freerun->overflow = 0;
 
   /* Set up to receive the callback when the counter overflow occurs */
 
@@ -208,7 +208,6 @@ int sam_freerun_initialize(struct sam_freerun_s *freerun, int chan,
   /* Start the counter */
 
   sam_tc_start(freerun->tch);
-
   return OK;
 }
 
@@ -273,10 +272,15 @@ int sam_freerun_counter(struct sam_freerun_s *freerun, struct timespec *ts)
              (unsigned long)counter, (unsigned long)overflow);
     }
 
-  /* Convert the whole thing to units of microseconds */
+  /* Convert the whole thing to units of microseconds.
+   *
+   *   frequency = ticks / second
+   *   seconds   = ticks * frequency
+   *   usecs     = (ticks * 1000) / frequency;
+   */
 
-  usec = (((uint64_t)overflow << 32) + (uint64_t)counter) *
-         freerun->resolution;
+  usec = ((((uint64_t)overflow << 32) + (uint64_t)counter) * 1000) /
+         sam_tc_divfreq(freerun->tch);
 
   /* And return the value of the timer */
 
diff --git a/nuttx/arch/arm/src/sama5/sam_freerun.h b/nuttx/arch/arm/src/sama5/sam_freerun.h
index a68865378..ce4e78340 100644
--- a/nuttx/arch/arm/src/sama5/sam_freerun.h
+++ b/nuttx/arch/arm/src/sama5/sam_freerun.h
@@ -67,7 +67,6 @@ struct sam_freerun_s
 {
   uint8_t chan;              /* The timer/counter in use */
   bool running;              /* True: the timer is running */
-  uint16_t resolution;       /* Timer resolution in microseconds */
   uint16_t overflow;         /* Timer counter overflow */
   TC_HANDLE tch;             /* Handle returned by sam_tc_initialize() */
 };
diff --git a/nuttx/arch/arm/src/sama5/sam_oneshot.c b/nuttx/arch/arm/src/sama5/sam_oneshot.c
index 4366194a4..45e70d7aa 100644
--- a/nuttx/arch/arm/src/sama5/sam_oneshot.c
+++ b/nuttx/arch/arm/src/sama5/sam_oneshot.c
@@ -122,7 +122,7 @@ static void sam_oneshot_handler(TC_HANDLE tch, void *arg, uint32_t sr)
 
   /* Forward the event, clearing out any vestiges */
 
-  oneshot_handler  = (struct sam_oneshot_s *)oneshot->handler;
+  oneshot_handler  = (oneshot_handler_t)oneshot->handler;
   oneshot->handler = NULL;
   oneshot_arg      = (void *)oneshot->arg;
   oneshot->arg     = NULL;
@@ -158,6 +158,7 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan,
                            uint16_t resolution)
 {
   uint32_t frequency;
+  uint32_t divisor;
   uint32_t cmr;
   int ret;
 
@@ -170,7 +171,7 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan,
 
   /* The pre-calculate values to use when we start the timer */
 
-  ret = sam_tc_divisor(frequency, &oneshot->divisor, &cmr);
+  ret = sam_tc_divisor(frequency, &divisor, &cmr);
   if (ret < 0)
     {
       tcdbg("ERROR: sam_tc_divisor failed: %d\n", ret);
@@ -178,7 +179,7 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan,
     }
 
   tcvdbg("frequency=%lu, divisor=%lu, cmr=%08lx\n",
-         (unsigned long)frequency, (unsigned long)oneshot->divisor,
+         (unsigned long)frequency, (unsigned long)divisor,
          (unsigned long)cmr);
 
   /* Allocate the timer/counter and select its mode of operation
@@ -223,7 +224,6 @@ int sam_oneshot_initialize(struct sam_oneshot_s *oneshot, int chan,
 
   oneshot->chan       = chan;
   oneshot->running    = false;
-  oneshot->resolution = resolution;
   oneshot->handler    = NULL;
   oneshot->arg        = NULL;
   return OK;
@@ -275,13 +275,18 @@ int sam_oneshot_start(struct sam_oneshot_s *oneshot, oneshot_handler_t handler,
   oneshot->handler = handler;
   oneshot->arg     = arg;
 
-  /* We configured the counter to run with an LSB of the specified
-   * resolution.  We now must need need to set RC to the number
-   * of resolution units corresponding to the requested delay.
+  /* Express the delay in microseconds */
+
+  usec = (uint64_t)ts->tv_sec * 1000000 + (uint64_t)(ts->tv_nsec / 1000);
+
+  /* Get the timer counter frequency and determine the number of counts need to achieve the requested delay.
+   *
+   *   frequency = ticks / second
+   *   ticks     = seconds * frequency
+   *             = (usecs * frequency) / 1000000;
    */
 
-  usec   = (uint64_t)ts->tv_sec * 1000000 + (uint64_t)(ts->tv_nsec / 1000);
-  regval = usec / oneshot->resolution;
+  regval = (usec * (uint64_t)sam_tc_divfreq(oneshot->tch)) / 1000000;
 
   tcvdbg("usec=%lu regval=%08lx\n",
          (unsigned long)usec, (unsigned long)regval);
@@ -339,10 +344,11 @@ 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)
 {
   irqstate_t flags;
+  uint64_t usec;
+  uint64_t sec;
+  uint64_t nsec;
   uint32_t count;
   uint32_t rc;
-  uint32_t usec;
-  uint32_t sec;
 
   /* Was the timer running? */
 
@@ -364,9 +370,11 @@ int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts)
    * the counter expires while we are doing this, the counter clock will be
    * stopped, but the clock will not be disabled.
    *
-   * NOTE: This is not documented, but I have observed in this case that the
-   * counter register freezes at a value equal to the RC register.  The
-   * following logic depends on this fact.
+   * The expected behavior is that the the counter register will freezes at
+   * a value equal to the RC register when the timer expires.  The counter
+   * should have values between 0 and RC in all other cased.
+   *
+   * REVISIT:  This does not appear to be the case.
    */
 
   tcvdbg("Cancelling...\n");
@@ -374,14 +382,8 @@ 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);
 
-  /* Now we can disable the interrupt and stop the timer.
-   *
-   * REVISIT: The assertion is there because I do no not know if the
-   * counter will be reset when the RC match occurs.  The counter
-   * clock will be disabled, so I am hoping not.
-   */
+  /* Now we can disable the interrupt and stop the timer. */
 
-  DEBUGASSERT(count > 0 || (sam_tc_getpending(oneshot->tch) & TC_INT_CPCS) == 0);
   sam_tc_attach(oneshot->tch, NULL, NULL, 0);
   sam_tc_stop(oneshot->tch);
 
@@ -390,22 +392,53 @@ int sam_oneshot_cancel(struct sam_oneshot_s *oneshot, struct timespec *ts)
   oneshot->arg     = NULL;
   irqrestore(flags);
 
-  /* The total time remaining is the difference */
+  /* Did the caller provide us with a location to return the time
+   * remaining?
+   */
 
-  DEBUGASSERT(rc >= count);
   if (ts)
     {
-      usec = (rc - count) * oneshot->resolution;
+      /* Yes.. then calculate and return the time remaining on the
+       * oneshot timer.
+       */
 
       tcvdbg("rc=%lu count=%lu resolution=%u usec=%lu\n",
              (unsigned long)rc, (unsigned long)count, oneshot->resolution,
              (unsigned long)usec);
 
-      /* Return the time remaining in the correct form */
+      /* REVISIT: I am not certain why the timer counter value sometimes
+       * exceeds RC.  Might be a bug, or perhaps the counter does not stop
+       * in all cases.
+       */
 
-      sec         = usec / 1000000;
-      ts->tv_sec  = sec;
-      ts->tv_nsec = ((usec) - (sec * 1000000)) * 1000;
+      if (count >= rc)
+        {
+          /* No time remaining (?) */
+
+          ts->tv_sec  = 0;
+          ts->tv_nsec = 0;
+        }
+      else
+        {
+          /* The total time remaining is the difference.  Convert the that
+           * to units of microseconds.
+           *
+           *   frequency = ticks / second
+           *   seconds   = ticks * frequency
+           *   usecs     = (ticks * 1000) / frequency;
+           */
+
+          usec        = (((uint64_t)(rc - count)) * 1000) /
+                        sam_tc_divfreq(oneshot->tch);
+
+          /* Return the time remaining in the correct form */
+
+          sec         = usec / 1000000;
+          nsec        = ((usec) - (sec * 1000000)) * 1000;
+
+          ts->tv_sec  = (time_t)sec;
+          ts->tv_nsec = (unsigned long)nsec;
+        }
 
       tcvdbg("remaining (%lu, %lu)\n",
              (unsigned long)ts->tv_sec, (unsigned long)ts->tv_nsec);
diff --git a/nuttx/arch/arm/src/sama5/sam_oneshot.h b/nuttx/arch/arm/src/sama5/sam_oneshot.h
index 144e95eb4..5ed433788 100644
--- a/nuttx/arch/arm/src/sama5/sam_oneshot.h
+++ b/nuttx/arch/arm/src/sama5/sam_oneshot.h
@@ -75,8 +75,6 @@ struct sam_oneshot_s
 {
   uint8_t chan;                       /* The timer/counter in use */
   volatile bool running;              /* True: the timer is running */
-  uint16_t resolution;                /* Timer resolution in microseconds */
-  uint32_t divisor;                   /* TC divisor derived from resolution */
   TC_HANDLE tch;                      /* Handle returned by
                                        * sam_tc_initialize() */
   volatile oneshot_handler_t handler; /* Oneshot expiration callback */
diff --git a/nuttx/arch/arm/src/sama5/sam_tc.c b/nuttx/arch/arm/src/sama5/sam_tc.c
index 64697d29b..8f3bda083 100644
--- a/nuttx/arch/arm/src/sama5/sam_tc.c
+++ b/nuttx/arch/arm/src/sama5/sam_tc.c
@@ -184,8 +184,8 @@ static int sam_tc678_interrupt(int irq, void *context);
 #ifdef SAMA5_HAVE_PMC_PCR_DIV
 static int sam_tc_mckdivider(uint32_t mck);
 #endif
-static int sam_tc_freqdiv(uint32_t ftc, int ndx);
-static uint32_t sam_tc_divfreq(uint32_t ftc, int ndx);
+static int sam_tc_freqdiv_lookup(uint32_t ftcin, int ndx);
+static uint32_t sam_tc_divfreq_lookup(uint32_t ftcin, int ndx);
 static inline struct sam_chan_s *sam_tc_initialize(int channel);
 
 /****************************************************************************
@@ -822,28 +822,28 @@ static int sam_tc_mckdivider(uint32_t mck)
 #endif
 
 /****************************************************************************
- * Name: sam_tc_freqdiv
+ * Name: sam_tc_freqdiv_lookup
  *
  * Description:
- *  Given the TC input frequency (Ftc) and a divider index, return the value of
- *  the Ftc divider.
+ *  Given the TC input frequency (Ftcin) and a divider index, return the value of
+ *  the Ftcin divider.
  *
  * Input Parameters:
- *   ftc - TC input frequency
- *   ndx - Divider index
+ *   ftcin - TC input frequency
+ *   ndx   - Divider index
  *
  * Returned Value:
- *   The ftc input divider value
+ *   The Ftcin input divider value
  *
  ****************************************************************************/
 
-static int sam_tc_freqdiv(uint32_t ftc, int ndx)
+static int sam_tc_freqdiv_lookup(uint32_t ftcin, int ndx)
 {
   /* The final option is to use the SLOW clock */
 
   if (ndx >= TC_NDIVIDERS)
     {
-      return ftc / BOARD_SLOWCLK_FREQUENCY;
+      return ftcin / BOARD_SLOWCLK_FREQUENCY;
     }
   else
     {
@@ -852,22 +852,22 @@ static int sam_tc_freqdiv(uint32_t ftc, int ndx)
 }
 
 /****************************************************************************
- * Name: sam_tc_divfreq
+ * Name: sam_tc_divfreq_lookup
  *
  * Description:
- *  Given the TC input frequency (Ftc) and a divider index, return the value of
+ *  Given the TC input frequency (Ftcin) and a divider index, return the value of
  *  the divided frequency
  *
  * Input Parameters:
- *   ftc - TC input frequency
- *   ndx - Divider index
+ *   ftcin - TC input frequency
+ *   ndx   - Divider index
  *
  * Returned Value:
  *   The divided frequency value
  *
  ****************************************************************************/
 
-static uint32_t sam_tc_divfreq(uint32_t ftc, int ndx)
+static uint32_t sam_tc_divfreq_lookup(uint32_t ftcin, int ndx)
 {
   /* The final option is to use the SLOW clock */
 
@@ -877,7 +877,7 @@ static uint32_t sam_tc_divfreq(uint32_t ftc, int ndx)
     }
   else
     {
-      return ftc >> g_log2divider[ndx];
+      return ftcin >> g_log2divider[ndx];
     }
 }
 
@@ -1359,10 +1359,10 @@ uint32_t sam_tc_getcounter(TC_HANDLE handle)
 }
 
 /****************************************************************************
- * Name: sam_tc_frequency
+ * Name: sam_tc_infreq
  *
  * Description:
- *   Return the timer input frequency (Ftc), that is, the MCK frequency
+ *   Return the timer input frequency (Ftcin), that is, the MCK frequency
  *   divided down so that the timer/counter is driven within its maximum
  *   frequency.
  *
@@ -1374,7 +1374,7 @@ uint32_t sam_tc_getcounter(TC_HANDLE handle)
  *
  ****************************************************************************/
 
-uint32_t sam_tc_frequency(void)
+uint32_t sam_tc_infreq(void)
 {
 #ifdef SAMA5_HAVE_PMC_PCR_DIV
   uint32_t mck = BOARD_MCK_FREQUENCY;
@@ -1386,18 +1386,54 @@ uint32_t sam_tc_frequency(void)
 }
 
 /****************************************************************************
+ * Name: sam_tc_divfreq
+ *
+ * Description:
+ *   Return the divided timer input frequency that is currently driving the
+ *   the timer counter.
+ *
+ * Input Parameters:
+ *   handle Channel handle previously allocated by sam_tc_allocate()
+ *
+ * Returned Value:
+ *  The timer counter frequency.
+ *
+ ****************************************************************************/
+
+uint32_t sam_tc_divfreq(TC_HANDLE handle)
+{
+  struct sam_chan_s *chan = (struct sam_chan_s *)handle;
+  uint32_t ftcin = sam_tc_infreq();
+  uint32_t regval;
+  int tcclks;
+
+  DEBUGASSERT(chan);
+
+  /* Get the the TC_CMR register contents for this channel and extract the
+   * TCCLKS index.
+   */
+
+  regval = sam_chan_getreg(chan, SAM_TC_CMR_OFFSET);
+  tcclks = (regval & TC_CMR_TCCLKS_MASK) >> TC_CMR_TCCLKS_SHIFT;
+
+  /* And use the TCCLKS index to calculate the timer counter frequency */
+
+  return sam_tc_divfreq_lookup(ftcin, tcclks);
+}
+
+/****************************************************************************
  * Name: sam_tc_divisor
  *
  * Description:
  *   Finds the best MCK divisor given the timer frequency and MCK.  The
  *   result is guaranteed to satisfy the following equation:
  *
- *     (Ftc / (div * 65536)) <= freq <= (Ftc / dev)
+ *     (Ftcin / (div * 65536)) <= freq <= (Ftcin / dev)
  *
  *   where:
- *     freq - the desired frequency
- *     Ftc  - The timer/counter input frequency
- *     div  - With DIV being the highest possible value.
+ *     freq  - the desired frequency
+ *     Ftcin - The timer/counter input frequency
+ *     div   - With DIV being the highest possible value.
  *
  * Input Parameters:
  *   frequency  Desired timer frequency.
@@ -1412,17 +1448,17 @@ uint32_t sam_tc_frequency(void)
 
 int sam_tc_divisor(uint32_t frequency, uint32_t *div, uint32_t *tcclks)
 {
-  uint32_t ftc = sam_tc_frequency();
+  uint32_t ftcin = sam_tc_infreq();
   int ndx = 0;
 
   tcvdbg("frequency=%d\n", frequency);
 
   /* Satisfy lower bound.  That is, the value of the divider such that:
    *
-   *   frequency >= tc_input_frequency / divider.
+   *   frequency >= (tc_input_frequency * 65536) / divider.
    */
 
-  while (frequency < (sam_tc_divfreq(ftc, ndx) >> 16))
+  while (frequency < (sam_tc_divfreq_lookup(ftcin, ndx) >> 16))
     {
       if (++ndx > TC_NDIVOPTIONS)
         {
@@ -1441,7 +1477,7 @@ int sam_tc_divisor(uint32_t frequency, uint32_t *div, uint32_t *tcclks)
 
   for (; ndx < (TC_NDIVOPTIONS-1); ndx++)
     {
-      if (frequency > sam_tc_divfreq(ftc, ndx + 1))
+      if (frequency > sam_tc_divfreq_lookup(ftcin, ndx + 1))
         {
           break;
         }
@@ -1451,7 +1487,7 @@ int sam_tc_divisor(uint32_t frequency, uint32_t *div, uint32_t *tcclks)
 
   if (div)
     {
-      uint32_t value = sam_tc_freqdiv(ftc, ndx);
+      uint32_t value = sam_tc_freqdiv_lookup(ftcin, ndx);
       tcvdbg("return div=%lu\n", (unsigned long)value);
       *div = value;
     }
@@ -1460,8 +1496,8 @@ int sam_tc_divisor(uint32_t frequency, uint32_t *div, uint32_t *tcclks)
 
   if (tcclks)
     {
-      tcvdbg("return tcclks=%d\n", ndx);
-      *tcclks = ndx;
+      tcvdbg("return tcclks=%08lx\n", (unsigned long)TC_CMR_TCCLKS(ndx));
+      *tcclks = TC_CMR_TCCLKS(ndx);
     }
 
   return OK;
diff --git a/nuttx/arch/arm/src/sama5/sam_tc.h b/nuttx/arch/arm/src/sama5/sam_tc.h
index 174f491a3..46681ce32 100644
--- a/nuttx/arch/arm/src/sama5/sam_tc.h
+++ b/nuttx/arch/arm/src/sama5/sam_tc.h
@@ -297,7 +297,7 @@ uint32_t sam_tc_getregister(TC_HANDLE handle, int regid);
 uint32_t sam_tc_getcounter(TC_HANDLE handle);
 
 /****************************************************************************
- * Name: sam_tc_frequency
+ * Name: sam_tc_infreq
  *
  * Description:
  *   Return the timer input frequency, that is, the MCK frequency divided
@@ -311,7 +311,24 @@ uint32_t sam_tc_getcounter(TC_HANDLE handle);
  *
  ****************************************************************************/
 
-uint32_t sam_tc_frequency(void);
+uint32_t sam_tc_infreq(void);
+
+/****************************************************************************
+ * Name: sam_tc_divfreq
+ *
+ * Description:
+ *   Return the divided timer input frequency that is currently driving the
+ *   the timer counter.
+ *
+ * Input Parameters:
+ *   handle Channel handle previously allocated by sam_tc_allocate()
+ *
+ * Returned Value:
+ *  The timer counter frequency.
+ *
+ ****************************************************************************/
+
+uint32_t sam_tc_divfreq(TC_HANDLE handle);
 
 /****************************************************************************
  * Name: sam_tc_divisor
@@ -320,12 +337,12 @@ uint32_t sam_tc_frequency(void);
  *   Finds the best MCK divisor given the timer frequency and MCK.  The
  *   result is guaranteed to satisfy the following equation:
  *
- *     (Ftc / (div * 65536)) <= freq <= (Ftc / div)
+ *     (Ftcin / (div * 65536)) <= freq <= (Ftcin / div)
  *
  *   where:
- *     freq - the desired frequency
- *     Ftc  - The timer/counter input frequency
- *     div  - With DIV being the highest possible value.
+ *     freq  - the desired frequency
+ *     Ftcin - The timer/counter input frequency
+ *     div   - With DIV being the highest possible value.
  *
  * Input Parameters:
  *   frequency  Desired timer frequency.