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

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

[Mao Jinlong]

With current design, the name of the non-cpu bounded coresight
component is the device type with the number. And with 'ls' command
we can get the register address of the component. But from these
information, we can't know what the HW or system the component belongs
to. Add label in DT and show the hw information by reading label sysfs
node.

cti_sys0 -> ../../../devices/platform/soc@0/138f0000.cti/cti_sys0
cti_sys1 -> ../../../devices/platform/soc@0/13900000.cti/cti_sys1
tpdm0 -> ../../../devices/platform/soc@0/10b0d000.tpdm/tpdm0
tpdm1 -> ../../../devices/platform/soc@0/10c28000.tpdm/tpdm1
tpdm2 -> ../../../devices/platform/soc@0/10c29000.tpdm/tpdm2

/sys/bus/coresight/devices # cat cti*/label
cti_dlct_0
cti_dlct_1
cti_apss_0
cti_apss_1
cti_apss_2

Change since V5:
1. Update the kernel version of ABI files.
2. Add link of different patch versions.
V5 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20241210122253.31926-1-quic_jinlmao@quicinc.com/

Change since V4:
1. Add label in DT and add label sysfs node for each coresight device. 
V4 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240703122340.26864-1-quic_jinlmao@quicinc.com/

Change since V3:
1. Change device-name to arm,cs-dev-name.
2. Add arm,cs-dev-name to only CTI and sources' dt-binding.
V3 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240131082628.6288-1-quic_jinlmao@quicinc.com/

Change since V2:
1. Fix the error in coresight core.
drivers/hwtracing/coresight/coresight-core.c:1775:7: error: assigning to 'char *' from 'const char *' discards qualifiers

2. Fix the warning when run dtbinding check.
Documentation/devicetree/bindings/arm/arm,coresight-cpu-debug.yaml: device-name: missing type definition
V2 link: https://patchwork.kernel.org/project/linux-arm-msm/cover/20240115164252.26510-1-quic_jinlmao@quicinc.com/

Change since V1:
1. Change coresight-name to device name.
2. Add the device-name in coresight dt bindings.
V1 link: https://patchwork.kernel.org/project/linux-arm-kernel/patch/20230208110716.18321-1-quic_jinlmao@quicinc.com/#25231737


Mao Jinlong (2):
  dt-bindings: arm: Add label in the coresight components
  coresight: Add label sysfs node support

 .../testing/sysfs-bus-coresight-devices-cti   |  6 ++++
 .../sysfs-bus-coresight-devices-funnel        |  6 ++++
 .../testing/sysfs-bus-coresight-devices-tpdm  |  6 ++++
 .../bindings/arm/arm,coresight-cti.yaml       |  6 ++++
 .../arm/arm,coresight-dummy-sink.yaml         |  6 ++++
 .../arm/arm,coresight-dummy-source.yaml       |  6 ++++
 .../arm/arm,coresight-dynamic-funnel.yaml     |  6 ++++
 .../arm/arm,coresight-dynamic-replicator.yaml |  6 ++++
 .../arm/arm,coresight-static-funnel.yaml      |  6 ++++
 .../arm/arm,coresight-static-replicator.yaml  |  6 ++++
 .../bindings/arm/arm,coresight-tmc.yaml       |  6 ++++
 .../bindings/arm/qcom,coresight-tpda.yaml     |  6 ++++
 .../bindings/arm/qcom,coresight-tpdm.yaml     |  6 ++++
 drivers/hwtracing/coresight/coresight-sysfs.c | 32 +++++++++++++++++++
 14 files changed, 110 insertions(+)

-- 
2.17.1

[PATCH v6 1/2] dt-bindings: arm: Add label in the coresight components

[Mao Jinlong]

Current name of coresight component's folder consists of prefix of
the device and the id in the device list. When run 'ls' command,
we can get the register address of the device. Take CTI for example,
if we want to set the config for modem CTI, but we can't know which
CTI is modem CTI from all current information.

cti_sys0 -> ../../../devices/platform/soc@0/138f0000.cti/cti_sys0
cti_sys1 -> ../../../devices/platform/soc@0/13900000.cti/cti_sys1

Add label to show hardware context information of each coresight
device. There will be a sysfs node label in each device folder.

