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

[sashiko-bot@kernel.org]

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

New issues:
- [High] Trace ID (ATID) leak on driver unbind when using the `qcom,coresight-agtnoc` compatible with the platform driver.

Pre-existing issues:
- [High] Switching the device to the platform driver exposes an unbalanced clock disable during probe error paths and driver unbind due to a conflict between devm_clk_get_optional_enabled() and manual PM runtime clock management.
--

commit 6dd81dc5c2c5997c8cc48dc9206b866b99451df0
Author: Jie Gan <jie.gan@oss.qualcomm.com>

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

The traceNoC node used the "qcom,coresight-tnoc", "arm,primecell" compatible,
which failed probing on the AMBA bus. This patch switches the node to the
"qcom,coresight-agtnoc" compatible so it probes through the platform driver.

> diff --git a/arch/arm64/boot/dts/qcom/kaanapali.dtsi b/arch/arm64/boot/dts/qcom/kaanapali.dtsi
> index 7aa9653bd456e..e98f4aa4b141a 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-agtnoc";

[Severity: High]
This is a pre-existing issue, but switching to the platform driver exposes
an unbalanced clock disable during probe error paths and driver unbind.

When probe uses devm_clk_get_optional_enabled(), it queues a devres cleanup
action to call clk_disable_unprepare(). However, the driver also manually
disables the clock during runtime suspend:

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

When unbinding the platform driver, tnoc_platform_remove() doesn't wake the
device up before disabling PM runtime:

drivers/hwtracing/coresight/coresight-tnoc.c:tnoc_platform_remove() {
    ...
	coresight_unregister(drvdata->csdev);
	pm_runtime_disable(&pdev->dev);
}

If the device is runtime suspended during unbind, the clock is already
manually disabled. The devres cleanup will then run and double-disable the
clock. Does tnoc_platform_remove() need a pm_runtime_get_sync() call to
ensure the clock is enabled before devres takes over?


[Severity: High]
Does switching to the platform driver expose a trace ID (ATID) leak on
driver unbind?

The platform driver allocates a system trace ID for agtnoc devices during
probe:

drivers/hwtracing/coresight/coresight-tnoc.c:trace_noc_init_default_data() {
    ...
	atid = coresight_trace_id_get_system_id();
	if (atid < 0)
		return atid;

	drvdata->atid = atid;
}

But the platform remove callback doesn't seem to free it, unlike the AMBA
driver's remove function:

drivers/hwtracing/coresight/coresight-tnoc.c:tnoc_platform_remove() {
    ...
	coresight_unregister(drvdata->csdev);
	pm_runtime_disable(&pdev->dev);
}

Could this exhaust the system trace ID pool if the device is repeatedly
unbound and bound?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630-fix-tracenoc-probe-issue-v3-0-7201e1841e94@oss.qualcomm.com?part=3