All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] iio: iio_format_value(): Use signed temporary for IIO_VAL_FRACTIONAL_LOG2
@ 2020-12-15 19:17 Lars-Peter Clausen
  2020-12-15 19:17 ` [PATCH 2/3] iio: iio_format_value(): Fix IIO_VAL_FRACTIONAL_LOG2 values between -1.0 and 0.0 Lars-Peter Clausen
  2020-12-15 19:17 ` [PATCH 3/3] iio: Add basic unit test for iio_format_value() Lars-Peter Clausen
  0 siblings, 2 replies; 7+ messages in thread
From: Lars-Peter Clausen @ 2020-12-15 19:17 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Peter Meerwald-Stadler, linux-iio, Lars-Peter Clausen

IIO_VAL_FRACTIONAL_LOG2 works with signed values, yet the temporary we use
is unsigned. This works at the moment because the variable is implicitly
cast to signed everywhere where it is used.

But it will certainly be cleaner to use a signed variable in the first
place.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/industrialio-core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index c2e4c267c36b..d84b74cc544b 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -592,7 +592,6 @@ EXPORT_SYMBOL(iio_read_mount_matrix);
 static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
 				  int size, const int *vals)
 {
-	unsigned long long tmp;
 	int tmp0, tmp1;
 	s64 tmp2;
 	bool scale_db = false;
@@ -625,8 +624,8 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
 		else
 			return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
 	case IIO_VAL_FRACTIONAL_LOG2:
-		tmp = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
-		tmp0 = (int)div_s64_rem(tmp, 1000000000LL, &tmp1);
+		tmp2 = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
+		tmp0 = (int)div_s64_rem(tmp2, 1000000000LL, &tmp1);
 		return scnprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
 	case IIO_VAL_INT_MULTIPLE:
 	{
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/3] iio: iio_format_value(): Fix IIO_VAL_FRACTIONAL_LOG2 values between -1.0 and 0.0
  2020-12-15 19:17 [PATCH 1/3] iio: iio_format_value(): Use signed temporary for IIO_VAL_FRACTIONAL_LOG2 Lars-Peter Clausen
@ 2020-12-15 19:17 ` Lars-Peter Clausen
  2020-12-15 19:17 ` [PATCH 3/3] iio: Add basic unit test for iio_format_value() Lars-Peter Clausen
  1 sibling, 0 replies; 7+ messages in thread
From: Lars-Peter Clausen @ 2020-12-15 19:17 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Peter Meerwald-Stadler, linux-iio, Lars-Peter Clausen

When formatting a value using IIO_VAL_FRACTIONAL_LOG2 and the values is
between -1 and 0 the sign is omitted.

We need the same trick as for IIO_VAL_FRACTIONAL to make sure this gets
formatted correctly.

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
Found while writing a unit test for this.

Considering nobody complained we probably do not have an driver that will
run into this issue at the moment. So this doesn't need to got to stable.
---
 drivers/iio/industrialio-core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
index d84b74cc544b..4b30c042e5f6 100644
--- a/drivers/iio/industrialio-core.c
+++ b/drivers/iio/industrialio-core.c
@@ -626,7 +626,10 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type,
 	case IIO_VAL_FRACTIONAL_LOG2:
 		tmp2 = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
 		tmp0 = (int)div_s64_rem(tmp2, 1000000000LL, &tmp1);
-		return scnprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
+		if (tmp0 == 0 && tmp2 < 0)
+			return snprintf(buf, len, "-0.%09u", abs(tmp1));
+		else
+			return scnprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
 	case IIO_VAL_INT_MULTIPLE:
 	{
 		int i;
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/3] iio: Add basic unit test for iio_format_value()
  2020-12-15 19:17 [PATCH 1/3] iio: iio_format_value(): Use signed temporary for IIO_VAL_FRACTIONAL_LOG2 Lars-Peter Clausen
  2020-12-15 19:17 ` [PATCH 2/3] iio: iio_format_value(): Fix IIO_VAL_FRACTIONAL_LOG2 values between -1.0 and 0.0 Lars-Peter Clausen
@ 2020-12-15 19:17 ` Lars-Peter Clausen
  2020-12-29 18:16   ` Jonathan Cameron
  1 sibling, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2020-12-15 19:17 UTC (permalink / raw)
  To: Jonathan Cameron; +Cc: Peter Meerwald-Stadler, linux-iio, Lars-Peter Clausen

The IIO core provides a function to do formatting of fixedpoint numbers.

In the past there have been some issues with the implementation of the
function where for example negative numbers were not handled correctly.

Introduce a basic unit test based on kunit that tests the function and
ensures that the generated output matches the expected output.

This gives us some confidence that future modifications to the function
implementation will not break ABI compatibility.

To run the unit tests follow the kunit documentation and add

  CONFIG_IIO=y
  CONFIG_IIO_TEST_FORMAT=y

to the .kunitconfig and run

  > ./tools/testing/kunit/kunit.py run
  Configuring KUnit Kernel ...
  Building KUnit Kernel ...
  Starting KUnit Kernel ...
  ============================================================
  ======== [PASSED] iio-format ========
  [PASSED] iio_test_iio_format_value_integer
  [PASSED] iio_test_iio_format_value_fixedpoint
  [PASSED] iio_test_iio_format_value_fractional
  [PASSED] iio_test_iio_format_value_fractional_log2
  [PASSED] iio_test_iio_format_value_multiple
  ============================================================
  Testing complete. 21 tests run. 0 failed. 0 crashed.
  Elapsed time: 8.242s total, 0.001s configuring, 3.865s building, 0.000s running

Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
---
 drivers/iio/Kconfig                |   1 +
 drivers/iio/Makefile               |   1 +
 drivers/iio/test/Kconfig           |   9 ++
 drivers/iio/test/Makefile          |   7 +
 drivers/iio/test/iio-test-format.c | 198 +++++++++++++++++++++++++++++
 5 files changed, 216 insertions(+)
 create mode 100644 drivers/iio/test/Kconfig
 create mode 100644 drivers/iio/test/Makefile
 create mode 100644 drivers/iio/test/iio-test-format.c

diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
index 267553386c71..b35e0c33b5e2 100644
--- a/drivers/iio/Kconfig
+++ b/drivers/iio/Kconfig
@@ -85,6 +85,7 @@ source "drivers/iio/light/Kconfig"
 source "drivers/iio/magnetometer/Kconfig"
 source "drivers/iio/multiplexer/Kconfig"
 source "drivers/iio/orientation/Kconfig"
+source "drivers/iio/test/Kconfig"
 if IIO_TRIGGER
    source "drivers/iio/trigger/Kconfig"
 endif #IIO_TRIGGER
diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
index 1712011c0f4a..2561325aaa74 100644
--- a/drivers/iio/Makefile
+++ b/drivers/iio/Makefile
@@ -38,4 +38,5 @@ obj-y += pressure/
 obj-y += proximity/
 obj-y += resolver/
 obj-y += temperature/
+obj-y += test/
 obj-y += trigger/
diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
new file mode 100644
index 000000000000..679a7794af20
--- /dev/null
+++ b/drivers/iio/test/Kconfig
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Industrial I/O subsystem unit tests configuration
+#
+
+# Keep in alphabetical order
+config IIO_TEST_FORMAT
+        bool "Test IIO formatting functions"
+        depends on KUNIT=y
diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
new file mode 100644
index 000000000000..f1099b495301
--- /dev/null
+++ b/drivers/iio/test/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the industrial I/O unit tests.
+#
+
+# Keep in alphabetical order
+obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
new file mode 100644
index 000000000000..55a0cfe9181d
--- /dev/null
+++ b/drivers/iio/test/iio-test-format.c
@@ -0,0 +1,198 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Unit tests for IIO formatting functions
+ *
+ * Copyright (c) 2020 Lars-Peter Clausen <lars@metafoo.de>
+ */
+
+#include <kunit/test.h>
+#include <linux/iio/iio.h>
+
+#define IIO_TEST_FORMAT_EXPECT_EQ(_test, _buf, _ret, _val) do { \
+		KUNIT_EXPECT_EQ(_test, (int)strlen(_buf), _ret); \
+		KUNIT_EXPECT_STREQ(_test, (_buf), (_val)); \
+	} while (0)
+
+static void iio_test_iio_format_value_integer(struct kunit *test)
+{
+	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+	int val;
+	int ret;
+
+	val = 42;
+	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "42\n");
+
+	val = -23;
+	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-23\n");
+
+	val = 0;
+	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
+
+	val = INT_MAX;
+	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "2147483647\n");
+
+	val = INT_MIN;
+	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-2147483648\n");
+}
+
+static void iio_test_iio_format_value_fixedpoint(struct kunit *test)
+{
+	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+	int values[2];
+	int ret;
+
+	/* positive >= 1 */
+	values[0] = 1;
+	values[1] = 10;
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010 dB\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000000010\n");
+
+	/* positive < 1 */
+	values[0] = 0;
+	values[1] = 12;
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012 dB\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000012\n");
+
+	/* negative <= -1 */
+	values[0] = -1;
+	values[1] = 10;
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010 dB\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000000010\n");
+
+	/* negative > -1 */
+	values[0] = 0;
+	values[1] = -123;
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123 dB\n");
+
+	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000123\n");
+}
+
+static void iio_test_iio_format_value_fractional(struct kunit *test)
+{
+	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+	int values[2];
+	int ret;
+
+	/* positive < 1 */
+	values[0] = 1;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.100000000\n");
+
+	/* positive >= 1 */
+	values[0] = 100;
+	values[1] = 3;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "33.333333333\n");
+
+	/* negative > -1 */
+	values[0] = -1;
+	values[1] = 1000000000;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000001\n");
+
+	/* negative <= -1 */
+	values[0] = -200;
+	values[1] = 3;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-66.666666666\n");
+
+	/* Zero */
+	values[0] = 0;
+	values[1] = -10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
+}
+
+static void iio_test_iio_format_value_fractional_log2(struct kunit *test)
+{
+	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+	int values[2];
+	int ret;
+
+	/* positive < 1 */
+	values[0] = 123;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.120117187\n");
+
+	/* positive >= 1 */
+	values[0] = 1234567;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1205.631835937\n");
+
+	/* negative > -1 */
+	values[0] = -123;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.120117187\n");
+
+	/* negative <= -1 */
+	values[0] = -1234567;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1205.631835937\n");
+
+	/* Zero */
+	values[0] = 0;
+	values[1] = 10;
+	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
+}
+
+static void iio_test_iio_format_value_multiple(struct kunit *test)
+{
+	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
+	int values[] = {1, -2, 3, -4, 5};
+	int ret;
+
+	ret = iio_format_value(buf, IIO_VAL_INT_MULTIPLE,
+			       ARRAY_SIZE(values), values);
+	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1 -2 3 -4 5 \n");
+}
+
+static struct kunit_case iio_format_test_cases[] = {
+		KUNIT_CASE(iio_test_iio_format_value_integer),
+		KUNIT_CASE(iio_test_iio_format_value_fixedpoint),
+		KUNIT_CASE(iio_test_iio_format_value_fractional),
+		KUNIT_CASE(iio_test_iio_format_value_fractional_log2),
+		KUNIT_CASE(iio_test_iio_format_value_multiple),
+		{}
+};
+
+static struct kunit_suite iio_format_test_suite = {
+	.name = "iio-format",
+	.test_cases = iio_format_test_cases,
+};
+kunit_test_suite(iio_format_test_suite);
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: Add basic unit test for iio_format_value()
  2020-12-15 19:17 ` [PATCH 3/3] iio: Add basic unit test for iio_format_value() Lars-Peter Clausen
@ 2020-12-29 18:16   ` Jonathan Cameron
  2021-01-02 14:59     ` Lars-Peter Clausen
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2020-12-29 18:16 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Peter Meerwald-Stadler, linux-iio

On Tue, 15 Dec 2020 20:17:43 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> The IIO core provides a function to do formatting of fixedpoint numbers.
> 
> In the past there have been some issues with the implementation of the
> function where for example negative numbers were not handled correctly.
> 
> Introduce a basic unit test based on kunit that tests the function and
> ensures that the generated output matches the expected output.
> 
> This gives us some confidence that future modifications to the function
> implementation will not break ABI compatibility.
> 
> To run the unit tests follow the kunit documentation and add
> 
>   CONFIG_IIO=y
>   CONFIG_IIO_TEST_FORMAT=y
> 
> to the .kunitconfig and run
> 
>   > ./tools/testing/kunit/kunit.py run  
>   Configuring KUnit Kernel ...
>   Building KUnit Kernel ...
>   Starting KUnit Kernel ...
>   ============================================================
>   ======== [PASSED] iio-format ========
>   [PASSED] iio_test_iio_format_value_integer
>   [PASSED] iio_test_iio_format_value_fixedpoint
>   [PASSED] iio_test_iio_format_value_fractional
>   [PASSED] iio_test_iio_format_value_fractional_log2
>   [PASSED] iio_test_iio_format_value_multiple
>   ============================================================
>   Testing complete. 21 tests run. 0 failed. 0 crashed.
>   Elapsed time: 8.242s total, 0.001s configuring, 3.865s building, 0.000s running
> 
> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>

Series looks good to me, but I wonder.  What is convention wrt to 
MAINTAINERS / Acks etc for self tests?

Jonathan


> ---
>  drivers/iio/Kconfig                |   1 +
>  drivers/iio/Makefile               |   1 +
>  drivers/iio/test/Kconfig           |   9 ++
>  drivers/iio/test/Makefile          |   7 +
>  drivers/iio/test/iio-test-format.c | 198 +++++++++++++++++++++++++++++
>  5 files changed, 216 insertions(+)
>  create mode 100644 drivers/iio/test/Kconfig
>  create mode 100644 drivers/iio/test/Makefile
>  create mode 100644 drivers/iio/test/iio-test-format.c
> 
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 267553386c71..b35e0c33b5e2 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -85,6 +85,7 @@ source "drivers/iio/light/Kconfig"
>  source "drivers/iio/magnetometer/Kconfig"
>  source "drivers/iio/multiplexer/Kconfig"
>  source "drivers/iio/orientation/Kconfig"
> +source "drivers/iio/test/Kconfig"
>  if IIO_TRIGGER
>     source "drivers/iio/trigger/Kconfig"
>  endif #IIO_TRIGGER
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 1712011c0f4a..2561325aaa74 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -38,4 +38,5 @@ obj-y += pressure/
>  obj-y += proximity/
>  obj-y += resolver/
>  obj-y += temperature/
> +obj-y += test/
>  obj-y += trigger/
> diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
> new file mode 100644
> index 000000000000..679a7794af20
> --- /dev/null
> +++ b/drivers/iio/test/Kconfig
> @@ -0,0 +1,9 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Industrial I/O subsystem unit tests configuration
> +#
> +
> +# Keep in alphabetical order
> +config IIO_TEST_FORMAT
> +        bool "Test IIO formatting functions"
> +        depends on KUNIT=y
> diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
> new file mode 100644
> index 000000000000..f1099b495301
> --- /dev/null
> +++ b/drivers/iio/test/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Makefile for the industrial I/O unit tests.
> +#
> +
> +# Keep in alphabetical order
> +obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
> diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
> new file mode 100644
> index 000000000000..55a0cfe9181d
> --- /dev/null
> +++ b/drivers/iio/test/iio-test-format.c
> @@ -0,0 +1,198 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Unit tests for IIO formatting functions
> + *
> + * Copyright (c) 2020 Lars-Peter Clausen <lars@metafoo.de>
> + */
> +
> +#include <kunit/test.h>
> +#include <linux/iio/iio.h>
> +
> +#define IIO_TEST_FORMAT_EXPECT_EQ(_test, _buf, _ret, _val) do { \
> +		KUNIT_EXPECT_EQ(_test, (int)strlen(_buf), _ret); \
> +		KUNIT_EXPECT_STREQ(_test, (_buf), (_val)); \
> +	} while (0)
> +
> +static void iio_test_iio_format_value_integer(struct kunit *test)
> +{
> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> +	int val;
> +	int ret;
> +
> +	val = 42;
> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "42\n");
> +
> +	val = -23;
> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-23\n");
> +
> +	val = 0;
> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
> +
> +	val = INT_MAX;
> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "2147483647\n");
> +
> +	val = INT_MIN;
> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-2147483648\n");
> +}
> +
> +static void iio_test_iio_format_value_fixedpoint(struct kunit *test)
> +{
> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> +	int values[2];
> +	int ret;
> +
> +	/* positive >= 1 */
> +	values[0] = 1;
> +	values[1] = 10;
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010 dB\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000000010\n");
> +
> +	/* positive < 1 */
> +	values[0] = 0;
> +	values[1] = 12;
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012 dB\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000012\n");
> +
> +	/* negative <= -1 */
> +	values[0] = -1;
> +	values[1] = 10;
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010 dB\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000000010\n");
> +
> +	/* negative > -1 */
> +	values[0] = 0;
> +	values[1] = -123;
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123 dB\n");
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000123\n");
> +}
> +
> +static void iio_test_iio_format_value_fractional(struct kunit *test)
> +{
> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> +	int values[2];
> +	int ret;
> +
> +	/* positive < 1 */
> +	values[0] = 1;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.100000000\n");
> +
> +	/* positive >= 1 */
> +	values[0] = 100;
> +	values[1] = 3;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "33.333333333\n");
> +
> +	/* negative > -1 */
> +	values[0] = -1;
> +	values[1] = 1000000000;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000001\n");
> +
> +	/* negative <= -1 */
> +	values[0] = -200;
> +	values[1] = 3;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-66.666666666\n");
> +
> +	/* Zero */
> +	values[0] = 0;
> +	values[1] = -10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
> +}
> +
> +static void iio_test_iio_format_value_fractional_log2(struct kunit *test)
> +{
> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> +	int values[2];
> +	int ret;
> +
> +	/* positive < 1 */
> +	values[0] = 123;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.120117187\n");
> +
> +	/* positive >= 1 */
> +	values[0] = 1234567;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1205.631835937\n");
> +
> +	/* negative > -1 */
> +	values[0] = -123;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.120117187\n");
> +
> +	/* negative <= -1 */
> +	values[0] = -1234567;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1205.631835937\n");
> +
> +	/* Zero */
> +	values[0] = 0;
> +	values[1] = 10;
> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
> +}
> +
> +static void iio_test_iio_format_value_multiple(struct kunit *test)
> +{
> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
> +	int values[] = {1, -2, 3, -4, 5};
> +	int ret;
> +
> +	ret = iio_format_value(buf, IIO_VAL_INT_MULTIPLE,
> +			       ARRAY_SIZE(values), values);
> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1 -2 3 -4 5 \n");
> +}
> +
> +static struct kunit_case iio_format_test_cases[] = {
> +		KUNIT_CASE(iio_test_iio_format_value_integer),
> +		KUNIT_CASE(iio_test_iio_format_value_fixedpoint),
> +		KUNIT_CASE(iio_test_iio_format_value_fractional),
> +		KUNIT_CASE(iio_test_iio_format_value_fractional_log2),
> +		KUNIT_CASE(iio_test_iio_format_value_multiple),
> +		{}
> +};
> +
> +static struct kunit_suite iio_format_test_suite = {
> +	.name = "iio-format",
> +	.test_cases = iio_format_test_cases,
> +};
> +kunit_test_suite(iio_format_test_suite);


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: Add basic unit test for iio_format_value()
  2020-12-29 18:16   ` Jonathan Cameron
@ 2021-01-02 14:59     ` Lars-Peter Clausen
  2021-02-07 12:45       ` Lars-Peter Clausen
  0 siblings, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2021-01-02 14:59 UTC (permalink / raw)
  To: Jonathan Cameron, Brendan Higgins; +Cc: Peter Meerwald-Stadler, linux-iio

On 12/29/20 7:16 PM, Jonathan Cameron wrote:
> On Tue, 15 Dec 2020 20:17:43 +0100
> Lars-Peter Clausen <lars@metafoo.de> wrote:
>
>> The IIO core provides a function to do formatting of fixedpoint numbers.
>>
>> In the past there have been some issues with the implementation of the
>> function where for example negative numbers were not handled correctly.
>>
>> Introduce a basic unit test based on kunit that tests the function and
>> ensures that the generated output matches the expected output.
>>
>> This gives us some confidence that future modifications to the function
>> implementation will not break ABI compatibility.
>>
>> To run the unit tests follow the kunit documentation and add
>>
>>    CONFIG_IIO=y
>>    CONFIG_IIO_TEST_FORMAT=y
>>
>> to the .kunitconfig and run
>>
>>    > ./tools/testing/kunit/kunit.py run
>>    Configuring KUnit Kernel ...
>>    Building KUnit Kernel ...
>>    Starting KUnit Kernel ...
>>    ============================================================
>>    ======== [PASSED] iio-format ========
>>    [PASSED] iio_test_iio_format_value_integer
>>    [PASSED] iio_test_iio_format_value_fixedpoint
>>    [PASSED] iio_test_iio_format_value_fractional
>>    [PASSED] iio_test_iio_format_value_fractional_log2
>>    [PASSED] iio_test_iio_format_value_multiple
>>    ============================================================
>>    Testing complete. 21 tests run. 0 failed. 0 crashed.
>>    Elapsed time: 8.242s total, 0.001s configuring, 3.865s building, 0.000s running
>>
>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
> Series looks good to me, but I wonder.  What is convention wrt to
> MAINTAINERS / Acks etc for self tests?

Let's ask Brendan.

>
> Jonathan
>
>
>> ---
>>   drivers/iio/Kconfig                |   1 +
>>   drivers/iio/Makefile               |   1 +
>>   drivers/iio/test/Kconfig           |   9 ++
>>   drivers/iio/test/Makefile          |   7 +
>>   drivers/iio/test/iio-test-format.c | 198 +++++++++++++++++++++++++++++
>>   5 files changed, 216 insertions(+)
>>   create mode 100644 drivers/iio/test/Kconfig
>>   create mode 100644 drivers/iio/test/Makefile
>>   create mode 100644 drivers/iio/test/iio-test-format.c
>>
>> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
>> index 267553386c71..b35e0c33b5e2 100644
>> --- a/drivers/iio/Kconfig
>> +++ b/drivers/iio/Kconfig
>> @@ -85,6 +85,7 @@ source "drivers/iio/light/Kconfig"
>>   source "drivers/iio/magnetometer/Kconfig"
>>   source "drivers/iio/multiplexer/Kconfig"
>>   source "drivers/iio/orientation/Kconfig"
>> +source "drivers/iio/test/Kconfig"
>>   if IIO_TRIGGER
>>      source "drivers/iio/trigger/Kconfig"
>>   endif #IIO_TRIGGER
>> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
>> index 1712011c0f4a..2561325aaa74 100644
>> --- a/drivers/iio/Makefile
>> +++ b/drivers/iio/Makefile
>> @@ -38,4 +38,5 @@ obj-y += pressure/
>>   obj-y += proximity/
>>   obj-y += resolver/
>>   obj-y += temperature/
>> +obj-y += test/
>>   obj-y += trigger/
>> diff --git a/drivers/iio/test/Kconfig b/drivers/iio/test/Kconfig
>> new file mode 100644
>> index 000000000000..679a7794af20
>> --- /dev/null
>> +++ b/drivers/iio/test/Kconfig
>> @@ -0,0 +1,9 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Industrial I/O subsystem unit tests configuration
>> +#
>> +
>> +# Keep in alphabetical order
>> +config IIO_TEST_FORMAT
>> +        bool "Test IIO formatting functions"
>> +        depends on KUNIT=y
>> diff --git a/drivers/iio/test/Makefile b/drivers/iio/test/Makefile
>> new file mode 100644
>> index 000000000000..f1099b495301
>> --- /dev/null
>> +++ b/drivers/iio/test/Makefile
>> @@ -0,0 +1,7 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +#
>> +# Makefile for the industrial I/O unit tests.
>> +#
>> +
>> +# Keep in alphabetical order
>> +obj-$(CONFIG_IIO_TEST_FORMAT) += iio-test-format.o
>> diff --git a/drivers/iio/test/iio-test-format.c b/drivers/iio/test/iio-test-format.c
>> new file mode 100644
>> index 000000000000..55a0cfe9181d
>> --- /dev/null
>> +++ b/drivers/iio/test/iio-test-format.c
>> @@ -0,0 +1,198 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/* Unit tests for IIO formatting functions
>> + *
>> + * Copyright (c) 2020 Lars-Peter Clausen <lars@metafoo.de>
>> + */
>> +
>> +#include <kunit/test.h>
>> +#include <linux/iio/iio.h>
>> +
>> +#define IIO_TEST_FORMAT_EXPECT_EQ(_test, _buf, _ret, _val) do { \
>> +		KUNIT_EXPECT_EQ(_test, (int)strlen(_buf), _ret); \
>> +		KUNIT_EXPECT_STREQ(_test, (_buf), (_val)); \
>> +	} while (0)
>> +
>> +static void iio_test_iio_format_value_integer(struct kunit *test)
>> +{
>> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> +	int val;
>> +	int ret;
>> +
>> +	val = 42;
>> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "42\n");
>> +
>> +	val = -23;
>> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-23\n");
>> +
>> +	val = 0;
>> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0\n");
>> +
>> +	val = INT_MAX;
>> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "2147483647\n");
>> +
>> +	val = INT_MIN;
>> +	ret = iio_format_value(buf, IIO_VAL_INT, 1, &val);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-2147483648\n");
>> +}
>> +
>> +static void iio_test_iio_format_value_fixedpoint(struct kunit *test)
>> +{
>> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> +	int values[2];
>> +	int ret;
>> +
>> +	/* positive >= 1 */
>> +	values[0] = 1;
>> +	values[1] = 10;
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000010 dB\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1.000000010\n");
>> +
>> +	/* positive < 1 */
>> +	values[0] = 0;
>> +	values[1] = 12;
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000012 dB\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000012\n");
>> +
>> +	/* negative <= -1 */
>> +	values[0] = -1;
>> +	values[1] = 10;
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000010 dB\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1.000000010\n");
>> +
>> +	/* negative > -1 */
>> +	values[0] = 0;
>> +	values[1] = -123;
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_MICRO_DB, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000123 dB\n");
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_PLUS_NANO, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000123\n");
>> +}
>> +
>> +static void iio_test_iio_format_value_fractional(struct kunit *test)
>> +{
>> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> +	int values[2];
>> +	int ret;
>> +
>> +	/* positive < 1 */
>> +	values[0] = 1;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.100000000\n");
>> +
>> +	/* positive >= 1 */
>> +	values[0] = 100;
>> +	values[1] = 3;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "33.333333333\n");
>> +
>> +	/* negative > -1 */
>> +	values[0] = -1;
>> +	values[1] = 1000000000;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.000000001\n");
>> +
>> +	/* negative <= -1 */
>> +	values[0] = -200;
>> +	values[1] = 3;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-66.666666666\n");
>> +
>> +	/* Zero */
>> +	values[0] = 0;
>> +	values[1] = -10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
>> +}
>> +
>> +static void iio_test_iio_format_value_fractional_log2(struct kunit *test)
>> +{
>> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> +	int values[2];
>> +	int ret;
>> +
>> +	/* positive < 1 */
>> +	values[0] = 123;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.120117187\n");
>> +
>> +	/* positive >= 1 */
>> +	values[0] = 1234567;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1205.631835937\n");
>> +
>> +	/* negative > -1 */
>> +	values[0] = -123;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-0.120117187\n");
>> +
>> +	/* negative <= -1 */
>> +	values[0] = -1234567;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "-1205.631835937\n");
>> +
>> +	/* Zero */
>> +	values[0] = 0;
>> +	values[1] = 10;
>> +	ret = iio_format_value(buf, IIO_VAL_FRACTIONAL_LOG2, 2, values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "0.000000000\n");
>> +}
>> +
>> +static void iio_test_iio_format_value_multiple(struct kunit *test)
>> +{
>> +	char *buf = kunit_kmalloc(test, PAGE_SIZE, GFP_KERNEL);
>> +	int values[] = {1, -2, 3, -4, 5};
>> +	int ret;
>> +
>> +	ret = iio_format_value(buf, IIO_VAL_INT_MULTIPLE,
>> +			       ARRAY_SIZE(values), values);
>> +	IIO_TEST_FORMAT_EXPECT_EQ(test, buf, ret, "1 -2 3 -4 5 \n");
>> +}
>> +
>> +static struct kunit_case iio_format_test_cases[] = {
>> +		KUNIT_CASE(iio_test_iio_format_value_integer),
>> +		KUNIT_CASE(iio_test_iio_format_value_fixedpoint),
>> +		KUNIT_CASE(iio_test_iio_format_value_fractional),
>> +		KUNIT_CASE(iio_test_iio_format_value_fractional_log2),
>> +		KUNIT_CASE(iio_test_iio_format_value_multiple),
>> +		{}
>> +};
>> +
>> +static struct kunit_suite iio_format_test_suite = {
>> +	.name = "iio-format",
>> +	.test_cases = iio_format_test_cases,
>> +};
>> +kunit_test_suite(iio_format_test_suite);



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: Add basic unit test for iio_format_value()
  2021-01-02 14:59     ` Lars-Peter Clausen
