Re: [PATCH v2 4/4] iio: adc: ad7768-1: add support for ADAQ776x-1 ADC Family

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Fri, Aug 29, 2025 at 10:11 PM Jonathan Santos <jonath4nns@gmail.com> wrote:
> On 08/24, Andy Shevchenko wrote:
> > On Sun, Aug 24, 2025 at 7:10 AM Jonathan Santos

...

> > > +       unsigned long denominator, numerator;
> >
> > struct u32_fract fract;
>
> Problem is that rational_best_approximation requires a unsigned long. I
> could do:
>
> rational_best_approximation(fract.numerator, fract.denominator,
>                                             INT_MAX, INT_MAX,
>                                             (unsigned long *)&fract.numerator,
>                                             (unsigned long *)&fract.denominator);

No, this is not good. And it will overflow. See below for a better alternative.

> But I don't know if this is the best practice.

The best practice is to add a patch to extend that to cover unsigned
long and long
https://elixir.bootlin.com/linux/v6.17-rc3/source/include/linux/math.h#L122
It's just a couple of lines which I review and acknowledge
immediately. (Just make them a precursor patch)

BUT, looking at the code, I don't see that people use the same
variables for input and output (otherwise it's also considered not the
best practice, for example, in the cases when input can be made a
const).

That said, the patching math.h is optional, either way, make sure your
input and output are different variables.

> One way around this would be to represent the gains in fractions instead of 1000x, example:
> then we could drop the rational approximation.

I like the use of rational best approximation. I think it gives a
really nice algo for simplifying fractions.


-- 
With Best Regards,
Andy Shevchenko