diff mbox series

dt-bindings: thermal: k3-j72xx: Update bindings for J721S2 SoCs

Message ID 20231228-b4-upstream-j721s2-vtm-dt-binding-v1-1-e866277f9c64@ti.com
State New
Headers show
Series dt-bindings: thermal: k3-j72xx: Update bindings for J721S2 SoCs | expand

Commit Message

Manorit Chawdhry Dec. 28, 2023, 6:39 a.m. UTC
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.

Signed-off-by: Manorit Chawdhry <m-chawdhry@ti.com>
---
 .../devicetree/bindings/thermal/ti,j72xx-thermal.yaml        | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)


---
base-commit: 39676dfe52331dba909c617f213fdb21015c8d10
change-id: 20231228-b4-upstream-j721s2-vtm-dt-binding-95e61c942c34

Best regards,

Comments

Krzysztof Kozlowski Jan. 4, 2024, 9:33 a.m. UTC | #1
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.

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@ti.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@42040000 {
> -        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
Nishanth Menon Jan. 4, 2024, 3:20 p.m. UTC | #2
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@ti.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@42040000 {
> > -        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
>
diff mbox series

Patch

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:
+      - items:
+          - enum:
+              - ti,j721e-vtm
+              - ti,j7200-vtm
+              - ti,j721s2-vtm
+      - maxItems: 2
 
   reg:
     items:
@@ -72,7 +76,7 @@  examples:
   - |
     #include <dt-bindings/soc/ti,sci_pm_domain.h>
     wkup_vtm0: thermal-sensor@42040000 {
-        compatible = "ti,j721e-vtm";
+        compatible = "ti,j721e-vtm", "ti,j7200-vtm";
         reg = <0x42040000 0x350>,
               <0x42050000 0x350>,
               <0x43000300 0x10>;