* [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
@ 2026-09-10 5:07 Lakshay Piplani
2026-09-10 5:07 ` [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL Lakshay Piplani
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Lakshay Piplani @ 2026-09-10 5:07 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux-watchdog, linux
Cc: vikash.bansal, priyanka.jain, 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>
---
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 */
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
@ 2026-09-10 5:07 ` Lakshay Piplani
2026-09-10 5:17 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode Lakshay Piplani
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-09-10 5:07 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux-watchdog, linux
Cc: vikash.bansal, priyanka.jain, 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>
---
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 | 178 ++++++++++++++++++++++++++++++++++---
1 file changed, 165 insertions(+), 13 deletions(-)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index ccc7834e5759..ffa957878fc9 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,49 @@ 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.
+ */
+ ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
+ 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
2026-09-10 5:07 ` [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL Lakshay Piplani
@ 2026-09-10 5:07 ` Lakshay Piplani
2026-09-10 5:19 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support Lakshay Piplani
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-09-10 5:07 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux-watchdog, linux
Cc: vikash.bansal, priyanka.jain, 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>
---
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 | 370 +++++++++++++++++++++++++++++++------
1 file changed, 317 insertions(+), 53 deletions(-)
diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
index ffa957878fc9..38bff8954e7f 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,18 +801,16 @@ 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");
}
@@ -573,57 +819,75 @@ static int pcf85363_probe(struct i2c_client *client)
* 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.
+ * (A1IE, WDIE, BSIE, TSRIE) and Alarm1 for their own paths to arm.
*/
ret = regmap_update_bits(pcf85363->regmap, CTRL_INTA_EN,
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");
+ return dev_err_probe(dev, ret, "Failed to clear stale flags\n");
+
+ if (irq_a > 0) {
+ unsigned long irqflags = IRQF_TRIGGER_LOW;
+
+ if (dev_fwnode(&client->dev))
+ irqflags = 0;
- 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,
+ return dev_err_probe(dev, ret,
"Failed to configure INTA pin\n");
- }
- if (client->irq > 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);
+
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;
+ clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
+ } else {
+ u8 inta_en = INT_BSIE;
+
+ /*
+ * Enable the timestamp interrupt only when a capture
+ * mode is configured; always enable battery-switch.
+ */
+ 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");
}
- }
-
- if (client->irq > 0 || wakeup_source) {
- device_init_wakeup(&client->dev, true);
- set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
} else {
+ /* Without an interrupt line the alarm cannot be delivered. */
clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
}
+ if (irq_a > 0 || wakeup_source)
+ device_init_wakeup(dev, true);
+
+ 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
2026-09-10 5:07 ` [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL Lakshay Piplani
2026-09-10 5:07 ` [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode Lakshay Piplani
@ 2026-09-10 5:07 ` Lakshay Piplani
2026-09-10 5:14 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size Lakshay Piplani
2026-09-10 5:13 ` [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config sashiko-bot
4 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-09-10 5:07 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux-watchdog, linux
Cc: vikash.bansal, priyanka.jain, 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>
---
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 38bff8954e7f..9da4b1617f82 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
` (2 preceding siblings ...)
2026-09-10 5:07 ` [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support Lakshay Piplani
@ 2026-09-10 5:07 ` Lakshay Piplani
2026-09-10 5:18 ` sashiko-bot
2026-09-10 5:13 ` [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config sashiko-bot
4 siblings, 1 reply; 10+ messages in thread
From: Lakshay Piplani @ 2026-09-10 5:07 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree, wim, linux-watchdog, linux
Cc: vikash.bansal, priyanka.jain, 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>
---
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 | 196 ++++++++++++++++++++++++++++++++++++-
2 files changed, 195 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 9da4b1617f82..2046643788fd 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,22 @@
#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
+
/* Cached timestamp; the flag is cleared once the value is copied here. */
struct pcf85363_ts {
bool valid;
@@ -169,6 +186,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 +507,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 +537,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 +725,153 @@ 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,
+};
+
+static int pcf85363_watchdog_init(struct device *dev, struct pcf85363 *pcf85363)
+{
+ struct pcf85363_watchdog *wd;
+ 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. */
+ pcf85363_wdt_select_clock(wd);
+
+ ret = pcf85363_clear_flags(pcf85363, FLAGS_WDF);
+ if (ret) {
+ dev_err(dev, "failed to clear WDF:%d\n", ret);
+ return ret;
+ }
+
+ 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)
{
@@ -943,6 +1124,17 @@ static int pcf85363_probe(struct i2c_client *client)
/* Without an interrupt line the alarm cannot be delivered. */
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");
+ }
+
if (irq_a > 0 || wakeup_source)
device_init_wakeup(dev, true);
--
2.25.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
` (3 preceding siblings ...)
2026-09-10 5:07 ` [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size Lakshay Piplani
@ 2026-09-10 5:13 ` sashiko-bot
4 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-10 5:13 UTC (permalink / raw)
To: Lakshay Piplani
Cc: conor+dt, robh, linux-watchdog, linux-rtc, Alexandre Belloni,
devicetree
> 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/20260910050747.1901440-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support
2026-09-10 5:07 ` [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support Lakshay Piplani
@ 2026-09-10 5:14 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-10 5:14 UTC (permalink / raw)
To: Lakshay Piplani
Cc: linux-rtc, devicetree, linux-watchdog, Alexandre Belloni, robh,
conor+dt
> 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/20260910050747.1901440-1-lakshay.piplani@nxp.com?part=4
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL
2026-09-10 5:07 ` [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL Lakshay Piplani
@ 2026-09-10 5:17 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-10 5:17 UTC (permalink / raw)
To: Lakshay Piplani
Cc: devicetree, linux-rtc, linux-watchdog, Alexandre Belloni, robh,
conor+dt
> 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/20260910050747.1901440-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size
2026-09-10 5:07 ` [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size Lakshay Piplani
@ 2026-09-10 5:18 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-10 5:18 UTC (permalink / raw)
To: Lakshay Piplani
Cc: linux-watchdog, conor+dt, linux-rtc, devicetree,
Alexandre Belloni, robh
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Watchdog is neither stopped nor adopted on probe, leading to spurious periodic interrupts across reboots.
--
commit a09fcbd2e909232c9247652551e0a3249e36a82e
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
rtc: pcf85363: add watchdog support with configurable step size
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.
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> index 9da4b1617f82a..2046643788fdc 100644
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
[ ... ]
> @@ -691,6 +725,153 @@ static const struct pcf85x63_config pcf_85363_config = {
> .num_nvram = 2
> };
[ ... ]
> +static int pcf85363_watchdog_init(struct device *dev, struct pcf85363 *pcf85363)
> +{
> + struct pcf85363_watchdog *wd;
> + 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. */
> + pcf85363_wdt_select_clock(wd);
> +
> + ret = pcf85363_clear_flags(pcf85363, FLAGS_WDF);
> + if (ret) {
> + dev_err(dev, "failed to clear WDF:%d\n", ret);
> + return ret;
> + }
> +
> + watchdog_set_drvdata(&wd->wdd, wd);
> +
> + return devm_watchdog_register_device(dev, &wd->wdd);
[Severity: Medium]
Since the watchdog configuration registers (CTRL_WDOG and CTRL_INTA_EN) are
battery-backed and persist across system reboots, could the watchdog already
be running when the driver probes?
The driver does not clear the watchdog interrupt enable bit (WDIE) during
probe, and pcf85363_watchdog_init() registers the watchdog but neither stops
the hardware nor sets WDOG_HW_RUNNING in wdd.status.
Without WDOG_HW_RUNNING, the watchdog core will be unaware the hardware is
active and will not automatically ping it.
If the watchdog was started before a reboot or by a bootloader, will this
cause it to continuously expire and perpetually trigger the interrupt
handler, resulting in repeated "watchdog timer expired" warnings in the
logs?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260910050747.1901440-1-lakshay.piplani@nxp.com?part=5
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode
2026-09-10 5:07 ` [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode Lakshay Piplani
@ 2026-09-10 5:19 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-09-10 5:19 UTC (permalink / raw)
To: Lakshay Piplani
Cc: linux-rtc, Alexandre Belloni, devicetree, robh, conor+dt,
linux-watchdog
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Ignored `dev_err_probe()` return value defeats probe deferral for the INTA interrupt line.
- [Medium] Unconditionally clearing `RTC_FEATURE_ALARM` when no IRQ is present breaks wake-on-alarm functionality for boards that only configure a wakeup-source.
--
commit c4131e782120105d3473c8eca13fcfb2ec8f0bf1
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
rtc: pcf85363: add timestamp support with configurable timestamp mode
This commit adds support for the timestamp capture registers on the
PCF85263/PCF85363 to latch the time on selected events. The capture source
is configured via device tree and exported through read-only sysfs attributes,
along with other initialization cleanups.
> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
> --- a/drivers/rtc/rtc-pcf85363.c
> +++ b/drivers/rtc/rtc-pcf85363.c
[ ... ]
> @@ -573,57 +819,75 @@ static int pcf85363_probe(struct i2c_client *client)
[ ... ]
> + ret = devm_request_threaded_irq(dev, irq_a, NULL,
> + pcf85363_rtc_handle_irq,
> + irqflags | IRQF_ONESHOT,
> + "pcf85363-inta", client);
> +
> + if (ret) {
> + dev_err_probe(dev, ret, "INTA IRQ request failed\n");
> + irq_a = 0;
> + clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
> + } else {
[Severity: Medium]
Will ignoring the return value of dev_err_probe() in this block defeat
probe deferral?
If devm_request_threaded_irq() fails because the interrupt provider is not
yet ready, it returns -EPROBE_DEFER. By not propagating this error and
falling back to setting irq_a to 0, the driver permanently drops interrupt
support on affected boots instead of allowing the core to defer probing.
> + u8 inta_en = INT_BSIE;
> +
> + /*
> + * Enable the timestamp interrupt only when a capture
> + * mode is configured; always enable battery-switch.
> + */
> + 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");
> }
> - }
> -
> - if (client->irq > 0 || wakeup_source) {
> - device_init_wakeup(&client->dev, true);
> - set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
> } else {
> + /* Without an interrupt line the alarm cannot be delivered. */
> clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
> }
[Severity: Medium]
Does clearing RTC_FEATURE_ALARM unconditionally when there is no direct IRQ
break wake-on-alarm functionality?
If a system routes the RTC interrupt pin to an external PMIC to wake the
system and defines the device tree with a wakeup-source property but no
direct interrupts property, this logic clears the alarm feature bit. The
RTC core will reject alarm setup requests when this feature bit is cleared,
which seems like it could cause a functional regression for userspace tools
like rtcwake that rely on the wakeup-source capability.
> + if (irq_a > 0 || wakeup_source)
> + device_init_wakeup(dev, true);
> +
> + dev_set_drvdata(&pcf85363->rtc->dev, pcf85363);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260910050747.1901440-1-lakshay.piplani@nxp.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-09-10 5:19 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-10 5:07 [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config Lakshay Piplani
2026-09-10 5:07 ` [PATCH v5 2/5] rtc: pcf85363: support reporting battery switch-over via RTC_VL Lakshay Piplani
2026-09-10 5:17 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 3/5] rtc: pcf85363: add timestamp support with configurable timestamp mode Lakshay Piplani
2026-09-10 5:19 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 4/5] rtc: pcf85363: add oscillator offset calibration support Lakshay Piplani
2026-09-10 5:14 ` sashiko-bot
2026-09-10 5:07 ` [PATCH v5 5/5] rtc: pcf85363: add watchdog support with configurable step size Lakshay Piplani
2026-09-10 5:18 ` sashiko-bot
2026-09-10 5:13 ` [PATCH v5 1/5] dt-bindings: rtc: nxp,pcf85363: add timestamp mode config sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox