[PATCH 0/5] iio: adc: ad7137: add filter support

[PATCH 0/5] iio: adc: ad7137: add filter support

[David Lechner]

Adding yet another feature to the ad7173 driver, this time,
filter support.

There are a couple of leading patches to rename some stuff to minimize
the diff in the main patch where filter support is actually added. And
there is a bonus patch to clean up the ABI docs for filter_type first
before adding the new filter types introduced in this series.

This was tested on the EVAL-AD7173-8ARDZ evaluation board.

This series depends on a bunch of fixes, so we'll have to wait for
those to make it back into iio/testing before we can merge this
series. There is also an outstanding patch to add SPI offload support
to this driver, but that doesn't actually have any merge conflicts
with this series, so they can be applied in any order.

---
David Lechner (5):
      iio: adc: ad7173: rename ad7173_chan_spec_ext_info
      iio: adc: ad7173: rename odr field
      iio: adc: ad7173: support changing filter type
      iio: ABI: alphabetize filter types
      iio: ABI: add filter types for ad7173

 Documentation/ABI/testing/sysfs-bus-iio |  25 ++--
 drivers/iio/adc/ad7173.c                | 204 +++++++++++++++++++++++++++++---
 2 files changed, 205 insertions(+), 24 deletions(-)
---
base-commit: f8f559752d573a051a984adda8d2d1464f92f954
change-id: 20250710-iio-adc-ad7137-add-filter-support-d0ffaa92afc9
prerequisite-change-id: 20250703-iio-adc-ad7173-fix-channels-index-for-syscalib_mode-49b404e99e0c:v1
prerequisite-patch-id: 982dde330c34b57a76a3e48ccfc73ea6977833d1
prerequisite-change-id: 20250703-iio-adc-ad7173-fix-num_slots-on-most-chips-b982206a20b1:v3
prerequisite-patch-id: 350fb675f3e0fe494e0ce4ddf5685d9369ffa11a
prerequisite-change-id: 20250708-iio-adc-ad7313-fix-calibration-channel-198ed65d9b0a:v1
prerequisite-patch-id: b94476eb0399877093321fd5010965d44738c097
prerequisite-change-id: 20250709-iio-adc-ad7173-fix-setup-use-limits-0a5d2b6a6780:v1
prerequisite-patch-id: 8ca40138b61bcf4eac7437b8184276576308536b
prerequisite-change-id: 20250710-iio-adc-ad7173-fix-setting-odr-in-probe-915972070e8a:v1
prerequisite-patch-id: 0f79cb2677f8a249283e239ca9ae9ae1a1eeb365

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

[PATCH 1/5] iio: adc: ad7173: rename ad7173_chan_spec_ext_info

[David Lechner]

Rename ad7173_calibsys_ext_info[] to ad7173_chan_spec_ext_info[]. This
array is not limited to calibration attributes, so the name should be
more generic.

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

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index ed5020167089ab96eb1927c9b1ae207e36f8b9e7..5daf21c6ba5637b2e47dcd052bdd019c3ecbb442 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -369,7 +369,7 @@ static const struct iio_enum ad7173_syscalib_mode_enum = {
 	.get = ad7173_get_syscalib_mode
 };
 
-static const struct iio_chan_spec_ext_info ad7173_calibsys_ext_info[] = {
+static const struct iio_chan_spec_ext_info ad7173_chan_spec_ext_info[] = {
 	{
 		.name = "sys_calibration",
 		.write = ad7173_write_syscalib,
@@ -1399,7 +1399,7 @@ static const struct iio_chan_spec ad7173_channel_template = {
 		.storagebits = 32,
 		.endianness = IIO_BE,
 	},
-	.ext_info = ad7173_calibsys_ext_info,
+	.ext_info = ad7173_chan_spec_ext_info,
 };
 
 static const struct iio_chan_spec ad7173_temp_iio_channel_template = {

-- 
2.43.0

[PATCH 2/5] iio: adc: ad7173: rename odr field

[David Lechner]

Rename odr to sinc5_odr_index in the channel setup structure. In a
following commit, we will be adding a separate odr field for when the
sinc3 filter is used instead so having sinc5 in the name will help
avoid confusion. And _index makes it more clear that this is an index
of the sinc5_data_rates array and not the output data rate itself.

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

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 5daf21c6ba5637b2e47dcd052bdd019c3ecbb442..01d78d531d6c024dd92fff21b8b2afb750092b66 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -205,7 +205,7 @@ struct ad7173_channel_config {
 	struct_group(config_props,
 		bool bipolar;
 		bool input_buf;
-		u8 odr;
+		u8 sinc5_odr_index;
 		u8 ref_sel;
 	);
 };
@@ -582,13 +582,13 @@ static bool ad7173_setup_equal(const struct ad7173_channel_config *cfg1,
 		      sizeof(struct {
 				     bool bipolar;
 				     bool input_buf;
-				     u8 odr;
+				     u8 sinc5_odr_index;
 				     u8 ref_sel;
 			     }));
 
 	return cfg1->bipolar == cfg2->bipolar &&
 	       cfg1->input_buf == cfg2->input_buf &&
-	       cfg1->odr == cfg2->odr &&
+	       cfg1->sinc5_odr_index == cfg2->sinc5_odr_index &&
 	       cfg1->ref_sel == cfg2->ref_sel;
 }
 
@@ -650,7 +650,7 @@ static int ad7173_load_config(struct ad7173_state *st,
 		return ret;
 
 	return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
-			       AD7173_FILTER_ODR0_MASK & cfg->odr);
+			       AD7173_FILTER_ODR0_MASK & cfg->sinc5_odr_index);
 }
 
 static int ad7173_config_channel(struct ad7173_state *st, int addr)
@@ -1183,7 +1183,7 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_SAMP_FREQ:
-		reg = st->channels[chan->address].cfg.odr;
+		reg = st->channels[chan->address].cfg.sinc5_odr_index;
 
 		*val = st->info->sinc5_data_rates[reg] / MILLI;
 		*val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO / MILLI);
@@ -1229,7 +1229,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
 				break;
 
 		cfg = &st->channels[chan->address].cfg;
-		cfg->odr = i;
+		cfg->sinc5_odr_index = i;
 		cfg->live = false;
 		break;
 
@@ -1655,7 +1655,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan_st_priv->cfg.bipolar = false;
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
 		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
-		chan_st_priv->cfg.odr = st->info->odr_start_value;
+		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
 		chan_st_priv->cfg.openwire_comp_chan = -1;
 		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
 		if (st->info->data_reg_only_16bit)
