[PATCH v3 0/2] coresight: Add Coresight Trace Network On Chip driver

[PATCH v3 0/2] coresight: Add Coresight Trace Network On Chip driver

[Yuanfang Zhang]

The Trace Network On Chip (TNOC) is an integration hierarchy which is a
hardware component that integrates the functionalities of TPDA and
funnels. It collects trace form subsystems and transfers to coresight
sink.

Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
---
Changes in v3:
- Remove unnecessary sysfs nodes.
- update commit messages.
- Use 'writel' instead of 'write_relaxed' when writing to the register for the last time.
- Add trace_id ops.
- Link to v2: https://lore.kernel.org/r/20250226-trace-noc-driver-v2-0-8afc6584afc5@quicinc.com

Changes in v2:
- Modified the format of DT binging file.
- Fix compile warnings.
- Link to v1: https://lore.kernel.org/r/46643089-b88d-49dc-be05-7bf0bb21f847@quicinc.com

---
Yuanfang Zhang (2):
      dt-bindings: arm: Add device Trace Network On Chip definition
      coresight: add coresight Trace Network On Chip driver

 .../bindings/arm/qcom,coresight-tnoc.yaml          | 111 ++++++++++++
 drivers/hwtracing/coresight/Kconfig                |  13 ++
 drivers/hwtracing/coresight/Makefile               |   1 +
 drivers/hwtracing/coresight/coresight-tnoc.c       | 186 +++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tnoc.h       |  34 ++++
 5 files changed, 345 insertions(+)
---
base-commit: a2cc6ff5ec8f91bc463fd3b0c26b61166a07eb11
change-id: 20250403-trace-noc-f8286b30408e

Best regards,
-- 
Yuanfang Zhang <quic_yuanfang@quicinc.com>

[PATCH v3 1/2] dt-bindings: arm: Add device Trace Network On Chip definition

[Yuanfang Zhang]

Add a new coresight-tnoc.yaml file to describe the bindings required to
define Trace Network On Chip (TNOC) in device trees. TNOC is an
integration hierarchy which is a hardware component that integrates the
functionalities of TPDA and funnels. It collects trace form subsystems
and transfers to coresight sink.

Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
---
 .../bindings/arm/qcom,coresight-tnoc.yaml          | 111 +++++++++++++++++++++
 1 file changed, 111 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..709c1bc63db48c29bb2b33e7a795a5999768c5e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/qcom,coresight-tnoc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Trace Network On Chip - TNOC
+
+maintainers:
+  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
+
+description:
+  The Trace Network On Chip (TNOC) is an integration hierarchy hardware
+  component that integrates the functionalities of TPDA and funnels.
+
+  It sits in the different subsystem of SOC and aggregates the trace and
+  transports it to Aggregation TNOC or to coresight trace sink eventually.
+  TNOC embeds bridges for all the interfaces APB, ATB, TPDA and NTS (Narrow
+  Time Stamp).
+
+  TNOC can take inputs from different trace sources i.e. ATB, TPDM.
+
+# Need a custom select here or 'arm,primecell' will match on lots of nodes
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - qcom,coresight-tnoc
+  required:
+    - compatible
+
+properties:
+  $nodename:
+    pattern: "^tn(@[0-9a-f]+)$"
+
+  compatible:
+    items:
+      - const: qcom,coresight-tnoc
+      - const: arm,primecell
+
+  reg:
+    maxItems: 1
+
+  clock-names:
+    items:
+      - const: apb_pclk
+
+  clocks:
+    items:
+      - description: APB register access clock
+
+  in-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    patternProperties:
+      '^port(@[0-9a-f]{1,2})?$':
+        description: Input connections from CoreSight Trace Bus
+        $ref: /schemas/graph.yaml#/properties/port
+
+  out-ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+    additionalProperties: false
+
+    properties:
+      port:
+        description:
+          Output connection to CoreSight Trace Bus
+        $ref: /schemas/graph.yaml#/properties/port
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - in-ports
+  - out-ports
+
+additionalProperties: false
+
+examples:
+  - |
+    tn@109ab000  {
+      compatible = "qcom,coresight-tnoc", "arm,primecell";
+      reg = <0x0 0x109ab000 0x0 0x4200>;
+
+      clocks = <&aoss_qmp>;
+      clock-names = "apb_pclk";
+
+      in-ports {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        port@0 {
+          reg = <0>;
+
+          tn_ag_in_tpdm_gcc: endpoint {
+            remote-endpoint = <&tpdm_gcc_out_tn_ag>;
+          };
+        };
+      };
+
+      out-ports {
+        port {
+          tn_ag_out_funnel_in1: endpoint {
+            remote-endpoint = <&funnel_in1_in_tn_ag>;
+          };
+        };
+      };
+    };
+...

-- 
2.34.1

Re: [PATCH v3 1/2] dt-bindings: arm: Add device Trace Network On Chip definition

[Rob Herring (Arm)]

On Fri, 11 Apr 2025 16:57:52 +0800, Yuanfang Zhang wrote:
> Add a new coresight-tnoc.yaml file to describe the bindings required to
> define Trace Network On Chip (TNOC) in device trees. TNOC is an
> integration hierarchy which is a hardware component that integrates the
> functionalities of TPDA and funnels. It collects trace form subsystems
> and transfers to coresight sink.
> 
> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
> ---
>  .../bindings/arm/qcom,coresight-tnoc.yaml          | 111 +++++++++++++++++++++
>  1 file changed, 111 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.example.dtb: tn@109ab000 (qcom,coresight-tnoc): reg: [[0, 278573056], [0, 16896]] is too long
	from schema $id: http://devicetree.org/schemas/arm/qcom,coresight-tnoc.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250411-trace-noc-v3-1-1f19ddf7699b@quicinc.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH v3 1/2] dt-bindings: arm: Add device Trace Network On Chip definition

[Rob Herring]

On Fri, Apr 11, 2025 at 04:57:52PM +0800, Yuanfang Zhang wrote:
> Add a new coresight-tnoc.yaml file to describe the bindings required to
> define Trace Network On Chip (TNOC) in device trees. TNOC is an
> integration hierarchy which is a hardware component that integrates the
> functionalities of TPDA and funnels. It collects trace form subsystems
> and transfers to coresight sink.
> 
> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
> ---
>  .../bindings/arm/qcom,coresight-tnoc.yaml          | 111 +++++++++++++++++++++
>  1 file changed, 111 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..709c1bc63db48c29bb2b33e7a795a5999768c5e7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/qcom,coresight-tnoc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Trace Network On Chip - TNOC
> +
> +maintainers:
> +  - Yuanfang Zhang <quic_yuanfang@quicinc.com>
> +
> +description:

'>' is needed for paragraphs.

