2023-07-25 07:35:38

by Tao Zhang

[permalink] [raw]
Subject: [PATCH v7 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

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 <[email protected]>
---
.../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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+Description:
+ (Write) Set the trigger type of DSB tpdm. Read the trigger
+ type 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
+
+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_trig_ts
+Date: March 2023
+KernelVersion 6.5
+Contact: Jinlong Mao (QUIC) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
+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)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+
+ if (drvdata && (drvdata->datasets & TPDM_PIDR0_DS_DSB))
+ 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)
+{
+ 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,
};

--
2.7.4



2023-08-07 10:51:49

by Suzuki K Poulose

[permalink] [raw]
Subject: Re: [PATCH v7 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type

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 <[email protected]>
> ---
> .../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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
> +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.

> +
> + 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) <[email protected]>, Tao Zhang (QUIC) <[email protected]>
> +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.

> +{
> + 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.

> + 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.

> +{
> + 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


2023-08-09 07:27:45

by Tao Zhang

[permalink] [raw]
Subject: Re: [PATCH v7 07/13] coresight-tpdm: Add nodes to set trigger timestamp and type


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 <[email protected]>
>> ---
>>   .../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) <[email protected]>, Tao Zhang
>> (QUIC) <[email protected]>
>> +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) <[email protected]>, Tao Zhang
>> (QUIC) <[email protected]>
>> +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
>