[PATCH v6 2/2] coresight: Add label sysfs node support

Mao Jinlong posted 2 patches 1 year ago
There is a newer version of this series
[PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Mao Jinlong 1 year ago
For some coresight components like CTI and TPDM, there could be
numerous of them. From the node name, we can only get the type and
register address of the component. We can't identify the HW or the
system the component belongs to. Add label sysfs node support for
showing the intuitive name of the device.

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
 .../sysfs-bus-coresight-devices-funnel        |  6 ++++
 .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
 drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
 4 files changed, 50 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
index bf2869c413e7..909670e0451a 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
@@ -239,3 +239,9 @@ Date:		March 2020
 KernelVersion	5.7
 Contact:	Mike Leach or Mathieu Poirier
 Description:	(Write) Clear all channel / trigger programming.
+
+What:           /sys/bus/coresight/devices/<cti-name>/label
+Date:           Dec 2024
+KernelVersion   6.14
+Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
+Description:    (Read) Show hardware context information of device.
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
index d75acda5e1b3..944aad879aeb 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
@@ -10,3 +10,9 @@ Date:		November 2014
 KernelVersion:	3.19
 Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
 Description:	(RW) Defines input port priority order.
+
+What:           /sys/bus/coresight/devices/<memory_map>.funnel/label
+Date:           Dec 2024
+KernelVersion   6.14
+Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
+Description:    (Read) Show hardware context information of device.
diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
index bf710ea6e0ef..309802246398 100644
--- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
+++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
@@ -257,3 +257,9 @@ Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
 Description:
 		(RW) Set/Get the MSR(mux select register) for the CMB subunit
 		TPDM.
+
+What:           /sys/bus/coresight/devices/<tpdm-name>/label
+Date:           Dec 2024
+KernelVersion   6.14
+Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
+Description:    (Read) Show hardware context information of device.
diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
index a01c9e54e2ed..4af40cd7d75a 100644
--- a/drivers/hwtracing/coresight/coresight-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-sysfs.c
@@ -7,6 +7,7 @@
 #include <linux/device.h>
 #include <linux/idr.h>
 #include <linux/kernel.h>
+#include <linux/property.h>
 
 #include "coresight-priv.h"
 #include "coresight-trace-id.h"
@@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device *dev,
 }
 static DEVICE_ATTR_RW(enable_source);
 
