Hi,

On 09/12/2025 21:03, T, Harini wrote:

[Public]

Hi,

quoted

-----Original Message-----
From: Tomas Melin <redacted>
Sent: Monday, December 1, 2025 6:20 PM
To: Alexandre Belloni <alexandre.belloni@bootlin.com>; Simek, Michal
[off-list ref]
Cc: linux-rtc@vger.kernel.org; linux-arm-kernel@lists.infradead.org; linux-
kernel@vger.kernel.org; Tomas Melin [off-list ref]
Subject: [PATCH 3/4] rtc: zynqmp: rework set_offset

Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.


set_offset was using remainder of do_div as tick_mult which resulted in
wrong offset. Calibration value also assumed builtin calibration default.
Update fract_offset to correctly calculate the value for negative offset and
replace the for loop with division.

Signed-off-by: Tomas Melin <redacted>
---
 drivers/rtc/rtc-zynqmp.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c index

7af5f6f99538f961a53ff56bfc656c907611b900..3bc8831ba2c4c4c701a49506b6
7ae6174f3ade3d 100644

--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c

@@ -208,13 +208,13 @@ static int xlnx_rtc_read_offset(struct device *dev,

long *offset)  static int xlnx_rtc_set_offset(struct device *dev, long offset)  {
        struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
-       unsigned long long rtc_ppb = RTC_PPB;
-       unsigned int tick_mult = do_div(rtc_ppb, xrtcdev->freq);
+       unsigned int calibval, tick_mult, fract_part;

tick_mult is mentioned as int in previous patch and unsigned here. Justify the type in commit description.

quoted

        unsigned char fract_tick = 0;
-       unsigned int calibval;
-       short int  max_tick;
-       int fract_offset;
+       int freq = xrtcdev->freq;
+       int max_tick, fract_offset;

Please follow reverse xmas tree variable ordering.
Also keep the frac_* variables uniform in both set and read offset functions.

Agreed, I will use same name of variables and types in next version.

quoted

+       /* ticks to reach RTC_PPB */

The comment is misleading. Its tick_mult is nanoseconds per tick, not a tick count.

Answered in patch 2/4.

quoted

+       tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, xrtcdev->freq);

We can first validate offset and then calculate tick_mult to reduce CPU instructions incase of invalid inputs

In this patch it would in theory apply, but when looking at patch 4/4
You will notice that we need to first calculate the helpers so offset is
then performed as soon as possible.

quoted

        if (offset < RTC_MIN_OFFSET || offset > RTC_MAX_OFFSET)
                return -ERANGE;

@@ -223,29 +223,22 @@ static int xlnx_rtc_set_offset(struct device *dev,

long offset)

        /* Number fractional ticks for given offset */
        if (fract_offset) {
+               /* round up here so we stay below a full tick */
+               fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
                if (fract_offset < 0) {
-                       fract_offset = fract_offset + tick_mult;
+                       fract_offset += (fract_part * RTC_FR_MAX_TICKS);

It would be better to add comment to explain on the negative offset borrowing logic

I will add comment about this.

quoted

                        max_tick--;
                }
-               if (fract_offset > (tick_mult / RTC_FR_MAX_TICKS)) {
-                       for (fract_tick = 1; fract_tick < 16; fract_tick++) {
-                               if (fract_offset <=
-                                   (fract_tick *
-                                    (tick_mult / RTC_FR_MAX_TICKS)))
-                                       break;
-                       }
-               }
+               fract_tick = fract_offset / fract_part;

Its better to use DIV_ROUND_UP()

Please explain why, that would change the end result from what is is now.

Thanks,
Tomas

quoted

        }

        /* Zynqmp RTC uses second and fractional tick
         * counters for compensation
         */
-       calibval = max_tick + RTC_CALIB_DEF;
+       calibval = max_tick + freq;

        if (fract_tick)
-               calibval |= RTC_FR_EN;
-
-       calibval |= (fract_tick << RTC_FR_DATSHIFT);
+               calibval |= (RTC_FR_EN | (fract_tick <<
+ RTC_FR_DATSHIFT));

        writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));


--
2.47.3

Regards,
Harini T