| 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
| ||
|
Message-ID:
<LV5PR12MB98041CC1F14EC5C526C4C16092ABA@LV5PR12MB9804.namprd12.prod.outlook.com>
Date: Wed, 17 Dec 2025 18:33:00 +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 3/4] rtc: zynqmp: rework set_offset
[Public]
Hi,
> -----Original Message-----
> From: Tomas Melin <tomas.melin@...sala.com>
> Sent: Wednesday, December 10, 2025 5:48 PM
> To: T, Harini <Harini.T@....com>; 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
> Subject: Re: [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.
>
>
> Hi,
>
> On 09/12/2025 21:03, T, Harini wrote:
> > [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 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 <tomas.melin@...sala.com>
> >> ---
> >> 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..3bc8831ba2c4c4c701a495
> 06b6
> >> 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.
> >> 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.
>
> >>
> >> + /* 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.
> >> + 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.
>
> >> 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.
>
>
> >> 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.
The truncating division is acceptable as-is. Given the hardware's discrete
4-bit fractional field (16 levels), the quantization error from truncation
is negligible compared to the fundamental hardware precision limit. If you
want to optimize, DIV_ROUND_CLOSEST() would minimize average error, but it's
not required for correctness.
>
> Thanks,
> Tomas
>
> >> }
> >>
> >> /* 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
Regards,
Harini T
Powered by blists - more mailing lists