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

[PATCH v6 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>
Reviewed-by: Rob Herring <robh@kernel.org>

---
V5 -> V6:
- No change

V4 -> V5:
- No change

V3 -> V4:
- Added Reviewed-by tag in commit message (previously only in changelog) 

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 */

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
-- 
2.25.1

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

[Lakshay Piplani]

Report battery switch-over on PCF85263/PCF85363 through the standard
RTC_VL_* ioctls: RTC_VL_READ exposes the BSF flag and RTC_VL_CLR clears
it. The flag is cached before it is cleared so it survives re-arming, and
it is collected from both the IRQ handler and RTC_VL_READ so poll-only
setups still see it.

CTRL_FLAGS is write-0-to-clear, so a read-modify-write can drop an
unrelated flag. Add pcf85363_clear_flags(), which writes the complement
of the mask to clear only the requested bits.

Probe no longer blanket-clears CTRL_FLAGS, so pre-existing events are
preserved. As the battery-backed registers can hold stale flags across a
power cycle and storm the level-triggered INTA line, probe masks off only
the interrupt sources and Alarm2 enables this driver never services and
clears their flags, and the IRQ handler clears any unserviced A2F/PIF
flags.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

---
V5 -> V6:
- Mask INT_WDIE alongside the other unused INTA sources at probe so a
  battery-backed watchdog-enable left by the bootloader cannot storm the
  level-triggered INTA line before watchdog support is added later in
  the series

V4 -> V5:
- Add pcf85363_clear_flags() for the write-0-to-clear CTRL_FLAGS register
  and use it everywhere, so a read-modify-write no longer drops flags.
- Cache BSF before clearing it and collect it from both the IRQ handler
  and RTC_VL_READ so poll-only setups still observe it.
- Latch pending events in probe instead of blanket-clearing CTRL_FLAGS,
  and mask/clear only the unused INTA sources to avoid an interrupt storm.
- Propagate register-access errors out of pcf85363_collect_events(); the
  IRQ handler logs them but still acknowledges the interrupt.
- Serialise the cached event state with a mutex.

V3 -> V4:
- No changes in v4.

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

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 | 181 ++++++++++++++++++++++++++++++++++---
 1 file changed, 168 insertions(+), 13 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index ccc7834e5759..c8c04312bc58 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -14,6 +14,8 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/bcd.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
 
@@ -113,6 +115,9 @@
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
+	/* Serialises access to the cached event state below. */
+	struct mutex		lock;
+	bool			bsf;
 };
 
 struct pcf85x63_config {
@@ -120,6 +125,15 @@ struct pcf85x63_config {
 	unsigned int num_nvram;
 };
 
+/*
+ * CTRL_FLAGS is write-0-to-clear, so write the complement of the mask to
+ * clear only the requested bits without disturbing the others.
+ */
+static int pcf85363_clear_flags(struct pcf85363 *pcf85363, u8 mask)
+{
+	return regmap_write(pcf85363->regmap, CTRL_FLAGS, (u8)~mask);
+}
+
 static int pcf85363_load_capacitance(struct pcf85363 *pcf85363, struct device_node *node)
 {
 	u32 load = 7000;
@@ -253,7 +267,7 @@ static int _pcf85363_rtc_alarm_irq_enable(struct pcf85363 *pcf85363, unsigned
 		return ret;
 
 	/* clear current flags */
-	return regmap_update_bits(pcf85363->regmap, CTRL_FLAGS, FLAGS_A1F, 0);
+	return pcf85363_clear_flags(pcf85363, FLAGS_A1F);
 }
 
 static int pcf85363_rtc_alarm_irq_enable(struct device *dev,
@@ -292,9 +306,40 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return _pcf85363_rtc_alarm_irq_enable(pcf85363, alrm->enabled);
 }
 
+/*
+ * Latch a pending battery-switch event into the software cache. Runs from
+ * the IRQ handler and the RTC_VL_READ path, so it is seen with or without
+ * an interrupt line. BSF is cached before it is cleared so it survives
+ * re-arming. Caller must hold pcf85363->lock. Returns a negative errno on
+ * a register-access failure, otherwise the handled flag mask.
+ */
+static int pcf85363_collect_events(struct pcf85363 *pcf85363)
+{
+	unsigned int flags;
+	int handled = 0;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
+	if (ret)
+		return ret;
+
+	if (flags & FLAGS_BSF) {
+		pcf85363->bsf = true;
+
+		ret = pcf85363_clear_flags(pcf85363, FLAGS_BSF);
+		if (ret)
+			return ret;
+
+		handled |= FLAGS_BSF;
+	}
+
+	return handled;
+}
+
 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;
 
@@ -302,16 +347,87 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 	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;
+		pcf85363_clear_flags(pcf85363, FLAGS_A1F);
+		handled = true;
+	}
+
+	if (flags & FLAGS_BSF) {
+		guard(mutex)(&pcf85363->lock);
+
+		err = pcf85363_collect_events(pcf85363);
+		if (err < 0)
+			dev_err_ratelimited(&pcf85363->rtc->dev,
+					    "failed to collect events: %d\n",
+					    err);
+		/*
+		 * This is our interrupt source even if servicing it hit an
+		 * I/O error, so acknowledge the interrupt either way.
+		 */
+		handled = true;
+	}
+
+	/*
+	 * Clear flags this handler does not service (e.g. A2F/PIF); otherwise
+	 * they hold the level-triggered INTA line asserted and storm the IRQ.
+	 */
+	if (flags & (FLAGS_A2F | FLAGS_PIF)) {
+		pcf85363_clear_flags(pcf85363, FLAGS_A2F | FLAGS_PIF);
+		handled = true;
 	}
 
-	return IRQ_NONE;
+	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);
+	int ret;
+
+	switch (cmd) {
+	case RTC_VL_READ: {
+		u32 status = 0;
+
+		guard(mutex)(&pcf85363->lock);
+
+		/* Refresh so a poll-only setup still latches the BSF flag. */
+		ret = pcf85363_collect_events(pcf85363);
+		if (ret < 0)
+			return ret;
+
+		if (pcf85363->bsf)
+			status |= RTC_VL_BACKUP_SWITCH;
+
+		return put_user(status, (u32 __user *)arg);
+	}
+
+	case RTC_VL_CLR: {
+		guard(mutex)(&pcf85363->lock);
+
+		ret = pcf85363_clear_flags(pcf85363, FLAGS_BSF);
+		if (ret)
+			return ret;
+
+		pcf85363->bsf = false;
+
+		return 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,
@@ -412,6 +528,10 @@ static int pcf85363_probe(struct i2c_client *client)
 	if (!pcf85363)
 		return -ENOMEM;
 
+	ret = devm_mutex_init(&client->dev, &pcf85363->lock);
+	if (ret)
+		return ret;
+
 	pcf85363->regmap = devm_regmap_init_i2c(client, &config->regmap);
 	if (IS_ERR(pcf85363->regmap)) {
 		dev_err(&client->dev, "regmap allocation failed\n");
@@ -435,17 +555,52 @@ static int pcf85363_probe(struct i2c_client *client)
 
 	wakeup_source = device_property_read_bool(&client->dev,
 						  "wakeup-source");
-	if (client->irq > 0 || wakeup_source) {
-		err = regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
-		if (err)
-			return dev_err_probe(&client->dev, err,
-					     "failed to clear flags\n");
 
-		err = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+	/*
+	 * Latch and clear any battery-switch event that occurred before
+	 * probe (for example while the main supply was off) instead of
+	 * blanket clearing CTRL_FLAGS, so the cached state is preserved.
+	 */
+	scoped_guard(mutex, &pcf85363->lock) {
+		ret = pcf85363_collect_events(pcf85363);
+		if (ret < 0)
+			return dev_err_probe(&client->dev, ret,
+					     "Failed to latch boot-time events\n");
+	}
+
+	/*
+	 * Battery-backed registers can retain stale state across a power cycle.
+	 * A stale asserted flag would storm the level-triggered INTA line, so
+	 * disable only the interrupt sources and Alarm2 enables this driver
+	 * never services and clear their flags; leave the managed sources
+	 * (A1IE, BSIE) and Alarm1 for their own paths to arm. WDIE is masked
+	 * too so a bootloader-armed watchdog cannot storm INTA before the
+	 * driver is ready to service it.
+	 */
+	ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
+				 INT_WDIE | INT_A2IE | INT_OIE | INT_PIE | INT_ILP,
+				 0);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to mask unused INTA sources\n");
+
+	ret = regmap_update_bits(pcf85363->regmap, DT_ALARM_EN,
+				 ALRM_MIN_A2E | ALRM_HR_A2E | ALRM_DAY_A2E, 0);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to mask Alarm2 enables\n");
+
+	ret = pcf85363_clear_flags(pcf85363, FLAGS_A2F | FLAGS_PIF);
+	if (ret)
+		return dev_err_probe(&client->dev, ret,
+				     "Failed to clear stale flags\n");
+
+	if (client->irq > 0 || wakeup_source) {
+		ret = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
 					 PIN_IO_INTAPM, PIN_IO_INTA_OUT);
-		if (err)
-			return dev_err_probe(&client->dev, err,
-					     "failed to set interrupt pin mode\n");
+		if (ret)
+			return dev_err_probe(&client->dev, ret,
+					     "Failed to configure INTA pin\n");
 	}
 
 	if (client->irq > 0) {
-- 
2.25.1

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

[Lakshay Piplani]

Add support for the timestamp capture registers on PCF85263/PCF85363,
which latch the time on selected events (TS pin or battery switch-over).
The capture source is selected via the nxp,timestamp-mode device tree
property and latched values are exported through read-only sysfs
attributes, refreshed on read so poll-only boards still latch events.

The two capture families need different handling: first-event modes hold
the value until the flag is cleared, so it is cached before clearing;
last-event modes overwrite on every event, so the flag is cleared first
and the block re-read until two reads agree.

While at it, register the attributes with rtc_add_group(), preserve
CTRL_INTA_EN with regmap_update_bits() instead of overwriting it, only
clear RTC_FEATURE_ALARM when no interrupt is available, and make the
nvmem_config array probe-local since it is patched per-device.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

---
V5 -> V6:
- Return -EPROBE_DEFER from probe when the IRQ request is deferred
  instead of silently disabling alarms; on any other failure warn and
  keep RTC timekeeping alive with interrupt-driven alarms disabled
- Configure the INTA pin and keep RTC_FEATURE_ALARM for
  wakeup-source-only boards (INTA routed to a PMIC) so rtcwake still
  works even without our own IRQ line

V4 -> V5:
- Handle first-event (cache before clearing) and last-event (clear, then
  re-read until stable, -EAGAIN on a torn read) capture modes separately,
  and propagate register-access errors, surfaced on the sysfs read path.
- Refresh the cache on sysfs read so poll-only boards still latch events.
- Treat a failed DT_TS_MODE write as fatal, and drive the TS pin only for
  the TS-pin modes: set TSPM and leave TSIM at its reset default.
- Preserve CTRL_INTA_EN via regmap_update_bits(); enable INT_TSRIE only
  when a timestamp mode is configured, and check the INTA writes.
- Clear RTC_FEATURE_ALARM only when no interrupt is available.
- Make the nvmem_config array probe-local, mask the reserved seconds bit,
  and don't hardcode IRQF_TRIGGER_LOW for a firmware-node interrupt.

V3 -> V4:
- No changes in v4.

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

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 | 396 +++++++++++++++++++++++++++++++------
 1 file changed, 341 insertions(+), 55 deletions(-)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index c8c04312bc58..5cc854d617f3 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-2026 NXP
+ * Added support for timestamps, battery switch-over,
+ * watchdog, offset calibration.
  */
 #include <linux/module.h>
 #include <linux/i2c.h>
@@ -18,6 +22,9 @@
 #include <linux/mutex.h>
 #include <linux/of.h>
 #include <linux/regmap.h>
+#include <linux/string.h>
+
+#include <dt-bindings/rtc/pcf85363-tsr.h>
 
 /*
  * Date/Time registers
@@ -102,22 +109,50 @@
 #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
+
+#define PCF85363_NUM_TS		3
+/* Bytes latched per timestamp register (sec, min, hour, day, mon, year). */
+#define PCF85363_TS_LEN		6
+/* Bit 7 of the seconds byte is reserved, not time data; mask it off. */
+#define PCF85363_SEC_MASK	0x7F
+#define PCF85363_TS_READ_RETRIES	3
+
+/* Cached timestamp; the flag is cleared once the value is copied here. */
+struct pcf85363_ts {
+	bool valid;
+	u8 regs[PCF85363_TS_LEN];
+};
+
 struct pcf85363 {
 	struct rtc_device	*rtc;
 	struct regmap		*regmap;
 	/* Serialises access to the cached event state below. */
 	struct mutex		lock;
 	bool			bsf;
+	struct pcf85363_ts	ts[PCF85363_NUM_TS];
+	/* Per-TSR: true when the register uses a last-event capture mode. */
+	bool			ts_last_event[PCF85363_NUM_TS];
 };
 
 struct pcf85x63_config {
@@ -306,23 +341,114 @@ static int pcf85363_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return _pcf85363_rtc_alarm_irq_enable(pcf85363, alrm->enabled);
 }
 
+static const u8 pcf85363_ts_base[PCF85363_NUM_TS] = {
+	DT_TIMESTAMP1, DT_TIMESTAMP2, DT_TIMESTAMP3,
+};
+
+static const u8 pcf85363_ts_flag[PCF85363_NUM_TS] = {
+	FLAGS_TSR1F, FLAGS_TSR2F, FLAGS_TSR3F,
+};
+
+/* Mark which TSRs use a last-event mode; those need torn-read handling. */
+static void pcf85363_classify_ts_modes(struct pcf85363 *pcf85363, u8 tsmode)
+{
+	u8 m1 = (tsmode >> TSR1_SHIFT) & TSR1_MASK;
+	u8 m2 = (tsmode >> TSR2_SHIFT) & TSR2_MASK;
+	u8 m3 = (tsmode >> TSR3_SHIFT) & TSR3_MASK;
+
+	pcf85363->ts_last_event[0] = (m1 == PCF85363_TSR1_LE);
+	pcf85363->ts_last_event[1] = (m2 == PCF85363_TSR2_LB ||
+				      m2 == PCF85363_TSR2_LV ||
+				      m2 == PCF85363_TSR2_LE);
+	pcf85363->ts_last_event[2] = (m3 == PCF85363_TSR3_LB ||
+				      m3 == PCF85363_TSR3_LV);
+}
+
 /*
- * Latch a pending battery-switch event into the software cache. Runs from
- * the IRQ handler and the RTC_VL_READ path, so it is seen with or without
- * an interrupt line. BSF is cached before it is cleared so it survives
- * re-arming. Caller must hold pcf85363->lock. Returns a negative errno on
- * a register-access failure, otherwise the handled flag mask.
+ * Last-event registers can change under us; re-read the block until two
+ * consecutive reads agree, then publish the stable value. Reject a torn
+ * read with -EAGAIN once the retry budget is exhausted.
+ */
+static int pcf85363_read_ts_stable(struct pcf85363 *pcf85363, int i)
+{
+	u8 prev[PCF85363_TS_LEN], cur[PCF85363_TS_LEN];
+	int retries, ret;
+
+	ret = regmap_bulk_read(pcf85363->regmap, pcf85363_ts_base[i],
+			       prev, PCF85363_TS_LEN);
+	if (ret)
+		return ret;
+
+	for (retries = 0; retries < PCF85363_TS_READ_RETRIES; retries++) {
+		ret = regmap_bulk_read(pcf85363->regmap, pcf85363_ts_base[i],
+				       cur, PCF85363_TS_LEN);
+		if (ret)
+			return ret;
+
+		if (!memcmp(prev, cur, PCF85363_TS_LEN)) {
+			memcpy(pcf85363->ts[i].regs, cur, PCF85363_TS_LEN);
+			return 0;
+		}
+
+		memcpy(prev, cur, PCF85363_TS_LEN);
+	}
+
+	return -EAGAIN;
+}
+
+/*
+ * Latch pending events into the cache. First-event modes hold the value
+ * until the flag is cleared, so cache before clearing; last-event modes
+ * overwrite every event, so clear first then read until stable. Runs from
+ * the IRQ and sysfs paths; caller holds the lock. Returns a negative errno
+ * on a register-access failure (including -EAGAIN for a torn last-event
+ * read), otherwise the handled flag mask.
  */
 static int pcf85363_collect_events(struct pcf85363 *pcf85363)
 {
 	unsigned int flags;
 	int handled = 0;
-	int ret;
+	int i, ret;
 
 	ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
 	if (ret)
 		return ret;
 
+	for (i = 0; i < PCF85363_NUM_TS; i++) {
+		if (!(flags & pcf85363_ts_flag[i]))
+			continue;
+
+		if (pcf85363->ts_last_event[i]) {
+			ret = pcf85363_clear_flags(pcf85363,
+						   pcf85363_ts_flag[i]);
+			if (ret)
+				return ret;
+
+			ret = pcf85363_read_ts_stable(pcf85363, i);
+			if (ret)
+				return ret;
+
+			pcf85363->ts[i].valid = true;
+			handled |= pcf85363_ts_flag[i];
+		} else {
+			ret = regmap_bulk_read(pcf85363->regmap,
+					       pcf85363_ts_base[i],
+					       pcf85363->ts[i].regs,
+					       PCF85363_TS_LEN);
+			if (ret)
+				return ret;
+
+			pcf85363->ts[i].valid = true;
+
+			ret = pcf85363_clear_flags(pcf85363,
+						   pcf85363_ts_flag[i]);
+			if (ret)
+				return ret;
+
+			handled |= pcf85363_ts_flag[i];
+		}
+	}
+
 	if (flags & FLAGS_BSF) {
 		pcf85363->bsf = true;
 
@@ -348,8 +474,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]" : "");
 	}
 
@@ -359,7 +488,7 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
-	if (flags & FLAGS_BSF) {
+	if (flags & (FLAGS_TSR1F | FLAGS_TSR2F | FLAGS_TSR3F | FLAGS_BSF)) {
 		guard(mutex)(&pcf85363->lock);
 
 		err = pcf85363_collect_events(pcf85363);
@@ -368,8 +497,8 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 					    "failed to collect events: %d\n",
 					    err);
 		/*
-		 * This is our interrupt source even if servicing it hit an
-		 * I/O error, so acknowledge the interrupt either way.
+		 * These are our interrupt sources even if servicing them hit
+		 * an I/O error, so acknowledge the interrupt either way.
 		 */
 		handled = true;
 	}
@@ -495,12 +624,88 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/* Six BCD bytes; bit 7 of the seconds byte is reserved, not time data. */
+static ssize_t pcf85363_format_timestamp(const u8 *regs, char *buf)
+{
+	struct rtc_time tm;
+
+	tm.tm_sec = bcd2bin(regs[0] & PCF85363_SEC_MASK);
+	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);
+}
+
+/* Refresh from hardware first so poll-only setups still latch events. */
+static ssize_t pcf85363_timestamp_show(struct device *dev, char *buf,
+				       unsigned int index)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	int ret;
+
+	guard(mutex)(&pcf85363->lock);
+
+	ret = pcf85363_collect_events(pcf85363);
+	if (ret < 0)
+		return ret;
+
+	if (!pcf85363->ts[index].valid)
+		return sysfs_emit(buf, "00-00-00 00:00:00\n");
+
+	return pcf85363_format_timestamp(pcf85363->ts[index].regs, buf);
+}
+
+static ssize_t timestamp1_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return pcf85363_timestamp_show(dev, buf, 0);
+}
+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, 1);
+}
+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, 2);
+}
+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);
-	static struct nvmem_config nvmem_cfg[] = {
+	struct device *dev = &client->dev;
+	struct pcf85363 *pcf85363;
+	int irq_a = client->irq;
+	bool ts_mode_configured = false;
+	bool wakeup_source;
+	int ret, i, err;
+	u32 tsr_mode[3];
+	u8 val;
+
+	struct nvmem_config nvmem_cfg[] = {
 		{
 			.name = "pcf85x63-",
 			.word_size = 1,
@@ -517,29 +722,72 @@ 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;
 
-	ret = devm_mutex_init(&client->dev, &pcf85363->lock);
+	ret = devm_mutex_init(dev, &pcf85363->lock);
 	if (ret)
 		return ret;
 
 	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)) {
+		bool ts_pin_used;
+
+		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)
+			return dev_err_probe(dev, ret,
+					     "Failed to write timestamp mode register\n");
+
+		pcf85363_classify_ts_modes(pcf85363, val);
+
+		ts_mode_configured = tsr_mode[0] || tsr_mode[1] || tsr_mode[2];
+
+		/*
+		 * Only the TS-pin capture modes drive the TS pin. Select the
+		 * timestamp function (TSPM) for those and leave the input mode
+		 * (TSIM) at its reset default rather than forcing the
+		 * mechanical-switch detector.
+		 */
+		ts_pin_used = tsr_mode[0] == PCF85363_TSR1_FE ||
+			      tsr_mode[0] == PCF85363_TSR1_LE ||
+			      tsr_mode[1] == PCF85363_TSR2_FE ||
+			      tsr_mode[1] == PCF85363_TSR2_LE;
+
+		if (ts_pin_used) {
+			ret = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+						 PIN_IO_TSPM, PIN_IO_TSPM);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to configure TS pin\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);
@@ -553,80 +801,118 @@ 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");
+	wakeup_source = device_property_read_bool(dev, "wakeup-source");
 
 	/*
-	 * Latch and clear any battery-switch event that occurred before
-	 * probe (for example while the main supply was off) instead of
-	 * blanket clearing CTRL_FLAGS, so the cached state is preserved.
+	 * Latch pre-probe events instead of blanket-clearing CTRL_FLAGS, so
+	 * pre-existing timestamps and the battery-switch flag are not lost.
 	 */
 	scoped_guard(mutex, &pcf85363->lock) {
 		ret = pcf85363_collect_events(pcf85363);
 		if (ret < 0)
-			return dev_err_probe(&client->dev, ret,
+			return dev_err_probe(dev, ret,
 					     "Failed to latch boot-time events\n");
 	}
 
 	/*
 	 * Battery-backed registers can retain stale state across a power cycle.
 	 * A stale asserted flag would storm the level-triggered INTA line, so
-	 * disable only the interrupt sources and Alarm2 enables this driver
-	 * never services and clear their flags; leave the managed sources
-	 * (A1IE, BSIE) and Alarm1 for their own paths to arm. WDIE is masked
-	 * too so a bootloader-armed watchdog cannot storm INTA before the
-	 * driver is ready to service it.
+	 * disable the interrupt sources and Alarm2 enables this driver does not
+	 * arm here and clear their flags; leave the managed sources (A1IE, BSIE,
+	 * TSRIE) and Alarm1 for their own paths to arm. WDIE is masked too so a
+	 * bootloader-armed watchdog cannot storm INTA before the driver is ready
+	 * to service it.
 	 */
 	ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
 				 INT_WDIE | INT_A2IE | INT_OIE | INT_PIE | INT_ILP,
 				 0);
 	if (ret)
