[PATCH 0/6] iio: adc: ad7124: add filter support

[PATCH 0/6] iio: adc: ad7124: add filter support

[David Lechner]

The AD7124 family of chips supports a number of different filter modes.
This series eventually gets around to adding support for selecting the
filter mode at runtime after first doing some cleanups to the existing
code.

The first 3 patches are just updating things to use newer APIs.

The 4th patch is addressing a shortcoming in the driver where the
sampling_frequency attributes were being limited to an integer value.

The 5th patch is the one that finally adds support for filter_type
and filter_type_available attributes.

And the last patch documents one new filter_type that was used that we
haven't seen before.

This applies on top of "[PATCH v3] iio: adc: ad7124: fix sample rate for
multi-channel use"

---
David Lechner (6):
      iio: adc: ad7124: use clamp()
      iio: adc: ad7124: use read_avail() for scale_available
      iio: adc: ad7124: use guard(mutex) to simplify return paths
      iio: adc: ad7124: support fractional sampling_frequency
      iio: adc: ad7124: add filter support
      iio: ABI: document "sinc4+rej60" filter_type

 Documentation/ABI/testing/sysfs-bus-iio |   1 +
 drivers/iio/adc/ad7124.c                | 380 +++++++++++++++++++++++++-------
 2 files changed, 297 insertions(+), 84 deletions(-)
---
base-commit: d1487b0b78720b86ec2a2ac7acc683ec90627e5b
change-id: 20250725-iio-adc-ad7124-add-filter-support-d1c9e53f64b5
prerequisite-message-id: 20250905-iio-adc-ad7124-fix-samp-freq-for-multi-channel-v3-1-702ff014ec61@baylibre.com
prerequisite-patch-id: b70de00c7ae218fa6c1e6aca34fb4210922f6d86

Best regards,
-- 
David Lechner <dlechner@baylibre.com>

[PATCH 1/6] iio: adc: ad7124: use clamp()

[David Lechner]

Use clamp() instead of open-coding clamping.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 70f458e3ccc12db884dda9003abcffdf48989e5e..117777fc8ad05b773da09c113cf84927c75d6b7b 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -15,6 +15,7 @@
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/kfifo.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <linux/mod_devicetable.h>
 #include <linux/property.h>
@@ -301,11 +302,7 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
 	 * FS[10:0] can have a value from 1 to 2047
 	 */
 	factor = 32 * 4; /* N = 4 for default sinc4 filter. */
-	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor);
-	if (odr_sel_bits < 1)
-		odr_sel_bits = 1;
-	else if (odr_sel_bits > 2047)
-		odr_sel_bits = 2047;
+	odr_sel_bits = clamp(DIV_ROUND_CLOSEST(fclk, odr * factor), 1, 2047);
 
 	if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)
 		st->channels[channel].cfg.live = false;

-- 
2.43.0

[PATCH 2/6] iio: adc: ad7124: use read_avail() for scale_available

[David Lechner]

Replace custom attribute with the standard IIO read_avail() callback
to provide in_voltage_scale_available attribute.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 117777fc8ad05b773da09c113cf84927c75d6b7b..6c17cc59f33c6ddc241e94d1b0f43bceced1e719 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -623,6 +623,33 @@ static const struct ad_sigma_delta_info ad7124_sigma_delta_info = {
 	.num_resetclks = 64,
 };
 