@ 2021-02-07 12:45       ` Lars-Peter Clausen
  2021-02-14 12:50         ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: Lars-Peter Clausen @ 2021-02-07 12:45 UTC (permalink / raw)
  To: Jonathan Cameron, Brendan Higgins; +Cc: Peter Meerwald-Stadler, linux-iio

On 1/2/21 3:59 PM, Lars-Peter Clausen wrote:
> On 12/29/20 7:16 PM, Jonathan Cameron wrote:
>> On Tue, 15 Dec 2020 20:17:43 +0100
>> Lars-Peter Clausen <lars@metafoo.de> wrote:
>>
>>> The IIO core provides a function to do formatting of fixedpoint 
>>> numbers.
>>>
>>> In the past there have been some issues with the implementation of the
>>> function where for example negative numbers were not handled correctly.
>>>
>>> Introduce a basic unit test based on kunit that tests the function and
>>> ensures that the generated output matches the expected output.
>>>
>>> This gives us some confidence that future modifications to the function
>>> implementation will not break ABI compatibility.
>>>
>>> To run the unit tests follow the kunit documentation and add
>>>
>>>    CONFIG_IIO=y
>>>    CONFIG_IIO_TEST_FORMAT=y
>>>
>>> to the .kunitconfig and run
>>>
>>>    > ./tools/testing/kunit/kunit.py run
>>>    Configuring KUnit Kernel ...
>>>    Building KUnit Kernel ...
>>>    Starting KUnit Kernel ...
>>> ============================================================
>>>    ======== [PASSED] iio-format ========
>>>    [PASSED] iio_test_iio_format_value_integer
>>>    [PASSED] iio_test_iio_format_value_fixedpoint
>>>    [PASSED] iio_test_iio_format_value_fractional
>>>    [PASSED] iio_test_iio_format_value_fractional_log2
>>>    [PASSED] iio_test_iio_format_value_multiple
>>> ============================================================
>>>    Testing complete. 21 tests run. 0 failed. 0 crashed.
>>>    Elapsed time: 8.242s total, 0.001s configuring, 3.865s building, 
>>> 0.000s running
>>>
>>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>
>> Series looks good to me, but I wonder.  What is convention wrt to
>> MAINTAINERS / Acks etc for self tests?
>
> Let's ask Brendan.
>
Jonathan, since there hasn't been any additional feedback should we just 
apply this series?

Thanks,

- Lars


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 3/3] iio: Add basic unit test for iio_format_value()
  2021-02-07 12:45       ` Lars-Peter Clausen