@@ -1727,7 +1727,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan->scan_index = chan_index;
 		chan->channel = ain[0];
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
-		chan_st_priv->cfg.odr = st->info->odr_start_value;
+		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
 		chan_st_priv->cfg.openwire_comp_chan = -1;
 
 		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");

-- 
2.43.0

[PATCH 3/5] iio: adc: ad7173: support changing filter type

[David Lechner]

Add support for changing the filter type to the ad7173 driver.

This family of chips by default uses a sinc5+sinc1 filter. There are
also optional post-filters that can be added in this configuration for
various 50/60Hz rejection purposes. The sinc3 filter doesn't have any
post-filters and handles the output data rate (ODR) a bit differently.
Here, we've opted to use SINC3_MAPx to get the maximum possible
sampling frequencies with the SINC3 filter.

Adding support consists of adding the filter_type and
filter_type_available attributes, making the sampling_frequency
attribute aware of the filter type, and programming the filter
parameters when we configure the channel of the ADC for reading
a sample.

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

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 01d78d531d6c024dd92fff21b8b2afb750092b66..c235096fbad4aeb77a7385001a16bc0ecd603f46 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -8,6 +8,7 @@
  *  AD7175-8/AD7176-2/AD7177-2
  *
  * Copyright (C) 2015, 2024 Analog Devices, Inc.
+ * Copyright (C) 2025 BayLibre, SAS
  */
 
 #include <linux/array_size.h>
@@ -149,7 +150,12 @@
 					       (pin2) < st->info->num_voltage_in && \
 					       (pin2) >= st->info->num_voltage_in_div)
 
-#define AD7173_FILTER_ODR0_MASK		GENMASK(5, 0)
+#define AD7173_FILTER_SINC3_MAP		BIT(15)
+#define AD7173_FILTER_SINC3_MAP_DIV	GENMASK(14, 0)
+#define AD7173_FILTER_ENHFILTEN		BIT(11)
+#define AD7173_FILTER_ENHFILT_MASK	GENMASK(10, 8)
+#define AD7173_FILTER_ORDER		BIT(6)
+#define AD7173_FILTER_ODR_MASK		GENMASK(5, 0)
 #define AD7173_MAX_CONFIGS		8
 #define AD4111_OW_DET_THRSH_MV		300
 
@@ -190,6 +196,15 @@ struct ad7173_device_info {
 	u8 num_gpios;
 };
 
+enum ad7173_filter_type {
+	AD7173_FILTER_SINC3,
+	AD7173_FILTER_SINC5_SINC1,
+	AD7173_FILTER_SINC5_SINC1_PF1,
+	AD7173_FILTER_SINC5_SINC1_PF2,
+	AD7173_FILTER_SINC5_SINC1_PF3,
+	AD7173_FILTER_SINC5_SINC1_PF4,
+};
+
 struct ad7173_channel_config {
 	/* Openwire detection threshold */
 	unsigned int openwire_thrsh_raw;
@@ -205,8 +220,10 @@ struct ad7173_channel_config {
 	struct_group(config_props,
 		bool bipolar;
 		bool input_buf;
+		u16 sinc3_ord_div;
 		u8 sinc5_odr_index;
 		u8 ref_sel;
+		enum ad7173_filter_type filter_type;
 	);
 };
 
@@ -266,6 +283,24 @@ static const unsigned int ad7175_sinc5_data_rates[] = {
 	5000,					/* 20    */
 };
 
+/**
+ * ad7173_sinc3_ord_div_from_odr() - Convert ODR to divider value
+ * @odr_millihz: ODR (sampling_frequency) in milliHz
+ * Returns: Divider value for SINC3 filter to pass.
+ */
+static u16 ad7173_sinc3_ord_div_from_odr(u32 odr_millihz)
+{
+	/*
+	 * Divider is f_MOD (1 MHz) / 32 / ODR. ODR freq is in milliHz, so
+	 * we need to convert f_MOD to the same units. When SING_CYC=1 or
+	 * multiple channels are enabled (currently always the case), there
+	 * is an additional factor of 3.
+	 */
+	u32 div = DIV_ROUND_CLOSEST(MEGA * MILLI, odr_millihz * 32 * 3);
+	/* Avoid divide by 0 and limit to register field size. */
+	return clamp(div, 1U, AD7173_FILTER_SINC3_MAP_DIV);
+}
+
 static unsigned int ad4111_current_channel_config[] = {
 	/* Ain sel: pos        neg    */
 	0x1E8, /* 15:IIN0+    8:IIN0− */
@@ -369,6 +404,47 @@ static const struct iio_enum ad7173_syscalib_mode_enum = {
 	.get = ad7173_get_syscalib_mode
 };
 
+static const char * const ad7173_filter_types_str[] = {
+	[AD7173_FILTER_SINC3] = "sinc3",
+	[AD7173_FILTER_SINC5_SINC1] = "sinc5+sinc1",
+	[AD7173_FILTER_SINC5_SINC1_PF1] = "sinc5+sinc1+pf1",
+	[AD7173_FILTER_SINC5_SINC1_PF2] = "sinc5+sinc1+pf2",
+	[AD7173_FILTER_SINC5_SINC1_PF3] = "sinc5+sinc1+pf3",
+	[AD7173_FILTER_SINC5_SINC1_PF4] = "sinc5+sinc1+pf4",
+};
+
+static int ad7173_set_filter_type(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan,
+				  unsigned int val)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+
+	st->channels[chan->address].cfg.filter_type = val;
+	st->channels[chan->address].cfg.live = false;
+
+	iio_device_release_direct(indio_dev);
+
+	return 0;
+}
+
+static int ad7173_get_filter_type(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+
+	return st->channels[chan->address].cfg.filter_type;
+}
+
+static const struct iio_enum ad7173_filter_type_enum = {
+	.items = ad7173_filter_types_str,
+	.num_items = ARRAY_SIZE(ad7173_filter_types_str),
+	.set = ad7173_set_filter_type,
+	.get = ad7173_get_filter_type,
+};
+
 static const struct iio_chan_spec_ext_info ad7173_chan_spec_ext_info[] = {
 	{
 		.name = "sys_calibration",
@@ -379,6 +455,16 @@ static const struct iio_chan_spec_ext_info ad7173_chan_spec_ext_info[] = {
 		 &ad7173_syscalib_mode_enum),
 	IIO_ENUM_AVAILABLE("sys_calibration_mode", IIO_SHARED_BY_TYPE,
 			   &ad7173_syscalib_mode_enum),
+	IIO_ENUM("filter_type", IIO_SEPARATE, &ad7173_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_TYPE,
+			   &ad7173_filter_type_enum),
+	{ }
+};
+
+static const struct iio_chan_spec_ext_info ad7173_temp_chan_spec_ext_info[] = {
+	IIO_ENUM("filter_type", IIO_SEPARATE, &ad7173_filter_type_enum),
+	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_TYPE,
+			   &ad7173_filter_type_enum),
 	{ }
 };
 
@@ -582,14 +668,18 @@ static bool ad7173_setup_equal(const struct ad7173_channel_config *cfg1,
 		      sizeof(struct {
 				     bool bipolar;
 				     bool input_buf;
+				     u16 sinc3_ord_div;
 				     u8 sinc5_odr_index;
 				     u8 ref_sel;
+				     enum ad7173_filter_type filter_type;
 			     }));
 
 	return cfg1->bipolar == cfg2->bipolar &&
 	       cfg1->input_buf == cfg2->input_buf &&
+	       cfg1->sinc3_ord_div == cfg2->sinc3_ord_div &&
 	       cfg1->sinc5_odr_index == cfg2->sinc5_odr_index &&
-	       cfg1->ref_sel == cfg2->ref_sel;
+	       cfg1->ref_sel == cfg2->ref_sel &&
+	       cfg1->filter_type == cfg2->filter_type;
 }
 
 static struct ad7173_channel_config *
