On 3/30/23 19:48, Matti Vaittinen wrote:
On 3/27/23 14:28, Matti Vaittinen 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@xxxxxxxxx>
---
Currently it is only BU27034 using these in this series. I am however
working
with drivers for RGB sensors BU27008 and BU27010 which have similar
[gain - integration time - scale] - relation. I hope sending those
follows soon after the BU27034 is done.
+/**
+ * iio_gts_find_new_gain_sel_by_old_gain_time - compensate for time
change
+ * @gts: Gain time scale descriptor
+ * @old_gain: Previously set gain
+ * @old_time_sel: Selector corresponding previously set time
+ * @new_time_sel: Selector corresponding new time to be set
+ * @new_gain: Pointer to value where new gain is to be written
+ *
+ * We may want to mitigate the scale change caused by setting a new
integration
+ * time (for a light sensor) by also updating the (HW)gain. This
helper computes
+ * new gain value to maintain the scale with new integration time.
+ *
+ * Return: 0 on success. -EINVAL if gain matching the new time is not
found.
Here we need to document another return value denote whether the
@new_gain was updated.
+ */
+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)
+{
+ const struct iio_itime_sel_mul *itime_old, *itime_new;
+ u64 scale;
+ int ret;
+
+ itime_old = iio_gts_find_itime_by_sel(gts, old_time_sel);
+ if (!itime_old)
+ return -EINVAL;
+
+ itime_new = iio_gts_find_itime_by_sel(gts, new_time_sel);
+ if (!itime_new)
+ return -EINVAL;
+
+ ret = iio_gts_get_scale_linear(gts, old_gain, itime_old->time_us,
+ &scale);
+ if (ret)
+ return ret;
+
+ ret = gain_get_scale_fraction(gts->max_scale, scale, itime_new->mul,
+ new_gain);
+ if (ret)
+ return ret;
+
+ if (!iio_gts_valid_gain(gts, *new_gain))
+ return -EINVAL;
I would change this to -ERANGE to differentiate the case where the new
gain was computed but was not valid. The bu27034 (and
not-yet-fully-finished bu27008) driver uses the computed gain to find
closest matching gain the hardware supports. I am not super happy with
the -ERANGE, as it is also possible the gain is in the "range" of
supported gains but not _exactly_ supported one. In a sense -EINVAL
would be more correct. The invalid time could in a sense be interpreted
as an "time selector not found" - so maybe the -ENOENT could be somehow
tolerated. Still, in my opinion the invalid integration time is very
much more an -EINVAL than anything else...
Looks like I keep discussing with myself. This however was not a good
solution as we might detect non integer gain to be required in the
gain_get_scale_fraction(). And deciding if that function should return
-ERANGE or -EINVAL got things even worse.
So, the take N (where N is a positive integer, much greater than 1) is
that I'll do:
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)
{
const struct iio_itime_sel_mul *itime_old, *itime_new;
u64 scale;
int ret;
*new_gain = -1;
and add return value doc like:
* Return: 0 if an exactly matching supported new gain was found. When
a
* non-zero value is returned, the @new_gain will be set to a negative or
* positive value. The negative value means that no gain could be computed.
* Positive value will be the "best possible new gain there could be".
There
* can be two reasons why finding the "best possible" new gain is not
deemed
* successful. 1) This new value cannot be supported by the hardware.
2) The new
* gain required to maintain the scale would not be an integer. In this
case,
* the "best possible" new gain will be a floored optimal gain, which
may or
* may not be supported by the hardware.
I will fix this for v7.
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
