Re: [PATCH v4 08/11] coresight-tpdm: Add nodes to configure pattern match output

From: Tao Zhang
Date: Fri Jun 02 2023 - 04:30:41 EST



On 6/1/2023 9:28 PM, Suzuki K Poulose wrote:
On 27/04/2023 10:00, Tao Zhang wrote:
Add nodes to configure trigger pattern and trigger pattern mask.
Each DSB subunit TPDM has maximum of n(n<7) XPR registers to
configure trigger pattern match output. Eight 32 bit registers
providing DSB interface trigger output pattern match comparison.
And each DSB subunit TPDM has maximum of m(m<7) XPMR registers to
configure trigger pattern mask match output. Eight 32 bit
registers providing DSB interface trigger output pattern match
mask.

Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
---
  .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 30 ++++++++
  drivers/hwtracing/coresight/coresight-tpdm.c       | 85 ++++++++++++++++++++++
  drivers/hwtracing/coresight/coresight-tpdm.h       |  8 ++
  3 files changed, 123 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index a57f000..c04c735 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -92,3 +92,33 @@ Description:
          <integer1> : Start EDCMR register number
          <integer2> : End EDCMR register number
          <integer3> : The value need to be written
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt_val
+Date:        March 2023
+KernelVersion    6.3
+Contact:    Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang (QUIC) <quic_taozha@xxxxxxxxxxx>
+Description:
+        (Write) Set the trigger pattern value of DSB tpdm.
+        Read the trigger pattern value of DSB tpdm.
+
+        Expected format is the following:
+        <integer1> <integer2>
+
+        Where:
+        <integer1> : Index number of XPR register, the range is 0 to 7
+        <integer2> : The value need to be written

I assume the values written to the registers are not special and doesn't
have meaning and thus need not be documented ?
Sure, I will update this in the next patch series.

+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_patt_mask
+Date:        March 2023
+KernelVersion    6.3

Same as the previous one, 6.5 please
Sure, I will update this in the next patch series.

+Contact:    Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang (QUIC) <quic_taozha@xxxxxxxxxxx>
+Description:
+        (Write) Set the trigger pattern mask of DSB tpdm.
+        Read the trigger pattern mask of DSB tpdm.
+
+        Expected format is the following:
+        <integer1> <integer2>
+
+        Where:
+        <integer1> : Index number of XPMR register,  the range is 0 to 7
+        <integer2> : The value need to be written
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
index a40e458..9387bdf 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -89,6 +89,13 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata)
          writel_relaxed(drvdata->dsb->edge_ctrl_mask[i],
                 drvdata->base + TPDM_DSB_EDCMR(i));
  +    for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {

Same as the previous, can we safely assume that write to these
registers won't trigger an Error if not impelemented ?

Yes, it won't trigger an error since these inexistent register's addresses are not occupied and safe

for being accessed.


+ writel_relaxed(drvdata->dsb->trig_patt_val[i],
+                drvdata->base + TPDM_DSB_XPR(i));
+        writel_relaxed(drvdata->dsb->trig_patt_mask[i],
+                drvdata->base + TPDM_DSB_XPMR(i));
+    }
+
      val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);
      /* Set trigger timestamp */
      if (drvdata->dsb->trig_ts)
@@ -444,6 +451,82 @@ static ssize_t dsb_edge_ctrl_mask_store(struct device *dev,
  }
  static DEVICE_ATTR_RW(dsb_edge_ctrl_mask);
  +static ssize_t dsb_trig_patt_val_show(struct device *dev,
+                       struct device_attribute *attr,
+                       char *buf)
+{
+    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    ssize_t size = 0;
+    int i = 0;
+
+    spin_lock(&drvdata->spinlock);
+    for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+        size += sysfs_emit_at(buf, size,
+                  "Index: 0x%x Value: 0x%x\n", i,
+                  drvdata->dsb->trig_patt_val[i]);

Please detect the return of 0 and break. Same below.

See my comments in patch #7 mail.


Best,

Tao



+    }
+    spin_unlock(&drvdata->spinlock);
+    return size;
+}
+
+static ssize_t dsb_trig_patt_val_store(struct device *dev,
+                        struct device_attribute *attr,
+                        const char *buf,
+                        size_t size)
+{
+    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    unsigned long index, val;
+
+    if (sscanf(buf, "%lx %lx", &index, &val) != 2)
+        return -EINVAL;
+    if (index >= TPDM_DSB_MAX_PATT)
+        return -EPERM;
+
+    spin_lock(&drvdata->spinlock);
+    drvdata->dsb->trig_patt_val[index] = val;
+    spin_unlock(&drvdata->spinlock);
+    return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_patt_val);
+
+static ssize_t dsb_trig_patt_mask_show(struct device *dev,
+                        struct device_attribute *attr,
+                        char *buf)
+{
+    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+    ssize_t size = 0;
+    int i = 0;
+
+    spin_lock(&drvdata->spinlock);
+    for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
+        size += sysfs_emit_at(buf, size,
+                  "Index: 0x%x Value: 0x%x\n", i,
+                  drvdata->dsb->trig_patt_mask[i]);
+    }
+    spin_unlock(&drvdata->spinlock);
+    return size;

Suzuki