Received-SPF: pass (google.com: domain of linux-kernel+bounces-73585-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 20 Feb 2024 19:21:03 +0000
From: Conor Dooley <conor@kernel.org>
To: Chris Packham <chris.packham@alliedtelesis.co.nz>
Cc: antoniu.miclaus@analog.com, alexandre.belloni@bootlin.com,
	robh+dt@kernel.org, krzysztof.kozlowski+dt@linaro.org,
	conor+dt@kernel.org, jdelvare@suse.com, linux@roeck-us.net,
	linux-rtc@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-hwmon@vger.kernel.org,
	Ibrahim Tilki <Ibrahim.Tilki@analog.com>,
	Zeynep Arslanbenzer <Zeynep.Arslanbenzer@analog.com>
Subject: Re: [PATCH v7 2/2] dt-bindings: rtc: add max313xx RTCs
Message-ID: <20240220-unified-payback-13b0964c8c29@spud>
References: <20240219221827.3821415-1-chris.packham@alliedtelesis.co.nz>
 <20240219221827.3821415-3-chris.packham@alliedtelesis.co.nz>
Precedence: bulk
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
	protocol="application/pgp-signature"; boundary="Lr0x75LRIzBUufR/"
Content-Disposition: inline
In-Reply-To: <20240219221827.3821415-3-chris.packham@alliedtelesis.co.nz>


--Lr0x75LRIzBUufR/
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hey Chris,

On Tue, Feb 20, 2024 at 11:18:24AM +1300, Chris Packham wrote:
> From: Ibrahim Tilki <Ibrahim.Tilki@analog.com>
>=20
> Add devicetree binding documentation for Analog Devices MAX313XX RTCs.
> This combines the new models with the existing max31335 binding.
>=20
> 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 =3D <1>;
> +        #size-cells =3D <0>;
> +
> +        rtc@68 {
> +            reg =3D <0x68>;
> +            compatible =3D "adi,max31329";
> +            clocks =3D <&clkin>;
> +            interrupt-parent =3D <&gpio>;
> +            interrupts =3D <26 IRQ_TYPE_EDGE_FALLING>;
> +            aux-voltage-chargeable =3D <1>;
> +            trickle-resistor-ohms =3D <6000>;
> +            adi,tc-diode =3D "schottky";
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells =3D <1>;
> +        #size-cells =3D <0>;
> +
> +        rtc@68 {
> +            compatible =3D "adi,max31335";
> +            reg =3D <0x68>;
> +            pinctrl-0 =3D <&rtc_nint_pins>;
> +            interrupts-extended =3D <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
> +            aux-voltage-chargeable =3D <1>;
> +            trickle-resistor-ohms =3D <6000>;
> +            adi,tc-diode =3D "schottky";
> +        };
> +    };
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    i2c {
> +        #address-cells =3D <1>;
> +        #size-cells =3D <0>;
> +
> +        rtc@68 {
> +            reg =3D <0x68>;
> +            compatible =3D "adi,max31331";
> +            #clock-cells =3D <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.

--Lr0x75LRIzBUufR/
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iHUEABYIAB0WIQRh246EGq/8RLhDjO14tDGHoIJi0gUCZdT7ngAKCRB4tDGHoIJi
0gcVAQCUm3UGEM+Z/9EWoRiY+5d63k/n2Hnz9XUiLW/AZJehEwEArBjG41/eCY1Z
LV4ymimhjQsiORRNpOuiTwJItdYREgY=
=hOoo
-----END PGP SIGNATURE-----

--Lr0x75LRIzBUufR/--