[PATCH v1 0/6] iio: Introduce and use value of π

[PATCH v1 0/6] iio: Introduce and use value of π

[Andy Shevchenko]

There are a few drivers that use value of π (small Greek PI)
of different precision. Instead of hard coding over and over
convert them to use a defined constant, which one of the patches
in this series introduces. No functional changes involved.

Note, the respective IIO macros are not converted as it might include unwanted
churn, and hard to use definitions in the initialisers.

Andy Shevchenko (6):
  media: vidtv: Rename PI definition to PI_SAMPLES
  units: Add value of π * 10⁹
  media: dvb-frontends: atbm8830: Convert to use PI definition
  iio: cros_ec_sensors: Convert to use PI definition
  iio: frequency: ad9523: Convert to use PI definition
  iio: position: iqs624-pos: Convert to use PI definition

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 4 ++--
 drivers/iio/frequency/ad9523.c                       | 5 +++--
 drivers/iio/position/iqs624-pos.c                    | 3 ++-
 drivers/media/dvb-frontends/atbm8830.c               | 5 +++--
 drivers/media/test-drivers/vidtv/vidtv_s302m.c       | 6 +++---
 include/linux/units.h                                | 3 +++
 6 files changed, 16 insertions(+), 10 deletions(-)

-- 
2.50.1

[PATCH v1 1/6] media: vidtv: Rename PI definition to PI_SAMPLES

[Andy Shevchenko]

The definition of PI in the driver is not the actual value in radians,
but rather degrees. Since we are going to have a value in radians
defined in a global header, rename this definition to avoid potential
collisions. No functional changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/test-drivers/vidtv/vidtv_s302m.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
index 9da18eac04b5..b3217d643b1e 100644
--- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
+++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
@@ -45,7 +45,7 @@
 #define FF_S302M_DEFAULT_PTS_OFFSET 100000
 
 /* Used by the tone generator: number of samples for PI */
-#define PI		180
+#define PI_SAMPLES		180
 
 static const u8 reverse[256] = {
 	/* from ffmpeg */
@@ -259,10 +259,10 @@ static u16 vidtv_s302m_get_sample(struct vidtv_encoder *e)
 		if (!ctx->last_tone)
 			return 0x8000;
 
-		pos = (2 * PI * ctx->note_offset * ctx->last_tone) / S302M_SAMPLING_RATE_HZ;
+		pos = (2 * PI_SAMPLES * ctx->note_offset * ctx->last_tone) / S302M_SAMPLING_RATE_HZ;
 		ctx->note_offset++;
 
-		return (fixp_sin32(pos % (2 * PI)) >> 16) + 0x8000;
+		return (fixp_sin32(pos % (2 * PI_SAMPLES)) >> 16) + 0x8000;
 	}
 
 	/* bug somewhere */
-- 
2.50.1

Re: [PATCH v1 1/6] media: vidtv: Rename PI definition to PI_SAMPLES

[Hans Verkuil]

On 27/10/2025 15:34, Andy Shevchenko wrote:
> The definition of PI in the driver is not the actual value in radians,
> but rather degrees. Since we are going to have a value in radians
> defined in a global header, rename this definition to avoid potential
> collisions. No functional changes.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Hans Verkuil <hverkuil+cisco@kernel.org>

Feel free to merge this through a non-media subsystem. If you want me to
take this as a separate media subsystem patch, then let me know.

Regards,

	Hans

> ---
>  drivers/media/test-drivers/vidtv/vidtv_s302m.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/test-drivers/vidtv/vidtv_s302m.c b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> index 9da18eac04b5..b3217d643b1e 100644
> --- a/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> +++ b/drivers/media/test-drivers/vidtv/vidtv_s302m.c
> @@ -45,7 +45,7 @@
>  #define FF_S302M_DEFAULT_PTS_OFFSET 100000
>  
>  /* Used by the tone generator: number of samples for PI */
> -#define PI		180
> +#define PI_SAMPLES		180
>  
>  static const u8 reverse[256] = {
>  	/* from ffmpeg */
> @@ -259,10 +259,10 @@ static u16 vidtv_s302m_get_sample(struct vidtv_encoder *e)
>  		if (!ctx->last_tone)
>  			return 0x8000;
>  
> -		pos = (2 * PI * ctx->note_offset * ctx->last_tone) / S302M_SAMPLING_RATE_HZ;
> +		pos = (2 * PI_SAMPLES * ctx->note_offset * ctx->last_tone) / S302M_SAMPLING_RATE_HZ;
>  		ctx->note_offset++;
>  
> -		return (fixp_sin32(pos % (2 * PI)) >> 16) + 0x8000;
> +		return (fixp_sin32(pos % (2 * PI_SAMPLES)) >> 16) + 0x8000;
>  	}
>  
>  	/* bug somewhere */

