lkml.org 
[lkml]   [2023]   [Mar]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 2/6] iio: light: Add gain-time-scale helpers
From
On 3/12/23 19:06, Jonathan Cameron wrote:
> On Mon, 6 Mar 2023 11:17:15 +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.
>>
>> Add some gain-time-scale helpers in order to not dublicate errors in all
>> drivers needing these computations.
>>
>> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
>
> Trying not to duplicate what Andy has raised...
>
>
> At some stage I want to go through the maths very carefully but it's
> not happening today and I don't want to delay resolving other remaining comments
> so that can wait for a later version. I'm sure it's fine but I like to be
> paranoid :)
>

This is more than welcome! I tried to add some test cases for verifying
some parts - but extra pair of eyes is always more than appreciated!
I've written and read way too many bugs to not appreciate a healthy
amount of paranoia :)

>> +int iio_gts_get_total_gain(struct iio_gts *gts, int gain, int time)
>> +{
>> + const struct iio_itime_sel_mul *itime;
>> +
>> + if (!iio_gts_valid_gain(gts, gain))
>> + return -EINVAL;
>> +
>> + if (!gts->num_itime)
>> + return gain;
>> +
>> + itime = iio_gts_find_itime_by_time(gts, time);
>> + if (!itime)
>> + return -EINVAL;
>> +
>> + return gain * itime->mul;
>> +}
>> +EXPORT_SYMBOL(iio_gts_get_total_gain);
>
> All of them want to be in the namespace.

Seems like I accidentally did not use the EXPORT_SYMBOL_GPL for this
one. It must thus have evaded my conversion to name space one. Thanks!

>
>
>> diff --git a/drivers/iio/light/iio-gts-helper.h b/drivers/iio/light/iio-gts-helper.h
>> new file mode 100644
>> index 000000000000..4b5a417946f4
>> --- /dev/null
>> +++ b/drivers/iio/light/iio-gts-helper.h
>
> ...
>
>> +int iio_gts_find_new_gain_sel_by_old_gain_time(struct iio_gts *gts,
>> + int old_gain, int old_time_sel,
>> + int new_time_sel, int *new_gain);
>> +int iio_gts_build_avail_tables(struct iio_gts *gts);
>> +int devm_iio_gts_build_avail_tables(struct device *dev, struct iio_gts *gts);
>> +int iio_gts_build_avail_scale_table(struct iio_gts *gts);
>> +int devm_iio_gts_build_avail_scale_table(struct device *dev, struct iio_gts *gts);
>> +int iio_gts_build_avail_time_table(struct iio_gts *gts);
>> +int devm_iio_gts_build_avail_time_table(struct device *dev, struct iio_gts *gts);
>
> Given most modern IIO drivers use fully devm_ based probing, for now I would not
> expose anything else. That will reduce the interface a lot which I think
> is probably a good thing at this stage.
>
> Keep the non devm stuff internally though as it is a nice structure to have
> an I can see we may want some of these in non devm form in the future.
>
> Similarly - for now don't expose the individual table building functions
> as we may never need them in drivers. We (more or less) only support interfaces
> that are used and so far they aren't.
>
> For other functions it's worth thinking about whether to not export them
> initially. I haven't been through them all to figure out what is not currently used.
>
>> +void iio_gts_purge_avail_scale_table(struct iio_gts *gts);
>> +void iio_gts_purge_avail_time_table(struct iio_gts *gts);
>> +void iio_gts_purge_avail_tables(struct iio_gts *gts);
>> +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,
>> + int *length);
>> +int iio_gts_avail_scales_for_time(struct iio_gts *gts, int time,
>> + const int **vals, int *type, int *length);
>> +
>> +#endif
>

--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland

~~ When things go utterly wrong vim users can always type :help! ~~

\
 
 \ /
  Last update: 2023-03-27 00:56    [W:1.110 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site