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

[Liam Beguin]

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@xxxxxxxxxx>
> > 
> > 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@xxxxxxxxxx>
> > ---
> >  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)
> >