On Sat, 21 Dec 2024 20:19:21 +0200
Matti Vaittinen <mazziesaccount@xxxxxxxxx> wrote:
> On 20/12/2024 21:28, Javier Carrasco wrote:
> > This helper functions reduce the burden in the drivers that want to
> > fetch a gain selector in all available times or a new optimal gain.
> >
> > The former is currently achieved by calling
> > iio_gts_find_gain_sel_for_scale_using_time() for the current time
> > selector, and then iterating over the rest of time selectors if the
> > gain selector was not found.
> >
> > The latter requires a combination of multiple iio-gts helpers to find
> > the new gain, look for an optimal gain if there was no exact match, and
> > set a minimum gain if the optimal gain is not in the range of available
> > gains.
> >
> > Provide simpler workflows by means of functions that address common
> > patterns in the users of the iio-gts helpers.
> >
> > Signed-off-by: Javier Carrasco <javier.carrasco.cruz@xxxxxxxxx>
>
> Acked-by: Matti Vaittinen <mazziesaccount@xxxxxxxxx>
>
A couple of comments inline.
Thanks,
Jonathan
> > ---
> > drivers/iio/industrialio-gts-helper.c | 74 +++++++++++++++++++++++++++++++++++
> > include/linux/iio/iio-gts-helper.h | 5 +++
> > 2 files changed, 79 insertions(+)
> >
> > diff --git a/drivers/iio/industrialio-gts-helper.c b/drivers/iio/industrialio-gts-helper.c
> > index 3b5a99815062..f88b0b7192dd 100644
> > --- a/drivers/iio/industrialio-gts-helper.c
> > +++ b/drivers/iio/industrialio-gts-helper.c
> > @@ -915,6 +915,38 @@ int iio_gts_find_gain_sel_for_scale_using_time(struct iio_gts *gts, int time_sel
> > }
> > EXPORT_SYMBOL_NS_GPL(iio_gts_find_gain_sel_for_scale_using_time, "IIO_GTS_HELPER");
> >
> > +/**
> > + * iio_gts_find_gain_sel_in_times - Fetch gain selector in the available times.
> > + * @gts: Gain time scale descriptor
> > + * @scale_int: Integral part of the scale (typically val1)
> > + * @scale_nano: Fractional part of the scale (nano or ppb)
> > + * @gain_sel: Pointer to value where gain selector is stored.
> > + * @time_sel: Pointer to value where time selector is stored.
> > + *
> > + * Wrapper around iio_gts_find_gain_for_scale_using_time() to fetch the
> > + * gain selector for all supported integration times.
> > + *
> > + * Return: 0 on success and -EINVAL on error.
> > + */
> > +int iio_gts_find_gain_sel_in_times(struct iio_gts *gts, int scale_int,
> > + int scale_nano, int *gain_sel, int *time_sel)
> > +{
> > + int i, ret;
> > +
> > + for (i = 0; i < gts->num_itime; i++) {
> > + *time_sel = gts->itime_table[i].sel;
> > + ret = iio_gts_find_gain_sel_for_scale_using_time(gts, *time_sel,
> > + scale_int,
> > + scale_nano,
> > + gain_sel);
> > + if (!ret)
> > + return 0;
Trivial but slight preference for keeping the error path out of line rather than
the good path.
Easily done with:
if (ret)
continue;
return 0;
> > + }
> > +
> > + return -EINVAL;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_gts_find_gain_sel_in_times, "IIO_GTS_HELPER");
> > +
> > static int iio_gts_get_total_gain(struct iio_gts *gts, int gain, int time)
> > {
> > const struct iio_itime_sel_mul *itime;
> > @@ -1086,6 +1118,48 @@ int iio_gts_find_new_gain_by_old_gain_time(struct iio_gts *gts, int old_gain,
> > }
> > EXPORT_SYMBOL_NS_GPL(iio_gts_find_new_gain_by_old_gain_time, "IIO_GTS_HELPER");
> >
> > +/**
> > + * iio_gts_find_new_gain_by_gain_time_min - compensate for time change
> > + * @gts: Gain time scale descriptor
> > + * @old_gain: Previously set gain
> > + * @old_time: Selector corresponding previously set time
> > + * @new_time: Selector corresponding new time to be set
> > + * @new_gain: Pointer to value where new gain is to be written
> > + * @in_range: Indicate if the @new_gain was in the range of
> > + * supported gains.
> > + *
> > + * Wrapper around iio_gts_find_new_gain_by_old_gain_time() that tries to
> > + * set an optimal value if no exact match was found, defaulting to the
> > + * minimum gain to avoid saturations if the optimal value is not in the
> > + * range of supported gains.
> > + *
> > + * Return: 0 on success and a negative value if no gain was found.
> > + */
> > +int iio_gts_find_new_gain_by_gain_time_min(struct iio_gts *gts, int old_gain,
> > + int old_time, int new_time,
> > + int *new_gain, bool *in_range)
> > +{
> > + int ret;
> > +
> > + *in_range = true;
> > + ret = iio_gts_find_new_gain_by_old_gain_time(gts, old_gain, old_time,
> > + new_time, new_gain);
> > + if (*new_gain < 0)
> > + return -EINVAL;
> > +
> > + if (ret) {
> > + *new_gain = iio_find_closest_gain_low(gts, *new_gain, in_range);
> > + if (*new_gain < 0) {
> > + *new_gain = iio_gts_get_min_gain(gts);
> > + if (*new_gain < 0)
> > + return ret;
The return value here is possibly misleading as it comes from the original
iio_gts_find_new_gain_by_old_gain_time() call.
Sure it may have the same value, but I'd rather it was set explicitly here
so it was clear that we have a case where all fallbacks failed.
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_NS_GPL(iio_gts_find_new_gain_by_gain_time_min, "IIO_GTS_HELPER");
> > +
> > MODULE_LICENSE("GPL");
> > MODULE_AUTHOR("Matti Vaittinen <mazziesaccount@xxxxxxxxx>");
> > MODULE_DESCRIPTION("IIO light sensor gain-time-scale helpers");
> > diff --git a/include/linux/iio/iio-gts-helper.h b/include/linux/iio/iio-gts-helper.h
> > index 9cb6c80dea71..ae91ad008cc8 100644
> > --- a/include/linux/iio/iio-gts-helper.h
> > +++ b/include/linux/iio/iio-gts-helper.h
> > @@ -188,6 +188,8 @@ int iio_gts_total_gain_to_scale(struct iio_gts *gts, int total_gain,
> > int iio_gts_find_gain_sel_for_scale_using_time(struct iio_gts *gts, int time_sel,
> > int scale_int, int scale_nano,
> > int *gain_sel);
> > +int iio_gts_find_gain_sel_in_times(struct iio_gts *gts, int scale_int,
> > + int scale_nano, int *gain_sel, int *time_sel);
> > int iio_gts_get_scale(struct iio_gts *gts, int gain, int time, int *scale_int,
> > int *scale_nano);
> > int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
> > @@ -196,6 +198,9 @@ int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
> > int iio_gts_find_new_gain_by_old_gain_time(struct iio_gts *gts, int old_gain,
> > int old_time, int new_time,
> > int *new_gain);
> > +int iio_gts_find_new_gain_by_gain_time_min(struct iio_gts *gts, int old_gain,
> > + int old_time, int new_time,
> > + int *new_gain, bool *in_range);
> > int iio_gts_avail_times(struct iio_gts *gts, const int **vals, int *type,
> > int *length);
> > int iio_gts_all_avail_scales(struct iio_gts *gts, const int **vals, int *type,
> >
>
