linux-kernel - Re: [PATCH v2] iio: afe: rescale: Accept only offset channels

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACRpkdaMj2XFcwqaSiRSA2A41zYsW=AG_Fi7v7AjbuLBByxVaQ@mail.gmail.com>
Date:   Mon, 16 Oct 2023 10:39:54 +0200
From:   Linus Walleij <linus.walleij@...aro.org>
To:     Peter Rosin <peda@...ntia.se>
Cc:     Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Liam Beguin <liambeguin@...il.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] iio: afe: rescale: Accept only offset channels

On Sun, Oct 15, 2023 at 12:38 AM Peter Rosin <peda@...ntia.se> wrote:
> 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 = true;

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

Yours,
Linus Walleij