[PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config

[PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config

[Lakshay Piplani]

NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
which latch the current time when a selected event occurs. Add a
vendor specific property, nxp,timestamp-mode, to select the event
source for each register.

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>
---
V2 -> V3:
- No changes in v3
- Added Reviewed-by: Rob Herring <robh@kernel.org>
V1 -> V2:
- Addressed review comments from Rob Herring:
  * use $ref: /schemas/types.yaml#/definitions/uint32-array
  * tuple form with exactly 3 items (TSR1/TSR2/TSR3), per items decimal enums
  * define 'nxp,timestamp-mode' clearly
  * drop watchdog related vendor properties
  * remove watchdog related vendor properties from i2c example

 .../devicetree/bindings/rtc/nxp,pcf85363.yaml | 23 ++++++++++++++-
 include/dt-bindings/rtc/pcf85363-tsr.h        | 28 +++++++++++++++++++
 2 files changed, 50 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..cf9c155162d6 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,24 @@ properties:
   start-year: true
   wakeup-source: true
 
+  nxp,timestamp-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - enum: [0, 1, 2] # TSR1: NONE, FE, LE
+        description: TSR1 mode
+      - enum: [0, 1, 2, 3, 4, 5] # TSR2: NONE, FB, LB, LV, FE, LE
+        description: TSR2 mode
+      - enum: [0, 1, 2, 3] # TSR3: NONE, FB, LB, LV
+        description: TSR3 mode
+    description: |
+      Defines timestamp modes for TSR1, TSR2, and TSR3.
+      Use macros from <dt-bindings/rtc/pcf85363-tsr.h>.
+
+      Each value corresponds to a mode constant:
+        - TSR1: NONE, FE, LE
+        - TSR2: NONE, FB, LB, LV, FE, LE
+        - TSR3: NONE, FB, LB, LV
+
 required:
   - compatible
   - reg
@@ -47,6 +65,7 @@ additionalProperties: false
 
 examples:
   - |
+    #include <dt-bindings/rtc/pcf85363-tsr.h>
     i2c {
         #address-cells = <1>;
         #size-cells = <0>;
@@ -56,5 +75,7 @@ examples:
             reg = <0x51>;
             #clock-cells = <0>;
             quartz-load-femtofarads = <12500>;
+            wakeup-source;
+            nxp,timestamp-mode = <PCF85363_TSR1_FE PCF85363_TSR2_LB PCF85363_TSR3_LV>;
         };
     };
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

[PATCH v3 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL

[Lakshay Piplani]

Add battery switch-over reporting for PCF85263/PCF85363 using the standard
RTC_VL_* ioctl interface. When the backup supply takes over, the BSF flag
is exposed to userspace through RTC_VL_READ and can be cleared using
RTC_VL_CLR.

This allows applications to detect loss of main power without relying on
non-standard interfaces.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and commentsi

 drivers/rtc/rtc-pcf85363.c | 49 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 540042b9eec8..c03d5a65c5f7 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -14,6 +14,7 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/bcd.h>
+#include <linux/device.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
 
@@ -295,23 +296,67 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 {
 	struct pcf85363 *pcf85363 = i2c_get_clientdata(dev_id);
+	bool handled = false;
 	unsigned int flags;
 	int err;
 
 	err = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
+
 	if (err)
 		return IRQ_NONE;
 
+	if (flags) {
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+			(flags & FLAGS_BSF) ? " [BSF]" : "");
+	}
+
 	if (flags & FLAGS_A1F) {
 		rtc_update_irq(pcf85363->rtc, 1, RTC_IRQF | RTC_AF);
 		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A1F, 0);
-		return IRQ_HANDLED;
+		handled = true;
 	}
 
-	return IRQ_NONE;
+	if (flags & FLAGS_BSF) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+		handled = true;
+	}
+
+	return handled ? IRQ_HANDLED : IRQ_NONE;
+}
+
+static int pcf85363_rtc_ioctl(struct device *dev,
+			      unsigned int cmd, unsigned long arg)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	switch (cmd) {
+	case RTC_VL_READ: {
+		u32 status = 0;
+
+		ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &val);
+
+		if (ret)
+			return ret;
+
+		if (val & FLAGS_BSF)
+			status |= RTC_VL_BACKUP_SWITCH;
+
+		return put_user(status, (u32 __user *)arg);
+	}
+
+	case RTC_VL_CLR:
+		return regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
+
+	default:
+		return -ENOIOCTLCMD;
+	}
 }
 
 static const struct rtc_class_ops rtc_ops = {
+	.ioctl  = pcf85363_rtc_ioctl,
 	.read_time	= pcf85363_rtc_read_time,
 	.set_time	= pcf85363_rtc_set_time,
 	.read_alarm	= pcf85363_rtc_read_alarm,
-- 
2.25.1

[PATCH v3 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode

[Lakshay Piplani]

Add support for the timestamp capture registers available on PCF85263 and
PCF85363. The registers latch the current time when selected events occur,
such as TS pin activation or battery switch-over.

The capture source can be configured via the nxp,timestamp-mode device
tree property, and latched values are exported through read-only sysfs
attributes.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments

 drivers/rtc/rtc-pcf85363.c | 210 +++++++++++++++++++++++++++++++------
 1 file changed, 176 insertions(+), 34 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index c03d5a65c5f7..a8b4f48d9894 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -101,19 +101,31 @@
 #define PIN_IO_INTA_OUT	2
 #define PIN_IO_INTA_HIZ	3
 
+#define PIN_IO_TSPM     GENMASK(3, 2)
+#define PIN_IO_TSIM     BIT(4)
+
 #define OSC_CAP_SEL	GENMASK(1, 0)
 #define OSC_CAP_6000	0x01
 #define OSC_CAP_12500	0x02
 
 #define STOP_EN_STOP	BIT(0)
+#define RTCM_BIT        BIT(4)
 
 #define RESET_CPR	0xa4
 
 #define NVRAM_SIZE	0x40
 
+#define TSR1_MASK       0x03
+#define TSR2_MASK       0x07
+#define TSR3_MASK       0x03
+#define TSR1_SHIFT      0
+#define TSR2_SHIFT      2
+#define TSR3_SHIFT      6
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
+	u8 ts_valid_flags;
 };
 
 struct pcf85x63_config {
@@ -306,8 +318,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (flags) {
-		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s\n",
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
 			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
+			(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
+			(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
+			(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
 			(flags & FLAGS_BSF) ? " [BSF]" : "");
 	}
 
@@ -317,6 +332,24 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
+	if (flags & FLAGS_TSR1F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR1F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR1F;
+		handled = true;
+	}
+
+	if (flags & FLAGS_TSR2F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR2F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR2F;
+		handled = true;
+	}
+
+	if (flags & FLAGS_TSR3F) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_TSR3F, 0);
+		pcf85363->ts_valid_flags |= FLAGS_TSR3F;
+		handled = true;
+	}
+
 	if (flags & FLAGS_BSF) {
 		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_BSF, 0);
 		handled = true;
@@ -424,11 +457,94 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/*
+ * Reads 6 bytes of timestamp data starting at the given base register,
+ * converts them from BCD to binary, and formats the result into a
+ * human-readable string in "YYYY-MM-DD HH:MM:SS" format.
+ */
+static int pcf85363_read_timestamp(struct pcf85363 *pcf85363, u8 base_reg, char *buf)
+{
+	struct rtc_time tm;
+	u8 regs[6];
+	int ret;
+
+	ret = regmap_bulk_read(pcf85363->regmap, base_reg, regs, sizeof(regs));
+
+	if (ret)
+		return ret;
+
+	tm.tm_sec = bcd2bin(regs[0]);
+	tm.tm_min = bcd2bin(regs[1]);
+	tm.tm_hour = bcd2bin(regs[2]);
+	tm.tm_mday = bcd2bin(regs[3]);
+	tm.tm_mon = bcd2bin(regs[4]) - 1;
+	tm.tm_year = bcd2bin(regs[5]) + 100;
+
+	return sysfs_emit(buf, "%04d-%02d-%02d %02d:%02d:%02d\n",
+			  tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday,
+			  tm.tm_hour, tm.tm_min, tm.tm_sec);
+}
+
+/*
+ * Checks whether a specific timestamp flag is set. If so, reads and
+ * returns the formatted timestamp. Otherwise, returns "00-00-00 00:00:00".
+ */
+
+static ssize_t pcf85363_timestamp_show(struct device *dev, char *buf,
+				       u8 timestamp_flag, u8 base_reg)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+	if (!(pcf85363->ts_valid_flags & timestamp_flag))
+		return sysfs_emit(buf, "00-00-00 00:00:00\n");
+
+	return pcf85363_read_timestamp(pcf85363, base_reg, buf);
+}
+
+static ssize_t timestamp1_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR1F, DT_TIMESTAMP1);
+}
+static DEVICE_ATTR_RO(timestamp1);
+
+static ssize_t timestamp2_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR2F, DT_TIMESTAMP2);
+}
+static DEVICE_ATTR_RO(timestamp2);
+
+static ssize_t timestamp3_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, FLAGS_TSR3F, DT_TIMESTAMP3);
+}
+static DEVICE_ATTR_RO(timestamp3);
+
+static struct attribute *pcf85363_attrs[] = {
+	&dev_attr_timestamp1.attr,
+	&dev_attr_timestamp2.attr,
+	&dev_attr_timestamp3.attr,
+	NULL,
+};
+
+static const struct attribute_group pcf85363_attr_group = {
+	.attrs = pcf85363_attrs,
+};
+
 static int pcf85363_probe(struct i2c_client *client)
 {
-	struct pcf85363 *pcf85363;
 	const struct pcf85x63_config *config = &pcf_85363_config;
 	const void *data = of_device_get_match_data(&client->dev);
+	struct device *dev = &client->dev;
+	struct pcf85363 *pcf85363;
+	int irq_a = client->irq;
+	bool wakeup_source;
+	int ret, i, err;
+	u32 tsr_mode[3];
+	u8 val;
+
 	static struct nvmem_config nvmem_cfg[] = {
 		{
 			.name = "pcf85x63-",
@@ -446,25 +562,43 @@ static int pcf85363_probe(struct i2c_client *client)
 			.reg_write = pcf85363_nvram_write,
 		},
 	};
-	int ret, i, err;
-	bool wakeup_source;
 
 	if (data)
 		config = data;
 
-	pcf85363 = devm_kzalloc(&client->dev, sizeof(struct pcf85363),
-				GFP_KERNEL);
+	pcf85363 = devm_kzalloc(&client->dev, sizeof(*pcf85363), GFP_KERNEL);
 	if (!pcf85363)
 		return -ENOMEM;
 
+	pcf85363->ts_valid_flags = 0;
+
 	pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
-	if (IS_ERR(pcf85363->regmap)) {
-		dev_err(&client->dev, "regmap allocation failed\n");
-		return PTR_ERR(pcf85363->regmap);
-	}
+	if (IS_ERR(pcf85363->regmap))
+		return dev_err_probe(dev, PTR_ERR(pcf85363->regmap), "regmap init failed\n");
 
 	i2c_set_clientdata(client, pcf85363);
 
+	ret = regmap_update_bits(pcf85363->regmap, CTRL_FUNCTION, RTCM_BIT, 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable RTC mode\n");
+
+	if (!device_property_read_u32_array(dev, "nxp,timestamp-mode", tsr_mode, 3)) {
+		tsr_mode[0] &= TSR1_MASK;
+		tsr_mode[1] &= TSR2_MASK;
+		tsr_mode[2] &= TSR3_MASK;
+
+		val = (tsr_mode[2] << TSR3_SHIFT) |
+		      (tsr_mode[1] << TSR2_SHIFT) |
+		      (tsr_mode[0] << TSR1_SHIFT);
+
+		ret = regmap_write(pcf85363->regmap, DT_TS_MODE, val);
+		if (ret)
+			dev_warn(dev, "Failed to write timestamp mode register\n");
+
+		dev_dbg(dev, "Timestamp mode set: TSR1=0x%x TSR2=0x%x TSR3=0x%x\n",
+			tsr_mode[0], tsr_mode[1], tsr_mode[2]);
+	}
+
 	pcf85363->rtc = devm_rtc_allocate_device(&client->dev);
 	if (IS_ERR(pcf85363->rtc))
 		return PTR_ERR(pcf85363->rtc);
@@ -478,39 +612,47 @@ static int pcf85363_probe(struct i2c_client *client)
 	pcf85363->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
 	pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
 
-	wakeup_source = device_property_read_bool(&client->dev,
-						  "wakeup-source");
-	if (client->irq > 0 || wakeup_source) {
-		regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
-		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
-				   PIN_IO_INTAPM, PIN_IO_INTA_OUT);
-	}
+	wakeup_source = device_property_read_bool(dev, "wakeup-source");
+
+	ret = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0x00);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to clear CTRL_FLAGS\n");
 
