| Message ID | 20250109-add_support_max31331_fix_3-v1-1-a74fac29bf49@analog.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | Add support for MAX31331 RTC | expand |
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. > > Signed-off-by: PavithraUdayakumar-adi <pavithra.u@analog.com> > --- > .../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..f249313bc485d7a6154ce684726d6a950405ef0e 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" > 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. > + }; > + }; > ... > > -- > 2.25.1 >
> -----Original Message----- > From: Krzysztof Kozlowski <krzk@kernel.org> > Sent: Friday, January 10, 2025 2:05 PM > To: U, Pavithra <Pavithra.U@analog.com> > Cc: Miclaus, Antoniu <Antoniu.Miclaus@analog.com>; Alexandre Belloni > <alexandre.belloni@bootlin.com>; Rob Herring <robh@kernel.org>; Krzysztof > Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Jean > Delvare <jdelvare@suse.com>; Guenter Roeck <linux@roeck-us.net>; > Christophe JAILLET <christophe.jaillet@wanadoo.fr>; linux-rtc@vger.kernel.org; > devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux- > hwmon@vger.kernel.org > 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@analog.com> > > --- > > .../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 > >
On 15/01/2025 11:21, U, Pavithra wrote: >>> >>> 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. Yes, please. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml index 0125cf6727cc3d9eb3e0253299904ee363ec40ca..f249313bc485d7a6154ce684726d6a950405ef0e 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>; 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"; + }; + }; ...