Re: [PATCH] clocksource: timer-ti-dm : Capture functionality for OMAP DM timer

From: Gokul Praveen
Date: Fri Aug 08 2025 - 03:42:08 EST


Hi Andrew,

On 07/08/25 23:07, Andrew Davis wrote:
On 8/7/25 7:32 AM, Gokul Praveen wrote:
Add PWM capture function in DM timer driver.

OMAP DM timer hardware supports capture feature.It can be used to
timestamp events (falling/rising edges) detected on input signal.

Signed-off-by: Gokul Praveen <g-praveen@xxxxxx>
---
Precondition : Before calling driver API,it is assumed that the
                capture signal is active else both duty cycle
                and period returned by driver will not be valid.
---
  drivers/clocksource/timer-ti-dm.c          | 127 ++++++++++++++++++++-
  include/linux/platform_data/dmtimer-omap.h |   4 +
  2 files changed, 129 insertions(+), 2 deletions(-)

diff --git a/drivers/clocksource/timer-ti-dm.c b/drivers/clocksource/ timer-ti-dm.c
index e9e32df6b566..a4bf72c850b5 100644
--- a/drivers/clocksource/timer-ti-dm.c
+++ b/drivers/clocksource/timer-ti-dm.c
@@ -31,6 +31,7 @@
  #include <linux/platform_data/dmtimer-omap.h>
  #include <clocksource/timer-ti-dm.h>
+#include <linux/delay.h>
  /*
   * timer errata flags
@@ -836,6 +837,49 @@ static int omap_dm_timer_set_match(struct omap_dm_timer *cookie, int enable,
      return 0;
  }
+static int omap_dm_timer_set_cap(struct omap_dm_timer *cookie,
+                    int autoreload, bool config_period)
+{
+    struct dmtimer *timer;
+    struct device *dev;
+    int rc;
+    u32 l;
+
+    timer = to_dmtimer(cookie);
+    if (unlikely(!timer))
+        return -EINVAL;
+
+    dev = &timer->pdev->dev;

Do this assignment up where the var is declared

Actually the timer is initialized after that in the to_dmtimer function.

So wouldn't initializing the 'dev' variable before that cause issues.

+    rc = pm_runtime_resume_and_get(dev);
+

Extra newline, fix this everywhere

+    if (rc)
+        return rc;
+    /*
+     *  1. Select autoreload mode. TIMER_TCLR[1] AR bit.
+     *  2. TIMER_TCLR[14]: Sets the functionality of the TIMER IO pin.
+     *  3. TIMER_TCLR[13] : Capture mode select bit.
+     *  3. TIMER_TCLR[9-8] : Select transition capture mode.
+     */
+
+    l = dmtimer_read(timer, OMAP_TIMER_CTRL_REG);
+
+    if (autoreload)
+        l |= OMAP_TIMER_CTRL_AR;
+
+    l |= OMAP_TIMER_CTRL_CAPTMODE | OMAP_TIMER_CTRL_GPOCFG;
+
+    if (config_period == true)
+        l |= OMAP_TIMER_CTRL_TCM_LOWTOHIGH; /*Time Period config*/

Add space around /* xxx */ in your comments, do this everywhere


Sure Andrew.

+    else
+        l |= OMAP_TIMER_CTRL_TCM_BOTHEDGES; /*Duty Cycle config*/
+
+    dmtimer_write(timer, OMAP_TIMER_CTRL_REG, l);
+
+    pm_runtime_put_sync(dev);
+
+    return 0;
+}
+
  static int omap_dm_timer_set_pwm(struct omap_dm_timer *cookie, int def_on,
                   int toggle, int trigger, int autoreload)
  {
@@ -1023,23 +1067,99 @@ static unsigned int omap_dm_timer_read_counter(struct omap_dm_timer *cookie)
      return __omap_dm_timer_read_counter(timer);
  }
