diff mbox series

[v7,2/2] dt-bindings: rtc: add max313xx RTCs

Message ID 20240219221827.3821415-3-chris.packham@alliedtelesis.co.nz
State Superseded
Headers show
Series drivers: rtc: add max313xx series rtc driver | expand

Commit Message

Chris Packham Feb. 19, 2024, 10:18 p.m. UTC 
From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>

Add devicetree binding documentation for Analog Devices MAX313XX RTCs.
This combines the new models with the existing max31335 binding.

Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 .../devicetree/bindings/rtc/adi,max31335.yaml |  70 --------
 .../devicetree/bindings/rtc/adi,max313xx.yaml | 167 ++++++++++++++++++
 2 files changed, 167 insertions(+), 70 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/rtc/adi,max31335.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/adi,max313xx.yaml

Comments

Conor Dooley Feb. 20, 2024, 7:21 p.m. UTC | #1 
Hey Chris,

On Tue, Feb 20, 2024 at 11:18:24AM +1300, Chris Packham wrote:
> From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> 
> Add devicetree binding documentation for Analog Devices MAX313XX RTCs.
> This combines the new models with the existing max31335 binding.
> 
> Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
>  .../devicetree/bindings/rtc/adi,max31335.yaml |  70 --------
>  .../devicetree/bindings/rtc/adi,max313xx.yaml | 167 ++++++++++++++++++

There's no need to do this rename. Having the filename matching one of
the compatibles is our preference.

In addition, it makes it difficult to see what your actual additions are
here. Fortunately, applying the patch locally allows me to use colour
moved and all that jazz, so I can see that the underlying changes to the
file actually look pretty good.

> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc@68 {
> +            reg = <0x68>;
> +            compatible = "adi,max31329";
> +            clocks = <&clkin>;
> +            interrupt-parent = <&gpio>;
> +            interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
> +            aux-voltage-chargeable = <1>;
> +            trickle-resistor-ohms = <6000>;
> +            adi,tc-diode = "schottky";
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc@68 {
> +            compatible = "adi,max31335";
> +            reg = <0x68>;
> +            pinctrl-0 = <&rtc_nint_pins>;
> +            interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
> +            aux-voltage-chargeable = <1>;
> +            trickle-resistor-ohms = <6000>;
> +            adi,tc-diode = "schottky";
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        rtc@68 {
> +            reg = <0x68>;
> +            compatible = "adi,max31331";
> +            #clock-cells = <0>;
> +        };
> +    };

The one thing I do want the comment on is the number of examples.
I don't really see what we gain from having 3 - I'd roll the clock
provider example into with one of the other ones I think.

Cheers,
Conor.
Chris Packham Feb. 20, 2024, 8:11 p.m. UTC | #2 
On 21/02/24 08:21, Conor Dooley wrote:
> Hey Chris,
>
> On Tue, Feb 20, 2024 at 11:18:24AM +1300, Chris Packham wrote:
>> From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
>>
>> Add devicetree binding documentation for Analog Devices MAX313XX RTCs.
>> This combines the new models with the existing max31335 binding.
>>
>> Signed-off-by: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
>> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>   .../devicetree/bindings/rtc/adi,max31335.yaml |  70 --------
>>   .../devicetree/bindings/rtc/adi,max313xx.yaml | 167 ++++++++++++++++++
> There's no need to do this rename. Having the filename matching one of
> the compatibles is our preference.
OK wasn't sure. I'll keep the existing name.
> In addition, it makes it difficult to see what your actual additions are
> here. Fortunately, applying the patch locally allows me to use colour
> moved and all that jazz, so I can see that the underlying changes to the
> file actually look pretty good.
>
>> +examples:
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        rtc@68 {
>> +            reg = <0x68>;
>> +            compatible = "adi,max31329";
>> +            clocks = <&clkin>;
>> +            interrupt-parent = <&gpio>;
>> +            interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
>> +            aux-voltage-chargeable = <1>;
>> +            trickle-resistor-ohms = <6000>;
>> +            adi,tc-diode = "schottky";
>> +        };
>> +    };
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        rtc@68 {
>> +            compatible = "adi,max31335";
>> +            reg = <0x68>;
>> +            pinctrl-0 = <&rtc_nint_pins>;
>> +            interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
>> +            aux-voltage-chargeable = <1>;
>> +            trickle-resistor-ohms = <6000>;
>> +            adi,tc-diode = "schottky";
>> +        };
>> +    };
>> +  - |
>> +    #include <dt-bindings/interrupt-controller/irq.h>
>> +    i2c {
>> +        #address-cells = <1>;
>> +        #size-cells = <0>;
>> +
>> +        rtc@68 {
>> +            reg = <0x68>;
>> +            compatible = "adi,max31331";
>> +            #clock-cells = <0>;
>> +        };
>> +    };
> The one thing I do want the comment on is the number of examples.
> I don't really see what we gain from having 3 - I'd roll the clock
> provider example into with one of the other ones I think.
The 3 examples are an artifact of me combining the in-flight max313xx 
series with the one that landed. The clock output is only valid for some 
chips but that's probably just a matter of picking the right compatible.
Conor Dooley Feb. 21, 2024, 8:02 p.m. UTC | #3 
On Tue, Feb 20, 2024 at 08:11:13PM +0000, Chris Packham wrote:
> On 21/02/24 08:21, Conor Dooley wrote:
> > On Tue, Feb 20, 2024 at 11:18:24AM +1300, Chris Packham wrote:

> > The one thing I do want the comment on is the number of examples.
> > I don't really see what we gain from having 3 - I'd roll the clock
> > provider example into with one of the other ones I think.
> The 3 examples are an artifact of me combining the in-flight max313xx 
> series with the one that landed. The clock output is only valid for some 
> chips but that's probably just a matter of picking the right compatible.

Yeah, I think you could collapse it in, if you modify the compatible
appropriately.

Cheers,
Conor.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
deleted file mode 100644
index 0125cf6727cc..000000000000
--- a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
+++ /dev/null
@@ -1,70 +0,0 @@ 
-# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/rtc/adi,max31335.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Analog Devices MAX31335 RTC
-
-maintainers:
-  - Antoniu Miclaus <antoniu.miclaus@analog.com>
-
-description:
-  Analog Devices MAX31335 I2C RTC ±2ppm Automotive Real-Time Clock with
-  Integrated MEMS Resonator.
-
-allOf:
-  - $ref: rtc.yaml#
-
-properties:
-  compatible:
-    const: adi,max31335
-
-  reg:
-    maxItems: 1
-
-  interrupts:
-    maxItems: 1
-
-  "#clock-cells":
-    description:
-      RTC can be used as a clock source through its clock output pin.
-    const: 0
-
-  adi,tc-diode:
-    description:
-      Select the diode configuration for the trickle charger.
-      schottky - Schottky diode in series.
-      standard+schottky - standard diode + Schottky diode in series.
-    enum: [schottky, standard+schottky]
-
-  trickle-resistor-ohms:
-    description:
-      Selected resistor for trickle charger. Should be specified if trickle
-      charger should be enabled.
-    enum: [3000, 6000, 11000]
-
-required:
-  - compatible
-  - reg
-
-unevaluatedProperties: false
-
-examples:
-  - |
-    #include <dt-bindings/interrupt-controller/irq.h>
-    i2c {
-        #address-cells = <1>;
-        #size-cells = <0>;
-
-        rtc@68 {
-            compatible = "adi,max31335";
-            reg = <0x68>;
-            pinctrl-0 = <&rtc_nint_pins>;
-            interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
-            aux-voltage-chargeable = <1>;
-            trickle-resistor-ohms = <6000>;
-            adi,tc-diode = "schottky";
-        };
-    };
-...
diff --git a/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml b/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml
new file mode 100644
index 000000000000..e56e5394aa86
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/adi,max313xx.yaml
@@ -0,0 +1,167 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+# Copyright 2022 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/adi,max313xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX313XX series I2C RTCs
+
+maintainers:
+  - Antoniu Miclaus <antoniu.miclaus@analog.com>
+  - Chris Packham <chris.packham@alliedtelesis.co.nz>
+
+description: Analog Devices MAX313XX series I2C RTCs.
+
+properties:
+  compatible:
+    enum:
+      - adi,max31328
+      - adi,max31329
+      - adi,max31331
+      - adi,max31334
+      - adi,max31335
+      - adi,max31341
+      - adi,max31342
+      - adi,max31343
+
+  reg:
+    description: I2C address of the RTC
+    items:
+      - enum: [0x68, 0x69]
+
+  interrupts:
+    description:
+      Alarm1 interrupt line of the RTC. Some of the RTCs have two interrupt
+      lines and alarm1 interrupt muxing depends on the clockin/clockout
+      configuration.
+    maxItems: 1
+
+  "#clock-cells":
+    description:
+      RTC can be used as a clock source through its clock output pin when
+      supplied.
+    const: 0
+
+  clocks:
+    description:
+      RTC uses this clock for clock input when supplied. Clock has to provide
+      one of these four frequencies - 1Hz, 50Hz, 60Hz or 32.768kHz.
+    maxItems: 1
+
+  adi,tc-diode:
+    description:
+      Select the diode configuration for the trickle charger.
+      schottky - Schottky diode in series.
+      standard+schottky - standard diode + Schottky diode in series.
+    enum: [schottky, standard+schottky]
+
+  trickle-resistor-ohms:
+    description:
+      Selected resistor for trickle charger. Should be specified if trickle
+      charger should be enabled.
+    enum: [3000, 6000, 11000]
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: rtc.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31328
+              - adi,max31342
+
+    then:
+      properties:
+        aux-voltage-chargeable: false
+        trickle-resistor-ohms: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31328
+              - adi,max31331
+              - adi,max31334
+              - adi,max31335
+              - adi,max31343
+
+    then:
+      properties:
+        clocks: false
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            enum:
+              - adi,max31341
+              - adi,max31342
+
+    then:
+      properties:
+        reg:
+          items:
+            - const: 0x69
+
+    else:
+      properties:
+        reg:
+          items:
+            - const: 0x68
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@68 {
+            reg = <0x68>;
+            compatible = "adi,max31329";
+            clocks = <&clkin>;
+            interrupt-parent = <&gpio>;
+            interrupts = <26 IRQ_TYPE_EDGE_FALLING>;
+            aux-voltage-chargeable = <1>;
+            trickle-resistor-ohms = <6000>;
+            adi,tc-diode = "schottky";
+        };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@68 {
+            compatible = "adi,max31335";
+            reg = <0x68>;
+            pinctrl-0 = <&rtc_nint_pins>;
+            interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
+            aux-voltage-chargeable = <1>;
+            trickle-resistor-ohms = <6000>;
+            adi,tc-diode = "schottky";
+        };
+    };
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@68 {
+            reg = <0x68>;
+            compatible = "adi,max31331";
+            #clock-cells = <0>;
+        };
+    };
+...