Re: [PATCH v2 1/2] dtbindings:rtc:max31335:Add max31331 support

[Krzysztof Kozlowski]

On Fri, Jan 03, 2025 at 12:34:19PM +0530, PavithraUdayakumar-adi wrote:
> Add support to max31331 RTC chip in adi,max31335.yaml

This we see from the diff. What we do not see is why these are not
compatible, for example.

> 
> Signed-off-by: PavithraUdayakumar-adi <pavithra.u@xxxxxxxxxx>

Subject - missing spaces. Use 'git log' to learn how subject is supposed
to look like.

> ---
>  .../devicetree/bindings/rtc/adi,max31335.yaml      | 32 ++++++++++++++++++----
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> index 0125cf6727cc3d9eb3e0253299904ee363ec40ca..6953553d98afd42ed9b79bac476657ffc8ec9210 100644
> --- a/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> +++ b/Documentation/devicetree/bindings/rtc/adi,max31335.yaml
> @@ -13,15 +13,15 @@ description:
>    Analog Devices MAX31335 I2C RTC ±2ppm Automotive Real-Time Clock with
>    Integrated MEMS Resonator.
>  
> -allOf:
> -  - $ref: rtc.yaml#
> -
>  properties:
>    compatible:
> -    const: adi,max31335
> +    enum:
> +      - adi,max31331
> +      - adi,max31335

Devices are not compatible?

>  
>    reg:
> -    maxItems: 1
> +    items:
> +      - enum: [0x68, 0x69]
>  
>    interrupts:
>      maxItems: 1
> @@ -50,6 +50,26 @@ required:
>  
>  unevaluatedProperties: false
>  
> +allOf:
> +  - $ref: rtc.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            enum:
> +              - adi,max31335
> +    then:
> +      properties:
> +        reg:
> +          items:
> +            - const: 0x69
> +    else:
> +      properties:
> +        reg:
> +          items:
> +            - const: 0x68

Just drop it, really no benefits. Why complicating the binding for
checking address?

> +
>  examples:
>    - |
>      #include <dt-bindings/interrupt-controller/irq.h>
> @@ -58,7 +78,7 @@ examples:
>          #size-cells = <0>;
>  
>          rtc@68 {
> -            compatible = "adi,max31335";
> +            compatible = "adi,max31331";

Why? Commit msg is silent about this.

Best regards,
Krzysztof