On Tue, 29 Aug 2023 09:17:00 +0200
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Mon, Aug 28, 2023 at 8:18 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> > Not 100% the follow is relevant as I've lost track of the discussion
> > but maybe it is useful.
> >
> > Worth noting there are a few reasons why RAW and PROCESSED can coexist
> > in drivers and indeed why SCALE can be absent.. (OFFSET is much the same)
>
> That's fine. If we have PROCESSED the rescaler will use that first.
>
> What we're discussing are channels that have just RAW
> and no PROCESSED, nor SCALE or OFFSET yet are connected to
> a rescaler.
>
> > 1) If SCALE = 1.0 the driver is allowed not to provide it - the channel
> > might still be raw if OFFSET != 0
>
> I'm not so sure the rescaler can handle that though. Just no-one
> ran into it yet...
>
> > 2) If the channel has a horrible non linear and none invertable conversion
> > to standard units and events support the you might need PROCESSED to
> > provide the useful value, but RAW to give you clue what the current value
> > is for setting an event (light sensors are usual place we see this).
> >
> > 3) Historical ABI errors. If we first had RAW and no scale or offset because
> > we had no known values for them. Then later we discovered that there
> > was a non linear transform involved (often when someone found a magic
> > calibration code somewhere). Given the RAW interface might be in use
> > and isn't a bug as such, we can't easily remove it. The new PROCESSED
> > interface needs to be there because of the non linear transform..
> >
> > Odd corner cases... In this particular case the original code made no
> > sense but might have allowed for case 3 by accident?
>
> I think it's fine, we make PROCESSED take precedence in all cases
> as long as SCALE is not there as well.
>
> rescale_configure_channel() does this:
>
> if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) &&
> iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
> dev_info(dev, "using raw+scale source channel\n");
> } else if (iio_channel_has_info(schan, IIO_CHAN_INFO_PROCESSED)) {
> dev_info(dev, "using processed channel\n");
> rescale->chan_processed = true;
> } else {
> dev_err(dev, "source channel is not supported\n");
> return -EINVAL;
> }
>
> I think the first line should be
>
> if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) &&
> (iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE ||
> iio_channel_has_info(schan,IIO_CHAN_INFO_OFFSET)))
>
> right? We just never ran into it.
Makes sense to me.
Jonathan
>
> Yours,
> Linus Walleij