> +  The Trace Network On Chip (TNOC) is an integration hierarchy hardware
> +  component that integrates the functionalities of TPDA and funnels.
> +
> +  It sits in the different subsystem of SOC and aggregates the trace and
> +  transports it to Aggregation TNOC or to coresight trace sink eventually.
> +  TNOC embeds bridges for all the interfaces APB, ATB, TPDA and NTS (Narrow
> +  Time Stamp).
> +
> +  TNOC can take inputs from different trace sources i.e. ATB, TPDM.
> +
> +# Need a custom select here or 'arm,primecell' will match on lots of nodes
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - qcom,coresight-tnoc
> +  required:
> +    - compatible
> +
> +properties:
> +  $nodename:
> +    pattern: "^tn(@[0-9a-f]+)$"
> +
> +  compatible:
> +    items:
> +      - const: qcom,coresight-tnoc
> +      - const: arm,primecell
> +
> +  reg:
> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +      - const: apb_pclk
> +
> +  clocks:
> +    items:
> +      - description: APB register access clock
> +
> +  in-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    patternProperties:
> +      '^port(@[0-9a-f]{1,2})?$':
> +        description: Input connections from CoreSight Trace Bus
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +  out-ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    additionalProperties: false
> +
> +    properties:
> +      port:
> +        description:
> +          Output connection to CoreSight Trace Bus
> +        $ref: /schemas/graph.yaml#/properties/port
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - in-ports
> +  - out-ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    tn@109ab000  {
> +      compatible = "qcom,coresight-tnoc", "arm,primecell";
> +      reg = <0x0 0x109ab000 0x0 0x4200>;
> +
> +      clocks = <&aoss_qmp>;
> +      clock-names = "apb_pclk";
> +
> +      in-ports {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        port@0 {
> +          reg = <0>;
> +
> +          tn_ag_in_tpdm_gcc: endpoint {
> +            remote-endpoint = <&tpdm_gcc_out_tn_ag>;
> +          };
> +        };
> +      };
> +
> +      out-ports {
> +        port {
> +          tn_ag_out_funnel_in1: endpoint {
> +            remote-endpoint = <&funnel_in1_in_tn_ag>;
> +          };
> +        };
> +      };
> +    };
> +...
> 
> -- 
> 2.34.1
> 

[PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Yuanfang Zhang]

Add a driver to support Coresight device Trace Network On Chip (TNOC),
which is an integration hierarchy integrating functionalities of TPDA
and funnels. It aggregates the trace and transports to coresight trace
bus.

Compared to current configuration, it has the following advantages:
1. Reduce wires between subsystems.
2. Continue cleaning the infrastructure.
3. Reduce Data overhead by transporting raw data from source to target.

  +------------------------+                +-------------------------+
  | Video Subsystem        |                |Video Subsystem          |
  |       +-------------+  |                |       +------------+    |
  |       | Video TPDM  |  |                |       | Video TPDM |    |
  |       +-------------+  |                |       +------------+    |
  |            |           |                |              |          |
  |            v           |                |              v          |
  |   +---------------+    |                |        +-----------+    |
  |   | Video funnel  |    |                |        |Video TNOC |    |
  |   +---------------+    |                |        +-----------+    |
  +------------|-----------+                +------------|------------+
               |                                         |
               v-----+                                   |
+--------------------|---------+                         |
|  Multimedia        v         |                         |
|  Subsystem   +--------+      |                         |
|              |  TPDA  |      |                         v
|              +----|---+      |              +---------------------+
|                   |          |              |   Aggoregator TNOC  |
|                   |          |              +----------|----------+
|                   +--        |                         |
|                     |        |                         |
|                     |        |                         |
|              +------v-----+  |                         |
|              |  Funnel    |  |                         |
|              +------------+  |                         |
+----------------|-------------+                         |
                 |                                       |
                 v                                       v
      +--------------------+                    +------------------+
      |   Coresight Sink   |                    |  Coresight Sink  |
      +--------------------+                    +------------------+

       Current Configuration                            TNOC

Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
---
 drivers/hwtracing/coresight/Kconfig          |  13 ++
 drivers/hwtracing/coresight/Makefile         |   1 +
 drivers/hwtracing/coresight/coresight-tnoc.c | 186 +++++++++++++++++++++++++++
 drivers/hwtracing/coresight/coresight-tnoc.h |  34 +++++
 4 files changed, 234 insertions(+)

diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
index ecd7086a5b83e86b6bc8ea039d6d26a628334ed3..f20600d58f38568f8178f69d3f678c2df2cbca7e 100644
--- a/drivers/hwtracing/coresight/Kconfig
+++ b/drivers/hwtracing/coresight/Kconfig
@@ -259,4 +259,17 @@ config CORESIGHT_DUMMY
 
 	  To compile this driver as a module, choose M here: the module will be
 	  called coresight-dummy.
+
+config CORESIGHT_TNOC
+	tristate "Coresight Trace Network On Chip driver"
+	help
+	  This driver provides support for Trace Network On Chip (TNOC) component.
+	  TNOC is an interconnect used to collect traces from various subsystems
+	  and transport to a coresight trace sink. It sits in the different
+	  tiles of SOC and aggregates the trace local to the tile and transports
+	  it another tile or to coresight trace sink eventually.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called coresight-tnoc.
+
 endif
diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
index 8e62c3150aebd1e82b445fafc97a0a9b44397b0e..880e9ed6bfe9c711492c6a2cd972751f56dd8010 100644
--- a/drivers/hwtracing/coresight/Makefile
+++ b/drivers/hwtracing/coresight/Makefile
@@ -34,6 +34,7 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o
 obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o
 obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
 					   coresight-replicator.o
+obj-$(CONFIG_CORESIGHT_TNOC) += coresight-tnoc.o
 obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
 coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \
 		     coresight-etm3x-sysfs.o
diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
new file mode 100644
index 0000000000000000000000000000000000000000..2ec4ead892f0166a3e84f777679c0f73f5da0e83
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tnoc.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+ #include <linux/amba/bus.h>
+ #include <linux/coresight.h>
+ #include <linux/device.h>
+ #include <linux/io.h>
+ #include <linux/kernel.h>
+ #include <linux/module.h>
+ #include <linux/of.h>
+ #include <linux/platform_device.h>
+
+#include "coresight-priv.h"
+#include "coresight-tnoc.h"
+#include "coresight-trace-id.h"
+
+DEFINE_CORESIGHT_DEVLIST(trace_noc_devs, "traceNoc");
+
+static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
+{
+	u32 val;
+
+	/* Set ATID */
+	writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
+
+	/* Config sync CR */
+	writel_relaxed(TRACE_NOC_SYN_VAL, drvdata->base + TRACE_NOC_SYNCR);
+
+	/* Set Ctrl register */
+	val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
+
+	val = val & ~TRACE_NOC_CTRL_FLAGTYPE;
+	val = val | TRACE_NOC_CTRL_FREQTYPE;
+	val = val | TRACE_NOC_CTRL_PORTEN;
+
+	writel(val, drvdata->base + TRACE_NOC_CTRL);
+}
+
+static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
+			    struct coresight_connection *outport)
+{
+	struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	spin_lock(&drvdata->spinlock);
+	if (csdev->refcnt == 0)
+		trace_noc_enable_hw(drvdata);
+
+	csdev->refcnt++;
+	spin_unlock(&drvdata->spinlock);
+
+	dev_dbg(drvdata->dev, "Trace NOC is enabled\n");
+	return 0;
+}
+
+static void trace_noc_disable_hw(struct trace_noc_drvdata *drvdata)
+{
+	writel(0x0, drvdata->base + TRACE_NOC_CTRL);
+}
+
+static void trace_noc_disable(struct coresight_device *csdev, struct coresight_connection *inport,
+			      struct coresight_connection *outport)
+{
+	struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	spin_lock(&drvdata->spinlock);
+	if (--csdev->refcnt == 0)
+		trace_noc_disable_hw(drvdata);
+
+	spin_unlock(&drvdata->spinlock);
+	dev_dbg(drvdata->dev, "Trace NOC is disabled\n");
+}
+
+static int trace_noc_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode,
+			__maybe_unused struct coresight_device *sink)
+{
+	struct trace_noc_drvdata *drvdata;
+
+	drvdata = dev_get_drvdata(csdev->dev.parent);
+
+	return drvdata->atid;
+}
+
+static const struct coresight_ops_link trace_noc_link_ops = {
+	.enable		= trace_noc_enable,
+	.disable	= trace_noc_disable,
+};
+
+static const struct coresight_ops trace_noc_cs_ops = {
+	.trace_id	= trace_noc_id,
+	.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 int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
+{
+	struct device *dev = &adev->dev;
+	struct coresight_platform_data *pdata;
+	struct trace_noc_drvdata *drvdata;
+	struct coresight_desc desc = { 0 };
+	int ret;
+
+	desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
+	if (!desc.name)
+		return -ENOMEM;
+	pdata = coresight_get_platform_data(dev);
+	if (IS_ERR(pdata))
+		return PTR_ERR(pdata);
+	adev->dev.platform_data = pdata;
+
+	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
+	if (!drvdata)
+		return -ENOMEM;
+
+	drvdata->dev = &adev->dev;
+	dev_set_drvdata(dev, drvdata);
+
+	drvdata->base = devm_ioremap_resource(dev, &adev->res);
+	if (!drvdata->base)
+		return -ENOMEM;
+
+	spin_lock_init(&drvdata->spinlock);
+
+	ret = trace_noc_init_default_data(drvdata);
+	if (ret)
+		return ret;
+
+	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.access = CSDEV_ACCESS_IOMEM(drvdata->base);
+	drvdata->csdev = coresight_register(&desc);
+	if (IS_ERR(drvdata->csdev))
+		return PTR_ERR(drvdata->csdev);
+
+	pm_runtime_put(&adev->dev);
+
+	return 0;
+}
+
+static void trace_noc_remove(struct amba_device *adev)
+{
+	struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
+
+	coresight_trace_id_put_system_id(drvdata->atid);
+	coresight_unregister(drvdata->csdev);
+}
+
+static struct amba_id trace_noc_ids[] = {
+	{
+		.id     = 0x000f0c00,
+		.mask   = 0x000fff00,
+	},
+	{},
+};
+MODULE_DEVICE_TABLE(amba, trace_noc_ids);
+
+static struct amba_driver trace_noc_driver = {
+	.drv = {
+		.name   = "coresight-trace-noc",
+		.suppress_bind_attrs = true,
+	},
+	.probe          = trace_noc_probe,
+	.remove		= trace_noc_remove,
+	.id_table	= trace_noc_ids,
+};
+
+module_amba_driver(trace_noc_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Trace NOC driver");
diff --git a/drivers/hwtracing/coresight/coresight-tnoc.h b/drivers/hwtracing/coresight/coresight-tnoc.h
new file mode 100644
index 0000000000000000000000000000000000000000..1291a153412c1c92be530cffe25fb56c2fca0395
--- /dev/null
+++ b/drivers/hwtracing/coresight/coresight-tnoc.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#define TRACE_NOC_CTRL      0x008
+#define TRACE_NOC_XLD       0x010
+#define TRACE_NOC_FREQVAL   0x018
+#define TRACE_NOC_SYNCR     0x020
+
+/* Enable generation of output ATB traffic.*/
+#define TRACE_NOC_CTRL_PORTEN   BIT(0)
+/* Sets the type of issued ATB FLAG packets.*/
+#define TRACE_NOC_CTRL_FLAGTYPE BIT(7)
+/* Sets the type of issued ATB FREQ packet*/
+#define TRACE_NOC_CTRL_FREQTYPE BIT(8)
+
+#define TRACE_NOC_SYN_VAL	0xFFFF
+
+/*
+ * struct trace_noc_drvdata - specifics associated to a trace noc component
+ * @base:	memory mapped base address for this component.
+ * @dev:	device node for trace_noc_drvdata.
+ * @csdev:	component vitals needed by the framework.
+ * @spinlock:	only one at a time pls.
+ * @atid:	id for the trace packet.
+ */
+struct trace_noc_drvdata {
+	void __iomem		*base;
+	struct device		*dev;
+	struct coresight_device	*csdev;
+	spinlock_t		spinlock; /* lock for the drvdata. */
+	u32			atid;
+};

-- 
2.34.1

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Suzuki K Poulose]

On 11/04/2025 09:57, Yuanfang Zhang wrote:
> Add a driver to support Coresight device Trace Network On Chip (TNOC),
> which is an integration hierarchy integrating functionalities of TPDA
> and funnels. It aggregates the trace and transports to coresight trace
> bus.
> 
> Compared to current configuration, it has the following advantages:
> 1. Reduce wires between subsystems.
> 2. Continue cleaning the infrastructure.
> 3. Reduce Data overhead by transporting raw data from source to target.
> 
>    +------------------------+                +-------------------------+
>    | Video Subsystem        |                |Video Subsystem          |
>    |       +-------------+  |                |       +------------+    |
>    |       | Video TPDM  |  |                |       | Video TPDM |    |
>    |       +-------------+  |                |       +------------+    |
>    |            |           |                |              |          |
>    |            v           |                |              v          |
>    |   +---------------+    |                |        +-----------+    |
>    |   | Video funnel  |    |                |        |Video TNOC |    |
>    |   +---------------+    |                |        +-----------+    |
>    +------------|-----------+                +------------|------------+
>                 |                                         |
>                 v-----+                                   |
> +--------------------|---------+                         |
> |  Multimedia        v         |                         |
> |  Subsystem   +--------+      |                         |
> |              |  TPDA  |      |                         v
> |              +----|---+      |              +---------------------+
> |                   |          |              |   Aggoregator TNOC  |
> |                   |          |              +----------|----------+
> |                   +--        |                         |
> |                     |        |                         |
> |                     |        |                         |
> |              +------v-----+  |                         |
> |              |  Funnel    |  |                         |
> |              +------------+  |                         |
> +----------------|-------------+                         |
>                   |                                       |
>                   v                                       v
>        +--------------------+                    +------------------+
>        |   Coresight Sink   |                    |  Coresight Sink  |
>        +--------------------+                    +------------------+

If each NOC has TraceID, how do you reliably decode the trace ?
Is there a single NOC/TPDA in the path from Source to sink ?

> 
>         Current Configuration                            TNOC
> 
> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
> ---
>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>   drivers/hwtracing/coresight/Makefile         |   1 +
>   drivers/hwtracing/coresight/coresight-tnoc.c | 186 +++++++++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-tnoc.h |  34 +++++
>   4 files changed, 234 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
> index ecd7086a5b83e86b6bc8ea039d6d26a628334ed3..f20600d58f38568f8178f69d3f678c2df2cbca7e 100644
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> @@ -259,4 +259,17 @@ config CORESIGHT_DUMMY
>   
>   	  To compile this driver as a module, choose M here: the module will be
>   	  called coresight-dummy.
> +
> +config CORESIGHT_TNOC
> +	tristate "Coresight Trace Network On Chip driver"
> +	help
> +	  This driver provides support for Trace Network On Chip (TNOC) component.
> +	  TNOC is an interconnect used to collect traces from various subsystems
> +	  and transport to a coresight trace sink. It sits in the different
> +	  tiles of SOC and aggregates the trace local to the tile and transports
> +	  it another tile or to coresight trace sink eventually.
> +
> +	  To compile this driver as a module, choose M here: the module will be
> +	  called coresight-tnoc.
> +
>   endif
> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
> index 8e62c3150aebd1e82b445fafc97a0a9b44397b0e..880e9ed6bfe9c711492c6a2cd972751f56dd8010 100644
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> @@ -34,6 +34,7 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o
>   obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o
>   obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>   					   coresight-replicator.o
> +obj-$(CONFIG_CORESIGHT_TNOC) += coresight-tnoc.o
>   obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
>   coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \
>   		     coresight-etm3x-sysfs.o
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..2ec4ead892f0166a3e84f777679c0f73f5da0e83
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
> @@ -0,0 +1,186 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> + #include <linux/amba/bus.h>
> + #include <linux/coresight.h>
> + #include <linux/device.h>
> + #include <linux/io.h>
> + #include <linux/kernel.h>
> + #include <linux/module.h>
> + #include <linux/of.h>
> + #include <linux/platform_device.h>
> +
> +#include "coresight-priv.h"
> +#include "coresight-tnoc.h"
> +#include "coresight-trace-id.h"
> +
> +DEFINE_CORESIGHT_DEVLIST(trace_noc_devs, "traceNoc");
> +
> +static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
> +{
> +	u32 val;
> +
> +	/* Set ATID */
> +	writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
> +
> +	/* Config sync CR */
> +	writel_relaxed(TRACE_NOC_SYN_VAL, drvdata->base + TRACE_NOC_SYNCR);

See my comment below about SYN_VAL. Please add a meaningful comment than
explaining what is obvious from the code.

> +
> +	/* Set Ctrl register */

Same here. Comment need not explain what is obvious from the code. But
a description of why we choose the values below is helpful.

> +	val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
> +
> +	val = val & ~TRACE_NOC_CTRL_FLAGTYPE;
> +	val = val | TRACE_NOC_CTRL_FREQTYPE;
> +	val = val | TRACE_NOC_CTRL_PORTEN;
> +
> +	writel(val, drvdata->base + TRACE_NOC_CTRL);
> +}
> +
> +static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
> +			    struct coresight_connection *outport)
> +{
> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	spin_lock(&drvdata->spinlock);
> +	if (csdev->refcnt == 0)
> +		trace_noc_enable_hw(drvdata);
> +
> +	csdev->refcnt++;
> +	spin_unlock(&drvdata->spinlock);
> +
> +	dev_dbg(drvdata->dev, "Trace NOC is enabled\n");
> +	return 0;
> +}
> +
> +static void trace_noc_disable_hw(struct trace_noc_drvdata *drvdata)
> +{
> +	writel(0x0, drvdata->base + TRACE_NOC_CTRL);
> +}
> +
> +static void trace_noc_disable(struct coresight_device *csdev, struct coresight_connection *inport,
> +			      struct coresight_connection *outport)
> +{
> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	spin_lock(&drvdata->spinlock);
> +	if (--csdev->refcnt == 0)
> +		trace_noc_disable_hw(drvdata);
> +
> +	spin_unlock(&drvdata->spinlock);
> +	dev_dbg(drvdata->dev, "Trace NOC is disabled\n");
> +}
> +
> +static int trace_noc_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode,
> +			__maybe_unused struct coresight_device *sink)
> +{
> +	struct trace_noc_drvdata *drvdata;
> +
> +	drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> +	return drvdata->atid;
> +}
> +
> +static const struct coresight_ops_link trace_noc_link_ops = {
> +	.enable		= trace_noc_enable,
> +	.disable	= trace_noc_disable,
> +};
> +
> +static const struct coresight_ops trace_noc_cs_ops = {
> +	.trace_id	= trace_noc_id,
> +	.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 int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
> +{
> +	struct device *dev = &adev->dev;
> +	struct coresight_platform_data *pdata;
> +	struct trace_noc_drvdata *drvdata;
> +	struct coresight_desc desc = { 0 };
> +	int ret;
> +
> +	desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
> +	if (!desc.name)
> +		return -ENOMEM;
> +	pdata = coresight_get_platform_data(dev);
> +	if (IS_ERR(pdata))
> +		return PTR_ERR(pdata);
> +	adev->dev.platform_data = pdata;
> +
> +	drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
> +	if (!drvdata)
> +		return -ENOMEM;
> +
> +	drvdata->dev = &adev->dev;
> +	dev_set_drvdata(dev, drvdata);
> +
> +	drvdata->base = devm_ioremap_resource(dev, &adev->res);
> +	if (!drvdata->base)
> +		return -ENOMEM;
> +
> +	spin_lock_init(&drvdata->spinlock);
> +
> +	ret = trace_noc_init_default_data(drvdata);
> +	if (ret)
> +		return ret;
> +
> +	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.access = CSDEV_ACCESS_IOMEM(drvdata->base);
> +	drvdata->csdev = coresight_register(&desc);
> +	if (IS_ERR(drvdata->csdev))
> +		return PTR_ERR(drvdata->csdev);
> +
> +	pm_runtime_put(&adev->dev);
> +
> +	return 0;
> +}
> +
> +static void trace_noc_remove(struct amba_device *adev)
> +{
> +	struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
> +
> +	coresight_trace_id_put_system_id(drvdata->atid);
> +	coresight_unregister(drvdata->csdev);
> +}
> +
> +static struct amba_id trace_noc_ids[] = {
> +	{
> +		.id     = 0x000f0c00,
> +		.mask   = 0x000fff00,

Is the mask sufficient ? fyi, the tpdm mask was fixed in
commit c8ea5f41b421.

Suzuk


> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(amba, trace_noc_ids);
> +
> +static struct amba_driver trace_noc_driver = {
> +	.drv = {
> +		.name   = "coresight-trace-noc",
> +		.suppress_bind_attrs = true,
> +	},
> +	.probe          = trace_noc_probe,
> +	.remove		= trace_noc_remove,
> +	.id_table	= trace_noc_ids,
> +};
> +
> +module_amba_driver(trace_noc_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("Trace NOC driver");
> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.h b/drivers/hwtracing/coresight/coresight-tnoc.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..1291a153412c1c92be530cffe25fb56c2fca0395
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#define TRACE_NOC_CTRL      0x008
> +#define TRACE_NOC_XLD       0x010
> +#define TRACE_NOC_FREQVAL   0x018
> +#define TRACE_NOC_SYNCR     0x020
> +
> +/* Enable generation of output ATB traffic.*/
> +#define TRACE_NOC_CTRL_PORTEN   BIT(0)
> +/* Sets the type of issued ATB FLAG packets.*/
> +#define TRACE_NOC_CTRL_FLAGTYPE BIT(7)
> +/* Sets the type of issued ATB FREQ packet*/
> +#define TRACE_NOC_CTRL_FREQTYPE BIT(8)
> +
> +#define TRACE_NOC_SYN_VAL	0xFFFF

TRACE_NOC_SYNCR_xx ?

What does VAL indicate ? It sounds too generic to indicate anything 
meaningful. Does it mean, allow all inputs ?

Suzuki

> +
> +/*
> + * struct trace_noc_drvdata - specifics associated to a trace noc component
> + * @base:	memory mapped base address for this component.
> + * @dev:	device node for trace_noc_drvdata.
> + * @csdev:	component vitals needed by the framework.
> + * @spinlock:	only one at a time pls.
> + * @atid:	id for the trace packet.
> + */
> +struct trace_noc_drvdata {
> +	void __iomem		*base;
> +	struct device		*dev;
> +	struct coresight_device	*csdev;
> +	spinlock_t		spinlock; /* lock for the drvdata. */
> +	u32			atid;
> +};
> 

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Yuanfang Zhang]

On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>> Add a driver to support Coresight device Trace Network On Chip (TNOC),
>> which is an integration hierarchy integrating functionalities of TPDA
>> and funnels. It aggregates the trace and transports to coresight trace
>> bus.
>>
>> Compared to current configuration, it has the following advantages:
>> 1. Reduce wires between subsystems.
>> 2. Continue cleaning the infrastructure.
>> 3. Reduce Data overhead by transporting raw data from source to target.
>>
>>    +------------------------+                +-------------------------+
>>    | Video Subsystem        |                |Video Subsystem          |
>>    |       +-------------+  |                |       +------------+    |
>>    |       | Video TPDM  |  |                |       | Video TPDM |    |
>>    |       +-------------+  |                |       +------------+    |
>>    |            |           |                |              |          |
>>    |            v           |                |              v          |
>>    |   +---------------+    |                |        +-----------+    |
>>    |   | Video funnel  |    |                |        |Video TNOC |    |
>>    |   +---------------+    |                |        +-----------+    |
>>    +------------|-----------+                +------------|------------+
>>                 |                                         |
>>                 v-----+                                   |
>> +--------------------|---------+                         |
>> |  Multimedia        v         |                         |
>> |  Subsystem   +--------+      |                         |
>> |              |  TPDA  |      |                         v
>> |              +----|---+      |              +---------------------+
>> |                   |          |              |    Aggregator TNOC  |
>> |                   |          |              +----------|----------+
>> |                   +--        |                         |
>> |                     |        |                         |
>> |                     |        |                         |
>> |              +------v-----+  |                         |
>> |              |  Funnel    |  |                         |
>> |              +------------+  |                         |
>> +----------------|-------------+                         |
>>                   |                                       |
>>                   v                                       v
>>        +--------------------+                    +------------------+
>>        |   Coresight Sink   |                    |  Coresight Sink  |
>>        +--------------------+                    +------------------+
> 
> If each NOC has TraceID, how do you reliably decode the trace ?
> Is there a single NOC/TPDA in the path from Source to sink ?

Not each TNOC has TraceID, there is only one TNOC has TraceID for one path
from Source to sink. In the example, only the aggregator TNOC has traceID. 
Decode trace relying on TraceID + Inport number.
It can has mutiple TNOC/TPDA in one path. 

> 
>>
>>         Current Configuration                            TNOC
>>
>> Signed-off-by: Yuanfang Zhang <quic_yuanfang@quicinc.com>
>> ---
>>   drivers/hwtracing/coresight/Kconfig          |  13 ++
>>   drivers/hwtracing/coresight/Makefile         |   1 +
>>   drivers/hwtracing/coresight/coresight-tnoc.c | 186 +++++++++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tnoc.h |  34 +++++
>>   4 files changed, 234 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index ecd7086a5b83e86b6bc8ea039d6d26a628334ed3..f20600d58f38568f8178f69d3f678c2df2cbca7e 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -259,4 +259,17 @@ config CORESIGHT_DUMMY
>>           To compile this driver as a module, choose M here: the module will be
>>         called coresight-dummy.
>> +
>> +config CORESIGHT_TNOC
>> +    tristate "Coresight Trace Network On Chip driver"
>> +    help
>> +      This driver provides support for Trace Network On Chip (TNOC) component.
>> +      TNOC is an interconnect used to collect traces from various subsystems
>> +      and transport to a coresight trace sink. It sits in the different
>> +      tiles of SOC and aggregates the trace local to the tile and transports
>> +      it another tile or to coresight trace sink eventually.
>> +
>> +      To compile this driver as a module, choose M here: the module will be
>> +      called coresight-tnoc.
>> +
>>   endif
>> diff --git a/drivers/hwtracing/coresight/Makefile b/drivers/hwtracing/coresight/Makefile
>> index 8e62c3150aebd1e82b445fafc97a0a9b44397b0e..880e9ed6bfe9c711492c6a2cd972751f56dd8010 100644
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>> @@ -34,6 +34,7 @@ obj-$(CONFIG_CORESIGHT_SINK_TPIU) += coresight-tpiu.o
>>   obj-$(CONFIG_CORESIGHT_SINK_ETBV10) += coresight-etb10.o
>>   obj-$(CONFIG_CORESIGHT_LINKS_AND_SINKS) += coresight-funnel.o \
>>                          coresight-replicator.o
>> +obj-$(CONFIG_CORESIGHT_TNOC) += coresight-tnoc.o
>>   obj-$(CONFIG_CORESIGHT_SOURCE_ETM3X) += coresight-etm3x.o
>>   coresight-etm3x-y := coresight-etm3x-core.o coresight-etm-cp14.o \
>>                coresight-etm3x-sysfs.o
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..2ec4ead892f0166a3e84f777679c0f73f5da0e83
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
>> @@ -0,0 +1,186 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> + #include <linux/amba/bus.h>
>> + #include <linux/coresight.h>
>> + #include <linux/device.h>
>> + #include <linux/io.h>
>> + #include <linux/kernel.h>
>> + #include <linux/module.h>
>> + #include <linux/of.h>
>> + #include <linux/platform_device.h>
>> +
>> +#include "coresight-priv.h"
>> +#include "coresight-tnoc.h"
>> +#include "coresight-trace-id.h"
>> +
>> +DEFINE_CORESIGHT_DEVLIST(trace_noc_devs, "traceNoc");
>> +
>> +static void trace_noc_enable_hw(struct trace_noc_drvdata *drvdata)
>> +{
>> +    u32 val;
>> +
>> +    /* Set ATID */
>> +    writel_relaxed(drvdata->atid, drvdata->base + TRACE_NOC_XLD);
>> +
>> +    /* Config sync CR */
>> +    writel_relaxed(TRACE_NOC_SYN_VAL, drvdata->base + TRACE_NOC_SYNCR);
> 
> See my comment below about SYN_VAL. Please add a meaningful comment than
> explaining what is obvious from the code.
sure, will update.
> 
>> +
>> +    /* Set Ctrl register */
> 
> Same here. Comment need not explain what is obvious from the code. But
> a description of why we choose the values below is helpful.
sure, will update.
> 
>> +    val = readl_relaxed(drvdata->base + TRACE_NOC_CTRL);
>> +
>> +    val = val & ~TRACE_NOC_CTRL_FLAGTYPE;
>> +    val = val | TRACE_NOC_CTRL_FREQTYPE;
>> +    val = val | TRACE_NOC_CTRL_PORTEN;
>> +
>> +    writel(val, drvdata->base + TRACE_NOC_CTRL);
>> +}
>> +
>> +static int trace_noc_enable(struct coresight_device *csdev, struct coresight_connection *inport,
>> +                struct coresight_connection *outport)
>> +{
>> +    struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    if (csdev->refcnt == 0)
>> +        trace_noc_enable_hw(drvdata);
>> +
>> +    csdev->refcnt++;
>> +    spin_unlock(&drvdata->spinlock);
>> +
>> +    dev_dbg(drvdata->dev, "Trace NOC is enabled\n");
>> +    return 0;
>> +}
>> +
>> +static void trace_noc_disable_hw(struct trace_noc_drvdata *drvdata)
>> +{
>> +    writel(0x0, drvdata->base + TRACE_NOC_CTRL);
>> +}
>> +
>> +static void trace_noc_disable(struct coresight_device *csdev, struct coresight_connection *inport,
>> +                  struct coresight_connection *outport)
>> +{
>> +    struct trace_noc_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    spin_lock(&drvdata->spinlock);
>> +    if (--csdev->refcnt == 0)
>> +        trace_noc_disable_hw(drvdata);
>> +
>> +    spin_unlock(&drvdata->spinlock);
>> +    dev_dbg(drvdata->dev, "Trace NOC is disabled\n");
>> +}
>> +
>> +static int trace_noc_id(struct coresight_device *csdev, __maybe_unused enum cs_mode mode,
>> +            __maybe_unused struct coresight_device *sink)
>> +{
>> +    struct trace_noc_drvdata *drvdata;
>> +
>> +    drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> +    return drvdata->atid;
>> +}
>> +
>> +static const struct coresight_ops_link trace_noc_link_ops = {
>> +    .enable        = trace_noc_enable,
>> +    .disable    = trace_noc_disable,
>> +};
>> +
>> +static const struct coresight_ops trace_noc_cs_ops = {
>> +    .trace_id    = trace_noc_id,
>> +    .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 int trace_noc_probe(struct amba_device *adev, const struct amba_id *id)
>> +{
>> +    struct device *dev = &adev->dev;
>> +    struct coresight_platform_data *pdata;
>> +    struct trace_noc_drvdata *drvdata;
>> +    struct coresight_desc desc = { 0 };
>> +    int ret;
>> +
>> +    desc.name = coresight_alloc_device_name(&trace_noc_devs, dev);
>> +    if (!desc.name)
>> +        return -ENOMEM;
>> +    pdata = coresight_get_platform_data(dev);
>> +    if (IS_ERR(pdata))
>> +        return PTR_ERR(pdata);
>> +    adev->dev.platform_data = pdata;
>> +
>> +    drvdata = devm_kzalloc(dev, sizeof(*drvdata), GFP_KERNEL);
>> +    if (!drvdata)
>> +        return -ENOMEM;
>> +
>> +    drvdata->dev = &adev->dev;
>> +    dev_set_drvdata(dev, drvdata);
>> +
>> +    drvdata->base = devm_ioremap_resource(dev, &adev->res);
>> +    if (!drvdata->base)
>> +        return -ENOMEM;
>> +
>> +    spin_lock_init(&drvdata->spinlock);
>> +
>> +    ret = trace_noc_init_default_data(drvdata);
>> +    if (ret)
>> +        return ret;
>> +
>> +    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.access = CSDEV_ACCESS_IOMEM(drvdata->base);
>> +    drvdata->csdev = coresight_register(&desc);
>> +    if (IS_ERR(drvdata->csdev))
>> +        return PTR_ERR(drvdata->csdev);
>> +
>> +    pm_runtime_put(&adev->dev);
>> +
>> +    return 0;
>> +}
>> +
>> +static void trace_noc_remove(struct amba_device *adev)
>> +{
>> +    struct trace_noc_drvdata *drvdata = dev_get_drvdata(&adev->dev);
>> +
>> +    coresight_trace_id_put_system_id(drvdata->atid);
>> +    coresight_unregister(drvdata->csdev);
>> +}
>> +
>> +static struct amba_id trace_noc_ids[] = {
>> +    {
>> +        .id     = 0x000f0c00,
>> +        .mask   = 0x000fff00,
> 
> Is the mask sufficient ? fyi, the tpdm mask was fixed in
> commit c8ea5f41b421.
will update mask to 0x00ffff00.
> 
> Suzuk
> 
> 
>> +    },
>> +    {},
>> +};
>> +MODULE_DEVICE_TABLE(amba, trace_noc_ids);
>> +
>> +static struct amba_driver trace_noc_driver = {
>> +    .drv = {
>> +        .name   = "coresight-trace-noc",
>> +        .suppress_bind_attrs = true,
>> +    },
>> +    .probe          = trace_noc_probe,
>> +    .remove        = trace_noc_remove,
>> +    .id_table    = trace_noc_ids,
>> +};
>> +
>> +module_amba_driver(trace_noc_driver);
>> +
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("Trace NOC driver");
>> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.h b/drivers/hwtracing/coresight/coresight-tnoc.h
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..1291a153412c1c92be530cffe25fb56c2fca0395
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-tnoc.h
>> @@ -0,0 +1,34 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
>> + */
>> +
>> +#define TRACE_NOC_CTRL      0x008
>> +#define TRACE_NOC_XLD       0x010
>> +#define TRACE_NOC_FREQVAL   0x018
>> +#define TRACE_NOC_SYNCR     0x020
>> +
>> +/* Enable generation of output ATB traffic.*/
>> +#define TRACE_NOC_CTRL_PORTEN   BIT(0)
>> +/* Sets the type of issued ATB FLAG packets.*/
>> +#define TRACE_NOC_CTRL_FLAGTYPE BIT(7)
>> +/* Sets the type of issued ATB FREQ packet*/
>> +#define TRACE_NOC_CTRL_FREQTYPE BIT(8)
>> +
>> +#define TRACE_NOC_SYN_VAL    0xFFFF
> 
> TRACE_NOC_SYNCR_xx ?
> 
> What does VAL indicate ? It sounds too generic to indicate anything meaningful. Does it mean, allow all inputs ?
> 
The value of TRACE_NOC_SYNCR register. it means the number of data words that are issued between two 'SYNC' packets.

> Suzuki
> 
>> +
>> +/*
>> + * struct trace_noc_drvdata - specifics associated to a trace noc component
>> + * @base:    memory mapped base address for this component.
>> + * @dev:    device node for trace_noc_drvdata.
>> + * @csdev:    component vitals needed by the framework.
>> + * @spinlock:    only one at a time pls.
>> + * @atid:    id for the trace packet.
>> + */
>> +struct trace_noc_drvdata {
>> +    void __iomem        *base;
>> +    struct device        *dev;
>> +    struct coresight_device    *csdev;
>> +    spinlock_t        spinlock; /* lock for the drvdata. */
>> +    u32            atid;
>> +};
>>
> 

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Suzuki K Poulose]

On 14/04/2025 10:16, Yuanfang Zhang wrote:
> 
> 
> On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
>> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>>> Add a driver to support Coresight device Trace Network On Chip (TNOC),
>>> which is an integration hierarchy integrating functionalities of TPDA
>>> and funnels. It aggregates the trace and transports to coresight trace
>>> bus.
>>>
>>> Compared to current configuration, it has the following advantages:
>>> 1. Reduce wires between subsystems.
>>> 2. Continue cleaning the infrastructure.
>>> 3. Reduce Data overhead by transporting raw data from source to target.
>>>
>>>     +------------------------+                +-------------------------+
>>>     | Video Subsystem        |                |Video Subsystem          |
>>>     |       +-------------+  |                |       +------------+    |
>>>     |       | Video TPDM  |  |                |       | Video TPDM |    |
>>>     |       +-------------+  |                |       +------------+    |
>>>     |            |           |                |              |          |
>>>     |            v           |                |              v          |
>>>     |   +---------------+    |                |        +-----------+    |
>>>     |   | Video funnel  |    |                |        |Video TNOC |    |
>>>     |   +---------------+    |                |        +-----------+    |
>>>     +------------|-----------+                +------------|------------+
>>>                  |                                         |
>>>                  v-----+                                   |
>>> +--------------------|---------+                         |
>>> |  Multimedia        v         |                         |
>>> |  Subsystem   +--------+      |                         |
>>> |              |  TPDA  |      |                         v
>>> |              +----|---+      |              +---------------------+
>>> |                   |          |              |    Aggregator TNOC  |
>>> |                   |          |              +----------|----------+
>>> |                   +--        |                         |
>>> |                     |        |                         |
>>> |                     |        |                         |
>>> |              +------v-----+  |                         |
>>> |              |  Funnel    |  |                         |
>>> |              +------------+  |                         |
>>> +----------------|-------------+                         |
>>>                    |                                       |
>>>                    v                                       v
>>>         +--------------------+                    +------------------+
>>>         |   Coresight Sink   |                    |  Coresight Sink  |
>>>         +--------------------+                    +------------------+
>>
>> If each NOC has TraceID, how do you reliably decode the trace ?
>> Is there a single NOC/TPDA in the path from Source to sink ?
> 
> Not each TNOC has TraceID, there is only one TNOC has TraceID for one path
> from Source to sink. In the example, only the aggregator TNOC has traceID.
> Decode trace relying on TraceID + Inport number.
> It can has mutiple TNOC/TPDA in one path.

So do we only describe the TNOCs that need traceId in the DT ? (e.g., 
Aggregator TNOC above ?) How about Video TNOC ? Don't we allocate a
trace id for it by default, when it is described ?

Suzuki

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Yuanfang Zhang]

On 5/6/2025 7:20 PM, Suzuki K Poulose wrote:
> On 14/04/2025 10:16, Yuanfang Zhang wrote:
>>
>>
>> On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
>>> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>>>> Add a driver to support Coresight device Trace Network On Chip (TNOC),
>>>> which is an integration hierarchy integrating functionalities of TPDA
>>>> and funnels. It aggregates the trace and transports to coresight trace
>>>> bus.
>>>>
>>>> Compared to current configuration, it has the following advantages:
>>>> 1. Reduce wires between subsystems.
>>>> 2. Continue cleaning the infrastructure.
>>>> 3. Reduce Data overhead by transporting raw data from source to target.
>>>>
>>>>     +------------------------+                +-------------------------+
>>>>     | Video Subsystem        |                |Video Subsystem          |
>>>>     |       +-------------+  |                |       +------------+    |
>>>>     |       | Video TPDM  |  |                |       | Video TPDM |    |
>>>>     |       +-------------+  |                |       +------------+    |
>>>>     |            |           |                |              |          |
>>>>     |            v           |                |              v          |
>>>>     |   +---------------+    |                |        +-----------+    |
>>>>     |   | Video funnel  |    |                |        |Video TNOC |    |
>>>>     |   +---------------+    |                |        +-----------+    |
>>>>     +------------|-----------+                +------------|------------+
>>>>                  |                                         |
>>>>                  v-----+                                   |
>>>> +--------------------|---------+                         |
>>>> |  Multimedia        v         |                         |
>>>> |  Subsystem   +--------+      |                         |
>>>> |              |  TPDA  |      |                         v
>>>> |              +----|---+      |              +---------------------+
>>>> |                   |          |              |    Aggregator TNOC  |
>>>> |                   |          |              +----------|----------+
>>>> |                   +--        |                         |
>>>> |                     |        |                         |
>>>> |                     |        |                         |
>>>> |              +------v-----+  |                         |
>>>> |              |  Funnel    |  |                         |
>>>> |              +------------+  |                         |
>>>> +----------------|-------------+                         |
>>>>                    |                                       |
>>>>                    v                                       v
>>>>         +--------------------+                    +------------------+
>>>>         |   Coresight Sink   |                    |  Coresight Sink  |
>>>>         +--------------------+                    +------------------+
>>>
>>> If each NOC has TraceID, how do you reliably decode the trace ?
>>> Is there a single NOC/TPDA in the path from Source to sink ?
>>
>> Not each TNOC has TraceID, there is only one TNOC has TraceID for one path
>> from Source to sink. In the example, only the aggregator TNOC has traceID.
>> Decode trace relying on TraceID + Inport number.
>> It can has mutiple TNOC/TPDA in one path.
> 
> So do we only describe the TNOCs that need traceId in the DT ? (e.g., Aggregator TNOC above ?) How about Video TNOC ? Don't we allocate a
> trace id for it by default, when it is described ?
> 
> Suzuki
> 
yes, now only describe the TNOCs which need traceID, Video TNOC is another type, it is interconnect TNOC which collects trace from subsystems
and transfers Aggr TNOC, it doesn't have ATID. Its driver is different from this patch, I want to describe it when upstream its driver.

Yuanfang

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Suzuki K Poulose]

On 07/05/2025 05:26, Yuanfang Zhang wrote:
> 
> 
> On 5/6/2025 7:20 PM, Suzuki K Poulose wrote:
>> On 14/04/2025 10:16, Yuanfang Zhang wrote:
>>>
>>>
>>> On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
>>>> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>>>>> Add a driver to support Coresight device Trace Network On Chip (TNOC),
>>>>> which is an integration hierarchy integrating functionalities of TPDA
>>>>> and funnels. It aggregates the trace and transports to coresight trace
>>>>> bus.
>>>>>
>>>>> Compared to current configuration, it has the following advantages:
>>>>> 1. Reduce wires between subsystems.
>>>>> 2. Continue cleaning the infrastructure.
>>>>> 3. Reduce Data overhead by transporting raw data from source to target.
>>>>>
>>>>>      +------------------------+                +-------------------------+
>>>>>      | Video Subsystem        |                |Video Subsystem          |
>>>>>      |       +-------------+  |                |       +------------+    |
>>>>>      |       | Video TPDM  |  |                |       | Video TPDM |    |
>>>>>      |       +-------------+  |                |       +------------+    |
>>>>>      |            |           |                |              |          |
>>>>>      |            v           |                |              v          |
>>>>>      |   +---------------+    |                |        +-----------+    |
>>>>>      |   | Video funnel  |    |                |        |Video TNOC |    |
>>>>>      |   +---------------+    |                |        +-----------+    |
>>>>>      +------------|-----------+                +------------|------------+
>>>>>                   |                                         |
>>>>>                   v-----+                                   |
>>>>> +--------------------|---------+                         |
>>>>> |  Multimedia        v         |                         |
>>>>> |  Subsystem   +--------+      |                         |
>>>>> |              |  TPDA  |      |                         v
>>>>> |              +----|---+      |              +---------------------+
>>>>> |                   |          |              |    Aggregator TNOC  |
>>>>> |                   |          |              +----------|----------+
>>>>> |                   +--        |                         |
>>>>> |                     |        |                         |
>>>>> |                     |        |                         |
>>>>> |              +------v-----+  |                         |
>>>>> |              |  Funnel    |  |                         |
>>>>> |              +------------+  |                         |
>>>>> +----------------|-------------+                         |
>>>>>                     |                                       |
>>>>>                     v                                       v
>>>>>          +--------------------+                    +------------------+
>>>>>          |   Coresight Sink   |                    |  Coresight Sink  |
>>>>>          +--------------------+                    +------------------+
>>>>
>>>> If each NOC has TraceID, how do you reliably decode the trace ?
>>>> Is there a single NOC/TPDA in the path from Source to sink ?
>>>
>>> Not each TNOC has TraceID, there is only one TNOC has TraceID for one path
>>> from Source to sink. In the example, only the aggregator TNOC has traceID.
>>> Decode trace relying on TraceID + Inport number.
>>> It can has mutiple TNOC/TPDA in one path.
>>
>> So do we only describe the TNOCs that need traceId in the DT ? (e.g., Aggregator TNOC above ?) How about Video TNOC ? Don't we allocate a
>> trace id for it by default, when it is described ?
>>
>> Suzuki
>>
> yes, now only describe the TNOCs which need traceID, Video TNOC is another type, it is interconnect TNOC which collects trace from subsystems
> and transfers Aggr TNOC, it doesn't have ATID. Its driver is different from this patch, I want to describe it when upstream its driver.

Thanks! Please could you make sure to describe all of this when sending
out a patch in the cover letter ?

Cheers
Suzuki


> 
> Yuanfang
> 
> 
> 
> 
> 
> 

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Suzuki K Poulose]

On 07/05/2025 09:52, Suzuki K Poulose wrote:
> On 07/05/2025 05:26, Yuanfang Zhang wrote:
>>
>>
>> On 5/6/2025 7:20 PM, Suzuki K Poulose wrote:
>>> On 14/04/2025 10:16, Yuanfang Zhang wrote:
>>>>
>>>>
>>>> On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
>>>>> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>>>>>> Add a driver to support Coresight device Trace Network On Chip 
>>>>>> (TNOC),
>>>>>> which is an integration hierarchy integrating functionalities of TPDA
>>>>>> and funnels. It aggregates the trace and transports to coresight 
>>>>>> trace
>>>>>> bus.
>>>>>>
>>>>>> Compared to current configuration, it has the following advantages:
>>>>>> 1. Reduce wires between subsystems.
>>>>>> 2. Continue cleaning the infrastructure.
>>>>>> 3. Reduce Data overhead by transporting raw data from source to 
>>>>>> target.
>>>>>>
>>>>>>      +------------------------+                
>>>>>> +-------------------------+
>>>>>>      | Video Subsystem        |                |Video 
>>>>>> Subsystem          |
>>>>>>      |       +-------------+  |                |       
>>>>>> +------------+    |
>>>>>>      |       | Video TPDM  |  |                |       | Video 
>>>>>> TPDM |    |
>>>>>>      |       +-------------+  |                |       
>>>>>> +------------+    |
>>>>>>      |            |           |                |              | 
>>>>>>           |
>>>>>>      |            v           |                |              
>>>>>> v          |
>>>>>>      |   +---------------+    |                |        
>>>>>> +-----------+    |
>>>>>>      |   | Video funnel  |    |                |        |Video 
>>>>>> TNOC |    |
>>>>>>      |   +---------------+    |                |        
>>>>>> +-----------+    |
>>>>>>      +------------|-----------+                
>>>>>> +------------|------------+
>>>>>>                   |                                         |
>>>>>>                   v-----+                                   |
>>>>>> +--------------------|---------+                         |
>>>>>> |  Multimedia        v         |                         |
>>>>>> |  Subsystem   +--------+      |                         |
>>>>>> |              |  TPDA  |      |                         v
>>>>>> |              +----|---+      |              +---------------------+
>>>>>> |                   |          |              |    Aggregator TNOC  |
>>>>>> |                   |          |              +----------|----------+
>>>>>> |                   +--        |                         |
>>>>>> |                     |        |                         |
>>>>>> |                     |        |                         |
>>>>>> |              +------v-----+  |                         |
>>>>>> |              |  Funnel    |  |                         |
>>>>>> |              +------------+  |                         |
>>>>>> +----------------|-------------+                         |
>>>>>>                     |                                       |
>>>>>>                     v                                       v
>>>>>>          +--------------------+                    
>>>>>> +------------------+
>>>>>>          |   Coresight Sink   |                    |  Coresight 
>>>>>> Sink  |
>>>>>>          +--------------------+                    
>>>>>> +------------------+
>>>>>
>>>>> If each NOC has TraceID, how do you reliably decode the trace ?
>>>>> Is there a single NOC/TPDA in the path from Source to sink ?
>>>>
>>>> Not each TNOC has TraceID, there is only one TNOC has TraceID for 
>>>> one path
>>>> from Source to sink. In the example, only the aggregator TNOC has 
>>>> traceID.
>>>> Decode trace relying on TraceID + Inport number.
>>>> It can has mutiple TNOC/TPDA in one path.
>>>
>>> So do we only describe the TNOCs that need traceId in the DT ? (e.g., 
>>> Aggregator TNOC above ?) How about Video TNOC ? Don't we allocate a
>>> trace id for it by default, when it is described ?
>>>
>>> Suzuki
>>>
>> yes, now only describe the TNOCs which need traceID, Video TNOC is 
>> another type, it is interconnect TNOC which collects trace from 
>> subsystems
>> and transfers Aggr TNOC, it doesn't have ATID. Its driver is different 
>> from this patch, I want to describe it when upstream its driver.