[PATCH v1 2/6] units: Add value of π * 10⁹

[Andy Shevchenko]

There are a few drivers that want to have this value, and at least one
known to come soon. Let's define a value for them.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/units.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/units.h b/include/linux/units.h
index f626e212d4ca..82bdc2041328 100644
--- a/include/linux/units.h
+++ b/include/linux/units.h
@@ -21,6 +21,9 @@
 #define PICO	1000000000000ULL
 #define FEMTO	1000000000000000ULL
 
+/* Value of π * 10⁹ */
+#define PI	3141592653LL
+
 /* Hz based multipliers */
 #define NANOHZ_PER_HZ		1000000000UL
 #define MICROHZ_PER_HZ		1000000UL
-- 
2.50.1

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[Jonathan Cameron]

On Mon, 27 Oct 2025 15:34:51 +0100
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> There are a few drivers that want to have this value, and at least one
> known to come soon. Let's define a value for them.

Is there any way we can make the x10^9 bit obvious in the naming?  Or do
something a bit nasty like defining a macro along the lines of

PI(scale)?
e.g. PI(NANO), PI(10000) 


> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/units.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/units.h b/include/linux/units.h
> index f626e212d4ca..82bdc2041328 100644
> --- a/include/linux/units.h
> +++ b/include/linux/units.h
> @@ -21,6 +21,9 @@
>  #define PICO	1000000000000ULL
>  #define FEMTO	1000000000000000ULL
>  
> +/* Value of π * 10⁹ */
> +#define PI	3141592653LL
> +
>  /* Hz based multipliers */
>  #define NANOHZ_PER_HZ		1000000000UL
>  #define MICROHZ_PER_HZ		1000000UL

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[David Lechner]

On 10/27/25 9:52 AM, Jonathan Cameron wrote:
> On Mon, 27 Oct 2025 15:34:51 +0100
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
>> There are a few drivers that want to have this value, and at least one
>> known to come soon. Let's define a value for them.
> 
> Is there any way we can make the x10^9 bit obvious in the naming?  Or do
> something a bit nasty like defining a macro along the lines of
> 
> PI(scale)?
> e.g. PI(NANO), PI(10000) 
> 
This was my first thought when looking at this as well.

Or something like PI_x10(6).

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[Andy Shevchenko]

On Mon, Oct 27, 2025 at 09:59:10AM -0500, David Lechner wrote:
> On 10/27/25 9:52 AM, Jonathan Cameron wrote:
> > On Mon, 27 Oct 2025 15:34:51 +0100
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> > 
> >> There are a few drivers that want to have this value, and at least one
> >> known to come soon. Let's define a value for them.
> > 
> > Is there any way we can make the x10^9 bit obvious in the naming?  Or do
> > something a bit nasty like defining a macro along the lines of
> > 
> > PI(scale)?
> > e.g. PI(NANO), PI(10000) 
> > 
> This was my first thought when looking at this as well.
> 
> Or something like PI_x10(6).

We need a good macro that may _at compile-time_ convert 64-bit input to 32-bit
value that may be suitable for 32-bit arithmetics / architectures.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[David Laight]

On Mon, 27 Oct 2025 15:34:51 +0100
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> There are a few drivers that want to have this value, and at least one
> known to come soon. Let's define a value for them.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  include/linux/units.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/include/linux/units.h b/include/linux/units.h
> index f626e212d4ca..82bdc2041328 100644
> --- a/include/linux/units.h
> +++ b/include/linux/units.h
> @@ -21,6 +21,9 @@
>  #define PICO	1000000000000ULL
>  #define FEMTO	1000000000000000ULL
>  
> +/* Value of π * 10⁹ */
> +#define PI	3141592653LL

Is that the right value?
IIRC the next digits are 58979 (I used to know the next few as well)
which means it should be rounded up.

	David

> +
>  /* Hz based multipliers */
>  #define NANOHZ_PER_HZ		1000000000UL
>  #define MICROHZ_PER_HZ		1000000UL

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[Andy Shevchenko]

On Mon, Oct 27, 2025 at 07:30:33PM +0000, David Laight wrote:
> On Mon, 27 Oct 2025 15:34:51 +0100
> Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> 
> > There are a few drivers that want to have this value, and at least one
> > known to come soon. Let's define a value for them.

> > +/* Value of π * 10⁹ */
> > +#define PI	3141592653LL
> 
> Is that the right value?
> IIRC the next digits are 58979 (I used to know the next few as well)
> which means it should be rounded up.

Right, today I have the same thought that actually ChromeOS driver has a
off-by-one issue there.

Btw, do you know if we can have compile-time divisions that can make 32-bit
constants out of the 64-bit input? DIV_ROUND_CLOSEST_ULL() doesn't seem allow
that.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 2/6] units: Add value of π * 10⁹

[David Laight]

On Tue, 28 Oct 2025 10:08:04 +0200
Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:

> On Mon, Oct 27, 2025 at 07:30:33PM +0000, David Laight wrote:
> > On Mon, 27 Oct 2025 15:34:51 +0100
> > Andy Shevchenko <andriy.shevchenko@linux.intel.com> wrote:
> >   
> > > There are a few drivers that want to have this value, and at least one
> > > known to come soon. Let's define a value for them.  
> 
> > > +/* Value of π * 10⁹ */

Thinks - should non-ascii characters be allowed in comments.
They can cause grief.

> > > +#define PI	3141592653LL  
> > 
> > Is that the right value?
> > IIRC the next digits are 58979 (I used to know the next few as well)
> > which means it should be rounded up.  
> 
> Right, today I have the same thought that actually ChromeOS driver has a
> off-by-one issue there.

Not as though it is likely to make a difference.
If any code needs anything more accurate than 3.1416 it probably cares
whether the value is rounded down or up.

> Btw, do you know if we can have compile-time divisions that can make 32-bit
> constants out of the 64-bit input? DIV_ROUND_CLOSEST_ULL() doesn't seem allow
> that.

Not sure, I did wonder how much thought had gone into the LL suffix.
The value used will fit in u32 - but then any maths becomes unsigned.
OTOH using LL forces 64bit maths on 32bit - not good.

I suspect it would be better to drop a digit so the value fits in a
signed 32bit variable.

The other issue is whether scaling by a power of 10 is even right.
If the code has to rescale the value using divides you really want
to divide by a big power of 2 not 10.
So multiplying by 2**29 may be more useful.

	David

[PATCH v1 3/6] media: dvb-frontends: atbm8830: Convert to use PI definition

[Andy Shevchenko]

Convert to use PI definition instead of open coded value of it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/media/dvb-frontends/atbm8830.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/dvb-frontends/atbm8830.c b/drivers/media/dvb-frontends/atbm8830.c
index 778c865085bf..74032364d183 100644
--- a/drivers/media/dvb-frontends/atbm8830.c
+++ b/drivers/media/dvb-frontends/atbm8830.c
@@ -6,7 +6,8 @@
  *    Copyright (C) 2009 David T.L. Wong <davidtlwong@gmail.com>
  */
 
-#include <asm/div64.h>
+#include <linux/math64.h>
+#include <linux/units.h>
 #include <media/dvb_frontend.h>
 
 #include "atbm8830.h"
@@ -112,7 +113,7 @@ static int set_if_freq(struct atbm_state *priv, u32 freq /*in kHz*/)
 
 	if (freq != 0) {
 		/* 2 * PI * (freq - fs) / fs * (2 ^ 22) */
-		t = (u64) 2 * 31416 * (freq - fs);
+		t = 2 * DIV_ROUND_UP_ULL(PI, 100000) * (freq - fs);
 		t <<= 22;
 		do_div(t, fs);
 		do_div(t, 1000);
-- 
2.50.1

Re: [PATCH v1 3/6] media: dvb-frontends: atbm8830: Convert to use PI definition

[Hans Verkuil]

On 27/10/2025 15:34, Andy Shevchenko wrote:
> Convert to use PI definition instead of open coded value of it.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Acked-by: Hans Verkuil <hverkuil+cisco@kernel.org>

Feel free to merge this through a non-media subsystem. If you want me to
take this as a separate media subsystem patch once units.h is patched,
then let me know.

Regards,

	Hans

> ---
>  drivers/media/dvb-frontends/atbm8830.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/dvb-frontends/atbm8830.c b/drivers/media/dvb-frontends/atbm8830.c
> index 778c865085bf..74032364d183 100644
> --- a/drivers/media/dvb-frontends/atbm8830.c
> +++ b/drivers/media/dvb-frontends/atbm8830.c
> @@ -6,7 +6,8 @@
>   *    Copyright (C) 2009 David T.L. Wong <davidtlwong@gmail.com>
>   */
>  
> -#include <asm/div64.h>
> +#include <linux/math64.h>
> +#include <linux/units.h>
>  #include <media/dvb_frontend.h>
>  
>  #include "atbm8830.h"
> @@ -112,7 +113,7 @@ static int set_if_freq(struct atbm_state *priv, u32 freq /*in kHz*/)
>  
>  	if (freq != 0) {
>  		/* 2 * PI * (freq - fs) / fs * (2 ^ 22) */
> -		t = (u64) 2 * 31416 * (freq - fs);
> +		t = 2 * DIV_ROUND_UP_ULL(PI, 100000) * (freq - fs);
>  		t <<= 22;
>  		do_div(t, fs);
>  		do_div(t, 1000);

[PATCH v1 4/6] iio: cros_ec_sensors: Convert to use PI definition

[Andy Shevchenko]

Convert to use PI definition instead of open coded value of it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 82cef4a12442..7e7b7f97454d 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -21,6 +21,7 @@
 #include <linux/platform_data/cros_ec_commands.h>
 #include <linux/platform_data/cros_ec_proto.h>
 #include <linux/platform_device.h>
+#include <linux/units.h>
 #include <linux/slab.h>
 
 #define CROS_EC_SENSORS_MAX_CHANNELS 4
@@ -120,8 +121,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
 			 * loss. Round to the nearest integer.
 			 */
 			*val = 0;
-			*val2 = div_s64(val64 * 3141592653ULL,
-					180 << (CROS_EC_SENSOR_BITS - 1));
+			*val2 = div_s64(val64 * PI, 180 << (CROS_EC_SENSOR_BITS - 1));
 			ret = IIO_VAL_INT_PLUS_NANO;
 			break;
 		case MOTIONSENSE_TYPE_MAG:
-- 
2.50.1

[PATCH v1 5/6] iio: frequency: ad9523: Convert to use PI definition

[Andy Shevchenko]

Convert to use PI definition instead of open coded value of it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/frequency/ad9523.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/frequency/ad9523.c b/drivers/iio/frequency/ad9523.c
index 63c485e9e44c..fab1671d9212 100644
--- a/drivers/iio/frequency/ad9523.c
+++ b/drivers/iio/frequency/ad9523.c
@@ -15,6 +15,7 @@
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/delay.h>
+#include <linux/units.h>
 
 #include <linux/iio/iio.h>
 #include <linux/iio/sysfs.h>
@@ -652,7 +653,7 @@ static int ad9523_read_raw(struct iio_dev *indio_dev,
 			AD9523_CLK_DIST_DIV_REV(ret);
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_PHASE:
-		code = (AD9523_CLK_DIST_DIV_PHASE_REV(ret) * 3141592) /
+		code = (AD9523_CLK_DIST_DIV_PHASE_REV(ret) * DIV_ROUND_UP_ULL(PI, 1000)) /
 			AD9523_CLK_DIST_DIV_REV(ret);
 		*val = code / 1000000;
 		*val2 = code % 1000000;
@@ -701,7 +702,7 @@ static int ad9523_write_raw(struct iio_dev *indio_dev,
 		break;
 	case IIO_CHAN_INFO_PHASE:
 		code = val * 1000000 + val2 % 1000000;
-		tmp = (code * AD9523_CLK_DIST_DIV_REV(ret)) / 3141592;
+		tmp = (code * AD9523_CLK_DIST_DIV_REV(reg)) / DIV_ROUND_UP_ULL(PI, 1000);
 		tmp = clamp(tmp, 0, 63);
 		reg &= ~AD9523_CLK_DIST_DIV_PHASE(~0);
 		reg |= AD9523_CLK_DIST_DIV_PHASE(tmp);
-- 
2.50.1

[PATCH v1 6/6] iio: position: iqs624-pos: Convert to use PI definition

[Andy Shevchenko]

Convert to use PI definition instead of open coded value of it.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/iio/position/iqs624-pos.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/position/iqs624-pos.c b/drivers/iio/position/iqs624-pos.c
index 8239239c6ee2..da432d1b515c 100644
--- a/drivers/iio/position/iqs624-pos.c
+++ b/drivers/iio/position/iqs624-pos.c
@@ -15,10 +15,11 @@
 #include <linux/notifier.h>
 #include <linux/platform_device.h>
 #include <linux/regmap.h>
+#include <linux/units.h>
 
 #define IQS624_POS_DEG_OUT			0x16
 
-#define IQS624_POS_SCALE1			(314159 / 180)
+#define IQS624_POS_SCALE1			DIV_ROUND_UP_ULL(PI, 1800000)
 #define IQS624_POS_SCALE2			100000
 
 struct iqs624_pos_private {
-- 
2.50.1