-	if (client->irq > 0) {
-		unsigned long irqflags = IRQF_TRIGGER_LOW;
+	if (irq_a > 0) {
+		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO, PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+		ret = devm_request_threaded_irq(dev, irq_a, NULL,
+						pcf85363_rtc_handle_irq,
+						IRQF_TRIGGER_LOW | IRQF_ONESHOT,
+						"pcf85363-inta", client);
 
-		if (dev_fwnode(&client->dev))
-			irqflags = 0;
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL, pcf85363_rtc_handle_irq,
-						irqflags | IRQF_ONESHOT,
-						"pcf85363", client);
 		if (ret) {
-			dev_warn(&client->dev,
-				 "unable to request IRQ, alarms disabled\n");
-			client->irq = 0;
+			dev_err_probe(dev, ret, "INTA IRQ request failed\n");
+			irq_a = 0;
+		} else {
+			regmap_write(pcf85363->regmap, CTRL_INTA_EN, INT_BSIE
+				     | INT_TSRIE);
 		}
 	}
 
-	if (client->irq > 0 || wakeup_source) {
-		device_init_wakeup(&client->dev, true);
-		set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
-	} else {
-		clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
-	}
+	regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+			   PIN_IO_TSPM | PIN_IO_TSIM,
+			   PIN_IO_TSPM | PIN_IO_TSIM);
+
+	if (irq_a > 0 || wakeup_source)
+		device_init_wakeup(dev, true);
+
+	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
 
 	ret = devm_rtc_register_device(pcf85363->rtc);
 
