On 31/01/2019 9:35 pm, Jonathan Cameron wrote:
On Tue, 29 Jan 2019 17:11:25 +0800 Phil Reid <preid@xxxxxxxxxxxxxxxxx> wrote:G'day Alex, On 29/01/2019 4:32 pm, Alexandru Ardelean wrote:On Tue, Jan 29, 2019 at 8:28 AM Phil Reid <preid@xxxxxxxxxxxxxxxxx> wrote:Currently FRACTIONAL values are outputed with 9 digits after the decimal place. This is not always sufficient to resolve the raw value with 1 bit. Output FRACTIONAL values to 15 decimal places of precision, regardless of the number of leading zeros. Currently for a 2.5V ref with 24 bits of precision the code outputs only to 9 decimal places. Cur: 0.00014901100000000000 * 16777216 = 2499.989733 New: 0.00014901161193847600 * 16777216 = 2500.000000 Signed-off-by: Phil Reid <preid@xxxxxxxxxxxxxxxxx> --- Notes: Alternatively I could add additonal FRACTIONAL types that select the new behaviour to prevent any possible regressions. drivers/iio/industrialio-core.c | 55 ++++++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c index a062cfd..bd9da64 100644 --- a/drivers/iio/industrialio-core.c +++ b/drivers/iio/industrialio-core.c @@ -571,11 +571,53 @@ int of_iio_read_mount_matrix(const struct device *dev, #endif EXPORT_SYMBOL(of_iio_read_mount_matrix); +static ssize_t __iio_format_div_prec(char *buf, unsigned int len, s64 x, s32 y) +{ + unsigned int prec = 0; + unsigned int idx = 0; + s64 d; + + if (!len) + return 0; + + if (!y) + return snprintf(buf, len, "inf"); + + if (!x) + return snprintf(buf, len, "0"); + + if (((x > 0) && (y < 0)) || ((x < 0) && (y > 0))) { + buf[idx++] = '-'; + x = x > 0 ? x : -x; + y = y > 0 ? y : -y; + } + + d = div64_s64(x, y); + idx += snprintf(buf+idx, len-idx, "%d", (int)d); + x = x - (y * d); + if ((x != 0) && (idx < len-1)) { + buf[idx++] = '.'; + x = x * 10; + d = div64_s64(x, y); + + while ((idx < len-1) && (prec < 15)) { + if (d || prec) + prec++; + buf[idx++] = '0' + (char)d; + x = x - (y * d); + if (!x) + break; + x = x * 10; + d = div64_s64(x, y); + } + buf[idx] = 0; + } + return idx; +} + static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type, int size, const int *vals) { - unsigned long long tmp; - int tmp0, tmp1; bool scale_db = false; switch (type) { @@ -598,14 +640,9 @@ static ssize_t __iio_format_value(char *buf, size_t len, unsigned int type, else return snprintf(buf, len, "%d.%09u", vals[0], vals[1]); case IIO_VAL_FRACTIONAL: - tmp = div_s64((s64)vals[0] * 1000000000LL, vals[1]); - tmp1 = vals[1]; - tmp0 = (int)div_s64_rem(tmp, 1000000000, &tmp1); - return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1)); + return __iio_format_div_prec(buf, len, vals[0], vals[1]);Maybe I'm a bit naive, but I'm also a bit curious. If you just bump the numbers here, would it work the same ? i.e. 10^9 -> 10^15 and "snprintf(buf, len, "%d.%15u", tmp0, abs(tmp1));"I did look at that solution. But I was running into overflow issues (even with 64 bit numbers). eg: with a 2500 reference and 32 bits. 2500 * 10^15 = 2e+18 = 61 bits And the result of 2500 / 2^32 = 0.000000582076609 Only provides 9 significant digits with 15 decimal places. I was looking to provide 15 significant digits to match a standard double precision floating point value.I'll ask the awkward engineering question. Is this precision actually valid? I.e. typical voltage references are +- 0.0x % The fact we have a 32 bit ADC means you'll detect small changes, but I'm dubious about whether the absolute value will ever be 'that good'. If we are going to go out of way to support greater precision we need a strong justification of why. To take advantage of these high precision devices you need to take into account non linear effects, temperature etc. These will swamp (I think) any effect of a lack of precision the scale value.
All valid points.
9 signification digits is probably fine.
However the current formatting doesn't always provide 9 significant digits.
So I believe this can start to add significant error.
Some typical ref voltages using 32 bit scale.
scale iio scale err err%
2500 5.820766091E-07 0.000000582 -7.660913467E-11 -0.0132%
3000 6.984919310E-07 0.000000698 -4.919309616E-10 -0.0705%
3300 7.683411241E-07 0.000000768 -3.411240578E-10 -0.0444%
5000 1.164153218E-06 0.000001164 -1.532182693E-10 -0.0132%
Looking at other drivers they seem to adjust the scale figure based on gain
selection as well. Is this expected?
If so when adding gain eg: x100
scale iio scale err err%
2500 5.820766091E-09 0.000000005 -8.207660913E-10 -16%
3000 6.984919310E-09 0.000000006 -9.849193096E-10 -16%
3300 7.683411241E-09 0.000000007 -6.834112406E-10 -10%
5000 1.164153218E-08 0.000000011 -6.415321827E-10 -6%
The limited number of significant digits is swamping everything else.
even at 24bit with gain 1x00
scale iio scale err err%
2500 1.490116119E-06 0.000001490 -1.161193848E-10 -0.01%
3000 1.788139343E-06 0.000001788 -1.393432617E-10 -0.01%
3300 1.966953278E-06 0.000001966 -9.532775879E-10 -0.05%
5000 2.980232239E-06 0.000002980 -2.322387695E-10 -0.01%
Similarly this while also affect the accuracy of values mapped thru the
rescale driver.
--
Regards
Phil Reid
ElectroMagnetic Imaging Technology Pty Ltd
Development of Geophysical Instrumentation & Software
www.electromag.com.au
3 The Avenue, Midland WA 6056, AUSTRALIA
Ph: +61 8 9250 8100
Fax: +61 8 9250 7100
Email: preid@xxxxxxxxxxxxxxxxx