cat /sys/bus/coresight/devices/cti_sys0/label

Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
 .../devicetree/bindings/arm/arm,coresight-cti.yaml          | 6 ++++++
 .../devicetree/bindings/arm/arm,coresight-dummy-sink.yaml   | 6 ++++++
 .../devicetree/bindings/arm/arm,coresight-dummy-source.yaml | 6 ++++++
 .../bindings/arm/arm,coresight-dynamic-funnel.yaml          | 6 ++++++
 .../bindings/arm/arm,coresight-dynamic-replicator.yaml      | 6 ++++++
 .../bindings/arm/arm,coresight-static-funnel.yaml           | 6 ++++++
 .../bindings/arm/arm,coresight-static-replicator.yaml       | 6 ++++++
 .../devicetree/bindings/arm/arm,coresight-tmc.yaml          | 6 ++++++
 .../devicetree/bindings/arm/qcom,coresight-tpda.yaml        | 6 ++++++
 .../devicetree/bindings/arm/qcom,coresight-tpdm.yaml        | 6 ++++++
 10 files changed, 60 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml
index 2d5545a2b49c..5ca6d3d313a3 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-cti.yaml
@@ -98,6 +98,12 @@ properties:
   power-domains:
     maxItems: 1
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   arm,cti-ctm-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description:
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy-sink.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy-sink.yaml
index 08b89b62c505..bc82cd1f3595 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-dummy-sink.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy-sink.yaml
@@ -39,6 +39,12 @@ properties:
     enum:
       - arm,coresight-dummy-sink
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   in-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dummy-source.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dummy-source.yaml
index 04a8c37b4aff..245929c36357 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-dummy-source.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dummy-source.yaml
@@ -38,6 +38,12 @@ properties:
     enum:
       - arm,coresight-dummy-source
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   out-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
index 44a1041cb0fc..30776610d4b4 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
@@ -57,6 +57,12 @@ properties:
   power-domains:
     maxItems: 1
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   in-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-replicator.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-replicator.yaml
index 03792e9bd97a..178a3861ee29 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-replicator.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-replicator.yaml
@@ -54,6 +54,12 @@ properties:
       - const: apb_pclk
       - const: atclk
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   power-domains:
     maxItems: 1
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-static-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-static-funnel.yaml
index cc8c3baa79b4..39b291909cc4 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-static-funnel.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-funnel.yaml
@@ -30,6 +30,12 @@ properties:
   power-domains:
     maxItems: 1
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   in-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
index 1892a091ac35..ba10897043cf 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-static-replicator.yaml
@@ -39,6 +39,12 @@ properties:
         description: Input connection from CoreSight Trace bus
         $ref: /schemas/graph.yaml#/properties/port
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   out-ports:
     $ref: /schemas/graph.yaml#/properties/ports
 
diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
index cb8dceaca70e..8d952150d7b9 100644
--- a/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,coresight-tmc.yaml
@@ -55,6 +55,12 @@ properties:
       - const: apb_pclk
       - const: atclk
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   iommus:
     maxItems: 1
 
diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
index 76163abed655..986b9b4f360d 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpda.yaml
@@ -65,6 +65,12 @@ properties:
     items:
       - const: apb_pclk
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   in-ports:
     description: |
       Input connections from TPDM to TPDA
diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
index 8eec07d9d454..59e533720bdd 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tpdm.yaml
@@ -77,6 +77,12 @@ properties:
     minimum: 0
     maximum: 32
 
+  label:
+    $ref: /schemas/types.yaml#/definitions/string
+    description:
+      Define the label which can describe what kind of HW or system the
+      coresight device belongs to.
+
   clocks:
     maxItems: 1
 
-- 
2.17.1

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

[Mao Jinlong]

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 1/2] dt-bindings: arm: Add label in the coresight components

[Krzysztof Kozlowski]

