Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date:   Mon, 22 May 2023 13:49:58 +0200
Message-Id: <CSSS892QYLWK.3T71MRNHOH0IZ@burritosblues>
Cc:     <linus.walleij@linaro.org>, <lgirdwood@gmail.com>,
        <broonie@kernel.org>, <a.zummo@towertech.it>,
        <alexandre.belloni@bootlin.com>, <linux-kernel@vger.kernel.org>,
        <linux-gpio@vger.kernel.org>, <linux-rtc@vger.kernel.org>,
        <jpanis@baylibre.com>, <jneanne@baylibre.com>,
        <aseketeli@baylibre.com>, <sterzik@ti.com>, <u-kumar1@ti.com>
Subject: Re: [PATCH v4 1/3] rtc: tps6594: Add driver for TPS6594 RTC
From:   "Esteban Blanc" <eblanc@baylibre.com>
To:     "Andy Shevchenko" <andy.shevchenko@gmail.com>
References: <20230512141755.1712358-1-eblanc@baylibre.com>
 <20230512141755.1712358-2-eblanc@baylibre.com>
 <ZF514wvUt_xrU1gG@surfacebook> <CSOPFJOVLSS1.1XKI60F9TLBMN@burritosblues>
 <CAHp75Vfg+yAhJ58qt76GHqxWHD48hF-6ZnT=xEUh+1rzr7UvVg@mail.gmail.com>
In-Reply-To: <CAHp75Vfg+yAhJ58qt76GHqxWHD48hF-6ZnT=xEUh+1rzr7UvVg@mail.gmail.com>
Precedence: bulk

On Wed May 17, 2023 at 6:52 PM CEST, Andy Shevchenko wrote:
> On Wed, May 17, 2023 at 7:47=E2=80=AFPM Esteban Blanc <eblanc@baylibre.co=
m> 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.

Thanks. I've checked and it should be NANO.

> > > > +   if (tmp < 0)
> > > > +           tmp -=3D TICKS_PER_HOUR / 2LL;
> > > > +   else
> > > > +           tmp +=3D 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 =3D 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.

I've reformatted this and put it in a SAFETY comment.

Thanks for your help,

--=20
Esteban Blanc
BayLibre