Re: [PATCH v3 3/4] staging: iio: adc: ad7192: get_filter_freq code optimization

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: Deepak R Varma <mh12gx2825@gmail.com>
Cc: outreachy-kernel@googlegroups.com,
	 Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Daniel Baluta <daniel.baluta@gmail.com>,
	 kieran.bingham@ideasonboard.com,
	Lars-Peter Clausen <lars@metafoo.de>,
	 Michael Hennerich <Michael.Hennerich@analog.com>,
	Jonathan Cameron <jic23@kernel.org>,
	 Hartmut Knaack <knaack.h@gmx.de>,
	Peter Meerwald <pmeerw@pmeerw.net>,
	 linux-iio <linux-iio@vger.kernel.org>
Subject: Re: [PATCH v3 3/4] staging: iio: adc: ad7192: get_filter_freq code optimization
Date: Mon, 23 Mar 2020 01:44:20 +0200	[thread overview]
Message-ID: <CAHp75VfC=y1mHW5=ghwdMMZYg=00_a5RB0Phz4j_vM77pehvMA@mail.gmail.com> (raw)
In-Reply-To: <dad3cdb5de76234cc460300c31aea6af671f81ad.1584904896.git.mh12gx2825@gmail.com>

On Sun, Mar 22, 2020 at 9:57 PM Deepak R Varma <mh12gx2825@gmail.com> wrote:
>
> Current implementation of the function ad7192_get_available_filter_freq
> repeats calculation of output data rate a few times. We can simplify
> these steps by refactoring out the calculation of fADC. This would also
> addresses the checkpatch warning of line exceeding 80 character.

I'm not sure you did an equivalent changes. I believe in the original
code precision is better. Consider low clock frequencies when 10 bit
right shift may hide some bits of the division.

Care to write a python script to check the precision between old and new code?

...

> -       /* Formulas for filter at page 25 of the datasheet */
> -       fadc = DIV_ROUND_CLOSEST(st->fclk,
> -                                AD7192_SINC4_FILTER * AD7192_MODE_RATE(st->mode));
> -       freq[0] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> +       fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode) * 1024);
>
> -       fadc = DIV_ROUND_CLOSEST(st->fclk,
> -                                AD7192_SINC3_FILTER * AD7192_MODE_RATE(st->mode));
> -       freq[1] = DIV_ROUND_CLOSEST(fadc * 240, 1024);
> -
> -       fadc = DIV_ROUND_CLOSEST(st->fclk, AD7192_MODE_RATE(st->mode));
> -       freq[2] = DIV_ROUND_CLOSEST(fadc * 230, 1024);
> -       freq[3] = DIV_ROUND_CLOSEST(fadc * 272, 1024);
> +       /* Formulas for filter at page 25 of the datasheet */
> +       freq[0] = DIV_ROUND_CLOSEST(fadc * 240, AD7192_SINC4_FILTER);
> +       freq[1] = DIV_ROUND_CLOSEST(fadc * 240, AD7192_SINC3_FILTER);
> +       freq[2] = fadc * 230;
> +       freq[3] = fadc * 272;

-- 
With Best Regards,
Andy Shevchenko