[PATCH 0/4] rtc: pcf8525: Add NXP PCF8525 RTC driver

[PATCH 0/4] rtc: pcf8525: Add NXP PCF8525 RTC driver

[Shiv Prakash Gupta]

This series adds initial support for the NXP PCF8525 real-time clock.

The NXP PCF8525 is an I2C RTC with the following features:
- Time/date keeping (seconds to years, BCD encoded)
- Alarm with interrupt support on INTA
- Two hardware timestamps (TS1/TS2) exposed via sysfs
- Crystal aging offset correction via the RTC offset interface
- Battery backup with configurable backup switch mode (BSM)
- Optional watchdog timer on INTB
- Optional temperature sensor via hwmon

Patch 1 adds the devicetree binding document (YAML schema).
Patch 2 adds the core RTC driver with time, alarm, timestamp,
        aging offset, and backup switch mode support.
Patch 3 adds optional watchdog support via Watchdog Core on INTB.
Patch 4 adds optional temperature sensor support via hwmon.

Shiv Prakash Gupta (4):
  dt-bindings: rtc: Add NXP PCF8525 RTC
  rtc: pcf8525: Add NXP PCF8525 RTC driver
  rtc: pcf8525: Add watchdog support
  rtc: pcf8525: Add temperature sensor support via hwmon

 .../devicetree/bindings/rtc/nxp,pcf8525.yaml  |   86 ++
 drivers/rtc/Kconfig                           |   26 +
 drivers/rtc/Makefile                          |    1 +
 drivers/rtc/rtc-pcf8525.c                     | 1366 +++++++++++++++++
 4 files changed, 1479 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml
 create mode 100644 drivers/rtc/rtc-pcf8525.c

-- 
2.34.1

[PATCH 1/4] dt-bindings: rtc: Add NXP PCF8525 RTC

[Shiv Prakash Gupta]

Add a Devicetree binding for the NXP PCF8525 I2C RTC. Describe the
optional INTA interrupt used for RTC alarm and timestamp events, and
the INTB interrupt used by the watchdog.

Add properties to select the external crystal load capacitance and
temperature-compensation crystal model. Also allow the common
start-year, wakeup-source and reset-source properties, with
reset-source enabling watchdog registration

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
---
 .../devicetree/bindings/rtc/nxp,pcf8525.yaml  | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml

diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml
new file mode 100644
index 000000000000..bee71ec3da80
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/nxp,pcf8525.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PCF8525 Real Time Clock
+
+description:
+  The PCF8525 is an I2C real-time clock with alarm, timestamp, watchdog,
+  backup battery switch-over, and temperature compensation for an external
+  32.768 kHz crystal.
+
+allOf:
+  - $ref: rtc.yaml#
+
+maintainers:
+  - Lakshay Piplani <lakshay.piplani@nxp.com>
+  - Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
+
+properties:
+  compatible:
+    enum:
+      - nxp,pcf8525
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    minItems: 1
+    maxItems: 2
+    description:
+      INTA, followed by INTB when both interrupt outputs are connected.
+
+  interrupt-names:
+    minItems: 1
+    items:
+      - const: inta
+      - const: intb
+
+  quartz-load-femtofarads:
+    description:
+      Effective load capacitance of the external 32.768 kHz crystal.
+    enum: [6000, 7000]
+    default: 6000
+
+  nxp,xtal-type:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Crystal model used by the temperature compensation engine.
+      Value 1 selects -0.035 ppm/degC^2 and value 2 selects
+      -0.04 ppm/degC^2.
+    enum: [1, 2]
+    default: 1
+
+  start-year: true
+
+  wakeup-source: true
+
+  reset-source: true
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        rtc@51 {
+            compatible = "nxp,pcf8525";
+            reg = <0x51>;
+            interrupt-parent = <&gpio2>;
+            interrupts = <2 IRQ_TYPE_EDGE_FALLING>,
+                          <3 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-names = "inta", "intb";
+            quartz-load-femtofarads = <7000>;
+            nxp,xtal-type = <2>;
+            reset-source;
+        };
+    };
-- 
2.34.1

[PATCH 2/4] rtc: pcf8525: Add NXP PCF8525 RTC driver

[Shiv Prakash Gupta]

Add support for time and date, alarms, timestamp capture, backup
switch-over configuration, external crystal configuration and clock
offset correction.

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
---
 drivers/rtc/Kconfig       |  10 +
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-pcf8525.c | 933 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 944 insertions(+)
 create mode 100644 drivers/rtc/rtc-pcf8525.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 01def8231873..3c22c7253aae 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -513,6 +513,16 @@ config RTC_DRV_PCF8523
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-pcf8523.
 
+config RTC_DRV_PCF8525
+        tristate "NXP PCF8525"
+        select REGMAP_I2C
+        help
+          If you say yes here you get support for the NXP PCF8525 RTC
+          chips.
+
+          This driver can also be built as a module. If so, the module
+          will be called rtc-pcf8525.
+
 config RTC_DRV_PCF85363
 	tristate "NXP PCF85363"
 	select REGMAP_I2C
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 0347645b021f..99035c0dcf17 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -132,6 +132,7 @@ obj-$(CONFIG_RTC_DRV_PCF2123)	+= rtc-pcf2123.o
 obj-$(CONFIG_RTC_DRV_PCF2127)	+= rtc-pcf2127.o
 obj-$(CONFIG_RTC_DRV_PCF85063)	+= rtc-pcf85063.o
 obj-$(CONFIG_RTC_DRV_PCF8523)	+= rtc-pcf8523.o
+obj-$(CONFIG_RTC_DRV_PCF8525)   += rtc-pcf8525.o
 obj-$(CONFIG_RTC_DRV_PCF85363)	+= rtc-pcf85363.o
 obj-$(CONFIG_RTC_DRV_PCF8563)	+= rtc-pcf8563.o
 obj-$(CONFIG_RTC_DRV_PCF8583)	+= rtc-pcf8583.o
diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
new file mode 100644
index 000000000000..7dfb4f0f6404
--- /dev/null
+++ b/drivers/rtc/rtc-pcf8525.c
@@ -0,0 +1,933 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * I2C driver for NXP PCF8525 RTC
+ * Features:
+ * - Time/date read + set
+ * - Alarm (sec/min/hour/day) using RTC wkalrm API
+ * - Optional IRQ support (alarm + timestamp) on INTA
+ * - Timestamp1/2 exported via custom sysfs attributes: timestamp0/1
+ * - Crystal load capacitance and crystal type configuration
+ * - Crystal aging offset correction through the RTC offset interface
+ *
+ * Register map summary (key ones):
+ * 0x00 Control_1: STOP, 12_24, MI, SI, etc
+ * 0x01 Control_2: MSF, TI_TP, WDTF, AF, OSFE[1:0], AIE, SMBUS_TIMEOUT
+ * 0x02 Control_3: PWRMNG[1:0], BF, OSIE, BIE
+ * 0x03 Control_4: TSF
+ * 0x04 Control_5: TSIE, TEMP_RD_EN, CL, XTL_TYP
+ * 0x05 Reset: command register (CPR, CTS, SR)
+ * 0x06 100th seconds
+ * 0x07 seconds (OSF bit7)
+ * 0x08 minutes (VLF bit7)
+ * 0x09 hours
+ * 0x0A days
+ * 0x0B weekdays
+ * 0x0C months
+ * 0x0D years
+ * 0x0E..0x14 alarm registers (sec/min/hour/day/weekday/month/year)
+ * 0x16 timestamp control
+ * 0x17..0x1C TS1 (sec/min/hour/day/month/year)
+ * 0x1D..0x23 TS2 (subsec/sec/min/hour/day/month/year)
+ * 0x26..0x27 aging offset registers
+ * 0x28..0x2B interrupt mask regs (INTA/B)
+ * 0x2E temperature (read-only), gated by TEMP_RD_EN
+ */
+
+#include <linux/bcd.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/bitfield.h>
+#include <linux/uaccess.h>
+
+/* Registers */
+#define PCF8525_REG_CTRL1		0x00
+#define PCF8525_REG_CTRL2		0x01
+#define PCF8525_REG_CTRL3		0x02
+#define PCF8525_REG_CTRL4		0x03
+#define PCF8525_REG_CTRL5		0x04
+#define PCF8525_REG_RESET		0x05
+
+#define PCF8525_REG_100THS		0x06
+#define PCF8525_REG_SECONDS		0x07
+#define PCF8525_REG_MINUTES		0x08
+#define PCF8525_REG_HOURS		0x09
+#define PCF8525_REG_DAYS		0x0A
+#define PCF8525_REG_WEEKDAYS		0x0B
+#define PCF8525_REG_MONTHS		0x0C
+#define PCF8525_REG_YEARS		0x0D
+
+#define PCF8525_REG_ALM_SEC		0x0E
+#define PCF8525_REG_ALM_MIN		0x0F
+#define PCF8525_REG_ALM_HOUR		0x10
+#define PCF8525_REG_ALM_DAY		0x11
+#define PCF8525_REG_ALM_WDAY		0x12
+#define PCF8525_REG_ALM_MONTH		0x13
+#define PCF8525_REG_ALM_YEAR		0x14
+
+#define PCF8525_REG_CLKOUT		0x15
+
+#define PCF8525_REG_TS_CTL1		0x16
+#define PCF8525_REG_TS1_SEC		0x17
+#define PCF8525_REG_TS1_MIN		0x18
+#define PCF8525_REG_TS1_HOUR		0x19
+#define PCF8525_REG_TS1_DAY		0x1A
+#define PCF8525_REG_TS1_MONTH		0x1B
+#define PCF8525_REG_TS1_YEAR		0x1C
+
+#define PCF8525_REG_TS2_SUBSEC		0x1D
+#define PCF8525_REG_TS2_SEC		0x1E
+#define PCF8525_REG_TS2_MIN		0x1F
+#define PCF8525_REG_TS2_HOUR		0x20
+#define PCF8525_REG_TS2_DAY		0x21
+#define PCF8525_REG_TS2_MONTH		0x22
+#define PCF8525_REG_TS2_YEAR		0x23
+
+#define PCF8525_REG_INTA_MASK1		0x28
+#define PCF8525_REG_INTA_MASK2		0x29
+#define PCF8525_REG_INTB_MASK1		0x2A
+#define PCF8525_REG_INTB_MASK2		0x2B
+
+#define PCF8525_REG_TEMP		0x2E
+
+/* CTRL1 */
+#define PCF8525_CTRL1_STOP		BIT(5)
+#define PCF8525_CTRL1_12_24		BIT(2)
+#define PCF8525_CTRL1_MI		BIT(1)
+#define PCF8525_CTRL1_SI		BIT(0)
+/* CTRL2 */
+#define PCF8525_CTRL2_MSF		BIT(7)
+#define PCF8525_CTRL2_TI_TP		BIT(6)
+#define PCF8525_CTRL2_AF		BIT(4)
+#define PCF8525_CTRL2_OSFE_MASK		GENMASK(3, 2)
+#define PCF8525_CTRL2_AIE		BIT(1)
+#define PCF8525_CTRL2_SMBTO		BIT(0)
+/* CTRL3 */
+#define PCF8525_CTRL3_PWRMNG_MASK	GENMASK(6, 5)
+#define PCF8525_CTRL3_BF		BIT(3)
+#define PCF8525_CTRL3_OSIE		BIT(2)
+#define PCF8525_CTRL3_BIE		BIT(1)
+/* CTRL4/5 */
+#define PCF8525_CTRL4_TSF		BIT(7)
+#define PCF8525_CTRL5_TSIE		BIT(7)
+#define PCF8525_CTRL5_TEMP_RD_EN	BIT(2)
+#define PCF8525_CTRL5_CL		BIT(1)
+#define PCF8525_CTRL5_XTL_TYP		BIT(0)
+
+/* Timestamp control */
+#define PCF8525_TS_CTL1_TSM             BIT(7)
+#define PCF8525_TS_CTL1_TSOFF           BIT(6)
+
+/* Seconds/Minutes flags */
+#define PCF8525_SC_OSF			BIT(7)
+#define PCF8525_MN_VLF			BIT(7)
+
+/* Alarm AE bits */
+#define PCF8525_ALM_AE			BIT(7)
+
+/* RESET commands */
+#define PCF8525_RESET_CPR_CMD		0xA4 /* clear prescaler */
+#define PCF8525_RESET_CTS_CMD		0x25 /* clear timestamp */
+#define PCF8525_RESET_CPR_CTS_CMD	0xA5 /* clear prescaler + timestamp */
+#define PCF8525_RESET_SR_CMD		0x2C /* software reset */
+
+/* Interrupt masks (mask bit = 1 means masked/disabled) - INTA_MASK1 bits */
+#define PCF8525_MASK1_BIE		BIT(1)
+#define PCF8525_MASK1_AIE		BIT(2)
+#define PCF8525_MASK1_OSIE		BIT(3)
+#define PCF8525_MASK1_SI		BIT(4)
+#define PCF8525_MASK1_MI		BIT(5)
+/* INTA_MASK2 - timestamp mask bit3 */
+#define PCF8525_MASK2_TSIE		BIT(3)
+
+#define PCF8525_ALM_AE_MONTH		BIT(7)
+#define PCF8525_ALM_AE_YEAR		BIT(6)
+
+#define PCF8525_REG_AGING_OFFSET_HI	0x26
+#define PCF8525_REG_AGING_OFFSET_LO	0x27
+/*
+ * One aging-offset register step is 0.0298 ppm, or 29.8 ppb.
+ * The RTC core offset interface uses ppb.
+ */
+#define PCF8525_AGING_OFFSET_STEP_NUM	298L
+#define PCF8525_AGING_OFFSET_STEP_DEN	10L
+
+#define PCF8525_AGING_OFFSET_MIN	(-32768L)
+#define PCF8525_AGING_OFFSET_MAX	32767L
+
+struct pcf8525 {
+	struct rtc_device *rtc;
+	struct regmap *regmap;
+	bool irq_enabled;
+	time64_t ts[2];
+	bool ts_valid[2];
+};
+
+static const struct regmap_config pcf8525_regmap_cfg = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = PCF8525_REG_TEMP,
+};
+
+/*
+ * Configure only the crystal fields explicitly provided by firmware.
+ * If a property is absent, preserve the current hardware setting.
+ *
+ * quartz-load-femtofarads:
+ *   6000 -> Control_5.CL = 0
+ *   7000 -> Control_5.CL = 1
+ *
+ * nxp,xtal-type:
+ *   1 -> Control_5.XTL_TYP = 0 (-0.035 ppm/degree C^2 model)
+ *   2 -> Control_5.XTL_TYP = 1 (-0.04 ppm/degree C^2 model)
+ */
+static int pcf8525_configure_crystal(struct device *dev,
+				     struct pcf8525 *pcf8525)
+{
+	u32 load, xtal_type;
+	unsigned int mask = 0;
+	unsigned int value = 0;
+	int ret;
+
+	ret = device_property_read_u32(dev,
+				       "quartz-load-femtofarads",
+				       &load);
+	if (!ret) {
+		switch (load) {
+		case 6000:
+			mask |= PCF8525_CTRL5_CL;
+			break;
+
+		case 7000:
+			mask |= PCF8525_CTRL5_CL;
+			value |= PCF8525_CTRL5_CL;
+			break;
+
+		default:
+			dev_warn(dev,
+				 "unsupported quartz-load-femtofarads=%u; "
+				 "preserving CL bit\n",
+				 load);
+			break;
+		}
+	}
+
+	ret = device_property_read_u32(dev,
+				       "nxp,xtal-type",
+				       &xtal_type);
+	if (!ret) {
+		switch (xtal_type) {
+		case 1:
+			mask |= PCF8525_CTRL5_XTL_TYP;
+			break;
+
+		case 2:
+			mask |= PCF8525_CTRL5_XTL_TYP;
+			value |= PCF8525_CTRL5_XTL_TYP;
+			break;
+
+		default:
+			dev_warn(dev,
+				 "unsupported nxp,xtal-type=%u; "
+				 "preserving XTL_TYP bit\n",
+				 xtal_type);
+			break;
+		}
+	}
+
+	if (!mask)
+		return 0;
+
+	return regmap_update_bits(pcf8525->regmap,
+				  PCF8525_REG_CTRL5,
+				  mask, value);
+}
+
+static int pcf8525_read_aging_offset(struct pcf8525 *pcf8525, s16 *offset)
+{
+	u8 buf[2];
+	int ret;
+
+	ret = regmap_bulk_read(pcf8525->regmap,
+			       PCF8525_REG_AGING_OFFSET_HI,
+			       buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	*offset = (s16)((buf[0] << 8) | buf[1]);
+
+	return 0;
+}
+
+static int pcf8525_write_aging_offset(struct pcf8525 *pcf8525, s16 offset)
+{
+	u8 buf[2];
+
+	buf[0] = (offset >> 8) & 0xff;
+	buf[1] = offset & 0xff;
+
+	/*
+	 * Write high byte first and low byte last. The new aging correction
+	 * starts after AgingOffset_Low is written.
+	 */
+	return regmap_bulk_write(pcf8525->regmap,
+				 PCF8525_REG_AGING_OFFSET_HI,
+				 buf, sizeof(buf));
+}
+
+static int pcf8525_rtc_read_offset(struct device *dev, long *offset)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	s16 raw;
+	int ret;
+
+	ret = pcf8525_read_aging_offset(pcf8525, &raw);
+	if (ret)
+		return ret;
+
+	/*
+	 * Convert the signed 16-bit hardware value to ppb:
+	 *
+	 * one raw step = 0.0298 ppm = 29.8 ppb
+	 */
+	*offset = DIV_ROUND_CLOSEST((long)raw *
+				    PCF8525_AGING_OFFSET_STEP_NUM,
+				    PCF8525_AGING_OFFSET_STEP_DEN);
+
+	return 0;
+}
+
+static int pcf8525_rtc_set_offset(struct device *dev, long offset)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	long min_offset;
+	long max_offset;
+	long raw;
+
+	min_offset =
+		DIV_ROUND_CLOSEST(PCF8525_AGING_OFFSET_MIN *
+				  PCF8525_AGING_OFFSET_STEP_NUM,
+				  PCF8525_AGING_OFFSET_STEP_DEN);
+
+	max_offset =
+		DIV_ROUND_CLOSEST(PCF8525_AGING_OFFSET_MAX *
+				  PCF8525_AGING_OFFSET_STEP_NUM,
+				  PCF8525_AGING_OFFSET_STEP_DEN);
+
+	if (offset < min_offset || offset > max_offset)
+		return -ERANGE;
+
+	/*
+	 * Convert the requested ppb value to the closest signed
+	 * 16-bit hardware value.
+	 */
+	raw = DIV_ROUND_CLOSEST(offset *
+				PCF8525_AGING_OFFSET_STEP_DEN,
+				PCF8525_AGING_OFFSET_STEP_NUM);
+
+	if (raw < PCF8525_AGING_OFFSET_MIN ||
+	    raw > PCF8525_AGING_OFFSET_MAX)
+		return -ERANGE;
+
+	return pcf8525_write_aging_offset(pcf8525, (s16)raw);
+}
+
+static int pcf8525_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	u8 buf[7];
+	int ret;
+
+	/* Read seconds..years (0x07..0x0D) */
+	ret = regmap_bulk_read(pcf8525->regmap, PCF8525_REG_SECONDS, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	/* OSF: oscillator stop => time not reliable */
+	if (buf[0] & PCF8525_SC_OSF)
+		return -EINVAL;
+
+	/*
+	 * VLF is on minutes register bit7;
+	 * Keep policy conservative: warn but still allow read.
+	 */
+	if (buf[1] & PCF8525_MN_VLF)
+		dev_warn(dev, "VLF set: clock integrity not guaranteed\n");
+
+	tm->tm_sec =  bcd2bin(buf[0] & 0x7f);
+	tm->tm_min =  bcd2bin(buf[1] & 0x7f);
+	tm->tm_hour = bcd2bin(buf[2] & 0x3f);
+	tm->tm_mday = bcd2bin(buf[3] & 0x3f);
+	tm->tm_wday = buf[4] & 0x07;
+	tm->tm_mon =  bcd2bin(buf[5] & 0x1f) - 1;
+	tm->tm_year = bcd2bin(buf[6]) + 100; /* 20xx */
+
+	return rtc_valid_tm(tm);
+}
+
+static int pcf8525_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	u8 buf[7];
+	int ret, ret2;
+
+	ret = rtc_valid_tm(tm);
+	if (ret)
+		return ret;
+
+	/*
+	 * Use STOP + clear prescaler (CPR) like RESET register describes.
+	 */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL1,
+				 PCF8525_CTRL1_STOP, PCF8525_CTRL1_STOP);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(pcf8525->regmap, PCF8525_REG_RESET, PCF8525_RESET_CPR_CMD);
+	if (ret)
+		goto out_start;
+
+	buf[0] = bin2bcd(tm->tm_sec); /* writing seconds also clears OSF per many NXP RTCs */
+	buf[1] = bin2bcd(tm->tm_min);
+	buf[2] = bin2bcd(tm->tm_hour);
+	buf[3] = bin2bcd(tm->tm_mday);
+	buf[4] = tm->tm_wday & 0x07;
+	buf[5] = bin2bcd(tm->tm_mon + 1);
+	buf[6] = bin2bcd(tm->tm_year - 100);
+
+	ret = regmap_bulk_write(pcf8525->regmap, PCF8525_REG_SECONDS, buf, sizeof(buf));
+
+out_start:
+	/* Clear STOP regardless of bulk write outcome */
+	ret2 = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL1, PCF8525_CTRL1_STOP, 0);
+	if (ret2)
+		return ret2;
+	return ret;
+}
+
+static int pcf8525_rtc_ioctl(struct device *dev,
+			     unsigned int cmd, unsigned long arg)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int val;
+	unsigned int flags = 0;
+	int ret;
+
+	switch (cmd) {
+	case RTC_VL_READ:
+		ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL3, &val);
+		if (ret)
+			return ret;
+
+		if (val & PCF8525_CTRL3_BF)
+			flags |= RTC_VL_BACKUP_SWITCH;
+
+		return put_user(flags, (unsigned int __user *)arg);
+
+	case RTC_VL_CLR:
+		/* Clear BF by writing 0 (AND semantics respected) */
+		return regmap_update_bits(pcf8525->regmap,
+					  PCF8525_REG_CTRL3,
+					  PCF8525_CTRL3_BF,
+					  0);
+
+	default:
+		return -ENOIOCTLCMD;
+	}
+}
+
+static int pcf8525_param_get(struct device *dev, struct rtc_param *param)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int val;
+	unsigned int mode;
+	int ret;
+
+	switch (param->param) {
+	case RTC_PARAM_BACKUP_SWITCH_MODE:
+		ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL3, &val);
+		if (ret)
+			return ret;
+
+		mode = FIELD_GET(PCF8525_CTRL3_PWRMNG_MASK, val);
+
+		switch (mode) {
+		case 0:
+			param->uvalue = RTC_BSM_LEVEL;
+			break;
+		case 1:
+			param->uvalue = RTC_BSM_DIRECT;
+			break;
+		default:
+			param->uvalue = RTC_BSM_DISABLED;
+			break;
+		}
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int pcf8525_param_set(struct device *dev, struct rtc_param *param)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int mode;
+
+	switch (param->param) {
+	case RTC_PARAM_BACKUP_SWITCH_MODE:
+		switch (param->uvalue) {
+		case RTC_BSM_LEVEL:
+			mode = 0;
+			break;
+		case RTC_BSM_DIRECT:
+			mode = 1;
+			break;
+		case RTC_BSM_DISABLED:
+			mode = 2;
+			break;
+		default:
+			return -EINVAL;
+		}
+
+	return regmap_update_bits(pcf8525->regmap,
+				  PCF8525_REG_CTRL3,
+				  PCF8525_CTRL3_PWRMNG_MASK,
+				  FIELD_PREP(PCF8525_CTRL3_PWRMNG_MASK, mode));
+	default:
+		return -EINVAL;
+	}
+}
+
+/* Alarm uses RTC wkalrm: map to sec/min/hour/day and disable others */
+static int pcf8525_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int ctrl2;
+	u8 buf[7];
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL2, &ctrl2);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(pcf8525->regmap, PCF8525_REG_ALM_SEC, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	alrm->enabled = !!(ctrl2 & PCF8525_CTRL2_AIE);
+	alrm->pending = !!(ctrl2 & PCF8525_CTRL2_AF);
+
+	alrm->time.tm_sec = bcd2bin(buf[0] & 0x7f);
+	alrm->time.tm_min = bcd2bin(buf[1] & 0x7f);
+	alrm->time.tm_hour = bcd2bin(buf[2] & 0x3f);
+	alrm->time.tm_mday = bcd2bin(buf[3] & 0x3f);
+
+	return 0;
+}
+
+static int pcf8525_alarm_irq_enable(struct device *dev, unsigned int enable)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	int ret;
+
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2,
+				 PCF8525_CTRL2_AIE,
+				 enable ? PCF8525_CTRL2_AIE : 0);
+	if (ret)
+		dev_err(dev, "alarm_irq_enable: failed ret=%d\n", ret);
+
+	return ret;
+}
+
+static int pcf8525_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	u8 buf[7];
+	int ret;
+
+	/* Clear AF first */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2,
+				 PCF8525_CTRL2_AF, 0);
+	if (ret)
+		return ret;
+
+	buf[0] = bin2bcd(alrm->time.tm_sec) & 0x7f;
+	buf[1] = bin2bcd(alrm->time.tm_min) & 0x7f;
+	buf[2] = bin2bcd(alrm->time.tm_hour) & 0x3f;
+	buf[3] = bin2bcd(alrm->time.tm_mday) & 0x3f;
+
+	/* Disable match on weekday/month/year by setting AE bits */
+	buf[4] = PCF8525_ALM_AE; /* weekday alarm ignored */
+	buf[5] = PCF8525_ALM_AE_MONTH | PCF8525_ALM_AE_YEAR;  /* disable month + year alarm */
+	buf[6] = 0x00; /* year alarm value (ignored if month/year disabled) */
+
+	ret = regmap_bulk_write(pcf8525->regmap, PCF8525_REG_ALM_SEC, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	return pcf8525_alarm_irq_enable(dev, alrm->enabled);
+}
+
+/* ---- Timestamp handling (sysfs) ---- */
+static int pcf8525_ts_read(struct device *dev, int id, time64_t *ts_out)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	struct rtc_time tm;
+	u8 buf[6];
+	int ret;
+	u8 base;
+
+	/*
+	 * TS0 maps to TS1 (0x17..0x1C)
+	 * TS1 maps to TS2 (0x1E..0x23) (skip subsec at 0x1D)
+	 */
+	if (id == 0)
+		base = PCF8525_REG_TS1_SEC;
+	else if (id == 1)
+		base = PCF8525_REG_TS2_SEC;
+	else
+		return -EINVAL;
+
+	ret = regmap_bulk_read(pcf8525->regmap, base, buf, sizeof(buf));
+	if (ret)
+		return ret;
+
+	tm.tm_sec = bcd2bin(buf[0] & 0x7f);
+	tm.tm_min = bcd2bin(buf[1] & 0x7f);
+	tm.tm_hour = bcd2bin(buf[2] & 0x3f);
+	tm.tm_mday = bcd2bin(buf[3] & 0x3f);
+	tm.tm_mon = bcd2bin(buf[4] & 0x1f) - 1;
+	tm.tm_year = bcd2bin(buf[5]) + 100;
+
+	ret = rtc_valid_tm(&tm);
+	if (ret)
+		return ret;
+
+	*ts_out = rtc_tm_to_time64(&tm);
+	return 0;
+}
+
+static void pcf8525_ts_snapshot(struct device *dev)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	int ret;
+
+	/*
+	 * One TSF flag covers timestamp events; snapshot both
+	 * (keeps behavior simple + userspace can pick the one it wants).
+	 */
+	if (!pcf8525->ts_valid[0]) {
+		ret = pcf8525_ts_read(dev, 0, &pcf8525->ts[0]);
+		if (!ret)
+			pcf8525->ts_valid[0] = true;
+	}
+
+	if (!pcf8525->ts_valid[1]) {
+		ret = pcf8525_ts_read(dev, 1, &pcf8525->ts[1]);
+		if (!ret)
+			pcf8525->ts_valid[1] = true;
+	}
+}
+
+static ssize_t timestamp_show_common(struct device *dev, char *buf, int id)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev->parent);
+	unsigned int ctrl4;
+	time64_t ts;
+	int ret;
+
+	if (id < 0 || id > 1)
+		return 0;
+
+	if (pcf8525->irq_enabled) {
+		if (!pcf8525->ts_valid[id])
+			return 0;
+		ts = pcf8525->ts[id];
+		return sysfs_emit(buf, "%llu\n", (unsigned long long)ts);
+	}
+
+	/* polling mode: only report if TSF is currently set */
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL4, &ctrl4);
+	if (ret)
+		return 0;
+
+	if (!(ctrl4 & PCF8525_CTRL4_TSF))
+		return 0;
+
+	ret = pcf8525_ts_read(dev->parent, id, &ts);
+	if (ret)
+		return 0;
+
+	return sysfs_emit(buf, "%llu\n", (unsigned long long)ts);
+}
+
+static ssize_t timestamp_store_common(struct device *dev, const char *buf,
+				      size_t count, int id)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev->parent);
+	int ret;
+
+	if (id < 0 || id > 1)
+		return -EINVAL;
+
+	if (pcf8525->irq_enabled) {
+		pcf8525->ts_valid[id] = false;
+		return count;
+	}
+
+	/*
+	 * Clear timestamp via RESET CTS command.
+	 * This also clears the timestamp flag path.
+	 */
+	ret = regmap_write(pcf8525->regmap, PCF8525_REG_RESET, PCF8525_RESET_CTS_CMD);
+	if (ret)
+		return ret;
+
+	/* Also clear TSF flag (belt-and-suspenders) */
+	regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL4, PCF8525_CTRL4_TSF, 0);
+
+	return count;
+}
+
+static ssize_t timestamp0_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return timestamp_show_common(dev, buf, 0);
+}
+
+static ssize_t timestamp1_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return timestamp_show_common(dev, buf, 1);
+}
+
+static ssize_t timestamp0_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	return timestamp_store_common(dev, buf, count, 0);
+}
+
+static ssize_t timestamp1_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
+{
+	return timestamp_store_common(dev, buf, count, 1);
+}
+
+static DEVICE_ATTR_RW(timestamp0);
+static DEVICE_ATTR_RW(timestamp1);
+
+static struct attribute *pcf8525_attrs[] = {
+	&dev_attr_timestamp0.attr,
+	&dev_attr_timestamp1.attr,
+	NULL
+};
+
+static const struct attribute_group pcf8525_attr_group = {
+	.attrs = pcf8525_attrs,
+};
+
+static irqreturn_t pcf8525_irq(int irq, void *data)
+{
+	struct device *dev = data;
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int ctrl2, ctrl4;
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL2, &ctrl2);
+	if (ret)
+		return IRQ_NONE;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL4, &ctrl4);
+	if (ret)
+		return IRQ_NONE;
+
+	if (!(ctrl2 & (PCF8525_CTRL2_AF | PCF8525_CTRL2_MSF)) &&
+	    !(ctrl4 & PCF8525_CTRL4_TSF))
+		return IRQ_NONE;
+
+	/* Timestamp */
+	if (ctrl4 & PCF8525_CTRL4_TSF) {
+		pcf8525_ts_snapshot(dev);
+		/* Clear TSF */
+		regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL4, PCF8525_CTRL4_TSF, 0);
+	}
+
+	/* Alarm */
+	if (ctrl2 & PCF8525_CTRL2_AF) {
+		rtc_update_irq(pcf8525->rtc, 1, RTC_IRQF | RTC_AF);
+		/* Clear AF */
+		regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2, PCF8525_CTRL2_AF, 0);
+	}
+
+	if (ctrl2 & PCF8525_CTRL2_MSF) {
+		ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2, PCF8525_CTRL2_MSF, 0);
+		if (ret)
+			return IRQ_NONE;
+	}
+
+	return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops pcf8525_rtc_ops = {
+	.ioctl = pcf8525_rtc_ioctl,
+	.read_time = pcf8525_read_time,
+	.set_time = pcf8525_set_time,
+	.read_alarm = pcf8525_read_alarm,
+	.set_alarm = pcf8525_set_alarm,
+	.alarm_irq_enable = pcf8525_alarm_irq_enable,
+	.read_offset = pcf8525_rtc_read_offset,
+	.set_offset = pcf8525_rtc_set_offset,
+	.param_get = pcf8525_param_get,
+	.param_set = pcf8525_param_set,
+};
+
+static int pcf8525_unmask_irqs_intA(struct pcf8525 *pcf8525)
+{
+	/*
+	 * Mask registers: bit=1 means masked/disabled.
+	 * We want alarm + timestamp unmasked on INTA by default:
+	 * - AIE => mask bit2 must be 0
+	 * - TSIE => mask2 bit3 must be 0
+	 */
+	unsigned int m1, m2;
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_INTA_MASK1, &m1);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_INTA_MASK2, &m2);
+	if (ret)
+		return ret;
+
+	m1 &= ~PCF8525_MASK1_AIE;    /* unmask alarm interrupt on INTA */
+	m2 &= ~PCF8525_MASK2_TSIE;   /* unmask timestamp interrupt on INTA */
+
+	ret = regmap_write(pcf8525->regmap, PCF8525_REG_INTA_MASK1, m1);
+	if (ret)
+		return ret;
+
+	return regmap_write(pcf8525->regmap, PCF8525_REG_INTA_MASK2, m2);
+}
+
+static int pcf8525_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct pcf8525 *pcf8525;
+	int ret;
+
+	pcf8525 = devm_kzalloc(dev, sizeof(*pcf8525), GFP_KERNEL);
+	if (!pcf8525)
+		return -ENOMEM;
+
+	pcf8525->regmap = devm_regmap_init_i2c(client, &pcf8525_regmap_cfg);
+	if (IS_ERR(pcf8525->regmap))
+		return dev_err_probe(dev, PTR_ERR(pcf8525->regmap),
+				     "failed to init regmap\n");
+
+	i2c_set_clientdata(client, pcf8525);
+
+	pcf8525->rtc = devm_rtc_allocate_device(dev);
+	if (IS_ERR(pcf8525->rtc))
+		return dev_err_probe(dev, PTR_ERR(pcf8525->rtc),
+				     "failed to allocate RTC device\n");
+
+	pcf8525->rtc->ops = &pcf8525_rtc_ops;
+	pcf8525->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+	pcf8525->rtc->range_max = RTC_TIMESTAMP_END_2099;
+	pcf8525->rtc->set_start_time = true;
+
+	clear_bit(RTC_FEATURE_ALARM, pcf8525->rtc->features);
+	set_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf8525->rtc->features);
+	/* Use 24-hour mode */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL1,
+				 PCF8525_CTRL1_12_24, 0);
+	if (ret)
+		return ret;
+
+	ret = pcf8525_configure_crystal(dev, pcf8525);
+	if (ret)
+		return ret;
+	/*
+	 * Enable timestamp engine + timestamp interrupt (TSIE).
+	 */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_TS_CTL1,
+				 PCF8525_TS_CTL1_TSM | PCF8525_TS_CTL1_TSOFF,
+				 PCF8525_TS_CTL1_TSM);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL5,
+				 PCF8525_CTRL5_TSIE, PCF8525_CTRL5_TSIE);
+	if (ret)
+		return ret;
+
+	/* Create sysfs timestamp0/1 */
+	ret = rtc_add_group(pcf8525->rtc, &pcf8525_attr_group);
+	if (ret)
+		return ret;
+
+	/* Optional IRQ */
+	if (client->irq > 0) {
+		ret = devm_request_threaded_irq(dev, client->irq,
+						NULL, pcf8525_irq,
+						IRQF_ONESHOT,
+						dev_name(dev), dev);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "failed to request INTA IRQ\n");
+
+		pcf8525->irq_enabled = true;
+
+		/* Route/unmask alarm + timestamp to INTA by default */
+		ret = pcf8525_unmask_irqs_intA(pcf8525);
+		if (ret)
+			return ret;
+
+		device_init_wakeup(dev, true);
+		set_bit(RTC_FEATURE_ALARM, pcf8525->rtc->features);
+	}
+
+	/* Register device */
+	ret = devm_rtc_register_device(pcf8525->rtc);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct of_device_id pcf8525_of_match[] = {
+	{ .compatible = "nxp,pcf8525" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, pcf8525_of_match);
+
+static const struct i2c_device_id pcf8525_i2c_id[] = {
+	{ "pcf8525", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, pcf8525_i2c_id);
+
+static struct i2c_driver pcf8525_driver = {
+	.driver = {
+		.name = "rtc-pcf8525",
+		.of_match_table = pcf8525_of_match,
+	},
+	.probe = pcf8525_probe,
+	.id_table = pcf8525_i2c_id,
+};
+module_i2c_driver(pcf8525_driver);
+
+MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
+MODULE_AUTHOR("Shiv Prakash Gupta <shivprakash.gupta@nxp.com>");
+MODULE_DESCRIPTION("NXP PCF8525 RTC driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

[PATCH 3/4] rtc: pcf8525: Add watchdog support

[Shiv Prakash Gupta]

Add watchdog support for NXP PCF8525 real time clock(RTC) device

Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
---
 drivers/rtc/Kconfig       |   1 +
 drivers/rtc/rtc-pcf8525.c | 245 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 244 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 3c22c7253aae..6c1c7b3d27b6 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -516,6 +516,7 @@ config RTC_DRV_PCF8523
 config RTC_DRV_PCF8525
         tristate "NXP PCF8525"
         select REGMAP_I2C
+	select WATCHDOG_CORE if WATCHDOG
         help
           If you say yes here you get support for the NXP PCF8525 RTC
           chips.
diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
index 7dfb4f0f6404..4ef648bdfc5b 100644
--- a/drivers/rtc/rtc-pcf8525.c
+++ b/drivers/rtc/rtc-pcf8525.c
@@ -45,6 +45,8 @@
 #include <linux/slab.h>
 #include <linux/bitfield.h>
 #include <linux/uaccess.h>
+#include <linux/kernel.h>
+#include <linux/watchdog.h>
 
 /* Registers */
 #define PCF8525_REG_CTRL1		0x00
@@ -94,6 +96,8 @@
 #define PCF8525_REG_INTB_MASK1		0x2A
 #define PCF8525_REG_INTB_MASK2		0x2B
 
+#define PCF8525_REG_WD_CTL              0x2C
+#define PCF8525_REG_WD_VAL              0x2D
 #define PCF8525_REG_TEMP		0x2E
 
 /* CTRL1 */
@@ -104,6 +108,7 @@
 /* CTRL2 */
 #define PCF8525_CTRL2_MSF		BIT(7)
 #define PCF8525_CTRL2_TI_TP		BIT(6)
+#define PCF8525_CTRL2_WDTF              BIT(5)
 #define PCF8525_CTRL2_AF		BIT(4)
 #define PCF8525_CTRL2_OSFE_MASK		GENMASK(3, 2)
 #define PCF8525_CTRL2_AIE		BIT(1)
@@ -120,6 +125,9 @@
 #define PCF8525_CTRL5_CL		BIT(1)
 #define PCF8525_CTRL5_XTL_TYP		BIT(0)
 
+/* CLKOUT_ctl */
+#define PCF8525_CLKOUT_CLKOE            BIT(3)
+
 /* Timestamp control */
 #define PCF8525_TS_CTL1_TSM             BIT(7)
 #define PCF8525_TS_CTL1_TSOFF           BIT(6)
@@ -138,6 +146,7 @@
 #define PCF8525_RESET_SR_CMD		0x2C /* software reset */
 
 /* Interrupt masks (mask bit = 1 means masked/disabled) - INTA_MASK1 bits */
+#define PCF8525_MASK1_WD_CD            BIT(0)
 #define PCF8525_MASK1_BIE		BIT(1)
 #define PCF8525_MASK1_AIE		BIT(2)
 #define PCF8525_MASK1_OSIE		BIT(3)
@@ -148,6 +157,14 @@
 
 #define PCF8525_ALM_AE_MONTH		BIT(7)
 #define PCF8525_ALM_AE_YEAR		BIT(6)
+#define PCF8525_WD_CTL_WD_CD            BIT(7)
+#define PCF8525_WD_CTL_TF0              BIT(0)
+#define PCF8525_WD_CTL_TF1              BIT(1)
+
+#define PCF8525_WD_CLOCK_HZ_X1000       250 /* 1/4 Hz */
+#define PCF8525_WD_MIN_HW_HEARTBEAT_MS  4000
+#define PCF8525_WD_VAL_STOP             0
+#define PCF8525_WD_DEFAULT_TIMEOUT_S    60
 
 #define PCF8525_REG_AGING_OFFSET_HI	0x26
 #define PCF8525_REG_AGING_OFFSET_LO	0x27
@@ -163,10 +180,13 @@
 
 struct pcf8525 {
 	struct rtc_device *rtc;
+	struct watchdog_device wdd;
 	struct regmap *regmap;
 	bool irq_enabled;
 	time64_t ts[2];
 	bool ts_valid[2];
+	int irq_inta;
+	int irq_intb;
 };
 
 static const struct regmap_config pcf8525_regmap_cfg = {
@@ -175,6 +195,218 @@ static const struct regmap_config pcf8525_regmap_cfg = {
 	.max_register = PCF8525_REG_TEMP,
 };
 
+static unsigned int pcf8525_wdt_timeout_to_val(unsigned int timeout)
+{
+	unsigned int val;
+
+	val = DIV_ROUND_UP(timeout * PCF8525_WD_CLOCK_HZ_X1000, 1000) + 1;
+
+	if (val < 2)
+		return 2;
+	if (val > 255)
+		return 255;
+
+	return val;
+}
+
+static int pcf8525_wdt_ping(struct watchdog_device *wdd)
+{
+	struct pcf8525 *pcf8525 = watchdog_get_drvdata(wdd);
+	unsigned int wd_val;
+
+	wd_val = pcf8525_wdt_timeout_to_val(wdd->timeout);
+
+	return regmap_write(pcf8525->regmap, PCF8525_REG_WD_VAL, wd_val);
+}
+
+static int pcf8525_wdt_active_ping(struct watchdog_device *wdd)
+{
+	if (watchdog_active(wdd))
+		return pcf8525_wdt_ping(wdd);
+
+	return 0;
+}
+
+static int pcf8525_wdt_start(struct watchdog_device *wdd)
+{
+	return pcf8525_wdt_ping(wdd);
+}
+
+static int pcf8525_wdt_stop(struct watchdog_device *wdd)
+{
+	struct pcf8525 *pcf8525 = watchdog_get_drvdata(wdd);
+
+	return regmap_write(pcf8525->regmap, PCF8525_REG_WD_VAL,
+				    PCF8525_WD_VAL_STOP);
+}
+
+static int pcf8525_wdt_set_timeout(struct watchdog_device *wdd,
+				   unsigned int timeout)
+{
+	wdd->timeout = timeout;
+
+	return pcf8525_wdt_active_ping(wdd);
+}
+
+static const struct watchdog_info pcf8525_wdt_info = {
+	.identity = "NXP PCF8525 Watchdog",
+	.options = WDIOF_KEEPALIVEPING | WDIOF_SETTIMEOUT,
+};
+
+static const struct watchdog_ops pcf8525_watchdog_ops = {
+	.owner = THIS_MODULE,
+	.start = pcf8525_wdt_start,
+	.stop = pcf8525_wdt_stop,
+	.ping = pcf8525_wdt_ping,
+	.set_timeout = pcf8525_wdt_set_timeout,
+};
+
+static int pcf8525_watchdog_get_period(int n, int f1000)
+{
+	return (1000 * (n - 1)) / f1000;
+}
+
+static irqreturn_t pcf8525_wdt_irq(int irq, void *data)
+{
+	struct device *dev = data;
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int ctrl2;
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL2, &ctrl2);
+	if (ret)
+		return IRQ_NONE;
+
+	if (!(ctrl2 & PCF8525_CTRL2_WDTF))
+		return IRQ_NONE;
+
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2,
+				 PCF8525_CTRL2_WDTF, 0);
+	if (ret)
+		return IRQ_NONE;
+
+	return IRQ_HANDLED;
+}
+
+static int pcf8525_watchdog_config(struct device *dev,
+				   struct pcf8525 *pcf8525)
+{
+	unsigned int m1, m2;
+	int ret;
+
+	/* Configure nINTB/CLKOUT as nINTB open-drain interrupt output. */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CLKOUT,
+				 PCF8525_CLKOUT_CLKOE, 0);
+	if (ret)
+		return ret;
+
+	/* Enable watchdog interrupt and select 1/4 Hz source: TF[1:0] = 10. */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_WD_CTL,
+				 PCF8525_WD_CTL_WD_CD |
+				 PCF8525_WD_CTL_TF1 |
+				 PCF8525_WD_CTL_TF0,
+				 PCF8525_WD_CTL_WD_CD |
+				 PCF8525_WD_CTL_TF1);
+	if (ret)
+		return ret;
+
+	/* Keep watchdog masked on INTA. */
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_INTA_MASK1, &m1);
+	if (ret)
+		return ret;
+
+	m1 |= PCF8525_MASK1_WD_CD;
+	ret = regmap_write(pcf8525->regmap, PCF8525_REG_INTA_MASK1, m1);
+	if (ret)
+		return ret;
+
+	/* Route watchdog only to INTB, and keep RTC interrupts masked on INTB. */
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_INTB_MASK1, &m1);
+	if (ret)
+		return ret;
+
+	m1 |= PCF8525_MASK1_BIE |
+	      PCF8525_MASK1_AIE |
+	      PCF8525_MASK1_OSIE |
+	      PCF8525_MASK1_SI |
+	      PCF8525_MASK1_MI;
+	m1 &= ~PCF8525_MASK1_WD_CD;
+
+	ret = regmap_write(pcf8525->regmap, PCF8525_REG_INTB_MASK1, m1);
+	if (ret)
+		return ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_INTB_MASK2, &m2);
+	if (ret)
+		return ret;
+
+	m2 |= PCF8525_MASK2_TSIE;
+
+	return regmap_write(pcf8525->regmap, PCF8525_REG_INTB_MASK2, m2);
+}
+
+static int pcf8525_watchdog_init(struct device *dev,
+				 struct pcf8525 *pcf8525)
+{
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_WATCHDOG_CORE) ||
+	    !device_property_read_bool(dev, "reset-source"))
+		return 0;
+
+	if (pcf8525->irq_intb > 0) {
+		ret = devm_request_threaded_irq(dev, pcf8525->irq_intb,
+						NULL, pcf8525_wdt_irq,
+						IRQF_ONESHOT,
+						"pcf8525-wdt", dev);
+		if (ret)
+			return ret;
+	}
+
+	ret = pcf8525_watchdog_config(dev, pcf8525);
+	if (ret)
+		return ret;
+
+	pcf8525->wdd.parent = dev;
+	pcf8525->wdd.info = &pcf8525_wdt_info;
+	pcf8525->wdd.ops = &pcf8525_watchdog_ops;
+	pcf8525->wdd.min_timeout = pcf8525_watchdog_get_period(2,
+							       PCF8525_WD_CLOCK_HZ_X1000);
+	pcf8525->wdd.max_timeout = pcf8525_watchdog_get_period(255,
+							       PCF8525_WD_CLOCK_HZ_X1000);
+	pcf8525->wdd.timeout = PCF8525_WD_DEFAULT_TIMEOUT_S;
+	watchdog_init_timeout(&pcf8525->wdd, 0, dev);
+	pcf8525->wdd.min_hw_heartbeat_ms = PCF8525_WD_MIN_HW_HEARTBEAT_MS;
+	pcf8525->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
+
+	watchdog_set_drvdata(&pcf8525->wdd, pcf8525);
+	watchdog_stop_on_reboot(&pcf8525->wdd);
+
+	return devm_watchdog_register_device(dev, &pcf8525->wdd);
+}
+
+static int pcf8525_get_irqs(struct i2c_client *client, struct pcf8525 *pcf8525)
+{
+	struct device *dev = &client->dev;
+	int irq;
+
+	irq = fwnode_irq_get_byname(dev_fwnode(dev), "inta");
+	if (irq == -ENOENT || irq == -EINVAL)
+		irq = client->irq;
+	else if (irq < 0)
+		return irq;
+	pcf8525->irq_inta = irq;
+
+	irq = fwnode_irq_get_byname(dev_fwnode(dev), "intb");
+	if (irq == -ENOENT || irq == -EINVAL)
+		irq = 0;
+	else if (irq < 0)
+		return irq;
+	pcf8525->irq_intb = irq;
+
+	return 0;
+}
+
 /*
  * Configure only the crystal fields explicitly provided by firmware.
  * If a property is absent, preserve the current hardware setting.
@@ -809,6 +1041,7 @@ static int pcf8525_unmask_irqs_intA(struct pcf8525 *pcf8525)
 	if (ret)
 		return ret;
 
+	m1 |= PCF8525_MASK1_WD_CD;
 	m1 &= ~PCF8525_MASK1_AIE;    /* unmask alarm interrupt on INTA */
 	m2 &= ~PCF8525_MASK2_TSIE;   /* unmask timestamp interrupt on INTA */
 
@@ -836,6 +1069,10 @@ static int pcf8525_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, pcf8525);
 
+	ret = pcf8525_get_irqs(client, pcf8525);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to get IRQs\n");
+
 	pcf8525->rtc = devm_rtc_allocate_device(dev);
 	if (IS_ERR(pcf8525->rtc))
 		return dev_err_probe(dev, PTR_ERR(pcf8525->rtc),
@@ -877,8 +1114,8 @@ static int pcf8525_probe(struct i2c_client *client)
 		return ret;
 
 	/* Optional IRQ */
-	if (client->irq > 0) {
-		ret = devm_request_threaded_irq(dev, client->irq,
+	if (pcf8525->irq_inta > 0) {
+		ret = devm_request_threaded_irq(dev, pcf8525->irq_inta,
 						NULL, pcf8525_irq,
 						IRQF_ONESHOT,
 						dev_name(dev), dev);
@@ -902,6 +1139,10 @@ static int pcf8525_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	ret = pcf8525_watchdog_init(dev, pcf8525);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.34.1

[PATCH 4/4] rtc: pcf8525: Add temperature sensor support via hwmon

[Shiv Prakash Gupta]

Register the PCF8525 internal temperature sensor through the hwmon
interface when CONFIG_RTC_DRV_PCF8525_HWMON is enabled.

Exposes temp1_input (millidegrees Celsius, read-only) and
update_interval (milliseconds, read/write).

Signed-off-by: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
 drivers/rtc/Kconfig       |  15 +++
 drivers/rtc/rtc-pcf8525.c | 192 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 6c1c7b3d27b6..b2f8537d602b 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -524,6 +524,21 @@ config RTC_DRV_PCF8525
           This driver can also be built as a module. If so, the module
           will be called rtc-pcf8525.
 
+config RTC_DRV_PCF8525_HWMON
+	bool "HWMON support for NXP PCF8525"
+	depends on RTC_DRV_PCF8525 && HWMON && \
+		   !(RTC_DRV_PCF8525=y && HWMON=m)
+	default y
+	help
+	  Say Y here to expose the PCF8525 internal temperature
+	  sensor through the HWMON interface.
+
+	  This option provides temperature input reporting and allows
+	  the temperature measurement update interval to be configured
+	  from userspace.
+
+	  The interface is registered only when HWMON support is enabled
+
 config RTC_DRV_PCF85363
 	tristate "NXP PCF85363"
 	select REGMAP_I2C
diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
index 4ef648bdfc5b..92bab3231ce7 100644
--- a/drivers/rtc/rtc-pcf8525.c
+++ b/drivers/rtc/rtc-pcf8525.c
@@ -34,6 +34,7 @@
  */
 
 #include <linux/bcd.h>
+#include <linux/hwmon.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/device.h>
@@ -165,6 +166,7 @@
 #define PCF8525_WD_MIN_HW_HEARTBEAT_MS  4000
 #define PCF8525_WD_VAL_STOP             0
 #define PCF8525_WD_DEFAULT_TIMEOUT_S    60
+#define PCF8525_CLKOUT_TCR_MASK		GENMASK(7, 5)
 
 #define PCF8525_REG_AGING_OFFSET_HI	0x26
 #define PCF8525_REG_AGING_OFFSET_LO	0x27
@@ -570,6 +572,194 @@ static int pcf8525_rtc_set_offset(struct device *dev, long offset)
 	return pcf8525_write_aging_offset(pcf8525, (s16)raw);
 }
 
+static int pcf8525_hwmon_read_temp(struct device *dev, long *temp)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int regval;
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_TEMP, &regval);
+	if (ret)
+		return ret;
+
+	/*
+	 * PCF8525: signed 8-bit, 1 degree C per LSB.
+	 * HWMON requires millidegree Celsius.
+	 */
+	*temp = (long)(s8)(u8)regval * 1000L;
+
+	return 0;
+}
+
+static int pcf8525_hwmon_read_update_interval(struct device *dev, long *val)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int regval;
+	unsigned int tcr;
+	int ret;
+
+	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CLKOUT, &regval);
+	if (ret)
+		return ret;
+
+	tcr = FIELD_GET(PCF8525_CLKOUT_TCR_MASK, regval);
+
+	switch (tcr) {
+	case 0:
+		*val = 32 * 60 * 1000L;
+		break;
+	case 1:
+		*val = 16 * 60 * 1000L;
+		break;
+	case 2:
+		*val = 8 * 60 * 1000L;
+		break;
+	case 3:
+		*val = 4 * 60 * 1000L;
+		break;
+	case 4:
+		*val = 2 * 60 * 1000L;
+		break;
+	default:
+		*val = 60 * 1000L;
+		break;
+	}
+
+	return 0;
+}
+
+static int pcf8525_hwmon_write_update_interval(struct device *dev, long val)
+{
+	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
+	unsigned int tcr;
+
+	switch (val) {
+	case 32 * 60 * 1000L:
+		tcr = 0;
+		break;
+	case 16 * 60 * 1000L:
+		tcr = 1;
+		break;
+	case 8 * 60 * 1000L:
+		tcr = 2;
+		break;
+	case 4 * 60 * 1000L:
+		tcr = 3;
+		break;
+	case 2 * 60 * 1000L:
+		tcr = 4;
+		break;
+	case 60 * 1000L:
+		tcr = 5;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/* Update only TCR[2:0]; preserve OTPR, CLKOE and COF[2:0]. */
+	return regmap_update_bits(pcf8525->regmap, PCF8525_REG_CLKOUT,
+				  PCF8525_CLKOUT_TCR_MASK,
+				  FIELD_PREP(PCF8525_CLKOUT_TCR_MASK, tcr));
+}
+
+static umode_t pcf8525_hwmon_is_visible(const void *data,
+					enum hwmon_sensor_types type,
+					u32 attr, int channel)
+{
+	switch (type) {
+	case hwmon_chip:
+		if (attr == hwmon_chip_update_interval)
+			return 0644;
+		break;
+	case hwmon_temp:
+		if (attr == hwmon_temp_input && channel == 0)
+			return 0444;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int pcf8525_hwmon_read(struct device *dev,
+			      enum hwmon_sensor_types type,
+			      u32 attr, int channel, long *val)
+{
+	switch (type) {
+	case hwmon_chip:
+		if (attr == hwmon_chip_update_interval)
+			return pcf8525_hwmon_read_update_interval(dev, val);
+		break;
+	case hwmon_temp:
+		if (attr == hwmon_temp_input && channel == 0)
+			return pcf8525_hwmon_read_temp(dev, val);
+		break;
+	default:
+		break;
+	}
+
+	return -EOPNOTSUPP;
+}
+
+static int pcf8525_hwmon_write(struct device *dev,
+			       enum hwmon_sensor_types type,
+			       u32 attr, int channel, long val)
+{
+	if (type == hwmon_chip && attr == hwmon_chip_update_interval)
+		return pcf8525_hwmon_write_update_interval(dev, val);
+
+	return -EOPNOTSUPP;
+}
+
+static const struct hwmon_channel_info * const pcf8525_hwmon_info[] = {
+	HWMON_CHANNEL_INFO(chip, HWMON_C_UPDATE_INTERVAL),
+	HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
+	NULL
+};
+
+static const struct hwmon_ops pcf8525_hwmon_ops = {
+	.is_visible = pcf8525_hwmon_is_visible,
+	.read = pcf8525_hwmon_read,
+	.write = pcf8525_hwmon_write,
+};
+
+static const struct hwmon_chip_info pcf8525_hwmon_chip_info = {
+	.ops = &pcf8525_hwmon_ops,
+	.info = pcf8525_hwmon_info,
+};
+
+/*
+ * Keep HWMON optional and non-fatal so RTC and watchdog registration remain
+ * usable even if the temperature interface cannot be registered.
+ */
+static void pcf8525_hwmon_register(struct device *dev,
+				   struct pcf8525 *pcf8525)
+{
+	struct device *hwmon_dev;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_RTC_DRV_PCF8525_HWMON))
+		return;
+
+	/* Enable only the digital readout; preserve TSIE, CL and XTL_TYP. */
+	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL5,
+				 PCF8525_CTRL5_TEMP_RD_EN,
+				 PCF8525_CTRL5_TEMP_RD_EN);
+	if (ret) {
+		dev_warn(dev, "failed to enable temperature readout: %d\n", ret);
+		return;
+	}
+
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, "pcf8525",
+							 pcf8525,
+							 &pcf8525_hwmon_chip_info,
+							 NULL);
+	if (IS_ERR(hwmon_dev))
+		dev_warn(dev, "failed to register HWMON device: %ld\n",
+			 PTR_ERR(hwmon_dev));
+}
+
 static int pcf8525_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
@@ -1143,6 +1333,8 @@ static int pcf8525_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	pcf8525_hwmon_register(dev, pcf8525);
+
 	return 0;
 }
 
-- 
2.34.1

Re: [PATCH 1/4] dt-bindings: rtc: Add NXP PCF8525 RTC

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `interrupt-names` schema strictly requires the first interrupt to be `inta`, incorrectly rejecting a configuration where only a single `intb` interrupt is connected.
--

--- Patch [1]: [PATCH 1/4] dt-bindings: rtc: Add NXP PCF8525 RTC ---
commit edd4cb5090a8c1765775edd3168f097a2f181903
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

dt-bindings: rtc: Add NXP PCF8525 RTC

This commit adds a Devicetree binding for the NXP PCF8525 I2C RTC. It
describes optional inta and intb interrupts alongside properties for
external crystal load capacitance and temperature-compensation crystal
models.

> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml
> new file mode 100644
> index 0000000000000..bee71ec3da808
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf8525.yaml

[ ... ]

> +  interrupts:
> +    minItems: 1
> +    maxItems: 2
> +    description:
> +      INTA, followed by INTB when both interrupt outputs are connected.
> +
> +  interrupt-names:
> +    minItems: 1
> +    items:
> +      - const: inta
> +      - const: intb

[Severity: Medium]
Will this strictly require the first interrupt to be inta, rejecting a
configuration where only a single intb interrupt is connected?

If a board designer creates a device tree node with a single interrupt and
sets interrupt-names to intb, the schema's items list imposes strict
positional constraints requiring the first element to be inta. This would
cause validation to fail for a valid hardware configuration where only the
intb pin is connected.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807082855.3762394-1-shivprakash.gupta@nxp.com?part=1

Re: [PATCH 4/4] rtc: pcf8525: Add temperature sensor support via hwmon

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Subsystem Layering Violation: Registering hwmon functionality directly from the RTC driver.
- [Medium] Missing hardware state rollback when hwmon registration fails.
--

--- Patch [4]: [PATCH 4/4] rtc: pcf8525: Add temperature sensor support via hwmon ---
commit 36335f5669e8a341626815565d14236fae935a84
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

rtc: pcf8525: Add temperature sensor support via hwmon
    
Register the PCF8525 internal temperature sensor through the hwmon
interface when CONFIG_RTC_DRV_PCF8525_HWMON is enabled.

> diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
> index 4ef648bdfc5b9..92bab3231ce7f 100644
> --- a/drivers/rtc/rtc-pcf8525.c
> +++ b/drivers/rtc/rtc-pcf8525.c
[ ... ]
> +static void pcf8525_hwmon_register(struct device *dev,
> +				   struct pcf8525 *pcf8525)
> +{
> +	struct device *hwmon_dev;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_RTC_DRV_PCF8525_HWMON))
> +		return;
> +
> +	/* Enable only the digital readout; preserve TSIE, CL and XTL_TYP. */
> +	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL5,
> +				 PCF8525_CTRL5_TEMP_RD_EN,
> +				 PCF8525_CTRL5_TEMP_RD_EN);
> +	if (ret) {
> +		dev_warn(dev, "failed to enable temperature readout: %d\n", ret);
> +		return;
> +	}
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "pcf8525",
> +							 pcf8525,
> +							 &pcf8525_hwmon_chip_info,
> +							 NULL);

