Re: [PATCH v7 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type
From: Tao Zhang
Date: Wed Aug 09 2023 - 02:56:24 EST
On 8/7/2023 5:42 PM, Suzuki K Poulose wrote:
On 25/07/2023 08:15, Tao Zhang wrote:
The nodes are needed to set or show the trigger timestamp and
trigger type. This change is to add these nodes to achieve these
function.
Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
---
.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 24 ++++++
drivers/hwtracing/coresight/coresight-tpdm.c | 94
++++++++++++++++++++++
2 files changed, 118 insertions(+)
diff --git
a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index dbc2fbd0..0b7b4ad 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -21,3 +21,27 @@ Description:
Accepts only one value - 1.
1 : Reset the dataset of the tpdm
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_type
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang
(QUIC) <quic_taozha@xxxxxxxxxxx>
+Description:
+ (Write) Set the trigger type of DSB tpdm. Read the trigger
+ type of DSB tpdm.
Please use: (RW) instead of (Write).
(RW) Set/Get the trigger type of the DSB for TPDM.
Similarly for the items below.
I will update this in the next patch series.
+
+ Accepts only one of the 2 values - 0 or 1.
+ 0 : Set the DSB trigger type to false
+ 1 : Set the DSB trigger type to true
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang
(QUIC) <quic_taozha@xxxxxxxxxxx>
+Description:
+ (Write) Set the trigger timestamp of DSB tpdm. Read the
+ trigger timestamp of DSB tpdm.
+
+ Accepts only one of the 2 values - 0 or 1.
+ 0 : Set the DSB trigger type to false
+ 1 : Set the DSB trigger type to true
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c
b/drivers/hwtracing/coresight/coresight-tpdm.c
index acc3eea..62efc18 100644
--- a/drivers/hwtracing/coresight/coresight-tpdm.c
+++ b/drivers/hwtracing/coresight/coresight-tpdm.c
@@ -20,6 +20,18 @@
DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm");
+static umode_t tpdm_dsb_is_visible(struct kobject *kobj,
+ struct attribute *attr, int n)
Please keep the alignment.
I will update this in the next patch series.
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (drvdata && (drvdata->datasets & TPDM_PIDR0_DS_DSB))
As suggested earlier, add a wrapper for the above check.
I will update this in the next patch series.
+ return attr->mode;
+
+ return 0;
+}
+
static void tpdm_reset_datasets(struct tpdm_drvdata *drvdata)
{
if (drvdata->datasets & TPDM_PIDR0_DS_DSB) {
@@ -229,8 +241,90 @@ static struct attribute_group tpdm_attr_grp = {
.attrs = tpdm_attrs,
};
+static ssize_t dsb_trig_type_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
Please follow the above alignment for all functions throughout the
series. There are unaligned parameter lists scattered around the series.
I will update this in the next patch series.
Best,
Tao
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ return sysfs_emit(buf, "%u\n",
+ (unsigned int)drvdata->dsb->trig_type);
+}
+
+/*
+ * Trigger type (boolean):
+ * false - Disable trigger type.
+ * true - Enable trigger type.
+ */
+static ssize_t dsb_trig_type_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 val;
+
+ if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ if (val)
+ drvdata->dsb->trig_type = true;
+ else
+ drvdata->dsb->trig_type = false;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_type);
+
+static ssize_t dsb_trig_ts_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ return sysfs_emit(buf, "%u\n",
+ (unsigned int)drvdata->dsb->trig_ts);
+}
+
+/*
+ * Trigger timestamp (boolean):
+ * false - Disable trigger timestamp.
+ * true - Enable trigger timestamp.
+ */
+static ssize_t dsb_trig_ts_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 val;
+
+ if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))
+ return -EINVAL;
+
+ spin_lock(&drvdata->spinlock);
+ if (val)
+ drvdata->dsb->trig_ts = true;
+ else
+ drvdata->dsb->trig_ts = false;
+ spin_unlock(&drvdata->spinlock);
+ return size;
+}
+static DEVICE_ATTR_RW(dsb_trig_ts);
+
+static struct attribute *tpdm_dsb_attrs[] = {
+ &dev_attr_dsb_trig_ts.attr,
+ &dev_attr_dsb_trig_type.attr,
+ NULL,
+};
+
+static struct attribute_group tpdm_dsb_attr_grp = {
+ .attrs = tpdm_dsb_attrs,
+ .is_visible = tpdm_dsb_is_visible,
+};
+
static const struct attribute_group *tpdm_attr_grps[] = {
&tpdm_attr_grp,
+ &tpdm_dsb_attr_grp,
NULL,
};
Rest looks fine.
Suzuk