[PATCH v2] rtc: zynqmp: Enable crystal oscillator when setting time

[PATCH v2] rtc: zynqmp: Enable crystal oscillator when setting time

[Daniel Viaño]

The RTC_OSC_EN macro was defined when the driver was originally
introduced, but was never asserted by the driver. On cold boots, after
battery exhaustion, or on platforms where firmware does not configure
RTC_CTRL, the oscillator remains disabled and the counter never ticks.

Unconditionally enabling the oscillator during driver initialization
would cause the RTC to free-run from an uninitialized or stale counter
value, reporting an untrusted time to userspace. Instead, keep the
oscillator disabled until a valid time is programmed: guard
xlnx_rtc_read_time() on RTC_OSC_EN and return -EINVAL when it is not
set, and assert RTC_OSC_EN in xlnx_rtc_set_time() once valid time is
programmed so the RTC can free-run.

Per the Zynq UltraScale+ TRM (UG1085), bit 24 (OSC_CNTRL) is a static
level enable for the crystal inverter rather than an edge-triggered reset;
asserting it from set_time() is non-destructive and will not glitch an
already-running oscillator.

Tested on Zynq UltraScale+ hardware, confirming that read_time returns
-EINVAL while the oscillator is disabled, and advances reliably after
setting time.

Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Viaño <danividanivi@gmail.com>
---
v2:
 - Leave xlnx_init_rtc() unchanged so the oscillator is not enabled on probe
 - Return -EINVAL in xlnx_rtc_read_time() when RTC_OSC_EN is not set
 - Enable RTC_OSC_EN in xlnx_rtc_set_time() once valid time is programmed

 drivers/rtc/rtc-zynqmp.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 5bcb7536e973..6c3ba8f1aa57 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -59,6 +59,8 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 	unsigned long new_time;
 
+	u32 rtc_ctrl;
+
 	/*
 	 * The value written will be updated after 1 sec into the
 	 * seconds read register, so we need to program time +1 sec
@@ -78,6 +80,14 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	 */
 	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_STS);
 
+	/*
+	 * Now that the time is valid, start the crystal oscillator so the
+	 * RTC free-runs.
+	 */
+	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
+	rtc_ctrl |= RTC_OSC_EN;
+	writel(rtc_ctrl, xrtcdev->reg_base + RTC_CTRL);
+
 	return 0;
 }
 
@@ -87,6 +97,9 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	unsigned long read_time;
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 
+	if (!(readl(xrtcdev->reg_base + RTC_CTRL) & RTC_OSC_EN))
+		return -EINVAL;
+
 	status = readl(xrtcdev->reg_base + RTC_INT_STS);
 
 	if (status & RTC_INT_SEC) {
-- 
2.53.0

Re: [PATCH v2] rtc: zynqmp: Enable crystal oscillator when setting time

[T, Harini]

Hi,

On 9/25/2026 10:50 AM, Daniel Viaño wrote:
> [You don't often get email from danividanivi@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> The RTC_OSC_EN macro was defined when the driver was originally
> introduced, but was never asserted by the driver. On cold boots, after
> battery exhaustion, or on platforms where firmware does not configure
> RTC_CTRL, the oscillator remains disabled and the counter never ticks.
> 
> Unconditionally enabling the oscillator during driver initialization
> would cause the RTC to free-run from an uninitialized or stale counter
> value, reporting an untrusted time to userspace. Instead, keep the
> oscillator disabled until a valid time is programmed: guard
> xlnx_rtc_read_time() on RTC_OSC_EN and return -EINVAL when it is not
> set, and assert RTC_OSC_EN in xlnx_rtc_set_time() once valid time is
> programmed so the RTC can free-run.
> 
> Per the Zynq UltraScale+ TRM (UG1085), bit 24 (OSC_CNTRL) is a static
> level enable for the crystal inverter rather than an edge-triggered reset;
> asserting it from set_time() is non-destructive and will not glitch an
> already-running oscillator.
>

Thanks, but the premise doesn't hold: Osc_Cntrl resets to "enabled", not
disabled - ZynqMP reset 0x1 (bit 24 = 1), Versal reset 0x2 (crystal 
mode, bit 24 = 0). So after a cold boot / battery exhaustion the 
oscillator is already on and the counter ticks from ~0;

- ZynqMP: bit 24 is 1 out of reset and never cleared, so the read_time
-EINVAL check is unreachable



> Tested on Zynq UltraScale+ hardware, confirming that read_time returns
> -EINVAL while the oscillator is disabled, and advances reliably after
> setting time.

Also, how was the "oscillator disabled" state produced in test? With bit 
24 = 1 out of reset I don't see how read_time hits -EINVAL on a real 
cold boot.

Thanks,
Harini T

> 
> Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Viaño <danividanivi@gmail.com>
> ---
> v2:
>   - Leave xlnx_init_rtc() unchanged so the oscillator is not enabled on probe
>   - Return -EINVAL in xlnx_rtc_read_time() when RTC_OSC_EN is not set
>   - Enable RTC_OSC_EN in xlnx_rtc_set_time() once valid time is programmed
> 
>   drivers/rtc/rtc-zynqmp.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> index 5bcb7536e973..6c3ba8f1aa57 100644
> --- a/drivers/rtc/rtc-zynqmp.c
> +++ b/drivers/rtc/rtc-zynqmp.c
> @@ -59,6 +59,8 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
>          struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>          unsigned long new_time;
> 
> +       u32 rtc_ctrl;
> +
>          /*
>           * The value written will be updated after 1 sec into the
>           * seconds read register, so we need to program time +1 sec
> @@ -78,6 +80,14 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
>           */
>          writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_STS);
> 
> +       /*
> +        * Now that the time is valid, start the crystal oscillator so the
> +        * RTC free-runs.
> +        */
> +       rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> +       rtc_ctrl |= RTC_OSC_EN;
> +       writel(rtc_ctrl, xrtcdev->reg_base + RTC_CTRL);
> +
>          return 0;
>   }
> 
> @@ -87,6 +97,9 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
>          unsigned long read_time;
>          struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> 
> +       if (!(readl(xrtcdev->reg_base + RTC_CTRL) & RTC_OSC_EN))
> +               return -EINVAL;
> +
>          status = readl(xrtcdev->reg_base + RTC_INT_STS);
> 
>          if (status & RTC_INT_SEC) {
> --
> 2.53.0