[Severity: Medium]
Is it permitted to register the hwmon device directly from this RTC driver?

The hwmon subsystem guidelines specify that if the main functionality of a
device is not hardware monitoring, the hwmon features should be implemented
as an auxiliary device driver residing in drivers/hwmon/.

> +	if (IS_ERR(hwmon_dev))
> +		dev_warn(dev, "failed to register HWMON device: %ld\n",
> +			 PTR_ERR(hwmon_dev));

[Severity: Medium]
If devm_hwmon_device_register_with_info() fails, should the hardware state
be rolled back?

The code sets the PCF8525_CTRL5_TEMP_RD_EN bit in the register before
calling the registration function. If registration fails, the error path
logs a warning and returns, leaving the temperature sensor enabled in the
hardware. Could this cause unnecessary power consumption?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807082855.3762394-1-shivprakash.gupta@nxp.com?part=4

Re: [PATCH 2/4] rtc: pcf8525: Add NXP PCF8525 RTC driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unsynchronized concurrent access to the shared timestamp state (`ts` and `ts_valid`) between the threaded IRQ handler and sysfs callbacks.
- [High] Read-modify-write on Write-0-to-Clear (W0C) interrupt flags drops concurrent interrupts.
- [High] Hardware timestamp engine freezes after the first event in IRQ mode due to missing CTS command.
- [High] The alarm interrupt is not properly configured as a system wake source, preventing the RTC alarm from waking the system from suspend.
- [Low] The I2C device ID array uses positional initializers rather than named initializers.
--

