Devicetree
 help / color / mirror / Atom feed
* [PATCH] arm64: dts: qcom: kaanapali: fix traceNoC probe issue
@ 2026-06-24  6:33 Jie Gan
  2026-06-24  6:49 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Jie Gan @ 2026-06-24  6:33 UTC (permalink / raw)
  To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Tingwei Zhang, Jingyi Wang, Jie Gan, Abel Vesa
  Cc: Konrad Dybcio, linux-arm-msm, devicetree, linux-kernel

Fix probing of the traceNoC device by switching from the AMBA bus to
the platform itnoc driver.

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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
index 7aa9653bd456..84561b5faf81 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
@@ -5003,12 +5003,12 @@ tpdm_pcie_rscc_out: endpoint {
 			};
 		};
 
-		tn@111b8000 {
-			compatible = "qcom,coresight-tnoc", "arm,primecell";
+		itnoc@111b8000 {
+			compatible = "qcom,coresight-itnoc";
 			reg = <0x0 0x111b8000 0x0 0x4200>;
 
 			clocks = <&aoss_qmp>;
-			clock-names = "apb_pclk";
+			clock-names = "apb";
 
 			in-ports {
 				#address-cells = <1>;

---
base-commit: 4e5dfb7c84012007c3c7061126491bbc92d71bf1
change-id: 20260624-fix-tracenoc-probe-issue-c6429da28df4

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


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] arm64: dts: qcom: kaanapali: fix traceNoC probe issue
  2026-06-24  6:33 [PATCH] arm64: dts: qcom: kaanapali: fix traceNoC probe issue Jie Gan
@ 2026-06-24  6:49 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-06-24  6:49 UTC (permalink / raw)
  To: Jie Gan; +Cc: devicetree, robh, conor+dt

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

New issues:
- [High] Switching traceNoC to the platform itnoc driver breaks trace enablement for connected sources.

Pre-existing issues:
- [High] Probe error path unconditionally invokes PM runtime suspend, causing a NULL pointer dereference.
- [Medium] Driver removal and unbind paths cause a clock enable count underflow warning.
--

commit 152b245041f20740bb96bedaa8fe0632753867d1
Author: Jie Gan <jie.gan@oss.qualcomm.com>

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

This commit fixes probing of the traceNoC device by switching from the AMBA
bus to the platform itnoc driver.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index 7aa9653bd456e..84561b5faf811 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> +++ b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> @@ -5003,12 +5003,12 @@ tpdm_pcie_rscc_out: endpoint {
>  			};
>  		};
>  
> -		tn@111b8000 {
> -			compatible = "qcom,coresight-tnoc", "arm,primecell";
> +		itnoc@111b8000 {
> +			compatible = "qcom,coresight-itnoc";

[Severity: High]
This is a pre-existing issue, but does switching to the platform driver expose
a NULL pointer dereference if probing fails?

If _tnoc_probe() fails in itnoc_probe(), the code unconditionally calls
pm_runtime_put() before drvdata is fully initialized. This triggers:

drivers/hwtracing/coresight/coresight-tnoc.c:itnoc_runtime_suspend() {
    struct trace_noc_drvdata *drvdata = dev_get_drvdata(dev);

    clk_disable_unprepare(drvdata->pclk);
}

Could this dereference a NULL drvdata during the error path?

[Severity: Medium]
This isn't a bug introduced by this patch, but does using the itnoc driver
lead to a clock enable count underflow on unbind?

The clock is fetched with managed enablement in _tnoc_probe():

    ret = coresight_get_enable_clocks(dev, &drvdata->pclk, NULL);

This registers a devres cleanup action. However, the driver also manages the
clock manually:

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

If the driver is unbound while in a runtime-suspended state, will the devres
framework attempt to disable the already-disabled clock?

[Severity: High]
Will changing the compatible string without "arm,primecell" break trace
enablement for connected sources like TPDM?

When probed as a platform device, the driver initializes atid to unsupported:

drivers/hwtracing/coresight/coresight-tnoc.c:trace_noc_init_default_data() {
    if (!dev_is_amba(drvdata->dev)) {
        drvdata->atid = -EOPNOTSUPP;
        return 0;
    }
}

During trace enablement, coresight_path_assign_trace_id() could fail because
-EOPNOTSUPP is evaluated as an invalid trace ID:

drivers/hwtracing/coresight/coresight-core.c:coresight_path_assign_trace_id() {
    if (!IS_VALID_CS_TRACE_ID(trace_id))
        return -EINVAL;
}

Does this immediately abort trace enablement for any source relying on this
traceNoC?

>  			reg = <0x0 0x111b8000 0x0 0x4200>;
>  
>  			clocks = <&aoss_qmp>;
> -			clock-names = "apb_pclk";
> +			clock-names = "apb";

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-fix-tracenoc-probe-issue-v1-1-bcc785198fc5@oss.qualcomm.com?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-06-24  6:49 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-24  6:33 [PATCH] arm64: dts: qcom: kaanapali: fix traceNoC probe issue Jie Gan
2026-06-24  6:49 ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox