linux-kernel - RE: [PATCH 2/4] rtc: zynqmp: rework read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <LV5PR12MB98047B0A754AFFFB01163E0992A3A@LV5PR12MB9804.namprd12.prod.outlook.com>
Date: Tue, 9 Dec 2025 17:28:47 +0000
From: "T, Harini" <Harini.T@....com>
To: Tomas Melin <tomas.melin@...sala.com>, Alexandre Belloni
	<alexandre.belloni@...tlin.com>, "Simek, Michal" <michal.simek@....com>
CC: "linux-rtc@...r.kernel.org" <linux-rtc@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/4] rtc: zynqmp: rework read_offset

[Public]

Hi,

> -----Original Message-----
> From: Tomas Melin <tomas.melin@...sala.com>
> Sent: Monday, December 1, 2025 6:20 PM
> To: Alexandre Belloni <alexandre.belloni@...tlin.com>; Simek, Michal
> <michal.simek@....com>
> Cc: linux-rtc@...r.kernel.org; linux-arm-kernel@...ts.infradead.org; linux-
> kernel@...r.kernel.org; Tomas Melin <tomas.melin@...sala.com>
> Subject: [PATCH 2/4] rtc: zynqmp: rework read_offset
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> read_offset() was using static frequency for determining the tick offset. It was
> also using remainder from do_div() operation as tick_mult value which
> caused the offset to be incorrect.
>
> At the same time, rework function to improve readability.
>
> Signed-off-by: Tomas Melin <tomas.melin@...sala.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c index
> 856bc1678e7d31144f320ae9f75fc58c742a2a64..7af5f6f99538f961a53ff56bfc6
> 56c907611b900 100644
> --- a/drivers/rtc/rtc-zynqmp.c
> +++ b/drivers/rtc/rtc-zynqmp.c
> @@ -178,21 +178,28 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> *xrtcdev)  static int xlnx_rtc_read_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;
> +       unsigned int calibval, fract_data, fract_part;
Prefer one variable assignment per line for readability.
> +       int max_tick, tick_mult;
It would be better to explain why tick_mult is changed to int in the commit message.
> +       int freq = xrtcdev->freq;
Please follow reverse xmas tree variable ordering.
>         long offset_val;
>
> +       /* ticks to reach RTC_PPB */
The comment is misleading. Its tick_mult is nanoseconds per tick, not a tick count.
> +       tick_mult = DIV_ROUND_CLOSEST(RTC_PPB, freq);
> +
>         calibval = readl(xrtcdev->reg_base + RTC_CALIB_RD);
>         /* Offset with seconds ticks */
> -       offset_val = calibval & RTC_TICK_MASK;
> -       offset_val = offset_val - RTC_CALIB_DEF;
> -       offset_val = offset_val * tick_mult;
> +       max_tick = calibval & RTC_TICK_MASK;
> +       offset_val = max_tick - freq;
> +       /* Convert to ppb */
> +       offset_val *= tick_mult;
>
>         /* Offset with fractional ticks */
> -       if (calibval & RTC_FR_EN)
> -               offset_val += ((calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT)
> -                       * (tick_mult / RTC_FR_MAX_TICKS);
> +       if (calibval & RTC_FR_EN) {
> +               fract_data = (calibval & RTC_FR_MASK) >> RTC_FR_DATSHIFT;
> +               fract_part = DIV_ROUND_UP(tick_mult, RTC_FR_MAX_TICKS);
> +               offset_val += (fract_part * fract_data);
> +       }
> +
>         *offset = offset_val;
>
>         return 0;
>
> --
> 2.47.3
>
Regards,
Harini T