@@ -630,6 +720,7 @@ static int ad7173_load_config(struct ad7173_state *st,
 {
 	unsigned int config;
 	int free_cfg_slot, ret;
+	u8 post_filter_enable, post_filter_select;
 
 	free_cfg_slot = ida_alloc_range(&st->cfg_slots_status, 0,
 					st->info->num_configs - 1, GFP_KERNEL);
@@ -649,8 +740,49 @@ static int ad7173_load_config(struct ad7173_state *st,
 	if (ret)
 		return ret;
 
+	/*
+	 * When SINC3_MAP flag is enabled, the rest of the register has a
+	 * different meaning. We are using this option to allow the most
+	 * possible sampling frequencies with SINC3 filter.
+	 */
+	if (cfg->filter_type == AD7173_FILTER_SINC3)
+		return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
+				       FIELD_PREP(AD7173_FILTER_SINC3_MAP, 1) |
+				       FIELD_PREP(AD7173_FILTER_SINC3_MAP_DIV,
+						  cfg->sinc3_ord_div));
+
+	switch (cfg->filter_type) {
+	case AD7173_FILTER_SINC5_SINC1_PF1:
+		post_filter_enable = 1;
+		post_filter_select = 2;
+		break;
+	case AD7173_FILTER_SINC5_SINC1_PF2:
+		post_filter_enable = 1;
+		post_filter_select = 3;
+		break;
+	case AD7173_FILTER_SINC5_SINC1_PF3:
+		post_filter_enable = 1;
+		post_filter_select = 5;
+		break;
+	case AD7173_FILTER_SINC5_SINC1_PF4:
+		post_filter_enable = 1;
+		post_filter_select = 6;
+		break;
+	default:
+		post_filter_enable = 0;
+		post_filter_select = 0;
+		break;
+	}
+
 	return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
-			       AD7173_FILTER_ODR0_MASK & cfg->sinc5_odr_index);
+			       FIELD_PREP(AD7173_FILTER_SINC3_MAP, 0) |
+			       FIELD_PREP(AD7173_FILTER_ENHFILT_MASK,
+					  post_filter_enable) |
+			       FIELD_PREP(AD7173_FILTER_ENHFILTEN,
+					  post_filter_select) |
+			       FIELD_PREP(AD7173_FILTER_ORDER, 0) |
+			       FIELD_PREP(AD7173_FILTER_ODR_MASK,
+					  cfg->sinc5_odr_index));
 }
 
 static int ad7173_config_channel(struct ad7173_state *st, int addr)
@@ -1183,6 +1315,13 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
 			return -EINVAL;
 		}
 	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (st->channels[chan->address].cfg.filter_type == AD7173_FILTER_SINC3) {
+			/* Inverse operation of ad7173_sinc3_ord_div_from_odr() */
+			*val = MEGA;
+			*val2 = 3 * 32 * st->channels[chan->address].cfg.sinc3_ord_div;
+			return IIO_VAL_FRACTIONAL;
+		}
+
 		reg = st->channels[chan->address].cfg.sinc5_odr_index;
 
 		*val = st->info->sinc5_data_rates[reg] / MILLI;
@@ -1221,6 +1360,10 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
 	 *
 	 * This will cause the reading of CH1 to be actually done once every
 	 * 200.16ms, an effective rate of 4.99sps.
+	 *
+	 * Both the sinc5 and sinc3 rates are set here so that if the filter
+	 * type is changed, the requested rate will still be set (aside from
+	 * rounding differences).
 	 */
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		freq = val * MILLI + val2 / MILLI;
@@ -1230,6 +1373,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
 
 		cfg = &st->channels[chan->address].cfg;
 		cfg->sinc5_odr_index = i;
+		cfg->sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(freq);
 		cfg->live = false;
 		break;
 
@@ -1246,17 +1390,40 @@ static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
 				   const unsigned long *scan_mask)
 {
 	struct ad7173_state *st = iio_priv(indio_dev);
+	u16 sinc3_count = 0;
+	u16 sinc3_div = 0;
 	int i, j, k, ret;
 
 	for (i = 0; i < indio_dev->num_channels; i++) {
-		if (test_bit(i, scan_mask))
+		const struct ad7173_channel_config *cfg = &st->channels[i].cfg;
+
+		if (test_bit(i, scan_mask)) {
+			if (cfg->filter_type == AD7173_FILTER_SINC3) {
+				sinc3_count++;
+
+				if (sinc3_div == 0) {
+					sinc3_div = cfg->sinc3_ord_div;
+				} else if (sinc3_div != cfg->sinc3_ord_div) {
+					dev_err(&st->sd.spi->dev,
+						"All enabled channels must have the same sampling_frequency for sinc3 filter_type\n");
+					return -EINVAL;
+				}
+			}
+
 			ret = ad7173_set_channel(&st->sd, i);
-		else
+		} else {
 			ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(i), 2, 0);
+		}
 		if (ret < 0)
 			return ret;
 	}
 
