Skip to content

Refactor filter config for AD7192 #2742

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Refactor filter config for AD7192 #2742

wants to merge 4 commits into from

Conversation

romandariana
Copy link
Contributor

PR Description

The first 3 commits are cherry-picked from the iio tree for synchronization.

The last commit is a big rework of the driver. Instead of letting the user set the 3db filter frequency or the oversampling ratio, it makes a lot more sense to expose the filter mode (like in ad4130.c). The user can set the sampling frequency and the filter mode, which provide the user with useful information about notch placement, settling time, noise rejection. Remove the 3db and oversampling options, these are set automatically based on sampling frequency and filter mode.

PR Type

  • New feature (a change that adds new functionality)

PR Checklist

  • I have conducted a self-review of my own code changes
  • I have tested the changes on the relevant hardware

jic23 and others added 4 commits March 17, 2025 16:23
…rror handling.

Factor out everything under the lock, use guard() for the mutex to
avoid need to manually unlock, and switch sense of matching checks
in loops to reduce indent.

There are some functional changes in here as well as the code
no longer updates the filter_freq_available if no change has
been made to the oversampling ratio.

Cc: Alisa-Dariana Roman <[email protected]>
Reviewed-by: Nuno Sá <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Jonathan Cameron <[email protected]>
…direct()

These new functions allow sparse to find failures to release
direct mode reducing chances of bugs over the claim_direct_mode()
functions that are deprecated.

Cc: Alisa-Dariana Roman <[email protected]>
Reviewed-by: Nuno Sá <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Jonathan Cameron <[email protected]>
While a calibration is running, better don't make the device do anything
else.

To enforce that, grab direct mode during calibration.

Fixes: 42776c1 ("staging: iio: adc: ad7192: Add system calibration support")
Signed-off-by: Uwe Kleine-König <[email protected]>
Link: https://patch.msgid.link/8aade802afca6a89641e24c1ae1d4b8d82cff58d.1740655250.git.u.kleine-koenig@baylibre.com
Signed-off-by: Jonathan Cameron <[email protected]>
It is not useful for users to set the 3db filter frequency or the
oversampling value.

Remove these and use a filter mode instead. Expose to user the filter
mode, which provides an intuitive way to select filter behaviour.

Signed-off-by: Alisa-Dariana Roman <[email protected]>
@nunojsa
Copy link
Collaborator

nunojsa commented Mar 18, 2025

Remove the 3db and oversampling options, these are set automatically based on sampling frequency and filter mode.

Before going into the review, not sure this is acceptable. Removing attributes like this breaks ABI and potential users of it. You can ask about this upstream but typically is not doable. That means we might have to live with the old interface. Now, what you can do (if you're willing to) is to support filter_mode together with what we have now. You said something like:

"these are set automatically based on sampling frequency and filter mode"

So you can support filter_mode and set the above values automatically but still report them on the "legacy" interfaces. On the write side might be a bit more complex but should be also doable in "reusable" way. Then, we need to document this (ideally also upstream) and encourage users to use filter_mode instead of the legacy interfaces.

The above would be my approach and when sending the patch upstream, I would still question in the cover about the possibility of removing the old interfaces.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants