Re: [PATCH v2 2/5] iio: add processed write API

[David Lechner <dlechner@baylibre.com>]

Apologies if you are receiving the same message twice. Re-sending as
text email so that the mailing lists will pick up the reply.

On Wed, Oct 1, 2025 at 9:19 AM Romain Gantois
<romain.gantois@bootlin.com> wrote:
>
> Hello David,
>
> On Thursday, 25 September 2025 23:10:14 CEST David Lechner wrote:
> > On 9/25/25 7:37 AM, Romain Gantois wrote:
> > > Add a function to allow IIO consumers to write a processed value to a
> ...
> > > +static int iio_convert_processed_to_raw_unlocked(struct iio_channel
> > > *chan,
> > > +                                            int processed, int *raw,
> > > +                                            unsigned int scale)
> > > +{
> > > +   int scale_type, scale_val, scale_val2;
> > > +   int offset_type, offset_val, offset_val2;
> > > +   int ret;
> > > +
> > > +   scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
> > > +                                 IIO_CHAN_INFO_SCALE);
> > > +   if (scale_type >= 0) {
> > > +           ret = iio_divide_by_value(raw, processed, scale_type,
> scale_val,
> > > scale_val2); +              if (ret < 0)
> > > +                   return ret;
> > > +   } else {
> > > +           *raw = processed;
> > > +   }
> > > +
> > > +   if (!scale)
> > > +           return -ERANGE;
> > > +
> > > +   *raw = div_s64(*raw, scale);
> > > +
> > > +   offset_type = iio_channel_read(chan, &offset_val, &offset_val2,
> > > +                                  IIO_CHAN_INFO_OFFSET);
> > > +   if (offset_type >= 0) {
> > > +           switch (offset_type) {
> > > +           case IIO_VAL_INT:
> > > +           case IIO_VAL_INT_PLUS_MICRO:
> > > +           case IIO_VAL_INT_PLUS_NANO:
> > > +                   break;
> > > +           case IIO_VAL_FRACTIONAL:
> > > +                   offset_val /= offset_val2;
> > > +                   break;
> > > +           case IIO_VAL_FRACTIONAL_LOG2:
> > > +                   offset_val >>= offset_val2;
> > > +                   break;
> > > +           default:
> > > +                   return -EINVAL;
> > > +           }
> > > +
> > > +           *raw -= offset_val;
> > > +   }
> >
> > There are some rounding biases in this function, but I'm not sure if
> > it is worth trying to make a perfectly fair function.
>
> I'm unfamiliar with the notion of rounding bias, does it mean that nested
> calls of this function would tend to amplify rounding errors? In this case,
> would rounding to the nearest integer instead of whatever is being done by the
> integer division here be a good solution?

In this case, the issue is when you are taking multiple samples. When you
look at the average of all of the samples, you will be able to see the
bias. For example, in one of the drivers I was looking at there is an
offset of xxxx.6. Since the IIO_VAL_INT_PLUS_MICRO case is just dropping
any fractional part, the raw value will be on average 0.6 lsb lower that
the requested value. This could be a problem in an application where high
precision is required. But probably not noticeable in cases where 1 lsb is
less than the noise level.

The floor division for IIO_VAL_FRACTIONAL creates a similar bias.
DIV_ROUND_CLOSEST can help there, but even that has a small bias because
values of exactly 0.5 always get rounded in the same direction. That kind
of bias is much smaller though, so easier to ignore.


>
> Maybe I'm misunderstanding what you mean, in that case please let me know.
>
> Thanks,
>
> --
> Romain Gantois, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com