| Message ID | 20240212-iio-improve-define-dont-build-v1-1-875883bb9565@analog.com (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | iio: core: make use of DIV_ROUND_CLOSEST_ULL() | expand |
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 "+"? > > /** > * IIO_G_TO_M_S_2() - Convert g to meter / second**2 > @@ -824,6 +825,6 @@ int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer, > * > * Returns the given value converted from meter / second**2 to g > */ > -#define IIO_M_S_2_TO_G(ms2) (((ms2) * 100000ULL + 980665ULL / 2) / 980665ULL) > +#define IIO_M_S_2_TO_G(ms2) DIV_ROUND_CLOSEST_ULL((ms2) * 100000ULL, 980665) > > #endif /* _INDUSTRIAL_IO_H_ */ > > --- > base-commit: bd2f1ed8873d4bbb2798151bbe28c86565251cfb > change-id: 20240212-iio-improve-define-dont-build-c0f9df006f85 > -- > > Thanks! > - Nuno Sá > >
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 - Nuno Sá > >
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á
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) /** * IIO_G_TO_M_S_2() - Convert g to meter / second**2 @@ -824,6 +825,6 @@ int iio_str_to_fixpoint(const char *str, int fract_mult, int *integer, * * Returns the given value converted from meter / second**2 to g */ -#define IIO_M_S_2_TO_G(ms2) (((ms2) * 100000ULL + 980665ULL / 2) / 980665ULL) +#define IIO_M_S_2_TO_G(ms2) DIV_ROUND_CLOSEST_ULL((ms2) * 100000ULL, 980665) #endif /* _INDUSTRIAL_IO_H_ */