Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
MIME-Version: 1.0
References: <20230902-iio-rescale-only-offset-v2-1-988b807754c8@linaro.org> <a0a55276-297b-e2b7-79b3-e4aadd39b400@axentia.se>
In-Reply-To: <a0a55276-297b-e2b7-79b3-e4aadd39b400@axentia.se>
From:   Linus Walleij <linus.walleij@linaro.org>
Date:   Mon, 16 Oct 2023 10:39:54 +0200
Message-ID: <CACRpkdaMj2XFcwqaSiRSA2A41zYsW=AG_Fi7v7AjbuLBByxVaQ@mail.gmail.com>
Subject: Re: [PATCH v2] iio: afe: rescale: Accept only offset channels
To:     Peter Rosin <peda@axentia.se>
Cc:     Jonathan Cameron <jic23@kernel.org>,
        Lars-Peter Clausen <lars@metafoo.de>,
        Liam Beguin <liambeguin@gmail.com>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Sun, Oct 15, 2023 at 12:38=E2=80=AFAM Peter Rosin <peda@axentia.se> wrot=
e:
> 2023-09-02 at 21:46, Linus Walleij wrote:

> >       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");
> > +         (iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE) ||
> > +          iio_channel_has_info(schan, IIO_CHAN_INFO_OFFSET))) {
> > +             dev_info(dev, "using raw+scale/offset source channel\n");
>
> If the rules really are that when not provided scale is 1 and offset 0
> (reasonable of course) then the above still looks suspect to me. Should
> this part not simply be
>
>         if (iio_channel_has_info(schan, IIO_CHAN_INFO_RAW)) {
>                 dev_info(dev, "using raw source channel\n");
>
> in that case?

The patch is based on Jonathan's comment that while we currently
support raw+scale, having just raw+offset provided is a possibility.

The if()-clause above (which I guess you are commenting) is meant
as "take this path if scale or offset or both are provided".

Just raw (with neither offset or rescale) doesn't make sense, since
the AFE rescaler does just offsetting and rescaling, in that case the
user should just use the raw channel. Also it would then take
precedence over a processed channel (which applies rescale and
offset internally) which doesn't make sense to me.

> Or was "raw + processed" some kind of special case that we want to handle
> as processed? If that's the case then we need to have more complex logic.

Processed is on the else-path, which will be tried only when neither
scale nor offset is provided:

>       } else if (iio_channel_has_info(schan, IIO_CHAN_INFO_PROCESSED)) {
>               dev_info(dev, "using processed channel\n");
>               rescale->chan_processed =3D true;

I'm not sure I fully understood the remark, please elaborate if I got it wr=
ong!

Yours,
Linus Walleij