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 for Android: free password hash cracker in your pocket

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

Message-ID: <021800f9-c81c-41fb-b907-5cb27a8854d2@alliedtelesis.co.nz>
Date: Tue, 20 Feb 2024 20:11:13 +0000
From: Chris Packham <Chris.Packham@...iedtelesis.co.nz>
To: Conor Dooley <conor@...nel.org>
CC: "antoniu.miclaus@...log.com" <antoniu.miclaus@...log.com>,
	"alexandre.belloni@...tlin.com" <alexandre.belloni@...tlin.com>,
	"robh+dt@...nel.org" <robh+dt@...nel.org>,
	"krzysztof.kozlowski+dt@...aro.org" <krzysztof.kozlowski+dt@...aro.org>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "jdelvare@...e.com"
	<jdelvare@...e.com>, "linux@...ck-us.net" <linux@...ck-us.net>,
	"linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-hwmon@...r.kernel.org" <linux-hwmon@...r.kernel.org>, "Zeynep
 Arslanbenzer" <Zeynep.Arslanbenzer@...log.com>
Subject: Re: [PATCH v7 2/2] dt-bindings: rtc: add max313xx RTCs


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@...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.
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.