[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFXKEHZn0XQMe6RBHDJzcGZy+JPpNpfidD1mT2MBmZ_WamFQKQ@mail.gmail.com>
Date: Tue, 5 Aug 2025 23:56:46 +0200
From: Lothar Rubusch <l.rubusch@...il.com>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
Cc: michal.simek@....com, srinivas.neeli@...inx.com, linux-rtc@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
Ivan Vera <ivan.vera@...lustra.com>
Subject: Re: [PATCH v1 1/1] rtc: zynqmp: ensure correct RTC calibration
On Mon, Aug 4, 2025 at 11:32 PM Alexandre Belloni
<alexandre.belloni@...tlin.com> wrote:
>
> On 04/08/2025 15:47:50+0000, Lothar Rubusch wrote:
> > From: Ivan Vera <ivan.vera@...lustra.com>
(...)
> > diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> > index f39102b66eac..0c063c3fae52 100644
> > --- a/drivers/rtc/rtc-zynqmp.c
> > +++ b/drivers/rtc/rtc-zynqmp.c
> > @@ -331,9 +331,9 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
> > if (ret)
> > xrtcdev->freq = RTC_CALIB_DEF;
> > }
> > - ret = readl(xrtcdev->reg_base + RTC_CALIB_RD);
> > - if (!ret)
> > - writel(xrtcdev->freq, (xrtcdev->reg_base + RTC_CALIB_WR));
> > +
> > + /* Enable unconditional re-calibration to RTC_CALIB_DEF or DTB entry. */
> > + writel(xrtcdev->freq, xrtcdev->reg_base + RTC_CALIB_WR);
>
> Doesn't this forcefully overwrite the proper value that has been set
> from userspace and so trashes the time at each reboot?
>
Yes, it overwrites the calibration, i.e. counting 1sec in about 1sec.
No, the time/date is not actually "trashed" (I double-checked that
with timesyncd disabled, having and not having register content and
over several reboots keeping a bogus date/time - it psersistet in the
same time space. The current patch always overwrites the calib
register content. So, a manual userspace setting will be lost after
reboot. That's true.
Would it rather make sense to extend it, say, instead of merely
checking whether the calibration register contains any data - which
could potentially be incorrect - also check for the presence of a
calibration property in the devicetree (or a similar property, since
'calibration' may be deprecated)? If such a property exists, perform a
re-calibration based on the devicetree at every reboot. Otherwise,
retain the current behavior of checking whether the register is empty?
> Isn't the proper way to reset it to simply set the offset from userspace
> again?
>
Hm.. I'm unsure if I understood you correctly. You mean the way as
described in AMD's link to perform the reset by executing the devmem
from Linux manually? If so, why is it preferable to adjust the RTC
calibration manually every time this happens, rather than to simply
put a correction value into the devicetree properties for problematic
setups? Or do I miss something, is there a config file for RTC
calibration for doing this persistently from Linux, that I'm not aware
of?
Before, the devicetree properties seemed to have generally priority
over userspace settings. Now, after the calibration rework, this
priorization seems to have changed and a devicetree calib correction
for such problematic cases will generally be ignored, with a
recommendation by Xilinx/AMD (see link in cover letter) to execute a
devmem command from off Linux (...). I mean, can't this be elaborated
a bit more to allow for a persistent correction method?
Just, let me know what you think about. Thank you for the feedback.
Best,
L
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Powered by blists - more mailing lists