* [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support
@ 2026-08-12 7:35 Lakshay Piplani
2026-08-12 7:35 ` [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support Lakshay Piplani
2026-08-12 7:49 ` [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support sashiko-bot
0 siblings, 2 replies; 4+ messages in thread
From: Lakshay Piplani @ 2026-08-12 7:35 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree
Cc: vikash.bansal, priyanka.jain, Lakshay Piplani, Pankit Garg,
Conor Dooley
Add device tree bindings for NXP PCF85053 RTC chip.
Signed-off-by: Pankit Garg <pankit.garg@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
V6 -> V7: - no changes
- Added Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
V5 -> V6: - Dropped driver-specific commentary from property descriptions.
- Simplified and clarified descriptions for better readability.
V4 -> V5: - Updated schema validation logic to enforce correct combinations of
'nxp,interface' and 'nxp,write-access' using oneOf clauses.
- Refined property descriptions for clarity and hardware alignment.
V3 -> V4: Add dedicated nxp,pcf85053.yaml.
Remove entry from trivial-rtc.yaml.
V2 -> V3: Moved MAINTAINERS file changes to the driver patch
V1 -> V2: Handled dt-bindings by trivial-rtc.yaml
.../devicetree/bindings/rtc/nxp,pcf85053.yaml | 114 ++++++++++++++++++
1 file changed, 114 insertions(+)
create mode 100644 Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
new file mode 100644
index 000000000000..81cfceabc04c
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2025 NXP
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/rtc/nxp,pcf85053.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP PCF85053 Real Time Clock
+
+maintainers:
+ - Pankit Garg <pankit.garg@nxp.com>
+ - Lakshay Piplani <lakshay.piplani@nxp.com>
+
+properties:
+ compatible:
+ enum:
+ - nxp,pcf85053
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ nxp,interface:
+ $ref: /schemas/types.yaml#/definitions/string
+ enum: [ primary, secondary ]
+ description: |
+ Identifies this host's logical role in a multi-host topology for the
+ PCF85053 RTC. The device exposes a "TWO" ownership bit in the CTRL
+ register that gates which host may write time/alarm registers.
+ - "primary": Designated host that *may* claim write ownership (set
+ CTRL.TWO=1) **if** write-access is explicitly requested.
+ - "secondary": Peer host that writes only when CTRL.TWO=0 (default).
+
+ This property determines the intended role of the host in relation to
+ the write ownership.
+
+ The actual role depends on whether 'nxp,write-access' is also specified.
+ Supported configurations are:-
+ 1. Primary with 'nxp,write-access' -> primary claims write ownership.
+ 2. Primary without 'nxp,write-access' -> primary is ready only; secondary may write.
+ 3. Secondary (must not specify 'nxp,write-access') -> Secondary writes only
+ when no primary claims ownership.
+
+ nxp,write-access:
+ type: boolean
+ description: |
+ Indicates that write ownership of the PCF85053 RTC should be claimed by setting
+ CTRL.TWO=1. This property is only valid when acting as the primary interface
+ (nxp,interface="primary").
+
+required:
+ - compatible
+ - reg
+ - nxp,interface
+
+additionalProperties: false
+
+allOf:
+ - $ref: rtc.yaml#
+ - if:
+ properties:
+ nxp,interface:
+ const: secondary
+ then:
+ not:
+ required: [ "nxp,write-access" ]
+
+examples:
+ # Single host example.
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "nxp,pcf85053";
+ reg = <0x6f>;
+ nxp,interface = "primary";
+ nxp,write-access;
+ interrupt-parent = <&gpio2>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ };
+ };
+
+ # Dual-host example: one primary that claims writes; one secondary that never claims writes.
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "nxp,pcf85053";
+ reg = <0x6f>;
+ nxp,interface = "primary";
+ nxp,write-access;
+ interrupt-parent = <&gpio2>;
+ interrupts = <3 IRQ_TYPE_EDGE_FALLING>;
+ };
+ };
+
+ i2c1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ rtc@6f {
+ compatible = "nxp,pcf85053";
+ reg = <0x6f>;
+ nxp,interface = "secondary";
+ };
+ };
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support
2026-08-12 7:35 [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support Lakshay Piplani
@ 2026-08-12 7:35 ` Lakshay Piplani
2026-08-12 7:52 ` sashiko-bot
2026-08-12 7:49 ` [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support sashiko-bot
1 sibling, 1 reply; 4+ messages in thread
From: Lakshay Piplani @ 2026-08-12 7:35 UTC (permalink / raw)
To: alexandre.belloni, linux-rtc, linux-kernel, robh, krzk+dt,
conor+dt, devicetree
Cc: vikash.bansal, priyanka.jain, Lakshay Piplani, Daniel Aguirre,
Pankit Garg
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=git send-email --to=alexandre.belloni@bootlin.com,linux-rtc@vger.kernel.org,linux-kernel@vger.kernel.org,robh@kernel.org,krzk+dt@kernel.org,conor+dt@kernel.org,devicetree@vger.kernel.org -cc=vikash.bansal@nxp.com,priyanka.jain@nxp.com --smtp-auth=none *.patch, Size: 22331 bytes --]
PCF85053 is i2c based RTC which supports timer and calendar
functionality.
Features supported:
1. Read/Write time
2. Get/Set Alarm
3. Wakeup Source
4. Generate up to 32768Hz clock output
5. Primary/Secondary i2c bus
Signed-off-by: Daniel Aguirre <daniel.aguirre@nxp.com>
Signed-off-by: Pankit Garg <pankit.garg@nxp.com>
Signed-off-by: Lakshay Piplani <lakshay.piplani@nxp.com>
---
V6 -> V7: - Addressed minor cleanups from review: use dev_get_drvdata()/dev_set_drvdata()
consistently, fix alarm IRQ dev_id handling, and switch to devm_device_init_wakeup().
- Simplified time/alarm programming by forcing 24h + binary mode in hardware,
dropping complex 12h/BCD handling in setters.
- Documented the 2000–2099 supported year range, explaining how the 00–99 year
register maps to leap-year behavior in the device.
V5 -> V6: no changes
V4 -> V5: no changes
V3 -> V4: - Handle multi-host ownership explicitly using primary/secondary bus hadling.
- Probe no longer changes any CTRL bits unconditionally and do not clear ST/AF/OF
avoiding lost interrupts or silent mode changes.
- Read/Set time & alarm now respect HF(12/24h) and DM(BCD/BIN) converting
hour fields correctly for all combinations.
- Minor changes: drop noisy warnings, tidy error paths/comments.
V2 -> V3: Add MAINTAINERS file changes to this patch
V1 -> V2: no changes
MAINTAINERS | 7 +
drivers/rtc/Kconfig | 10 +
drivers/rtc/Makefile | 1 +
drivers/rtc/rtc-pcf85053.c | 717 +++++++++++++++++++++++++++++++++++++
4 files changed, 735 insertions(+)
create mode 100644 drivers/rtc/rtc-pcf85053.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 047d242faf68..ca441affa186 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18692,6 +18692,13 @@ S: Maintained
F: Documentation/devicetree/bindings/sound/nxp,tfa989x.yaml
F: sound/soc/codecs/tfa989x.c
+NXP RTC PCF85053 DRIVER
+M: Pankit Gargi <pankit.garg@nxp.com>
+M: Lakshay Piplani <lakshay.piplani@nxp.com>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+F: drivers/rtc/rtc-pcf85053.c
+
NZXT-KRAKEN2 HARDWARE MONITORING DRIVER
M: Jonas Malaco <jonas@protocubo.io>
L: linux-hwmon@vger.kernel.org
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 2933c41c77c8..e25b75d38849 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -994,6 +994,16 @@ config RTC_DRV_PCF85063
This driver can also be built as a module. If so, the module
will be called rtc-pcf85063.
+config RTC_DRV_PCF85053
+ tristate "NXP PCF85053"
+ depends on OF && I2C
+ help
+ If you say yes here you get support for the NXP PCF85053 I2C Bootable CPU RTC
+ chip.
+
+ This driver can also be built as a module. If so, the module
+ will be called rtc-pcf85053.
+
config RTC_DRV_RV3029C2
tristate "Micro Crystal RV3029/3049"
depends on RTC_I2C_AND_SPI
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 8221bda6e6dc..90c4f73393af 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -129,6 +129,7 @@ obj-$(CONFIG_RTC_DRV_PCAP) += rtc-pcap.o
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_PCF85053) += rtc-pcf85053.o
obj-$(CONFIG_RTC_DRV_PCF8523) += rtc-pcf8523.o
obj-$(CONFIG_RTC_DRV_PCF85363) += rtc-pcf85363.o
obj-$(CONFIG_RTC_DRV_PCF8563) += rtc-pcf8563.o
diff --git a/drivers/rtc/rtc-pcf85053.c b/drivers/rtc/rtc-pcf85053.c
new file mode 100644
index 000000000000..5829b0e7a67a
--- /dev/null
+++ b/drivers/rtc/rtc-pcf85053.c
@@ -0,0 +1,717 @@
+// SPDX-License-Identifier: GPL-2.0
+// Copyright 2025 NXP
+
+#include <linux/bcd.h>
+#include <linux/clk-provider.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+#include <linux/pm_wakeirq.h>
+#include <linux/regmap.h>
+
+#define PCF85053_REG_SC 0x00 /* seconds */
+#define PCF85053_REG_SCA 0x01 /* alarm */
+#define PCF85053_REG_MN 0x02 /* minutes */
+#define PCF85053_REG_MNA 0x03 /* alarm */
+#define PCF85053_REG_HR 0x04 /* hour */
+#define PCF85053_REG_HRA 0x05 /* alarm */
+#define PCF85053_REG_DW 0x06 /* day of week */
+#define PCF85053_REG_DM 0x07 /* day of month */
+#define PCF85053_REG_MO 0x08 /* month */
+#define PCF85053_REG_YR 0x09 /* year */
+#define PCF85053_REG_CTRL 0x0A /* timer control */
+#define PCF85053_REG_ST 0x0B /* status */
+#define PCF85053_REG_CLKO 0x0C /* clock out */
+#define PCF85053_REG_ACC 0x14 /* xclk access */
+
+#define PCF85053_BIT_AF BIT(7)
+#define PCF85053_BIT_ST BIT(7)
+#define PCF85053_BIT_DM BIT(6)
+#define PCF85053_BIT_HF BIT(5)
+#define PCF85053_BIT_DSM BIT(4)
+#define PCF85053_BIT_AIE BIT(3)
+#define PCF85053_BIT_OFIE BIT(2)
+#define PCF85053_BIT_CIE BIT(1)
+#define PCF85053_BIT_TWO BIT(0)
+#define PCF85053_BIT_XCLK BIT(7)
+
+#define PCF85053_REG_BAT_MASK 0x07 /* Battery mask */
+#define PCF85053A_BVL_MASK 0x07
+#define PCF85053A_BVL_LOW_THRESHOLD 0x02
+#define PCF85053_REG_CLKO_F_MASK 0x03 /* Frequenc mask */
+#define PCF85053_REG_CLKO_CKE 0x80 /* clock out enabled */
+#define PCF85053_BIT_OF BIT(6)
+
+#define PCF85053_HR_PM BIT(7)
+#define PCF85053_HR_24H_MASK GENMASK(5, 0)
+
+struct pcf85053_config {
+ const struct regmap_config regmap;
+ unsigned has_alarms:1;
+};
+
+struct pcf85053 {
+ struct rtc_device *rtc;
+ struct regmap *regmap;
+#ifdef CONFIG_COMMON_CLK
+ struct clk_hw clkout_hw;
+#endif
+ bool is_primary;
+};
+
+static inline int pcf85053_read_two_bit(struct pcf85053 *pcf85053, bool *two)
+{
+ unsigned int ctrl;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ *two = !!(ctrl & PCF85053_BIT_TWO);
+
+ return 0;
+}
+
+static inline bool pcf85053_time_write_access(struct pcf85053 *pcf85053)
+{
+ bool two;
+
+ if (pcf85053_read_two_bit(pcf85053, &two))
+ return false;
+
+ /* Primary writes iff TWO=1; secondary writes iff TWO=0 */
+ return pcf85053->is_primary ? two : !two;
+}
+
+static int pcf85053_set_alarm_mode(struct regmap *regmap, bool on)
+{
+ unsigned int val;
+ int err;
+
+ val = on ? PCF85053_BIT_AIE : 0;
+
+ err = regmap_update_bits(regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_AIE,
+ val);
+ if (err)
+ return err;
+
+ return regmap_update_bits(regmap, PCF85053_REG_ST,
+ PCF85053_BIT_AF, 0);
+}
+
+static int pcf85053_get_alarm_mode(struct device *dev,
+ unsigned char *alarm_enable, unsigned char *alarm_flag)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int val;
+ int err;
+
+ if (alarm_enable) {
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &val);
+ if (err)
+ return err;
+
+ *alarm_enable = val & PCF85053_BIT_AIE;
+ }
+
+ if (alarm_flag) {
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_ST, &val);
+ if (err)
+ return err;
+
+ *alarm_flag = val & PCF85053_BIT_AF;
+ }
+
+ return 0;
+}
+
+static irqreturn_t pcf85053_irq(int irq, void *dev_id)
+{
+ struct device *dev = dev_id;
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ bool changed;
+ int err;
+
+ err = regmap_update_bits_check(pcf85053->regmap,
+ PCF85053_REG_ST,
+ PCF85053_BIT_AF, 0,
+ &changed);
+
+ if (err || !changed)
+ return IRQ_NONE;
+
+ rtc_update_irq(pcf85053->rtc, 1, RTC_IRQF | RTC_AF);
+ return IRQ_HANDLED;
+}
+
+/*
+ * In the routines that deal directly with the PCF85053 hardware, we use
+ * rtc_time -- month 0-11, hour 0-23, yr = calendar year-epoch.
+ */
+static int pcf85053_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int ctrl, st, h12;
+ bool is_24h, is_bin;
+ u8 regs[10], hr;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_ST, &st);
+ if (err)
+ return err;
+
+ if (ctrl & PCF85053_BIT_ST)
+ return -EINVAL;
+
+ err = regmap_bulk_read(pcf85053->regmap, PCF85053_REG_SC, regs, sizeof(regs));
+ if (err)
+ return err;
+
+ if (ctrl & PCF85053_BIT_DM) {
+ tm->tm_sec = regs[PCF85053_REG_SC] & 0x7F;
+ tm->tm_min = regs[PCF85053_REG_MN] & 0x7F;
+ tm->tm_mday = regs[PCF85053_REG_DM] & 0x3F;
+ tm->tm_mon = (regs[PCF85053_REG_MO] & 0x1F) - 1;
+ tm->tm_year = regs[PCF85053_REG_YR] + 100;
+ } else {
+ tm->tm_sec = bcd2bin(regs[PCF85053_REG_SC] & 0x7F);
+ tm->tm_min = bcd2bin(regs[PCF85053_REG_MN] & 0x7F);
+ tm->tm_mday = bcd2bin(regs[PCF85053_REG_DM] & 0x3F);
+ tm->tm_mon = bcd2bin(regs[PCF85053_REG_MO] & 0x1F) - 1;
+ tm->tm_year = bcd2bin(regs[PCF85053_REG_YR]) + 100;
+ }
+ tm->tm_wday = regs[PCF85053_REG_DW] & 0x07;
+
+ hr = regs[PCF85053_REG_HR];
+ is_24h = ctrl & PCF85053_BIT_HF;
+ is_bin = ctrl & PCF85053_BIT_DM;
+
+ if (is_24h) {
+ tm->tm_hour = is_bin
+ ? (hr & PCF85053_HR_24H_MASK)
+ : bcd2bin(hr & PCF85053_HR_24H_MASK);
+ } else {
+ h12 = is_bin ? (hr & PCF85053_HR_24H_MASK)
+ : bcd2bin(hr & PCF85053_HR_24H_MASK);
+
+ tm->tm_hour = (h12 == 12) ? ((hr & PCF85053_HR_PM) ? 12 : 0) :
+ ((hr & PCF85053_HR_PM) ? h12 + 12 : h12);
+ }
+
+ return 0;
+}
+
+static int pcf85053_rtc_set_time(struct device *dev, struct rtc_time *tm)
+
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int ctrl;
+ int err, ret;
+ u8 buf[10];
+
+ /*
+ * By default, secondary have write access to time registers as TWO
+ * bit is 0 by default, if we set nxp,interface = "primary" and the
+ * nxp,write-access in device tree, then TWO bits gets set and primary
+ * gets write access to time registers.
+ */
+ if (!pcf85053_time_write_access(pcf85053))
+ return -EACCES;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ /*
+ * HF = 1, 24-hour mode
+ * DM = 1, binary (not BCD)
+ */
+ if (!(ctrl & PCF85053_BIT_HF) ||
+ !(ctrl & PCF85053_BIT_DM)) {
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_HF | PCF85053_BIT_DM,
+ PCF85053_BIT_HF | PCF85053_BIT_DM);
+ if (err)
+ return err;
+ }
+
+ buf[0] = tm->tm_sec & 0x7F;
+ buf[1] = 0;
+ buf[2] = tm->tm_min & 0x7F;
+ buf[3] = 0;
+ buf[4] = tm->tm_hour & PCF85053_HR_24H_MASK;
+ buf[5] = 0;
+ buf[6] = tm->tm_wday & 0x07;
+ buf[7] = tm->tm_mday & 0x3F;
+ buf[8] = (tm->tm_mon + 1) & 0x1F;
+ buf[9] = (tm->tm_year - 100) & 0xFF;
+
+ if (pcf85053->is_primary) {
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_ST, PCF85053_BIT_ST);
+ if (err)
+ return err;
+
+ ret = regmap_bulk_write(pcf85053->regmap, PCF85053_REG_SC, buf, sizeof(buf));
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_ST, 0);
+ return ret ? ret : err;
+ }
+
+ return regmap_bulk_write(pcf85053->regmap, PCF85053_REG_SC, buf, sizeof(buf));
+}
+
+static int pcf85053_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *tm)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int ctrl, h12;
+ bool is_24h, is_bin, pm;
+ u8 buf[5];
+ u8 hr;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ err = regmap_bulk_read(pcf85053->regmap, PCF85053_REG_SCA, buf, sizeof(buf));
+ if (err)
+ return err;
+
+ if (ctrl & PCF85053_BIT_DM) {
+ tm->time.tm_sec = buf[0] & 0x7F; /* SCA */
+ tm->time.tm_min = buf[2] & 0x7F; /* MNA */
+ } else {
+ tm->time.tm_sec = bcd2bin(buf[0] & 0x7F);
+ tm->time.tm_min = bcd2bin(buf[2] & 0x7F);
+ }
+
+ hr = buf[4];
+ is_24h = !!(ctrl & PCF85053_BIT_HF);
+ is_bin = !!(ctrl & PCF85053_BIT_DM);
+
+ if (is_24h) {
+ tm->time.tm_hour = is_bin
+ ? (hr & PCF85053_HR_24H_MASK)
+ : bcd2bin(hr & PCF85053_HR_24H_MASK);
+ } else {
+ pm = !!(hr & PCF85053_HR_PM);
+
+ if (is_bin)
+ h12 = (hr & PCF85053_HR_24H_MASK);
+ else
+ h12 = (bcd2bin(hr & PCF85053_HR_24H_MASK));
+
+ if (h12 == 12)
+ h12 = 0;
+ tm->time.tm_hour = pm ? (h12 + 12) : h12;
+ }
+
+ return pcf85053_get_alarm_mode(dev, &tm->enabled, &tm->pending);
+}
+
+static int pcf85053_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int ctrl;
+ u8 sec, min, hr;
+ int err;
+
+ /*
+ * Only primary can set alarm, as secondary have read only access
+ * to alarm, control and status registers
+ */
+ if (!pcf85053->is_primary)
+ return -EACCES;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ /*
+ * HF = 1, 24-hour mode
+ * DM = 1, binary (not BCD)
+ */
+ if (!(ctrl & PCF85053_BIT_HF) ||
+ !(ctrl & PCF85053_BIT_DM)) {
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_HF | PCF85053_BIT_DM,
+ PCF85053_BIT_HF | PCF85053_BIT_DM);
+ if (err)
+ return err;
+ }
+
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_ST,
+ PCF85053_BIT_AF, 0);
+ if (err)
+ return err;
+
+ sec = tm->time.tm_sec & 0x7F;
+ min = tm->time.tm_min & 0x7F;
+ hr = tm->time.tm_hour & PCF85053_HR_24H_MASK;
+
+ err = regmap_write(pcf85053->regmap, PCF85053_REG_SCA, sec);
+ if (err)
+ return err;
+
+ err = regmap_write(pcf85053->regmap, PCF85053_REG_MNA, min);
+ if (err)
+ return err;
+
+ err = regmap_write(pcf85053->regmap, PCF85053_REG_HRA, hr);
+ if (err)
+ return err;
+
+ return pcf85053_set_alarm_mode(pcf85053->regmap, tm->enabled);
+}
+
+static int pcf85053_irq_enable(struct device *dev, unsigned int enabled)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "%s: alarm enable=%d\n", __func__, enabled);
+
+ return pcf85053_set_alarm_mode(pcf85053->regmap, enabled);
+}
+
+static int pcf85053_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
+{
+ struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
+ unsigned int val = 0, vl_status = 0;
+ unsigned int bvl;
+ int status;
+
+ switch (cmd) {
+ case RTC_VL_READ:
+ status = regmap_read(pcf85053->regmap, PCF85053_REG_ST, &val);
+ if (status)
+ return status;
+
+ if (val & PCF85053_BIT_OF)
+ vl_status |= RTC_VL_DATA_INVALID;
+
+ bvl = val & PCF85053A_BVL_MASK;
+
+ if (bvl == 0x00)
+ vl_status |= RTC_VL_BACKUP_EMPTY;
+ else if (bvl <= PCF85053A_BVL_LOW_THRESHOLD)
+ vl_status |= RTC_VL_BACKUP_LOW;
+
+ return put_user(vl_status, (unsigned int __user *)arg);
+
+ default:
+ return -ENOIOCTLCMD;
+ }
+}
+
+#ifdef CONFIG_COMMON_CLK
+/*
+ * Handling of the clkout
+ */
+
+#define clkout_hw_to_pcf85053(_hw) container_of(_hw, struct pcf85053, clkout_hw)
+
+static const int clkout_rates[] = {
+ 32768,
+ 1024,
+ 32,
+ 1,
+};
+
+static unsigned long pcf85053_clkout_recalc_rate(struct clk_hw *hw,
+ unsigned long parent_rate)
+{
+ struct pcf85053 *pcf85053 = clkout_hw_to_pcf85053(hw);
+ unsigned int val = 0;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CLKO, &val);
+ if (err)
+ return 0;
+
+ val &= PCF85053_REG_CLKO_F_MASK;
+ return clkout_rates[val];
+}
+
+static int pcf85053_clkout_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ int i;
+ unsigned long best = 0;
+
+ for (i = 0; i < ARRAY_SIZE(clkout_rates); i++) {
+ if (clkout_rates[i] <= req->rate) {
+ best = clkout_rates[i];
+ break;
+ }
+ }
+ if (!best)
+ best = clkout_rates[ARRAY_SIZE(clkout_rates) - 1];
+
+ req->rate = best;
+ return 0;
+}
+
+static int pcf85053_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct pcf85053 *pcf85053 = clkout_hw_to_pcf85053(hw);
+ unsigned int val = 0;
+ int err, i;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CLKO, &val);
+ if (err)
+ return err;
+
+ for (i = 0; i < ARRAY_SIZE(clkout_rates); i++)
+ if (clkout_rates[i] == rate) {
+ val &= ~PCF85053_REG_CLKO_F_MASK;
+ val |= i;
+ return regmap_write(pcf85053->regmap, PCF85053_REG_CLKO, val);
+ }
+
+ return -EINVAL;
+}
+
+static int pcf85053_clkout_control(struct clk_hw *hw, bool enable)
+{
+ struct pcf85053 *pcf85053 = clkout_hw_to_pcf85053(hw);
+ unsigned int val = 0;
+ int err;
+
+ if (!pcf85053->is_primary)
+ return -EACCES;
+
+ val = PCF85053_BIT_XCLK;
+ err = regmap_write(pcf85053->regmap, PCF85053_REG_ACC, val);
+ if (err)
+ return err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CLKO, &val);
+ if (err)
+ return err;
+
+ if (enable)
+ val |= PCF85053_REG_CLKO_CKE;
+ else
+ val &= ~PCF85053_REG_CLKO_CKE;
+
+ return regmap_write(pcf85053->regmap, PCF85053_REG_CLKO, val);
+}
+
+static int pcf85053_clkout_prepare(struct clk_hw *hw)
+{
+ return pcf85053_clkout_control(hw, 1);
+}
+
+static void pcf85053_clkout_unprepare(struct clk_hw *hw)
+{
+ pcf85053_clkout_control(hw, 0);
+}
+
+static int pcf85053_clkout_is_prepared(struct clk_hw *hw)
+{
+ struct pcf85053 *pcf85053 = clkout_hw_to_pcf85053(hw);
+ unsigned int val = 0;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CLKO, &val);
+ if (err)
+ return err;
+
+ return val & PCF85053_REG_CLKO_CKE;
+}
+
+static const struct clk_ops pcf85053_clkout_ops = {
+ .prepare = pcf85053_clkout_prepare,
+ .unprepare = pcf85053_clkout_unprepare,
+ .is_prepared = pcf85053_clkout_is_prepared,
+ .recalc_rate = pcf85053_clkout_recalc_rate,
+ .determine_rate = pcf85053_clkout_determine_rate,
+ .set_rate = pcf85053_clkout_set_rate,
+};
+
+static struct clk *pcf85053_clkout_register_clk(struct pcf85053 *pcf85053)
+{
+ struct device *dev = pcf85053->rtc->dev.parent;
+ struct device_node *node = dev->of_node;
+ struct clk *clk;
+ struct clk_init_data init;
+
+ init.name = "pcf85053-clkout";
+ init.ops = &pcf85053_clkout_ops;
+ init.flags = 0;
+ init.parent_names = NULL;
+ init.num_parents = 0;
+ pcf85053->clkout_hw.init = &init;
+
+ /* optional override of the clockname */
+ of_property_read_string(node, "clock-output-names", &init.name);
+
+ /* register the clock */
+ clk = devm_clk_register(dev, &pcf85053->clkout_hw);
+
+ if (!IS_ERR(clk))
+ of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+ return clk;
+}
+#endif
+
+static const struct rtc_class_ops pcf85053_rtc_ops = {
+ .read_time = pcf85053_rtc_read_time,
+ .set_time = pcf85053_rtc_set_time,
+ .read_alarm = pcf85053_rtc_read_alarm,
+ .set_alarm = pcf85053_rtc_set_alarm,
+ .alarm_irq_enable = pcf85053_irq_enable,
+ .ioctl = pcf85053_ioctl,
+};
+
+static const struct pcf85053_config config_pcf85053 = {
+ .regmap = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = 0x1D,
+ },
+ .has_alarms = 1,
+};
+
+static int pcf85053_probe(struct i2c_client *client)
+{
+ const struct pcf85053_config *config;
+ struct device *dev = &client->dev;
+ const char *iface = NULL;
+ struct pcf85053 *pcf85053;
+ int err;
+
+ if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C |
+ I2C_FUNC_SMBUS_BYTE |
+ I2C_FUNC_SMBUS_BLOCK_DATA))
+ return -ENODEV;
+
+ pcf85053 = devm_kzalloc(dev, sizeof(struct pcf85053),
+ GFP_KERNEL);
+ if (!pcf85053)
+ return -ENOMEM;
+
+ config = i2c_get_match_data(client);
+ if (!config)
+ return -ENODEV;
+
+ pcf85053->regmap = devm_regmap_init_i2c(client, &config->regmap);
+ if (IS_ERR(pcf85053->regmap))
+ return PTR_ERR(pcf85053->regmap);
+
+ dev_set_drvdata(dev, pcf85053);
+
+ pcf85053->is_primary = true;
+
+ if (of_property_read_string(dev->of_node, "nxp,interface", &iface))
+ return dev_err_probe(dev, -EINVAL,
+ "Missing mandatory property: nxp,interface\n");
+ if (!strcmp(iface, "primary"))
+ pcf85053->is_primary = true;
+ else if (!strcmp(iface, "secondary"))
+ pcf85053->is_primary = false;
+ else
+ return dev_err_probe(dev, -EINVAL,
+ "Invalid value for nxp,interface: %s\n", iface);
+
+ if (pcf85053->is_primary) {
+ unsigned int ctrl;
+ int err;
+
+ err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
+ if (err)
+ return err;
+
+ if (of_property_read_bool(dev->of_node, "nxp,write-access")) {
+ if (!(ctrl & PCF85053_BIT_TWO)) {
+ err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
+ PCF85053_BIT_TWO, PCF85053_BIT_TWO);
+ if (err)
+ return err;
+ }
+ dev_dbg(dev, "Ownership set: TWO=1 (primary writes)\n");
+ } else {
+ /* TWO (Time Write Ownership) bit defaults to 0 (Secondary) */
+ dev_dbg(dev, "Default ownership set: TWO=0 (secondary writes)\n");
+ }
+ }
+
+ pcf85053->rtc = devm_rtc_allocate_device(dev);
+ if (IS_ERR(pcf85053->rtc))
+ return PTR_ERR(pcf85053->rtc);
+
+ /*
+ * The year register stores 00-99 and the RTC treats any value where
+ * (year % 4) == 0 as a leap year, with no special handling for the
+ * centuries. Mapping 00-99 to 2000-2099 keeps the leap-year logic
+ * correct over the whole usable range.
+ */
+ pcf85053->rtc->ops = &pcf85053_rtc_ops;
+ pcf85053->rtc->range_min = RTC_TIMESTAMP_BEGIN_2000;
+ pcf85053->rtc->range_max = RTC_TIMESTAMP_END_2099;
+ clear_bit(RTC_FEATURE_UPDATE_INTERRUPT, pcf85053->rtc->features);
+ clear_bit(RTC_FEATURE_ALARM, pcf85053->rtc->features);
+
+ if (config->has_alarms && client->irq > 0) {
+ err = devm_request_threaded_irq(dev, client->irq,
+ NULL, pcf85053_irq,
+ IRQF_ONESHOT | IRQF_TRIGGER_FALLING,
+ "pcf85053", dev);
+ if (err) {
+ dev_err(dev, "unable to request IRQ %d\n", client->irq);
+ } else {
+ set_bit(RTC_FEATURE_ALARM, pcf85053->rtc->features);
+ devm_device_init_wakeup(dev);
+ err = dev_pm_set_wake_irq(dev, client->irq);
+ if (err)
+ dev_err(dev, "failed to enable irq wake\n");
+ }
+ }
+
+#ifdef CONFIG_COMMON_CLK
+ /* register clk in common clk framework */
+ pcf85053_clkout_register_clk(pcf85053);
+#endif
+
+ return devm_rtc_register_device(pcf85053->rtc);
+}
+
+static const struct i2c_device_id pcf85053_id[] = {
+ { "pcf85053", .driver_data = (kernel_ulong_t)&config_pcf85053 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, pcf85053_id);
+
+static const struct of_device_id pcf85053_of_match[] = {
+ { .compatible = "nxp,pcf85053", .data = &config_pcf85053 },
+ {}
+};
+MODULE_DEVICE_TABLE(of, pcf85053_of_match);
+
+static struct i2c_driver pcf85053_driver = {
+ .driver = {
+ .name = "rtc-pcf85053",
+ .of_match_table = of_match_ptr(pcf85053_of_match),
+ },
+ .probe = pcf85053_probe,
+ .id_table = pcf85053_id,
+};
+
+module_i2c_driver(pcf85053_driver);
+
+MODULE_AUTHOR("Pankit Garg <pankit.garg@nxp.com>");
+MODULE_AUTHOR("Lakshay Piplani <lakshay.piplani@nxp.com>");
+MODULE_DESCRIPTION("NXP pcf85053 RTC driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support
2026-08-12 7:35 [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support Lakshay Piplani
2026-08-12 7:35 ` [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support Lakshay Piplani
@ 2026-08-12 7:49 ` sashiko-bot
1 sibling, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-12 7:49 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: devicetree, conor+dt, robh, linux-rtc, Alexandre Belloni
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The schema incorrectly uses `additionalProperties: false` instead of `unevaluatedProperties: false` while inheriting from `rtc.yaml#`, which improperly rejects standard RTC properties.
- [Low] The description for `nxp,interface` contains a typo ('ready only') that alters the meaning of the sentence.
--
--- Patch [1]: [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support ---
commit c4402003b175003ee64d2fbdc100051ae1f80c64
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
dt-bindings: rtc: Add pcf85053 support
This commit adds device tree bindings for the NXP PCF85053 RTC chip,
introducing a new schema file and defining properties like nxp,interface
and nxp,write-access.
> diff --git a/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/nxp,pcf85053.yaml
[ ... ]
> + nxp,interface:
> + $ref: /schemas/types.yaml#/definitions/string
> + enum: [ primary, secondary ]
> + description: |
[ ... ]
> + The actual role depends on whether 'nxp,write-access' is also specified.
> + Supported configurations are:-
> + 1. Primary with 'nxp,write-access' -> primary claims write ownership.
> + 2. Primary without 'nxp,write-access' -> primary is ready only; secondary may write.
[Severity: Low]
This isn't a bug, but is "ready only" a typo for "read only" in this
description?
> + 3. Secondary (must not specify 'nxp,write-access') -> Secondary writes only
> + when no primary claims ownership.
[ ... ]
> +required:
> + - compatible
> + - reg
> + - nxp,interface
> +
> +additionalProperties: false
> +
> +allOf:
> + - $ref: rtc.yaml#
[Severity: Medium]
Does using additionalProperties: false here incorrectly reject standard RTC
properties from rtc.yaml?
Since this schema includes an allOf reference to rtc.yaml, shouldn't this be
unevaluatedProperties: false so that properties like wakeup-source are
properly permitted during schema validation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812073552.3542956-1-lakshay.piplani@nxp.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support
2026-08-12 7:35 ` [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support Lakshay Piplani
@ 2026-08-12 7:52 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-12 7:52 UTC (permalink / raw)
To: Lakshay Piplani; +Cc: devicetree, Alexandre Belloni, conor+dt, linux-rtc, robh
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] The OF clock provider registered with `of_clk_add_provider()` is never unregistered, leading to a resource leak and use-after-free on driver unbind or probe failure.
- [High] `pcf85053_rtc_set_time()` unconditionally zeroes out the interleaved alarm registers, destroying any configured alarm when setting the time.
- [High] Forcing 24-hour and binary mode in `set_time` and `set_alarm` corrupts existing timekeeping and writes binary data to a BCD-configured secondary interface.
- [Medium] The Oscillator Fail (OF) bit is never cleared after setting a valid time, permanently locking the RTC in an invalid state after a power loss.
- [Medium] `pcf85053_rtc_read_time()` reads the status register but ignores the result, failing to check the Oscillator Fail (OF) bit for invalid time.
--
--- Patch [2]: [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support ---
commit 9c8ab988d6c43472440d6251d1c52f7cbe264dbb
Author: Lakshay Piplani <lakshay.piplani@nxp.com>
rtc: Add NXP PCF85053 driver support
This commit adds support for the NXP PCF85053 I2C RTC, which supports timer
and calendar functionality. It introduces the driver code, Kconfig options,
and Makefile updates.
> diff --git a/drivers/rtc/rtc-pcf85053.c b/drivers/rtc/rtc-pcf85053.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/rtc/rtc-pcf85053.c
[ ... ]
> +static int pcf85053_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> + struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
> + unsigned int ctrl, st, h12;
> + bool is_24h, is_bin;
> + u8 regs[10], hr;
> + int err;
> +
> + err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
> + if (err)
> + return err;
> +
> + err = regmap_read(pcf85053->regmap, PCF85053_REG_ST, &st);
> + if (err)
> + return err;
> +
> + if (ctrl & PCF85053_BIT_ST)
> + return -EINVAL;
[Severity: Medium]
Is it intentional that the st variable is read but never evaluated here?
Because st is ignored, the driver doesn't check the PCF85053_BIT_OF flag.
Would this allow the system to boot and blindly use a corrupted time from a
failed oscillator instead of returning -EINVAL?
[ ... ]
> +static int pcf85053_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +
> +{
> + struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
> + unsigned int ctrl;
> + int err, ret;
> + u8 buf[10];
> +
> + /*
> + * By default, secondary have write access to time registers as TWO
> + * bit is 0 by default, if we set nxp,interface = "primary" and the
> + * nxp,write-access in device tree, then TWO bits gets set and primary
> + * gets write access to time registers.
> + */
> + if (!pcf85053_time_write_access(pcf85053))
> + return -EACCES;
> +
> + err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
> + if (err)
> + return err;
> +
> + /*
> + * HF = 1, 24-hour mode
> + * DM = 1, binary (not BCD)
> + */
> + if (!(ctrl & PCF85053_BIT_HF) ||
> + !(ctrl & PCF85053_BIT_DM)) {
> + err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
> + PCF85053_BIT_HF | PCF85053_BIT_DM,
> + PCF85053_BIT_HF | PCF85053_BIT_DM);
> + if (err)
> + return err;
> + }
[Severity: High]
Does forcing 24-hour and binary mode here corrupt existing timekeeping?
If the hardware is currently configured in BCD mode, updating the control
register without converting the current time registers would instantly cause
the hardware to interpret the existing BCD values as binary.
Additionally, since the secondary interface has read-only access to control
registers, this regmap_update_bits() call will be ignored by the hardware when
invoked from a secondary interface. However, the driver will still proceed to
write binary-formatted time values. Could this break the clock by writing
binary values that the hardware will interpret as BCD?
> +
> + buf[0] = tm->tm_sec & 0x7F;
> + buf[1] = 0;
> + buf[2] = tm->tm_min & 0x7F;
> + buf[3] = 0;
> + buf[4] = tm->tm_hour & PCF85053_HR_24H_MASK;
> + buf[5] = 0;
> + buf[6] = tm->tm_wday & 0x07;
> + buf[7] = tm->tm_mday & 0x3F;
> + buf[8] = (tm->tm_mon + 1) & 0x1F;
> + buf[9] = (tm->tm_year - 100) & 0xFF;
> +
> + if (pcf85053->is_primary) {
> + err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
> + PCF85053_BIT_ST, PCF85053_BIT_ST);
> + if (err)
> + return err;
> +
> + ret = regmap_bulk_write(pcf85053->regmap, PCF85053_REG_SC, buf, sizeof(buf));
> + err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
> + PCF85053_BIT_ST, 0);
> + return ret ? ret : err;
> + }
> +
> + return regmap_bulk_write(pcf85053->regmap, PCF85053_REG_SC, buf, sizeof(buf));
> +}
[Severity: High]
Does this bulk write unconditionally destroy any configured alarms?
The hardware interleaves time and alarm registers. By hardcoding 0 at the
alarm register indices (buf[1], buf[3], buf[5]) and doing a 10-byte bulk
write, calling set_time will clobber any pending alarm previously configured.
[ ... ]
> +static int pcf85053_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *tm)
> +{
> + struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
> + unsigned int ctrl;
> + u8 sec, min, hr;
> + int err;
> +
> + /*
> + * Only primary can set alarm, as secondary have read only access
> + * to alarm, control and status registers
> + */
> + if (!pcf85053->is_primary)
> + return -EACCES;
> +
> + err = regmap_read(pcf85053->regmap, PCF85053_REG_CTRL, &ctrl);
> + if (err)
> + return err;
> +
> + /*
> + * HF = 1, 24-hour mode
> + * DM = 1, binary (not BCD)
> + */
> + if (!(ctrl & PCF85053_BIT_HF) ||
> + !(ctrl & PCF85053_BIT_DM)) {
> + err = regmap_update_bits(pcf85053->regmap, PCF85053_REG_CTRL,
> + PCF85053_BIT_HF | PCF85053_BIT_DM,
> + PCF85053_BIT_HF | PCF85053_BIT_DM);
> + if (err)
> + return err;
> + }
[Severity: High]
Just like in pcf85053_rtc_set_time(), does flipping the mode bits here without
updating the time registers cause the hardware to incorrectly interpret existing
time data?
[ ... ]
> +static int pcf85053_ioctl(struct device *dev, unsigned int cmd, unsigned long arg)
> +{
> + struct pcf85053 *pcf85053 = dev_get_drvdata(dev);
> + unsigned int val = 0, vl_status = 0;
> + unsigned int bvl;
> + int status;
> +
> + switch (cmd) {
> + case RTC_VL_READ:
> + status = regmap_read(pcf85053->regmap, PCF85053_REG_ST, &val);
> + if (status)
> + return status;
> +
> + if (val & PCF85053_BIT_OF)
> + vl_status |= RTC_VL_DATA_INVALID;
[Severity: Medium]
Is there a missing mechanism to clear the Oscillator Fail (OF) bit?
The OF bit is reported here, but neither pcf85053_rtc_set_time() nor the ioctl
handler clears it. Since the hardware does not auto-clear this bit, will the
RTC permanently report invalid data to userspace after a power loss?
[ ... ]
> +static struct clk *pcf85053_clkout_register_clk(struct pcf85053 *pcf85053)
> +{
> + struct device *dev = pcf85053->rtc->dev.parent;
> + struct device_node *node = dev->of_node;
> + struct clk *clk;
> + struct clk_init_data init;
> +
> + init.name = "pcf85053-clkout";
> + init.ops = &pcf85053_clkout_ops;
> + init.flags = 0;
> + init.parent_names = NULL;
> + init.num_parents = 0;
> + pcf85053->clkout_hw.init = &init;
> +
> + /* optional override of the clockname */
> + of_property_read_string(node, "clock-output-names", &init.name);
> +
> + /* register the clock */
> + clk = devm_clk_register(dev, &pcf85053->clkout_hw);
> +
> + if (!IS_ERR(clk))
> + of_clk_add_provider(node, of_clk_src_simple_get, clk);
> +
> + return clk;
> +}
[Severity: High]
Does this leak the OF clock provider?
The of_clk_add_provider() is called to register the clock, but there is no
corresponding teardown mechanism (like a devm action or a remove callback
calling of_clk_del_provider()).
If the driver is unbound or if devm_rtc_register_device() fails in
pcf85053_probe(), the clock hardware structure is freed by devres, leaving a
dangling pointer in the global of_clk_providers list. Any subsequent device
tree clock resolution could access freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812073552.3542956-1-lakshay.piplani@nxp.com?part=2
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-12 7:52 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 7:35 [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support Lakshay Piplani
2026-08-12 7:35 ` [RESEND v7 2/2] rtc: Add NXP PCF85053 driver support Lakshay Piplani
2026-08-12 7:52 ` sashiko-bot
2026-08-12 7:49 ` [RESEND v7 1/2] dt-bindings: rtc: Add pcf85053 support sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.