[0/3] KUnit tests for the IIO GTS helpers

  Add some KUnit tests for the IIO GTS helpers.

These tests were originally part of the BU27034 ALS sensor driver series.
https://lore.kernel.org/all/cover.1679915278.git.mazziesaccount@gmail.com/

Merging the tests was postponed because we lacked of a good generic way
of creating tests devices for testing the devm managed interfaces. Now we
have kunit_device APIs being merged (seems like they'll be part of the
v6.8-rc1) so precondition for merging these tests are (being) met.

The series is based on
commit ab27740f7665 ("Merge tag 'linux_kselftest-next-6.8-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest")
in Linus' tree.

I can rebase and resend when v6.8-rc1 is out if needed.

Matti Vaittinen (3):
  iio: gts-helper: Fix division loop
  iio: test: test gain-time-scale helpers
  MAINTAINERS: add IIO GTS tests

 MAINTAINERS                           |   1 +
 drivers/iio/industrialio-gts-helper.c |   5 +-
 drivers/iio/test/Kconfig              |  14 +
 drivers/iio/test/Makefile             |   1 +
 drivers/iio/test/iio-test-gts.c       | 517 ++++++++++++++++++++++++++
 5 files changed, 535 insertions(+), 3 deletions(-)
 create mode 100644 drivers/iio/test/iio-test-gts.c


base-commit: ab27740f76654ed58dd32ac0ba0031c18a6dea3b
  

On 1/10/24 12:07, Matti Vaittinen wrote:
> Add some KUnit tests for the IIO GTS helpers.

..

> Matti Vaittinen (3):
>    iio: gts-helper: Fix division loop

Please ignore the patch 1/3 above. It has been already merged. 
Unfortunately I noticed this commit being in the series only after 
sending the cover-letter. In order to not consumer reviewer's time, I 
omitted sending the 1/3 - so it's missing the series on purpose. 
Subsequent versions won't include this patch anymore.

>    iio: test: test gain-time-scale helper >    MAINTAINERS: add IIO GTS tests
> 
>   MAINTAINERS                           |   1 +
>   drivers/iio/industrialio-gts-helper.c |   5 +-

Also, this file is not changed by patches 2/3 and 3/3.

Sorry!

Yours,
	-- Matti
  

On Wed, 10 Jan 2024 12:12:55 +0200
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> Some light sensors can adjust both the HW-gain and integration time.
> There are cases where adjusting the integration time has similar impact
> to the scale of the reported values as gain setting has.
> 
> IIO users do typically expect to handle scale by a single writable 'scale'
> entry. Driver should then adjust the gain/time accordingly.
> 
> It however is difficult for a driver to know whether it should change
> gain or integration time to meet the requested scale. Usually it is
> preferred to have longer integration time which usually improves
> accuracy, but there may be use-cases where long measurement times can be
> an issue. Thus it can be preferable to allow also changing the
> integration time - but mitigate the scale impact by also changing the gain
> underneath. Eg, if integration time change doubles the measured values,
> the driver can reduce the HW-gain to half.
> 
> The theory of the computations of gain-time-scale is simple. However,
> some people (undersigned) got that implemented wrong for more than once.
> Hence some gain-time-scale helpers were introduced.
> 
> Add some simple tests to verify the most hairy functions.
> 
> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> 
Hi Matti,

All seems reasonable to me. Some trivial formatting things inline
+ I'm not planning to check the maths as you are the expert in all of
this so I'll just trust you!

Also if this fails you get to pick up the pieces :)

Jonathan

>  CFLAGS_iio-test-format.o += $(DISABLE_STRUCTLEAK_PLUGIN)
> diff --git a/drivers/iio/test/iio-test-gts.c b/drivers/iio/test/iio-test-gts.c
> new file mode 100644
> index 000000000000..4d5271b0c7bc
> --- /dev/null
> +++ b/drivers/iio/test/iio-test-gts.c
> @@ -0,0 +1,517 @@
..