--- Patch [2]: [PATCH 2/4] rtc: pcf8525: Add NXP PCF8525 RTC driver ---
commit b181a191f404e5910be38077d8059abeffe4a95b
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

rtc: pcf8525: Add NXP PCF8525 RTC driver

Add support for time and date, alarms, timestamp capture, backup
switch-over configuration, external crystal configuration and clock
offset correction.

--- a/drivers/rtc/rtc-pcf8525.c
+++ b/drivers/rtc/rtc-pcf8525.c
[ ... ]
> +static ssize_t timestamp_show_common(struct device *dev, char *buf, int id)
> +{
> +	struct pcf8525 *pcf8525 = dev_get_drvdata(dev->parent);
> +	unsigned int ctrl4;
> +	time64_t ts;
> +	int ret;
> +
> +	if (id < 0 || id > 1)
> +		return 0;
> +
> +	if (pcf8525->irq_enabled) {
> +		if (!pcf8525->ts_valid[id])
> +			return 0;
> +		ts = pcf8525->ts[id];
> +		return sysfs_emit(buf, "%llu\n", (unsigned long long)ts);

[Severity: High]
Is it possible for the threaded IRQ handler to update the timestamp state
concurrently with this sysfs read?

If a user-space process reads the sysfs timestamp attribute at the same time
an RTC timestamp interrupt is being processed in pcf8525_ts_snapshot(),
this could result in a torn read on 32-bit platforms where time64_t requires
two 32-bit memory accesses.

Should a mutex or memory barrier be used to synchronize access to the
timestamp state?

> +	}
[ ... ]
> +static ssize_t timestamp_store_common(struct device *dev, const char *buf,
> +				      size_t count, int id)
> +{
> +	struct pcf8525 *pcf8525 = dev_get_drvdata(dev->parent);
> +	int ret;
> +
> +	if (id < 0 || id > 1)
> +		return -EINVAL;
> +
> +	if (pcf8525->irq_enabled) {
> +		pcf8525->ts_valid[id] = false;
> +		return count;
> +	}

[Severity: High]
When in IRQ mode, this function returns early without sending the Clear
Timestamp (CTS) command to the hardware.

Since the hardware is configured in freeze mode (PCF8525_TS_CTL1_TSM), it
will ignore new events until the CTS command is issued. The interrupt handler
pcf8525_irq() only clears the TSF interrupt flag.

Will this cause the timestamp engine to stop capturing events after the first
one in IRQ mode?

> +
> +	/*
> +	 * Clear timestamp via RESET CTS command.
[ ... ]
> +static irqreturn_t pcf8525_irq(int irq, void *data)
> +{
> +	struct device *dev = data;
> +	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
> +	unsigned int ctrl2, ctrl4;
> +	int ret;
> +
> +	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL2, &ctrl2);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL4, &ctrl4);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	if (!(ctrl2 & (PCF8525_CTRL2_AF | PCF8525_CTRL2_MSF)) &&
> +	    !(ctrl4 & PCF8525_CTRL4_TSF))
> +		return IRQ_NONE;
> +
> +	/* Timestamp */
> +	if (ctrl4 & PCF8525_CTRL4_TSF) {
> +		pcf8525_ts_snapshot(dev);
> +		/* Clear TSF */
> +		regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL4, PCF8525_CTRL4_TSF, 0);
> +	}
> +
> +	/* Alarm */
> +	if (ctrl2 & PCF8525_CTRL2_AF) {
> +		rtc_update_irq(pcf8525->rtc, 1, RTC_IRQF | RTC_AF);
> +		/* Clear AF */
> +		regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2, PCF8525_CTRL2_AF, 0);

[Severity: High]
Does using regmap_update_bits() here risk dropping concurrent interrupts?

This function performs a read-modify-write. If it writes 0 to clear AF or MSF,
it writes back 0 for any other Write-0-to-Clear flag that was 0 at the time
of the read. If a new hardware event sets another W0C flag between the read
and write, the write of 0 will inadvertently clear the new event before it can
be handled.

> +	}
> +
> +	if (ctrl2 & PCF8525_CTRL2_MSF) {
> +		ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2, PCF8525_CTRL2_MSF, 0);
> +		if (ret)
> +			return IRQ_NONE;
> +	}
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> +static int pcf8525_probe(struct i2c_client *client)
> +{
[ ... ]
> +	/* Optional IRQ */
> +	if (client->irq > 0) {
> +		ret = devm_request_threaded_irq(dev, client->irq,
> +						NULL, pcf8525_irq,
> +						IRQF_ONESHOT,
> +						dev_name(dev), dev);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to request INTA IRQ\n");
> +
> +		pcf8525->irq_enabled = true;
> +
> +		/* Route/unmask alarm + timestamp to INTA by default */
> +		ret = pcf8525_unmask_irqs_intA(pcf8525);
> +		if (ret)
> +			return ret;
> +
> +		device_init_wakeup(dev, true);
> +		set_bit(RTC_FEATURE_ALARM, pcf8525->rtc->features);

[Severity: High]
This declares wakeup capability, but is the IRQ ever connected as a wake
source?

Without calling dev_pm_set_wake_irq() or implementing suspend/resume
callbacks with enable_irq_wake(), the IRQ controller is never instructed to
keep the interrupt armed during suspend.

Could this prevent the RTC alarm from waking the system from suspend?

> +	}
> +
> +	/* Register device */
> +	ret = devm_rtc_register_device(pcf8525->rtc);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id pcf8525_of_match[] = {
> +	{ .compatible = "nxp,pcf8525" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, pcf8525_of_match);
> +
> +static const struct i2c_device_id pcf8525_i2c_id[] = {
> +	{ "pcf8525", 0 },
> +	{ }
> +};

[Severity: Low]
Could this use named initializers?

The I2C subsystem guidelines require initialized arrays of type
struct i2c_device_id to use named initializers.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807082855.3762394-1-shivprakash.gupta@nxp.com?part=2

Re: [PATCH 3/4] rtc: pcf8525: Add watchdog support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:

New issues:
- [Critical] Unmasking watchdog interrupt without clearing persistent WDTF flag causes inescapable boot loop.
- [High] Watchdog IRQ handler silently clears timeout flag, aborting hardware resets and defeating the watchdog.
- [High] Missing WDOG_HW_RUNNING reporting causes spurious system resets if the watchdog is armed by the bootloader.
- [High] Requesting INTB without IRQF_SHARED breaks probe on boards tying INTA and INTB together.
- [Medium] Watchdog initialization occurs after RTC registration, creating a race condition exposing the uninitialized device.

Pre-existing issues:
- [High] Read-modify-write on CTRL2 causes lost hardware interrupts for RTC alarm and watchdog.
--

--- Patch [3]: [PATCH 3/4] rtc: pcf8525: Add watchdog support ---
commit 3cb38601cbc74987657b19d7480456e2da5babc7
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

rtc: pcf8525: Add watchdog support

This commit adds watchdog support for the NXP PCF8525 real time clock device.

> diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
> index 7dfb4f0f64047..4ef648bdfc5b9 100644
> --- a/drivers/rtc/rtc-pcf8525.c
> +++ b/drivers/rtc/rtc-pcf8525.c
[ ... ]
> +static irqreturn_t pcf8525_wdt_irq(int irq, void *data)
> +{
> +	struct device *dev = data;
> +	struct pcf8525 *pcf8525 = dev_get_drvdata(dev);
> +	unsigned int ctrl2;
> +	int ret;
> +
> +	ret = regmap_read(pcf8525->regmap, PCF8525_REG_CTRL2, &ctrl2);
> +	if (ret)
> +		return IRQ_NONE;
> +
> +	if (!(ctrl2 & PCF8525_CTRL2_WDTF))
> +		return IRQ_NONE;
> +
> +	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL2,
> +				 PCF8525_CTRL2_WDTF, 0);

[Severity: High]
Does returning IRQ_HANDLED here without notifying the watchdog core or
invoking panic() silently abort the hardware reset? If INTB is routed to
a reset controller, de-asserting it before the reset cycle completes might
leave the system in a hung state rather than resetting it.

[Severity: High]
This isn't a bug introduced by this patch, but this read-modify-write
on the CTRL2 register extends a pre-existing race condition to the
watchdog handler thread.

CTRL2 contains write-0-to-clear flags like AF, MSF, and WDTF. Because
regmap_update_bits() reads the register and writes it back, if a hardware
flag asserts after the read but before the write, could this write back
a 0 and accidentally clear the newly asserted flag?

This same pattern exists in pcf8525_irq() when clearing AF, which could
cause lost hardware interrupts.

> +	if (ret)
> +		return IRQ_NONE;
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> +static int pcf8525_watchdog_config(struct device *dev,
> +				   struct pcf8525 *pcf8525)
> +{
[ ... ]
> +	m1 |= PCF8525_MASK1_BIE |
> +	      PCF8525_MASK1_AIE |
> +	      PCF8525_MASK1_OSIE |
> +	      PCF8525_MASK1_SI |
> +	      PCF8525_MASK1_MI;
> +	m1 &= ~PCF8525_MASK1_WD_CD;
> +
> +	ret = regmap_write(pcf8525->regmap, PCF8525_REG_INTB_MASK1, m1);

[Severity: Critical]
If the watchdog resets the system, the battery-backed RTC retains the WDTF
(Watchdog Timeout Flag) set to 1. On reboot, when this unmasks the watchdog
interrupt on the INTB pin, will it immediately assert INTB since WDTF is
still 1? This regression could trigger the hardware reset again and cause
an inescapable boot loop.

> +	if (ret)
> +		return ret;
[ ... ]
> +static int pcf8525_watchdog_init(struct device *dev,
> +				 struct pcf8525 *pcf8525)
> +{
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_WATCHDOG_CORE) ||
> +	    !device_property_read_bool(dev, "reset-source"))
> +		return 0;
> +
> +	if (pcf8525->irq_intb > 0) {
> +		ret = devm_request_threaded_irq(dev, pcf8525->irq_intb,
> +						NULL, pcf8525_wdt_irq,
> +						IRQF_ONESHOT,
> +						"pcf8525-wdt", dev);

[Severity: High]
Many hardware designs tie open-drain I2C interrupts (INTA and INTB) to a
single SoC GPIO. If they are mapped to the same IRQ, will this request fail
because it lacks IRQF_SHARED and uses the same dev_id token (dev)?
This could result in a -EBUSY error and cause the entire device probe
to fail.

[ ... ]
> +	pcf8525->wdd.timeout = PCF8525_WD_DEFAULT_TIMEOUT_S;
> +	watchdog_init_timeout(&pcf8525->wdd, 0, dev);
> +	pcf8525->wdd.min_hw_heartbeat_ms = PCF8525_WD_MIN_HW_HEARTBEAT_MS;
> +	pcf8525->wdd.status = WATCHDOG_NOWAYOUT_INIT_STATUS;
> +
> +	watchdog_set_drvdata(&pcf8525->wdd, pcf8525);
> +	watchdog_stop_on_reboot(&pcf8525->wdd);
> +
> +	return devm_watchdog_register_device(dev, &pcf8525->wdd);

[Severity: High]
If the bootloader starts the watchdog, does unconditionally setting the
status without checking if the hardware timer is active cause issues?
Because it fails to set WDOG_HW_RUNNING, the watchdog core will not
initiate the ping worker, which might allow the timer to expire and
cause a spurious system reboot during boot.

[ ... ]
> @@ -902,6 +1139,10 @@ static int pcf8525_probe(struct i2c_client *client)
>  	if (ret)
>  		return ret;
>  
> +	ret = pcf8525_watchdog_init(dev, pcf8525);
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }

[Severity: Medium]
Could initializing the watchdog after RTC registration expose the RTC
device to userspace prematurely? If pcf8525_watchdog_init() fails,
probe will return an error and devres will unregister the RTC,
which might cause operations on the opened device to fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807082855.3762394-1-shivprakash.gupta@nxp.com?part=3