Hi! 2023-10-16 at 10:39, Linus Walleij wrote: > On Sun, Oct 15, 2023 at 12:38 AM Peter Rosin <peda@xxxxxxxxxx> 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. 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