> +
> +static void test_iio_find_closest_gain_low(struct kunit *test)
> +{
> +	struct device *dev;
> +	bool in_range;
> +	int ret;
> +
> +	const struct iio_gain_sel_pair gts_test_gains_gain_low[] = {
> +		GAIN_SCALE_GAIN(4, TEST_GSEL_4),
> +		GAIN_SCALE_GAIN(16, TEST_GSEL_16),
> +		GAIN_SCALE_GAIN(32, TEST_GSEL_32),
> +	};
> +
> +	dev = test_init_iio_gain_scale(test, &gts);
> +	if (!dev)
> +		return;
> +
> +	ret = iio_find_closest_gain_low(&gts, 2, &in_range);
> +	KUNIT_EXPECT_EQ(test, 1, ret);
> +	KUNIT_EXPECT_EQ(test, true, in_range);
> +
> +	ret = iio_find_closest_gain_low(&gts, 1, &in_range);
> +	KUNIT_EXPECT_EQ(test, 1, ret);
> +	KUNIT_EXPECT_EQ(test, true, in_range);
> +
> +	ret = iio_find_closest_gain_low(&gts, 4095, &in_range);
> +	KUNIT_EXPECT_EQ(test, 2048, ret);
> +	KUNIT_EXPECT_EQ(test, true, in_range);
> +
> +	ret = iio_find_closest_gain_low(&gts, 4097, &in_range);
> +	KUNIT_EXPECT_EQ(test, 4096, ret);
> +	KUNIT_EXPECT_EQ(test, false, in_range);
> +
> +	kunit_device_unregister(test, dev);
> +
> +	dev = __test_init_iio_gain_scale(test, &gts, gts_test_gains_gain_low,
> +				ARRAY_SIZE(gts_test_gains_gain_low),
> +				gts_test_itimes, ARRAY_SIZE(gts_test_itimes));
> +	if (!dev)
> +		return;
> +
> +	ret = iio_find_closest_gain_low(&gts, 3, &in_range);
> +	KUNIT_EXPECT_EQ(test, -EINVAL, ret);
> +	KUNIT_EXPECT_EQ(test, false, in_range);
> +
As below.

> +}
> +
> +static void test_iio_gts_total_gain_to_scale(struct kunit *test)
> +{
> +	struct device *dev;
> +	int ret, scale_int, scale_nano;
> +
> +	dev = test_init_iio_gain_scale(test, &gts);
> +	if (!dev)
> +		return;
> +
> +	ret = iio_gts_total_gain_to_scale(&gts, 1, &scale_int, &scale_nano);
> +	KUNIT_EXPECT_EQ(test, 0, ret);
> +	KUNIT_EXPECT_EQ(test, TEST_SCALE_1X, scale_int);
> +	KUNIT_EXPECT_EQ(test, 0, scale_nano);
> +
> +	ret = iio_gts_total_gain_to_scale(&gts, 1, &scale_int, &scale_nano);
> +	KUNIT_EXPECT_EQ(test, 0, ret);
> +	KUNIT_EXPECT_EQ(test, TEST_SCALE_1X, scale_int);
> +	KUNIT_EXPECT_EQ(test, 0, scale_nano);
> +
> +	ret = iio_gts_total_gain_to_scale(&gts, 4096 * 8, &scale_int,
> +					  &scale_nano);
> +	KUNIT_EXPECT_EQ(test, 0, ret);
> +	KUNIT_EXPECT_EQ(test, 0, scale_int);
> +	KUNIT_EXPECT_EQ(test, TEST_SCALE_NANO_4096X8, scale_nano);
> +
No need for a blank line here.

> +}
> +

