Re: [PATCH] iio: afe: rescale: Fix logic bug

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

 



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.

Yours,
Linus Walleij




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux