[Public] Hi @Tomas Melin
> From: Tomas Melin <[email protected]> > Sent: Wednesday, January 7, 2026 6:17 PM > To: Vinay Tilak, Pranav <[email protected]>; [email protected] > Cc: git (AMD-Xilinx) <[email protected]>; Simek, Michal > <[email protected]> > Subject: Re: [PATCH] rtc: zynqmp: Add clock framework support with > calibration fallback > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Hi, > > On 06/01/2026 12:12, Pranav Tilak wrote: > > [Some people who received this message don't often get email from > > [email protected]. Learn why this is important at > > https://aka.ms/LearnAboutSenderIdentification ] > > > > Add support for reading RTC clock from device tree using clock > > framework also update the default calibration value to 0x7FFF as per > > RTC specifications. > > Falls back to 'calibration' property if clock unavailable, and uses > > default calibration if neither is present. Only writes calibration > > when hardware register reads zero. > > > > Signed-off-by: Pranav Tilak <[email protected]> > > --- > > drivers/rtc/zynqmp_rtc.c | 51 > > ++++++++++++++++++++++++---------------- > > 1 file changed, 31 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/rtc/zynqmp_rtc.c b/drivers/rtc/zynqmp_rtc.c index > > 15122a04838..83d66aa1aaa 100644 > > --- a/drivers/rtc/zynqmp_rtc.c > > +++ b/drivers/rtc/zynqmp_rtc.c > > @@ -8,23 +8,27 @@ > > #include <dm.h> > > #include <rtc.h> > > #include <asm/io.h> > > +#include <clk.h> > > +#include <dm/device_compat.h> > > > > /* RTC Registers */ > > #define RTC_SET_TM_WR 0x00 > > #define RTC_SET_TM_RD 0x04 > > #define RTC_CALIB_WR 0x08 > > +#define RTC_CALIB_RD 0x0C > > #define RTC_CUR_TM 0x10 > > #define RTC_INT_STS 0x20 > > #define RTC_CTRL 0x40 > > > > #define RTC_INT_SEC BIT(0) > > #define RTC_BATT_EN BIT(31) > > -#define RTC_CALIB_DEF 0x198233 > > +#define RTC_CALIB_DEF 0x7FFF > > +#define RTC_FREQ_MAX 0x10000 > > #define RTC_CALIB_MASK 0x1FFFFF > > > > struct zynqmp_rtc_priv { > > fdt_addr_t base; > > - unsigned int calibval; > > + unsigned long clk_freq; > > Using calibval as variable name might actually be better here as the value > corresponds to register value, and not the actual frequency. Ok. > > > }; > > > > static int zynqmp_rtc_get(struct udevice *dev, struct rtc_time *tm) > > @@ -70,13 +74,6 @@ static int zynqmp_rtc_set(struct udevice *dev, const > struct rtc_time *tm) > > */ > > new_time = rtc_mktime(tm) + 1; > > > > - /* > > - * Writing into calibration register will clear the Tick Counter and > > - * force the next second to be signaled exactly in 1 second period > > - */ > > - priv->calibval &= RTC_CALIB_MASK; > > - writel(priv->calibval, (priv->base + RTC_CALIB_WR)); > > - > > writel(new_time, priv->base + RTC_SET_TM_WR); > > > > /* > > @@ -107,15 +104,6 @@ static int zynqmp_rtc_init(struct udevice *dev) > > rtc_ctrl |= RTC_BATT_EN; > > writel(rtc_ctrl, priv->base + RTC_CTRL); > > > > - /* > > - * Based on crystal freq of 33.330 KHz > > - * set the seconds counter and enable, set fractions counter > > - * to default value suggested as per design spec > > - * to correct RTC delay in frequency over period of time. > > - */ > > - priv->calibval &= RTC_CALIB_MASK; > > - writel(priv->calibval, (priv->base + RTC_CALIB_WR)); > > - > > return 0; > > } > > > > @@ -128,8 +116,31 @@ static int zynqmp_rtc_probe(struct udevice *dev) > > if (priv->base == FDT_ADDR_T_NONE) > > return -EINVAL; > > > > - priv->calibval = dev_read_u32_default(dev, "calibration", > > - RTC_CALIB_DEF); > > + ret = readl(priv->base + RTC_CALIB_RD); > > + if (!ret) { > > + struct clk rtc_clk; > > + > > + /* Get the RTC clock rate */ > > + ret = clk_get_by_name_optional(dev, "rtc", &rtc_clk); > > + if (!ret) { > In case the rtc clock reference is missing, this will still return 0. I think > in this > case idea was to read the calibration devicetree value instead? Will update in v2 patch. > > > + priv->clk_freq = clk_get_rate(&rtc_clk); > > + /* Use clock frequency if valid, fallback to > > calibration value */ > > + if (priv->clk_freq > 0 && priv->clk_freq <= > > RTC_FREQ_MAX) { > > + priv->clk_freq = priv->clk_freq - 1; > > + } else { > > + dev_warn(dev, "Invalid frequency 0x%lx, > > using default to > 0x%x\n", > > + priv->clk_freq, > > + RTC_CALIB_DEF); > In case the frequency is invalid, that is a user error and it should IMHO be > treated as error, and bail out here. Will update in v2 patch. > > Thanks, > Tomas > > > + priv->clk_freq = RTC_CALIB_DEF; > > + } > > + } else { > > + priv->clk_freq = dev_read_u32_default(dev, > > "calibration", > > + > > RTC_CALIB_DEF); > > + } > > + > > + writel(priv->clk_freq, (priv->base + RTC_CALIB_WR)); > > + } else { > > + priv->clk_freq = ret & RTC_CALIB_MASK; > > + } > > > > ret = zynqmp_rtc_init(dev); > > > > -- > > 2.34.1 > > Regards, Pranav Tilak
