linux-kernel - Re: [PATCH 01/12] dt-bindings: arm: coresight: Add cpu cluster tmc/funnel/replicator support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ9a7Vg1jR=UBWwQanvyW_-ucXQ1HWQha47oJCUL6vkfQwgpKQ@mail.gmail.com>
Date: Wed, 29 Oct 2025 09:39:39 +0000
From: Mike Leach <mike.leach@...aro.org>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: Yuanfang Zhang <yuanfang.zhang@....qualcomm.com>, 
	Suzuki K Poulose <suzuki.poulose@....com>, James Clark <james.clark@...aro.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Mathieu Poirier <mathieu.poirier@...aro.org>, Leo Yan <leo.yan@...ux.dev>, 
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konradybcio@...nel.org>, kernel@....qualcomm.com, 
	coresight@...ts.linaro.org, linux-arm-kernel@...ts.infradead.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH 01/12] dt-bindings: arm: coresight: Add cpu cluster
 tmc/funnel/replicator support

Hi,

On Tue, 28 Oct 2025 at 09:09, Krzysztof Kozlowski <krzk@...nel.org> wrote:
>
> On Mon, Oct 27, 2025 at 11:28:03PM -0700, Yuanfang Zhang wrote:
> > Add the following compatible strings to the bindings:
> > - arm,coresight-cpu-funnel
> > - arm,coresight-cpu-replicator
> > - arm,coresight-cpu-tmc
>

These are redundant - the actual hardware has not changed - what has
is how the device is powered up / down on the system

> We see that from the diff. Explain here the hardware instead.
>
> >
> > Each requires 'power-domains' when used.

So why is this not used to adjust the power handling in the driver? Or
another attribute.
Look at the CTI bindings - these can be associated with a CPU or be a
system CTI - we look at the cpu attribute to differentiate, not have
two separate compatibles.

Regards

Mike


> >
> > Signed-off-by: Yuanfang Zhang <yuanfang.zhang@....qualcomm.com>
> > ---
> >  .../bindings/arm/arm,coresight-dynamic-funnel.yaml | 23 +++++++++++++++++-----
> >  .../arm/arm,coresight-dynamic-replicator.yaml      | 22 +++++++++++++++++----
> >  .../devicetree/bindings/arm/arm,coresight-tmc.yaml | 22 +++++++++++++++++----
> >  3 files changed, 54 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> > index b74db15e5f8af2226b817f6af5f533b1bfc74736..8f32d4e3bbb750f5a6262db0032318875739cf81 100644
> > --- a/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> > +++ b/Documentation/devicetree/bindings/arm/arm,coresight-dynamic-funnel.yaml
> > @@ -28,19 +28,32 @@ select:
> >    properties:
> >      compatible:
> >        contains:
> > -        const: arm,coresight-dynamic-funnel
> > +        enum:
> > +          - arm,coresight-dynamic-funnel
> > +          - arm,coresight-cpu-funnel
>
> Keep alphabetical sorting. We asked this multiple times already.
>
> >    required:
> >      - compatible
> >
> >  allOf:
> >    - $ref: /schemas/arm/primecell.yaml#
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: arm,coresight-cpu-funnel
> > +    then:
> > +      required:
> > +        - power-domains
>
> Just move the allOf to the bottom like in example-schema.
>
> > +
> >  properties:
> >    compatible:
> > -    items:
> > -      - const: arm,coresight-dynamic-funnel
> > -      - const: arm,primecell
> > -
>
> Why do you remove this?
>
> > +    oneOf:
> > +      - items:
> > +          - const: arm,coresight-dynamic-funnel
> > +          - const: arm,primecell
> > +      - items:
> > +          - const: arm,coresight-cpu-funnel
>
> Hm? Why do you need custom select if this is not primecell? And nothing
> in commit msg explains why this is not primecell anymore.
>
> You have entire commit msg to say something useful, WHY you are doing
> this, WHY you are doing it DIFFERENTLY. Don't say what you did - that's
> obvious, we are capable of reading diffs.
>
> Best regards,
> Krzysztof
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK