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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hey Chris,

On Tue, Feb 20, 2024 at 11:18:24AM +1300, Chris Packham wrote:
> From: Ibrahim Tilki <Ibrahim.Tilki@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> Signed-off-by: Zeynep Arslanbenzer <Zeynep.Arslanbenzer@xxxxxxxxxx>
> Signed-off-by: Chris Packham <chris.packham@xxxxxxxxxxxxxxxxxxx>
> ---
>  .../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.

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux