[PATCH] iio: test: fixed-point: new kunit test

[PATCH] iio: test: fixed-point: new kunit test

[David Lechner]

Add a kunit test for iio_str_to_fixpoint(). This function has an
unintuitive API so this is helpful to see how to use it and shows the
various edge cases.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
When reviewing [1], I noticed that iio_str_to_fixpoint() has an odd API
which lead me to find the bug in [2]. To make sure I was understanding
the API correctly, I wrote a KUnit test for it. So here it is.

[1]: https://lore.kernel.org/linux-iio/20251009173609.992452-3-flavra@baylibre.com/
[2]: https://lore.kernel.org/linux-iio/20251010-iio-adc-ad7280a-fix-ad7280_store_balance_timer-v1-1-e11746735192@baylibre.com/
---
Discussion unrelated to the patch:

I'm also a little tempted to introduce a new function that is a bit
easier to use. Many callers of iio_str_to_fixpoint_s64() are doing
something like int_part * 1000 + fract_part and ignoring the possibility
of negative values which require special handling.

static int iio_str_to_fixpoint_s64(const char *str, u32 decimal_places, s64 *value)
{
	int int_part, fract_part;
	int ret;

	ret = iio_str_to_fixpoint(str, int_pow(10, decimal_places - 1),
				  &int_part, &fract_part);
	if (ret)
		return ret;

	*value = (s64)int_part * int_pow(10, decimal_places) +
		 (int_part < 0 ? -1 : 1) * fract_part;

	return 0;
}
---
 drivers/iio/test/Kconfig                | 13 ++++++++
 drivers/iio/test/Makefile               |  1 +
 drivers/iio/test/iio-test-fixed-point.c | 58 +++++++++++++++++++++++++++++++++
 3 files changed, 72 insertions(+)

diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
index 6e65e929791ca247df9ac993fddbb4da557d5dfa..d210067ea59199d342b15bf373e724d0aa2c55a0 100644
--- a/drivers/iio/test/Kconfig
+++ b/drivers/iio/test/Kconfig
@@ -4,6 +4,19 @@
 #
 
 # Keep in alphabetical order
+
+config IIO_FIXED_POINT_KUNIT_TEST
+	tristate "Test IIO fixedpoint functions" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  Build unit tests for the IIO fixed-point code.
+
+	  For more information on KUnit and unit tests in general, please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config IIO_GTS_KUNIT_TEST
 	tristate "Test IIO gain-time-scale helpers" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
index 0c846bc21acda5a487b3a6977a8e9feaef20159a..a580e78b0fe00adc65f8f9df80023d6d738d11f7 100644
--- a/drivers/iio/test/Makefile
+++ b/drivers/iio/test/Makefile
@@ -4,6 +4,7 @@
 #
 
 # Keep in alphabetical order
+obj-$(CONFIG_IIO_FIXED_POINT_KUNIT_TEST) += iio-test-fixed-point.o
 obj-$(CONFIG_IIO_RESCALE_KUNIT_TEST) += iio-test-rescale.o
 obj-$(CONFIG_IIO_FORMAT_KUNIT_TEST) += iio-test-format.o
 obj-$(CONFIG_IIO_GTS_KUNIT_TEST) += iio-test-gts.o
diff --git a/drivers/iio/test/iio-test-fixed-point.c b/drivers/iio/test/iio-test-fixed-point.c
new file mode 100644
index 0000000000000000000000000000000000000000..edfed5434af2391a31d206fda2be0455a9e5e443
--- /dev/null
+++ b/drivers/iio/test/iio-test-fixed-point.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Unit tests for IIO fixed-point functions
+ *
+ * Copyright (c) 2025 BayLibre, SAS
+ */
+
+#include <kunit/test.h>
+#include <linux/iio/iio.h>
+#include <linux/math.h>
+#include <linux/types.h>
+
+static void iio_test_iio_str_to_fixed_point(struct kunit *test)
+{
+	int int_part, fract_part;
+	int ret;
+
+	/* Positive value > 1 */
+	ret = iio_str_to_fixpoint("1.234", 100, &int_part, &fract_part);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, 1234);
+
+	/* Truncates rather than rounding closest. */
+	ret = iio_str_to_fixpoint("1.234567", 100, &int_part, &fract_part);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, 1234);
+
+	/* Positive value < 1 */
+	ret = iio_str_to_fixpoint("0.001", 100, &int_part, &fract_part);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, 1);
+
+	/* Negative value > -1 */
+	ret = iio_str_to_fixpoint("-0.001", 100, &int_part, &fract_part);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, -1);
+
+	/* Negative value < -1 */
+	ret = iio_str_to_fixpoint("-1.001", 100, &int_part, &fract_part);
+	KUNIT_EXPECT_EQ(test, 0, ret);
+	/* The fractional part is subtracted rather than added in this case! */
+	KUNIT_EXPECT_EQ(test, int_part * 1000 - fract_part, -1001);
+}
+
+static const struct kunit_case iio_fixed_point_test_cases[] = {
+	KUNIT_CASE(iio_test_iio_str_to_fixed_point),
+	{ }
+};
+
+static struct kunit_suite iio_fixed_point_test_suite = {
+	.name = "iio-fixed-point",
+	.test_cases = iio_fixed_point_test_cases,
+};
+kunit_test_suite(iio_fixed_point_test_suite);
+
+MODULE_AUTHOR("David Lechner <dlechner@baylibre.com>");
+MODULE_DESCRIPTION("Test IIO fixed-point functions");
+MODULE_LICENSE("GPL");

