[PATCH 0/3] ti: davinci, keystone: txt to yaml

[PATCH 0/3] ti: davinci, keystone: txt to yaml

[Kousik Sanagavarapu]

Convert txt bindings of DaVinci Timer and DaVinci/Keystone WDT
Controller to dtschema.

Some comments in-patch.

Kousik Sanagavarapu (3):
  dt-bindings: timer: ti,davinci-timer: convert to dtschema
  dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema
  ARM: dts: davinci, keystone: correct watchdog nodenames

 .../bindings/timer/ti,davinci-timer.txt       | 37 ----------
 .../bindings/timer/ti,davinci-timer.yaml      | 68 +++++++++++++++++++
 .../bindings/watchdog/davinci-wdt.txt         | 24 -------
 .../bindings/watchdog/ti,davinci-wdt.yaml     | 52 ++++++++++++++
 arch/arm/boot/dts/ti/davinci/da850.dtsi       |  2 +-
 .../boot/dts/ti/keystone/keystone-k2g.dtsi    |  4 +-
 arch/arm/boot/dts/ti/keystone/keystone.dtsi   |  4 +-
 7 files changed, 125 insertions(+), 66 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
 create mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml
 delete mode 100644 Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml

-- 
2.45.2.827.g557ae147e6.dirty

[PATCH 1/3] dt-bindings: timer: ti,davinci-timer: convert to dtschema

[Kousik Sanagavarapu]

Convert txt binding of TI's DaVinci timer to dtschema to allow for
validation.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
 .../bindings/timer/ti,davinci-timer.txt       | 37 ----------
 .../bindings/timer/ti,davinci-timer.yaml      | 68 +++++++++++++++++++
 2 files changed, 68 insertions(+), 37 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
 create mode 100644 Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/ti,davinci-timer.txt b/Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
deleted file mode 100644
index 29bf91ccf5b7..000000000000
--- a/Documentation/devicetree/bindings/timer/ti,davinci-timer.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-* Device tree bindings for Texas Instruments DaVinci timer
-
-This document provides bindings for the 64-bit timer in the DaVinci
-architecture devices. The timer can be configured as a general-purpose 64-bit
-timer, dual general-purpose 32-bit timers. When configured as dual 32-bit
-timers, each half can operate in conjunction (chain mode) or independently
-(unchained mode) of each other.
-
-The timer is a free running up-counter and can generate interrupts when the
-counter reaches preset counter values.
-
-Also see ../watchdog/davinci-wdt.txt for timers that are configurable as
-watchdog timers.
-
-Required properties:
-
-- compatible : should be "ti,da830-timer".
-- reg : specifies base physical address and count of the registers.
-- interrupts : interrupts generated by the timer.
-- interrupt-names: should be "tint12", "tint34", "cmpint0", "cmpint1",
-		   "cmpint2", "cmpint3", "cmpint4", "cmpint5", "cmpint6",
-		   "cmpint7" ("cmpintX" may be omitted if not present in the
-		   hardware).
-- clocks : the clock feeding the timer clock.
-
-Example:
-
-	clocksource: timer@20000 {
-		compatible = "ti,da830-timer";
-		reg = <0x20000 0x1000>;
-		interrupts = <21>, <22>, <74>, <75>, <76>, <77>, <78>, <79>,
-			     <80>, <81>;
-		interrupt-names = "tint12", "tint34", "cmpint0", "cmpint1",
-				  "cmpint2", "cmpint3", "cmpint4", "cmpint5",
-				  "cmpint6", "cmpint7";
-		clocks = <&pll0_auxclk>;
-	};
diff --git a/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml b/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml
new file mode 100644
index 000000000000..615ceb8f30af
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/ti,davinci-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DaVinci Timer
+
+maintainers:
+  - Kousik Sanagavarapu <five231003@gmail.com>
+
+description: |
+
+  This is a 64-bit timer found on TI's DaVinci architecture devices. The timer
+  can be configured as a general-purpose 64-bit timer, dual general-purpose
+  32-bit timers. When configured as dual 32-bit timers, each half can operate
+  in conjunction (chain mode) or independently (unchained mode) of each other.
+
+  The timer is a free running up-counter and can generate interrupts when the
+  counter reaches preset counter values.
+
+properties:
+  compatible:
+    const: ti,da830-timer
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 2
+
+  interrupt-names:
+    minItems: 2
+    items:
+      - const: tint12
+      - const: tint34
+      - const: cmpint0
+      - const: cmpint1
+      - const: cmpint2
+      - const: cmpint3
+      - const: cmpint4
+      - const: cmpint5
+      - const: cmpint6
+      - const: cmpint7
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    timer@20000 {
+        compatible = "ti,da830-timer";
+        reg = <0x20000 0x1000>;
+        interrupts = <21>, <22>;
+        interrupt-names = "tint12", "tint34";
+        clocks = <&pll0_auxclk>;
+    };
+
+...
-- 
2.45.2.827.g557ae147e6.dirty

[RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Kousik Sanagavarapu]

Convert txt bindings of TI's DaVinci/Keystone Watchdog Timer Controller
to dtschema to allow for validation.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
This patch was submitted to the lists before by Nik

	https://lore.kernel.org/linux-devicetree/20231024195839.49607-1-n2h9z4@gmail.com/

Although it seems that the right way include the "power-domians"
property was not decided upon (read through the thread).

I grepped for instances of "power-domains" in ti related SoCs and other
subsystems and it seems that there is always only 1 such "power-domains"
phandle

	Documentation/devicetree/bindings/display/ti/ti,k2g-dss.yaml

The existing dts code also confirms this

	arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi

Again, I guess it would be great if someone could point out if this is
right - so RFC.

Also, shouldn't "clocks" be "required"? - RFC.

 .../bindings/watchdog/davinci-wdt.txt         | 24 ---------
 .../bindings/watchdog/ti,davinci-wdt.yaml     | 52 +++++++++++++++++++
 2 files changed, 52 insertions(+), 24 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
 create mode 100644 Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml

diff --git a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt b/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
deleted file mode 100644
index aa10b8ec36e2..000000000000
--- a/Documentation/devicetree/bindings/watchdog/davinci-wdt.txt
+++ /dev/null
@@ -1,24 +0,0 @@
-Texas Instruments DaVinci/Keystone Watchdog Timer (WDT) Controller
-
-Required properties:
-- compatible : Should be "ti,davinci-wdt", "ti,keystone-wdt"
-- reg : Should contain WDT registers location and length
-
-Optional properties:
-- timeout-sec : Contains the watchdog timeout in seconds
-- clocks : the clock feeding the watchdog timer.
-	   Needed if platform uses clocks.
-	   See clock-bindings.txt
-
-Documentation:
-Davinci DM646x - https://www.ti.com/lit/ug/spruer5b/spruer5b.pdf
-Keystone - https://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
-
-Examples:
-
-wdt: wdt@2320000 {
-	compatible = "ti,davinci-wdt";
-	reg = <0x02320000 0x80>;
-	timeout-sec = <30>;
-	clocks = <&clkwdtimer0>;
-};
diff --git a/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml
new file mode 100644
index 000000000000..1829c407147d
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml
@@ -0,0 +1,52 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/ti,davinci-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI DaVinci/Keystone Watchdog Timer Controller
+
+maintainers:
+  - Kousik Sanagavarapu <five231003@gmail.com>
+
+description: |
+  TI's Watchdog Timer Controller for DaVinci and Keystone Processors.
+
+  Datasheets
+
+    Davinci DM646x - https://www.ti.com/lit/ug/spruer5b/spruer5b.pdf
+    Keystone - https://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
+
+allOf:
+  - $ref: watchdog.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,davinci-wdt
+      - ti,keystone-wdt
+
+  reg:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    watchdog@22f0080 {
+        compatible = "ti,davinci-wdt";
+        reg = <0x022f0080 0x80>;
+        clocks = <&clkwdtimer0>;
+    };
+
+...
-- 
2.45.2.827.g557ae147e6.dirty

