Re: [PATCH v2 1/3] coresight: tpda: add sysfs nodes for tpda cross-trigger configuration

Jie Gan <jie.gan@xxxxxxxxxxxxxxxx> · Wed, 27 Aug 2025 17:18:37 +0800

On 8/27/2025 5:06 PM, James Clark wrote:


On 27/08/2025 5:20 am, Jie Gan wrote:
From: Tao Zhang <tao.zhang@xxxxxxxxxxxxxxxx>

Introduce sysfs nodes to configure cross-trigger parameters for TPDA.
These registers define the characteristics of cross-trigger packets,
including generation frequency and flag values.

Signed-off-by: Tao Zhang <tao.zhang@xxxxxxxxxxxxxxxx>
Co-developed-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
Signed-off-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
---
  .../testing/sysfs-bus-coresight-devices-tpda  |  43 ++++
  drivers/hwtracing/coresight/coresight-tpda.c  | 227 ++++++++++++++++++
  drivers/hwtracing/coresight/coresight-tpda.h  |  27 ++-
  3 files changed, 296 insertions(+), 1 deletion(-)
  create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight- 
devices-tpda

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- 
tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
new file mode 100644
index 000000000000..fb651aebeb31
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
@@ -0,0 +1,43 @@
+What:        /sys/bus/coresight/devices/<tpda-name>/trig_async_enable
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Enable/disable cross trigger synchronization sequence 
interface.
+
+What:        /sys/bus/coresight/devices/<tpda-name>/trig_flag_ts_enable
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Enable/disable cross trigger FLAG packet request interface.
+
+What:        /sys/bus/coresight/devices/<tpda-name>/trig_freq_enable
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Enable/disable cross trigger FREQ packet request interface.
+
+What:        /sys/bus/coresight/devices/<tpda-name>/freq_ts_enable
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Enable/disable the timestamp for all FREQ packets.
+
+What:        /sys/bus/coresight/devices/<tpda-name>/global_flush_req
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Set global (all ports) flush request bit. The bit 
remains set until a
+        global flush request sequence completes.
+
+What:        /sys/bus/coresight/devices/<tpda-name>/cmbchan_mode
+Date:        August 2025
+KernelVersion:    6.17
+Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
<tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
+Description:
+        (RW) Configure the CMB/MCMB channel mode for all enabled ports.
+        Value 0 means raw channel mapping mode. Value 1 means channel 
pair marking mode.
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/ 
hwtracing/coresight/coresight-tpda.c
index 4e93fa5bace4..647ab49a98d7 100644
--- a/drivers/hwtracing/coresight/coresight-tpda.c
+++ b/drivers/hwtracing/coresight/coresight-tpda.c
@@ -156,9 +156,37 @@ static void tpda_enable_pre_port(struct 
tpda_drvdata *drvdata)
      u32 val;
      val = readl_relaxed(drvdata->base + TPDA_CR);
+    val &= ~TPDA_CR_MID;
      val &= ~TPDA_CR_ATID;
      val |= FIELD_PREP(TPDA_CR_ATID, drvdata->atid);
+    if (drvdata->trig_async)
+        val |= TPDA_CR_SRIE;
+    else
+        val &= ~TPDA_CR_SRIE;
+    if (drvdata->trig_flag_ts)
+        val |= TPDA_CR_FLRIE;
+    else
+        val &= ~TPDA_CR_FLRIE;
+    if (drvdata->trig_freq)
+        val |= TPDA_CR_FRIE;
+    else
+        val &= ~TPDA_CR_FRIE;
+    if (drvdata->freq_ts)
+        val |= TPDA_CR_FREQTS;
+    else
+        val &= ~TPDA_CR_FREQTS;
+    if (drvdata->cmbchan_mode)
+        val |= TPDA_CR_CMBCHANMODE;
+    else
+        val &= ~TPDA_CR_CMBCHANMODE;
      writel_relaxed(val, drvdata->base + TPDA_CR);