-		return dev_err_probe(&client->dev, ret,
-				     "Failed to mask unused INTA sources\n");
+		return dev_err_probe(dev, ret, "Failed to mask unused INTA sources\n");
 
 	ret = regmap_update_bits(pcf85363->regmap, DT_ALARM_EN,
 				 ALRM_MIN_A2E | ALRM_HR_A2E | ALRM_DAY_A2E, 0);
 	if (ret)
-		return dev_err_probe(&client->dev, ret,
-				     "Failed to mask Alarm2 enables\n");
+		return dev_err_probe(dev, ret, "Failed to mask Alarm2 enables\n");
 
 	ret = pcf85363_clear_flags(pcf85363, FLAGS_A2F | FLAGS_PIF);
 	if (ret)
-		return dev_err_probe(&client->dev, ret,
-				     "Failed to clear stale flags\n");
-
-	if (client->irq > 0 || wakeup_source) {
-		ret = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
-					 PIN_IO_INTAPM, PIN_IO_INTA_OUT);
-		if (ret)
-			return dev_err_probe(&client->dev, ret,
-					     "Failed to configure INTA pin\n");
-	}
+		return dev_err_probe(dev, ret, "Failed to clear stale flags\n");
 
-	if (client->irq > 0) {
+	if (irq_a > 0) {
 		unsigned long irqflags = IRQF_TRIGGER_LOW;
 
 		if (dev_fwnode(&client->dev))
 			irqflags = 0;
-		ret = devm_request_threaded_irq(&client->dev, client->irq,
-						NULL, pcf85363_rtc_handle_irq,
+
+		ret = devm_request_threaded_irq(dev, irq_a, NULL,
+						pcf85363_rtc_handle_irq,
 						irqflags | IRQF_ONESHOT,
-						"pcf85363", client);
+						"pcf85363-inta", client);
+		/*
+		 * Let the driver core retry when the interrupt provider is not
+		 * ready yet; only a genuine failure disables interrupt-driven
+		 * alarms while leaving RTC timekeeping (and any wakeup-source
+		 * path) intact.
+		 */
+		if (ret == -EPROBE_DEFER)
+			return ret;
 		if (ret) {
-			dev_warn(&client->dev,
-				 "unable to request IRQ, alarms disabled\n");
-			client->irq = 0;
+			dev_warn(dev, "unable to request IRQ, alarms disabled: %d\n",
+				 ret);
+			irq_a = 0;
 		}
 	}
 
-	if (client->irq > 0 || wakeup_source) {
-		device_init_wakeup(&client->dev, true);
+	if (irq_a > 0 || wakeup_source) {
+		/*
+		 * The alarm can be delivered either through our own IRQ line or
+		 * via an external wakeup path (INTA routed to a PMIC), so route
+		 * INTA to its interrupt output and keep the alarm feature in
+		 * both cases; rtcwake relies on it for wakeup-source-only boards.
+		 */
+		ret = regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
+					 PIN_IO_INTAPM, PIN_IO_INTA_OUT);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Failed to configure INTA pin\n");
+
+		if (irq_a > 0) {
+			u8 inta_en = INT_BSIE;
+
+			/*
+			 * Enable the timestamp interrupt only when a capture
+			 * mode is configured; always enable battery-switch.
+			 * Only relevant when we service the IRQ ourselves.
+			 */
+			if (ts_mode_configured)
+				inta_en |= INT_TSRIE;
+
+			ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
+						 INT_BSIE | INT_TSRIE, inta_en);
+			if (ret)
+				return dev_err_probe(dev, ret,
+						     "Failed to enable INTA sources\n");
+		}
+
+		device_init_wakeup(dev, true);
 		set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
 	} else {
+		/*
+		 * Neither an interrupt line nor a wakeup source: the alarm
+		 * cannot be delivered, so drop the alarm feature.
+		 */
 		clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
 	}
 
+	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
+
+	ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
+	if (ret)
+		return ret;
+
 	ret = devm_rtc_register_device(pcf85363->rtc);
+	if (ret)
+		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

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

[Lakshay Piplani]

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

The offset is in parts per billion (ppb). CTRL_OFFSET is a signed 8-bit
step count whose size depends on the OFFM bit: 2170 ppb/step normal and
2034.5 ppb/step fast, the latter computed with an x10-scaled constant to
keep the arithmetic integer. Values are range-checked before that scaling
so the multiply cannot overflow, and out-of-range values are rejected
with -ERANGE rather than clamped.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

---
V5 -> V6:
- No change

V4 -> V5:
- Report and accept the offset in ppb (per the RTC offset ABI) using the
  step selected by OFFM (2170 ppb, or 2034.5 ppb fast via x10 scaling).
- Range-check before the x10 scaling to avoid overflow, and reject
  out-of-range values with -ERANGE instead of clamping.

V3 -> V4:
- No changes in v4.

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

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 | 67 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 5cc854d617f3..22661abd9ebb 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -115,6 +115,7 @@
 #define OSC_CAP_SEL	GENMASK(1, 0)
 #define OSC_CAP_6000	0x01
 #define OSC_CAP_12500	0x02
+#define OSC_OFFM	BIT(6)
 
 #define STOP_EN_STOP	BIT(0)
 