+	if (ret)
+		return dev_err_probe(dev, ret, "RTC registration failed\n");
+
+	ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
+
+	if (ret)
+		return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
+
 	for (i = 0; i < config->num_nvram; i++) {
 		nvmem_cfg[i].priv = pcf85363;
 		devm_rtc_nvmem_register(pcf85363->rtc, &nvmem_cfg[i]);
-- 
2.25.1

[PATCH v3 4/5] rtc: pcf85363: add oscillator offset calibration support

[Lakshay Piplani]

Expose the oscillator offset register of PCF85263/PCF85363 through the
rtc_class_ops read_offset and set_offset callbacks, allowing userspace
to apply frequency correction for drift compensation.

The correction mode defaults to normal mode (OFFM = 0), where each step
introduces an offset of approximately 2.170 ppm and corrections occur
every 4 hours.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments

 drivers/rtc/rtc-pcf85363.c | 46 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index a8b4f48d9894..3d733375187b 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -122,6 +122,11 @@
 #define TSR2_SHIFT      2
 #define TSR3_SHIFT      6
 
+#define OFFSET_SIGN_BIT 7
+#define OFFSET_MINIMUM  -128
+#define OFFSET_MAXIMUM  127
+#define OFFSET_MASK     0xFF
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
@@ -358,6 +363,45 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
+/*
+ * Read the current RTC offset from the CTRL_OFFSET
+ * register. This value is an 8-bit signed 2's complement
+ * value that corrects osciallator drift.
+ */
+static int pcf85363_read_offset(struct device *dev, long *offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OFFSET, &val);
+
+	if (ret)
+		return ret;
+
+	*offset = sign_extend32(val & OFFSET_MASK, OFFSET_SIGN_BIT);
+
+	return 0;
+}
+
+/*
+ * Write an oscillator offset correction value to
+ * the CTRL_OFFSET register. The valid range is
+ * -128 to 127 (8-bit signed), typically used to fine
+ * tune accuracy.
+ */
+static int pcf85363_set_offset(struct device *dev, long offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+
+	if (offset < OFFSET_MINIMUM || offset > OFFSET_MAXIMUM) {
+		dev_warn(dev, "Offset out of range: %ld\n", offset);
+		return -ERANGE;
+	}
+
+	return regmap_write(pcf85363->regmap, CTRL_OFFSET, offset & OFFSET_MASK);
+}
+
 static int pcf85363_rtc_ioctl(struct device *dev,
 			      unsigned int cmd, unsigned long arg)
 {
@@ -395,6 +439,8 @@ static const struct rtc_class_ops rtc_ops = {
 	.read_alarm	= pcf85363_rtc_read_alarm,
 	.set_alarm	= pcf85363_rtc_set_alarm,
 	.alarm_irq_enable = pcf85363_rtc_alarm_irq_enable,
+	.read_offset = pcf85363_read_offset,
+	.set_offset = pcf85363_set_offset,
 };
 
 static int pcf85363_nvram_read(void *priv, unsigned int offset, void *val,
-- 
2.25.1

[PATCH v3 5/5] rtc: pcf85363: add watchdog support with configurable step size

[Lakshay Piplani]

Add watchdog timer support to PCF85263/PCF85363 using the linux watchdog
subsystem. The driver programs the hardware watchdog timeout based on
the requested period.

Also use rtc_add_group() instead of sysfs_create_group() to register
timestamp attributes under the RTC class device (/sys/class/rtc/rtcX).

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V2 -> V3:
- Split into separate patches as suggested:
  - Battery switch-over detection.
  - Timestamp recording for TS pin and battery switch-over events.
  - Offset calibration.
  - Watchdog timer (to be reviewed by watchdog maintainers).
- Dropped Alarm2 support
- Switched to rtc_add_group() for sysfs attributes
- Removed failure paths after RTC device registration as per subsystem guidelines.
V1 -> V2:
- Watchdog related changes due to removal of vendor specific properties
  from device tree
  * remove vendor DT knobs (enable/timeout/stepsize/repeat)
  * use watchdog_init_timeout (with 10s default)
  * derive clock_sel from final timeout
  * default, repeat=true (repeat mode)
- Fixed uninitalised warning on 'ret' (reported by kernel test robot)
- Use dev_dbg instead of dev_info for debug related print messages
- Minor cleanup and comments

 drivers/rtc/rtc-pcf85363.c | 168 +++++++++++++++++++++++++++++++++++--
 1 file changed, 160 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 3d733375187b..34d4c2e16774 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -5,6 +5,10 @@
  * Driver for NXP PCF85363 real-time clock.
  *
  * Copyright (C) 2017 Eric Nelson
+ *
+ * Copyright 2025 NXP
+ * Added support for timestamps, battery switch-over,
+ * watchdog, offset calibration.
  */
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -17,6 +21,8 @@
 #include <linux/device.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/watchdog.h>
 
 /*
  * Date/Time registers
@@ -127,6 +133,18 @@
 #define OFFSET_MAXIMUM  127
 #define OFFSET_MASK     0xFF
 
+#define WD_MODE_REPEAT  BIT(7)
+#define WD_TIMEOUT_MASK GENMASK(6, 2)
+#define WD_TIMEOUT_SHIFT        2
+#define WD_CLKSEL_MASK  GENMASK(1, 0)
+#define WD_CLKSEL_0_25HZ        0x00
+#define WD_CLKSEL_1HZ   0x01
+#define WD_CLKSEL_4HZ   0x02
+#define WD_CLKSEL_16HZ  0x03
+
+#define WD_TIMEOUT_MIN  1
+#define WD_TIMEOUT_MAX  0x1F
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
@@ -138,6 +156,15 @@ struct pcf85x63_config {
 	unsigned int num_nvram;
 };
 
+struct pcf85363_watchdog {
+	struct watchdog_device wdd;
+	struct regmap *regmap;
+	struct device *dev;
+	u8 timeout_val;
+	u8 clock_sel;
+	bool repeat;
+};
+
 static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
 {
 	u32 load = 7000;
@@ -323,12 +350,13 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		return IRQ_NONE;
 
 	if (flags) {
-		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
+		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s\n",
 			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
 			(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
 			(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
 			(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
-			(flags & FLAGS_BSF) ? " [BSF]" : "");
+			(flags & FLAGS_BSF) ? " [BSF]" : "",
+			(flags & FLAGS_WDF) ? " [WDF]" : "");
 	}
 
 	if (flags & FLAGS_A1F) {
@@ -360,6 +388,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
+	if (flags & FLAGS_WDF) {
+		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+		handled = true;
+	}
+
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
@@ -503,6 +536,123 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/*
+ * This function sets the watchdog control register based on the timeout,
+ * clock selection and repeat mode settings. It prepares the value to
+ * write into the watchdog control register (CTRL_WDOG).
+ */
+static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
+{
+	u8 val;
+
+	val = (wd->repeat ? WD_MODE_REPEAT : 0) |
+	       ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
+	       (wd->clock_sel & WD_CLKSEL_MASK);
+
+	return regmap_write(wd->regmap, CTRL_WDOG, val);
+}
+
+static int pcf85363_wdt_start(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_stop(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	return regmap_write(wd->regmap, CTRL_WDOG, 0);
+}
+
+static int pcf85363_wdt_ping(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
+				    unsigned int timeout)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+
+	wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
+	wdd->timeout = wd->timeout_val;
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static const struct watchdog_info pcf85363_wdt_info = {
+	.identity = "PCF85363 Watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops pcf85363_wdt_ops = {
+	.owner = THIS_MODULE,
+	.start = pcf85363_wdt_start,
+	.stop = pcf85363_wdt_stop,
+	.ping = pcf85363_wdt_ping,
+	.set_timeout = pcf85363_wdt_set_timeout,
+};
+
+static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
+{
+	struct pcf85363_watchdog *wd;
+	unsigned int timeout_sec;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_WATCHDOG))
+		return 0;
+
+	wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
+	if (!wd)
+		return -ENOMEM;
+
+	wd->regmap = regmap;
+	wd->dev = dev;
+
+	wd->wdd.info = &pcf85363_wdt_info;
+	wd->wdd.ops = &pcf85363_wdt_ops;
+	wd->wdd.min_timeout = WD_TIMEOUT_MIN;
+	wd->wdd.max_timeout = WD_TIMEOUT_MAX;
+	wd->wdd.parent = dev;
+	wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+	ret = watchdog_init_timeout(&wd->wdd, 10, dev);
+	if (ret)
+		wd->wdd.timeout = clamp(10U, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
+
+	timeout_sec = wd->wdd.timeout;
+
+	if (timeout_sec <= 2)
+		wd->clock_sel = WD_CLKSEL_16HZ;
+	else if (timeout_sec <= 8)
+		wd->clock_sel = WD_CLKSEL_4HZ;
+	else if (timeout_sec <= 16)
+		wd->clock_sel = WD_CLKSEL_1HZ;
+	else
+		wd->clock_sel = WD_CLKSEL_0_25HZ;
+
+	wd->repeat = true;
+
+	ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
+	if (ret) {
+		dev_err(dev, "failed to clear WDF:%d\n", ret);
+		return ret;
+	}
+
+	watchdog_set_drvdata(&wd->wdd, wd);
+
+	dev_dbg(dev, "pcf85363 watchdog registered (timeout=%us, clk_sel=%u)\n",
+		timeout_sec, wd->clock_sel);
+
+	return devm_watchdog_register_device(dev, &wd->wdd);
+}
+
 /*
  * Reads 6 bytes of timestamp data starting at the given base register,
  * converts them from BCD to binary, and formats the result into a
@@ -684,20 +834,22 @@ static int pcf85363_probe(struct i2c_client *client)
 			   PIN_IO_TSPM | PIN_IO_TSIM,
 			   PIN_IO_TSPM | PIN_IO_TSIM);
 
+	ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
+	if (ret)
+		dev_err_probe(dev, ret, "Watchdog init failed\n");
+
 	if (irq_a > 0 || wakeup_source)
 		device_init_wakeup(dev, true);
 
 	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
 
-	ret = devm_rtc_register_device(pcf85363->rtc);
-
+	ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
 	if (ret)
-		return dev_err_probe(dev, ret, "RTC registration failed\n");
-
-	ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
+		return ret;
 
+	ret = devm_rtc_register_device(pcf85363->rtc);
 	if (ret)
-		return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
+		return dev_err_probe(dev, ret, "RTC registration failed\n");
 
 	for (i = 0; i < config->num_nvram; i++) {
 		nvmem_cfg[i].priv = pcf85363;
-- 
2.25.1

Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config

[Krzysztof Kozlowski]

On 19/11/2025 09:33, Lakshay Piplani wrote:
> NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
> which latch the current time when a selected event occurs. Add a
> vendor specific property, nxp,timestamp-mode, to select the event
> source for each register.
> 
> 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>
> ---
> V2 -> V3:
> - No changes in v3
> - Added Reviewed-by: Rob Herring <robh@kernel.org>

I don't see it. Please start using b4, so such trivialities won't affect
the process.

Best regards,
Krzysztof

RE: [EXT] Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config

[Lakshay Piplani]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, November 19, 2025 3:53 PM
> To: Lakshay Piplani <lakshay.piplani@nxp.com>;
> alexandre.belloni@bootlin.com; linux-rtc@vger.kernel.org; linux-
> kernel@vger.kernel.org; robh@kernel.org; krzk+dt@kernel.org;
> conor+dt@kernel.org; devicetree@vger.kernel.org; wim@linux-watchdog.org;
> linux@roeck-us.net; linux-watchdog@vger.kernel.org
> Cc: Vikash Bansal <vikash.bansal@nxp.com>; Priyanka Jain
> <priyanka.jain@nxp.com>; Shashank Rebbapragada
> <shashank.rebbapragada@nxp.com>
> Subject: [EXT] Re: [PATCH v3 1/5] dt-bindings: rtc: nxp,pcf85363: add
> timestamp mode config
> 
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
> 
> 
> On 19/11/2025 09:33, Lakshay Piplani wrote:
> > NXP PCF85263/PCF85363 provides three timestamp registers (TSR1-TSR3)
> > which latch the current time when a selected event occurs. Add a
> > vendor specific property, nxp,timestamp-mode, to select the event
> > source for each register.
> >
> > 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>
> > ---
> > V2 -> V3:
> > - No changes in v3
> > - Added Reviewed-by: Rob Herring <robh@kernel.org>
> 
> I don't see it. Please start using b4, so such trivialities won't affect the process.
> 
> Best regards,
> Krzysztof

Hi Krzysztof,

Thanks for pointing that out.

I mistakenly mentioned the Reviewed-by tag in the changelog but forgot to include it in the actual commit. I’ll make sure to add it in the v4 DT patch and start using b4 for preparing future revisions to avoid such issues.

Best regards,
Lakshay

Re: [PATCH v3 5/5] rtc: pcf85363: add watchdog support with configurable step size

[Guenter Roeck]

On 11/19/25 00:33, Lakshay Piplani wrote:
> Add watchdog timer support to PCF85263/PCF85363 using the linux watchdog
> subsystem. The driver programs the hardware watchdog timeout based on
> the requested period.
> 
> Also use rtc_add_group() instead of sysfs_create_group() to register
> timestamp attributes under the RTC class device (/sys/class/rtc/rtcX).
> 
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
> ---
> V2 -> V3:
> - Split into separate patches as suggested:
>    - Battery switch-over detection.
>    - Timestamp recording for TS pin and battery switch-over events.
>    - Offset calibration.
>    - Watchdog timer (to be reviewed by watchdog maintainers).
> - Dropped Alarm2 support
> - Switched to rtc_add_group() for sysfs attributes
> - Removed failure paths after RTC device registration as per subsystem guidelines.
> V1 -> V2:
> - Watchdog related changes due to removal of vendor specific properties
>    from device tree
>    * remove vendor DT knobs (enable/timeout/stepsize/repeat)
>    * use watchdog_init_timeout (with 10s default)
>    * derive clock_sel from final timeout
>    * default, repeat=true (repeat mode)
> - Fixed uninitalised warning on 'ret' (reported by kernel test robot)
> - Use dev_dbg instead of dev_info for debug related print messages
> - Minor cleanup and comments
> 
>   drivers/rtc/rtc-pcf85363.c | 168 +++++++++++++++++++++++++++++++++++--
>   1 file changed, 160 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 3d733375187b..34d4c2e16774 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
> @@ -5,6 +5,10 @@
>    * Driver for NXP PCF85363 real-time clock.
>    *
>    * Copyright (C) 2017 Eric Nelson
> + *
> + * Copyright 2025 NXP
> + * Added support for timestamps, battery switch-over,
> + * watchdog, offset calibration.
>    */
>   #include <linux/module.h>
>   #include <linux/i2c.h>
> @@ -17,6 +21,8 @@
>   #include <linux/device.h>
>   #include <linux/of.h>
>   #include <linux/regmap.h>
> +#include <linux/rtc.h>
> +#include <linux/watchdog.h>
>   
>   /*
>    * Date/Time registers
> @@ -127,6 +133,18 @@
>   #define OFFSET_MAXIMUM  127
>   #define OFFSET_MASK     0xFF
>   
> +#define WD_MODE_REPEAT  BIT(7)
> +#define WD_TIMEOUT_MASK GENMASK(6, 2)
> +#define WD_TIMEOUT_SHIFT        2
> +#define WD_CLKSEL_MASK  GENMASK(1, 0)
> +#define WD_CLKSEL_0_25HZ        0x00
> +#define WD_CLKSEL_1HZ   0x01
> +#define WD_CLKSEL_4HZ   0x02
> +#define WD_CLKSEL_16HZ  0x03
> +
> +#define WD_TIMEOUT_MIN  1
> +#define WD_TIMEOUT_MAX  0x1F
> +
>   struct pcf85363 {
>   	struct rtc_device	*rtc;
>   	struct regmap		*regmap;
> @@ -138,6 +156,15 @@ struct pcf85x63_config {
>   	unsigned int num_nvram;
>   };
>   
> +struct pcf85363_watchdog {
> +	struct watchdog_device wdd;
> +	struct regmap *regmap;
> +	struct device *dev;
> +	u8 timeout_val;
> +	u8 clock_sel;
> +	bool repeat;
> +};
> +
>   static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
>   {
>   	u32 load = 7000;
> @@ -323,12 +350,13 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
>   		return IRQ_NONE;
>   
>   	if (flags) {
> -		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s\n",
> +		dev_dbg(&pcf85363->rtc->dev, "IRQ flags: 0x%02x%s%s%s%s%s%s\n",
>   			flags, (flags & FLAGS_A1F) ? " [A1F]" : "",
>   			(flags & FLAGS_TSR1F) ? " [TSR1F]" : "",
>   			(flags & FLAGS_TSR2F) ? " [TSR2F]" : "",
>   			(flags & FLAGS_TSR3F) ? " [TSR3F]" : "",
> -			(flags & FLAGS_BSF) ? " [BSF]" : "");
> +			(flags & FLAGS_BSF) ? " [BSF]" : "",
> +			(flags & FLAGS_WDF) ? " [WDF]" : "");
>   	}
>   
>   	if (flags & FLAGS_A1F) {
> @@ -360,6 +388,11 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
>   		handled = true;
>   	}
>   
> +	if (flags & FLAGS_WDF) {
> +		regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> +		handled = true;
> +	}
> +
>   	return handled ? IRQ_HANDLED : IRQ_NONE;
>   }
>   
> @@ -503,6 +536,123 @@ static const struct pcf85x63_config pcf_85363_config = {
>   	.num_nvram = 2
>   };
>   
> +/*
> + * This function sets the watchdog control register based on the timeout,
> + * clock selection and repeat mode settings. It prepares the value to
> + * write into the watchdog control register (CTRL_WDOG).
> + */
> +static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
> +{
> +	u8 val;
> +
> +	val = (wd->repeat ? WD_MODE_REPEAT : 0) |
> +	       ((wd->timeout_val & WD_TIMEOUT_MAX) << WD_TIMEOUT_SHIFT) |
> +	       (wd->clock_sel & WD_CLKSEL_MASK);
> +
> +	return regmap_write(wd->regmap, CTRL_WDOG, val);
> +}
> +
> +static int pcf85363_wdt_start(struct watchdog_device *wdd)
> +{
> +	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> +	return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_stop(struct watchdog_device *wdd)
> +{
> +	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> +	return regmap_write(wd->regmap, CTRL_WDOG, 0);
> +}
> +
> +static int pcf85363_wdt_ping(struct watchdog_device *wdd)
> +{
> +	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> +	regmap_update_bits(wd->regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> +
> +	return pcf85363_wdt_reload(wd);
> +}
> +
> +static int pcf85363_wdt_set_timeout(struct watchdog_device *wdd,
> +				    unsigned int timeout)
> +{
> +	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
> +
> +	wd->timeout_val = clamp(timeout, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);
> +	wdd->timeout = wd->timeout_val;
> +
> +	return pcf85363_wdt_reload(wd);
> +}
> +
> +static const struct watchdog_info pcf85363_wdt_info = {
> +	.identity = "PCF85363 Watchdog",
> +	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
> +};
> +
> +static const struct watchdog_ops pcf85363_wdt_ops = {
> +	.owner = THIS_MODULE,
> +	.start = pcf85363_wdt_start,
> +	.stop = pcf85363_wdt_stop,
> +	.ping = pcf85363_wdt_ping,
> +	.set_timeout = pcf85363_wdt_set_timeout,
> +};
> +
> +static int pcf85363_watchdog_init(struct device *dev, struct regmap *regmap)
> +{
> +	struct pcf85363_watchdog *wd;
> +	unsigned int timeout_sec;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_WATCHDOG))
> +		return 0;
> +
> +	wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
> +	if (!wd)
> +		return -ENOMEM;
> +
> +	wd->regmap = regmap;
> +	wd->dev = dev;
> +
> +	wd->wdd.info = &pcf85363_wdt_info;
> +	wd->wdd.ops = &pcf85363_wdt_ops;
> +	wd->wdd.min_timeout = WD_TIMEOUT_MIN;
> +	wd->wdd.max_timeout = WD_TIMEOUT_MAX;
> +	wd->wdd.parent = dev;
> +	wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
> +
> +	ret = watchdog_init_timeout(&wd->wdd, 10, dev);

Calling watchdog_init_timeout() with a value other than 0 means that
a parameter from devicetree won't be accepted. Calling it with a fixed
value is usually pointless unless the value is out of the valid range,
which by itself would be pointless.

watchdog_init_timeout() is normally called to pass and validate a module
parameter or to pick a timeout from devicetree. Calling it with a constant
value other than 0 is unnecessary.

> +	if (ret)
> +		wd->wdd.timeout = clamp(10U, WD_TIMEOUT_MIN, WD_TIMEOUT_MAX);

So if 10 seconds is invalid, 10 is clamped to [1, 31] and applied directly.
That is an odd and complicated way of setting the timeout to 10 seconds.

If you don't want a timeout value from devicetree to be accepted, just make this

	wd->wdd.timeout = 10;

and do not call watchdog_init_timeout() in the first place.

> +
> +	timeout_sec = wd->wdd.timeout;
> +
> +	if (timeout_sec <= 2)
> +		wd->clock_sel = WD_CLKSEL_16HZ;
> +	else if (timeout_sec <= 8)
> +		wd->clock_sel = WD_CLKSEL_4HZ;
> +	else if (timeout_sec <= 16)
> +		wd->clock_sel = WD_CLKSEL_1HZ;
> +	else
> +		wd->clock_sel = WD_CLKSEL_0_25HZ;
> +

This seems an odd location for this code. What if the timeout changes
later on to one of the other values ?

Also, the timeout is set to a fixed value of 10. That means the above
can be simplified to
	wd->clock_sel = WD_CLKSEL_1HZ;
... and that in turn means that the variable is pointless, and that
WD_CLKSEL_1HZ could be used as constant instead.

Why all that complexity ? Am I missing something ? I am quite concerned that
I may be missing trees in the forest, meaning that the real problems are hiding
behind the noise.

Guenter

> +	wd->repeat = true;

What is the purpose of this variable ? It is always set to true.
You might as well drop it.

> +
> +	ret = regmap_update_bits(regmap, CTRL_FLAGS, FLAGS_WDF, 0);
> +	if (ret) {
> +		dev_err(dev, "failed to clear WDF:%d\n", ret);
> +		return ret;
> +	}
> +
> +	watchdog_set_drvdata(&wd->wdd, wd);
> +
> +	dev_dbg(dev, "pcf85363 watchdog registered (timeout=%us, clk_sel=%u)\n",
> +		timeout_sec, wd->clock_sel);
> +
> +	return devm_watchdog_register_device(dev, &wd->wdd);
> +}
> +
>   /*
>    * Reads 6 bytes of timestamp data starting at the given base register,
>    * converts them from BCD to binary, and formats the result into a
> @@ -684,20 +834,22 @@ static int pcf85363_probe(struct i2c_client *client)
>   			   PIN_IO_TSPM | PIN_IO_TSIM,
>   			   PIN_IO_TSPM | PIN_IO_TSIM);
>   
> +	ret = pcf85363_watchdog_init(dev, pcf85363->regmap);
> +	if (ret)
> +		dev_err_probe(dev, ret, "Watchdog init failed\n");
> +
>   	if (irq_a > 0 || wakeup_source)
>   		device_init_wakeup(dev, true);
>   
>   	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
>   
> -	ret = devm_rtc_register_device(pcf85363->rtc);
> -
> +	ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
>   	if (ret)
> -		return dev_err_probe(dev, ret, "RTC registration failed\n");
> -
> -	ret = sysfs_create_group(&pcf85363->rtc->dev.kobj, &pcf85363_attr_group);
> +		return ret;
>   
> +	ret = devm_rtc_register_device(pcf85363->rtc);
>   	if (ret)
> -		return dev_err_probe(dev, ret, "Timestamp sysfs creation failed\n");
> +		return dev_err_probe(dev, ret, "RTC registration failed\n");
>   
It is not entirely obvious how those changes are related to adding watchdog support
to this driver.

>   	for (i = 0; i < config->num_nvram; i++) {
>   		nvmem_cfg[i].priv = pcf85363;