linux-kernel - Re: [PATCH v6 05/13] iio: afe: rescale: add INT_PLUS

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: <c9d77dc0-7f4c-0df0-cce1-8cb30074e115@axentia.se>
Date:   Fri, 23 Jul 2021 23:16:53 +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 v6 05/13] iio: afe: rescale: add INT_PLUS_{MICRO,NANO}
 support

On 2021-07-21 05:06, Liam Beguin wrote:
> From: Liam Beguin <lvb@...hos.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@...hos.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;
>  	}
>