+static inline unsigned int __omap_dm_timer_cap(struct dmtimer *timer, int idx)
+{
+    return idx == 0 ? dmtimer_read(timer, OMAP_TIMER_CAPTURE_REG) :
+              dmtimer_read(timer, OMAP_TIMER_CAPTURE2_REG);
+}
+
  static int omap_dm_timer_write_counter(struct omap_dm_timer *cookie, unsigned int value)
  {
      struct dmtimer *timer;
+    struct device *dev;
      timer = to_dmtimer(cookie);
-    if (unlikely(!timer || !atomic_read(&timer->enabled))) {
-        pr_err("%s: timer not available or enabled.\n", __func__);
+    if (unlikely(!timer)) {
+        pr_err("%s: timer not available.\n", __func__);
          return -EINVAL;
      }
+    dev = &timer->pdev->dev;
+
+    pm_runtime_resume_and_get(dev);
      dmtimer_write(timer, OMAP_TIMER_COUNTER_REG, value);
+    pm_runtime_put_sync(dev);
      /* Save the context */
      timer->context.tcrr = value;
      return 0;
  }
+/**
+ * omap_dm_timer_cap_counter() - Calculate the high count or period count depending on the
+ * configuration.
+ * @cookie:Pointer to OMAP DM timer
+ * @is_period:Whether to configure timer in period or duty cycle mode
+ *
+ * Return high count or period count if timer is enabled else appropriate error.
+ */
+static unsigned int omap_dm_timer_cap_counter(struct omap_dm_timer *cookie,    bool is_period)
+{
+    struct dmtimer *timer;
+    unsigned int cap1 = 0;
+    unsigned int cap2 = 0;
+    u32 l, ret;
+
+    timer = to_dmtimer(cookie);
+    if (unlikely(!timer || !atomic_read(&timer->enabled))) {
+        pr_err("%s:timer is not available or enabled.%p\n", __func__, (void *)timer);
+        return -EINVAL;
+    }
+
+    /*Stop the timer*/
+    omap_dm_timer_stop(cookie);
+
+    /* Clear the timer counter value to 0 */
+    ret = omap_dm_timer_write_counter(cookie, 0);
+
+    if (ret)
+        return ret;
+
+    /*Sets the timer capture configuration for duty cycle calculation*/
+    if (is_period == false)
+        ret = omap_dm_timer_set_cap(cookie, true, false);
+
+    /*Sets the timer capture configuration for period calculation*/
+    else
+        ret = omap_dm_timer_set_cap(cookie, true, true);
+

The above 8 lines could just be:

ret = omap_dm_timer_set_cap(cookie, true, is_period);

Andrew


Sure Andrew.

Regards
Gokul
+    if (ret) {
+        pr_err("%s: Failed to set timer capture configuration.\n", __func__);
+        return ret;
+    }
+    /*Start the timer*/
+    omap_dm_timer_start(cookie);
+
+    /*
+     * 1 sec delay is given so as to provide
+     * enough time to capture low frequency signals.
+     */
+    msleep(1000);
+
+    cap1 = __omap_dm_timer_cap(timer, 0);
+    cap2 = __omap_dm_timer_cap(timer, 1);
+
+    /*
+     *    Clears the TCLR configuration.
+     *  The start bit must be set to 1 as the timer is already in start mode.
+     */
+    l = dmtimer_read(timer, OMAP_TIMER_CTRL_REG);
+    l &= ~(0xffff) | 0x1;
+    dmtimer_write(timer, OMAP_TIMER_CTRL_REG, l);
+
+    return (cap2-cap1);
+}
+
  static int __maybe_unused omap_dm_timer_runtime_suspend(struct device *dev)
  {
      struct dmtimer *timer = dev_get_drvdata(dev);
@@ -1246,6 +1366,9 @@ static const struct omap_dm_timer_ops dmtimer_ops = {
      .write_counter = omap_dm_timer_write_counter,
      .read_status = omap_dm_timer_read_status,
      .write_status = omap_dm_timer_write_status,
+    .set_cap = omap_dm_timer_set_cap,
+    .get_cap_status = omap_dm_timer_get_pwm_status,
+    .read_cap = omap_dm_timer_cap_counter,
  };
  static const struct dmtimer_platform_data omap3plus_pdata = {
diff --git a/include/linux/platform_data/dmtimer-omap.h b/include/ linux/platform_data/dmtimer-omap.h
index 95d852aef130..726d89143842 100644
--- a/include/linux/platform_data/dmtimer-omap.h
+++ b/include/linux/platform_data/dmtimer-omap.h
@@ -36,9 +36,13 @@ struct omap_dm_timer_ops {
      int    (*set_pwm)(struct omap_dm_timer *timer, int def_on,
                 int toggle, int trigger, int autoreload);
      int    (*get_pwm_status)(struct omap_dm_timer *timer);
+    int    (*set_cap)(struct omap_dm_timer *timer,
+               int autoreload, bool config_period);
+    int    (*get_cap_status)(struct omap_dm_timer *timer);
      int    (*set_prescaler)(struct omap_dm_timer *timer, int prescaler);
      unsigned int (*read_counter)(struct omap_dm_timer *timer);
+    unsigned int (*read_cap)(struct omap_dm_timer *timer, bool is_period);
      int    (*write_counter)(struct omap_dm_timer *timer,
                   unsigned int value);
      unsigned int (*read_status)(struct omap_dm_timer *timer);