On Tue, Dec 17, 2024 at 02:33:23PM +0800, Mao Jinlong wrote:
> Current name of coresight component's folder consists of prefix of
> the device and the id in the device list. When run 'ls' command,
> we can get the register address of the device. Take CTI for example,
> if we want to set the config for modem CTI, but we can't know which
> CTI is modem CTI from all current information.
> 
> cti_sys0 -> ../../../devices/platform/soc@0/138f0000.cti/cti_sys0
> cti_sys1 -> ../../../devices/platform/soc@0/13900000.cti/cti_sys1
> 
> Add label to show hardware context information of each coresight
> device. There will be a sysfs node label in each device folder.
> 
> cat /sys/bus/coresight/devices/cti_sys0/label
> 
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

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

[Suzuki K Poulose]

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

[Mike Leach]

Hi

> -----Original Message-----
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> Sent: Wednesday, December 18, 2024 9:38 AM
> To: Mao Jinlong <quic_jinlmao@quicinc.com>; Mike Leach
> <mike.leach@linaro.org>; James Clark <James.Clark@arm.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Bjorn
> Andersson <andersson@kernel.org>; Konrad Dybcio
> <konradybcio@kernel.org>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> msm@vger.kernel.org
> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
> 
> 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 ? 
> 

No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.

Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.

Mike

> 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 */
> 
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an
> email to coresight-leave@lists.linaro.org

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

[Jinlong Mao]

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

[Mike Leach]

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

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

[Mike Leach]

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 1/2] dt-bindings: arm: Add label in the coresight components

[Mike Leach]

On Wed, 18 Dec 2024 at 09:21, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Tue, Dec 17, 2024 at 02:33:23PM +0800, Mao Jinlong wrote:
> > Current name of coresight component's folder consists of prefix of
> > the device and the id in the device list. When run 'ls' command,
> > we can get the register address of the device. Take CTI for example,
> > if we want to set the config for modem CTI, but we can't know which
> > CTI is modem CTI from all current information.
> >
> > cti_sys0 -> ../../../devices/platform/soc@0/138f0000.cti/cti_sys0
> > cti_sys1 -> ../../../devices/platform/soc@0/13900000.cti/cti_sys1
> >
> > Add label to show hardware context information of each coresight
> > device. There will be a sysfs node label in each device folder.
> >
> > cat /sys/bus/coresight/devices/cti_sys0/label
> >
> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> Best regards,
> Krzysztof
>

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

[Suzuki K Poulose]

Hi Mike

On 18/12/2024 09:56, Mike Leach wrote:
> Hi
> 
>> -----Original Message-----
>> From: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Sent: Wednesday, December 18, 2024 9:38 AM
>> To: Mao Jinlong <quic_jinlmao@quicinc.com>; Mike Leach
>> <mike.leach@linaro.org>; James Clark <James.Clark@arm.com>; Rob Herring
>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>> <conor+dt@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Bjorn
>> Andersson <andersson@kernel.org>; Konrad Dybcio
>> <konradybcio@kernel.org>
>> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org;
>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>> msm@vger.kernel.org
>> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
>>
>> 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 ?
>>
> 
> No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.

Anything that doesn't contain '/' can be a device name ? And it is very 
easy to find if the device name has been used in the coresight bus, 
after all these devices only appear there.

It is as good as :

bus_find_device_by_name(coresight_bus_type, NULL, name) == NULL

Of course with coresight_mutex() held.

Suzuki


> 
> Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.
> 
> Mike
> 
>> 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 */
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an
>> email to coresight-leave@lists.linaro.org

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

[Mike Leach]

Hi,

On Wed, 18 Dec 2024 at 18:16, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>
> Hi Mike
>
> On 18/12/2024 09:56, Mike Leach wrote:
> > Hi
> >
> >> -----Original Message-----
> >> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> Sent: Wednesday, December 18, 2024 9:38 AM
> >> To: Mao Jinlong <quic_jinlmao@quicinc.com>; Mike Leach
> >> <mike.leach@linaro.org>; James Clark <James.Clark@arm.com>; Rob Herring
> >> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> >> <conor+dt@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Bjorn
> >> Andersson <andersson@kernel.org>; Konrad Dybcio
> >> <konradybcio@kernel.org>
> >> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org;
> >> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> >> msm@vger.kernel.org
> >> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
> >>
> >> 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 ?
> >>
> >
> > No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.
>
> Anything that doesn't contain '/' can be a device name ? And it is very
> easy to find if the device name has been used in the coresight bus,
> after all these devices only appear there.
>
> It is as good as :
>
> bus_find_device_by_name(coresight_bus_type, NULL, name) == NULL
>
> Of course with coresight_mutex() held.
>
> Suzuki
>

