Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

Liam Beguin
2021-05-31T14:45:35Z

Hi Peter,

On Mon May 31, 2021 at 4:52 AM EDT, Peter Rosin wrote:
> Hi!
>
> Thanks for the patch!
>
> On 2021-05-30 02:59, Liam Beguin wrote:
> > From: Liam Beguin <lvb@xiphos.com>
> >
> > This is a preparatory change required for the addition of temperature
> > sensing front ends.
>
> I think this is too simplistic. I think that if the upstream iio-dev has
> an offset, it should be dealt with (i.e. be rescaled). The rescale
> driver
> cannot ignore such an upstream offset and then throw in some other
> unrelated offset of its own. That would be thoroughly confusing.

I'm not sure I fully understand. The upstream offset should be dealt
with when calling iio_read_channel_processed(). That was my main
motivation behind using the IIO core to get a processed value.

>
> Also, I see no reason of expose an offset channel if there is no offset.

You're right, I'll add conditions to only expose an offset when
required.

Liam

>
> Cheers,
> Peter
>
> > Signed-off-by: Liam Beguin <lvb@xiphos.com>
> > ---
> > drivers/iio/afe/iio-rescale.c | 12 ++++++++++--
> > 1 file changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> > index 4d0813b274d1..3bd1f11f21db 100644
> > --- a/drivers/iio/afe/iio-rescale.c
> > +++ b/drivers/iio/afe/iio-rescale.c
> > @@ -31,6 +31,7 @@ struct rescale {
> > struct iio_chan_spec_ext_info *ext_info;
> > s32 numerator;
> > s32 denominator;
> > + s32 offset;
> > };
> >
> > static int rescale_read_raw(struct iio_dev *indio_dev,
> > @@ -52,6 +53,10 @@ static int rescale_read_raw(struct iio_dev *indio_dev,
> > *val2 = rescale->denominator;
> >
> > return IIO_VAL_FRACTIONAL;
> > + case IIO_CHAN_INFO_OFFSET:
> > + *val = rescale->offset;
> > +
> > + return IIO_VAL_INT;
> > default:
> > return -EINVAL;
> > }
> > @@ -119,8 +124,10 @@ static int rescale_configure_channel(struct device *dev,
> > return -EINVAL;
> > }
> >
> > - chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> > - BIT(IIO_CHAN_INFO_SCALE);
> > + chan->info_mask_separate =
> > + BIT(IIO_CHAN_INFO_RAW) |
> > + BIT(IIO_CHAN_INFO_SCALE) |
> > + BIT(IIO_CHAN_INFO_OFFSET);
> >
> > if (iio_channel_has_available(schan, IIO_CHAN_INFO_RAW))
> > chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> > @@ -280,6 +287,7 @@ static int rescale_probe(struct platform_device *pdev)
> > rescale->cfg = of_device_get_match_data(dev);
> > rescale->numerator = 1;
> > rescale->denominator = 1;
> > + rescale->offset = 0;
> >
> > ret = rescale->cfg->props(dev, rescale);
> > if (ret)
> >