[RFC PATCH 3/3] ARM: dts: davinci, keystone: correct watchdog nodenames

[Kousik Sanagavarapu]

Using "wdt" instead of "watchdog" for watchdog timer nodes doesn't allow
for validation with the corresponding dtschema and gives errors

	$ make CHECK_DTBS=y DT_SCHEMA_FILES=ti,davinci-wdt.yaml \
		ti/keystone/keystone-k2g-ice.dtb

	DTC_CHK arch/arm/boot/dts/ti/keystone/keystone-k2g-ice.dtb
	arch/arm/boot/dts/ti/keystone/keystone-k2g-ice.dtb:
	wdt@02250000: $nodename:0: 'wdt@02250000' does not match
	'^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$'
	from schema $id:
	http://devicetree.org/schemas/watchdog/ti,davinci-wdt.yaml#

Therefore change "wdt@" to "watchdog@".

While at it, remove "ti,davinci-wdt" compatible from the keystone dts
code.

Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
---
Question: Should "wdt@" be changed everywhere in the dts code or is it
only a requirement of validation against dtschema?

Also, I'm not sure about removing "ti,davinci-wdt" from the keystone dts
code.  I'm thinking it is only there so that the driver code can get
information from keystone nodes too, because it seems that there is no
code for ti,keystone-wdt.

So question,

- Is WDT Controller driver for keystone not written yet?

Or

- Does the WDT Controller driver for keystone have the same
  functionality as one on davinci - hence leading us to simply do

	.compatible = "ti,keystone-wdt"

  ?

 arch/arm/boot/dts/ti/davinci/da850.dtsi         | 2 +-
 arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi | 4 ++--
 arch/arm/boot/dts/ti/keystone/keystone.dtsi     | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/ti/davinci/da850.dtsi b/arch/arm/boot/dts/ti/davinci/da850.dtsi
index 1d3fb5397ce3..89055ab87256 100644
--- a/arch/arm/boot/dts/ti/davinci/da850.dtsi
+++ b/arch/arm/boot/dts/ti/davinci/da850.dtsi
@@ -525,7 +525,7 @@ clocksource: timer@20000 {
 			interrupt-names = "tint12", "tint34";
 			clocks = <&pll0_auxclk>;
 		};
