[PATCH 0/3] coresight-tnoc: Add support for Interconnect TNOC

[PATCH 0/3] coresight-tnoc: Add support for Interconnect TNOC

[Yuanfang Zhang]

This patch series adds support for the Qualcomm CoreSight Interconnect TNOC
(Trace Network On Chip) block, which acts as a CoreSight graph link forwarding
trace data from subsystems to the Aggregator TNOC. Unlike the Aggregator TNOC,
this block does not support aggregation or ATID assignment.

Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
---
Yuanfang Zhang (3):
      dt-bindings: arm: qcom: Add Coresight Interconnect TNOC
      coresight-tnoc: add platform driver to support Interconnect TNOC
      coresight-tnoc: Add runtime PM support for Interconnect TNOC

 .../bindings/arm/qcom,coresight-itnoc.yaml         | 108 +++++++++++++
 drivers/hwtracing/coresight/coresight-tnoc.c       | 179 +++++++++++++++------
 2 files changed, 240 insertions(+), 47 deletions(-)
---
base-commit: 2b52cf338d39d684a1c6af298e8204902c026aca
change-id: 20250815-itnoc-460273d1b80c

Best regards,
-- 
Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>

[PATCH 1/3] dt-bindings: arm: qcom: Add Coresight Interconnect TNOC

[Yuanfang Zhang]

Add device tree binding for Qualcomm Coresight Interconnect Trace
Netwrok On Chip (ITNOC). This TNOC acts as a CoreSight
graph link that forwards trace data from a subsystem to the
Aggregator TNOC, without aggregation or ATID functionality.

Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
---
 .../bindings/arm/qcom,coresight-itnoc.yaml         | 108 +++++++++++++++++++++
 1 file changed, 108 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..fd224e07ce68918b453210763aacda585d5a5ca2
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
@@ -0,0 +1,108 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,coresight-itnoc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Interconnect Trace Network On Chip - ITNOC
+
+maintainers:
+  - Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
+
+description: |
+  The Interconnect TNOC is a CoreSight graph link that forwards trace data
+  from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC, it
+  does not have aggregation and ATID functionality.
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - qcom,coresight-itnoc
+  required:
+    - compatible
+
+properties:
+  $nodename:
+    pattern: "^tnoc(@[0-9a-f]+)?$"
+
+  compatible:
+    items:
+      - const: qcom,coresight-itnoc
+
+  reg:
+    maxItems: 1
+    description: Base address and size of the ITNOC registers.
+
+  clock-names:
+    items:
+      - const: apb
+
+  clocks:
+    maxItems: 1
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      '^port(@[0-9a-f]{1,2})?$':
+        description: Input connections from CoreSight Trace Bus
+        $ref: /schemas/graph.yaml#/properties/port
+    additionalProperties: false
+
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port:
+        description: out connections to aggregator TNOC
+        $ref: /schemas/graph.yaml#/properties/port
+    additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - in-ports
+  - out-ports
+
+additionalProperties: false
+
+examples:
+  - |
+    tnoc@109ac000 {
+        compatible = "qcom,coresight-itnoc";
+        reg = <0x109ac000 0x1000>;
+
+        clocks = <&aoss_qmp>;
+        clock-names = "apb";
+
+        in-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            port@0 {
+                reg = <0>;
+                tn_ic_in_tpdm_dcc: endpoint {
+                    remote-endpoint = <&tpdm_dcc_out_tn_ic>;
+                };
+            };
+        };
+
+        out-ports {
+            port {
+                tn_ic_out_tnoc_aggr: endpoint {
+                    /* to Aggregator TNOC input */
+                    remote-endpoint = <&tn_ag_in_tn_ic>;
+                };
+            };
+        };
+    };
+...

-- 
2.34.1

[PATCH 2/3] coresight-tnoc: add platform driver to support Interconnect TNOC

[Yuanfang Zhang]

This patch adds platform driver support for the CoreSight Interconnect
TNOC, Interconnect TNOC is a CoreSight link that forwards trace data
from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC,
it does not have aggregation and ATID functionality.

Key changes:
- Add platform driver `coresight-itnoc` with device tree match support.
- Refactor probe logic into a common `_tnoc_probe()` function.
- Conditionally initialize ATID only for AMBA-based TNOC blocks.

Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tnoc.c | 153 +++++++++++++++++++--------
 1 file changed, 106 insertions(+), 47 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
index d542df46ea39314605290311f683010337bfd4bd..aa6f48d838c00d71eff22c18e34e00b93755fd82 100644
--- a/drivers/hwtracing/coresight/coresight-tnoc.c
+++ b/drivers/hwtracing/coresight/coresight-tnoc.c
@@ -34,6 +34,7 @@
  * @base:      memory mapped base address for this component.
  * @dev:       device node for trace_noc_drvdata.
  * @csdev:     component vitals needed by the framework.
+ * @pclk:	APB clock if present, otherwise NULL
  * @spinlock:  serialize enable/disable operation.
  * @atid:      id for the trace packet.
  */
@@ -41,6 +42,7 @@ struct trace_noc_drvdata {
 	void __iomem		*base;
 	struct device		*dev;
 	struct coresight_device	*csdev;
+	struct clk		*pclk;
 	spinlock_t		spinlock;
 	u32			atid;
 };
