Re: [PATCH v6 05/13] iio: afe: rescale: add INT_PLUS_{MICRO,NANO} support

[Peter Rosin <peda@axentia.se>]

On 2021-07-21 05:06, Liam Beguin wrote:
> From: Liam Beguin <lvb@xiphos.com>
> 
> Some ADCs use IIO_VAL_INT_PLUS_{NANO,MICRO} scale types.
> Add support for these to allow using the iio-rescaler with them.
> 
> Signed-off-by: Liam Beguin <lvb@xiphos.com>
> ---
>  drivers/iio/afe/iio-rescale.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/iio/afe/iio-rescale.c b/drivers/iio/afe/iio-rescale.c
> index d0669fd8eac5..2b73047365cc 100644
> --- a/drivers/iio/afe/iio-rescale.c
> +++ b/drivers/iio/afe/iio-rescale.c
> @@ -41,6 +41,20 @@ int rescale_process_scale(struct rescale *rescale, int scale_type,
>  		do_div(tmp, 1000000000LL);
>  		*val = tmp;
>  		return scale_type;
> +	case IIO_VAL_INT_PLUS_NANO:
> +		tmp = ((s64)*val * 1000000000LL + *val2) * rescale->numerator;
> +		tmp = div_s64(tmp, rescale->denominator);
> +
> +		*val = div_s64(tmp, 1000000000LL);
> +		*val2 = tmp - *val * 1000000000LL;
> +		return scale_type;

Hi!

My objection from v5 still stands. Did you forget or did you simply send the
wrong patch?

Untested suggestion, this time handling negative values and canonicalizing any
overflow from the fraction calculation.

	neg = *val < 0 || *val2 < 0;
	tmp = (s64)abs(*val) * rescale->numerator;
	rem = do_div(tmp, rescale->denominator);
	*val = tmp;
	tmp = rem * 1000000000LL + (s64)abs(*val2) * rescale->numerator;
	do_div(tmp, rescale->denominator);
	*val2 = do_div(tmp, 1000000000LL);
	*val += tmp;
	if (neg) {
		if (*val < 0)
			*val = -*val;
		else
			*val2 = -*val;
	}

> +	case IIO_VAL_INT_PLUS_MICRO:
> +		tmp = ((s64)*val * 1000000LL + *val2) * rescale->numerator;
> +		tmp = div_s64(tmp, rescale->denominator);
> +
> +		*val = div_s64(tmp, 1000000);

Why do you not have the LL suffix here?

Cheers,
Peter

> +		*val2 = tmp - *val * 1000000;
> +		return scale_type;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>