> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@xxxxxxxxxx>
> Sent: Friday, January 10, 2025 2:05 PM
> To: U, Pavithra <Pavithra.U@xxxxxxxxxx>
> Cc: Miclaus, Antoniu <Antoniu.Miclaus@xxxxxxxxxx>; Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxx>; Rob Herring <robh@xxxxxxxxxx>; Krzysztof
> Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Jean
> Delvare <jdelvare@xxxxxxxx>; Guenter Roeck <linux@xxxxxxxxxxxx>;
> Christophe JAILLET <christophe.jaillet@xxxxxxxxxx>; linux-rtc@xxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-
> hwmon@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v3 1/2] dt-bindings: rtc: max31335: Add max31331 support
>
> [External]
>
> On Thu, Jan 09, 2025 at 03:59:57PM +0530, PavithraUdayakumar-adi wrote:
> > MAX31331 is an ultra-low-power, I2C Real-Time Clock RTC with flexible
> > crystal support. While, MAX31335 offers higher precision, MEMS
> > resonator, and integrated temperature sensor. MAX31331 uses I2C
> > address as 0x68 where as max31335 uses 0x69.
> >
> > Changes: Added example for max31331 and modified the register address
> > for max31335.
>
> 1. Why?
> 2. What does it mean "changes"? You did much more so I really do not
> understand this paragraph.
- Added DT compatible string for MAX31331. MAX31331 is compatible with MAX31335 without any additional features.
- Updated I2C address for MAX31335 RTC to 0x69. (I will be reverting this change and sending as fix separately.)
- Included the address 0x69 in property reg for MAX31335. (will remove this change and include in fix)
>
> >
> > Signed-off-by: PavithraUdayakumar-adi <pavithra.u@xxxxxxxxxx>
> > ---
> > .../devicetree/bindings/rtc/adi,max31335.yaml | 22
> ++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
>
> What changed here exactly?
>
> > diff --git a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> > b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> > index
> >
> 0125cf6727cc3d9eb3e0253299904ee363ec40ca..f249313bc485d7a6154ce6847
> 26d
> > 6a950405ef0e 100644
> > --- a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> > +++ b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> > @@ -18,10 +18,13 @@ allOf:
> >
> > properties:
> > compatible:
> > - const: adi,max31335
> > + enum:
> > + - adi,max31331
> > + - adi,max31335
> >
> > reg:
> > - maxItems: 1
> > + items:
> > + - enum: [0x68, 0x69]
> >
> > interrupts:
> > maxItems: 1
> > @@ -57,9 +60,9 @@ examples:
> > #address-cells = <1>;
> > #size-cells = <0>;
> >
> > - rtc@68 {
> > + rtc@69 {
> > compatible = "adi,max31335";
> > - reg = <0x68>;
> > + reg = <0x69>;
>
> Why? I already asked about this - the same question "Why"
While testing, it was identified that the i2c address for max31335 is 0x69. Sorry, I will revert and send the fix in a separate patch.
>
>
> > pinctrl-0 = <&rtc_nint_pins>;
> > interrupts-extended = <&gpio1 16 IRQ_TYPE_LEVEL_HIGH>;
> > aux-voltage-chargeable = <1>; @@ -67,4 +70,15 @@
> > examples:
> > 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";
>
> Drop this example, not necessary.
Ok, I will remove and send in next patch.
>
> > + };
> > + };
> > ...
> >
> > --
> > 2.25.1
> >
