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

From: Peter Rosin
Date: Mon May 31 2021 - 04:52:18 EST


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.

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

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