lkml.org 
[lkml]   [2023]   [Oct]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] iio: afe: rescale: Accept only offset channels
On Mon, 16 Oct 2023 12:05:32 +0200
Peter Rosin <peda@axentia.se> wrote:

> Hi!
>
> 2023-10-16 at 10:39, Linus Walleij wrote:
> > On Sun, Oct 15, 2023 at 12:38 AM Peter Rosin <peda@axentia.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
>
> And I don't see why not. That's the crux.
>
> > 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.
>
> Why isn't it perfectly fine for a device to provide only a raw
> channel and then expect that to be interpreted as the real unit?
> Why would it need a processed channel when no processing is
> going on? E.g. a device reporting the temp in the expected unit
> in one of its registers. Or whatever with such a friendly
> register.

In that case it should report a processed value to indicate that.
It's admittedly a bit of a corner case given it's not processed by
the kernel - there is an argument that this (more or less) only
happens when someone has processed a raw ADC count but in theory
that's not necessarily true.

There are a few examples of drivers passing through the register value
as processed in tree - normally when there
is a microprocessor doing some fusion of signals or similar.

Raw gets reported on it's own in a few other cases, such as when
there are no known units - that happens for things like light intensity,
proximity (which is often reflected light intensity).
For those I'm not sure the rescaler is useful.

>
> And if the above holds, it should also be perfectly fine to run
> that through the rescaler.
>
> >
> >> 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!
>
> I agree that the patch does exactly as you intend. I question if
> what you intend is correct, but since I don't know the rules, I'd
> simply like to have the rules clarified.
>
> Is that clearer?
>
> Cheers,
> Peter

\
 
 \ /
  Last update: 2023-10-17 11:06    [W:0.172 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site