* [PATCH] iio: afe: iio-rescale: Support processed channels
@ 2020-11-01 23:22 Linus Walleij
2020-11-15 11:21 ` Linus Walleij
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Linus Walleij @ 2020-11-01 23:22 UTC (permalink / raw)
To: Jonathan Cameron, linux-iio
Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler,
Linus Walleij, Peter Rosin
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>
---
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) |
--
2.26.2
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 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 ` (2 subsequent siblings) 3 siblings, 0 replies; 16+ messages in thread From: Linus Walleij @ 2020-11-15 11:21 UTC (permalink / raw) To: Jonathan Cameron, linux-iio Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Peter Rosin On Mon, Nov 2, 2020 at 12:22 AM 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> Any comments on this? The reason the AB8500 does not provide any scaling is that it is not linear so what can I do. Processed is what we can provide... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 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-12 12:26 ` Linus Walleij 2020-12-13 15:16 ` Andy Shevchenko 3 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2020-11-15 17:44 UTC (permalink / raw) To: Linus Walleij Cc: linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Peter Rosin 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! 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) | ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-11-15 17:44 ` Jonathan Cameron @ 2020-11-16 8:18 ` Peter Rosin 2020-12-13 12:12 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2020-11-16 8:18 UTC (permalink / raw) To: Jonathan Cameron, Linus Walleij Cc: linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler 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? 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... 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) | > -- Peter Rosin +46 730 746 224 Axentia Technologies AB ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-11-16 8:18 ` Peter Rosin @ 2020-12-13 12:12 ` Jonathan Cameron 0 siblings, 0 replies; 16+ messages in thread From: Jonathan Cameron @ 2020-12-13 12:12 UTC (permalink / raw) To: Peter Rosin Cc: Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler 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) | > > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 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-12-12 12:26 ` Linus Walleij 2020-12-12 23:22 ` Peter Rosin 2020-12-13 15:16 ` Andy Shevchenko 3 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2020-12-12 12:26 UTC (permalink / raw) To: Jonathan Cameron, linux-iio Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Peter Rosin On Mon, Nov 2, 2020 at 12:22 AM 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> Did we reach any conclusion on this? I really need to use the rescaler on an ADC that only handles processed channels... I'm sorry that I can't make this ADC disappear :D Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-12 12:26 ` Linus Walleij @ 2020-12-12 23:22 ` Peter Rosin 2020-12-13 12:16 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2020-12-12 23:22 UTC (permalink / raw) To: Linus Walleij, Jonathan Cameron, linux-iio Cc: Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On 2020-12-12 13:26, Linus Walleij wrote: > On Mon, Nov 2, 2020 at 12:22 AM 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> > > Did we reach any conclusion on this? I really need to use > the rescaler on an ADC that only handles processed channels... > > I'm sorry that I can't make this ADC disappear :D Hi! My conclusion was that the patch is buggy since it presents inconsistent information. That needs to be fixed one way or the other. If the offending information cannot be filtered out for some reason, I don't know what to do. Details in my previous comment [1]. BTW, I still do not know the answer to the .read_avail question at the end of that message, and I don't have time to dig into it. Sorry. Cheers, Peter [1] https://lore.kernel.org/linux-iio/320464d8-659c-01de-0e08-34e4c744ef16@axentia.se/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-12 23:22 ` Peter Rosin @ 2020-12-13 12:16 ` Jonathan Cameron 2020-12-14 8:34 ` Peter Rosin 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2020-12-13 12:16 UTC (permalink / raw) To: Peter Rosin Cc: Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On Sun, 13 Dec 2020 00:22:17 +0100 Peter Rosin <peda@axentia.se> wrote: > On 2020-12-12 13:26, Linus Walleij wrote: > > On Mon, Nov 2, 2020 at 12:22 AM 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> > > > > Did we reach any conclusion on this? I really need to use > > the rescaler on an ADC that only handles processed channels... > > > > I'm sorry that I can't make this ADC disappear :D > > Hi! > > My conclusion was that the patch is buggy since it presents inconsistent > information. That needs to be fixed one way or the other. If the offending > information cannot be filtered out for some reason, I don't know what to > do. Details in my previous comment [1]. BTW, I still do not know the answer > to the .read_avail question at the end of that message, and I don't have > time to dig into it. Sorry. Unless I'm missing something, I think it presents no information unless we strangely have a driver providing read_avail for _RAW but only _PROCESSED channels which is a bug. I'm not that bothered about missing information in this particular, somewhat obscure, corner case. So I think we should take the patch as it stands. It's missed the merge window now anyway unfortunately. So Peter, I would suggest we take this and perhaps revisit to tidy up loose corners when we all have more time. Thanks, Jonathan > > Cheers, > Peter > > [1] https://lore.kernel.org/linux-iio/320464d8-659c-01de-0e08-34e4c744ef16@axentia.se/ ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-13 12:16 ` Jonathan Cameron @ 2020-12-14 8:34 ` Peter Rosin 2020-12-14 15:07 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2020-12-14 8:34 UTC (permalink / raw) To: Jonathan Cameron Cc: Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On 2020-12-13 13:16, Jonathan Cameron wrote: > On Sun, 13 Dec 2020 00:22:17 +0100 > Peter Rosin <peda@axentia.se> wrote: > >> On 2020-12-12 13:26, Linus Walleij wrote: >>> On Mon, Nov 2, 2020 at 12:22 AM 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> >>> >>> Did we reach any conclusion on this? I really need to use >>> the rescaler on an ADC that only handles processed channels... >>> >>> I'm sorry that I can't make this ADC disappear :D >> >> Hi! >> >> My conclusion was that the patch is buggy since it presents inconsistent >> information. That needs to be fixed one way or the other. If the offending >> information cannot be filtered out for some reason, I don't know what to >> do. Details in my previous comment [1]. BTW, I still do not know the answer >> to the .read_avail question at the end of that message, and I don't have >> time to dig into it. Sorry. > > Unless I'm missing something, I think it presents no information unless > we strangely have a driver providing read_avail for _RAW but only > _PROCESSED channels which is a bug. I'm not that bothered about > missing information in this particular, somewhat obscure, corner case. > > So I think we should take the patch as it stands. It's missed the > merge window now anyway unfortunately. So Peter, I would suggest we > take this and perhaps revisit to tidy up loose corners when we all have > more time. My concern was a driver with a raw channel, including read_avail, providing raw sample values but that no easy conversion existed to get from that to the processed values. One option for the driver in that case would be to provide these raw values, but then have no scaling info. I.e. the way I see it, it is perfectly reasonable for a driver to provide raw with read_avail, no scaling but also processed values. And that gets transformed by the rescaler into the processed values being presented as raw, with rescaling added on top, but with the read_avail info for this new raw channel being completely wrong. For the intended driver (ab8500-gpadc) this is not the case (it has no read_avail for its raw channel). But it does have a raw channel, so adding read_avail seems easy and I can easily see other drivers already doing it. Haven't checked that though... But if you say that this never happens, fine. Otherwise, since it's too late for the merge window anyway, the patch might as well be updated such that the rescaler blocks the read_avail channel in this situation, if it exists. Cheers, Peter ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-14 8:34 ` Peter Rosin @ 2020-12-14 15:07 ` Jonathan Cameron 2020-12-14 15:30 ` Peter Rosin 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2020-12-14 15:07 UTC (permalink / raw) To: Peter Rosin Cc: Jonathan Cameron, Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On Mon, 14 Dec 2020 09:34:40 +0100 Peter Rosin <peda@axentia.se> wrote: > On 2020-12-13 13:16, Jonathan Cameron wrote: > > On Sun, 13 Dec 2020 00:22:17 +0100 > > Peter Rosin <peda@axentia.se> wrote: > > > >> On 2020-12-12 13:26, Linus Walleij wrote: > >>> On Mon, Nov 2, 2020 at 12:22 AM 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> > >>> > >>> Did we reach any conclusion on this? I really need to use > >>> the rescaler on an ADC that only handles processed channels... > >>> > >>> I'm sorry that I can't make this ADC disappear :D > >> > >> Hi! > >> > >> My conclusion was that the patch is buggy since it presents inconsistent > >> information. That needs to be fixed one way or the other. If the offending > >> information cannot be filtered out for some reason, I don't know what to > >> do. Details in my previous comment [1]. BTW, I still do not know the answer > >> to the .read_avail question at the end of that message, and I don't have > >> time to dig into it. Sorry. > > > > Unless I'm missing something, I think it presents no information unless > > we strangely have a driver providing read_avail for _RAW but only > > _PROCESSED channels which is a bug. I'm not that bothered about > > missing information in this particular, somewhat obscure, corner case. > > > > So I think we should take the patch as it stands. It's missed the > > merge window now anyway unfortunately. So Peter, I would suggest we > > take this and perhaps revisit to tidy up loose corners when we all have > > more time. > > My concern was a driver with a raw channel, including read_avail, providing > raw sample values but that no easy conversion existed to get from that to > the processed values. One option for the driver in that case would be to > provide these raw values, but then have no scaling info. Generally I resist this a lot. The reason is that it is impossible to write generic userspace software against it. The one time we did let this happen was with some of the heart rate sensors (pulse oximeters) where the algorithm to derive the eventual value is both complex - based on published literature, and proprietary (what was actually readily usable). What the measurement being provided to userspace was is well documented, but not how on earth you get from that to something useable for what the sensor is designed to measure. > I.e. the way I see > it, it is perfectly reasonable for a driver to provide raw with read_avail, > no scaling but also processed values. Why? What use would the raw values actually be? There are a couple of historical drivers where they evolved to this state, but it is not one we would normally accept. We go to a lot of effort to try and avoid this. > And that gets transformed by the > rescaler into the processed values being presented as raw, with rescaling > added on top, but with the read_avail info for this new raw channel being > completely wrong. > > For the intended driver (ab8500-gpadc) this is not the case (it has no > read_avail for its raw channel). But it does have a raw channel, so adding > read_avail seems easy and I can easily see other drivers already doing it. > Haven't checked that though... Drat. I'd failed to register this is one of those corner cases. > > But if you say that this never happens, fine. Otherwise, since it's too > late for the merge window anyway, the patch might as well be updated such > that the rescaler blocks the read_avail channel in this situation, if it > exists. That's fair enough. A sanity check and then suitable warning message to explain why it is blocked makes sense. Jonathan > > Cheers, > Peter ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-14 15:07 ` Jonathan Cameron @ 2020-12-14 15:30 ` Peter Rosin 2020-12-14 16:34 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Peter Rosin @ 2020-12-14 15:30 UTC (permalink / raw) To: Jonathan Cameron Cc: Jonathan Cameron, Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On 2020-12-14 16:07, Jonathan Cameron wrote: > On Mon, 14 Dec 2020 09:34:40 +0100 > Peter Rosin <peda@axentia.se> wrote: > >> On 2020-12-13 13:16, Jonathan Cameron wrote: >>> On Sun, 13 Dec 2020 00:22:17 +0100 >>> Peter Rosin <peda@axentia.se> wrote: >>> >>>> On 2020-12-12 13:26, Linus Walleij wrote: >>>>> On Mon, Nov 2, 2020 at 12:22 AM 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> >>>>> >>>>> Did we reach any conclusion on this? I really need to use >>>>> the rescaler on an ADC that only handles processed channels... >>>>> >>>>> I'm sorry that I can't make this ADC disappear :D >>>> >>>> Hi! >>>> >>>> My conclusion was that the patch is buggy since it presents inconsistent >>>> information. That needs to be fixed one way or the other. If the offending >>>> information cannot be filtered out for some reason, I don't know what to >>>> do. Details in my previous comment [1]. BTW, I still do not know the answer >>>> to the .read_avail question at the end of that message, and I don't have >>>> time to dig into it. Sorry. >>> >>> Unless I'm missing something, I think it presents no information unless >>> we strangely have a driver providing read_avail for _RAW but only >>> _PROCESSED channels which is a bug. I'm not that bothered about >>> missing information in this particular, somewhat obscure, corner case. >>> >>> So I think we should take the patch as it stands. It's missed the >>> merge window now anyway unfortunately. So Peter, I would suggest we >>> take this and perhaps revisit to tidy up loose corners when we all have >>> more time. >> >> My concern was a driver with a raw channel, including read_avail, providing >> raw sample values but that no easy conversion existed to get from that to >> the processed values. One option for the driver in that case would be to >> provide these raw values, but then have no scaling info. > > Generally I resist this a lot. The reason is that it is impossible to write > generic userspace software against it. The one time we did let this happen > was with some of the heart rate sensors (pulse oximeters) where the algorithm > to derive the eventual value is both complex - based on published literature, > and proprietary (what was actually readily usable). What the measurement being > provided to userspace was is well documented, but not how on earth you get from > that to something useable for what the sensor is designed to measure. > >> I.e. the way I see >> it, it is perfectly reasonable for a driver to provide raw with read_avail, >> no scaling but also processed values. > > Why? What use would the raw values actually be? There are a couple of historical > drivers where they evolved to this state, but it is not one we would normally accept. > We go to a lot of effort to try and avoid this. Drivers that have eveloved over time is exactly one such reason. E.g. a driver starts out by not caring about wrong measurements at one end of the spectrum because it is "linear enough" for the first use, someone comes along and fixes that. But by that time it's impossible to completely remove the raw channel because that would be a regression for some reason. And there you are. A driver with raw plus read_avail, no scaling but a processed channel. Or something like that... >> And that gets transformed by the >> rescaler into the processed values being presented as raw, with rescaling >> added on top, but with the read_avail info for this new raw channel being >> completely wrong. >> >> For the intended driver (ab8500-gpadc) this is not the case (it has no >> read_avail for its raw channel). But it does have a raw channel, so adding >> read_avail seems easy and I can easily see other drivers already doing it. >> Haven't checked that though... > > Drat. I'd failed to register this is one of those corner cases. I'm not sure, I just browsed the code. Maybe I misread it? Cheers, Peter >> But if you say that this never happens, fine. Otherwise, since it's too >> late for the merge window anyway, the patch might as well be updated such >> that the rescaler blocks the read_avail channel in this situation, if it >> exists. > > That's fair enough. A sanity check and then suitable warning message to explain > why it is blocked makes sense. ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-14 15:30 ` Peter Rosin @ 2020-12-14 16:34 ` Jonathan Cameron 2021-01-04 14:45 ` Linus Walleij 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2020-12-14 16:34 UTC (permalink / raw) To: Peter Rosin Cc: Jonathan Cameron, Linus Walleij, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On Mon, 14 Dec 2020 16:30:22 +0100 Peter Rosin <peda@axentia.se> wrote: > On 2020-12-14 16:07, Jonathan Cameron wrote: > > On Mon, 14 Dec 2020 09:34:40 +0100 > > Peter Rosin <peda@axentia.se> wrote: > > > >> On 2020-12-13 13:16, Jonathan Cameron wrote: > >>> On Sun, 13 Dec 2020 00:22:17 +0100 > >>> Peter Rosin <peda@axentia.se> wrote: > >>> > >>>> On 2020-12-12 13:26, Linus Walleij wrote: > >>>>> On Mon, Nov 2, 2020 at 12:22 AM 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> > >>>>> > >>>>> Did we reach any conclusion on this? I really need to use > >>>>> the rescaler on an ADC that only handles processed channels... > >>>>> > >>>>> I'm sorry that I can't make this ADC disappear :D > >>>> > >>>> Hi! > >>>> > >>>> My conclusion was that the patch is buggy since it presents inconsistent > >>>> information. That needs to be fixed one way or the other. If the offending > >>>> information cannot be filtered out for some reason, I don't know what to > >>>> do. Details in my previous comment [1]. BTW, I still do not know the answer > >>>> to the .read_avail question at the end of that message, and I don't have > >>>> time to dig into it. Sorry. > >>> > >>> Unless I'm missing something, I think it presents no information unless > >>> we strangely have a driver providing read_avail for _RAW but only > >>> _PROCESSED channels which is a bug. I'm not that bothered about > >>> missing information in this particular, somewhat obscure, corner case. > >>> > >>> So I think we should take the patch as it stands. It's missed the > >>> merge window now anyway unfortunately. So Peter, I would suggest we > >>> take this and perhaps revisit to tidy up loose corners when we all have > >>> more time. > >> > >> My concern was a driver with a raw channel, including read_avail, providing > >> raw sample values but that no easy conversion existed to get from that to > >> the processed values. One option for the driver in that case would be to > >> provide these raw values, but then have no scaling info. > > > > Generally I resist this a lot. The reason is that it is impossible to write > > generic userspace software against it. The one time we did let this happen > > was with some of the heart rate sensors (pulse oximeters) where the algorithm > > to derive the eventual value is both complex - based on published literature, > > and proprietary (what was actually readily usable). What the measurement being > > provided to userspace was is well documented, but not how on earth you get from > > that to something useable for what the sensor is designed to measure. > > > >> I.e. the way I see > >> it, it is perfectly reasonable for a driver to provide raw with read_avail, > >> no scaling but also processed values. > > > > Why? What use would the raw values actually be? There are a couple of historical > > drivers where they evolved to this state, but it is not one we would normally accept. > > We go to a lot of effort to try and avoid this. > > Drivers that have eveloved over time is exactly one such reason. E.g. a driver > starts out by not caring about wrong measurements at one end of the spectrum > because it is "linear enough" for the first use, someone comes along and fixes > that. But by that time it's impossible to completely remove the raw channel > because that would be a regression for some reason. And there you are. A > driver with raw plus read_avail, no scaling but a processed channel. Or > something like that... Yup, that's pretty much what tends to happen. I've gotten a lot stricter on checking datasheets to try and stop this happening, but still possible more will slip through (particularly as can't always get the datasheet) > > >> And that gets transformed by the > >> rescaler into the processed values being presented as raw, with rescaling > >> added on top, but with the read_avail info for this new raw channel being > >> completely wrong. > >> > >> For the intended driver (ab8500-gpadc) this is not the case (it has no > >> read_avail for its raw channel). But it does have a raw channel, so adding > >> read_avail seems easy and I can easily see other drivers already doing it. > >> Haven't checked that though... > > > > Drat. I'd failed to register this is one of those corner cases. > > I'm not sure, I just browsed the code. Maybe I misread it? It's doing both - you were right. I think there are only a small number of drivers that have that history. Looks superficially like it's easy enough to catch this corner case and block it - so lets do that. Jonathan > > Cheers, > Peter > > >> But if you say that this never happens, fine. Otherwise, since it's too > >> late for the merge window anyway, the patch might as well be updated such > >> that the rescaler blocks the read_avail channel in this situation, if it > >> exists. > > > > That's fair enough. A sanity check and then suitable warning message to explain > > why it is blocked makes sense. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-12-14 16:34 ` Jonathan Cameron @ 2021-01-04 14:45 ` Linus Walleij 2021-01-04 17:11 ` Jonathan Cameron 0 siblings, 1 reply; 16+ messages in thread From: Linus Walleij @ 2021-01-04 14:45 UTC (permalink / raw) To: Jonathan Cameron Cc: Peter Rosin, Jonathan Cameron, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On Mon, Dec 14, 2020 at 5:34 PM Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote: > On Mon, 14 Dec 2020 16:30:22 +0100 Peter Rosin <peda@axentia.se> wrote: > > >> And that gets transformed by the > > >> rescaler into the processed values being presented as raw, with rescaling > > >> added on top, but with the read_avail info for this new raw channel being > > >> completely wrong. > > >> > > >> For the intended driver (ab8500-gpadc) this is not the case (it has no > > >> read_avail for its raw channel). But it does have a raw channel, so adding > > >> read_avail seems easy and I can easily see other drivers already doing it. > > >> Haven't checked that though... > > > > > > Drat. I'd failed to register this is one of those corner cases. > > > > I'm not sure, I just browsed the code. Maybe I misread it? > > It's doing both - you were right. I think there are only a small number of > drivers that have that history. > > Looks superficially like it's easy enough to catch this corner case and > block it - so lets do that. Sorry if I am a bit confused here. I don't understand what I am supposed to do to proceed with using this driver with the ab8500 GPADC... Shall I fix something in the AB8500 GPADC as a prerequisite? In that case I think I need some more pointers... Yours, Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2021-01-04 14:45 ` Linus Walleij @ 2021-01-04 17:11 ` Jonathan Cameron 2021-01-04 18:09 ` Peter Rosin 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Cameron @ 2021-01-04 17:11 UTC (permalink / raw) To: Linus Walleij Cc: Peter Rosin, Jonathan Cameron, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On Mon, 4 Jan 2021 15:45:07 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > On Mon, Dec 14, 2020 at 5:34 PM Jonathan Cameron > <Jonathan.Cameron@huawei.com> wrote: > > On Mon, 14 Dec 2020 16:30:22 +0100 Peter Rosin <peda@axentia.se> wrote: > > > > >> And that gets transformed by the > > > >> rescaler into the processed values being presented as raw, with rescaling > > > >> added on top, but with the read_avail info for this new raw channel being > > > >> completely wrong. > > > >> > > > >> For the intended driver (ab8500-gpadc) this is not the case (it has no > > > >> read_avail for its raw channel). But it does have a raw channel, so adding > > > >> read_avail seems easy and I can easily see other drivers already doing it. > > > >> Haven't checked that though... > > > > > > > > Drat. I'd failed to register this is one of those corner cases. > > > > > > I'm not sure, I just browsed the code. Maybe I misread it? > > > > It's doing both - you were right. I think there are only a small number of > > drivers that have that history. > > > > Looks superficially like it's easy enough to catch this corner case and > > block it - so lets do that. > > Sorry if I am a bit confused here. I don't understand what I am supposed > to do to proceed with using this driver with the ab8500 GPADC... > > Shall I fix something in the AB8500 GPADC as a prerequisite? > In that case I think I need some more pointers... I confess I'm a bit lost, but I 'think' the problem we had left was around read_avail which doesn't play well if we it defined for the _raw value in the provider, but not the _processed value. So if we detect their is a _processed channel (which we are going to use) we just need to make sure that we don't pass the read_avail for _raw through to be exposed by the rescale driver as the consumer as it will be garbage. Best plan is probably to just pretend the read_avail for the provider doesn't exist in this case. @Peter, does that cover it of are there other similar cases? It definitely also wants a big fat comment saying why we are hiding this! Jonathan > > Yours, > Linus Walleij ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2021-01-04 17:11 ` Jonathan Cameron @ 2021-01-04 18:09 ` Peter Rosin 0 siblings, 0 replies; 16+ messages in thread From: Peter Rosin @ 2021-01-04 18:09 UTC (permalink / raw) To: Jonathan Cameron, Linus Walleij Cc: Jonathan Cameron, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler On 2021-01-04 18:11, Jonathan Cameron wrote: > On Mon, 4 Jan 2021 15:45:07 +0100 > Linus Walleij <linus.walleij@linaro.org> wrote: > >> On Mon, Dec 14, 2020 at 5:34 PM Jonathan Cameron >> <Jonathan.Cameron@huawei.com> wrote: >>> On Mon, 14 Dec 2020 16:30:22 +0100 Peter Rosin <peda@axentia.se> wrote: >> >>>>>> And that gets transformed by the >>>>>> rescaler into the processed values being presented as raw, with rescaling >>>>>> added on top, but with the read_avail info for this new raw channel being >>>>>> completely wrong. >>>>>> >>>>>> For the intended driver (ab8500-gpadc) this is not the case (it has no >>>>>> read_avail for its raw channel). But it does have a raw channel, so adding >>>>>> read_avail seems easy and I can easily see other drivers already doing it. >>>>>> Haven't checked that though... >>>>> >>>>> Drat. I'd failed to register this is one of those corner cases. >>>> >>>> I'm not sure, I just browsed the code. Maybe I misread it? >>> >>> It's doing both - you were right. I think there are only a small number of >>> drivers that have that history. >>> >>> Looks superficially like it's easy enough to catch this corner case and >>> block it - so lets do that. >> >> Sorry if I am a bit confused here. I don't understand what I am supposed >> to do to proceed with using this driver with the ab8500 GPADC... >> >> Shall I fix something in the AB8500 GPADC as a prerequisite? >> In that case I think I need some more pointers... > > I confess I'm a bit lost, but I 'think' the problem we had > left was around read_avail which doesn't play well if we > it defined for the _raw value in the provider, but not the _processed value. > > So if we detect their is a _processed channel (which we are going to use) we > just need to make sure that we don't pass the read_avail for _raw through > to be exposed by the rescale driver as the consumer as it will be garbage. > Best plan is probably to just pretend the read_avail for the provider doesn't > exist in this case. > > @Peter, does that cover it of are there other similar cases? Yes, that's it. Just hide _raw in read_avail if we are proceding with _processed as _raw. > It definitely also wants a big fat comment saying why we are hiding this! Yup. Cheers, Peter ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] iio: afe: iio-rescale: Support processed channels 2020-11-01 23:22 [PATCH] iio: afe: iio-rescale: Support processed channels Linus Walleij ` (2 preceding siblings ...) 2020-12-12 12:26 ` Linus Walleij @ 2020-12-13 15:16 ` Andy Shevchenko 3 siblings, 0 replies; 16+ messages in thread From: Andy Shevchenko @ 2020-12-13 15:16 UTC (permalink / raw) To: Linus Walleij Cc: Jonathan Cameron, linux-iio, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Peter Rosin On Mon, Nov 2, 2020 at 1:23 AM 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. ... > 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; A nit: Move this to the end of the branch, so in both branches the ret = will be the last code line (for better readability). > + *val = 1; > + *val2 = 1; > + } else { > + ret = iio_read_channel_scale(rescale->source, val, val2); > + } -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2021-01-04 18:10 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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).