-		wdt: wdt@21000 {
+		wdt: watchdog@21000 {
 			compatible = "ti,davinci-wdt";
 			reg = <0x21000 0x1000>;
 			clocks = <&pll0_auxclk>;
diff --git a/arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi b/arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi
index dafe485dfe19..884402a5fe4a 100644
--- a/arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi
+++ b/arch/arm/boot/dts/ti/keystone/keystone-k2g.dtsi
@@ -610,8 +610,8 @@ spi3: spi@21806000 {
 			clocks = <&k2g_clks 0x0013 0>;
 		};
 
-		wdt: wdt@02250000 {
-			compatible = "ti,keystone-wdt", "ti,davinci-wdt";
+		wdt: watchdog@02250000 {
+			compatible = "ti,keystone-wdt";
 			reg = <0x02250000 0x80>;
 			power-domains = <&k2g_pds 0x22>;
 			clocks = <&k2g_clks 0x22 0>;
diff --git a/arch/arm/boot/dts/ti/keystone/keystone.dtsi b/arch/arm/boot/dts/ti/keystone/keystone.dtsi
index ff16428860a9..f697f27edcfc 100644
--- a/arch/arm/boot/dts/ti/keystone/keystone.dtsi
+++ b/arch/arm/boot/dts/ti/keystone/keystone.dtsi
@@ -225,8 +225,8 @@ usb0: usb@2690000 {
 			};
 		};
 
-		wdt: wdt@22f0080 {
-			compatible = "ti,keystone-wdt","ti,davinci-wdt";
+		wdt: watchdog@22f0080 {
+			compatible = "ti,keystone-wdt";
 			reg = <0x022f0080 0x80>;
 			clocks = <&clkwdtimer0>;
 		};
-- 
2.45.2.827.g557ae147e6.dirty

Re: [PATCH 1/3] dt-bindings: timer: ti,davinci-timer: convert to dtschema

[Krzysztof Kozlowski]

On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> Convert txt binding of TI's DaVinci timer to dtschema to allow for
> validation.
> 


> diff --git a/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml b/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml
> new file mode 100644
> index 000000000000..615ceb8f30af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml

Use compatible as filename.

> @@ -0,0 +1,68 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/ti,davinci-timer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI DaVinci Timer
> +
> +maintainers:
> +  - Kousik Sanagavarapu <five231003@gmail.com>
> +
> +description: |
> +

Drop blank line

> +  This is a 64-bit timer found on TI's DaVinci architecture devices. The timer
> +  can be configured as a general-purpose 64-bit timer, dual general-purpose
> +  32-bit timers. When configured as dual 32-bit timers, each half can operate
> +  in conjunction (chain mode) or independently (unchained mode) of each other.
> +
> +  The timer is a free running up-counter and can generate interrupts when the
> +  counter reaches preset counter values.
> +
> +properties:
> +  compatible:
> +    const: ti,da830-timer
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    minItems: 2

Missing maxItems

> +
> +  interrupt-names:
> +    minItems: 2
> +    items:
> +      - const: tint12
> +      - const: tint34
> +      - const: cmpint0
> +      - const: cmpint1
> +      - const: cmpint2
> +      - const: cmpint3
> +      - const: cmpint4
> +      - const: cmpint5
> +      - const: cmpint6
> +      - const: cmpint7

Best regards,
Krzysztof

Re: [RFC PATCH 3/3] ARM: dts: davinci, keystone: correct watchdog nodenames

[Krzysztof Kozlowski]

On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> Using "wdt" instead of "watchdog" for watchdog timer nodes doesn't allow
> for validation with the corresponding dtschema and gives errors
> 
> 	$ make CHECK_DTBS=y DT_SCHEMA_FILES=ti,davinci-wdt.yaml \
> 		ti/keystone/keystone-k2g-ice.dtb
> 
> 	DTC_CHK arch/arm/boot/dts/ti/keystone/keystone-k2g-ice.dtb
> 	arch/arm/boot/dts/ti/keystone/keystone-k2g-ice.dtb:
> 	wdt@02250000: $nodename:0: 'wdt@02250000' does not match
> 	'^(timer|watchdog)(@.*|-([0-9]|[1-9][0-9]+))?$'
> 	from schema $id:
> 	http://devicetree.org/schemas/watchdog/ti,davinci-wdt.yaml#
> 
> Therefore change "wdt@" to "watchdog@".
> 
> While at it, remove "ti,davinci-wdt" compatible from the keystone dts
> code.

That's entirely unrelated patch. Don't mix simple cleanups with patches
affecting ABI and users. Also, explain why.

> 
> Signed-off-by: Kousik Sanagavarapu <five231003@gmail.com>
> ---


Best regards,
Krzysztof

Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Krzysztof Kozlowski]

On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> diff --git a/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml b/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml
> new file mode 100644
> index 000000000000..1829c407147d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/watchdog/ti,davinci-wdt.yaml

Use fallback as filename, so ti,keystone-wdt.yaml

> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/watchdog/ti,davinci-wdt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI DaVinci/Keystone Watchdog Timer Controller
> +
> +maintainers:
> +  - Kousik Sanagavarapu <five231003@gmail.com>
> +
> +description: |
> +  TI's Watchdog Timer Controller for DaVinci and Keystone Processors.
> +
> +  Datasheets
> +
> +    Davinci DM646x - https://www.ti.com/lit/ug/spruer5b/spruer5b.pdf
> +    Keystone - https://www.ti.com/lit/ug/sprugv5a/sprugv5a.pdf
> +
> +allOf:
> +  - $ref: watchdog.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,davinci-wdt
> +      - ti,keystone-wdt

This does not match the original binding and commit msg did not explain
why such change is necessary.

This also does not match DTS.


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: timer: ti,davinci-timer: convert to dtschema

[Kousik Sanagavarapu]

On Mon, Jul 22, 2024 at 10:11:45AM +0200, Krzysztof Kozlowski wrote:
> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> > +++ b/Documentation/devicetree/bindings/timer/ti,davinci-timer.yaml
> 
> Use compatible as filename.
> 
> > +description: |
> > +
> 
> Drop blank line
> 
> > +  interrupts:
> > +    minItems: 2
> 
> Missing maxItems

Thanks for pointing out.  Will fix in v2.

Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Kousik Sanagavarapu]

On Mon, Jul 22, 2024 at 10:15:03AM +0200, Krzysztof Kozlowski wrote:
> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,davinci-wdt
> > +      - ti,keystone-wdt
> 
> This does not match the original binding and commit msg did not explain
> why such change is necessary.

I don't understand.  Do you mean both the compatibles are always
compulsory?  Meaning

	compatible:
	  items:
	    - const: ti,davinci-wdt
	    - const: ti,keystone-wdt

It is enum because I intended it to align with the subsequent patch
which changes DTS.

> This also does not match DTS.

Yes.  I've asked about changing the DTS in the subsequent patch.

Thanks for the review

Re: [RFC PATCH 3/3] ARM: dts: davinci, keystone: correct watchdog nodenames

[Kousik Sanagavarapu]

On Mon, Jul 22, 2024 at 10:13:59AM +0200, Krzysztof Kozlowski wrote:
> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> > Using "wdt" instead of "watchdog" for watchdog timer nodes doesn't allow
> > for validation with the corresponding dtschema and gives errors
> >
> > [...]
> >
> That's entirely unrelated patch. Don't mix simple cleanups with patches
> affecting ABI and users. Also, explain why.

Got it.  Will submit v2 as a seperate patch, outside of this series.

Thanks

Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Krzysztof Kozlowski]

On 22/07/2024 15:12, Kousik Sanagavarapu wrote:
> On Mon, Jul 22, 2024 at 10:15:03AM +0200, Krzysztof Kozlowski wrote:
>> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,davinci-wdt
>>> +      - ti,keystone-wdt
>>
>> This does not match the original binding and commit msg did not explain
>> why such change is necessary.
> 
> I don't understand.  Do you mean both the compatibles are always
> compulsory?  Meaning
> 
> 	compatible:
> 	  items:
> 	    - const: ti,davinci-wdt
> 	    - const: ti,keystone-wdt

Yes, this is what old binding said.

> 
> It is enum because I intended it to align with the subsequent patch
> which changes DTS.
> 
>> This also does not match DTS.
> 
> Yes.  I've asked about changing the DTS in the subsequent patch.
> 

Changing the DTS cannot be the reason to affect users and DTS... It's
tautology. You change DTS because you intent to change DTS?

Best regards,
Krzysztof

Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Kousik Sanagavarapu]

