On 10/30/23 12:21, Matti Vaittinen wrote:
Hi dee Ho peeps,
On 10/29/23 17:51, Matti Vaittinen wrote:
On 10/28/23 18:20, Jonathan Cameron wrote:
On Fri, 27 Oct 2023 18:15:45 +1030
Subhajit Ghosh <subhajit.ghosh@xxxxxxxxxxxxxx> wrote:
Driver support for Avago (Broadcom) APDS9306 Ambient Light Sensor
with als
and clear channels with i2c interface. Hardware interrupt
configuration is
optional. It is a low power device with 20 bit resolution and has
configurable adaptive interrupt mode and interrupt persistence mode.
The device also features inbuilt hardware gain, multiple integration
time
selection options and sampling frequency selection options.
...
+static int apds9306_scale_set(struct apds9306_data *data, int val,
int val2)
+{
+ int i, ret, time_sel, gain_sel;
+
+ /* Rounding up the last digit by one, otherwise matching table
fails! */
Interesting. Sounds like a question for Matti?
Sounds odd indeed. I assume this happens when scale setting is
requested using one of the exact values advertised by the available
scales from the GTS? This does not feel right and the +1 does not ring
a bell to me. I need to investigate what's going on. It would help if
you could provide the values used as val and val2 for the setting.
This will take a while from me though - I'll try to get to this next
week. Thanks for pointing out the anomaly!
I think I have a rough understanding. I did a Kunit test which goes
through all the available scales values from the
gts->avail_all_scales_table and all integration times, and feeds them to
the logic below. It seems the first culprit is hit by:
val = 0, val2 = 125025502.
Problem is that the 125025502 is rounded. The exact linearized NANO
scale resulting from time multiplier 128, gain multiplier 1 is
125025502.5 - which means we will see rounding.
+ if (val2 % 10)
+ val2 += 1;
For a while I was unsure if this check works for all cases because I see
linearized scales:
250051005 - multipliers 1x, 64x
83350335 - multipliers 3x, 64x and 6x, 32x
27783445 - multipliers 9x, 64x.
For those we will get + 1 added to val2 even though there is no
rounding. It appears this is not a problem because the
iio_gts_get_gain() (which is used to figure out the required total gain
to get the desired scale) does not require the scale to be formed by
exact multiples of gain.
...
I think it would be very nice if the gts-helpers could do the rounding
when computing the available scales, but that'd require some thinking.
Fixup patch is still very welcome ;)
I did some further experimenting. Basically, I did a "hack" which always
rounds up the available-scales values if division results a remainder.
This way the values advertised by the available_scales did find the
matching table.
It is a tiny bit icky because for example the scale 6945861.25 becomes
6945862 in available-scales. Also, I assume that if we "hack" just the
available-scales and don't fix the rest of the logic, setting 6945862
will read back as 6945861 (I haven't tested this though). Also, the
20837583.75 will be 20837583 in available-scales but 20837582 when read
back, resulting small error. (I haven't tested this either but I assume
the current GTS code is flooring the 20837583.75 to 20837583.
I am wondering if changing the iio_gts_get_gain() to do rounding instead
of flooring and changing also the iio_gts_total_gain_to_scale() to
something like:
int iio_gts_total_gain_to_scale(struct iio_gts *gts, int total_gain,
int *scale_int, int *scale_nano)
{
u64 tmp;
int rem;
tmp = gts->max_scale;
rem = do_div(tmp, total_gain);
if (total_gain > 1 && rem >= total_gain / 2)
tmp += 1ULL;
return iio_gts_delinearize(tmp, NANO, scale_int, scale_nano);
}
would do the trick. It's just that I'm a bit afraid of touching the
iio_gts_get_gain() - by the very least I need to fire up the GTS tests
which I implemented but are not in-tree due to the test-device
dependency... :/
Any thoughts?
+
+ ret = iio_gts_find_gain_sel_for_scale_using_time(&data->gts,
+ data->intg_time_idx, val, val2, &gain_sel);
+ if (ret) {
+ for (i = 0; i < data->gts.num_itime; i++) {
+ time_sel = data->gts.itime_table[i].sel;
+
+ if (time_sel == data->intg_time_idx)
+ continue;
+
+ ret =
iio_gts_find_gain_sel_for_scale_using_time(&data->gts,
+ time_sel, val, val2, &gain_sel);
+ if (!ret)
+ break;
+ }
+ if (ret)
+ return -EINVAL;
+
+ ret = apds9306_intg_time_set_hw(data, time_sel);
+ if (ret)
+ return ret;
+ }
+
+ return apds9306_gain_set_hw(data, gain_sel);
+}
Yours,
-- Matti
--
Matti Vaittinen
Linux kernel developer at ROHM Semiconductors
Oulu Finland
~~ When things go utterly wrong vim users can always type :help! ~~