@@ -131,6 +132,14 @@
 #define TSR2_SHIFT      2
 #define TSR3_SHIFT      6
 
+#define OFFSET_SIGN_BIT 7
+#define OFFSET_MASK     0xFF
+/* Offset step in ppb; fast mode (OFFM=1) is scaled x10 for integer maths. */
+#define OFFSET_STEP_PPB		2170
+#define OFFSET_STEP_PPB_FAST_X10	20345
+#define OFFSET_PPB_MAX		(127 * OFFSET_STEP_PPB)
+#define OFFSET_PPB_MIN		(-128 * OFFSET_STEP_PPB)
+
 #define PCF85363_NUM_TS		3
 /* Bytes latched per timestamp register (sec, min, hour, day, mon, year). */
 #define PCF85363_TS_LEN		6
@@ -515,6 +524,62 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
+/*
+ * CTRL_OFFSET is a signed step count; the step is 2170 ppb (normal) or
+ * 2034.5 ppb (fast/OFFM, scaled x10 to keep the arithmetic integer).
+ */
+static int pcf85363_read_offset(struct device *dev, long *offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int val, osc;
+	long steps;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OFFSET, &val);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OSCILLATOR, &osc);
+	if (ret)
+		return ret;
+
+	steps = sign_extend32(val, OFFSET_SIGN_BIT);
+
+	if (osc & OSC_OFFM)
+		*offset = steps * OFFSET_STEP_PPB_FAST_X10 / 10;
+	else
+		*offset = steps * OFFSET_STEP_PPB;
+
+	return 0;
+}
+
+static int pcf85363_set_offset(struct device *dev, long offset)
+{
+	struct pcf85363 *pcf85363 = dev_get_drvdata(dev);
+	unsigned int osc;
+	long steps;
+	int ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_OSCILLATOR, &osc);
+	if (ret)
+		return ret;
+
+	/* Range-check before the x10 scaling so the multiply cannot overflow. */
+	if (offset > OFFSET_PPB_MAX || offset < OFFSET_PPB_MIN)
+		return -ERANGE;
+
+	if (osc & OSC_OFFM)
+		steps = DIV_ROUND_CLOSEST(offset * 10, OFFSET_STEP_PPB_FAST_X10);
+	else
+		steps = DIV_ROUND_CLOSEST(offset, OFFSET_STEP_PPB);
+
+	if (steps < -128 || steps > 127)
+		return -ERANGE;
+
+	return regmap_write(pcf85363->regmap, CTRL_OFFSET,
+			    steps & OFFSET_MASK);
+}
+
 static int pcf85363_rtc_ioctl(struct device *dev,
 			      unsigned int cmd, unsigned long arg)
 {
@@ -562,6 +627,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 v6 5/5] rtc: pcf85363: add watchdog support with configurable step size