On Mon, Jul 22, 2024 at 03:50:15PM +0200, Krzysztof Kozlowski wrote:
> On 22/07/2024 15:12, Kousik Sanagavarapu wrote:
> > On Mon, Jul 22, 2024 at 10:15:03AM +0200, Krzysztof Kozlowski wrote:
> >> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - ti,davinci-wdt
> >>> +      - ti,keystone-wdt
> >>
> >> This does not match the original binding and commit msg did not explain
> >> why such change is necessary.
> > 
> > I don't understand.  Do you mean both the compatibles are always
> > compulsory?  Meaning
> > 
> > 	compatible:
> > 	  items:
> > 	    - const: ti,davinci-wdt
> > 	    - const: ti,keystone-wdt
> 
> Yes, this is what old binding said.

That was what I thought initially too, but the example in the old
binding says otherwise and also the DTS from ti/davinci/da850.dtsi
says

	wdt: watchdog@21000 {
		compatible = "ti,davinci-wdt";
		reg = <0x21000 0x1000>;
		clocks = <&pll0_auxclk>;
		status = "disabled";
	};

Or am I seeing it the wrong way?

> > 
> > It is enum because I intended it to align with the subsequent patch
> > which changes DTS.
> > 
> >> This also does not match DTS.
> > 
> > Yes.  I've asked about changing the DTS in the subsequent patch.
> > 
> 
> Changing the DTS cannot be the reason to affect users and DTS... It's
> tautology. You change DTS because you intent to change DTS?

