Re: [PATCH 1/1] iio: core: Improve precision of __iio_format_value for FRACTIONAL values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

Proposed solution seemed to work with a wider range, but I admit it's not elegant.
Certainly interested in other peoples ideas.

My alternative thought was to introduce a new type that returns the scale
as a rational. eg: return string like
scale_rational = 2500/4294967296

But that'd require existing user space to become aware of the new format.



But in any case, what would be interesting now, is to extend the IIO
core logic to provide [somehow] a precision number, default being 9.
This could probably be specified on per-channel basis [somehow],
similar to other channel params.

So, for example in the default case, if you have "uint32_t precision =
9", you would have the same behavior, with something like

       tmp = div_s64((s64)vals[0] * pow_of_10(precision), vals[1]);
       tmp1 = vals[1];
       tmp0 = (int)div_s64_rem(tmp, pow_of_10(precision), &tmp1);
       return snprintf(buf, len, "%d.%" precision "u", tmp0, abs(tmp1));

Obviously, the above code is just pseudo-code, where pow_of_10()
multiplies 10 a "precision" number of times, and the snprintf() would
need a temporary buffer to create a format string, which then would be
used.

Good idea to have some kind of overide, I'll have a look


Thanks
Alex

         case IIO_VAL_FRACTIONAL_LOG2:
-               tmp = shift_right((s64)vals[0] * 1000000000LL, vals[1]);
-               tmp0 = (int)div_s64_rem(tmp, 1000000000LL, &tmp1);
-               return snprintf(buf, len, "%d.%09u", tmp0, abs(tmp1));
+               return __iio_format_div_prec(buf, len, vals[0], 1 << vals[1]);
         case IIO_VAL_INT_MULTIPLE:
         {
                 int i;
--
1.8.3.1





--
Regards
Phil Reid




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux