Re: [PATCH v2 2/3] iio: adc: ad7124: Remove ability to write filter_low_pass_3db_frequency

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 26 Mar 2025 15:35:04 -0300
Marcelo Schmitt <marcelo.schmitt1@gmail.com> wrote:

> On 03/17, Uwe Kleine-König wrote:
> > There are several issues with the function that implements writing to
> > the filter_low_pass_3db_frequency property:
> > 
> >  - The sinc3 factor should be 0.272 not 0.262 (this is fixed for the
> >    reading side in the previous patch).
> >  - For freq > 1 the if condition is always true so the sinc4 filter is
> >    hardly ever chosen.
> >  - In the nearly always taken if branch the filter is set to sinc3, but
> >    the frequency is set for sinc4. (And vice versa in the else branch.)
> > 
> > This is broken enough to justify the claim that there isn't any serious
> > user. Also it it counter-intuitive that setting the 3db frequency
> > modifies the sample frequency and the filter type.  
> 
> Not from engineering background but, as a Linux developer, I agree changing 3dB
> frequency to set the ODR (sampling frequency) is counter-intuitive and uncommon
> among other IIO sigma-delta ADC drivers.
First, ABI wise, any parameter is allowed to change any other - so we have
no firm rules on this.

Changing the filter type isn't completely silly as a side effect of changing
3db point as when we are selecting between fixed filters that may be precisely
what the user is trying to do.

Changing the sampling frequency is harder to argue.

Given it's horribly broken anyway I guess no one ever touched it and we are
fine to clean things up.

Jonathan

> 
> > 
> > So drop the ability to write that property.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@baylibre.com>  
> 
> Reviewed-by: Marcelo Schmitt <marcelo.schmitt@analog.com>