On 31/10/23 17:41, Matti Vaittinen wrote:
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?
Hi Matti,
Sorry, got busy with my full time job.
It's nice to see that you have found the issue without my test results:)
Please find below my tests -
root@stm32mp1:/sys/bus/iio/devices/iio:device1# cat scale_available
14.009712000 4.669904000 2.334952000 1.751214000 1.556634666 0.875607000 0.778317333 0.583738000 0.437803500 0.291869000 0.218901750 0.194579333 0.145934500 0.109450875 0.097289666 0.072967250 0.048644833 0.036483625
0.024322416 0.018241812 0.012161208 0.006080604
root@stm32mp1:/sys/bus/iio/devices/iio:device1# echo 0.875607000 > scale ## This works
root@stm32mp1:/sys/bus/iio/devices/iio:device1# echo 0.097289666 > scale ## This fails
root@stm32mp1:/sys/bus/iio/devices/iio:device1# echo 0.097289667 > scale ## However if I add 1, it works! I figured, its a rounding issue so used this trick: "if (val2 % 10) val2 += 1;"
I am sorry, I haven't gone through the full gts internals and only used your driver as a reference to understand it's implementation. I do not have any thoughts on top of my head now but let me go through the code.
Regards,
Subhajit Ghosh