DTS label property (DT spec 4.1.2) is a freeform string, specified to
be a human readable description of the device, e.g. :-

cti0@0x1000 {
reg = <0x1000>;
label = "main system CTI";
};

Arguably the label is completely unnecessary - as the @0x1000 tells
the knowledgeable user, with a hardware specification of the device
precisely what this CTI does and is related to.

The point of this patchset is to add context to the name@addr to make
the identification of the devices easier.

The DT compiler should ensure that the device tree is well formed.
Using driver selected names (cti_cpu0 ... etc) guarantees  that every
device found in the DT has a unique representation in sysfs.

Once a freeform string is used, then not only are duplicates possible,
illegal device names are possible, all of which can result in missing
nodes or worse. This requires handling / complications that are
unnecessary for the purpose.

Yes of course it is easy to look for duplicate names, reject bad ones,
emit errors - but that could end up with a partially working system
with missing components.

Why add potential for breakage when it is not necessary?

Regards

Mike

>
> >
> > Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.
> >
> > Mike
> >
> >> 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 */
> >>
> >> _______________________________________________
> >> CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an
> >> email to coresight-leave@lists.linaro.org
>


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

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

[Krzysztof Kozlowski]

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

[Suzuki K Poulose]

On 19/12/2024 11:17, Mike Leach wrote:
> Hi,
> 
> On Wed, 18 Dec 2024 at 18:16, Suzuki K Poulose <suzuki.poulose@arm.com> wrote:
>>
>> Hi Mike
>>
>> On 18/12/2024 09:56, Mike Leach wrote:
>>> Hi
>>>
>>>> -----Original Message-----
>>>> From: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Sent: Wednesday, December 18, 2024 9:38 AM
>>>> To: Mao Jinlong <quic_jinlmao@quicinc.com>; Mike Leach
>>>> <mike.leach@linaro.org>; James Clark <James.Clark@arm.com>; Rob Herring
>>>> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
>>>> <conor+dt@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Bjorn
>>>> Andersson <andersson@kernel.org>; Konrad Dybcio
>>>> <konradybcio@kernel.org>
>>>> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org;
>>>> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
>>>> msm@vger.kernel.org
>>>> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
>>>>
>>>> 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 ?
>>>>
>>>
>>> No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.
>>
>> Anything that doesn't contain '/' can be a device name ? And it is very
>> easy to find if the device name has been used in the coresight bus,
>> after all these devices only appear there.
>>
>> It is as good as :
>>
>> bus_find_device_by_name(coresight_bus_type, NULL, name) == NULL
>>
>> Of course with coresight_mutex() held.
>>
>> Suzuki
>>
> 
> DTS label property (DT spec 4.1.2) is a freeform string, specified to
> be a human readable description of the device, e.g. :-
> 
> cti0@0x1000 {
> reg = <0x1000>;
> label = "main system CTI";
> };
> 
> Arguably the label is completely unnecessary - as the @0x1000 tells
> the knowledgeable user, with a hardware specification of the device
> precisely what this CTI does and is related to.
> 
> The point of this patchset is to add context to the name@addr to make
> the identification of the devices easier.
> 
> The DT compiler should ensure that the device tree is well formed.
> Using driver selected names (cti_cpu0 ... etc) guarantees  that every
> device found in the DT has a unique representation in sysfs.
> 
> Once a freeform string is used, then not only are duplicates possible,
> illegal device names are possible, all of which can result in missing
> nodes or worse. This requires handling / complications that are
> unnecessary for the purpose.
> 
> Yes of course it is easy to look for duplicate names, reject bad ones,
> emit errors - but that could end up with a partially working system
> with missing components.
> 
> Why add potential for breakage when it is not necessary?

Fair enough, we can do away with exporting the label.

Cheers
Suzuki


> 
> Regards
> 
> Mike
> 
>>
>>>
>>> Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.
>>>
>>> Mike
>>>
>>>> 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 */
>>>>
>>>> _______________________________________________
>>>> CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an
>>>> email to coresight-leave@lists.linaro.org
>>
> 
>