[PATCH v5 0/5] Fix traceNoC probe issue on multiple QCOM platforms

[PATCH v5 0/5] Fix traceNoC probe issue on multiple QCOM platforms

[Jie Gan]

The CoreSight TNOC (Trace Network-On-Chip) binding so far only allowed the
two-string AMBA form "qcom,coresight-tnoc", "arm,primecell". That form
forces the device onto the AMBA bus, where the driver must read the
peripheral ID from the device registers during probe. On several QCOM
platforms this AMBA peripheral-ID probing fails, so the traceNoC device
never probes and its trace path is unavailable.

This series drops the "arm,primecell" entry and turns "qcom,coresight-tnoc"
into a standalone platform compatible. Without "arm,primecell" the device
is created on the platform bus and probes through the platform driver by
compatible string, bypassing the AMBA peripheral-ID read while remaining a
fully functional Aggregator TNOC that allocates a system trace ID (ATID).

The series is organized as: binding first, then the driver support for the
platform-bus compatible, followed by the per-platform DT fixes that switch
the affected nodes over to it.

- Patch 1 (dt-bindings) drops "arm,primecell" and makes
"qcom,coresight-tnoc" a single standalone compatible, and updates the
example node accordingly.
- Patch 2 (driver) adds "qcom,coresight-tnoc" to the platform driver's
match table and renames the itnoc-specific names to generic tnoc names,
since the platform driver now serves both the Interconnect and Aggregator
TNOC. It also restricts the ATID-unsupported handling to
"qcom,coresight-itnoc" only, so the Aggregator TNOC on the platform bus is
no longer wrongly covered and correctly allocates a trace ID.
- Patches 3-4 (kaanapali, sm8750) drop the "arm,primecell" entry on the
traceNoC nodes and use the standalone "qcom,coresight-tnoc" compatible,
fixing the probe failure on those platforms.
- Patch 5 (glymur) switches the node from "qcom,coresight-itnoc" to
"qcom,coresight-tnoc". This node is actually an Aggregator TNOC (its
tn_ag_* endpoints show aggregation), so it should expose a system trace
ID rather than being treated as an Interconnect TNOC.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
Changes in v5:
- Drop the separate "qcom,coresight-agtnoc" compatible. Instead drop the
  "arm,primecell" entry and reuse the existing "qcom,coresight-tnoc" as a
  standalone platform compatible.
- Update the binding, driver match table, and per-platform DT nodes to the
  standalone "qcom,coresight-tnoc" compatible accordingly.
- Rewrite the commit messages to describe dropping "arm,primecell" and
  binding on the platform bus.
- Link to v4: https://lore.kernel.org/r/20260701-fix-tracenoc-probe-issue-v4-0-aefab449a470@oss.qualcomm.com

Changes in v4:
- rewrite the commit message for the patch 3 - patch 5.
- free the allocated ATID in remove path.
- Link to v3: https://lore.kernel.org/r/20260630-fix-tracenoc-probe-issue-v3-0-7201e1841e94@oss.qualcomm.com

Changes in v3:
- add standalone compatible for AG traceNoC device, allow it to be
  probed with platform driver.
- add fix patches for sm8750 and Glymur platforms
- Link to v2: https://lore.kernel.org/r/20260624-fix-tracenoc-probe-issue-v2-0-786520f62f21@oss.qualcomm.com

Changes in v2:
- address the ATID issue reported by Sashiko.
- update binding to accept arm,primecell-periphid property.
- Link to v1: https://lore.kernel.org/r/20260624-fix-tracenoc-probe-issue-v1-1-bcc785198fc5@oss.qualcomm.com

---
Jie Gan (5):
      dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus
      coresight: tnoc: Bind Aggregator TNOC on the platform bus
      arm64: dts: qcom: kaanapali: fix traceNoC probe issue
      arm64: dts: qcom: sm8750: fix traceNoC probe issue
      arm64: dts: qcom: glymur: use Aggregator TNOC compatible

 .../bindings/arm/qcom,coresight-tnoc.yaml          | 16 ++--------
 arch/arm64/boot/dts/qcom/glymur.dtsi               |  6 ++--
 arch/arm64/boot/dts/qcom/kaanapali.dtsi            |  2 +-
 arch/arm64/boot/dts/qcom/sm8750.dtsi               |  2 +-
 drivers/hwtracing/coresight/coresight-tnoc.c       | 37 ++++++++++++----------
 5 files changed, 27 insertions(+), 36 deletions(-)