@@ -51,25 +53,27 @@ static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
 {
 	u32 val;
 
-	/* Set ATID */
-	writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
-
-	/* Set the data word count between 'SYNC' packets */
-	writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
-
-	/* Set the Control register:
-	 * - Set the FLAG packets to 'FLAG' packets
-	 * - Set the FREQ packets to 'FREQ_TS' packets
-	 * - Enable generation of output ATB traffic
-	 */
-
-	val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
-
-	val &= ~TRACE_NOC_CTRL_FLAGTYPE;
-	val |= TRACE_NOC_CTRL_FREQTYPE;
-	val |= TRACE_NOC_CTRL_PORTEN;
-
-	writel(val, drvdata->base + TRACE_NOC_CTRL);
+	if (drvdata->atid) {
+		/* Set ATID */
+		writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
+
+		/* Set the data word count between 'SYNC' packets */
+		writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
+		/* Set the Control register:
+		 * - Set the FLAG packets to 'FLAG' packets
+		 * - Set the FREQ packets to 'FREQ_TS' packets
+		 * - Enable generation of output ATB traffic
+		 */
+
+		val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
+
+		val &= ~TRACE_NOC_CTRL_FLAGTYPE;
+		val |= TRACE_NOC_CTRL_FREQTYPE;
+		val |= TRACE_NOC_CTRL_PORTEN;
+		writel(val, drvdata->base + TRACE_NOC_CTRL);
+	} else {
+		writel(0x1, drvdata->base + TRACE_NOC_CTRL);
+	}
 }
 
 static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
@@ -120,19 +124,6 @@ static const struct coresight_ops trace_noc_cs_ops = {
 	.link_ops	= &trace_noc_link_ops,
 };
 
