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.dirtyOn 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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.