> +static void test_iio_gts_chk_scales_all(struct kunit *test, struct iio_gts *gts,
> +					const int *vals, int len)
> +{
> +	static const int gains[] = {1, 2, 4, 8, 16, 32, 64, 128, 256, 512,
> +				    1024, 2048, 4096, 4096 * 2, 4096 * 4,
> +				    4096 * 8};
> +
> +	int expected[ARRAY_SIZE(gains) * 2];
> +	int i, ret;
> +	int exp_len = ARRAY_SIZE(gains) * 2;

Use this for expected[*] just above?

> +
> +	KUNIT_EXPECT_EQ(test, exp_len, len);
> +	if (len != exp_len)
> +		return;
> +
> +	for (i = 0; i < ARRAY_SIZE(gains); i++) {
> +		ret = iio_gts_total_gain_to_scale(gts, gains[i],
> +						  &expected[2 * i],
> +						  &expected[2 * i + 1]);
> +		KUNIT_EXPECT_EQ(test, 0, ret);
> +		if (ret)
> +			return;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(expected); i++)
> +		KUNIT_EXPECT_EQ(test, expected[i], vals[i]);
> +}

> +
> +static void test_iio_gts_avail_test(struct kunit *test)
> +{
> +	struct device *dev;
> +	int ret;
> +	int type, len;
> +	const int *vals;
> +
> +	dev = test_init_iio_gain_scale(test, &gts);
> +	if (!dev)
> +		return;
> +
> +	/* test table building for times and iio_gts_avail_times() */
> +	ret = iio_gts_avail_times(&gts, &vals, &type, &len);
> +	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
> +	if (ret)
> +		return;
> +
> +	KUNIT_EXPECT_EQ(test, IIO_VAL_INT_PLUS_MICRO, type);
> +	KUNIT_EXPECT_EQ(test, 8, len);
> +	if (len < 8)
> +		return;
> +
> +	test_iio_gts_chk_times(test, vals);
> +
> +	/* Test table building for all scales and iio_gts_all_avail_scales() */
> +	ret = iio_gts_all_avail_scales(&gts, &vals, &type, &len);
> +	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
> +	if (ret)
> +		return;
> +
> +	KUNIT_EXPECT_EQ(test, IIO_VAL_INT_PLUS_NANO, type);
> +
> +	test_iio_gts_chk_scales_all(test, &gts, vals, len);
> +
> +	/*
> +	 * Test table building for scales/time and
> +	 * iio_gts_avail_scales_for_time()
> +	 */
> +	ret = iio_gts_avail_scales_for_time(&gts, 200000, &vals, &type, &len);
> +	KUNIT_EXPECT_EQ(test, IIO_AVAIL_LIST, ret);
> +	if (ret)
> +		return;
> +
> +	KUNIT_EXPECT_EQ(test, IIO_VAL_INT_PLUS_NANO, type);
> +	test_iio_gts_chk_scales_t200(test, &gts, vals, len);
> +}
> +
> +static struct kunit_case iio_gts_test_cases[] = {
> +	KUNIT_CASE(test_init_iio_gts_invalid),
> +	KUNIT_CASE(test_iio_gts_find_gain_for_scale_using_time),
> +	KUNIT_CASE(test_iio_gts_find_new_gain_sel_by_old_gain_time),
> +	KUNIT_CASE(test_iio_find_closest_gain_low),
> +	KUNIT_CASE(test_iio_gts_total_gain_to_scale),
> +	KUNIT_CASE(test_iio_gts_avail_test),
> +	{}
> +};
> +
> +static struct kunit_suite iio_gts_test_suite = {
> +	.name = "iio-gain-time-scale",
> +	.test_cases = iio_gts_test_cases,
> +};
> +
> +kunit_test_suite(iio_gts_test_suite);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@gmail.com>");
> +MODULE_DESCRIPTION("Test IIO light sensor gain-time-scale helpers");
> +MODULE_IMPORT_NS(IIO_GTS_HELPER);
> +
Looks like a bonus blank line at the end here.

Jonathan