[Lakshay Piplani]

Add watchdog support to PCF85263/PCF85363 using the watchdog subsystem.
The 5-bit count selects the timeout with a clock step: timeouts up to
30 seconds use the 1 Hz step, longer ones the 0.25 Hz step (max 120 s),
reported back through the watchdog device. One count of margin is added
because the first period after a reload lasts between WDR and WDR-1
counts, so the timeout is never shorter than requested. Select
WATCHDOG_CORE if WATCHDOG so the device can register.

The chip has no reset output; expiry is only signalled via the WDF flag
routed to INTA, so this is an alarm-only watchdog (WDIOF_ALARMONLY). WDIE
is toggled on start/stop with rollback-safe register ordering, expiry is
rate-limited from the IRQ handler, and the watchdog is only registered
when an interrupt line is present. WDF is cleared through the
write-0-to-clear helper.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

---
V5 -> V6:
- Adopt an already-running watchdog (bootloader- or battery-backed):
  read CTRL_WDOG until two reads agree, set WDOG_HW_RUNNING and re-arm
  WDIE when it is running, otherwise leave WDIE masked. Do not decode
  the live countdown as the configured timeout; keep the fixed default
  and let the core's first keep-alive reload it
- Report watchdog probe failures via dev_err_probe()

V4 -> V5:
- Select WATCHDOG_CORE if WATCHDOG so the device can register.
- Scale the timeout by the clock select with one count of margin (1 Hz
  step up to 30 s, else 0.25 Hz, max 120 s) and report it back.
- Alarm-only watchdog (WDIOF_ALARMONLY): expiry is only signalled via WDF
  routed to INTA; toggle WDIE on start/stop with rollback-safe ordering.