+
+    /*
+     * If FLRIE bit is set, set the master and channel
+     * id as zero
+     */
+    if (drvdata->trig_flag_ts)
+        writel_relaxed(0x0, drvdata->base + TPDA_FPID_CR);
  }
  static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
@@ -274,6 +302,203 @@ static const struct coresight_ops tpda_cs_ops = {
      .link_ops    = &tpda_link_ops,
  };
+static ssize_t trig_async_enable_show(struct device *dev,
+                      struct device_attribute *attr,
+                      char *buf)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+    return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_async);
+}
+
+static ssize_t trig_async_enable_store(struct device *dev,
+                       struct device_attribute *attr,
+                       const char *buf,
+                       size_t size)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (kstrtoul(buf, 0, &val))
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    drvdata->trig_async = !!val;
+
+    return size;
+}
+static DEVICE_ATTR_RW(trig_async_enable);
+
+static ssize_t trig_flag_ts_enable_show(struct device *dev,
+                    struct device_attribute *attr,
+                    char *buf)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+    return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_flag_ts);
+}
+
+static ssize_t trig_flag_ts_enable_store(struct device *dev,
+                     struct device_attribute *attr,
+                     const char *buf,
+                     size_t size)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (kstrtoul(buf, 0, &val))
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    drvdata->trig_flag_ts = !!val;
+
+    return size;
+}
+static DEVICE_ATTR_RW(trig_flag_ts_enable);
+
+static ssize_t trig_freq_enable_show(struct device *dev,
+                     struct device_attribute *attr,
+                     char *buf)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+    return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_freq);
+}
+
+static ssize_t trig_freq_enable_store(struct device *dev,
+                      struct device_attribute *attr,
+                      const char *buf,
+                      size_t size)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (kstrtoul(buf, 0, &val))
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    drvdata->trig_freq = !!val;
+
+    return size;
+}
+static DEVICE_ATTR_RW(trig_freq_enable);
+
+static ssize_t freq_ts_enable_show(struct device *dev,
+                   struct device_attribute *attr,
+                   char *buf)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+    return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->freq_ts);
+}
+
+static ssize_t freq_ts_enable_store(struct device *dev,
+                    struct device_attribute *attr,
+                    const char *buf,
+                    size_t size)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (kstrtoul(buf, 0, &val))
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    drvdata->freq_ts = !!val;
+
+    return size;
+}
+static DEVICE_ATTR_RW(freq_ts_enable);
+
+static ssize_t global_flush_req_show(struct device *dev,
+                     struct device_attribute *attr,
+                     char *buf)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (!drvdata->csdev->refcnt)
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    CS_UNLOCK(drvdata->base);
+    val = readl_relaxed(drvdata->base + TPDA_CR);
+    CS_LOCK(drvdata->base);
+
+    return sysfs_emit(buf, "%lx\n", val);

I know in practice it's probably only either 0 or 1, but this should 
either be decimal or have the 0x prefix otherwise it looks like a mistake.


You are right. It looks strange here and I missed this point. I think 
display the value in decimal would be better.

+}
+
+static ssize_t global_flush_req_store(struct device *dev,
+                      struct device_attribute *attr,
+                      const char *buf,
+                      size_t size)
+{
+    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long val;
+
+    if (kstrtoul(buf, 0, &val))
+        return -EINVAL;
+
+    if (!drvdata->csdev->refcnt || !val)
+        return -EINVAL;
+
+    guard(spinlock)(&drvdata->spinlock);
+    CS_UNLOCK(drvdata->base);
+    val = readl_relaxed(drvdata->base + TPDA_CR);
+    val |= BIT(0);

If you only set bit 0 do you only want to show bit 0 in 
global_flush_req_show() above? The sysfs files should be divided up by 
function rather than dumping the whole register, otherwise tools need 
their own copy of the fields to interperet them.

Got your point here. In the show function, we only need read the value 
of the bit 0 and display the value.

Thanks,
Jie