+static const int ad7124_voltage_scales[][2] = {
+	{ 0, 1164 },
+	{ 0, 2328 },
+	{ 0, 4656 },
+	{ 0, 9313 },
+	{ 0, 18626 },
+	{ 0, 37252 },
+	{ 0, 74505 },
+	{ 0, 149011 },
+	{ 0, 298023 },
+};
+
+static int ad7124_read_avail(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     const int **vals, int *type, int *length, long info)
+{
+	switch (info) {
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)ad7124_voltage_scales;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(ad7124_voltage_scales) * 2;
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
 static int ad7124_read_raw(struct iio_dev *indio_dev,
 			   struct iio_chan_spec const *chan,
 			   int *val, int *val2, long info)
@@ -777,18 +804,6 @@ static int ad7124_reg_access(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static IIO_CONST_ATTR(in_voltage_scale_available,
-	"0.000001164 0.000002328 0.000004656 0.000009313 0.000018626 0.000037252 0.000074505 0.000149011 0.000298023");
-
-static struct attribute *ad7124_attributes[] = {
-	&iio_const_attr_in_voltage_scale_available.dev_attr.attr,
-	NULL,
-};
-
-static const struct attribute_group ad7124_attrs_group = {
-	.attrs = ad7124_attributes,
-};
-
 static int ad7124_update_scan_mode(struct iio_dev *indio_dev,
 				   const unsigned long *scan_mask)
 {
@@ -818,12 +833,12 @@ static int ad7124_update_scan_mode(struct iio_dev *indio_dev,
 }
 
 static const struct iio_info ad7124_info = {
+	.read_avail = ad7124_read_avail,
 	.read_raw = ad7124_read_raw,
 	.write_raw = ad7124_write_raw,
 	.debugfs_reg_access = &ad7124_reg_access,
 	.validate_trigger = ad_sd_validate_trigger,
 	.update_scan_mode = ad7124_update_scan_mode,
-	.attrs = &ad7124_attrs_group,
 };
 
 /* Only called during probe, so dev_err_probe() can be used */
@@ -1013,6 +1028,7 @@ static const struct iio_chan_spec ad7124_channel_template = {
 		BIT(IIO_CHAN_INFO_OFFSET) |
 		BIT(IIO_CHAN_INFO_SAMP_FREQ) |
 		BIT(IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY),
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),
 	.scan_type = {
 		.sign = 'u',
 		.realbits = 24,

-- 
2.43.0

[PATCH 3/6] iio: adc: ad7124: use guard(mutex) to simplify return paths

[David Lechner]

Use guard(mutex) in a couple of functions to allow direct returns. This
simplifies the code a bit and will make later changes easier.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 35 ++++++++++++-----------------------
 1 file changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index 6c17cc59f33c6ddc241e94d1b0f43bceced1e719..d0c6462bcf410efcc664b602beb94a9ab6a869c0 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -741,24 +741,20 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
 {
 	struct ad7124_state *st = iio_priv(indio_dev);
 	unsigned int res, gain, full_scale, vref;
-	int ret = 0;
 
-	mutex_lock(&st->cfgs_lock);
+	guard(mutex)(&st->cfgs_lock);
 
 	switch (info) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val2 != 0 || val == 0) {
-			ret = -EINVAL;
-			break;
-		}
+		if (val2 != 0 || val == 0)
+			return -EINVAL;
 
 		ad7124_set_channel_odr(st, chan->address, val);
-		break;
+
+		return 0;
 	case IIO_CHAN_INFO_SCALE:
-		if (val != 0) {
-			ret = -EINVAL;
-			break;
-		}
+		if (val != 0)
+			return -EINVAL;
 
 		if (st->channels[chan->address].cfg.bipolar)
 			full_scale = 1 << (chan->scan_type.realbits - 1);
@@ -774,13 +770,10 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
 			st->channels[chan->address].cfg.live = false;
 
 		st->channels[chan->address].cfg.pga_bits = res;
-		break;
+		return 0;
 	default:
-		ret = -EINVAL;
+		return -EINVAL;
 	}
-
-	mutex_unlock(&st->cfgs_lock);
-	return ret;
 }
 
 static int ad7124_reg_access(struct iio_dev *indio_dev,
@@ -812,7 +805,8 @@ static int ad7124_update_scan_mode(struct iio_dev *indio_dev,
 	int ret;
 	int i;
 
-	mutex_lock(&st->cfgs_lock);
+	guard(mutex)(&st->cfgs_lock);
+
 	for (i = 0; i < st->num_channels; i++) {
 		bit_set = test_bit(i, scan_mask);
 		if (bit_set)
@@ -820,15 +814,10 @@ static int ad7124_update_scan_mode(struct iio_dev *indio_dev,
 		else
 			ret = ad7124_spi_write_mask(st, AD7124_CHANNEL(i), AD7124_CHANNEL_ENABLE,
 						    0, 2);
-		if (ret < 0) {
-			mutex_unlock(&st->cfgs_lock);
-
+		if (ret < 0)
 			return ret;
-		}
 	}
 
-	mutex_unlock(&st->cfgs_lock);
-
 	return 0;
 }
 

-- 
2.43.0

[PATCH 4/6] iio: adc: ad7124: support fractional sampling_frequency

[David Lechner]

Modify the attribute read/write functions for sampling_frequency and
filter_low_pass_3db_frequency to return fractional values.

These ADCs support output data rates in the single digits, so being
able to specify fractional values is necessary to use all possible
sampling frequencies.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 65 +++++++++++++++++++++++++++++-------------------
 1 file changed, 39 insertions(+), 26 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index d0c6462bcf410efcc664b602beb94a9ab6a869c0..e2d92f2e3e2eb96d11dcaaf53de9124a6cc44ca4 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -168,7 +168,6 @@ struct ad7124_channel_config {
 		bool buf_negative;
 		unsigned int vref_mv;
 		unsigned int pga_bits;
-		unsigned int odr;
 		unsigned int odr_sel_bits;
 		unsigned int filter_type;
 		unsigned int calibration_offset;
@@ -287,7 +286,17 @@ static u32 ad7124_get_fclk_hz(struct ad7124_state *st)
 	return fclk_hz;
 }
 
-static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel, unsigned int odr)
+static u32 ad7124_get_fadc_divisor(struct ad7124_state *st, unsigned int channel)
+{
+	/*
+	 * The output data rate (f_ADC) is f_CLK / divisor. We are returning
+	 * the divisor.
+	 */
+	return st->channels[channel].cfg.odr_sel_bits * 32 * 4;
+}
+
+static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel,
+				   unsigned int odr, unsigned int odr_micro)
 {
 	unsigned int fclk, factor, odr_sel_bits;
 
@@ -302,29 +311,28 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
 	 * FS[10:0] can have a value from 1 to 2047
 	 */
 	factor = 32 * 4; /* N = 4 for default sinc4 filter. */
-	odr_sel_bits = clamp(DIV_ROUND_CLOSEST(fclk, odr * factor), 1, 2047);
+	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor +
+					       odr_micro * factor / MICRO);
+	odr_sel_bits = clamp(odr_sel_bits, 1, 2047);
 
 	if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)
 		st->channels[channel].cfg.live = false;
 
-	/* fADC = fCLK / (FS[10:0] x 32) */
-	st->channels[channel].cfg.odr = DIV_ROUND_CLOSEST(fclk, odr_sel_bits *
-								factor);
 	st->channels[channel].cfg.odr_sel_bits = odr_sel_bits;
 }
 
-static int ad7124_get_3db_filter_freq(struct ad7124_state *st,
-				      unsigned int channel)
+static int ad7124_get_3db_filter_factor(struct ad7124_state *st,
+					unsigned int channel)
 {
-	unsigned int fadc;
-
-	fadc = st->channels[channel].cfg.odr;
-
+	/*
+	 * 3dB point is the f_CLK rate times some factor. This functions returns
+	 * the factor times 1000.
+	 */
 	switch (st->channels[channel].cfg.filter_type) {
 	case AD7124_FILTER_FILTER_SINC3:
-		return DIV_ROUND_CLOSEST(fadc * 272, 1000);
+		return 272;
 	case AD7124_FILTER_FILTER_SINC4:
-		return DIV_ROUND_CLOSEST(fadc * 230, 1000);
+		return 230;
 	default:
 		return -EINVAL;
 	}
@@ -348,7 +356,6 @@ static struct ad7124_channel_config *ad7124_find_similar_live_cfg(struct ad7124_
 				     bool buf_negative;
 				     unsigned int vref_mv;
 				     unsigned int pga_bits;
-				     unsigned int odr;
 				     unsigned int odr_sel_bits;
 				     unsigned int filter_type;
 				     unsigned int calibration_offset;
@@ -365,7 +372,6 @@ static struct ad7124_channel_config *ad7124_find_similar_live_cfg(struct ad7124_
 		    cfg->buf_negative == cfg_aux->buf_negative &&
 		    cfg->vref_mv == cfg_aux->vref_mv &&
 		    cfg->pga_bits == cfg_aux->pga_bits &&
-		    cfg->odr == cfg_aux->odr &&
 		    cfg->odr_sel_bits == cfg_aux->odr_sel_bits &&
 		    cfg->filter_type == cfg_aux->filter_type &&
 		    cfg->calibration_offset == cfg_aux->calibration_offset &&
@@ -720,16 +726,23 @@ static int ad7124_read_raw(struct iio_dev *indio_dev,
 
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		mutex_lock(&st->cfgs_lock);
-		*val = st->channels[chan->address].cfg.odr;
+		*val = ad7124_get_fclk_hz(st);
+		*val2 = ad7124_get_fadc_divisor(st, chan->address);
 		mutex_unlock(&st->cfgs_lock);
 
-		return IIO_VAL_INT;
-	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY:
-		mutex_lock(&st->cfgs_lock);
-		*val = ad7124_get_3db_filter_freq(st, chan->scan_index);
-		mutex_unlock(&st->cfgs_lock);
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: {
+		guard(mutex)(&st->cfgs_lock);
 
-		return IIO_VAL_INT;
+		ret = ad7124_get_3db_filter_factor(st, chan->address);
+		if (ret < 0)
+			return ret;
+
+		/* 3dB point is the f_CLK rate times a fractional value */
+		*val = ret * ad7124_get_fclk_hz(st);
+		*val2 = MILLI * ad7124_get_fadc_divisor(st, chan->address);
+		return IIO_VAL_FRACTIONAL;
+	}
 	default:
 		return -EINVAL;
 	}
@@ -746,10 +759,10 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
 
 	switch (info) {
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		if (val2 != 0 || val == 0)
+		if (val2 < 0 || val < 0 || (val2 == 0 && val == 0))
 			return -EINVAL;
 
-		ad7124_set_channel_odr(st, chan->address, val);
+		ad7124_set_channel_odr(st, chan->address, val, val2);
 
 		return 0;
 	case IIO_CHAN_INFO_SCALE:
@@ -1298,7 +1311,7 @@ static int ad7124_setup(struct ad7124_state *st)
 		 * regardless of the selected power mode. Round it up to 10 and
 		 * set all channels to this default value.
 		 */
-		ad7124_set_channel_odr(st, i, 10);
+		ad7124_set_channel_odr(st, i, 10, 0);
 	}
 
 	ad7124_disable_all(&st->sd);

-- 
2.43.0

[PATCH 5/6] iio: adc: ad7124: add filter support

[David Lechner]

Add support to the ad7124 driver for selecting the filter type.

The filter type has an influence on the effective sampling frequency of
each channel. For sinc3+pf{1,2,3,4}, the sampling frequency is fixed.
For sinc{3,4} (without post filter), there is a factor of 3 or 4
depending on the filter type. For the extra +sinc1, there is an extra
averaging factor that depends on the power mode.

In order to select the closest sampling frequency for each filter type,
we keep a copy of the requested sampling frequency. This way, if the
user sets the sampling frequency first and then selects the filter type,
the sampling frequency will still be as close as possible to the
requested value.

Since we always either have the SINGLE_CYCLE bit set or have more than
one channel enabled, the sampling frequency is always using the
"zero-latency" calculation from the data sheet. This is only documented
for the basic sinc{3,4} filters, so the other filter types had to be
inferred and confirmed through testing.

Since the flat filter type list consists of multiple register fields,
the struct ad7124_channel_config::filter_type field is changed to the
enum ad7124_filter_type type to avoid nested switch statements in a
lot of places.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7124.c | 257 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 227 insertions(+), 30 deletions(-)

diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
index e2d92f2e3e2eb96d11dcaaf53de9124a6cc44ca4..a776354a25b65dc92770705e769b6351da00c2cc 100644
--- a/drivers/iio/adc/ad7124.c
+++ b/drivers/iio/adc/ad7124.c
@@ -3,6 +3,7 @@
  * AD7124 SPI ADC driver
  *
  * Copyright 2018 Analog Devices Inc.
+ * Copyright 2025 BayLibre, SAS
  */
 #include <linux/bitfield.h>
 #include <linux/bitops.h>
@@ -99,6 +100,13 @@
 #define AD7124_FILTER_FILTER_SINC3		2
 #define AD7124_FILTER_FILTER_SINC4_SINC1	4
 #define AD7124_FILTER_FILTER_SINC3_SINC1	5
+#define AD7124_FILTER_FILTER_SINC3_PF		7
+#define AD7124_FILTER_REJ60		BIT(20)
+#define AD7124_FILTER_POST_FILTER	GENMASK(19, 17)
+#define AD7124_FILTER_POST_FILTER_47dB		2
+#define AD7124_FILTER_POST_FILTER_62dB		3
+#define AD7124_FILTER_POST_FILTER_86dB		5
+#define AD7124_FILTER_POST_FILTER_92dB		6
 #define AD7124_FILTER_SINGLE_CYCLE	BIT(16)
 #define AD7124_FILTER_FS		GENMASK(10, 0)
 
@@ -153,9 +161,24 @@ struct ad7124_chip_info {
 	unsigned int num_inputs;
 };
 
+enum ad7124_filter_type {
+	AD7124_FILTER_TYPE_SINC3,
+	AD7124_FILTER_TYPE_SINC3_PF1,
+	AD7124_FILTER_TYPE_SINC3_PF2,
+	AD7124_FILTER_TYPE_SINC3_PF3,
+	AD7124_FILTER_TYPE_SINC3_PF4,
+	AD7124_FILTER_TYPE_SINC3_REJ60,
+	AD7124_FILTER_TYPE_SINC3_SINC1,
+	AD7124_FILTER_TYPE_SINC4,
+	AD7124_FILTER_TYPE_SINC4_REJ60,
+	AD7124_FILTER_TYPE_SINC4_SINC1,
+};
+
 struct ad7124_channel_config {
 	bool live;
 	unsigned int cfg_slot;
+	unsigned int requested_odr;
+	unsigned int requested_odr_micro;
 	/*
 	 * Following fields are used to compare for equality. If you
 	 * make adaptations in it, you most likely also have to adapt
@@ -169,7 +192,7 @@ struct ad7124_channel_config {
 		unsigned int vref_mv;
 		unsigned int pga_bits;
 		unsigned int odr_sel_bits;
-		unsigned int filter_type;
+		enum ad7124_filter_type filter_type;
 		unsigned int calibration_offset;
 		unsigned int calibration_gain;
 	);
@@ -286,21 +309,50 @@ static u32 ad7124_get_fclk_hz(struct ad7124_state *st)
 	return fclk_hz;
 }
 
+static u32 ad7124_get_fs_factor(struct ad7124_state *st, unsigned int channel)
+{
+	enum ad7124_power_mode power_mode =
+		FIELD_GET(AD7124_ADC_CONTROL_POWER_MODE, st->adc_control);
+	u32 avg = power_mode == AD7124_LOW_POWER ? 8 : 16;
+
+	/*
+	 * These are the "zero-latency" factors from the data sheet. For the
+	 * sinc1 filters, these aren't documented, but derived by taking the
+	 * single-channel formula from the sinc1 section of the data sheet and
+	 * multiplying that by the sinc3/4 factor from the corresponding zero-
+	 * latency sections.
+	 */
+	switch (st->channels[channel].cfg.filter_type) {
+	case AD7124_FILTER_TYPE_SINC4:
+	case AD7124_FILTER_TYPE_SINC4_REJ60:
+		return 4 * 32;
+	case AD7124_FILTER_TYPE_SINC4_SINC1:
+		return 4 * avg * 32;
+	case AD7124_FILTER_TYPE_SINC3_SINC1:
+		return 3 * avg * 32;
+	default:
+		return 3 * 32;
+	}
+}
+
 static u32 ad7124_get_fadc_divisor(struct ad7124_state *st, unsigned int channel)
 {
+	u32 factor = ad7124_get_fs_factor(st, channel);
+
 	/*
 	 * The output data rate (f_ADC) is f_CLK / divisor. We are returning
 	 * the divisor.
 	 */
-	return st->channels[channel].cfg.odr_sel_bits * 32 * 4;
+	return st->channels[channel].cfg.odr_sel_bits * factor;
 }
 
-static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel,
-				   unsigned int odr, unsigned int odr_micro)
+static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel)
 {
+	struct ad7124_channel_config *cfg = &st->channels[channel].cfg;
 	unsigned int fclk, factor, odr_sel_bits;
 
 	fclk = ad7124_get_fclk_hz(st);
+	factor = ad7124_get_fs_factor(st, channel);
 
 	/*
 	 * FS[10:0] = fCLK / (fADC x 32 * N) where:
@@ -310,9 +362,9 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
 	 * FS[10:0] are the bits in the filter register
 	 * FS[10:0] can have a value from 1 to 2047
 	 */
-	factor = 32 * 4; /* N = 4 for default sinc4 filter. */
-	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor +
-					       odr_micro * factor / MICRO);
+	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, cfg->requested_odr * factor +
+					       cfg->requested_odr_micro *
+					       factor / MICRO);
 	odr_sel_bits = clamp(odr_sel_bits, 1, 2047);
 
 	if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)
