linux-kernel - Re: [PATCH] dt-bindings: thermal: k3-j72xx: Update bindings for J721S2 SoCs

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] [day] [month] [year] [list]

Message-ID: <20240104152034.gijjaeehlcylorws@tranquil>
Date: Thu, 4 Jan 2024 09:20:34 -0600
From: Nishanth Menon <nm@...com>
To: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
CC: Manorit Chawdhry <m-chawdhry@...com>,
        "Rafael J. Wysocki"
	<rafael@...nel.org>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Zhang Rui
	<rui.zhang@...el.com>, Lukasz Luba <lukasz.luba@....com>,
        Rob Herring
	<robh+dt@...nel.org>,
        Krzysztof Kozlowski
	<krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, J
 Keerthy <j-keerthy@...com>,
        <linux-pm@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Udit Kumar
	<u-kumar1@...com>,
        Vignesh Raghavendra <vigneshr@...com>
Subject: Re: [PATCH] dt-bindings: thermal: k3-j72xx: Update bindings for
 J721S2 SoCs

On 10:33-20240104, Krzysztof Kozlowski wrote:
> On 28/12/2023 07:39, Manorit Chawdhry wrote:
> > The clock and processor ID for J721S2 differs from the existing
> > compatibles, add a new compatible to represent this change for adding
> > support for Adaptive voltage scaling.

This makes no sense to begin with. You do not need a new compatible just
for clock ID change (processor ID has nothing to do with vtm node).

This approach is just plain wrong. AVS support has been done in the past
(class 3,2,1.5 and 0) and bindings have been mature for more that a
decade for the same.

So NAK for this patch

> 
> Subject: everything is "update". Write proper subjects.
> 
> A nit, subject: drop second/last, redundant "bindings for". The
> "dt-bindings" prefix is already stating that these are bindings.
> 
> 
> 
> > 
> > Signed-off-by: Manorit Chawdhry <m-chawdhry@...com>
> > ---
> >  .../devicetree/bindings/thermal/ti,j72xx-thermal.yaml        | 12 ++++++++----
> >  1 file changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/thermal/ti,j72xx-thermal.yaml b/Documentation/devicetree/bindings/thermal/ti,j72xx-thermal.yaml
> > index 171b3622ed84..5792ccc058aa 100644
> > --- a/Documentation/devicetree/bindings/thermal/ti,j72xx-thermal.yaml
> > +++ b/Documentation/devicetree/bindings/thermal/ti,j72xx-thermal.yaml
> > @@ -24,9 +24,13 @@ description: |
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - ti,j721e-vtm
> > -      - ti,j7200-vtm
> > +    anyOf:
> 
> ? Eh, what?
> 
> > +      - items:
> > +          - enum:
> > +              - ti,j721e-vtm
> > +              - ti,j7200-vtm
> > +              - ti,j721s2-vtm
> > +      - maxItems: 2
> 
> What? I really do not understand what are you doing here.
> 
> 
> >  
> >    reg:
> >      items:
> > @@ -72,7 +76,7 @@ examples:
> >    - |
> >      #include <dt-bindings/soc/ti,sci_pm_domain.h>
> >      wkup_vtm0: thermal-sensor@...40000 {
> > -        compatible = "ti,j721e-vtm";
> > +        compatible = "ti,j721e-vtm", "ti,j7200-vtm";
> 
> It's an enum, not a list.
> 
> NAK, please read example-schema and other bindings. Then get review from
> TI folks before posting new versions.
> 
> Best regards,
> Krzysztof
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D