mbox series

[0/2] iio: adc: ad7124: Fix 3dB filter frequency reading

Message ID cover.1741801853.git.u.kleine-koenig@baylibre.com (mailing list archive)
Headers show
Series iio: adc: ad7124: Fix 3dB filter frequency reading | expand

Message

Uwe Kleine-König March 12, 2025, 6:38 p.m. UTC
Hello,

here comes another fix for the ad7124: The getter function for the
filter_low_pass_3db_frequency sysfs property used wrong factors to
calculate the f_{3dB}.

The first patch is a cleanup I implemented before I noticed the issue. I
didn't switch their ordering because I was lazy. If I continue to
discover issues in the ad7124 driver at that rate, swapping for this one
fix doesn't really matter :-)

Note the setter function is still broken. And it's worse enough that I
don't know how to fix it at all. The relevant part of the function looks
as follows:

	sinc4_3db_odr = DIV_ROUND_CLOSEST(freq * 1000, 230);
	sinc3_3db_odr = DIV_ROUND_CLOSEST(freq * 1000, 262);

	if (sinc4_3db_odr > sinc3_3db_odr) {
		new_filter = AD7124_FILTER_FILTER_SINC3;
		new_odr = sinc4_3db_odr;
	} else {
		new_filter = AD7124_FILTER_FILTER_SINC4;
		new_odr = sinc3_3db_odr;
	}

The issues I'm aware of in this function are:

 - the sinc3 factor should be 0.272 not 0.262 (which is fixed for the
   getter in patch #2)
 - for freq > 1 the if condition is always true
 - 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.)

Also it's unclear to me why sinc4_3db_odr > sinc3_3db_odr is the test to
decide between the two branches. Maybe something like

	if (abs(sinc4_3db_odr - current_odr) < abs(sinc3_3db_odr - current_odr))
		use_sinc4()
	else
		use_sinc3()

would make more sense.

I intend to add a filter_type property to the driver next. When this is
implemented setting the filter_low_pass_3db_frequency shouldn't be
needed any more and we can either keep the function as is (and
discourage its use) or just drop it.

Best regards
Uwe

Uwe Kleine-König (2):
  iio: adc: ad7124: Make register naming consistent
  iio: adc: ad7124: Fix 3dB filter frequency reading

 drivers/iio/adc/ad7124.c | 176 +++++++++++++++++++--------------------
 1 file changed, 85 insertions(+), 91 deletions(-)


base-commit: 8dbeb413806f9f810d97d25284f585b201aa3bdc