Not exactly.  I thought that the DTS was wrong when it said

	compatible = "ti,keystone-wdt", "ti,davinci-wdt";

while it should have been

	compatible = "ti,keystone-wdt";

I was not sure about this though and hence marked both the patches as
RFC, in case I was interpretting them the wrong way.

Thanks

Re: [RFC PATCH 3/3] ARM: dts: davinci, keystone: correct watchdog nodenames

[Krzysztof Kozlowski]

On 22/07/2024 15:21, Kousik Sanagavarapu wrote:
> On Mon, Jul 22, 2024 at 10:13:59AM +0200, Krzysztof Kozlowski wrote:
>> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
>>> Using "wdt" instead of "watchdog" for watchdog timer nodes doesn't allow
>>> for validation with the corresponding dtschema and gives errors
>>>
>>> [...]
>>>
>> That's entirely unrelated patch. Don't mix simple cleanups with patches
>> affecting ABI and users. Also, explain why.
> 
> Got it.  Will submit v2 as a seperate patch, outside of this series.

... and carefully re-think why. I am 99% sure your patch breaks the
users. Better if you test your changes.

Conversion of the bindings is a task which requires to know how DTS works.

Best regards,
Krzysztof

Re: [RFC PATCH 2/3] dt-bindings: watchdog: ti,davinci-wdt: convert to dtschema

[Krzysztof Kozlowski]

On 22/07/2024 16:02, Kousik Sanagavarapu wrote:
> On Mon, Jul 22, 2024 at 03:50:15PM +0200, Krzysztof Kozlowski wrote:
>> On 22/07/2024 15:12, Kousik Sanagavarapu wrote:
>>> On Mon, Jul 22, 2024 at 10:15:03AM +0200, Krzysztof Kozlowski wrote:
>>>> On 21/07/2024 18:28, Kousik Sanagavarapu wrote:
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - ti,davinci-wdt
>>>>> +      - ti,keystone-wdt
>>>>
>>>> This does not match the original binding and commit msg did not explain
>>>> why such change is necessary.
>>>
>>> I don't understand.  Do you mean both the compatibles are always
>>> compulsory?  Meaning
>>>
>>> 	compatible:
>>> 	  items:
>>> 	    - const: ti,davinci-wdt
>>> 	    - const: ti,keystone-wdt
>>
>> Yes, this is what old binding said.
> 
> That was what I thought initially too, but the example in the old
> binding says otherwise and also the DTS from ti/davinci/da850.dtsi
> says
> 
> 	wdt: watchdog@21000 {
> 		compatible = "ti,davinci-wdt";
> 		reg = <0x21000 0x1000>;
> 		clocks = <&pll0_auxclk>;
> 		status = "disabled";
> 	};
> 
> Or am I seeing it the wrong way?
> 
>>>
>>> It is enum because I intended it to align with the subsequent patch
>>> which changes DTS.
>>>
>>>> This also does not match DTS.
>>>
>>> Yes.  I've asked about changing the DTS in the subsequent patch.
>>>
>>
>> Changing the DTS cannot be the reason to affect users and DTS... It's
>> tautology. You change DTS because you intent to change DTS?
> 
> Not exactly.  I thought that the DTS was wrong when it said
> 
> 	compatible = "ti,keystone-wdt", "ti,davinci-wdt";
> 
> while it should have been
> 
> 	compatible = "ti,keystone-wdt";
> 
> I was not sure about this though and hence marked both the patches as
> RFC, in case I was interpretting them the wrong way.

Ah, right, the DTS says keystone+davinci while old binding suggested
davinci+keystone. Considering there is no driver binding to keystone, I
think the answer is obvious - intention was keystone+davinci. Anyway,
commit msg should mention why you are doing something else than pure
conversion.

Best regards,
Krzysztof