+	if (sinc3_count && sinc3_count < bitmap_weight(scan_mask, indio_dev->num_channels)) {
+		dev_err(&st->sd.spi->dev,
+			"All enabled channels must have sinc3 filter_type\n");
+		return -EINVAL;
+	}
+
 	/*
 	 * On some chips, there are more channels that setups, so if there were
 	 * more unique setups requested than the number of available slots,
@@ -1415,6 +1582,7 @@ static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
 		.storagebits = 32,
 		.endianness = IIO_BE,
 	},
+	.ext_info = ad7173_temp_chan_spec_ext_info,
 };
 
 static void ad7173_disable_regulators(void *data)
@@ -1655,7 +1823,11 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan_st_priv->cfg.bipolar = false;
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
 		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
+		chan_st_priv->cfg.sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(
+			st->info->sinc5_data_rates[st->info->odr_start_value]
+		);
 		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
+		chan_st_priv->cfg.filter_type = AD7173_FILTER_SINC5_SINC1;
 		chan_st_priv->cfg.openwire_comp_chan = -1;
 		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
 		if (st->info->data_reg_only_16bit)
@@ -1727,7 +1899,11 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan->scan_index = chan_index;
 		chan->channel = ain[0];
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
+		chan_st_priv->cfg.sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(
+			st->info->sinc5_data_rates[st->info->odr_start_value]
+		);
 		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
+		chan_st_priv->cfg.filter_type = AD7173_FILTER_SINC5_SINC1;
 		chan_st_priv->cfg.openwire_comp_chan = -1;
 
 		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");

-- 
2.43.0

[PATCH 4/5] iio: ABI: alphabetize filter types

[David Lechner]

Put the filter types in alphabetical order by name. This makes it easier
to find a specific filter type when looking through the documentation.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index fcc40d211ddf388ad70f489177ba2fcebdb9f8dc..3a13596b6eb081483a1f45fb0c446d2ab3b4c708 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2307,21 +2307,21 @@ Description:
 		  conversion time. Poor noise performance.
 		* "sinc3" - The digital sinc3 filter. Moderate 1st
 		  conversion time. Good noise performance.
-		* "sinc4" - Sinc 4. Excellent noise performance. Long
-		  1st conversion time.
-		* "sinc5" - The digital sinc5 filter. Excellent noise
-		  performance
-		* "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
-		  time.
-		* "sinc3+rej60" - Sinc3 + 60Hz rejection.
-		* "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
-		  time.
 		* "sinc3+pf1" - Sinc3 + device specific Post Filter 1.
 		* "sinc3+pf2" - Sinc3 + device specific Post Filter 2.
 		* "sinc3+pf3" - Sinc3 + device specific Post Filter 3.
 		* "sinc3+pf4" - Sinc3 + device specific Post Filter 4.
-		* "sinc5+pf1" - Sinc5 + device specific Post Filter 1.
+		* "sinc3+rej60" - Sinc3 + 60Hz rejection.
+		* "sinc3+sinc1" - Sinc3 + averaging by 8. Low 1st conversion
+		  time.
+		* "sinc4" - Sinc 4. Excellent noise performance. Long
+		  1st conversion time.
+		* "sinc4+sinc1" - Sinc4 + averaging by 8. Low 1st conversion
+		  time.
+		* "sinc5" - The digital sinc5 filter. Excellent noise
+		  performance
 		* "sinc5+avg" - Sinc5 + averaging by 4.
+		* "sinc5+pf1" - Sinc5 + device specific Post Filter 1.
 		* "wideband" - filter with wideband low ripple passband
 		  and sharp transition band.
 

-- 
2.43.0

[PATCH 5/5] iio: ABI: add filter types for ad7173

[David Lechner]

Add new filter types used in the ad7173 driver to the IIO ABI
documentation. These chips have a few filter types that haven't been
seen before in other drivers.

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

diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
index 3a13596b6eb081483a1f45fb0c446d2ab3b4c708..05ecf0b551698d3c7bf2d225507a1e0ecc253c86 100644
--- a/Documentation/ABI/testing/sysfs-bus-iio
+++ b/Documentation/ABI/testing/sysfs-bus-iio
@@ -2322,6 +2322,11 @@ Description:
 		  performance
 		* "sinc5+avg" - Sinc5 + averaging by 4.
 		* "sinc5+pf1" - Sinc5 + device specific Post Filter 1.
+		* "sinc5+sinc1" - Sinc5 + Sinc1.
+		* "sinc5+sinc1+pf1" - Sinc5 + Sinc1 + device specific Post Filter 1.
+		* "sinc5+sinc1+pf2" - Sinc5 + Sinc1 + device specific Post Filter 2.
+		* "sinc5+sinc1+pf3" - Sinc5 + Sinc1 + device specific Post Filter 3.
+		* "sinc5+sinc1+pf4" - Sinc5 + Sinc1 + device specific Post Filter 4.
 		* "wideband" - filter with wideband low ripple passband
 		  and sharp transition band.
 

-- 
2.43.0

Re: [PATCH 0/5] iio: adc: ad7137: add filter support

[David Lechner]

On 7/10/25 5:39 PM, David Lechner wrote:
> Adding yet another feature to the ad7173 driver, this time,
> filter support.
> 
> There are a couple of leading patches to rename some stuff to minimize
> the diff in the main patch where filter support is actually added. And
> there is a bonus patch to clean up the ABI docs for filter_type first
> before adding the new filter types introduced in this series.
> 
> This was tested on the EVAL-AD7173-8ARDZ evaluation board.
> 
> This series depends on a bunch of fixes, so we'll have to wait for
> those to make it back into iio/testing before we can merge this
> series. There is also an outstanding patch to add SPI offload support
> to this driver, but that doesn't actually have any merge conflicts
> with this series, so they can be applied in any order.
> 
> ---
> David Lechner (5):
>       iio: adc: ad7173: rename ad7173_chan_spec_ext_info
>       iio: adc: ad7173: rename odr field
>       iio: adc: ad7173: support changing filter type
>       iio: ABI: alphabetize filter types
>       iio: ABI: add filter types for ad7173
> 
I don't know why, but I really struggle to write this part number
correctly. The subject of this cover letter is wrong, but at least
I got it right in all of the patch subject lines.

Re: [PATCH 2/5] iio: adc: ad7173: rename odr field

[Jonathan Cameron]

On Thu, 10 Jul 2025 17:39:51 -0500
David Lechner <dlechner@baylibre.com> wrote:

> Rename odr to sinc5_odr_index in the channel setup structure. In a
> following commit, we will be adding a separate odr field for when the
> sinc3 filter is used instead so having sinc5 in the name will help
> avoid confusion. And _index makes it more clear that this is an index
> of the sinc5_data_rates array and not the output data rate itself.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
This one can't be applied until the fixes has worked it's way back.
Looks fine though.

> ---
>  drivers/iio/adc/ad7173.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 5daf21c6ba5637b2e47dcd052bdd019c3ecbb442..01d78d531d6c024dd92fff21b8b2afb750092b66 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -205,7 +205,7 @@ struct ad7173_channel_config {
>  	struct_group(config_props,
>  		bool bipolar;
>  		bool input_buf;
> -		u8 odr;
> +		u8 sinc5_odr_index;
>  		u8 ref_sel;
>  	);
>  };
> @@ -582,13 +582,13 @@ static bool ad7173_setup_equal(const struct ad7173_channel_config *cfg1,
>  		      sizeof(struct {
>  				     bool bipolar;
>  				     bool input_buf;
> -				     u8 odr;
> +				     u8 sinc5_odr_index;
>  				     u8 ref_sel;
>  			     }));
>  
>  	return cfg1->bipolar == cfg2->bipolar &&
>  	       cfg1->input_buf == cfg2->input_buf &&
> -	       cfg1->odr == cfg2->odr &&
> +	       cfg1->sinc5_odr_index == cfg2->sinc5_odr_index &&
>  	       cfg1->ref_sel == cfg2->ref_sel;
>  }
>  
> @@ -650,7 +650,7 @@ static int ad7173_load_config(struct ad7173_state *st,
>  		return ret;
>  
>  	return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
> -			       AD7173_FILTER_ODR0_MASK & cfg->odr);
> +			       AD7173_FILTER_ODR0_MASK & cfg->sinc5_odr_index);
>  }
>  
>  static int ad7173_config_channel(struct ad7173_state *st, int addr)
> @@ -1183,7 +1183,7 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> -		reg = st->channels[chan->address].cfg.odr;
> +		reg = st->channels[chan->address].cfg.sinc5_odr_index;
>  
>  		*val = st->info->sinc5_data_rates[reg] / MILLI;
>  		*val2 = (st->info->sinc5_data_rates[reg] % MILLI) * (MICRO / MILLI);
> @@ -1229,7 +1229,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
>  				break;
>  
>  		cfg = &st->channels[chan->address].cfg;
> -		cfg->odr = i;
> +		cfg->sinc5_odr_index = i;
>  		cfg->live = false;
>  		break;
>  
> @@ -1655,7 +1655,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan_st_priv->cfg.bipolar = false;
>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
>  		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
> -		chan_st_priv->cfg.odr = st->info->odr_start_value;
> +		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
>  		chan_st_priv->cfg.openwire_comp_chan = -1;
>  		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>  		if (st->info->data_reg_only_16bit)
> @@ -1727,7 +1727,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan->scan_index = chan_index;
>  		chan->channel = ain[0];
>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> -		chan_st_priv->cfg.odr = st->info->odr_start_value;
> +		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
>  		chan_st_priv->cfg.openwire_comp_chan = -1;
>  
>  		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
> 

Re: [PATCH 0/5] iio: adc: ad7137: add filter support

[Jonathan Cameron]

On Thu, 10 Jul 2025 17:47:14 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/10/25 5:39 PM, David Lechner wrote:
> > Adding yet another feature to the ad7173 driver, this time,
> > filter support.
> > 
> > There are a couple of leading patches to rename some stuff to minimize
> > the diff in the main patch where filter support is actually added. And
> > there is a bonus patch to clean up the ABI docs for filter_type first
> > before adding the new filter types introduced in this series.
> > 
> > This was tested on the EVAL-AD7173-8ARDZ evaluation board.
> > 
> > This series depends on a bunch of fixes, so we'll have to wait for
> > those to make it back into iio/testing before we can merge this
> > series. There is also an outstanding patch to add SPI offload support
> > to this driver, but that doesn't actually have any merge conflicts
> > with this series, so they can be applied in any order.
> > 
> > ---
> > David Lechner (5):
> >       iio: adc: ad7173: rename ad7173_chan_spec_ext_info
> >       iio: adc: ad7173: rename odr field
> >       iio: adc: ad7173: support changing filter type
> >       iio: ABI: alphabetize filter types
> >       iio: ABI: add filter types for ad7173
> >   
> I don't know why, but I really struggle to write this part number
> correctly. The subject of this cover letter is wrong, but at least
> I got it right in all of the patch subject lines.
> 

Series look good to me. Give me a poke if it looks like I've forgotten
to pick this up after the precursor fix is in my tree.

Jonathan

Re: [PATCH 0/5] iio: adc: ad7137: add filter support

[David Lechner]

On 7/24/25 8:20 AM, Jonathan Cameron wrote:
> On Thu, 10 Jul 2025 17:47:14 -0500
> David Lechner <dlechner@baylibre.com> wrote:
> 
>> On 7/10/25 5:39 PM, David Lechner wrote:
>>> Adding yet another feature to the ad7173 driver, this time,
>>> filter support.
>>>
>>> There are a couple of leading patches to rename some stuff to minimize
>>> the diff in the main patch where filter support is actually added. And
>>> there is a bonus patch to clean up the ABI docs for filter_type first
>>> before adding the new filter types introduced in this series.
>>>
>>> This was tested on the EVAL-AD7173-8ARDZ evaluation board.
>>>
>>> This series depends on a bunch of fixes, so we'll have to wait for
>>> those to make it back into iio/testing before we can merge this
>>> series. There is also an outstanding patch to add SPI offload support
>>> to this driver, but that doesn't actually have any merge conflicts
>>> with this series, so they can be applied in any order.
>>>
>>> ---
>>> David Lechner (5):
>>>       iio: adc: ad7173: rename ad7173_chan_spec_ext_info
>>>       iio: adc: ad7173: rename odr field
>>>       iio: adc: ad7173: support changing filter type
>>>       iio: ABI: alphabetize filter types
>>>       iio: ABI: add filter types for ad7173
>>>   
>> I don't know why, but I really struggle to write this part number
>> correctly. The subject of this cover letter is wrong, but at least
>> I got it right in all of the patch subject lines.
>>
> 
> Series look good to me. Give me a poke if it looks like I've forgotten
> to pick this up after the precursor fix is in my tree.
> 
> Jonathan

Sure, no problem. Can we pick up PATCH 4/5 ("iio: ABI: alphabetize
filter types") sooner though? I know there is at least one other
series under review that is adding more filter types and I am
getting ready to start on another one that will likely introduce
some more variants.

Re: [PATCH 3/5] iio: adc: ad7173: support changing filter type

[Nuno Sá]

On Thu, Jul 10, 2025 at 05:39:52PM -0500, David Lechner wrote:
> Add support for changing the filter type to the ad7173 driver.
> 
> This family of chips by default uses a sinc5+sinc1 filter. There are
> also optional post-filters that can be added in this configuration for
> various 50/60Hz rejection purposes. The sinc3 filter doesn't have any
> post-filters and handles the output data rate (ODR) a bit differently.
> Here, we've opted to use SINC3_MAPx to get the maximum possible
> sampling frequencies with the SINC3 filter.
> 
> Adding support consists of adding the filter_type and
> filter_type_available attributes, making the sampling_frequency
> attribute aware of the filter type, and programming the filter
> parameters when we configure the channel of the ADC for reading
> a sample.
> 
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/adc/ad7173.c | 186 +++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 181 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 01d78d531d6c024dd92fff21b8b2afb750092b66..c235096fbad4aeb77a7385001a16bc0ecd603f46 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -8,6 +8,7 @@
>   *  AD7175-8/AD7176-2/AD7177-2
>   *
>   * Copyright (C) 2015, 2024 Analog Devices, Inc.
> + * Copyright (C) 2025 BayLibre, SAS
>   */
>  
>  #include <linux/array_size.h>
> @@ -149,7 +150,12 @@
>  					       (pin2) < st->info->num_voltage_in && \
>  					       (pin2) >= st->info->num_voltage_in_div)
>  
> -#define AD7173_FILTER_ODR0_MASK		GENMASK(5, 0)
> +#define AD7173_FILTER_SINC3_MAP		BIT(15)
> +#define AD7173_FILTER_SINC3_MAP_DIV	GENMASK(14, 0)
> +#define AD7173_FILTER_ENHFILTEN		BIT(11)
> +#define AD7173_FILTER_ENHFILT_MASK	GENMASK(10, 8)
> +#define AD7173_FILTER_ORDER		BIT(6)
> +#define AD7173_FILTER_ODR_MASK		GENMASK(5, 0)
>  #define AD7173_MAX_CONFIGS		8
>  #define AD4111_OW_DET_THRSH_MV		300
>  
> @@ -190,6 +196,15 @@ struct ad7173_device_info {
>  	u8 num_gpios;
>  };
>  
> +enum ad7173_filter_type {
> +	AD7173_FILTER_SINC3,
> +	AD7173_FILTER_SINC5_SINC1,
> +	AD7173_FILTER_SINC5_SINC1_PF1,
> +	AD7173_FILTER_SINC5_SINC1_PF2,
> +	AD7173_FILTER_SINC5_SINC1_PF3,
> +	AD7173_FILTER_SINC5_SINC1_PF4,
> +};
> +
>  struct ad7173_channel_config {
>  	/* Openwire detection threshold */
>  	unsigned int openwire_thrsh_raw;
> @@ -205,8 +220,10 @@ struct ad7173_channel_config {
>  	struct_group(config_props,
>  		bool bipolar;
>  		bool input_buf;
> +		u16 sinc3_ord_div;

typo? ord -> odr?

>  		u8 sinc5_odr_index;
>  		u8 ref_sel;
> +		enum ad7173_filter_type filter_type;
>  	);
>  };
>  
> @@ -266,6 +283,24 @@ static const unsigned int ad7175_sinc5_data_rates[] = {
>  	5000,					/* 20    */
>  };
>  
> +/**
> + * ad7173_sinc3_ord_div_from_odr() - Convert ODR to divider value
> + * @odr_millihz: ODR (sampling_frequency) in milliHz
> + * Returns: Divider value for SINC3 filter to pass.
> + */
> +static u16 ad7173_sinc3_ord_div_from_odr(u32 odr_millihz)

same typo in the func name?

> +{
> +	/*
> +	 * Divider is f_MOD (1 MHz) / 32 / ODR. ODR freq is in milliHz, so
> +	 * we need to convert f_MOD to the same units. When SING_CYC=1 or
> +	 * multiple channels are enabled (currently always the case), there
> +	 * is an additional factor of 3.
> +	 */
> +	u32 div = DIV_ROUND_CLOSEST(MEGA * MILLI, odr_millihz * 32 * 3);
> +	/* Avoid divide by 0 and limit to register field size. */
> +	return clamp(div, 1U, AD7173_FILTER_SINC3_MAP_DIV);
> +}
> +
>  static unsigned int ad4111_current_channel_config[] = {
>  	/* Ain sel: pos        neg    */
>  	0x1E8, /* 15:IIN0+    8:IIN0− */
> @@ -369,6 +404,47 @@ static const struct iio_enum ad7173_syscalib_mode_enum = {
>  	.get = ad7173_get_syscalib_mode
>  };
>  
> +static const char * const ad7173_filter_types_str[] = {
> +	[AD7173_FILTER_SINC3] = "sinc3",
> +	[AD7173_FILTER_SINC5_SINC1] = "sinc5+sinc1",
> +	[AD7173_FILTER_SINC5_SINC1_PF1] = "sinc5+sinc1+pf1",
> +	[AD7173_FILTER_SINC5_SINC1_PF2] = "sinc5+sinc1+pf2",
> +	[AD7173_FILTER_SINC5_SINC1_PF3] = "sinc5+sinc1+pf3",
> +	[AD7173_FILTER_SINC5_SINC1_PF4] = "sinc5+sinc1+pf4",
> +};
> +
> +static int ad7173_set_filter_type(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan,
> +				  unsigned int val)
> +{
> +	struct ad7173_state *st = iio_priv(indio_dev);
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +
> +	st->channels[chan->address].cfg.filter_type = val;
> +	st->channels[chan->address].cfg.live = false;
> +
> +	iio_device_release_direct(indio_dev);
> +
> +	return 0;
> +}
> +
> +static int ad7173_get_filter_type(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan)
> +{
> +	struct ad7173_state *st = iio_priv(indio_dev);
> +
> +	return st->channels[chan->address].cfg.filter_type;
> +}

I know, I'm usual picky with this but FWIW, the above is a possible data
race. Anyways, still up to you...

- Nuno Sá

> +
> +static const struct iio_enum ad7173_filter_type_enum = {
> +	.items = ad7173_filter_types_str,
> +	.num_items = ARRAY_SIZE(ad7173_filter_types_str),
> +	.set = ad7173_set_filter_type,
> +	.get = ad7173_get_filter_type,
> +};
> +
>  static const struct iio_chan_spec_ext_info ad7173_chan_spec_ext_info[] = {
>  	{
>  		.name = "sys_calibration",
> @@ -379,6 +455,16 @@ static const struct iio_chan_spec_ext_info ad7173_chan_spec_ext_info[] = {
>  		 &ad7173_syscalib_mode_enum),
>  	IIO_ENUM_AVAILABLE("sys_calibration_mode", IIO_SHARED_BY_TYPE,
>  			   &ad7173_syscalib_mode_enum),
> +	IIO_ENUM("filter_type", IIO_SEPARATE, &ad7173_filter_type_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_TYPE,
> +			   &ad7173_filter_type_enum),
> +	{ }
> +};
> +
> +static const struct iio_chan_spec_ext_info ad7173_temp_chan_spec_ext_info[] = {
> +	IIO_ENUM("filter_type", IIO_SEPARATE, &ad7173_filter_type_enum),
> +	IIO_ENUM_AVAILABLE("filter_type", IIO_SHARED_BY_TYPE,
> +			   &ad7173_filter_type_enum),
>  	{ }
>  };
>  
> @@ -582,14 +668,18 @@ static bool ad7173_setup_equal(const struct ad7173_channel_config *cfg1,
>  		      sizeof(struct {
>  				     bool bipolar;
>  				     bool input_buf;
> +				     u16 sinc3_ord_div;
>  				     u8 sinc5_odr_index;
>  				     u8 ref_sel;
> +				     enum ad7173_filter_type filter_type;
>  			     }));
>  
>  	return cfg1->bipolar == cfg2->bipolar &&
>  	       cfg1->input_buf == cfg2->input_buf &&
> +	       cfg1->sinc3_ord_div == cfg2->sinc3_ord_div &&
>  	       cfg1->sinc5_odr_index == cfg2->sinc5_odr_index &&
> -	       cfg1->ref_sel == cfg2->ref_sel;
> +	       cfg1->ref_sel == cfg2->ref_sel &&
> +	       cfg1->filter_type == cfg2->filter_type;
>  }
>  
>  static struct ad7173_channel_config *
> @@ -630,6 +720,7 @@ static int ad7173_load_config(struct ad7173_state *st,
>  {
>  	unsigned int config;
>  	int free_cfg_slot, ret;
> +	u8 post_filter_enable, post_filter_select;
>  
>  	free_cfg_slot = ida_alloc_range(&st->cfg_slots_status, 0,
>  					st->info->num_configs - 1, GFP_KERNEL);
> @@ -649,8 +740,49 @@ static int ad7173_load_config(struct ad7173_state *st,
>  	if (ret)
>  		return ret;
>  
> +	/*
> +	 * When SINC3_MAP flag is enabled, the rest of the register has a
> +	 * different meaning. We are using this option to allow the most
> +	 * possible sampling frequencies with SINC3 filter.
> +	 */
> +	if (cfg->filter_type == AD7173_FILTER_SINC3)
> +		return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
> +				       FIELD_PREP(AD7173_FILTER_SINC3_MAP, 1) |
> +				       FIELD_PREP(AD7173_FILTER_SINC3_MAP_DIV,
> +						  cfg->sinc3_ord_div));
> +
> +	switch (cfg->filter_type) {
> +	case AD7173_FILTER_SINC5_SINC1_PF1:
> +		post_filter_enable = 1;
> +		post_filter_select = 2;
> +		break;
> +	case AD7173_FILTER_SINC5_SINC1_PF2:
> +		post_filter_enable = 1;
> +		post_filter_select = 3;
> +		break;
> +	case AD7173_FILTER_SINC5_SINC1_PF3:
> +		post_filter_enable = 1;
> +		post_filter_select = 5;
> +		break;
> +	case AD7173_FILTER_SINC5_SINC1_PF4:
> +		post_filter_enable = 1;
> +		post_filter_select = 6;
> +		break;
> +	default:
> +		post_filter_enable = 0;
> +		post_filter_select = 0;
> +		break;
> +	}
> +
>  	return ad_sd_write_reg(&st->sd, AD7173_REG_FILTER(free_cfg_slot), 2,
> -			       AD7173_FILTER_ODR0_MASK & cfg->sinc5_odr_index);
> +			       FIELD_PREP(AD7173_FILTER_SINC3_MAP, 0) |
> +			       FIELD_PREP(AD7173_FILTER_ENHFILT_MASK,
> +					  post_filter_enable) |
> +			       FIELD_PREP(AD7173_FILTER_ENHFILTEN,
> +					  post_filter_select) |
> +			       FIELD_PREP(AD7173_FILTER_ORDER, 0) |
> +			       FIELD_PREP(AD7173_FILTER_ODR_MASK,
> +					  cfg->sinc5_odr_index));
>  }
>  
>  static int ad7173_config_channel(struct ad7173_state *st, int addr)
> @@ -1183,6 +1315,13 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
>  			return -EINVAL;
>  		}
>  	case IIO_CHAN_INFO_SAMP_FREQ:
> +		if (st->channels[chan->address].cfg.filter_type == AD7173_FILTER_SINC3) {
> +			/* Inverse operation of ad7173_sinc3_ord_div_from_odr() */
> +			*val = MEGA;
> +			*val2 = 3 * 32 * st->channels[chan->address].cfg.sinc3_ord_div;
> +			return IIO_VAL_FRACTIONAL;
> +		}
> +
>  		reg = st->channels[chan->address].cfg.sinc5_odr_index;
>  
>  		*val = st->info->sinc5_data_rates[reg] / MILLI;
> @@ -1221,6 +1360,10 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
>  	 *
>  	 * This will cause the reading of CH1 to be actually done once every
>  	 * 200.16ms, an effective rate of 4.99sps.
> +	 *
> +	 * Both the sinc5 and sinc3 rates are set here so that if the filter
> +	 * type is changed, the requested rate will still be set (aside from
> +	 * rounding differences).
>  	 */
>  	case IIO_CHAN_INFO_SAMP_FREQ:
>  		freq = val * MILLI + val2 / MILLI;
> @@ -1230,6 +1373,7 @@ static int ad7173_write_raw(struct iio_dev *indio_dev,
>  
>  		cfg = &st->channels[chan->address].cfg;
>  		cfg->sinc5_odr_index = i;
> +		cfg->sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(freq);
>  		cfg->live = false;
>  		break;
>  
> @@ -1246,17 +1390,40 @@ static int ad7173_update_scan_mode(struct iio_dev *indio_dev,
>  				   const unsigned long *scan_mask)
>  {
>  	struct ad7173_state *st = iio_priv(indio_dev);
> +	u16 sinc3_count = 0;
> +	u16 sinc3_div = 0;
>  	int i, j, k, ret;
>  
>  	for (i = 0; i < indio_dev->num_channels; i++) {
> -		if (test_bit(i, scan_mask))
> +		const struct ad7173_channel_config *cfg = &st->channels[i].cfg;
> +
> +		if (test_bit(i, scan_mask)) {
> +			if (cfg->filter_type == AD7173_FILTER_SINC3) {
> +				sinc3_count++;
> +
> +				if (sinc3_div == 0) {
> +					sinc3_div = cfg->sinc3_ord_div;
> +				} else if (sinc3_div != cfg->sinc3_ord_div) {
> +					dev_err(&st->sd.spi->dev,
> +						"All enabled channels must have the same sampling_frequency for sinc3 filter_type\n");
> +					return -EINVAL;
> +				}
> +			}
> +
>  			ret = ad7173_set_channel(&st->sd, i);
> -		else
> +		} else {
>  			ret = ad_sd_write_reg(&st->sd, AD7173_REG_CH(i), 2, 0);
> +		}
>  		if (ret < 0)
>  			return ret;
>  	}
>  
> +	if (sinc3_count && sinc3_count < bitmap_weight(scan_mask, indio_dev->num_channels)) {
> +		dev_err(&st->sd.spi->dev,
> +			"All enabled channels must have sinc3 filter_type\n");
> +		return -EINVAL;
> +	}
> +
>  	/*
>  	 * On some chips, there are more channels that setups, so if there were
>  	 * more unique setups requested than the number of available slots,
> @@ -1415,6 +1582,7 @@ static const struct iio_chan_spec ad7173_temp_iio_channel_template = {
>  		.storagebits = 32,
>  		.endianness = IIO_BE,
>  	},
> +	.ext_info = ad7173_temp_chan_spec_ext_info,
>  };
>  
>  static void ad7173_disable_regulators(void *data)
> @@ -1655,7 +1823,11 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan_st_priv->cfg.bipolar = false;
>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
>  		chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
> +		chan_st_priv->cfg.sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(
> +			st->info->sinc5_data_rates[st->info->odr_start_value]
> +		);
>  		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
> +		chan_st_priv->cfg.filter_type = AD7173_FILTER_SINC5_SINC1;
>  		chan_st_priv->cfg.openwire_comp_chan = -1;
>  		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>  		if (st->info->data_reg_only_16bit)
> @@ -1727,7 +1899,11 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan->scan_index = chan_index;
>  		chan->channel = ain[0];
>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> +		chan_st_priv->cfg.sinc3_ord_div = ad7173_sinc3_ord_div_from_odr(
> +			st->info->sinc5_data_rates[st->info->odr_start_value]
> +		);
>  		chan_st_priv->cfg.sinc5_odr_index = st->info->odr_start_value;
> +		chan_st_priv->cfg.filter_type = AD7173_FILTER_SINC5_SINC1;
>  		chan_st_priv->cfg.openwire_comp_chan = -1;
>  
>  		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
> 
> -- 
> 2.43.0
> 

Re: [PATCH 0/5] iio: adc: ad7137: add filter support

[Nuno Sá]

On Thu, Jul 10, 2025 at 05:47:14PM -0500, David Lechner wrote:
> On 7/10/25 5:39 PM, David Lechner wrote:
> > Adding yet another feature to the ad7173 driver, this time,
> > filter support.
> > 
> > There are a couple of leading patches to rename some stuff to minimize
> > the diff in the main patch where filter support is actually added. And
> > there is a bonus patch to clean up the ABI docs for filter_type first
> > before adding the new filter types introduced in this series.
> > 
> > This was tested on the EVAL-AD7173-8ARDZ evaluation board.
> > 
> > This series depends on a bunch of fixes, so we'll have to wait for
> > those to make it back into iio/testing before we can merge this
> > series. There is also an outstanding patch to add SPI offload support
> > to this driver, but that doesn't actually have any merge conflicts
> > with this series, so they can be applied in any order.
> > 
> > ---
> > David Lechner (5):
> >       iio: adc: ad7173: rename ad7173_chan_spec_ext_info
> >       iio: adc: ad7173: rename odr field
> >       iio: adc: ad7173: support changing filter type
> >       iio: ABI: alphabetize filter types
> >       iio: ABI: add filter types for ad7173
> > 
> I don't know why, but I really struggle to write this part number
> correctly. The subject of this cover letter is wrong, but at least
> I got it right in all of the patch subject lines.

Series looks good. Just some (apparent) typos and some other thing that
you may or may not change. So, at least with the typos fixed:

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

Re: [PATCH 0/5] iio: adc: ad7137: add filter support

[Jonathan Cameron]

On Thu, 24 Jul 2025 13:50:01 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/24/25 8:20 AM, Jonathan Cameron wrote:
> > On Thu, 10 Jul 2025 17:47:14 -0500
> > David Lechner <dlechner@baylibre.com> wrote:
> >   
> >> On 7/10/25 5:39 PM, David Lechner wrote:  
> >>> Adding yet another feature to the ad7173 driver, this time,
> >>> filter support.
> >>>
> >>> There are a couple of leading patches to rename some stuff to minimize
> >>> the diff in the main patch where filter support is actually added. And
> >>> there is a bonus patch to clean up the ABI docs for filter_type first
> >>> before adding the new filter types introduced in this series.
> >>>
> >>> This was tested on the EVAL-AD7173-8ARDZ evaluation board.
> >>>
> >>> This series depends on a bunch of fixes, so we'll have to wait for
> >>> those to make it back into iio/testing before we can merge this
> >>> series. There is also an outstanding patch to add SPI offload support
> >>> to this driver, but that doesn't actually have any merge conflicts
> >>> with this series, so they can be applied in any order.
> >>>
> >>> ---
> >>> David Lechner (5):
> >>>       iio: adc: ad7173: rename ad7173_chan_spec_ext_info
> >>>       iio: adc: ad7173: rename odr field
> >>>       iio: adc: ad7173: support changing filter type
> >>>       iio: ABI: alphabetize filter types
> >>>       iio: ABI: add filter types for ad7173
> >>>     
> >> I don't know why, but I really struggle to write this part number
> >> correctly. The subject of this cover letter is wrong, but at least
> >> I got it right in all of the patch subject lines.
> >>  
> > 
> > Series look good to me. Give me a poke if it looks like I've forgotten
> > to pick this up after the precursor fix is in my tree.
> > 
> > Jonathan  
> 
> Sure, no problem. Can we pick up PATCH 4/5 ("iio: ABI: alphabetize
> filter types") sooner though? I know there is at least one other
> series under review that is adding more filter types and I am
> getting ready to start on another one that will likely introduce
> some more variants.
> 
Sure.  Patch 4 applied to the testing branch of iio.git.

Thanks,

Jonathan