- Don't program WDR from .set_timeout when inactive (it would start the
  watchdog); only reload when watchdog_active(), and drop the clamp.
- Register the watchdog only when an interrupt line is present.
- Use a fixed default timeout (no timeout-sec); userspace can change it.
- Give the watchdog struct a back-pointer to struct pcf85363 and clear
  WDF via the shared write-0-to-clear helper.

V3 -> V4:
- Use watchdog_init_timeout(&wd->wdd, 0, dev) to allow devicetree or module parameter overrides; 
  fallback to WD_DEFAULT_TIMEOUT if not provided.
- Centralized clock selection logic in pcf85363_wdt_select_clock() and applied dynamically 
  whenever timeout changes.
- Removed unused repeat variable and simplified timeout handling for clarity.

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

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/Kconfig        |   1 +
 drivers/rtc/rtc-pcf85363.c | 259 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 258 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 05b9233b9418..4871cb3fb24d 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -516,6 +516,7 @@ config RTC_DRV_PCF8523
 config RTC_DRV_PCF85363
 	tristate "NXP PCF85363"
 	select REGMAP_I2C
+	select WATCHDOG_CORE if WATCHDOG
 	help
 	  If you say yes here you get support for the PCF85363 RTC chip.
 
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index 22661abd9ebb..aa45d71e6ab5 100644
--- a/drivers/rtc/rtc-pcf85363.c
+++ b/drivers/rtc/rtc-pcf85363.c
@@ -23,6 +23,7 @@
 #include <linux/of.h>
 #include <linux/regmap.h>
 #include <linux/string.h>
+#include <linux/watchdog.h>
 
 #include <dt-bindings/rtc/pcf85363-tsr.h>
 
@@ -147,6 +148,24 @@
 #define PCF85363_SEC_MASK	0x7F
 #define PCF85363_TS_READ_RETRIES	3
 
+#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_MODE_REPEAT	BIT(7)
+
+#define WD_DEFAULT_TIMEOUT  10
+#define WD_TIMEOUT_MIN	1
+#define WD_COUNT_MAX	0x1F
+/* Longest guaranteed timeout at the 0.25 Hz step (WDR=31, one count margin). */
+#define WD_TIMEOUT_MAX	120
+/* Longest timeout served by the 1 Hz step (WDR=31, one count margin). */
+#define WD_TIMEOUT_1HZ_MAX	30
+/* CTRL_WDOG reads back the live countdown; retry until two reads agree. */
+#define WD_READ_RETRIES		3
+
 /* Cached timestamp; the flag is cleared once the value is copied here. */
 struct pcf85363_ts {
 	bool valid;
@@ -169,6 +188,13 @@ struct pcf85x63_config {
 	unsigned int num_nvram;
 };
 
+struct pcf85363_watchdog {
+	struct watchdog_device wdd;
+	struct pcf85363 *pcf85363;
+	u8 timeout_val;
+	u8 clock_sel;
+};
+
 /*
  * CTRL_FLAGS is write-0-to-clear, so write the complement of the mask to
  * clear only the requested bits without disturbing the others.
@@ -483,12 +509,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) {
@@ -512,6 +539,15 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
 		handled = true;
 	}
 
+	if (flags & FLAGS_WDF) {
+		dev_warn_ratelimited(&pcf85363->rtc->dev,
+				     "watchdog timer expired\n");
+		if (pcf85363_clear_flags(pcf85363, FLAGS_WDF))
+			dev_err(&pcf85363->rtc->dev,
+				"failed to clear watchdog flag\n");
+		handled = true;
+	}
+
 	/*
 	 * Clear flags this handler does not service (e.g. A2F/PIF); otherwise
 	 * they hold the level-triggered INTA line asserted and storm the IRQ.
@@ -691,6 +727,215 @@ static const struct pcf85x63_config pcf_85363_config = {
 	.num_nvram = 2
 };
 
+/*
+ * Program the watchdog counter (WDR) so the reported timeout is never
+ * shorter than requested: the first period after a reload lasts between
+ * WDR and WDR-1 counts, so add one count of margin. Timeouts up to 30 s
+ * use the 1 Hz step (1 s/count); longer ones the 0.25 Hz step (4 s/count).
+ */
+static void pcf85363_wdt_select_clock(struct pcf85363_watchdog *wd)
+{
+	unsigned int timeout = wd->wdd.timeout;
+
+	if (timeout <= WD_TIMEOUT_1HZ_MAX) {
+		wd->clock_sel = WD_CLKSEL_1HZ;
+		wd->timeout_val = timeout + 1;
+		wd->wdd.timeout = timeout;
+	} else {
+		wd->clock_sel = WD_CLKSEL_0_25HZ;
+		wd->timeout_val = DIV_ROUND_UP(timeout, 4) + 1;
+		wd->wdd.timeout = DIV_ROUND_UP(timeout, 4) * 4;
+	}
+}
+
+/* Repeat mode restarts the watchdog automatically after each period. */
+static int pcf85363_wdt_reload(struct pcf85363_watchdog *wd)
+{
+	u8 val;
+
+	val = WD_MODE_REPEAT |
+	      ((wd->timeout_val & WD_COUNT_MAX) << WD_TIMEOUT_SHIFT) |
+	      (wd->clock_sel & WD_CLKSEL_MASK);
+
+	return regmap_write(wd->pcf85363->regmap, CTRL_WDOG, val);
+}
+
+static int pcf85363_wdt_start(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+	int ret;
+
+	/* Route WDF to INTA; alarm-only, the chip has no reset output. */
+	ret = regmap_update_bits(wd->pcf85363->regmap, CTRL_INTA_EN,
+				 INT_WDIE, INT_WDIE);
+	if (ret)
+		return ret;
+
+	ret = pcf85363_wdt_reload(wd);
+	if (ret)
+		regmap_update_bits(wd->pcf85363->regmap, CTRL_INTA_EN,
+				   INT_WDIE, 0);
+
+	return ret;
+}
+
+static int pcf85363_wdt_stop(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+	int ret;
+
+	/* Halt the counter first so a failed disable cannot leave it armed. */
+	ret = regmap_write(wd->pcf85363->regmap, CTRL_WDOG, 0);
+	if (ret)
+		return ret;
+
+	return regmap_update_bits(wd->pcf85363->regmap, CTRL_INTA_EN,
+				  INT_WDIE, 0);
+}
+
+static int pcf85363_wdt_ping(struct watchdog_device *wdd)
+{
+	struct pcf85363_watchdog *wd = watchdog_get_drvdata(wdd);
+	int ret;
+
+	ret = pcf85363_clear_flags(wd->pcf85363, FLAGS_WDF);
+	if (ret)
+		return ret;
+
+	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);
+
+	wdd->timeout = timeout;
+
+	pcf85363_wdt_select_clock(wd);
+
+	/*
+	 * Programming the counter with a non-zero value starts it, so only
+	 * reprogram when the watchdog is already running; the core keeps the
+	 * new timeout for the next start otherwise.
+	 */
+	if (!watchdog_active(wdd))
+		return 0;
+
+	return pcf85363_wdt_reload(wd);
+}
+
+static const struct watchdog_info pcf85363_wdt_info = {
+	.identity = "PCF85363 Watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT | WDIOF_ALARMONLY,
+};
+
+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,
+};
+
+/*
+ * CTRL_WDOG reads back the live countdown, which decrements while the
+ * watchdog runs, so a single read can catch a transient value. Read until
+ * two consecutive reads agree before deciding whether the watchdog is armed.
+ */
+static int pcf85363_read_wdog_stable(struct pcf85363 *pcf85363, unsigned int *out)
+{
+	unsigned int prev, cur;
+	int retries, ret;
+
+	ret = regmap_read(pcf85363->regmap, CTRL_WDOG, &prev);
+	if (ret)
+		return ret;
+
+	for (retries = 0; retries < WD_READ_RETRIES; retries++) {
+		ret = regmap_read(pcf85363->regmap, CTRL_WDOG, &cur);
+		if (ret)
+			return ret;
+
+		if (cur == prev) {
+			*out = cur;
+			return 0;
+		}
+
+		prev = cur;
+	}
+
+	*out = cur;
+	return 0;
+}
+
+static int pcf85363_watchdog_init(struct device *dev, struct pcf85363 *pcf85363)
+{
+	struct pcf85363_watchdog *wd;
+	unsigned int regval;
+	bool running;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_WATCHDOG))
+		return 0;
+
+	wd = devm_kzalloc(dev, sizeof(*wd), GFP_KERNEL);
+	if (!wd)
+		return -ENOMEM;
+
+	wd->pcf85363 = pcf85363;
+
+	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.timeout = WD_DEFAULT_TIMEOUT;
+	wd->wdd.parent = dev;
+	wd->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+	/*
+	 * Fixed default timeout; userspace can change it via WDIOC_SETTIMEOUT.
+	 * CTRL_WDOG reads back the live countdown, not the programmed reload
+	 * value, so it must not be decoded into wdd.timeout: adopt the known
+	 * default and let the watchdog core's first keep-alive reload it.
+	 */
+	pcf85363_wdt_select_clock(wd);
+
+	/*
+	 * The counter and WDIE are battery-backed and survive a reboot, so a
+	 * bootloader may leave the watchdog armed. Detect that and hand it to
+	 * the core as already-running rather than silently disabling it; only
+	 * a stopped watchdog gets WDIE masked so a stale enable cannot storm
+	 * the level-triggered INTA line.
+	 */
+	ret = pcf85363_read_wdog_stable(pcf85363, &regval);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to read watchdog register\n");
+
+	running = ((regval >> WD_TIMEOUT_SHIFT) & WD_COUNT_MAX) != 0;
+
+	ret = pcf85363_clear_flags(pcf85363, FLAGS_WDF);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to clear WDF\n");
+
+	/*
+	 * Once WDOG_HW_RUNNING is set the core keeps the watchdog alive via
+	 * .ping(), which does not touch WDIE, so arm WDIE here for an adopted
+	 * watchdog; otherwise keep it masked.
+	 */
+	ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN, INT_WDIE,
+				 running ? INT_WDIE : 0);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to configure WDIE\n");
+
+	if (running)
+		set_bit(WDOG_HW_RUNNING, &wd->wdd.status);
+
+	watchdog_set_drvdata(&wd->wdd, wd);
+
+	return devm_watchdog_register_device(dev, &wd->wdd);
+}
+
 /* Six BCD bytes; bit 7 of the seconds byte is reserved, not time data. */
 static ssize_t pcf85363_format_timestamp(const u8 *regs, char *buf)
 {
@@ -971,6 +1216,16 @@ static int pcf85363_probe(struct i2c_client *client)
 		clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
 	}
 
