Re: [PATCH v1 2/9] iio: inkern: error out on unsupported offset type

[Peter Rosin]

Hi!

On 2021-05-30 02:59, Liam Beguin wrote:
> From: Liam Beguin <lvb@xxxxxxxxxx>
> 
> iio_convert_raw_to_processed_unlocked() assumes the offset is an
> integer. Make that clear to the consumer by returning an error when an
> unsupported offset type is detected.
> 
> Signed-off-by: Liam Beguin <lvb@xxxxxxxxxx>
> ---
>  drivers/iio/inkern.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 4b6a8e11116a..dede4536d499 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -595,8 +595,12 @@ static int iio_convert_raw_to_processed_unlocked(struct iio_channel *chan,
>  	int ret;
>  
>  	ret = iio_channel_read(chan, &offset, NULL, IIO_CHAN_INFO_OFFSET);
> -	if (ret >= 0)
> +	if (ret == IIO_VAL_INT) {
>  		raw64 += offset;
> +	} else if (ret >= 0) {
> +		dev_err(&chan->indio_dev->dev, "unsupported offset type");
> +		return -EINVAL;
> +	}
>  
>  	scale_type = iio_channel_read(chan, &scale_val, &scale_val2,
>  					IIO_CHAN_INFO_SCALE);
> 

This breaks the implicit truncation that happens for drivers that have
offsets of type IIO_VAL_INT_PLUS_{MICRO_DB,MICRO,NANO}

Implicit truncation might be more appropriate than an error?

However, to error out on fractional offsets etc still seem appropriate, but
there are corner cases where the existing code did the right thing. E.g.
a denominator of one or a fractional-log2 of zero, but a big denominator and
a smaller numerator would also just result in a relatively harmless truncation.

I don't know if it's really right to just break that?

Cheers,
Peter