diff mbox series

[1/2] dt-bindings: rtc: Add TI K3 RTC devicetree bindings documentation

Message ID 20220412073138.25027-2-nm@ti.com (mailing list archive)
State New, archived
Headers show
Series rtc: Introduce rtc-ti-k3 | expand

Commit Message

Nishanth Menon April 12, 2022, 7:31 a.m. UTC
This adds the documentation for the devicetree bindings of the Texas
Instruments RTC modules on K3 family of SoCs such as AM62x SoCs or
newer.

Signed-off-by: Nishanth Menon <nm@ti.com>
---
 .../devicetree/bindings/rtc/ti,k3-rtc.yaml    | 87 +++++++++++++++++++
 1 file changed, 87 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/ti,k3-rtc.yaml

Comments

Krzysztof Kozlowski April 12, 2022, 12:06 p.m. UTC | #1
On 12/04/2022 09:31, Nishanth Menon wrote:
> This adds the documentation for the devicetree bindings of the Texas
> Instruments RTC modules on K3 family of SoCs such as AM62x SoCs or
> newer.
> 

Thank you for your patch. There is something to discuss/improve.

(...)

> +properties:
> +  compatible:
> +    items:

No need for items. Just enum under the compatible.

> +      - enum:
> +          - ti,am62-rtc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: VBUS Interface clock
> +      - description: 32k Clock source (external or internal).
> +
> +  clock-names:
> +    items:
> +      - const: "vbus"
> +      - const: "osc32k"

No quotes.

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  assigned-clocks:
> +    description: |
> +      override default osc32k parent clock reference to the osc32k clock entry
> +    maxItems: 1
> +
> +  assigned-clock-parents:
> +    description: |
> +      override default osc32k parent clock phandle of the new parent clock of osc32k
> +    maxItems: 1

Usually assigned-clockXXX are not needed in the bindings. Is here
something different? They are put only to indicate something special.

> +
> +  wakeup-source: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    rtc@2b1f0000 {
> +        compatible = "ti,am62-rtc";
> +        reg = <0x2b1f0000 0x100>;
> +        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +        power-domains = <&bar 0>;
> +        clocks = <&foo 0>, <&foo 1>;
> +        clock-names = "vbus", "osc32k";
> +        wakeup-source;
> +    };
> +
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    rtc@2b1f0000 {
> +        compatible = "ti,am62-rtc";
> +        reg = <0x2b1f0000 0x100>;
> +        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> +        power-domains = <&bar 0>;
> +        clocks = <&foo 0>, <&foo 1>;
> +        clock-names = "vbus", "osc32k";
> +        wakeup-source;
> +        assigned-clocks = <&foo 1>;
> +        assigned-clock-parents = <&foo 2>;
> +

Unneeded blank line.

> +    };


Best regards,
Krzysztof
Nishanth Menon April 12, 2022, 10:17 p.m. UTC | #2
On 14:06-20220412, Krzysztof Kozlowski wrote:
> > +properties:
> > +  compatible:
> > +    items:
> 
> No need for items. Just enum under the compatible.

Will fix in next rev. Thanks for catching.

> 
> > +      - enum:
> > +          - ti,am62-rtc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: VBUS Interface clock
> > +      - description: 32k Clock source (external or internal).
> > +
> > +  clock-names:
> > +    items:
> > +      - const: "vbus"
> > +      - const: "osc32k"
> 
> No quotes.

Uggh.. my bad. yup
> 
> > +
> > +  power-domains:
> > +    maxItems: 1
> > +
> > +  assigned-clocks:
> > +    description: |
> > +      override default osc32k parent clock reference to the osc32k clock entry
> > +    maxItems: 1
> > +
> > +  assigned-clock-parents:
> > +    description: |
> > +      override default osc32k parent clock phandle of the new parent clock of osc32k
> > +    maxItems: 1
> 
> Usually assigned-clockXXX are not needed in the bindings. Is here
> something different? They are put only to indicate something special.

I wonder if I should rather use unevaluatedproperties instead? If I use
additionalProperties: False, then the second example below fails.

Thoughts?
> 
> > +
> > +  wakeup-source: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    rtc@2b1f0000 {
> > +        compatible = "ti,am62-rtc";
> > +        reg = <0x2b1f0000 0x100>;
> > +        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> > +        power-domains = <&bar 0>;
> > +        clocks = <&foo 0>, <&foo 1>;
> > +        clock-names = "vbus", "osc32k";
> > +        wakeup-source;
> > +    };
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    rtc@2b1f0000 {
> > +        compatible = "ti,am62-rtc";
> > +        reg = <0x2b1f0000 0x100>;
> > +        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
> > +        power-domains = <&bar 0>;
> > +        clocks = <&foo 0>, <&foo 1>;
> > +        clock-names = "vbus", "osc32k";
> > +        wakeup-source;
> > +        assigned-clocks = <&foo 1>;
> > +        assigned-clock-parents = <&foo 2>;
> > +
> 
> Unneeded blank line.

Ack.
Krzysztof Kozlowski April 13, 2022, 6:42 a.m. UTC | #3
On 13/04/2022 00:17, Nishanth Menon wrote:
>>> +  assigned-clocks:
>>> +    description: |
>>> +      override default osc32k parent clock reference to the osc32k clock entry
>>> +    maxItems: 1
>>> +
>>> +  assigned-clock-parents:
>>> +    description: |
>>> +      override default osc32k parent clock phandle of the new parent clock of osc32k
>>> +    maxItems: 1
>>
>> Usually assigned-clockXXX are not needed in the bindings. Is here
>> something different? They are put only to indicate something special.
> 
> I wonder if I should rather use unevaluatedproperties instead? If I use
> additionalProperties: False, then the second example below fails.
> 

Are you sure it fails? I just checked and it worked in my case. This
AFAIR was working since some time (or fixed some time ago), so maybe
update your dtschema?

Best regards,
Krzysztof
Nishanth Menon April 13, 2022, 3:45 p.m. UTC | #4
On 08:42-20220413, Krzysztof Kozlowski wrote:
> On 13/04/2022 00:17, Nishanth Menon wrote:
> >>> +  assigned-clocks:
> >>> +    description: |
> >>> +      override default osc32k parent clock reference to the osc32k clock entry
> >>> +    maxItems: 1
> >>> +
> >>> +  assigned-clock-parents:
> >>> +    description: |
> >>> +      override default osc32k parent clock phandle of the new parent clock of osc32k
> >>> +    maxItems: 1
> >>
> >> Usually assigned-clockXXX are not needed in the bindings. Is here
> >> something different? They are put only to indicate something special.
> > 
> > I wonder if I should rather use unevaluatedproperties instead? If I use
> > additionalProperties: False, then the second example below fails.
> > 
> 
> Are you sure it fails? I just checked and it worked in my case. This
> AFAIR was working since some time (or fixed some time ago), so maybe
> update your dtschema?

Arrgh, Thanks and you are right.
Apologies, I should have cross checked again since developing late
last year (understood the min schema currently is 2022.3).

Will fix this up in the repost v2 in a short while.
Rob Herring (Arm) April 14, 2022, 3:53 p.m. UTC | #5
On Tue, Apr 12, 2022 at 02:31:37AM -0500, Nishanth Menon wrote:
> This adds the documentation for the devicetree bindings of the Texas
> Instruments RTC modules on K3 family of SoCs such as AM62x SoCs or
> newer.

'devicetree bindings documentation' in the subject is redundant. You 
already said that with 'dt-bindings'.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/rtc/ti,k3-rtc.yaml b/Documentation/devicetree/bindings/rtc/ti,k3-rtc.yaml
new file mode 100644
index 000000000000..16aebb8013a1
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/ti,k3-rtc.yaml
@@ -0,0 +1,87 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/ti,k3-rtc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments K3 Real Time Clock
+
+maintainers:
+  - Nishanth Menon <nm@ti.com>
+
+description: |
+  This RTC appears in the AM62x family of SoCs.
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - ti,am62-rtc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: VBUS Interface clock
+      - description: 32k Clock source (external or internal).
+
+  clock-names:
+    items:
+      - const: "vbus"
+      - const: "osc32k"
+
+  power-domains:
+    maxItems: 1
+
+  assigned-clocks:
+    description: |
+      override default osc32k parent clock reference to the osc32k clock entry
+    maxItems: 1
+
+  assigned-clock-parents:
+    description: |
+      override default osc32k parent clock phandle of the new parent clock of osc32k
+    maxItems: 1
+
+  wakeup-source: true
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    rtc@2b1f0000 {
+        compatible = "ti,am62-rtc";
+        reg = <0x2b1f0000 0x100>;
+        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+        power-domains = <&bar 0>;
+        clocks = <&foo 0>, <&foo 1>;
+        clock-names = "vbus", "osc32k";
+        wakeup-source;
+    };
+
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    rtc@2b1f0000 {
+        compatible = "ti,am62-rtc";
+        reg = <0x2b1f0000 0x100>;
+        interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
+        power-domains = <&bar 0>;
+        clocks = <&foo 0>, <&foo 1>;
+        clock-names = "vbus", "osc32k";
+        wakeup-source;
+        assigned-clocks = <&foo 1>;
+        assigned-clock-parents = <&foo 2>;
+
+    };