On Mon, 15 Jan 2024 15:01:32 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> On 1/13/24 18:12, Jonathan Cameron wrote:
> > On Wed, 10 Jan 2024 12:12:55 +0200
> > Matti Vaittinen <mazziesaccount@xxxxxxxxx> 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@xxxxxxxxx>
> >>
>
> ...
>
> >> +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?
>
> Doing:
> const int exp_len = ARRAY_SIZE(gains) * 2;
> int expected[exp_len];
>
> gives me:
> warning: ISO C90 forbids variable length array ‘expected’ [-Wvla]
Huh. That's a compiler being impressively stupid :(
Just leave it as it is - maybe add a comment to so no one tries to tidy this
up in future.
>
> I could drop the whole exp_len variable, but I prefer test code which is
> as obvious as it gets if any of the checks fails. For me the check:
>
> >> + KUNIT_EXPECT_EQ(test, exp_len, len);
> >> + if (len != exp_len)
> >> + return;
>
> is (very slightly) more obvious than:
> KUNIT_EXPECT_EQ(test, ARRAY_SIZE(gains) * 2, len);
> if (len != ARRAY_SIZE(gains) * 2)
> return;
>
> I guess I'll leave this one as it is. Just kick me in v2 if I
> misunderstood you :)
yeah, leave it be. Annoying but such is life.
>
> Yours,
> -- Matti
>