+	/*
+	 * Watchdog expiry is only signalled via INTA (no reset output), so it
+	 * needs an interrupt line to be usable.
+	 */
+	if (irq_a > 0) {
+		ret = pcf85363_watchdog_init(dev, pcf85363);
+		if (ret)
+			return dev_err_probe(dev, ret, "Watchdog init failed\n");
+	}
+
 	dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
 
 	ret = rtc_add_group(pcf85363->rtc, &pcf85363_attr_group);
-- 
2.25.1

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

[sashiko-bot]

> 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>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092126.3757418-1-lakshay.piplani@nxp.com?part=1

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

[sashiko-bot]

> Expose the oscillator offset register of PCF85263/PCF85363 through the
> read_offset and set_offset rtc_class_ops callbacks, so userspace can
> apply frequency correction for drift compensation.
> 
> The offset is in parts per billion (ppb). CTRL_OFFSET is a signed 8-bit
> step count whose size depends on the OFFM bit: 2170 ppb/step normal and
> 2034.5 ppb/step fast, the latter computed with an x10-scaled constant to
> keep the arithmetic integer. Values are range-checked before that scaling
> so the multiply cannot overflow, and out-of-range values are rejected
> with -ERANGE rather than clamped.
> 
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092126.3757418-1-lakshay.piplani@nxp.com?part=4

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

[sashiko-bot]

> Report battery switch-over on PCF85263/PCF85363 through the standard
> RTC_VL_* ioctls: RTC_VL_READ exposes the BSF flag and RTC_VL_CLR clears
> it. The flag is cached before it is cleared so it survives re-arming, and
> it is collected from both the IRQ handler and RTC_VL_READ so poll-only
> setups still see it.
> 
> CTRL_FLAGS is write-0-to-clear, so a read-modify-write can drop an
> unrelated flag. Add pcf85363_clear_flags(), which writes the complement
> of the mask to clear only the requested bits.
> 
> Probe no longer blanket-clears CTRL_FLAGS, so pre-existing events are
> preserved. As the battery-backed registers can hold stale flags across a
> power cycle and storm the level-triggered INTA line, probe masks off only
> the interrupt sources and Alarm2 enables this driver never services and
> clears their flags, and the IRQ handler clears any unserviced A2F/PIF
> flags.
> 
> Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092126.3757418-1-lakshay.piplani@nxp.com?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] An I2C read error during event collection causes an early return, bypassing hardware flag clearance. Combined with returning IRQ_HANDLED, this leads to an infinite interrupt storm.
--