@@ -324,15 +376,29 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
 static int ad7124_get_3db_filter_factor(struct ad7124_state *st,
 					unsigned int channel)
 {
+	struct ad7124_channel_config *cfg = &st->channels[channel].cfg;
+
 	/*
 	 * 3dB point is the f_CLK rate times some factor. This functions returns
 	 * the factor times 1000.
 	 */
-	switch (st->channels[channel].cfg.filter_type) {
-	case AD7124_FILTER_FILTER_SINC3:
+	switch (cfg->filter_type) {
+	case AD7124_FILTER_TYPE_SINC3:
+	case AD7124_FILTER_TYPE_SINC3_REJ60:
+	case AD7124_FILTER_TYPE_SINC3_SINC1:
 		return 272;
-	case AD7124_FILTER_FILTER_SINC4:
+	case AD7124_FILTER_TYPE_SINC4:
+	case AD7124_FILTER_TYPE_SINC4_REJ60:
+	case AD7124_FILTER_TYPE_SINC4_SINC1:
 		return 230;
+	case AD7124_FILTER_TYPE_SINC3_PF1:
+		return 633;
+	case AD7124_FILTER_TYPE_SINC3_PF2:
+		return 605;
+	case AD7124_FILTER_TYPE_SINC3_PF3:
+		return 669;
+	case AD7124_FILTER_TYPE_SINC3_PF4:
+		return 759;
 	default:
 		return -EINVAL;
 	}
@@ -357,7 +423,7 @@ static struct ad7124_channel_config *ad7124_find_similar_live_cfg(struct ad7124_
 				     unsigned int vref_mv;
 				     unsigned int pga_bits;
 				     unsigned int odr_sel_bits;
-				     unsigned int filter_type;
+				     enum ad7124_filter_type filter_type;
 				     unsigned int calibration_offset;
 				     unsigned int calibration_gain;
 			     }));