---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260624-fix-tracenoc-probe-issue-c6429da28df4

Best regards,
-- 
Jie Gan <jie.gan@oss.qualcomm.com>

[PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Jie Gan]

The TNOC compatible previously required the two-string AMBA form
"qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
AMBA bus.

Change the compatible to a single "qcom,coresight-tnoc" string with no
"arm,primecell" entry, so the device is created on the platform bus and
bound by the platform driver through its compatible string.

Drop the custom select block as well. It was only needed to avoid
matching the "arm,primecell" string on unrelated nodes; with the single
"qcom,coresight-tnoc" compatible the schema's auto-generated select
already matches the right nodes.

Update the example node to use the standalone compatible form.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 .../devicetree/bindings/arm/qcom,coresight-tnoc.yaml     | 16 ++--------------
 1 file changed, 2 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
index ef648a15b806..72fb1210e22e 100644
--- a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
+++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
@@ -22,24 +22,12 @@ description: >
 
   Note this binding is specifically intended for Aggregator TNOC instances.
 
-# 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
+    const: qcom,coresight-tnoc
 
   reg:
     maxItems: 1
@@ -83,7 +71,7 @@ additionalProperties: false
 examples:
   - |
     tn@109ab000  {
-      compatible = "qcom,coresight-tnoc", "arm,primecell";
+      compatible = "qcom,coresight-tnoc";
       reg = <0x109ab000 0x4200>;
 
       clocks = <&aoss_qmp>;

-- 
2.34.1

[PATCH v5 2/5] coresight: tnoc: Bind Aggregator TNOC on the platform bus

[Jie Gan]

The Aggregator TNOC is bound as an AMBA device through the
"qcom,coresight-tnoc", "arm,primecell" compatible. Binding on the AMBA
bus requires the peripheral to expose valid CoreSight peripheral and
component ID registers, which the AMBA bus reads to match and probe the
device. The Aggregator TNOC does not implement these ID registers, so
AMBA probing fails and the device never comes up.

Bind the Aggregator TNOC on the platform bus instead, where the device
is matched by its compatible string and no peripheral-ID probing is
performed. Add "qcom,coresight-tnoc" to the platform driver's match
table, and rename the platform driver and its callbacks from the
"itnoc"-specific names to generic "tnoc" names, since the driver now
serves both the Interconnect and Aggregator TNOC. Update the platform
driver name to "coresight-tnoc" accordingly.

The ATID-unsupported handling keyed off dev_is_amba(), which disabled
ATID allocation for every platform-bus device. With the Aggregator TNOC
now on the platform bus, that check would wrongly disable its ATID, even
though the Aggregator TNOC owns the ATID that tags the whole aggregation
path. The Interconnect TNOC aggregates trace within its subsystem but
carries no ATID of its own, because the downstream Aggregator TNOC
already owns the ATID for the path. So base the check on the
"qcom,coresight-itnoc" compatible and let every other form allocate a
trace ID.

Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 drivers/hwtracing/coresight/coresight-tnoc.c | 37 +++++++++++++++-------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
index 9e8de4323d28..737cc802aefe 100644
--- a/drivers/hwtracing/coresight/coresight-tnoc.c
+++ b/drivers/hwtracing/coresight/coresight-tnoc.c
@@ -130,7 +130,7 @@ static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
 {
 	int atid;
 
-	if (!dev_is_amba(drvdata->dev)) {
+	if (of_device_is_compatible(drvdata->dev->of_node, "qcom,coresight-itnoc")) {
 		drvdata->atid = -EOPNOTSUPP;
 		return 0;
 	}
@@ -278,7 +278,7 @@ static struct amba_driver trace_noc_driver = {
 	.id_table	= trace_noc_ids,
 };
 
-static int itnoc_probe(struct platform_device *pdev)
+static int tnoc_platform_probe(struct platform_device *pdev)
 {
 	struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int ret;
@@ -295,16 +295,18 @@ static int itnoc_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static void itnoc_remove(struct platform_device *pdev)
+static void tnoc_platform_remove(struct platform_device *pdev)
 {
 	struct trace_noc_drvdata *drvdata = platform_get_drvdata(pdev);
 
 	coresight_unregister(drvdata->csdev);
 	pm_runtime_disable(&pdev->dev);
+	if (drvdata->atid > 0)
+		coresight_trace_id_put_system_id(drvdata->atid);
 }
 
 #ifdef CONFIG_PM
-static int itnoc_runtime_suspend(struct device *dev)
+static int tnoc_runtime_suspend(struct device *dev)
 {
 	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
 
@@ -313,7 +315,7 @@ static int itnoc_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int itnoc_runtime_resume(struct device *dev)
+static int tnoc_runtime_resume(struct device *dev)
 {
 	struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);
 
@@ -321,35 +323,36 @@ static int itnoc_runtime_resume(struct device *dev)
 }
 #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 dev_pm_ops tnoc_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(tnoc_runtime_suspend, tnoc_runtime_resume, NULL)
 };
 
-static const struct of_device_id itnoc_of_match[] = {
+static const struct of_device_id tnoc_of_match[] = {
 	{ .compatible = "qcom,coresight-itnoc" },
+	{ .compatible = "qcom,coresight-tnoc" },
 	{}
 };
-MODULE_DEVICE_TABLE(of, itnoc_of_match);
+MODULE_DEVICE_TABLE(of, tnoc_of_match);
 
-static struct platform_driver itnoc_driver = {
-	.probe = itnoc_probe,
-	.remove = itnoc_remove,
+static struct platform_driver tnoc_platform_driver = {
+	.probe = tnoc_platform_probe,
+	.remove = tnoc_platform_remove,
 	.driver = {
-		.name = "coresight-itnoc",
-		.of_match_table = itnoc_of_match,
+		.name = "coresight-tnoc",
+		.of_match_table = tnoc_of_match,
 		.suppress_bind_attrs = true,
-		.pm = &itnoc_dev_pm_ops,
+		.pm = &tnoc_dev_pm_ops,
 	},
 };
 
 static int __init tnoc_init(void)
 {
-	return coresight_init_driver("tnoc", &trace_noc_driver, &itnoc_driver);
+	return coresight_init_driver("tnoc", &trace_noc_driver, &tnoc_platform_driver);
 }
 
 static void __exit tnoc_exit(void)
 {
-	coresight_remove_driver(&trace_noc_driver, &itnoc_driver);
+	coresight_remove_driver(&trace_noc_driver, &tnoc_platform_driver);
 }
 module_init(tnoc_init);
 module_exit(tnoc_exit);

-- 
2.34.1

[PATCH v5 3/5] arm64: dts: qcom: kaanapali: fix traceNoC probe issue

[Jie Gan]

The traceNoC node used the "qcom,coresight-tnoc", "arm,primecell"
compatible, which places the device on the AMBA bus. To bind an AMBA
device, the bus first reads the peripheral and component ID registers
(PID/CID) at the top of the device's register block and matches them
against the primecell ID. On this platform the traceNoC does not expose
a valid CID in that register, so the read never yields the expected
primecell value, the AMBA match fails, and the device is left stuck in
deferred probe indefinitely.

Drop the "arm,primecell" entry and use the standalone
"qcom,coresight-tnoc" compatible, which binds via the platform driver by
compatible string and does not rely on reading the primecell ID register
at all. This lets the device probe on hardware that does not expose a
readable CID, while remaining an Aggregator TNOC that retains ATID
functionality.

Fixes: f73959d86c15 ("arm64: dts: qcom: kaanapali: add coresight nodes")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/kaanapali.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index 7aa9653bd456..5d0e7e8c2e58 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -5004,7 +5004,7 @@ tpdm_pcie_rscc_out: endpoint {
 		};
 
 		tn@111b8000 {
-			compatible = "qcom,coresight-tnoc", "arm,primecell";
+			compatible = "qcom,coresight-tnoc";
 			reg = <0x0 0x111b8000 0x0 0x4200>;
 
 			clocks = <&aoss_qmp>;

-- 
2.34.1

[PATCH v5 4/5] arm64: dts: qcom: sm8750: fix traceNoC probe issue

[Jie Gan]

The traceNoC node used the "qcom,coresight-tnoc", "arm,primecell"
compatible, which places the device on the AMBA bus. To bind an AMBA
device, the bus first reads the peripheral and component ID registers
(PID/CID) at the top of the device's register block and matches them
against the primecell ID. On this platform the traceNoC does not expose
a valid CID in that register, so the read never yields the expected
primecell value, the AMBA match fails, and the device is left stuck in
deferred probe indefinitely.

Drop the "arm,primecell" entry and use the standalone
"qcom,coresight-tnoc" compatible, which binds via the platform driver by
compatible string and does not rely on reading the primecell ID register
at all. This lets the device probe on hardware that does not expose a
readable CID, while remaining an Aggregator TNOC that retains ATID
functionality.

Fixes: ebd1eb365cae ("arm64: qcom: dts: sm8750: add coresight nodes")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/sm8750.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
index fafed417c66f..1e25be41aef5 100644
--- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
@@ -4687,7 +4687,7 @@ tpdm_rdpm_cmb2_out: endpoint {
 		};
 
 		tn@109ab000 {
-			compatible = "qcom,coresight-tnoc", "arm,primecell";
+			compatible = "qcom,coresight-tnoc";
 			reg = <0x0 0x109ab000 0x0 0x4200>;
 
 			clocks = <&aoss_qmp>;

-- 
2.34.1

[PATCH v5 5/5] arm64: dts: qcom: glymur: use Aggregator TNOC compatible

[Jie Gan]

The traceNoC node is the system-level Aggregator TNOC, so it must own a
valid ATID that tags the whole aggregation path. It is marked compatible
with "qcom,coresight-itnoc", an Interconnect TNOC, which is never
assigned an ATID. As a result the aggregator has no trace ID and cannot
tag the merged trace.

An Interconnect TNOC is a subsystem-level aggregator: it merges trace
from the ATB sources within its subsystem (TPDMs and other ATB masters)
and forwards the combined stream to the system-level Aggregator TNOC. It
carries no ATID of its own, because the Aggregator TNOC downstream in the
path already owns the ATID.

Switch the node to "qcom,coresight-tnoc" so it is described as the
Aggregator TNOC it is and is assigned a system trace ID. Rename the node
to "tn" and use the "apb_pclk" clock name as required by the Aggregator
TNOC binding.

Fixes: 1f7d0c42a08d ("arm64: dts: qcom: glymur: add coresight nodes")
Signed-off-by: Jie Gan <jie.gan@oss.qualcomm.com>
---
 arch/arm64/boot/dts/qcom/glymur.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/glymur.dtsi b/arch/arm64/boot/dts/qcom/glymur.dtsi
index 20b49af7298e..0c04dca5e557 100644
--- a/arch/arm64/boot/dts/qcom/glymur.dtsi
+++ b/arch/arm64/boot/dts/qcom/glymur.dtsi
@@ -6038,12 +6038,12 @@ qm_tpdm_out: endpoint {
 			};
 		};
 
-		itnoc@11200000  {
-			compatible = "qcom,coresight-itnoc";
+		tn@11200000 {
+			compatible = "qcom,coresight-tnoc";
 			reg = <0x0 0x11200000 0x0 0x3c00>;
 
 			clocks = <&aoss_qmp>;
-			clock-names = "apb";
+			clock-names = "apb_pclk";
 
 			in-ports {
 				#address-cells = <1>;

-- 
2.34.1

Re: [PATCH v5 3/5] arm64: dts: qcom: kaanapali: fix traceNoC probe issue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete migration of the traceNoC binding leaves the sm8750 device tree violating the updated schema.
--

--- Patch [3]: [PATCH v5 3/5] arm64: dts: qcom: kaanapali: fix traceNoC probe issue ---
commit 9b027ffc45673a64efa19cd6c9d0acb4efe7595e
Author: Jie Gan <jie.gan@oss.qualcomm.com>

arm64: dts: qcom: kaanapali: fix traceNoC probe issue

This commit drops the arm,primecell compatible string from the traceNoC
node in the kaanapali device tree. By doing so, the device binds via the
platform driver rather than the AMBA bus, bypassing the missing primecell CID
and fixing an indefinite deferred probe issue.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index 7aa9653bd456e..5d0e7e8c2e58c 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
[ ... ]
> @@ -5004,7 +5004,7 @@ tpdm_pcie_rscc_out: endpoint {
>  		};
>  
>  		tn@111b8000 {
> -			compatible = "qcom,coresight-tnoc", "arm,primecell";
> +			compatible = "qcom,coresight-tnoc";

[Severity: Medium]
Does this incomplete migration leave other device trees violating the updated
binding schema?

If the qcom,coresight-tnoc.yaml binding was modified to strictly accept only
the single qcom,coresight-tnoc string by explicitly dropping arm,primecell,
then arch/arm64/boot/dts/qcom/sm8750.dtsi appears to still retain the old
compatible string:

arch/arm64/boot/dts/qcom/sm8750.dtsi:
			compatible = "qcom,coresight-tnoc", "arm,primecell";

Will this cause a make dtbs_check validation failure for sm8750?

>  			reg = <0x0 0x111b8000 0x0 0x4200>;
>  
>  			clocks = <&aoss_qmp>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-fix-tracenoc-probe-issue-v5-0-bf733ed9ebd0@oss.qualcomm.com?part=3

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Changing the compatible schema to strictly require only 'qcom,coresight-tnoc' breaks validation for existing device trees, and removing the custom select block prevents safe restoration of backward compatibility.
- [High] The platform driver is not configured to match the 'qcom,coresight-tnoc' compatible string, causing devices to fail probing if they conform to the new binding.
--

--- Patch [1]: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus ---
commit 04928b8d12a8ec737b7aeaf2dab2136838ce0ddd
Author: Jie Gan <jie.gan@oss.qualcomm.com>

dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

The TNOC compatible previously required the two-string AMBA form
"qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
AMBA bus. Change the compatible to a single "qcom,coresight-tnoc" string
with no "arm,primecell" entry.

> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> index ef648a15b8065..72fb1210e22e0 100644
> --- a/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml
> @@ -22,24 +22,12 @@ description: >
>  
>    Note this binding is specifically intended for Aggregator TNOC instances.
>  
> -# 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
> +    const: qcom,coresight-tnoc

[Severity: High]
Does this change break schema validation for existing device trees?

There are existing in-tree nodes, such as in 
arch/arm64/boot/dts/qcom/sm8750.dtsi, that still use the older fallback:

	compatible = "qcom,coresight-tnoc", "arm,primecell";

If this schema is updated to strictly require only qcom,coresight-tnoc,
validation for those existing device trees will fail.

Additionally, by removing the custom select block, if the arm,primecell
fallback is restored to maintain backward compatibility, won't dtschema
auto-generate a select that matches any node with arm,primecell and
erroneously apply this schema to unrelated AMBA devices across the system?

[Severity: High]
Will devices conforming to this new binding fail to probe?

Looking at the platform driver in 
drivers/hwtracing/coresight/coresight-tnoc.c, the OF match table lacks the 
required string:

static const struct of_device_id itnoc_of_match[] = {
	{ .compatible = "qcom,coresight-itnoc" },
	{}
};

Since the schema drops arm,primecell, the device will be instantiated
on the platform bus. Without qcom,coresight-tnoc in the platform driver's
match table, won't this result in a complete probe failure?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-fix-tracenoc-probe-issue-v5-0-bf733ed9ebd0@oss.qualcomm.com?part=1

Re: [PATCH v5 2/5] coresight: tnoc: Bind Aggregator TNOC on the platform bus

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unconditional trace ID put in AMBA remove path causes WARN splat if an AMBA device has the ITNOC compatible string.
--

--- Patch [2]: [PATCH v5 2/5] coresight: tnoc: Bind Aggregator TNOC on the platform bus ---
commit f735d2886baa617f8b66d797a2762302ad70464e
Author: Jie Gan <jie.gan@oss.qualcomm.com>

coresight: tnoc: Bind Aggregator TNOC on the platform bus

The Aggregator TNOC was originally bound as an AMBA device but lacks the
required CoreSight component ID registers, leading to probe failure. This
commit binds it to the platform bus where it matches by compatible string
instead. It also updates driver names and ID allocations so the ATID
unsupported check uses the qcom,coresight-itnoc compatible instead of the
bus type.

> diff --git a/drivers/hwtracing/coresight/coresight-tnoc.c b/drivers/hwtracing/coresight/coresight-tnoc.c
> index 9e8de4323d285..737cc802aefe6 100644
> --- a/drivers/hwtracing/coresight/coresight-tnoc.c
> +++ b/drivers/hwtracing/coresight/coresight-tnoc.c
> @@ -130,7 +130,7 @@ static int trace_noc_init_default_data(struct trace_noc_drvdata *drvdata)
>  {
>  	int atid;
>  
> -	if (!dev_is_amba(drvdata->dev)) {
> +	if (of_device_is_compatible(drvdata->dev->of_node, "qcom,coresight-itnoc")) {
>  		drvdata->atid = -EOPNOTSUPP;
>  		return 0;
>  	}

[Severity: Medium]
Will this cause a WARN splat in trace_noc_remove() if an AMBA device is
probed with the qcom,coresight-itnoc compatible string?

Previously, AMBA devices were guaranteed to have a positive trace ID because
of the !dev_is_amba() check. With this change, an AMBA device can be assigned
-EOPNOTSUPP.

During driver unbind, trace_noc_remove() unconditionally releases the ID:

drivers/hwtracing/coresight/coresight-tnoc.c:trace_noc_remove() {
	...
	coresight_unregister(drvdata->csdev);
	coresight_trace_id_put_system_id(drvdata->atid);
}

Should trace_noc_remove() include the same atid > 0 check that was
added below to tnoc_platform_remove()?

[ ... ]

> @@ -295,16 +295,18 @@ static int itnoc_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> -static void itnoc_remove(struct platform_device *pdev)
> +static void tnoc_platform_remove(struct platform_device *pdev)
>  {
>  	struct trace_noc_drvdata *drvdata = platform_get_drvdata(pdev);
>  
>  	coresight_unregister(drvdata->csdev);
>  	pm_runtime_disable(&pdev->dev);
> +	if (drvdata->atid > 0)
> +		coresight_trace_id_put_system_id(drvdata->atid);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-fix-tracenoc-probe-issue-v5-0-bf733ed9ebd0@oss.qualcomm.com?part=2

Re: [PATCH v5 4/5] arm64: dts: qcom: sm8750: fix traceNoC probe issue

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Switching the traceNoC device from the AMBA bus to the platform bus actively exposes a PM runtime / devres clock unbalance bug that triggers kernel WARNINGs on device unbind.
--

--- Patch [4]: [PATCH v5 4/5] arm64: dts: qcom: sm8750: fix traceNoC probe issue ---
commit 614093568676102a83b2efb2ba08c1718071ad73
Author: Jie Gan <jie.gan@oss.qualcomm.com>

arm64: dts: qcom: sm8750: fix traceNoC probe issue

This patch drops the "arm,primecell" compatible string from the traceNoC
node to prevent indefinite probe deferral. By using only the standalone
"qcom,coresight-tnoc" compatible, the device binds via the platform
bus instead of the AMBA bus, bypassing the primecell ID check.

> diff --git a/arch/arm64/boot/dts/qcom/sm8750.dtsi b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> index fafed417c66fc..1e25be41aef5c 100644
> --- a/arch/arm64/boot/dts/qcom/sm8750.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8750.dtsi
> @@ -4687,7 +4687,7 @@ tpdm_rdpm_cmb2_out: endpoint {
>  		};
>  
>  		tn@109ab000 {
> -			compatible = "qcom,coresight-tnoc", "arm,primecell";
> +			compatible = "qcom,coresight-tnoc";

[Severity: High]
This isn't a bug introduced by this patch, but does changing this to a
platform device expose a clock management unbalance during driver unbind?

When bound as a platform device, tnoc_platform_probe() acquires the clock
via devm_clk_get_optional_enabled() in coresight_get_enable_clocks(). This
queues a devres action to automatically disable the clock upon unbind.

If the device enters runtime suspend, tnoc_runtime_suspend() manually
disables this same clock:

drivers/hwtracing/coresight/coresight-tnoc.c:tnoc_runtime_suspend() {
    ...
    clk_disable_unprepare(drvdata->pclk);
    ...
}

Unlike the AMBA bus framework which automatically wakes devices during
removal, tnoc_platform_remove() does not call pm_runtime_get_sync() before
disabling runtime PM.

If the driver is unbound while suspended, the device stays suspended and the
clock remains disabled. Once tnoc_platform_remove() returns, the devres
unwind process executes the cleanup action, calling clk_disable_unprepare()
a second time.

Will this sequence trigger kernel warnings in clk_core_disable() and
clk_core_unprepare() due to the counts already being zero?

>  			reg = <0x0 0x109ab000 0x0 0x4200>;
>  
>  			clocks = <&aoss_qmp>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707-fix-tracenoc-probe-issue-v5-0-bf733ed9ebd0@oss.qualcomm.com?part=4

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Krzysztof Kozlowski]

On Tue, Jul 07, 2026 at 05:08:49PM +0800, Jie Gan wrote:
> The TNOC compatible previously required the two-string AMBA form
> "qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
> AMBA bus.
> 
> Change the compatible to a single "qcom,coresight-tnoc" string with no
> "arm,primecell" entry, so the device is created on the platform bus and
> bound by the platform driver through its compatible string.

I asked already: this is not a valid reason. References to Linux
structures are not correct here. If this is your reason, then answer is:
fix drivers, by renaming or doing whatever is necessary to platform bus
to behave like amba bus.
> 
> Drop the custom select block as well. It was only needed to avoid
> matching the "arm,primecell" string on unrelated nodes; with the single
> "qcom,coresight-tnoc" compatible the schema's auto-generated select
> already matches the right nodes.

Why are you describing the diff? It's obvious.


> 
> Update the example node to use the standalone compatible form.

And here.

You got comments at v4, nothing improveed. Your commit msg tells useless
things like what you did or you want different Linux behavior.

Best regards,
Krzysztof

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Jie Gan]

On 7/9/2026 3:59 PM, Krzysztof Kozlowski wrote:
> On Tue, Jul 07, 2026 at 05:08:49PM +0800, Jie Gan wrote:
>> The TNOC compatible previously required the two-string AMBA form
>> "qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
>> AMBA bus.
>>
>> Change the compatible to a single "qcom,coresight-tnoc" string with no
>> "arm,primecell" entry, so the device is created on the platform bus and
>> bound by the platform driver through its compatible string.
> 
> I asked already: this is not a valid reason. References to Linux
> structures are not correct here. If this is your reason, then answer is:
> fix drivers, by renaming or doing whatever is necessary to platform bus
> to behave like amba bus.

The issue is that we are not allowed to introduce a DT property to 
bypass the AMBA bus validation, which reads the Component ID (CID) 
registers. Besides, I cannot fix this in the TraceNoC AMBA driver 
because the driver is only invoked after amba_match() succeeds. Since 
the device fails the AMBA match, the TraceNoC AMBA driver is never reached.

The solution proposed by the CoreSight maintainer, Suzuki, is to define 
a dedicated compatible string for the Aggregator TraceNoC device and 
have it probed by the existing platform driver. If the AMBA bus cannot 
identify the device as a valid AMBA device, we should not fake the 
hardware description to make it appear as one.

Thanks,
Jie

>>
>> Drop the custom select block as well. It was only needed to avoid
>> matching the "arm,primecell" string on unrelated nodes; with the single
>> "qcom,coresight-tnoc" compatible the schema's auto-generated select
>> already matches the right nodes.
> 
> Why are you describing the diff? It's obvious.
> 
> 
>>
>> Update the example node to use the standalone compatible form.
> 
> And here.
> 
> You got comments at v4, nothing improveed. Your commit msg tells useless
> things like what you did or you want different Linux behavior.
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Krzysztof Kozlowski]

On 09/07/2026 10:34, Jie Gan wrote:
> 
> 
> On 7/9/2026 3:59 PM, Krzysztof Kozlowski wrote:
>> On Tue, Jul 07, 2026 at 05:08:49PM +0800, Jie Gan wrote:
>>> The TNOC compatible previously required the two-string AMBA form
>>> "qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
>>> AMBA bus.
>>>
>>> Change the compatible to a single "qcom,coresight-tnoc" string with no
>>> "arm,primecell" entry, so the device is created on the platform bus and
>>> bound by the platform driver through its compatible string.
>>
>> I asked already: this is not a valid reason. References to Linux
>> structures are not correct here. If this is your reason, then answer is:
>> fix drivers, by renaming or doing whatever is necessary to platform bus
>> to behave like amba bus.
> 
> The issue is that we are not allowed to introduce a DT property to 
> bypass the AMBA bus validation, which reads the Component ID (CID) 

It is the first time "bypass the AMBU bus validation" appears in this
patch. You do not see that as a problem?

Again, third time and the last: describe the actual hardware and the
actual problem coming from the hardware.

Best regards,
Krzysztof

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Jie Gan]

On 7/9/2026 4:46 PM, Krzysztof Kozlowski wrote:
> On 09/07/2026 10:34, Jie Gan wrote:
>>
>>
>> On 7/9/2026 3:59 PM, Krzysztof Kozlowski wrote:
>>> On Tue, Jul 07, 2026 at 05:08:49PM +0800, Jie Gan wrote:
>>>> The TNOC compatible previously required the two-string AMBA form
>>>> "qcom,coresight-tnoc", "arm,primecell", which forces the device onto the
>>>> AMBA bus.
>>>>
>>>> Change the compatible to a single "qcom,coresight-tnoc" string with no
>>>> "arm,primecell" entry, so the device is created on the platform bus and
>>>> bound by the platform driver through its compatible string.
>>>
>>> I asked already: this is not a valid reason. References to Linux
>>> structures are not correct here. If this is your reason, then answer is:
>>> fix drivers, by renaming or doing whatever is necessary to platform bus
>>> to behave like amba bus.
>>
>> The issue is that we are not allowed to introduce a DT property to
>> bypass the AMBA bus validation, which reads the Component ID (CID)
> 
> It is the first time "bypass the AMBU bus validation" appears in this
> patch. You do not see that as a problem?
> 
> Again, third time and the last: describe the actual hardware and the
> actual problem coming from the hardware.

Understood, will focus on describe the hardware and the hardware issue.

Commit message description updated like below:

The Aggregator TraceNoC hardware exposes CID registers, but the 
Component ID value read from those registers is 0 instead of a valid 
AMBA Component ID. As a result, the device cannot be identified by the 
AMBA bus and is not discovered as an AMBA device.

Since the hardware does not provide a valid AMBA identification, 
describe the Aggregator TraceNoC using a dedicated compatible string and 
bind it to the existing platform driver.

Thanks,
Jie

> 
> Best regards,
> Krzysztof

Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus

[Krzysztof Kozlowski]

On 09/07/2026 11:02, Jie Gan wrote:
>>> The issue is that we are not allowed to introduce a DT property to
>>> bypass the AMBA bus validation, which reads the Component ID (CID)
>>
>> It is the first time "bypass the AMBU bus validation" appears in this
>> patch. You do not see that as a problem?
>>
>> Again, third time and the last: describe the actual hardware and the
>> actual problem coming from the hardware.
> 
> Understood, will focus on describe the hardware and the hardware issue.
> 
> Commit message description updated like below:
> 
> The Aggregator TraceNoC hardware exposes CID registers, but the 
> Component ID value read from those registers is 0 instead of a valid 
> AMBA Component ID. As a result, the device cannot be identified by the 
> AMBA bus and is not discovered as an AMBA device.
> 
> Since the hardware does not provide a valid AMBA identification, 
> describe the Aggregator TraceNoC using a dedicated compatible string and 
> bind it to the existing platform driver.
> 

yes, that's good.

Best regards,
Krzysztof