---
base-commit: a9682f53c2d1678b93a123cdaa260e955430bc5c
change-id: 20251010-iio-tests-fixed-point-new-kunit-5a0d3eb19865

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

Re: [PATCH] iio: test: fixed-point: new kunit test

[Matti Vaittinen]

On 14/10/2025 00:33, David Lechner wrote:
> Add a kunit test for iio_str_to_fixpoint(). This function has an
> unintuitive API so this is helpful to see how to use it and shows the
> various edge cases.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
> When reviewing [1], I noticed that iio_str_to_fixpoint() has an odd API
> which lead me to find the bug in [2]. To make sure I was understanding
> the API correctly, I wrote a KUnit test for it. So here it is.
> 
> [1]: https://lore.kernel.org/linux-iio/20251009173609.992452-3-flavra@baylibre.com/
> [2]: https://lore.kernel.org/linux-iio/20251010-iio-adc-ad7280a-fix-ad7280_store_balance_timer-v1-1-e11746735192@baylibre.com/
> ---
> Discussion unrelated to the patch:
> 
> I'm also a little tempted to introduce a new function that is a bit
> easier to use. Many callers of iio_str_to_fixpoint_s64() are doing
> something like int_part * 1000 + fract_part and ignoring the possibility
> of negative values which require special handling.
> 
> static int iio_str_to_fixpoint_s64(const char *str, u32 decimal_places, s64 *value)
> {
> 	int int_part, fract_part;
> 	int ret;
> 
> 	ret = iio_str_to_fixpoint(str, int_pow(10, decimal_places - 1),
> 				  &int_part, &fract_part);
> 	if (ret)
> 		return ret;
> 
> 	*value = (s64)int_part * int_pow(10, decimal_places) +
> 		 (int_part < 0 ? -1 : 1) * fract_part;
> 
> 	return 0;
> }

FWIW: I like this.

> ---
>   drivers/iio/test/Kconfig                | 13 ++++++++
>   drivers/iio/test/Makefile               |  1 +
>   drivers/iio/test/iio-test-fixed-point.c | 58 +++++++++++++++++++++++++++++++++
>   3 files changed, 72 insertions(+)
> 
> diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
> index 6e65e929791ca247df9ac993fddbb4da557d5dfa..d210067ea59199d342b15bf373e724d0aa2c55a0 100644
> --- a/drivers/iio/test/Kconfig
> +++ b/drivers/iio/test/Kconfig
> @@ -4,6 +4,19 @@

// snip

> +static void iio_test_iio_str_to_fixed_point(struct kunit *test)
> +{
> +	int int_part, fract_part;
> +	int ret;
> +
> +	/* Positive value > 1 */
> +	ret = iio_str_to_fixpoint("1.234", 100, &int_part, &fract_part);
> +	KUNIT_EXPECT_EQ(test, 0, ret);
> +	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, 1234);

Do you think doing two checks:
	KUNIT_EXPECT_EQ(test, int_part, 1);
	KUNIT_EXPECT_EQ(test, fract_part, 234);

instead of the:
	KUNIT_EXPECT_EQ(test, int_part * 1000 + fract_part, 1234);

would work? For me seeing it all ass
	ret = iio_str_to_fixpoint("1.234", 100, &int_part, &fract_part);
	KUNIT_EXPECT_EQ(test, 0, ret);
	KUNIT_EXPECT_EQ(test, int_part, 1);
	KUNIT_EXPECT_EQ(test, fract_part, 234);

would be super clear - albeit also a line longer :)
(Same applies to the rest of the cases).

Well, it's not critical, so with or without that change:

Reviewed-by: Matti Vaittinen <mazziesaccount@gmail.com>

Thanks!

Yours,
	-- Matti