@@ -424,8 +490,9 @@ static int ad7124_init_config_vref(struct ad7124_state *st, struct ad7124_channe
 static int ad7124_write_config(struct ad7124_state *st, struct ad7124_channel_config *cfg,
 			       unsigned int cfg_slot)
 {
-	unsigned int tmp;
-	unsigned int val;
+	unsigned int val, filter;
+	unsigned int rej60 = 0;
+	unsigned int post = 0;
 	int ret;
 
 	cfg->cfg_slot = cfg_slot;
@@ -448,6 +515,47 @@ static int ad7124_write_config(struct ad7124_state *st, struct ad7124_channel_co
 	if (ret < 0)
 		return ret;
 
+	switch (cfg->filter_type) {
+	case AD7124_FILTER_TYPE_SINC3:
+		filter = AD7124_FILTER_FILTER_SINC3;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_PF1:
+		filter = AD7124_FILTER_FILTER_SINC3_PF;
+		post = AD7124_FILTER_POST_FILTER_47dB;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_PF2:
+		filter = AD7124_FILTER_FILTER_SINC3_PF;
+		post = AD7124_FILTER_POST_FILTER_62dB;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_PF3:
+		filter = AD7124_FILTER_FILTER_SINC3_PF;
+		post = AD7124_FILTER_POST_FILTER_86dB;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_PF4:
+		filter = AD7124_FILTER_FILTER_SINC3_PF;
+		post = AD7124_FILTER_POST_FILTER_92dB;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_REJ60:
+		filter = AD7124_FILTER_FILTER_SINC3;
+		rej60 = 1;
+		break;
+	case AD7124_FILTER_TYPE_SINC3_SINC1:
+		filter = AD7124_FILTER_FILTER_SINC3_SINC1;
+		break;
+	case AD7124_FILTER_TYPE_SINC4:
+		filter = AD7124_FILTER_FILTER_SINC4;
+		break;
+	case AD7124_FILTER_TYPE_SINC4_REJ60:
+		filter = AD7124_FILTER_FILTER_SINC4;
+		rej60 = 1;
+		break;
+	case AD7124_FILTER_TYPE_SINC4_SINC1:
+		filter = AD7124_FILTER_FILTER_SINC4_SINC1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
 	/*
 	 * NB: AD7124_FILTER_SINGLE_CYCLE is always set so that we get the same
 	 * sampling frequency even when only one channel is enabled in a
@@ -455,14 +563,12 @@ static int ad7124_write_config(struct ad7124_state *st, struct ad7124_channel_co
 	 * would be 1 and we would get a faster sampling frequency than what
 	 * was requested.
 	 */
-	tmp = FIELD_PREP(AD7124_FILTER_FILTER, cfg->filter_type) |
-		AD7124_FILTER_SINGLE_CYCLE |
-		FIELD_PREP(AD7124_FILTER_FS, cfg->odr_sel_bits);
-	return ad7124_spi_write_mask(st, AD7124_FILTER(cfg->cfg_slot),
-				     AD7124_FILTER_FILTER |
-				     AD7124_FILTER_SINGLE_CYCLE |
-				     AD7124_FILTER_FS,
-				     tmp, 3);
+	return ad_sd_write_reg(&st->sd, AD7124_FILTER(cfg->cfg_slot), 3,
+			       FIELD_PREP(AD7124_FILTER_FILTER, filter) |
+			       FIELD_PREP(AD7124_FILTER_REJ60, rej60) |
+			       FIELD_PREP(AD7124_FILTER_POST_FILTER, post) |
+			       AD7124_FILTER_SINGLE_CYCLE |
+			       FIELD_PREP(AD7124_FILTER_FS, cfg->odr_sel_bits));
 }
 
 static struct ad7124_channel_config *ad7124_pop_config(struct ad7124_state *st)
@@ -724,13 +830,47 @@ static int ad7124_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 
-	case IIO_CHAN_INFO_SAMP_FREQ:
-		mutex_lock(&st->cfgs_lock);
-		*val = ad7124_get_fclk_hz(st);
-		*val2 = ad7124_get_fadc_divisor(st, chan->address);
-		mutex_unlock(&st->cfgs_lock);
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		struct ad7124_channel_config *cfg = &st->channels[chan->address].cfg;
 
-		return IIO_VAL_FRACTIONAL;
+		guard(mutex)(&st->cfgs_lock);
+
+		switch (cfg->filter_type) {
+		case AD7124_FILTER_TYPE_SINC3:
+		case AD7124_FILTER_TYPE_SINC3_REJ60:
+		case AD7124_FILTER_TYPE_SINC3_SINC1:
+		case AD7124_FILTER_TYPE_SINC4:
+		case AD7124_FILTER_TYPE_SINC4_REJ60:
+		case AD7124_FILTER_TYPE_SINC4_SINC1:
+			*val = ad7124_get_fclk_hz(st);
+			*val2 = ad7124_get_fadc_divisor(st, chan->address);
+			return IIO_VAL_FRACTIONAL;
+		/*
+		 * Post filters force the chip to a fixed rate. These are the
+		 * single-channel rates from the data sheet divided by 3 for
+		 * the multi-channel case (data sheet doesn't explicitly state
+		 * this but confirmed through testing).
+		 */
+		case AD7124_FILTER_TYPE_SINC3_PF1:
+			*val = 300;
+			*val2 = 33;
+			return IIO_VAL_FRACTIONAL;
+		case AD7124_FILTER_TYPE_SINC3_PF2:
+			*val = 25;
+			*val2 = 3;
+			return IIO_VAL_FRACTIONAL;
+		case AD7124_FILTER_TYPE_SINC3_PF3:
+			*val = 20;
+			*val2 = 3;
+			return IIO_VAL_FRACTIONAL;
+		case AD7124_FILTER_TYPE_SINC3_PF4:
+			*val = 50;
+			*val2 = 9;
+			return IIO_VAL_FRACTIONAL;
+		default:
+			return -EINVAL;
+		}
+	}
 	case IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY: {
 		guard(mutex)(&st->cfgs_lock);
 
@@ -753,6 +893,7 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
 			    int val, int val2, long info)
 {
 	struct ad7124_state *st = iio_priv(indio_dev);
+	struct ad7124_channel_config *cfg = &st->channels[chan->address].cfg;
 	unsigned int res, gain, full_scale, vref;
 
 	guard(mutex)(&st->cfgs_lock);
@@ -762,7 +903,9 @@ static int ad7124_write_raw(struct iio_dev *indio_dev,
 		if (val2 < 0 || val < 0 || (val2 == 0 && val == 0))
 			return -EINVAL;
 
-		ad7124_set_channel_odr(st, chan->address, val, val2);
+		cfg->requested_odr = val;
+		cfg->requested_odr_micro = val2;
+		ad7124_set_channel_odr(st, chan->address);
 
 		return 0;
 	case IIO_CHAN_INFO_SCALE:
@@ -1008,6 +1151,52 @@ static const struct iio_enum ad7124_syscalib_mode_enum = {
 	.get = ad7124_get_syscalib_mode
 };
 
+static const char * const ad7124_filter_types[] = {
+	[AD7124_FILTER_TYPE_SINC3] = "sinc3",
+	[AD7124_FILTER_TYPE_SINC3_PF1] = "sinc3+pf1",
+	[AD7124_FILTER_TYPE_SINC3_PF2] = "sinc3+pf2",
+	[AD7124_FILTER_TYPE_SINC3_PF3] = "sinc3+pf3",
+	[AD7124_FILTER_TYPE_SINC3_PF4] = "sinc3+pf4",
+	[AD7124_FILTER_TYPE_SINC3_REJ60] = "sinc3+rej60",
+	[AD7124_FILTER_TYPE_SINC3_SINC1] = "sinc3+sinc1",
+	[AD7124_FILTER_TYPE_SINC4] = "sinc4",
+	[AD7124_FILTER_TYPE_SINC4_REJ60] = "sinc4+rej60",
+	[AD7124_FILTER_TYPE_SINC4_SINC1] = "sinc4+sinc1",
+};
+
+static int ad7124_set_filter_type_attr(struct iio_dev *dev,
+				       const struct iio_chan_spec *chan,
+				       unsigned int value)
+{
+	struct ad7124_state *st = iio_priv(dev);
+	struct ad7124_channel_config *cfg = &st->channels[chan->address].cfg;
+
+	guard(mutex)(&st->cfgs_lock);
+
+	cfg->live = false;
+	cfg->filter_type = value;
+	ad7124_set_channel_odr(st, chan->address);
+
+	return 0;
+}
+
+static int ad7124_get_filter_type_attr(struct iio_dev *dev,
+				       const struct iio_chan_spec *chan)
+{
+	struct ad7124_state *st = iio_priv(dev);
+
+	guard(mutex)(&st->cfgs_lock);
+
+	return st->channels[chan->address].cfg.filter_type;
+}
+
+static const struct iio_enum ad7124_filter_type_enum = {
+	.items = ad7124_filter_types,
+	.num_items = ARRAY_SIZE(ad7124_filter_types),
+	.set = ad7124_set_filter_type_attr,
+	.get = ad7124_get_filter_type_attr,
+};
+
 static const struct iio_chan_spec_ext_info ad7124_calibsys_ext_info[] = {
 	{
 		.name = "sys_calibration",
@@ -1018,6 +1207,9 @@ static const struct iio_chan_spec_ext_info ad7124_calibsys_ext_info[] = {
 		 &ad7124_syscalib_mode_enum),
 	IIO_ENUM_AVAILABLE("sys_calibration_mode", IIO_SHARED_BY_TYPE,
 			   &ad7124_syscalib_mode_enum),
+	IIO_ENUM("filter_type", IIO_SEPARATE, &ad7124_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_TYPE,
+			   &ad7124_filter_type_enum),
 	{ }
 };
 
@@ -1301,17 +1493,22 @@ static int ad7124_setup(struct ad7124_state *st)
 	mutex_init(&st->cfgs_lock);
 	INIT_KFIFO(st->live_cfgs_fifo);
 	for (i = 0; i < st->num_channels; i++) {
+		struct ad7124_channel_config *cfg = &st->channels[i].cfg;
 
-		ret = ad7124_init_config_vref(st, &st->channels[i].cfg);
+		ret = ad7124_init_config_vref(st, cfg);
 		if (ret < 0)
 			return ret;
 
+		/* Default filter type on the ADC after reset. */
+		cfg->filter_type = AD7124_FILTER_TYPE_SINC4;
+
 		/*
 		 * 9.38 SPS is the minimum output data rate supported
 		 * regardless of the selected power mode. Round it up to 10 and
 		 * set all channels to this default value.
 		 */
-		ad7124_set_channel_odr(st, i, 10, 0);
+		cfg->requested_odr = 10;
+		ad7124_set_channel_odr(st, i);
 	}
 
 	ad7124_disable_all(&st->sd);

-- 
2.43.0

[PATCH 6/6] iio: ABI: document "sinc4+rej60" filter_type

[David Lechner]

Add a bullet point for "sinc4+rej60" filter_type that is used in the
ad7124 driver.

Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 Documentation/ABI/testing/sysfs-bus-iio | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 2fb2cea4b19249743398b1ff0b538b03ced0340b..829ccfd902f2ca4e5ae38ca025eb3c4a9fe1389d 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2292,6 +2292,7 @@ Description:
 		  1st conversion time.
 		* "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
 		  time.
+		* "sinc4+rej60" - Sinc4 + 60Hz rejection.
 		* "sinc5" - The digital sinc5 filter. Excellent noise
 		  performance
 		* "sinc5+avg" - Sinc5 + averaging by 4.

-- 
2.43.0

Re: [PATCH 1/6] iio: adc: ad7124: use clamp()

[Jonathan Cameron]

On Fri, 05 Sep 2025 13:11:56 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Use clamp() instead of open-coding clamping.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Good tidy up on it's own so I'll pick this up now.

I haven't read the rest yet but I'd imagine others will need
more review than this.

Applied this patch to the togreg branch of iio.git and pushed out as testing.
> ---
>  drivers/iio/adc/ad7124.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7124.c b/drivers/iio/adc/ad7124.c
> index 70f458e3ccc12db884dda9003abcffdf48989e5e..117777fc8ad05b773da09c113cf84927c75d6b7b 100644
> --- a/drivers/iio/adc/ad7124.c
> +++ b/drivers/iio/adc/ad7124.c
> @@ -15,6 +15,7 @@
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/kfifo.h>
> +#include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
>  #include <linux/property.h>
> @@ -301,11 +302,7 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
>  	 * FS[10:0] can have a value from 1 to 2047
>  	 */
>  	factor = 32 * 4; /* N = 4 for default sinc4 filter. */
> -	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor);
> -	if (odr_sel_bits < 1)
> -		odr_sel_bits = 1;
> -	else if (odr_sel_bits > 2047)
> -		odr_sel_bits = 2047;
> +	odr_sel_bits = clamp(DIV_ROUND_CLOSEST(fclk, odr * factor), 1, 2047);
>  
>  	if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)
>  		st->channels[channel].cfg.live = false;
> 

Re: [PATCH 2/6] iio: adc: ad7124: use read_avail() for scale_available

[Jonathan Cameron]

On Fri, 05 Sep 2025 13:11:57 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Replace custom attribute with the standard IIO read_avail() callback
> to provide in_voltage_scale_available attribute.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Another (hopefully) uncontroversial improvement.

Applied.

Re: [PATCH 3/6] iio: adc: ad7124: use guard(mutex) to simplify return paths

[Jonathan Cameron]

On Fri, 05 Sep 2025 13:11:58 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Use guard(mutex) in a couple of functions to allow direct returns. This
> simplifies the code a bit and will make later changes easier.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
Another nice improvement that stands on it's own.

Applied.

Re: [PATCH 5/6] iio: adc: ad7124: add filter support

[Jonathan Cameron]

On Fri, 05 Sep 2025 13:12:00 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Add support to the ad7124 driver for selecting the filter type.
> 
> The filter type has an influence on the effective sampling frequency of
> each channel. For sinc3+pf{1,2,3,4}, the sampling frequency is fixed.
> For sinc{3,4} (without post filter), there is a factor of 3 or 4
> depending on the filter type. For the extra +sinc1, there is an extra
> averaging factor that depends on the power mode.
> 
> In order to select the closest sampling frequency for each filter type,
> we keep a copy of the requested sampling frequency. This way, if the
> user sets the sampling frequency first and then selects the filter type,
> the sampling frequency will still be as close as possible to the
> requested value.
> 
> Since we always either have the SINGLE_CYCLE bit set or have more than
> one channel enabled, the sampling frequency is always using the
> "zero-latency" calculation from the data sheet. This is only documented
> for the basic sinc{3,4} filters, so the other filter types had to be
> inferred and confirmed through testing.
> 
> Since the flat filter type list consists of multiple register fields,
> the struct ad7124_channel_config::filter_type field is changed to the
> enum ad7124_filter_type type to avoid nested switch statements in a
> lot of places.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>

One really trivial comment inline.  Not worth a v2 for just that.
However, this is complex enough code I'd like to keep this on list anyway
for a little longer to see if anyone else has review comments.

Jonathan



> -static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel,
> -				   unsigned int odr, unsigned int odr_micro)
> +static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel)
>  {
> +	struct ad7124_channel_config *cfg = &st->channels[channel].cfg;
>  	unsigned int fclk, factor, odr_sel_bits;
>  
>  	fclk = ad7124_get_fclk_hz(st);
> +	factor = ad7124_get_fs_factor(st, channel);
>  
>  	/*
>  	 * FS[10:0] = fCLK / (fADC x 32 * N) where:
> @@ -310,9 +362,9 @@ static void ad7124_set_channel_odr(struct ad7124_state *st, unsigned int channel
>  	 * FS[10:0] are the bits in the filter register
>  	 * FS[10:0] can have a value from 1 to 2047
>  	 */
> -	factor = 32 * 4; /* N = 4 for default sinc4 filter. */
> -	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, odr * factor +
> -					       odr_micro * factor / MICRO);
> +	odr_sel_bits = DIV_ROUND_CLOSEST(fclk, cfg->requested_odr * factor +
> +					       cfg->requested_odr_micro *
> +					       factor / MICRO);

This second parameter is getting complex enough, perhaps worth introducing
a local variable?
	divisor = cfg->requested_odr * factor +
		  cfg->requested_odr_micro * factor / MICRO;
perhaps, or local variables for odr and odr_micro to keep this unchanged in this path.


>  	odr_sel_bits = clamp(odr_sel_bits, 1, 2047);
>  
>  	if (odr_sel_bits != st->channels[channel].cfg.odr_sel_bits)

Re: [PATCH 3/6] iio: adc: ad7124: use guard(mutex) to simplify return paths

[Jonathan Cameron]

On Sun, 7 Sep 2025 11:06:52 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 05 Sep 2025 13:11:58 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > Use guard(mutex) in a couple of functions to allow direct returns. This
> > simplifies the code a bit and will make later changes easier.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>  
> Another nice improvement that stands on it's own.
> 
> Applied.
> 

Unapplied due to drop of the precursor fix.  Same with patches 1 and 2.

J

Re: [PATCH 5/6] iio: adc: ad7124: add filter support

[Jonathan Cameron]

On Sun, 7 Sep 2025 11:22:14 +0100
Jonathan Cameron <jic23@kernel.org> wrote:

> On Fri, 05 Sep 2025 13:12:00 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
> > Add support to the ad7124 driver for selecting the filter type.
> > 
> > The filter type has an influence on the effective sampling frequency of
> > each channel. For sinc3+pf{1,2,3,4}, the sampling frequency is fixed.
> > For sinc{3,4} (without post filter), there is a factor of 3 or 4
> > depending on the filter type. For the extra +sinc1, there is an extra
> > averaging factor that depends on the power mode.
> > 
> > In order to select the closest sampling frequency for each filter type,
> > we keep a copy of the requested sampling frequency. This way, if the
> > user sets the sampling frequency first and then selects the filter type,
> > the sampling frequency will still be as close as possible to the
> > requested value.
> > 
> > Since we always either have the SINGLE_CYCLE bit set or have more than
> > one channel enabled, the sampling frequency is always using the
> > "zero-latency" calculation from the data sheet. This is only documented
> > for the basic sinc{3,4} filters, so the other filter types had to be
> > inferred and confirmed through testing.
> > 
> > Since the flat filter type list consists of multiple register fields,
> > the struct ad7124_channel_config::filter_type field is changed to the
> > enum ad7124_filter_type type to avoid nested switch statements in a
> > lot of places.
> > 
> > Signed-off-by: David Lechner <dlechner@baylibre.com>  
> 
> One really trivial comment inline.  Not worth a v2 for just that.
> However, this is complex enough code I'd like to keep this on list anyway
> for a little longer to see if anyone else has review comments.
> 
> Jonathan
Hi David,

Given your self review of the fix, I am assuming you will do a new
version of that and these.  Which mostly means I'll mark them as waiting
for changes in patchwork and forget about them till I see the new version :)

Thanks,

Jonathan