linux-kernel - Re: [PATCH v4 1/3] rtc: tps6594: Add driver for TPS6594 RTC

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]

Date:   Wed, 17 May 2023 19:52:06 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Esteban Blanc <eblanc@...libre.com>
Cc:     linus.walleij@...aro.org, lgirdwood@...il.com, broonie@...nel.org,
        a.zummo@...ertech.it, alexandre.belloni@...tlin.com,
        linux-kernel@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-rtc@...r.kernel.org, jpanis@...libre.com,
        jneanne@...libre.com, aseketeli@...libre.com, sterzik@...com,
        u-kumar1@...com
Subject: Re: [PATCH v4 1/3] rtc: tps6594: Add driver for TPS6594 RTC

On Wed, May 17, 2023 at 7:47 PM Esteban Blanc <eblanc@...libre.com> wrote:
> On Fri May 12, 2023 at 7:22 PM CEST,  wrote:
> > Fri, May 12, 2023 at 04:17:53PM +0200, Esteban Blanc kirjoitti:

...

> > > +/* Multiplier for ppb conversions */
> > > +#define PPB_MULT (1000000000LL)
> >
> > We have something in units.h. Can you use generic macro?
>
> I found GIGA, NANO and NANOHZ_PER_HZ that have the same value in
> units.h. However I'm not sure any of them have the correct meaning in
> this situation.

MULT[IPLIER] has no units AFAIU, so SI macro can be used, no? NANO or
GIGA depends on what the actual sign of the exponent of the multiplier
is. Write it on paper and check the exponent in the equation(s) and
hence decide which one to use.

...

> > > +   if (tmp < 0)
> > > +           tmp -= TICKS_PER_HOUR / 2LL;
> > > +   else
> > > +           tmp += TICKS_PER_HOUR / 2LL;
> >
> > Is it guaranteed to have no overflow here?
>
> We know from `tps6594_rtc_set_offset` that the loaded value can't be
> more than 277774 (register default value is 0), So `tmp` can't exceed
> 277774000000000 which is lower than 2^63-1. No overflow here.
>
> TICK_PER_HOUR / 2LL = 117964800, so at the end of this computation,
> `tmp` can have a maximum value of 277774117964800 which is still
> inferior to 2^63-1.

Please add a respective comment.
-- 
With Best Regards,
Andy Shevchenko