Re: [PATCH] iio: core: make use of DIV_ROUND_CLOSEST_ULL()

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, 2024-02-13 at 08:51 +0100, Nuno Sá wrote:
> On Mon, 2024-02-12 at 18:02 -0600, David Lechner wrote:
> > On Mon, Feb 12, 2024 at 3:07 AM Nuno Sa via B4 Relay
> > <devnull+nuno.sa.analog.com@kernel.org> wrote:
> > > 
> > > From: Nuno Sa <nuno.sa@analog.com>
> > > 
> > > Instead of open code DIV_ROUND_CLOSEST_ULL(), let's use it.
> > > 
> > > Signed-off-by: Nuno Sa <nuno.sa@analog.com>
> > > ---
> > > 1) For some reason IIO_G_TO_M_S_2() does do not a closest division. Not
> > >    sure if there's a reason for it or just something that was forgotten.
> > >    Anyways, I left it as it was before.
> > > 
> > > 2) This conversion could actually be required. In some experiments with
> > >    it (in a series I'm working on), I actually realized with
> > >    IIO_RAD_TO_DEGREE() that  we could have a 64bit division in 32bits
> > >    archs. I'm still not treating it as a fix as no one ever complained.
> > >    Jonathan, let me know if you want me to send a follow up email (or v2)
> > >    with a proper tag.
> > > ---
> > >  include/linux/iio/iio.h | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/include/linux/iio/iio.h b/include/linux/iio/iio.h
> > > index e370a7bb3300..3ebf9fe97f0e 100644
> > > --- a/include/linux/iio/iio.h
> > > +++ b/include/linux/iio/iio.h
> > > @@ -10,6 +10,7 @@
> > >  #include <linux/device.h>
> > >  #include <linux/cdev.h>
> > >  #include <linux/cleanup.h>
> > > +#include <linux/math.h>
> > >  #include <linux/slab.h>
> > >  #include <linux/iio/types.h>
> > >  /* IIO TODO LIST */
> > > @@ -799,8 +800,8 @@ int iio_str_to_fixpoint(const char *str, int
> > > fract_mult,
> > > int *integer,
> > >   *
> > >   * Returns the given value converted from degree to rad
> > >   */
> > > -#define IIO_DEGREE_TO_RAD(deg) (((deg) * 314159ULL + 9000000ULL) /
> > > 18000000ULL)
> > > -
> > > +#define IIO_DEGREE_TO_RAD(deg) \
> > > +       DIV_ROUND_CLOSEST_ULL((deg) * 314159ULL, 18000000)
> > >  /**
> > >   * IIO_RAD_TO_DEGREE() - Convert rad to degree
> > >   * @rad: A value in rad
> > > @@ -808,7 +809,7 @@ int iio_str_to_fixpoint(const char *str, int
> > > fract_mult,
> > > int *integer,
> > >   * Returns the given value converted from rad to degree
> > >   */
> > >  #define IIO_RAD_TO_DEGREE(rad) \
> > > -       (((rad) * 18000000ULL + 314159ULL / 2) / 314159ULL)
> > > +       DIV_ROUND_CLOSEST_ULL((rad) * 18000000ULL + 314159)
> > 
> > Is this supposed to be "," instead of "+"?
> 
> Definitely! Obviously I did some mistake preparing the final patch and out of
> confidence didn't even built it... Curious why clangd did not complained.
> Anyways, thanks! I'll send a v2
> 

Ups, actually lets forget about this... Most users of these macros use it at
file scope (if not all) which would bring us issues with DIV_ROUND_CLOSEST_ULL()
(no statements allowed at file scope).

- Nuno Sá