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

[sashiko-bot@kernel.org]

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