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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 20 Feb 2024 19:21:03 +0000
From: Conor Dooley <conor@...nel.org>
To: Chris Packham <chris.packham@...iedtelesis.co.nz>
Cc: antoniu.miclaus@...log.com, alexandre.belloni@...tlin.com,
	robh+dt@...nel.org, krzysztof.kozlowski+dt@...aro.org,
	conor+dt@...nel.org, jdelvare@...e.com, linux@...ck-us.net,
	linux-rtc@...r.kernel.org, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hwmon@...r.kernel.org,
	Ibrahim Tilki <Ibrahim.Tilki@...log.com>,
	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@...log.com>
Subject: Re: [PATCH v7 2/2] dt-bindings: rtc: add max313xx RTCs

Hey Chris,

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

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)