Re: [PATCH] iio: test: fixed-point: new kunit test

[kernel test robot]

Hi David,

kernel test robot noticed the following build warnings:

[auto build test WARNING on a9682f53c2d1678b93a123cdaa260e955430bc5c]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Lechner/iio-test-fixed-point-new-kunit-test/20251014-053648
base:   a9682f53c2d1678b93a123cdaa260e955430bc5c
patch link:    https://lore.kernel.org/r/20251013-iio-tests-fixed-point-new-kunit-v1-1-7b52021925e6%40baylibre.com
patch subject: [PATCH] iio: test: fixed-point: new kunit test
config: i386-buildonly-randconfig-001-20251015 (https://download.01.org/0day-ci/archive/20251015/202510151125.QEa3kUJR-lkp@intel.com/config)
compiler: gcc-13 (Debian 13.3.0-16) 13.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251015/202510151125.QEa3kUJR-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510151125.QEa3kUJR-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/iio/test/iio-test-fixed-point.c:52:23: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      52 |         .test_cases = iio_fixed_point_test_cases,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~


vim +/const +52 drivers/iio/test/iio-test-fixed-point.c

    49	
    50	static struct kunit_suite iio_fixed_point_test_suite = {
    51		.name = "iio-fixed-point",
  > 52		.test_cases = iio_fixed_point_test_cases,
    53	};
    54	kunit_test_suite(iio_fixed_point_test_suite);
    55	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] iio: test: fixed-point: new kunit test

[kernel test robot]

Hi David,

kernel test robot noticed the following build errors:

[auto build test ERROR on a9682f53c2d1678b93a123cdaa260e955430bc5c]

url:    https://github.com/intel-lab-lkp/linux/commits/David-Lechner/iio-test-fixed-point-new-kunit-test/20251014-053648
base:   a9682f53c2d1678b93a123cdaa260e955430bc5c
patch link:    https://lore.kernel.org/r/20251013-iio-tests-fixed-point-new-kunit-v1-1-7b52021925e6%40baylibre.com
patch subject: [PATCH] iio: test: fixed-point: new kunit test
config: i386-buildonly-randconfig-004-20251015 (https://download.01.org/0day-ci/archive/20251015/202510151318.gmNqcrgt-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251015/202510151318.gmNqcrgt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202510151318.gmNqcrgt-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/test/iio-test-fixed-point.c:52:16: error: initializing 'struct kunit_case *' with an expression of type 'const struct kunit_case[2]' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
      52 |         .test_cases = iio_fixed_point_test_cases,
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +52 drivers/iio/test/iio-test-fixed-point.c

    49	
    50	static struct kunit_suite iio_fixed_point_test_suite = {
    51		.name = "iio-fixed-point",
  > 52		.test_cases = iio_fixed_point_test_cases,
    53	};
    54	kunit_test_suite(iio_fixed_point_test_suite);
    55	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] iio: test: fixed-point: new kunit test

[Jonathan Cameron]

On Mon, 13 Oct 2025 16:33:43 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Add a kunit test for iio_str_to_fixpoint(). This function has an
> unintuitive API so this is helpful to see how to use it and shows the
> various edge cases.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Thanks for doing this. Looks good to me other than the
missing const the bots shouted about and whatever you and Matti conclude
on the test conditions.

Re: [PATCH] iio: test: fixed-point: new kunit test

[Andy Shevchenko]

On Mon, Oct 13, 2025 at 04:33:43PM -0500, David Lechner wrote:
> Add a kunit test for iio_str_to_fixpoint(). This function has an
> unintuitive API so this is helpful to see how to use it and shows the
> various edge cases.

...

> Discussion unrelated to the patch:
> 
> I'm also a little tempted to introduce a new function that is a bit
> easier to use. Many callers of iio_str_to_fixpoint_s64() are doing
> something like int_part * 1000 + fract_part and ignoring the possibility
> of negative values which require special handling.
> 
> static int iio_str_to_fixpoint_s64(const char *str, u32 decimal_places, s64 *value)
> {
> 	int int_part, fract_part;
> 	int ret;
> 
> 	ret = iio_str_to_fixpoint(str, int_pow(10, decimal_places - 1),
> 				  &int_part, &fract_part);
> 	if (ret)
> 		return ret;
> 
> 	*value = (s64)int_part * int_pow(10, decimal_places) +
> 		 (int_part < 0 ? -1 : 1) * fract_part;


Obviously if you go this path, the int_pow() can be called only once
(yes, we would need a multiplication or division for the other case).
The question is how we treat the decimal_places == 0 case.

> 	return 0;
> }


-- 
With Best Regards,
Andy Shevchenko