From: Jonathan Cameron <jic23@kernel.org>
To: Peter Rosin <peda@axentia.se>
Cc: Linus Walleij <linus.walleij@linaro.org>,
linux-iio@vger.kernel.org, Hartmut Knaack <knaack.h@gmx.de>,
Lars-Peter Clausen <lars@metafoo.de>,
Peter Meerwald-Stadler <pmeerw@pmeerw.net>
Subject: Re: [PATCH] iio: afe: iio-rescale: Support processed channels
Date: Sun, 13 Dec 2020 12:12:19 +0000 [thread overview]
Message-ID: <20201213121219.6623bb13@archlinux> (raw)
In-Reply-To: <320464d8-659c-01de-0e08-34e4c744ef16@axentia.se>
On Mon, 16 Nov 2020 09:18:24 +0100
Peter Rosin <peda@axentia.se> wrote:
> Hi!
>
> On 2020-11-15 18:44, Jonathan Cameron wrote:
> > On Mon, 2 Nov 2020 00:22:11 +0100
> > Linus Walleij <linus.walleij@linaro.org> wrote:
> >
> >> It happens that an ADC will only provide raw or processed
> >> voltage conversion channels. (adc/ab8500-gpadc.c).
> >> On the Samsung GT-I9070 this is used for a light sensor
> >> and current sense amplifier so we need to think of something.
> >>
> >> The idea is to allow processed channels and scale them
> >> with 1/1 and then the rescaler can modify the result
> >> on top.
> >>
> >> Cc: Peter Rosin <peda@axentia.se>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >
> > Sorry, I kept leaving this to last as it was in the 'needed thought'
> > pile - then running out of time and not getting to it.
> >
> > Anyhow, I think this is the best we can do for the situation
> > you describe so I'm happy with this.
> >
> > @Peter, I definitely want your input on this one as well though
> > before I apply it!
>
> Yes, sorry about the delay. Same pile here, amplified with way too much
> to do at work. My immediate reaction was that this is not that simple,
> but after looking at it for a few minutes I also came to think that it's
> perhaps the best that can be done.
>
> But it's been a while, so it just took a while for things to dawn on me.
>
> The rescaler passes on IIO_CHAN_INFO_RAW as-is to the underlying
> driver in the .read_avail op, and this patch xforms the processed
> channel into the raw channel. So that is a mismatch. I don't think
> it's easily fixable in the general case because the processed channel
> rarely, if ever, implements .read_avail?
There is nothing stopping them doing so if we have a particular usecase
that requires it. To be honest, very few drivers implement read_avail
at all yet!
> And I don't know if it
> is allowed to return -EINVAL for the .read_avail op for the raw
> channel, because that would be the obvious patch to squash-in...
I'm not sure it matters. As things stand the rescale_configure_channel
queries if read_avail is available for the _RAW element only.
So currently we'd just not register that at all if only processed
is available.
It might be a nice to have, but there are plenty of other cases
where read_avail isn't provided and this driver might be used
so I'm not that fussed.
Thanks,
Jonathan
>
> Cheers,
> Peter
>
>
> > Jonathan
> >
> >
> >> ---
> >> drivers/iio/afe/iio-rescale.c | 31 +++++++++++++++++++++++++++----
> >> 1 file changed, 27 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> >> index e42ea2b1707d..ea90034cb257 100644
> >> --- a/drivers/iio/afe/iio-rescale.c
> >> +++ b/drivers/iio/afe/iio-rescale.c
> >> @@ -29,6 +29,7 @@ struct rescale {
> >> struct iio_channel *source;
> >> struct iio_chan_spec chan;
> >> struct iio_chan_spec_ext_info *ext_info;
> >> + bool chan_processed;
> >> s32 numerator;
> >> s32 denominator;
> >> };
> >> @@ -43,10 +44,27 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
> >>
> >> switch (mask) {
> >> case IIO_CHAN_INFO_RAW:
> >> - return iio_read_channel_raw(rescale->source, val);
> >> + if (rescale->chan_processed)
> >> + /*
> >> + * When only processed channels are supported, we
> >> + * read the processed data and scale it by 1/1
> >> + * augmented with whatever the rescaler has calculated.
> >> + */
> >> + return iio_read_channel_processed(rescale->source, val);
> >> + else
> >> + return iio_read_channel_raw(rescale->source, val);
> >>
> >> case IIO_CHAN_INFO_SCALE:
> >> - ret = iio_read_channel_scale(rescale->source, val, val2);
> >> + if (rescale->chan_processed) {
> >> + /*
> >> + * Processed channels are scaled 1-to-1
> >> + */
> >> + ret = IIO_VAL_FRACTIONAL;
> >> + *val = 1;
> >> + *val2 = 1;
> >> + } else {
> >> + ret = iio_read_channel_scale(rescale->source, val, val2);
> >> + }
> >> switch (ret) {
> >> case IIO_VAL_FRACTIONAL:
> >> *val *= rescale->numerator;
> >> @@ -132,8 +150,13 @@ static int rescale_configure_channel(struct device *dev,
> >>
> >> if (!iio_channel_has_info(schan, IIO_CHAN_INFO_RAW) ||
> >> !iio_channel_has_info(schan, IIO_CHAN_INFO_SCALE)) {
> >> - dev_err(dev, "source channel does not support raw/scale\n");
> >> - return -EINVAL;
> >> + 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 does not support raw+scale or processed data\n");
> >> + return -EINVAL;
> >> + }
> >> }
> >>
> >> chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> >
>
next prev parent reply other threads:[~2020-12-13 12:13 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-01 23:22 [PATCH] iio: afe: iio-rescale: Support processed channels Linus Walleij
2020-11-15 11:21 ` Linus Walleij
2020-11-15 17:44 ` Jonathan Cameron
2020-11-16 8:18 ` Peter Rosin
2020-12-13 12:12 ` Jonathan Cameron [this message]
2020-12-12 12:26 ` Linus Walleij
2020-12-12 23:22 ` Peter Rosin
2020-12-13 12:16 ` Jonathan Cameron
2020-12-14 8:34 ` Peter Rosin
2020-12-14 15:07 ` Jonathan Cameron
2020-12-14 15:30 ` Peter Rosin
2020-12-14 16:34 ` Jonathan Cameron
2021-01-04 14:45 ` Linus Walleij
2021-01-04 17:11 ` Jonathan Cameron
2021-01-04 18:09 ` Peter Rosin
2020-12-13 15:16 ` Andy Shevchenko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20201213121219.6623bb13@archlinux \
--to=jic23@kernel.org \
--cc=knaack.h@gmx.de \
--cc=lars@metafoo.de \
--cc=linus.walleij@linaro.org \
--cc=linux-iio@vger.kernel.org \
--cc=peda@axentia.se \
--cc=pmeerw@pmeerw.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).