All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Jie Gan" <jie.gan@oss.qualcomm.com>
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
Subject: Re: [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus
Date: Tue, 07 Jul 2026 09:22:13 +0000	[thread overview]
Message-ID: <20260707092214.476641F00ACF@smtp.kernel.org> (raw)
In-Reply-To: <20260707-fix-tracenoc-probe-issue-v5-1-bf733ed9ebd0@oss.qualcomm.com>

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

  reply	other threads:[~2026-07-07  9:22 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  9:08 [PATCH v5 0/5] Fix traceNoC probe issue on multiple QCOM platforms Jie Gan
2026-07-07  9:08 ` [PATCH v5 1/5] dt-bindings: arm: coresight-tnoc: Drop arm,primecell to bind on platform bus Jie Gan
2026-07-07  9:22   ` sashiko-bot [this message]
2026-07-09  7:59   ` Krzysztof Kozlowski
2026-07-09  8:34     ` Jie Gan
2026-07-09  8:46       ` Krzysztof Kozlowski
2026-07-09  9:02         ` Jie Gan
2026-07-09  9:17           ` Krzysztof Kozlowski
2026-07-07  9:08 ` [PATCH v5 2/5] coresight: tnoc: Bind Aggregator TNOC on the " Jie Gan
2026-07-07  9:22   ` sashiko-bot
2026-07-07  9:08 ` [PATCH v5 3/5] arm64: dts: qcom: kaanapali: fix traceNoC probe issue Jie Gan
2026-07-07  9:21   ` sashiko-bot
2026-07-07  9:08 ` [PATCH v5 4/5] arm64: dts: qcom: sm8750: " Jie Gan
2026-07-07  9:30   ` sashiko-bot
2026-07-07  9:08 ` [PATCH v5 5/5] arm64: dts: qcom: glymur: use Aggregator TNOC compatible Jie Gan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260707092214.476641F00ACF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=jie.gan@oss.qualcomm.com \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.