So, if both are TNOC and there different types of them, how do you plan
to identify, which is what ?

And we also have a dt-bindings which simply says "coresight-tnoc". Isn't 
too generic if it is meant to be "aggregator" ?

Suzuki



> 
> Thanks! Please could you make sure to describe all of this when sending
> out a patch in the cover letter ?
> 
> Cheers
> Suzuki
> 
> 
>>
>> Yuanfang
>>
>>
>>
>>
>>
>>
> 

Re: [PATCH v3 2/2] coresight: add coresight Trace Network On Chip driver

[Yuanfang Zhang]

On 5/7/2025 7:59 PM, Suzuki K Poulose wrote:
> On 07/05/2025 09:52, Suzuki K Poulose wrote:
>> On 07/05/2025 05:26, Yuanfang Zhang wrote:
>>>
>>>
>>> On 5/6/2025 7:20 PM, Suzuki K Poulose wrote:
>>>> On 14/04/2025 10:16, Yuanfang Zhang wrote:
>>>>>
>>>>>
>>>>> On 4/11/2025 5:59 PM, Suzuki K Poulose wrote:
>>>>>> On 11/04/2025 09:57, Yuanfang Zhang wrote:
>>>>>>> Add a driver to support Coresight device Trace Network On Chip (TNOC),
>>>>>>> which is an integration hierarchy integrating functionalities of TPDA
>>>>>>> and funnels. It aggregates the trace and transports to coresight trace
>>>>>>> bus.
>>>>>>>
>>>>>>> Compared to current configuration, it has the following advantages:
>>>>>>> 1. Reduce wires between subsystems.
>>>>>>> 2. Continue cleaning the infrastructure.
>>>>>>> 3. Reduce Data overhead by transporting raw data from source to target.
>>>>>>>
>>>>>>>      +------------------------+                +-------------------------+
>>>>>>>      | Video Subsystem        |                |Video Subsystem          |
>>>>>>>      |       +-------------+  |                |       +------------+    |
>>>>>>>      |       | Video TPDM  |  |                |       | Video TPDM |    |
>>>>>>>      |       +-------------+  |                |       +------------+    |
>>>>>>>      |            |           |                |              |           |
>>>>>>>      |            v           |                |              v          |
>>>>>>>      |   +---------------+    |                |        +-----------+    |
>>>>>>>      |   | Video funnel  |    |                |        |Video TNOC |    |
>>>>>>>      |   +---------------+    |                |        +-----------+    |
>>>>>>>      +------------|-----------+                +------------|------------+
>>>>>>>                   |                                         |
>>>>>>>                   v-----+                                   |
>>>>>>> +--------------------|---------+                         |
>>>>>>> |  Multimedia        v         |                         |
>>>>>>> |  Subsystem   +--------+      |                         |
>>>>>>> |              |  TPDA  |      |                         v
>>>>>>> |              +----|---+      |              +---------------------+
>>>>>>> |                   |          |              |    Aggregator TNOC  |
>>>>>>> |                   |          |              +----------|----------+
>>>>>>> |                   +--        |                         |
>>>>>>> |                     |        |                         |
>>>>>>> |                     |        |                         |
>>>>>>> |              +------v-----+  |                         |
>>>>>>> |              |  Funnel    |  |                         |
>>>>>>> |              +------------+  |                         |
>>>>>>> +----------------|-------------+                         |
>>>>>>>                     |                                       |
>>>>>>>                     v                                       v
>>>>>>>          +--------------------+                    +------------------+
>>>>>>>          |   Coresight Sink   |                    |  Coresight Sink  |
>>>>>>>          +--------------------+                    +------------------+
>>>>>>
>>>>>> If each NOC has TraceID, how do you reliably decode the trace ?
>>>>>> Is there a single NOC/TPDA in the path from Source to sink ?
>>>>>
>>>>> Not each TNOC has TraceID, there is only one TNOC has TraceID for one path
>>>>> from Source to sink. In the example, only the aggregator TNOC has traceID.
>>>>> Decode trace relying on TraceID + Inport number.
>>>>> It can has mutiple TNOC/TPDA in one path.
>>>>
>>>> So do we only describe the TNOCs that need traceId in the DT ? (e.g., Aggregator TNOC above ?) How about Video TNOC ? Don't we allocate a
>>>> trace id for it by default, when it is described ?
>>>>
>>>> Suzuki
>>>>
>>> yes, now only describe the TNOCs which need traceID, Video TNOC is another type, it is interconnect TNOC which collects trace from subsystems
>>> and transfers Aggr TNOC, it doesn't have ATID. Its driver is different from this patch, I want to describe it when upstream its driver.
> 
> So, if both are TNOC and there different types of them, how do you plan
> to identify, which is what ?
> 
I will describe interconnect TNOC as a platform device because it doesn't have PID register.
It will have a compatible "qcom,coresight-interconnect-tnoc".
 
> And we also have a dt-bindings which simply says "coresight-tnoc". Isn't too generic if it is meant to be "aggregator" ?
> 
aggregator TNOC is generic TNOC, the dt-bindings is for this generic TNOC. interconnect TNOC is special TNOC, There will be a separate dt-bindings to describe it.
> Suzuki
> 
> 
> 
>>
>> Thanks! Please could you make sure to describe all of this when sending
>> out a patch in the cover letter ?
>>
sure, will update the cover letter.
>> Cheers
>> Suzuki
>>
>>
>>>
>>> Yuanfang
>>>
>>>
>>>
>>>
>>>
>>>
>>
>