Re: [PATCH 1/2] rtc: pcf85363: add support for timestamp and watchdog

[Rob Herring <robh@kernel.org>]

On Fri, Aug 08, 2025 at 04:52:45PM +0530, Lakshay Piplani wrote:
> Extend the device tree binding for NXP PCF85263/PCF85363 RTC with:
> - Timestamp mode configuration
> - Watchdog timer configuration

The subject should match the subsystem. So:

"dt-bindings: rtc: nxp,pcf85363: ..."

> 
> Also introduce a new header 'pcf85363-tsr.h' to expose
> macros for timestamp mode fields, improving readability
> of device tree file.
> 
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
>  .../devicetree/bindings/rtc/nxp,pcf85363.yaml | 44 ++++++++++++++++++-
>  include/dt-bindings/rtc/pcf85363-tsr.h        | 28 ++++++++++++
>  2 files changed, 71 insertions(+), 1 deletion(-)
>  create mode 100644 include/dt-bindings/rtc/pcf85363-tsr.h
> 
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
> index 52aa3e2091e9..2d2b52f7a9ba 100644
> --- a/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85363.yaml
> @@ -4,7 +4,7 @@
>  $id: http://devicetree.org/schemas/rtc/nxp,pcf85363.yaml#
>  $schema: http://devicetree.org/meta-schemas/core.yaml#
>  
> -title: Philips PCF85263/PCF85363 Real Time Clock
> +title: NXP PCF85263/PCF85363 Real Time Clock
>  
>  maintainers:
>    - Alexandre Belloni <alexandre.belloni@bootlin.com>
> @@ -39,6 +39,41 @@ properties:
>    start-year: true
>    wakeup-source: true
>  
> +  nxp,timestamp-mode:
> +    description: |
> +      Defines timestamp modes for TSR1, TSR2, and TSR3.

You need to define what timestamp mode is.

> +      Use macros from `dt-bindings/rtc/pcf85363-tsr.h`.
> +    items:
> +      - description: TSR1 mode (e.g., PCF85363_TSR1_FE)
> +      - description: TSR2 mode (e.g., PCF85363_TSR2_LB)
> +      - description: TSR3 mode (e.g., PCF85363_TSR3_LV)
> +
> +  nxp,enable-watchdog:
> +    type: boolean
> +    description: |
> +      If present, the RTC watchdog timer is enabled and integrated with Linux watchdog subsystem.

This is OS policy and doesn't belong in DT. If it did, nothing NXP 
specific about it.

You don't need '|' when there is no formatting to preserve, and lines 
should wrap at 80 char.

> +
> +  nxp,watchdog-timeout:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 1
> +    maximum: 31
> +    default: 10
> +    description: |
> +      Watchdog timeout value in seconds. Allowed values range from 1 to 31.

There's already a standard property for this.

> +
> +  nxp,watchdog-stepsize:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 3
> +    default: 0
> +    description: |
> +      Watchdog step size select: 0=0.25Hz, 1=1Hz, 2=4Hz, 3=16Hz.

What's step size? This is counter resolution. Shouldn't this just be the 
best value based on the timeout period.

> +
> +  nxp,watchdog-repeat:
> +    type: boolean
> +    description: |
> +      If present, sets the watchdog to repeat mode. If omitted, watchdog runs in one-shot mode.

Also seems like OS policy.

> +
>  required:
>    - compatible
>    - reg
> @@ -47,6 +82,7 @@ additionalProperties: false
>  
>  examples:
>    - |
> +    #include <dt-bindings/rtc/pcf85363-tsr.h>
>      i2c {
>          #address-cells = <1>;
>          #size-cells = <0>;
> @@ -56,5 +92,11 @@ examples:
>              reg = <0x51>;
>              #clock-cells = <0>;
>              quartz-load-femtofarads = <12500>;
> +            wakeup-source;
> +            nxp,timestamp-mode = <PCF85363_TSR1_FE PCF85363_TSR2_LB PCF85363_TSR3_LV>;
> +            nxp,enable-watchdog;
> +            nxp,watchdog-timeout = <10>;
> +            nxp,watchdog-stepsize = <0>;
> +            nxp,watchdog-repeat;
>          };
>      };
> diff --git a/include/dt-bindings/rtc/pcf85363-tsr.h b/include/dt-bindings/rtc/pcf85363-tsr.h
> new file mode 100644
> index 000000000000..1fb5b9b3601e
> --- /dev/null
> +++ b/include/dt-bindings/rtc/pcf85363-tsr.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> +/*
> + * Copyright 2025 NXP
> + */
> +
> +#ifndef _DT_BINDINGS_RTC_PCF85363_TSR_H
> +#define _DT_BINDINGS_RTC_PCF85363_TSR_H
> +
> +/* TSR1 modes */
> +#define PCF85363_TSR1_NONE 0x00
> +#define PCF85363_TSR1_FE 0x01
> +#define PCF85363_TSR1_LE 0x02
> +
> +/* TSR2 modes */
> +#define PCF85363_TSR2_NONE 0x00
> +#define PCF85363_TSR2_FB 0x01
> +#define PCF85363_TSR2_LB 0x02
> +#define PCF85363_TSR2_LV 0x03
> +#define PCF85363_TSR2_FE 0x04
> +#define PCF85363_TSR2_LE 0x05
> +
> +/* TSR3 modes */
> +#define PCF85363_TSR3_NONE 0x00
> +#define PCF85363_TSR3_FB 0x01
> +#define PCF85363_TSR3_LB 0x02
> +#define PCF85363_TSR3_LV 0x03
> +
> +#endif /* _DT_BINDINGS_RTC_PCF85363_TSR_H */
> -- 
> 2.25.1
>