-static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
-{
-	int atid;
-
-	atid = coresight_trace_id_get_system_id();
-	if (atid < 0)
-		return atid;
-
-	drvdata->atid = atid;
-
-	return 0;
-}
-
 static ssize_t traceid_show(struct device *dev,
 			    struct device_attribute *attr, char *buf)
 {
@@ -158,13 +149,12 @@ static const struct attribute_group *coresight_tnoc_groups[] = {
 	NULL,
 };
 
-static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
+static int _tnoc_probe(struct device *dev, struct resource *res, bool has_id)
 {
-	struct device *dev = &adev->dev;
 	struct coresight_platform_data *pdata;
 	struct trace_noc_drvdata *drvdata;
 	struct coresight_desc desc = { 0 };
-	int ret;
+	int ret, atid = 0;
 
 	desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
 	if (!desc.name)
@@ -173,42 +163,61 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
 	pdata = coresight_get_platform_data(dev);
 	if (IS_ERR(pdata))
 		return PTR_ERR(pdata);
-	adev->dev.platform_data = pdata;
+	dev->platform_data = pdata;
 
 	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
 	if (!drvdata)
 		return -ENOMEM;
 
-	drvdata->dev = &adev->dev;
+	drvdata->dev = dev;
 	dev_set_drvdata(dev, drvdata);
 
-	drvdata->base = devm_ioremap_resource(dev, &adev->res);
+	ret = coresight_get_enable_clocks(dev, &drvdata->pclk, NULL);
+	if (ret)
+		return ret;
+
+	drvdata->base = devm_ioremap_resource(dev, res);
 	if (IS_ERR(drvdata->base))
 		return PTR_ERR(drvdata->base);
 
 	spin_lock_init(&drvdata->spinlock);
 
-	ret = trace_noc_init_default_data(drvdata);
-	if (ret)
-		return ret;
+	if (has_id) {
+		atid = coresight_trace_id_get_system_id();
+		if (atid < 0)
+			return atid;
+	}
+
+	drvdata->atid = atid;
 
 	desc.ops = &trace_noc_cs_ops;
 	desc.type = CORESIGHT_DEV_TYPE_LINK;
 	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
-	desc.pdata = adev->dev.platform_data;
-	desc.dev = &adev->dev;
+	desc.pdata = pdata;
+	desc.dev = dev;
 	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
-	desc.groups = coresight_tnoc_groups;
+	if (has_id)
+		desc.groups = coresight_tnoc_groups;
 	drvdata->csdev = coresight_register(&desc);
-	if (IS_ERR(drvdata->csdev)) {
+	if (IS_ERR(drvdata->csdev) && has_id) {
 		coresight_trace_id_put_system_id(drvdata->atid);
 		return PTR_ERR(drvdata->csdev);
 	}
-	pm_runtime_put(&adev->dev);
 
 	return 0;
 }
 
+static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	int ret;
+
+	ret = _tnoc_probe(&adev->dev, &adev->res, true);
+	if (!ret)
+		pm_runtime_put(&adev->dev);
+
+	return ret;
+}
+
 static void trace_noc_remove(struct amba_device *adev)
 {
 	struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
@@ -236,7 +245,57 @@ static struct amba_driver trace_noc_driver = {
 	.id_table	= trace_noc_ids,
 };
 
-module_amba_driver(trace_noc_driver);
+static int itnoc_probe(struct platform_device *pdev)
+{
+	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	int ret;
+
+	pm_runtime_get_noresume(&pdev->dev);
+	pm_runtime_set_active(&pdev->dev);
+	pm_runtime_enable(&pdev->dev);
+
+	ret = _tnoc_probe(&pdev->dev, res, false);
+	pm_runtime_put(&pdev->dev);
+	if (ret)
+		pm_runtime_disable(&pdev->dev);
+
+	return ret;
+}
+
+static void itnoc_remove(struct platform_device *pdev)
+{
+	struct trace_noc_drvdata *drvdata = platform_get_drvdata(pdev);
+
+	coresight_unregister(drvdata->csdev);
+	pm_runtime_disable(&pdev->dev);
+}
+
+static const struct of_device_id itnoc_of_match[] = {
+	{ .compatible = "qcom,coresight-itnoc" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, itnoc_of_match);
+
+static struct platform_driver itnoc_driver = {
+	.probe = itnoc_probe,
+	.remove = itnoc_remove,
+	.driver = {
+		.name = "coresight-itnoc",
+		.of_match_table = itnoc_of_match,
+	},
+};
+
+static int __init tnoc_init(void)
+{
+	return coresight_init_driver("tnoc", &trace_noc_driver, &itnoc_driver, THIS_MODULE);
+}
+
+static void __exit tnoc_exit(void)
+{
+	coresight_remove_driver(&trace_noc_driver, &itnoc_driver);
+}
+module_init(tnoc_init);
+module_exit(tnoc_exit);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Trace NOC driver");

-- 
2.34.1

[PATCH 3/3] coresight-tnoc: Add runtime PM support for Interconnect TNOC

[Yuanfang Zhang]

This patch adds runtime power management support for platform-based
CoreSight Interconnect TNOC (ITNOC) devices. It introduces suspend and
resume callbacks to manage the APB clock (`pclk`) during device runtime
transitions.

Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tnoc.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
index aa6f48d838c00d71eff22c18e34e00b93755fd82..f12a1698824bc678545319a3f482fd27e67a7352 100644
--- a/drivers/hwtracing/coresight/coresight-tnoc.c
+++ b/drivers/hwtracing/coresight/coresight-tnoc.c
@@ -270,6 +270,31 @@ static void itnoc_remove(struct platform_device *pdev)
 	pm_runtime_disable(&pdev->dev);
 }
 
+#ifdef CONFIG_PM
+static int itnoc_runtime_suspend(struct device *dev)
+{
+	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(drvdata->pclk);
+
+	return 0;
+}
+
+static int itnoc_runtime_resume(struct device *dev)
+{
+	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(drvdata->pclk);
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops itnoc_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(itnoc_runtime_suspend, itnoc_runtime_resume, NULL)
+};
+
 static const struct of_device_id itnoc_of_match[] = {
 	{ .compatible = "qcom,coresight-itnoc" },
 	{}
@@ -282,6 +307,7 @@ static struct platform_driver itnoc_driver = {
 	.driver = {
 		.name = "coresight-itnoc",
 		.of_match_table = itnoc_of_match,
+		.pm = &itnoc_dev_pm_ops,
 	},
 };
 

-- 
2.34.1

Re: [PATCH 1/3] dt-bindings: arm: qcom: Add Coresight Interconnect TNOC

[Krzysztof Kozlowski]

On 15/08/2025 15:18, Yuanfang Zhang wrote:
> Add device tree binding for Qualcomm Coresight Interconnect Trace
> Netwrok On Chip (ITNOC). This TNOC acts as a CoreSight
> graph link that forwards trace data from a subsystem to the
> Aggregator TNOC, without aggregation or ATID functionality.
> 
> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
> ---
>  .../bindings/arm/qcom,coresight-itnoc.yaml         | 108 +++++++++++++++++++++
>  1 file changed, 108 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..fd224e07ce68918b453210763aacda585d5a5ca2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
> @@ -0,0 +1,108 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-itnoc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Interconnect Trace Network On Chip - ITNOC
> +
> +maintainers:
> +  - Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The Interconnect TNOC is a CoreSight graph link that forwards trace data
> +  from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC, it
> +  does not have aggregation and ATID functionality.
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - qcom,coresight-itnoc
> +  required:
> +    - compatible

Why all this? Drop

> +
> +properties:
> +  $nodename:
> +    pattern: "^tnoc(@[0-9a-f]+)?$"

Why are you requiring a non-generic name?

> +
> +  compatible:
> +    items:

No need for items

> +      - const: qcom,coresight-itnoc
> +
> +  reg:
> +    maxItems: 1
> +    description: Base address and size of the ITNOC registers.

Drop, redundant

> +
> +  clock-names:
> +    items:
> +      - const: apb

Drop clock-names, obvious. Also, odd order - names are never before
actual property.

> +
> +  clocks:
> +    maxItems: 1
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      '^port(@[0-9a-f]{1,2})?$':

Why do you have here 255 ports?

> +        description: Input connections from CoreSight Trace Bus
> +        $ref: /schemas/graph.yaml#/properties/port
> +    additionalProperties: false

This goes after $ref

> +
> +  out-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port:
> +        description: out connections to aggregator TNOC
> +        $ref: /schemas/graph.yaml#/properties/port
> +    additionalProperties: false

This goes after ref

> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names

And here different order... Be consistent. See also DTS coding style.

> +  - in-ports
> +  - out-ports
> +
> +additionalProperties: false
> +



Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: arm: qcom: Add Coresight Interconnect TNOC

[yuanfang zhang]

On 8/16/2025 4:33 PM, Krzysztof Kozlowski wrote:
> On 15/08/2025 15:18, Yuanfang Zhang wrote:
>> Add device tree binding for Qualcomm Coresight Interconnect Trace
>> Netwrok On Chip (ITNOC). This TNOC acts as a CoreSight
>> graph link that forwards trace data from a subsystem to the
>> Aggregator TNOC, without aggregation or ATID functionality.
>>
>> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
>> ---
>>  .../bindings/arm/qcom,coresight-itnoc.yaml         | 108 +++++++++++++++++++++
>>  1 file changed, 108 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..fd224e07ce68918b453210763aacda585d5a5ca2
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml
>> @@ -0,0 +1,108 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/qcom,coresight-itnoc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Interconnect Trace Network On Chip - ITNOC
>> +
>> +maintainers:
>> +  - Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
>> +
>> +description: |
> Do not need '|' unless you need to preserve formatting.
sure, will remove it.
>> +  The Interconnect TNOC is a CoreSight graph link that forwards trace data
>> +  from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC, it
>> +  does not have aggregation and ATID functionality.
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - qcom,coresight-itnoc
>> +  required:
>> +    - compatible
> Why all this? Drop
sure.
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^tnoc(@[0-9a-f]+)?$"
> Why are you requiring a non-generic name?
will update the name.
>> +
>> +  compatible:
>> +    items:
> No need for items
sure, will remove it.
>> +      - const: qcom,coresight-itnoc
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: Base address and size of the ITNOC registers.
> Drop, redundant
sure, will remove it.
>> +
>> +  clock-names:
>> +    items:
>> +      - const: apb
> Drop clock-names, obvious. Also, odd order - names are never before
> actual property.
sure, will update the order.
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  in-ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      '#address-cells':
>> +        const: 1
>> +      '#size-cells':
>> +        const: 0
>> +
>> +    patternProperties:
>> +      '^port(@[0-9a-f]{1,2})?$':
> Why do you have here 255 ports?
It supports a maximum of 256 input ports, so it is limited to 0-255.
>> +        description: Input connections from CoreSight Trace Bus
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +    additionalProperties: false
> This goes after $ref
sure, will update.
>> +
>> +  out-ports:
>> +    $ref: /schemas/graph.yaml#/properties/ports
>> +
>> +    properties:
>> +      port:
>> +        description: out connections to aggregator TNOC
>> +        $ref: /schemas/graph.yaml#/properties/port
>> +    additionalProperties: false
> This goes after ref
sure, will update.
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - clocks
>> +  - clock-names
> And here different order... Be consistent. See also DTS coding style.
sure, will update the order.
>> +  - in-ports
>> +  - out-ports
>> +
>> +additionalProperties: false
>> +
>
>
> Best regards,
> Krzysztof

Re: [PATCH 2/3] coresight-tnoc: add platform driver to support Interconnect TNOC

[Leo Yan]

On Fri, Aug 15, 2025 at 06:18:13AM -0700, Yuanfang Zhang wrote:
> This patch adds platform driver support for the CoreSight Interconnect
> TNOC, Interconnect TNOC is a CoreSight link that forwards trace data
> from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC,
> it does not have aggregation and ATID functionality.

Such kind of driver is not complex, it would be fine to had sent driver
in one go for support both AMBA and platform devices.

> Key changes:
> - Add platform driver `coresight-itnoc` with device tree match support.
> - Refactor probe logic into a common `_tnoc_probe()` function.
> - Conditionally initialize ATID only for AMBA-based TNOC blocks.

An AMBA or platform device is only about device probing; it is not
necessarily bound to a device feature.

So I am suspicious of the conclusion that an AMBA-based TNOC always
supports ATID, while a platform device never supports it.

Otherwise, you might need to consider using a DT property to indicate
whether ATID is present or not.

> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tnoc.c | 153 +++++++++++++++++++--------
>  1 file changed, 106 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
> index d542df46ea39314605290311f683010337bfd4bd..aa6f48d838c00d71eff22c18e34e00b93755fd82 100644
> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
> @@ -34,6 +34,7 @@
>   * @base:      memory mapped base address for this component.
>   * @dev:       device node for trace_noc_drvdata.
>   * @csdev:     component vitals needed by the framework.
> + * @pclk:	APB clock if present, otherwise NULL
>   * @spinlock:  serialize enable/disable operation.
>   * @atid:      id for the trace packet.
>   */
> @@ -41,6 +42,7 @@ struct trace_noc_drvdata {
>  	void __iomem		*base;
>  	struct device		*dev;
>  	struct coresight_device	*csdev;
> +	struct clk		*pclk;
>  	spinlock_t		spinlock;
>  	u32			atid;
>  };
> @@ -51,25 +53,27 @@ static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
>  {
>  	u32 val;
>  
> -	/* Set ATID */
> -	writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
> -
> -	/* Set the data word count between 'SYNC' packets */
> -	writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
> -
> -	/* Set the Control register:
> -	 * - Set the FLAG packets to 'FLAG' packets
> -	 * - Set the FREQ packets to 'FREQ_TS' packets
> -	 * - Enable generation of output ATB traffic
> -	 */
> -
> -	val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> -
> -	val &= ~TRACE_NOC_CTRL_FLAGTYPE;
> -	val |= TRACE_NOC_CTRL_FREQTYPE;
> -	val |= TRACE_NOC_CTRL_PORTEN;
> -
> -	writel(val, drvdata->base + TRACE_NOC_CTRL);
> +	if (drvdata->atid) {
> +		/* Set ATID */
> +		writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
> +
> +		/* Set the data word count between 'SYNC' packets */
> +		writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
> +		/* Set the Control register:
> +		 * - Set the FLAG packets to 'FLAG' packets
> +		 * - Set the FREQ packets to 'FREQ_TS' packets
> +		 * - Enable generation of output ATB traffic
> +		 */
> +
> +		val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> +
> +		val &= ~TRACE_NOC_CTRL_FLAGTYPE;
> +		val |= TRACE_NOC_CTRL_FREQTYPE;
> +		val |= TRACE_NOC_CTRL_PORTEN;
> +		writel(val, drvdata->base + TRACE_NOC_CTRL);
> +	} else {
> +		writel(0x1, drvdata->base + TRACE_NOC_CTRL);
> +	}

Change "atid" type from u32 to int, then you could set it as
"-EOPNOTSUPP" for non-AMBA device. Here:

    /* No valid ATID, simply enable the unit */
    if (drvdata->atid == -EOPNOTSUPP) {
        writel(TRACE_NOC_CTRL_PORTEN, drvdata->base + TRACE_NOC_CTRL);
        return;
    }

>  }
>  
>  static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
> @@ -120,19 +124,6 @@ static const struct coresight_ops trace_noc_cs_ops = {
>  	.link_ops	= &trace_noc_link_ops,
>  };
>  
> -static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
> -{
> -	int atid;
> -

Don't need to remove this function. Just check AMBA device case:

    /* ATID is not supported for interconnect TNOC */
    if (!dev_is_amba(drvdata->dev)) {
        drvdata->atid = -EOPNOTSUPP;
        return 0;
    }

> -	atid = coresight_trace_id_get_system_id();
> -	if (atid < 0)
> -		return atid;
> -
> -	drvdata->atid = atid;
> -
> -	return 0;
> -}
> -
>  static ssize_t traceid_show(struct device *dev,
>  			    struct device_attribute *attr, char *buf)
>  {
> @@ -158,13 +149,12 @@ static const struct attribute_group *coresight_tnoc_groups[] = {
>  	NULL,
>  };
>  
> -static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
> +static int _tnoc_probe(struct device *dev, struct resource *res, bool has_id)

As a result, no need the parameter "has_id".

>  {
> -	struct device *dev = &adev->dev;
>  	struct coresight_platform_data *pdata;
>  	struct trace_noc_drvdata *drvdata;
>  	struct coresight_desc desc = { 0 };
> -	int ret;
> +	int ret, atid = 0;
>  
>  	desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
>  	if (!desc.name)
> @@ -173,42 +163,61 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>  	pdata = coresight_get_platform_data(dev);
>  	if (IS_ERR(pdata))
>  		return PTR_ERR(pdata);
> -	adev->dev.platform_data = pdata;
> +	dev->platform_data = pdata;
>  
>  	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>  	if (!drvdata)
>  		return -ENOMEM;
>  
> -	drvdata->dev = &adev->dev;
> +	drvdata->dev = dev;
>  	dev_set_drvdata(dev, drvdata);
>  
> -	drvdata->base = devm_ioremap_resource(dev, &adev->res);
> +	ret = coresight_get_enable_clocks(dev, &drvdata->pclk, NULL);
> +	if (ret)
> +		return ret;
> +
> +	drvdata->base = devm_ioremap_resource(dev, res);
>  	if (IS_ERR(drvdata->base))
>  		return PTR_ERR(drvdata->base);
>  
>  	spin_lock_init(&drvdata->spinlock);
>  
> -	ret = trace_noc_init_default_data(drvdata);
> -	if (ret)
> -		return ret;
> +	if (has_id) {
> +		atid = coresight_trace_id_get_system_id();
> +		if (atid < 0)
> +			return atid;
> +	}
> +
> +	drvdata->atid = atid;

Drop this change and simply keep the code for invoking
trace_noc_init_default_data().

>  	desc.ops = &trace_noc_cs_ops;
>  	desc.type = CORESIGHT_DEV_TYPE_LINK;
>  	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
> -	desc.pdata = adev->dev.platform_data;
> -	desc.dev = &adev->dev;
> +	desc.pdata = pdata;
> +	desc.dev = dev;
>  	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
> -	desc.groups = coresight_tnoc_groups;
> +	if (has_id)
> +		desc.groups = coresight_tnoc_groups;

No need to change for groups.

Just return "-EOPNOTSUPP" in traceid_show() if drvdata->atid is negative.
Or, you could use the .is_visible() callback to decide if the "trace_id"
node appears or not.

>  	drvdata->csdev = coresight_register(&desc);
> -	if (IS_ERR(drvdata->csdev)) {
> +	if (IS_ERR(drvdata->csdev) && has_id) {
>  		coresight_trace_id_put_system_id(drvdata->atid);
>  		return PTR_ERR(drvdata->csdev);
>  	}
> -	pm_runtime_put(&adev->dev);
>  
>  	return 0;
>  }
>  
> +static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	int ret;
> +
> +	ret = _tnoc_probe(&adev->dev, &adev->res, true);
> +	if (!ret)
> +		pm_runtime_put(&adev->dev);
> +
> +	return ret;
> +}
> +
>  static void trace_noc_remove(struct amba_device *adev)
>  {
>  	struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> @@ -236,7 +245,57 @@ static struct amba_driver trace_noc_driver = {
>  	.id_table	= trace_noc_ids,
>  };
>  
> -module_amba_driver(trace_noc_driver);
> +static int itnoc_probe(struct platform_device *pdev)
> +{
> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	int ret;
> +
> +	pm_runtime_get_noresume(&pdev->dev);
> +	pm_runtime_set_active(&pdev->dev);
> +	pm_runtime_enable(&pdev->dev);
> +
> +	ret = _tnoc_probe(&pdev->dev, res, false);
> +	pm_runtime_put(&pdev->dev);
> +	if (ret)
> +		pm_runtime_disable(&pdev->dev);
> +
> +	return ret;
> +}
> +
> +static void itnoc_remove(struct platform_device *pdev)
> +{
> +	struct trace_noc_drvdata *drvdata = platform_get_drvdata(pdev);
> +
> +	coresight_unregister(drvdata->csdev);
> +	pm_runtime_disable(&pdev->dev);
> +}
> +
> +static const struct of_device_id itnoc_of_match[] = {
> +	{ .compatible = "qcom,coresight-itnoc" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, itnoc_of_match);
> +
> +static struct platform_driver itnoc_driver = {
> +	.probe = itnoc_probe,
> +	.remove = itnoc_remove,
> +	.driver = {
> +		.name = "coresight-itnoc",
> +		.of_match_table = itnoc_of_match,

You might need to set:

    .suppress_bind_attrs = true,

Thanks,
Leo

> +	},
> +};
> +
> +static int __init tnoc_init(void)
> +{
> +	return coresight_init_driver("tnoc", &trace_noc_driver, &itnoc_driver, THIS_MODULE);
> +}
> +
> +static void __exit tnoc_exit(void)
> +{
> +	coresight_remove_driver(&trace_noc_driver, &itnoc_driver);
> +}
> +module_init(tnoc_init);
> +module_exit(tnoc_exit);
>  
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("Trace NOC driver");
> 
> -- 
> 2.34.1
> 
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org

Re: [PATCH 3/3] coresight-tnoc: Add runtime PM support for Interconnect TNOC

[Leo Yan]

On Fri, Aug 15, 2025 at 06:18:14AM -0700, Yuanfang Zhang wrote:
> This patch adds runtime power management support for platform-based
> CoreSight Interconnect TNOC (ITNOC) devices. It introduces suspend and
> resume callbacks to manage the APB clock (`pclk`) during device runtime
> transitions.
> 
> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
> ---
>  drivers/hwtracing/coresight/coresight-tnoc.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
> index aa6f48d838c00d71eff22c18e34e00b93755fd82..f12a1698824bc678545319a3f482fd27e67a7352 100644
> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
> @@ -270,6 +270,31 @@ static void itnoc_remove(struct platform_device *pdev)
>  	pm_runtime_disable(&pdev->dev);
>  }
>  
> +#ifdef CONFIG_PM
> +static int itnoc_runtime_suspend(struct device *dev)
> +{
> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(drvdata->pclk);
> +
> +	return 0;
> +}
> +
> +static int itnoc_runtime_resume(struct device *dev)
> +{
> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(drvdata->pclk);
> +
> +	return ret;

Here can be simplified:

    return clk_prepare_enable(drvdata->pclk);

> +}
> +#endif
> +
> +static const struct dev_pm_ops itnoc_dev_pm_ops = {
> +	SET_RUNTIME_PM_OPS(itnoc_runtime_suspend, itnoc_runtime_resume, NULL)
> +};
> +
>  static const struct of_device_id itnoc_of_match[] = {
>  	{ .compatible = "qcom,coresight-itnoc" },
>  	{}
> @@ -282,6 +307,7 @@ static struct platform_driver itnoc_driver = {
>  	.driver = {
>  		.name = "coresight-itnoc",
>  		.of_match_table = itnoc_of_match,
> +		.pm = &itnoc_dev_pm_ops,
>  	},
>  };
>  
> 
> -- 
> 2.34.1
> 
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org
> To unsubscribe send an email to coresight-leave@lists.linaro.org

Re: [PATCH 2/3] coresight-tnoc: add platform driver to support Interconnect TNOC

[yuanfang zhang]

On 8/18/2025 10:27 PM, Leo Yan wrote:
> On Fri, Aug 15, 2025 at 06:18:13AM -0700, Yuanfang Zhang wrote:
>> This patch adds platform driver support for the CoreSight Interconnect
>> TNOC, Interconnect TNOC is a CoreSight link that forwards trace data
>> from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC,
>> it does not have aggregation and ATID functionality.
> 
> Such kind of driver is not complex, it would be fine to had sent driver
> in one go for support both AMBA and platform devices.
> 
>> Key changes:
>> - Add platform driver `coresight-itnoc` with device tree match support.
>> - Refactor probe logic into a common `_tnoc_probe()` function.
>> - Conditionally initialize ATID only for AMBA-based TNOC blocks.
> 
> An AMBA or platform device is only about device probing; it is not
> necessarily bound to a device feature.
> 
> So I am suspicious of the conclusion that an AMBA-based TNOC always
> supports ATID, while a platform device never supports it.
> 
> Otherwise, you might need to consider using a DT property to indicate
> whether ATID is present or not.
> 

Unlike the AMBA-based design, ITNOC not only lacks ATID support but also does not
include PID registers. This is why a separate platform driver is required to support it.

>> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tnoc.c | 153 +++++++++++++++++++--------
>>  1 file changed, 106 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
>> index d542df46ea39314605290311f683010337bfd4bd..aa6f48d838c00d71eff22c18e34e00b93755fd82 100644
>> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
>> @@ -34,6 +34,7 @@
>>   * @base:      memory mapped base address for this component.
>>   * @dev:       device node for trace_noc_drvdata.
>>   * @csdev:     component vitals needed by the framework.
>> + * @pclk:	APB clock if present, otherwise NULL
>>   * @spinlock:  serialize enable/disable operation.
>>   * @atid:      id for the trace packet.
>>   */
>> @@ -41,6 +42,7 @@ struct trace_noc_drvdata {
>>  	void __iomem		*base;
>>  	struct device		*dev;
>>  	struct coresight_device	*csdev;
>> +	struct clk		*pclk;
>>  	spinlock_t		spinlock;
>>  	u32			atid;
>>  };
>> @@ -51,25 +53,27 @@ static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
>>  {
>>  	u32 val;
>>  
>> -	/* Set ATID */
>> -	writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
>> -
>> -	/* Set the data word count between 'SYNC' packets */
>> -	writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
>> -
>> -	/* Set the Control register:
>> -	 * - Set the FLAG packets to 'FLAG' packets
>> -	 * - Set the FREQ packets to 'FREQ_TS' packets
>> -	 * - Enable generation of output ATB traffic
>> -	 */
>> -
>> -	val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
>> -
>> -	val &= ~TRACE_NOC_CTRL_FLAGTYPE;
>> -	val |= TRACE_NOC_CTRL_FREQTYPE;
>> -	val |= TRACE_NOC_CTRL_PORTEN;
>> -
>> -	writel(val, drvdata->base + TRACE_NOC_CTRL);
>> +	if (drvdata->atid) {
>> +		/* Set ATID */
>> +		writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
>> +
>> +		/* Set the data word count between 'SYNC' packets */
>> +		writel_relaxed(TRACE_NOC_SYNC_INTERVAL, drvdata->base + TRACE_NOC_SYNCR);
>> +		/* Set the Control register:
>> +		 * - Set the FLAG packets to 'FLAG' packets
>> +		 * - Set the FREQ packets to 'FREQ_TS' packets
>> +		 * - Enable generation of output ATB traffic
>> +		 */
>> +
>> +		val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
>> +
>> +		val &= ~TRACE_NOC_CTRL_FLAGTYPE;
>> +		val |= TRACE_NOC_CTRL_FREQTYPE;
>> +		val |= TRACE_NOC_CTRL_PORTEN;
>> +		writel(val, drvdata->base + TRACE_NOC_CTRL);
>> +	} else {
>> +		writel(0x1, drvdata->base + TRACE_NOC_CTRL);
>> +	}
> 
> Change "atid" type from u32 to int, then you could set it as
> "-EOPNOTSUPP" for non-AMBA device. Here:
> 
>     /* No valid ATID, simply enable the unit */
>     if (drvdata->atid == -EOPNOTSUPP) {
>         writel(TRACE_NOC_CTRL_PORTEN, drvdata->base + TRACE_NOC_CTRL);
>         return;
>     }

sure, will update.
> 
>>  }
>>  
>>  static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
>> @@ -120,19 +124,6 @@ static const struct coresight_ops trace_noc_cs_ops = {
>>  	.link_ops	= &trace_noc_link_ops,
>>  };
>>  
>> -static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
>> -{
>> -	int atid;
>> -
> 
> Don't need to remove this function. Just check AMBA device case:
> 
>     /* ATID is not supported for interconnect TNOC */
>     if (!dev_is_amba(drvdata->dev)) {
>         drvdata->atid = -EOPNOTSUPP;
>         return 0;
>     }
> 
sure, will update.
>> -	atid = coresight_trace_id_get_system_id();
>> -	if (atid < 0)
>> -		return atid;
>> -
>> -	drvdata->atid = atid;
>> -
>> -	return 0;
>> -}
>> -
>>  static ssize_t traceid_show(struct device *dev,
>>  			    struct device_attribute *attr, char *buf)
>>  {
>> @@ -158,13 +149,12 @@ static const struct attribute_group *coresight_tnoc_groups[] = {
>>  	NULL,
>>  };
>>  
>> -static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>> +static int _tnoc_probe(struct device *dev, struct resource *res, bool has_id)
> 
> As a result, no need the parameter "has_id".
> 
sure, will update.
>>  {
>> -	struct device *dev = &adev->dev;
>>  	struct coresight_platform_data *pdata;
>>  	struct trace_noc_drvdata *drvdata;
>>  	struct coresight_desc desc = { 0 };
>> -	int ret;
>> +	int ret, atid = 0;
>>  
>>  	desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
>>  	if (!desc.name)
>> @@ -173,42 +163,61 @@ static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>>  	pdata = coresight_get_platform_data(dev);
>>  	if (IS_ERR(pdata))
>>  		return PTR_ERR(pdata);
>> -	adev->dev.platform_data = pdata;
>> +	dev->platform_data = pdata;
>>  
>>  	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>>  	if (!drvdata)
>>  		return -ENOMEM;
>>  
>> -	drvdata->dev = &adev->dev;
>> +	drvdata->dev = dev;
>>  	dev_set_drvdata(dev, drvdata);
>>  
>> -	drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> +	ret = coresight_get_enable_clocks(dev, &drvdata->pclk, NULL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	drvdata->base = devm_ioremap_resource(dev, res);
>>  	if (IS_ERR(drvdata->base))
>>  		return PTR_ERR(drvdata->base);
>>  
>>  	spin_lock_init(&drvdata->spinlock);
>>  
>> -	ret = trace_noc_init_default_data(drvdata);
>> -	if (ret)
>> -		return ret;
>> +	if (has_id) {
>> +		atid = coresight_trace_id_get_system_id();
>> +		if (atid < 0)
>> +			return atid;
>> +	}
>> +
>> +	drvdata->atid = atid;
> 
> Drop this change and simply keep the code for invoking
> trace_noc_init_default_data().
> 
sure, will update.
>>  	desc.ops = &trace_noc_cs_ops;
>>  	desc.type = CORESIGHT_DEV_TYPE_LINK;
>>  	desc.subtype.link_subtype = CORESIGHT_DEV_SUBTYPE_LINK_MERG;
>> -	desc.pdata = adev->dev.platform_data;
>> -	desc.dev = &adev->dev;
>> +	desc.pdata = pdata;
>> +	desc.dev = dev;
>>  	desc.access = CSDEV_ACCESS_IOMEM(drvdata->base);
>> -	desc.groups = coresight_tnoc_groups;
>> +	if (has_id)
>> +		desc.groups = coresight_tnoc_groups;
> 
> No need to change for groups.
> 
> Just return "-EOPNOTSUPP" in traceid_show() if drvdata->atid is negative.
> Or, you could use the .is_visible() callback to decide if the "trace_id"
> node appears or not.
> 
sure, will updata.
>>  	drvdata->csdev = coresight_register(&desc);
>> -	if (IS_ERR(drvdata->csdev)) {
>> +	if (IS_ERR(drvdata->csdev) && has_id) {
>>  		coresight_trace_id_put_system_id(drvdata->atid);
>>  		return PTR_ERR(drvdata->csdev);
>>  	}
>> -	pm_runtime_put(&adev->dev);
>>  
>>  	return 0;
>>  }
>>  
>> +static int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> +	int ret;
>> +
>> +	ret = _tnoc_probe(&adev->dev, &adev->res, true);
>> +	if (!ret)
>> +		pm_runtime_put(&adev->dev);
>> +
>> +	return ret;
>> +}
>> +
>>  static void trace_noc_remove(struct amba_device *adev)
>>  {
>>  	struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> @@ -236,7 +245,57 @@ static struct amba_driver trace_noc_driver = {
>>  	.id_table	= trace_noc_ids,
>>  };
>>  
>> -module_amba_driver(trace_noc_driver);
>> +static int itnoc_probe(struct platform_device *pdev)
>> +{
>> +	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> +	int ret;
>> +
>> +	pm_runtime_get_noresume(&pdev->dev);
>> +	pm_runtime_set_active(&pdev->dev);
>> +	pm_runtime_enable(&pdev->dev);
>> +
>> +	ret = _tnoc_probe(&pdev->dev, res, false);
>> +	pm_runtime_put(&pdev->dev);
>> +	if (ret)
>> +		pm_runtime_disable(&pdev->dev);
>> +
>> +	return ret;
>> +}
>> +
>> +static void itnoc_remove(struct platform_device *pdev)
>> +{
>> +	struct trace_noc_drvdata *drvdata = platform_get_drvdata(pdev);
>> +
>> +	coresight_unregister(drvdata->csdev);
>> +	pm_runtime_disable(&pdev->dev);
>> +}
>> +
>> +static const struct of_device_id itnoc_of_match[] = {
>> +	{ .compatible = "qcom,coresight-itnoc" },
>> +	{}
>> +};
>> +MODULE_DEVICE_TABLE(of, itnoc_of_match);
>> +
>> +static struct platform_driver itnoc_driver = {
>> +	.probe = itnoc_probe,
>> +	.remove = itnoc_remove,
>> +	.driver = {
>> +		.name = "coresight-itnoc",
>> +		.of_match_table = itnoc_of_match,
> 
> You might need to set:
> 
>     .suppress_bind_attrs = true,
> 
sure, will update.
> Thanks,
> Leo
> 
>> +	},
>> +};
>> +
>> +static int __init tnoc_init(void)
>> +{
>> +	return coresight_init_driver("tnoc", &trace_noc_driver, &itnoc_driver, THIS_MODULE);
>> +}
>> +
>> +static void __exit tnoc_exit(void)
>> +{
>> +	coresight_remove_driver(&trace_noc_driver, &itnoc_driver);
>> +}
>> +module_init(tnoc_init);
>> +module_exit(tnoc_exit);
>>  
>>  MODULE_LICENSE("GPL");
>>  MODULE_DESCRIPTION("Trace NOC driver");
>>
>> -- 
>> 2.34.1
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org

Re: [PATCH 3/3] coresight-tnoc: Add runtime PM support for Interconnect TNOC

[yuanfang zhang]

On 8/18/2025 10:30 PM, Leo Yan wrote:
> On Fri, Aug 15, 2025 at 06:18:14AM -0700, Yuanfang Zhang wrote:
>> This patch adds runtime power management support for platform-based
>> CoreSight Interconnect TNOC (ITNOC) devices. It introduces suspend and
>> resume callbacks to manage the APB clock (`pclk`) during device runtime
>> transitions.
>>
>> Signed-off-by: Yuanfang Zhang <yuanfang.zhang@oss.qualcomm.com>
>> ---
>>  drivers/hwtracing/coresight/coresight-tnoc.c | 26 ++++++++++++++++++++++++++
>>  1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
>> index aa6f48d838c00d71eff22c18e34e00b93755fd82..f12a1698824bc678545319a3f482fd27e67a7352 100644
>> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
>> @@ -270,6 +270,31 @@ static void itnoc_remove(struct platform_device *pdev)
>>  	pm_runtime_disable(&pdev->dev);
>>  }
>>  
>> +#ifdef CONFIG_PM
>> +static int itnoc_runtime_suspend(struct device *dev)
>> +{
>> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
>> +
>> +	clk_disable_unprepare(drvdata->pclk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int itnoc_runtime_resume(struct device *dev)
>> +{
>> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(drvdata->pclk);
>> +
>> +	return ret;
> 
> Here can be simplified:
> 
>     return clk_prepare_enable(drvdata->pclk);
> 
sure, will update.
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops itnoc_dev_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(itnoc_runtime_suspend, itnoc_runtime_resume, NULL)
>> +};
>> +
>>  static const struct of_device_id itnoc_of_match[] = {
>>  	{ .compatible = "qcom,coresight-itnoc" },
>>  	{}
>> @@ -282,6 +307,7 @@ static struct platform_driver itnoc_driver = {
>>  	.driver = {
>>  		.name = "coresight-itnoc",
>>  		.of_match_table = itnoc_of_match,
>> +		.pm = &itnoc_dev_pm_ops,
>>  	},
>>  };
>>  
>>
>> -- 
>> 2.34.1
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org