@ 2021-02-14 12:50         ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2021-02-14 12:50 UTC (permalink / raw)
  To: Lars-Peter Clausen; +Cc: Brendan Higgins, Peter Meerwald-Stadler, linux-iio

On Sun, 7 Feb 2021 13:45:02 +0100
Lars-Peter Clausen <lars@metafoo.de> wrote:

> On 1/2/21 3:59 PM, Lars-Peter Clausen wrote:
> > On 12/29/20 7:16 PM, Jonathan Cameron wrote:  
> >> On Tue, 15 Dec 2020 20:17:43 +0100
> >> Lars-Peter Clausen <lars@metafoo.de> wrote:
> >>  
> >>> The IIO core provides a function to do formatting of fixedpoint 
> >>> numbers.
> >>>
> >>> In the past there have been some issues with the implementation of the
> >>> function where for example negative numbers were not handled correctly.
> >>>
> >>> Introduce a basic unit test based on kunit that tests the function and
> >>> ensures that the generated output matches the expected output.
> >>>
> >>> This gives us some confidence that future modifications to the function
> >>> implementation will not break ABI compatibility.
> >>>
> >>> To run the unit tests follow the kunit documentation and add
> >>>
> >>>    CONFIG_IIO=y
> >>>    CONFIG_IIO_TEST_FORMAT=y
> >>>
> >>> to the .kunitconfig and run
> >>>
> >>>    > ./tools/testing/kunit/kunit.py run
> >>>    Configuring KUnit Kernel ...
> >>>    Building KUnit Kernel ...
> >>>    Starting KUnit Kernel ...
> >>> ============================================================
> >>>    ======== [PASSED] iio-format ========
> >>>    [PASSED] iio_test_iio_format_value_integer
> >>>    [PASSED] iio_test_iio_format_value_fixedpoint
> >>>    [PASSED] iio_test_iio_format_value_fractional
> >>>    [PASSED] iio_test_iio_format_value_fractional_log2
> >>>    [PASSED] iio_test_iio_format_value_multiple
> >>> ============================================================
> >>>    Testing complete. 21 tests run. 0 failed. 0 crashed.
> >>>    Elapsed time: 8.242s total, 0.001s configuring, 3.865s building, 
> >>> 0.000s running
> >>>
> >>> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de>  
> >> Series looks good to me, but I wonder.  What is convention wrt to
> >> MAINTAINERS / Acks etc for self tests?  
> >
> > Let's ask Brendan.
> >  
> Jonathan, since there hasn't been any additional feedback should we just 
> apply this series?
> 
Sure, it's been long enough.

Applied to the togreg branch of iio.git and pushed out as testing for
all the normal reasons.

Thanks,

Jonathan

> Thanks,
> 
> - Lars
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2021-02-14 12:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 19:17 [PATCH 1/3] iio: iio_format_value(): Use signed temporary for IIO_VAL_FRACTIONAL_LOG2 Lars-Peter Clausen
2020-12-15 19:17 ` [PATCH 2/3] iio: iio_format_value(): Fix IIO_VAL_FRACTIONAL_LOG2 values between -1.0 and 0.0 Lars-Peter Clausen
2020-12-15 19:17 ` [PATCH 3/3] iio: Add basic unit test for iio_format_value() Lars-Peter Clausen
2020-12-29 18:16   ` Jonathan Cameron
2021-01-02 14:59     ` Lars-Peter Clausen
2021-02-07 12:45       ` Lars-Peter Clausen
2021-02-14 12:50         ` Jonathan Cameron

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.