+static ssize_t label_show(struct device *dev,
+		struct device_attribute *attr, char *buf)
+{
+
+	const char *str;
+	int ret = 0;
+
+	ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
+	if (ret == 0)
+		return scnprintf(buf, PAGE_SIZE, "%s\n", str);
+	else
+		return ret;
+}
+static DEVICE_ATTR_RO(label);
+
 static struct attribute *coresight_sink_attrs[] = {
 	&dev_attr_enable_sink.attr,
+	&dev_attr_label.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(coresight_sink);
 
 static struct attribute *coresight_source_attrs[] = {
 	&dev_attr_enable_source.attr,
+	&dev_attr_label.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(coresight_source);
 
+static struct attribute *coresight_link_attrs[] = {
+	&dev_attr_label.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(coresight_link);
+
+static struct attribute *coresight_helper_attrs[] = {
+	&dev_attr_label.attr,
+	NULL,
+};
+ATTRIBUTE_GROUPS(coresight_helper);
+
 const struct device_type coresight_dev_type[] = {
 	[CORESIGHT_DEV_TYPE_SINK] = {
 		.name = "sink",
@@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
 	},
 	[CORESIGHT_DEV_TYPE_LINK] = {
 		.name = "link",
+		.groups = coresight_link_groups,
 	},
 	[CORESIGHT_DEV_TYPE_LINKSINK] = {
 		.name = "linksink",
@@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
 	},
 	[CORESIGHT_DEV_TYPE_HELPER] = {
 		.name = "helper",
+		.groups = coresight_helper_groups,
 	}
 };
 /* Ensure the enum matches the names and groups */
-- 
2.17.1
Re: [PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Mike Leach 1 year ago
Hi

On Tue, 17 Dec 2024 at 06:33, Mao Jinlong <quic_jinlmao@quicinc.com> wrote:
>
> For some coresight components like CTI and TPDM, there could be
> numerous of them. From the node name, we can only get the type and
> register address of the component. We can't identify the HW or the
> system the component belongs to. Add label sysfs node support for
> showing the intuitive name of the device.
>
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>  .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
>  .../sysfs-bus-coresight-devices-funnel        |  6 ++++
>  .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
>  drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
>  4 files changed, 50 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> index bf2869c413e7..909670e0451a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> @@ -239,3 +239,9 @@ Date:               March 2020
>  KernelVersion  5.7
>  Contact:       Mike Leach or Mathieu Poirier
>  Description:   (Write) Clear all channel / trigger programming.
> +
> +What:           /sys/bus/coresight/devices/<cti-name>/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> index d75acda5e1b3..944aad879aeb 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> @@ -10,3 +10,9 @@ Date:         November 2014
>  KernelVersion: 3.19
>  Contact:       Mathieu Poirier <mathieu.poirier@linaro.org>
>  Description:   (RW) Defines input port priority order.
> +
> +What:           /sys/bus/coresight/devices/<memory_map>.funnel/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index bf710ea6e0ef..309802246398 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -257,3 +257,9 @@ Contact:    Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
>  Description:
>                 (RW) Set/Get the MSR(mux select register) for the CMB subunit
>                 TPDM.
> +
> +What:           /sys/bus/coresight/devices/<tpdm-name>/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index a01c9e54e2ed..4af40cd7d75a 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -7,6 +7,7 @@
>  #include <linux/device.h>
>  #include <linux/idr.h>
>  #include <linux/kernel.h>
> +#include <linux/property.h>
>
>  #include "coresight-priv.h"
>  #include "coresight-trace-id.h"
> @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device *dev,
>  }
>  static DEVICE_ATTR_RW(enable_source);
>
> +static ssize_t label_show(struct device *dev,
> +               struct device_attribute *attr, char *buf)
> +{
> +
> +       const char *str;
> +       int ret = 0;
> +
> +       ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
> +       if (ret == 0)
> +               return scnprintf(buf, PAGE_SIZE, "%s\n", str);
> +       else
> +               return ret;
> +}
> +static DEVICE_ATTR_RO(label);
> +
>  static struct attribute *coresight_sink_attrs[] = {
>         &dev_attr_enable_sink.attr,
> +       &dev_attr_label.attr,
>         NULL,
>  };
>  ATTRIBUTE_GROUPS(coresight_sink);
>
>  static struct attribute *coresight_source_attrs[] = {
>         &dev_attr_enable_source.attr,
> +       &dev_attr_label.attr,
>         NULL,
>  };
>  ATTRIBUTE_GROUPS(coresight_source);
>
> +static struct attribute *coresight_link_attrs[] = {
> +       &dev_attr_label.attr,
> +       NULL,
> +};
> +ATTRIBUTE_GROUPS(coresight_link);
> +
> +static struct attribute *coresight_helper_attrs[] = {
> +       &dev_attr_label.attr,
> +       NULL,
> +};
> +ATTRIBUTE_GROUPS(coresight_helper);
> +
>  const struct device_type coresight_dev_type[] = {
>         [CORESIGHT_DEV_TYPE_SINK] = {
>                 .name = "sink",
> @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
>         },
>         [CORESIGHT_DEV_TYPE_LINK] = {
>                 .name = "link",
> +               .groups = coresight_link_groups,
>         },
>         [CORESIGHT_DEV_TYPE_LINKSINK] = {
>                 .name = "linksink",
> @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
>         },
>         [CORESIGHT_DEV_TYPE_HELPER] = {
>                 .name = "helper",
> +               .groups = coresight_helper_groups,
>         }
>  };
>  /* Ensure the enum matches the names and groups */
> --
> 2.17.1
>

This is a clean and simple solution to this issue. In this form...

Reviewed-by: Mike Leach <mike.leach@linaro.org>

-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK
Re: [PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Suzuki K Poulose 1 year ago
On 17/12/2024 06:33, Mao Jinlong wrote:
> For some coresight components like CTI and TPDM, there could be
> numerous of them. From the node name, we can only get the type and
> register address of the component. We can't identify the HW or the
> system the component belongs to. Add label sysfs node support for
> showing the intuitive name of the device.
> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
>   .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
>   .../sysfs-bus-coresight-devices-funnel        |  6 ++++
>   .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
>   drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
>   4 files changed, 50 insertions(+)

Do you think we need to name the devices using the label ? Or is this 
enough ?

Suzuki


> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> index bf2869c413e7..909670e0451a 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> @@ -239,3 +239,9 @@ Date:		March 2020
>   KernelVersion	5.7
>   Contact:	Mike Leach or Mathieu Poirier
>   Description:	(Write) Clear all channel / trigger programming.
> +
> +What:           /sys/bus/coresight/devices/<cti-name>/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> index d75acda5e1b3..944aad879aeb 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> @@ -10,3 +10,9 @@ Date:		November 2014
>   KernelVersion:	3.19
>   Contact:	Mathieu Poirier <mathieu.poirier@linaro.org>
>   Description:	(RW) Defines input port priority order.
> +
> +What:           /sys/bus/coresight/devices/<memory_map>.funnel/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> index bf710ea6e0ef..309802246398 100644
> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> @@ -257,3 +257,9 @@ Contact:	Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
>   Description:
>   		(RW) Set/Get the MSR(mux select register) for the CMB subunit
>   		TPDM.
> +
> +What:           /sys/bus/coresight/devices/<tpdm-name>/label
> +Date:           Dec 2024
> +KernelVersion   6.14
> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> +Description:    (Read) Show hardware context information of device.
> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/hwtracing/coresight/coresight-sysfs.c
> index a01c9e54e2ed..4af40cd7d75a 100644
> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> @@ -7,6 +7,7 @@
>   #include <linux/device.h>
>   #include <linux/idr.h>
>   #include <linux/kernel.h>
> +#include <linux/property.h>
>   
>   #include "coresight-priv.h"
>   #include "coresight-trace-id.h"
> @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(enable_source);
>   
> +static ssize_t label_show(struct device *dev,
> +		struct device_attribute *attr, char *buf)
> +{
> +
> +	const char *str;
> +	int ret = 0;
> +
> +	ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
> +	if (ret == 0)
> +		return scnprintf(buf, PAGE_SIZE, "%s\n", str);
> +	else
> +		return ret;
> +}
> +static DEVICE_ATTR_RO(label);
> +
>   static struct attribute *coresight_sink_attrs[] = {
>   	&dev_attr_enable_sink.attr,
> +	&dev_attr_label.attr,
>   	NULL,
>   };
>   ATTRIBUTE_GROUPS(coresight_sink);
>   
>   static struct attribute *coresight_source_attrs[] = {
>   	&dev_attr_enable_source.attr,
> +	&dev_attr_label.attr,
>   	NULL,
>   };
>   ATTRIBUTE_GROUPS(coresight_source);
>   
> +static struct attribute *coresight_link_attrs[] = {
> +	&dev_attr_label.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(coresight_link);
> +
> +static struct attribute *coresight_helper_attrs[] = {
> +	&dev_attr_label.attr,
> +	NULL,
> +};
> +ATTRIBUTE_GROUPS(coresight_helper);
> +
>   const struct device_type coresight_dev_type[] = {
>   	[CORESIGHT_DEV_TYPE_SINK] = {
>   		.name = "sink",
> @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
>   	},
>   	[CORESIGHT_DEV_TYPE_LINK] = {
>   		.name = "link",
> +		.groups = coresight_link_groups,
>   	},
>   	[CORESIGHT_DEV_TYPE_LINKSINK] = {
>   		.name = "linksink",
> @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
>   	},
>   	[CORESIGHT_DEV_TYPE_HELPER] = {
>   		.name = "helper",
> +		.groups = coresight_helper_groups,
>   	}
>   };
>   /* Ensure the enum matches the names and groups */
Re: [PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Jinlong Mao 1 year ago

On 2024/12/18 17:38, Suzuki K Poulose wrote:
> On 17/12/2024 06:33, Mao Jinlong wrote:
>> For some coresight components like CTI and TPDM, there could be
>> numerous of them. From the node name, we can only get the type and
>> register address of the component. We can't identify the HW or the
>> system the component belongs to. Add label sysfs node support for
>> showing the intuitive name of the device.
>>
>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>> ---
>>   .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
>>   .../sysfs-bus-coresight-devices-funnel        |  6 ++++
>>   .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
>>   drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
>>   4 files changed, 50 insertions(+)
> 
> Do you think we need to name the devices using the label ? Or is this 
> enough ?
> 
> Suzuki
Hi Suzuki,

In my opinion, we should use label as the device name.

It will be easier to identify the component with the folder name in 
/sys/bus/coresight/devices/


Thanks
Jinlong Mao

> 
> 
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti 
>> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>> index bf2869c413e7..909670e0451a 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
>> @@ -239,3 +239,9 @@ Date:        March 2020
>>   KernelVersion    5.7
>>   Contact:    Mike Leach or Mathieu Poirier
>>   Description:    (Write) Clear all channel / trigger programming.
>> +
>> +What:           /sys/bus/coresight/devices/<cti-name>/label
>> +Date:           Dec 2024
>> +KernelVersion   6.14
>> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
>> +Description:    (Read) Show hardware context information of device.
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- 
>> funnel b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>> index d75acda5e1b3..944aad879aeb 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
>> @@ -10,3 +10,9 @@ Date:        November 2014
>>   KernelVersion:    3.19
>>   Contact:    Mathieu Poirier <mathieu.poirier@linaro.org>
>>   Description:    (RW) Defines input port priority order.
>> +
>> +What:           /sys/bus/coresight/devices/<memory_map>.funnel/label
>> +Date:           Dec 2024
>> +KernelVersion   6.14
>> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
>> +Description:    (Read) Show hardware context information of device.
>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- 
>> tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> index bf710ea6e0ef..309802246398 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
>> @@ -257,3 +257,9 @@ Contact:    Jinlong Mao (QUIC) 
>> <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
>>   Description:
>>           (RW) Set/Get the MSR(mux select register) for the CMB subunit
>>           TPDM.
>> +
>> +What:           /sys/bus/coresight/devices/<tpdm-name>/label
>> +Date:           Dec 2024
>> +KernelVersion   6.14
>> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
>> +Description:    (Read) Show hardware context information of device.
>> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/ 
>> hwtracing/coresight/coresight-sysfs.c
>> index a01c9e54e2ed..4af40cd7d75a 100644
>> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
>> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
>> @@ -7,6 +7,7 @@
>>   #include <linux/device.h>
>>   #include <linux/idr.h>
>>   #include <linux/kernel.h>
>> +#include <linux/property.h>
>>   #include "coresight-priv.h"
>>   #include "coresight-trace-id.h"
>> @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device 
>> *dev,
>>   }
>>   static DEVICE_ATTR_RW(enable_source);
>> +static ssize_t label_show(struct device *dev,
>> +        struct device_attribute *attr, char *buf)
>> +{
>> +
>> +    const char *str;
>> +    int ret = 0;
>> +
>> +    ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
>> +    if (ret == 0)
>> +        return scnprintf(buf, PAGE_SIZE, "%s\n", str);
>> +    else
>> +        return ret;
>> +}
>> +static DEVICE_ATTR_RO(label);
>> +
>>   static struct attribute *coresight_sink_attrs[] = {
>>       &dev_attr_enable_sink.attr,
>> +    &dev_attr_label.attr,
>>       NULL,
>>   };
>>   ATTRIBUTE_GROUPS(coresight_sink);
>>   static struct attribute *coresight_source_attrs[] = {
>>       &dev_attr_enable_source.attr,
>> +    &dev_attr_label.attr,
>>       NULL,
>>   };
>>   ATTRIBUTE_GROUPS(coresight_source);
>> +static struct attribute *coresight_link_attrs[] = {
>> +    &dev_attr_label.attr,
>> +    NULL,
>> +};
>> +ATTRIBUTE_GROUPS(coresight_link);
>> +
>> +static struct attribute *coresight_helper_attrs[] = {
>> +    &dev_attr_label.attr,
>> +    NULL,
>> +};
>> +ATTRIBUTE_GROUPS(coresight_helper);
>> +
>>   const struct device_type coresight_dev_type[] = {
>>       [CORESIGHT_DEV_TYPE_SINK] = {
>>           .name = "sink",
>> @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
>>       },
>>       [CORESIGHT_DEV_TYPE_LINK] = {
>>           .name = "link",
>> +        .groups = coresight_link_groups,
>>       },
>>       [CORESIGHT_DEV_TYPE_LINKSINK] = {
>>           .name = "linksink",
>> @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
>>       },
>>       [CORESIGHT_DEV_TYPE_HELPER] = {
>>           .name = "helper",
>> +        .groups = coresight_helper_groups,
>>       }
>>   };
>>   /* Ensure the enum matches the names and groups */
> 

Re: [PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Krzysztof Kozlowski 12 months ago
On 18/12/2024 10:57, Jinlong Mao wrote:
> 
> 
> On 2024/12/18 17:38, Suzuki K Poulose wrote:
>> On 17/12/2024 06:33, Mao Jinlong wrote:
>>> For some coresight components like CTI and TPDM, there could be
>>> numerous of them. From the node name, we can only get the type and
>>> register address of the component. We can't identify the HW or the
>>> system the component belongs to. Add label sysfs node support for
>>> showing the intuitive name of the device.
>>>
>>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>>> ---
>>>   .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
>>>   .../sysfs-bus-coresight-devices-funnel        |  6 ++++
>>>   .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
>>>   drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
>>>   4 files changed, 50 insertions(+)
>>
>> Do you think we need to name the devices using the label ? Or is this 
>> enough ?
>>
>> Suzuki
> Hi Suzuki,
> 
> In my opinion, we should use label as the device name.

As Linux device driver name?

No, that's not the point of label. We don't do such stuff, nowhere.


Best regards,
Krzysztof
Re: [PATCH v6 2/2] coresight: Add label sysfs node support
Posted by Mike Leach 1 year ago
Hi

On Wed, 18 Dec 2024 at 09:57, Jinlong Mao <quic_jinlmao@quicinc.com> wrote:
>
>
>
> On 2024/12/18 17:38, Suzuki K Poulose wrote:
> > On 17/12/2024 06:33, Mao Jinlong wrote:
> >> For some coresight components like CTI and TPDM, there could be
> >> numerous of them. From the node name, we can only get the type and
> >> register address of the component. We can't identify the HW or the
> >> system the component belongs to. Add label sysfs node support for
> >> showing the intuitive name of the device.
> >>
> >> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> >> ---
> >>   .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
> >>   .../sysfs-bus-coresight-devices-funnel        |  6 ++++
> >>   .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
> >>   drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
> >>   4 files changed, 50 insertions(+)
> >
> > Do you think we need to name the devices using the label ? Or is this
> > enough ?
> >
> > Suzuki
> Hi Suzuki,
>
> In my opinion, we should use label as the device name.
>
> It will be easier to identify the component with the folder name in
> /sys/bus/coresight/devices/
>
>

Please see the my comments from v4 of this patchset that explains why
this should not be done- re uniqueness and allowing scripting to
work:-

https://lists.linaro.org/archives/list/coresight@lists.linaro.org/message/Y65RKVUJUQGNARMWCOLSD636K7LROLYA/

Mike


> Thanks
> Jinlong Mao
>
> >
> >
> >>
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> >> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> >> index bf2869c413e7..909670e0451a 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> >> @@ -239,3 +239,9 @@ Date:        March 2020
> >>   KernelVersion    5.7
> >>   Contact:    Mike Leach or Mathieu Poirier
> >>   Description:    (Write) Clear all channel / trigger programming.
> >> +
> >> +What:           /sys/bus/coresight/devices/<cti-name>/label
> >> +Date:           Dec 2024
> >> +KernelVersion   6.14
> >> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> >> +Description:    (Read) Show hardware context information of device.
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
> >> funnel b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> >> index d75acda5e1b3..944aad879aeb 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> >> @@ -10,3 +10,9 @@ Date:        November 2014
> >>   KernelVersion:    3.19
> >>   Contact:    Mathieu Poirier <mathieu.poirier@linaro.org>
> >>   Description:    (RW) Defines input port priority order.
> >> +
> >> +What:           /sys/bus/coresight/devices/<memory_map>.funnel/label
> >> +Date:           Dec 2024
> >> +KernelVersion   6.14
> >> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> >> +Description:    (Read) Show hardware context information of device.
> >> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-
> >> tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >> index bf710ea6e0ef..309802246398 100644
> >> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> >> @@ -257,3 +257,9 @@ Contact:    Jinlong Mao (QUIC)
> >> <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
> >>   Description:
> >>           (RW) Set/Get the MSR(mux select register) for the CMB subunit
> >>           TPDM.
> >> +
> >> +What:           /sys/bus/coresight/devices/<tpdm-name>/label
> >> +Date:           Dec 2024
> >> +KernelVersion   6.14
> >> +Contact:        Mao Jinlong <quic_jinlmao@quicinc.com>
> >> +Description:    (Read) Show hardware context information of device.
> >> diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c b/drivers/
> >> hwtracing/coresight/coresight-sysfs.c
> >> index a01c9e54e2ed..4af40cd7d75a 100644
> >> --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> >> +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> >> @@ -7,6 +7,7 @@
> >>   #include <linux/device.h>
> >>   #include <linux/idr.h>
> >>   #include <linux/kernel.h>
> >> +#include <linux/property.h>
> >>   #include "coresight-priv.h"
> >>   #include "coresight-trace-id.h"
> >> @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device
> >> *dev,
> >>   }
> >>   static DEVICE_ATTR_RW(enable_source);
> >> +static ssize_t label_show(struct device *dev,
> >> +        struct device_attribute *attr, char *buf)
> >> +{
> >> +
> >> +    const char *str;
> >> +    int ret = 0;
> >> +
> >> +    ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
> >> +    if (ret == 0)
> >> +        return scnprintf(buf, PAGE_SIZE, "%s\n", str);
> >> +    else
> >> +        return ret;
> >> +}
> >> +static DEVICE_ATTR_RO(label);
> >> +
> >>   static struct attribute *coresight_sink_attrs[] = {
> >>       &dev_attr_enable_sink.attr,
> >> +    &dev_attr_label.attr,
> >>       NULL,
> >>   };
> >>   ATTRIBUTE_GROUPS(coresight_sink);
> >>   static struct attribute *coresight_source_attrs[] = {
> >>       &dev_attr_enable_source.attr,
> >> +    &dev_attr_label.attr,
> >>       NULL,
> >>   };
> >>   ATTRIBUTE_GROUPS(coresight_source);
> >> +static struct attribute *coresight_link_attrs[] = {
> >> +    &dev_attr_label.attr,
> >> +    NULL,
> >> +};
> >> +ATTRIBUTE_GROUPS(coresight_link);
> >> +
> >> +static struct attribute *coresight_helper_attrs[] = {
> >> +    &dev_attr_label.attr,
> >> +    NULL,
> >> +};
> >> +ATTRIBUTE_GROUPS(coresight_helper);
> >> +
> >>   const struct device_type coresight_dev_type[] = {
> >>       [CORESIGHT_DEV_TYPE_SINK] = {
> >>           .name = "sink",
> >> @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
> >>       },
> >>       [CORESIGHT_DEV_TYPE_LINK] = {
> >>           .name = "link",
> >> +        .groups = coresight_link_groups,
> >>       },
> >>       [CORESIGHT_DEV_TYPE_LINKSINK] = {
> >>           .name = "linksink",
> >> @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
> >>       },
> >>       [CORESIGHT_DEV_TYPE_HELPER] = {
> >>           .name = "helper",
> >> +        .groups = coresight_helper_groups,
> >>       }
> >>   };
> >>   /* Ensure the enum matches the names and groups */
> >
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK