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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0769aaae-8925-d943-e57d-c787d560a8dc@axentia.se>
Date:   Mon, 31 May 2021 10:52:05 +0200
From:   Peter Rosin <peda@...ntia.se>
To:     Liam Beguin <liambeguin@...il.com>, jic23@...nel.org,
        lars@...afoo.de, pmeerw@...erw.net
Cc:     linux-kernel@...r.kernel.org, linux-iio@...r.kernel.org,
        devicetree@...r.kernel.org, robh+dt@...nel.org
Subject: Re: [PATCH v1 4/9] iio: afe: rescale: add offset support

Hi!

Thanks for the patch!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@...hos.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.

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

Cheers,
Peter

> Signed-off-by: Liam Beguin <lvb@...hos.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)
>