commit 71137d386eeb017385125a0418e12cdc1cafbbf2
Author: Lakshay Piplani <lakshay.piplani@nxp.com>

rtc: pcf85363: add timestamp support with configurable timestamp mode

Add support for the timestamp capture registers on PCF85263/PCF85363,
which latch the time on selected events (TS pin or battery switch-over).
The capture source is selected via the nxp,timestamp-mode device tree
property and latched values are exported through read-only sysfs
attributes, refreshed on read so poll-only boards still latch events.

> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index c8c04312bc583..5cc854d617f3a 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c

[ ... ]

> +static int pcf85363_collect_events(struct pcf85363 *pcf85363)
>  {
>  	unsigned int flags;
>  	int handled = 0;
> -	int ret;
> +	int i, ret;
>  
>  	ret = regmap_read(pcf85363->regmap, CTRL_FLAGS, &flags);
>  	if (ret)
>  		return ret;
>  
> +	for (i = 0; i < PCF85363_NUM_TS; i++) {
> +		if (!(flags & pcf85363_ts_flag[i]))
> +			continue;
> +
> +		if (pcf85363->ts_last_event[i]) {
> +			ret = pcf85363_clear_flags(pcf85363,
> +						   pcf85363_ts_flag[i]);

[ ... ]

> +		} else {
> +			ret = regmap_bulk_read(pcf85363->regmap,
> +					       pcf85363_ts_base[i],
> +					       pcf85363->ts[i].regs,
> +					       PCF85363_TS_LEN);
> +			if (ret)
> +				return ret;

[Severity: High]
If this bulk read fails, the function returns early, bypassing the call to
pcf85363_clear_flags() below and leaving the interrupt asserted in the
hardware. Is it possible for this early return to create a persistent
interrupt condition?

> +
> +			pcf85363->ts[i].valid = true;
> +
> +			ret = pcf85363_clear_flags(pcf85363,
> +						   pcf85363_ts_flag[i]);
> +			if (ret)
> +				return ret;
> +
> +			handled |= pcf85363_ts_flag[i];
> +		}
> +	}
> +

[ ... ]

> @@ -359,7 +488,7 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
>  		handled = true;
>  	}
>  
> -	if (flags & FLAGS_BSF) {
> +	if (flags & (FLAGS_TSR1F | FLAGS_TSR2F | FLAGS_TSR3F | FLAGS_BSF)) {
>  		guard(mutex)(&pcf85363->lock);
>  
>  		err = pcf85363_collect_events(pcf85363);
> @@ -368,8 +497,8 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
>  					    "failed to collect events: %d\n",
>  					    err);
>  		/*
> -		 * This is our interrupt source even if servicing it hit an
> -		 * I/O error, so acknowledge the interrupt either way.
> +		 * These are our interrupt sources even if servicing them hit
> +		 * an I/O error, so acknowledge the interrupt either way.
>  		 */
>  		handled = true;
>  	}

[Severity: High]
By unconditionally setting handled = true even after an I/O error, the
handler returns IRQ_HANDLED while the hardware interrupt line might still be
asserted (since pcf85363_collect_events() bails out early on a read error
without clearing the flags).

For a level-triggered interrupt, doesn't returning IRQ_HANDLED without clearing
the source circumvent the kernel's spurious IRQ detector (which relies on
IRQ_NONE to mask stuck interrupts)?

Could this result in an infinite interrupt storm that will peg a CPU core?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092126.3757418-1-lakshay.piplani@nxp.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The watchdog is left running on driver unbind without an IRQ handler, leading to an unhandled interrupt storm.
- [High] The watchdog timer is left running during system suspend, which will cause spurious wakeups.
- [Medium] Un-ratelimited dev_err() in the threaded IRQ handler can cause a log storm and consume 100% of a CPU core if clearing the watchdog flag fails.
--

commit 5bad6cdbd177d2e1690c8afe67f641dfa821a364
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
Subject: rtc: pcf85363: add watchdog support with configurable step size

This commit adds watchdog support to PCF85263/PCF85363 using the watchdog 
subsystem. The chip has no reset output, so expiry is only signalled via 
the WDF flag routed to INTA as an alarm-only watchdog.

> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 22661abd9ebb5..aa45d71e6ab58 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
[ ... ]
> @@ -512,6 +539,15 @@ static irqreturn_t pcf85363_rtc_handle_irq(int irq, void *dev_id)
>  		handled = true;
>  	}
>  
> +	if (flags & FLAGS_WDF) {
> +		dev_warn_ratelimited(&pcf85363->rtc->dev,
> +				     "watchdog timer expired\n");
> +		if (pcf85363_clear_flags(pcf85363, FLAGS_WDF))
> +			dev_err(&pcf85363->rtc->dev,
> +				"failed to clear watchdog flag\n");
> +		handled = true;
> +	}
> +

[Severity: Medium]
If the I2C write inside pcf85363_clear_flags() fails (e.g., due to a 
transient bus error), this path prints a non-ratelimited error and 
returns IRQ_HANDLED.

Because the physical level-triggered INTA line would remain asserted in 
hardware, won't the IRQ immediately re-fire and loop infinitely? Could 
this result in a CPU lockup and a log storm from the un-ratelimited 
dev_err()? 

Should the error log be rate-limited, or the interrupt disabled if it 
cannot be successfully cleared?

[ ... ]
> +static int pcf85363_watchdog_init(struct device *dev, struct pcf85363 *pcf85363)
> +{
> +	struct pcf85363_watchdog *wd;
[ ... ]
> +	watchdog_set_drvdata(&wd->wdd, wd);
> +
> +	return devm_watchdog_register_device(dev, &wd->wdd);
> +}

[Severity: High]
If a user unbinds the driver via sysfs while the watchdog is running, 
will the devm-managed IRQ handler be freed while the hardware watchdog 
continues to count down? 

When the hardware timer subsequently expires and asserts the INTA line, 
could this cause an unhandled interrupt storm that disables the shared 
IRQ line? 

Should WDOG_STOP_ON_UNREGISTER be set on the watchdog device, or a 
custom devm cleanup action be registered to halt the hardware timer 
on unbind?

[Severity: High]
Does the watchdog timer continue running during system suspend? 

Since INTA is configured as a system wakeup source earlier in the driver, 
if the watchdog is active when the system goes to sleep, won't it 
eventually expire and cause an unexpected, spurious system wakeup? 

Should dev_pm_ops be provided to halt the hardware timer during suspend 
and resume it upon waking up?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914092126.3757418-1-lakshay.piplani@nxp.com?part=5