1. Patchew
  2. linux
  3. [PATCH RFC v3 0/2] iio: adc: ad7173: add ad4111 openwire detection support
  4. View patch

[PATCH RFC v3 2/2] iio: adc: ad7173: add openwire detection support for single conversions

Guillaume Ranquet posted 2 patches 11 months ago
There is a newer version of this series
[PATCH RFC v3 2/2] iio: adc: ad7173: add openwire detection support for single conversions
Posted by Guillaume Ranquet 11 months ago 
Some chips of the ad7173 family supports open wire detection.

Generate a level fault event whenever an external source is disconnected
from the system input on single conversions.

Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
---
 drivers/iio/adc/ad7173.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
index 11ff148cb5a315d32485acf04b8d6f7d0fb6e5fa..d1cba93722673d2f7fd9239074d36e5274527557 100644
--- a/drivers/iio/adc/ad7173.c
+++ b/drivers/iio/adc/ad7173.c
@@ -35,6 +35,7 @@
 #include <linux/units.h>
 
 #include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
@@ -102,6 +103,7 @@
 
 #define AD7173_GPIO_PDSW	BIT(14)
 #define AD7173_GPIO_OP_EN2_3	BIT(13)
+#define AD4111_GPIO_GP_OW_EN	BIT(12)
 #define AD7173_GPIO_MUX_IO	BIT(12)
 #define AD7173_GPIO_SYNC_EN	BIT(11)
 #define AD7173_GPIO_ERR_EN	BIT(10)
@@ -149,6 +151,7 @@
 
 #define AD7173_FILTER_ODR0_MASK		GENMASK(5, 0)
 #define AD7173_MAX_CONFIGS		8
+#define AD4111_OW_DET_THRSH_MV		300
 
 #define AD7173_MODE_CAL_INT_ZERO		0x4 /* Internal Zero-Scale Calibration */
 #define AD7173_MODE_CAL_INT_FULL		0x5 /* Internal Full-Scale Calibration */
@@ -181,11 +184,15 @@ struct ad7173_device_info {
 	bool has_int_ref;
 	bool has_ref2;
 	bool has_internal_fs_calibration;
+	bool has_openwire_det;
 	bool higher_gpio_bits;
 	u8 num_gpios;
 };
 
 struct ad7173_channel_config {
+	/* Openwire detection threshold */
+	unsigned int openwire_thrsh_raw;
+	int openwire_comp_chan;
 	u8 cfg_slot;
 	bool live;
 
@@ -202,6 +209,7 @@ struct ad7173_channel {
 	unsigned int chan_reg;
 	unsigned int ain;
 	struct ad7173_channel_config cfg;
+	bool openwire_det_en;
 };
 
 struct ad7173_state {
@@ -280,6 +288,7 @@ static const struct ad7173_device_info ad4111_device_info = {
 	.has_current_inputs = true,
 	.has_int_ref = true,
 	.has_internal_fs_calibration = true,
+	.has_openwire_det = true,
 	.clock = 2 * HZ_PER_MHZ,
 	.sinc5_data_rates = ad7173_sinc5_data_rates,
 	.num_sinc5_data_rates = ARRAY_SIZE(ad7173_sinc5_data_rates),
@@ -616,6 +625,72 @@ static int ad7173_calibrate_all(struct ad7173_state *st, struct iio_dev *indio_d
 	return 0;
 }
 
+/*
+ * Associative array of channel pairs for open wire detection
+ * The array is indexed by ain and gives the associated channel pair
+ * to perform the open wire detection with
+ * the channel pair [0] is for non differential and pair [1]
+ * is for differential inputs
+ */
+static int openwire_ain_to_channel_pair[][2][2] = {
+/*	AIN     Single    Differential */
+	[0] = { {0, 15},  {1, 2}   },
+	[1] = { {1, 2},   {2, 1}   },
+	[2] = { {3, 4},   {5, 6}   },
+	[3] = { {5, 6},   {6, 5}   },
+	[4] = { {7, 8},   {9, 10}  },
+	[5] = { {9, 10},  {10, 9}  },
+	[6] = { {11, 12}, {13, 14} },
+	[7] = { {13, 14}, {14, 13} },
+};
+
+/*
+ * Openwire detection on ad4111 works by running the same input measurement
+ * on two different channels and compare if the difference between the two
+ * measurements exceeds a certain value (typical 300mV)
+ */
+static int ad4111_openwire_event(struct iio_dev *indio_dev,
+				  const struct iio_chan_spec *chan)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct ad7173_channel *adchan = &st->channels[chan->address];
+	struct ad7173_channel_config *cfg = &adchan->cfg;
+	int ret, val1, val2;
+
+	ret = regmap_set_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, AD4111_GPIO_GP_OW_EN);
+	if (ret)
+		return ret;
+
+	adchan->cfg.openwire_comp_chan =
+		openwire_ain_to_channel_pair[chan->channel][chan->differential][0];
+
+	ret = ad_sigma_delta_single_conversion(indio_dev, chan, &val1);
+	if (ret < 0) {
+		dev_err(&indio_dev->dev, "Error running ad_sigma_delta single conversion: %d", ret);
+		goto out;
+	}
+
+	adchan->cfg.openwire_comp_chan =
+		openwire_ain_to_channel_pair[chan->channel][chan->differential][1];
+
+	ret = ad_sigma_delta_single_conversion(indio_dev, chan, &val2);
+	if (ret < 0) {
+		dev_err(&indio_dev->dev, "Error running ad_sigma_delta single conversion: %d", ret);
+		goto out;
+	}
+
+	if (abs(val1 - val2) > cfg->openwire_thrsh_raw)
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, chan->address,
+						    IIO_EV_TYPE_FAULT, IIO_EV_DIR_OPENWIRE),
+			       iio_get_time_ns(indio_dev));
+
+out:
+	adchan->cfg.openwire_comp_chan = -1;
+	regmap_clear_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, AD4111_GPIO_GP_OW_EN);
+	return ret;
+}
+
 static int ad7173_mask_xlate(struct gpio_regmap *gpio, unsigned int base,
 			     unsigned int offset, unsigned int *reg,
 			     unsigned int *mask)
@@ -813,6 +888,9 @@ static int ad7173_set_channel(struct ad_sigma_delta *sd, unsigned int channel)
 	      FIELD_PREP(AD7173_CH_SETUP_SEL_MASK, st->channels[channel].cfg.cfg_slot) |
 	      st->channels[channel].ain;
 
+	if (st->channels[channel].cfg.openwire_comp_chan >= 0)
+		channel = st->channels[channel].cfg.openwire_comp_chan;
+
 	return ad_sd_write_reg(&st->sd, AD7173_REG_CH(channel), 2, val);
 }
 
@@ -861,6 +939,11 @@ static int ad7173_disable_all(struct ad_sigma_delta *sd)
 
 static int ad7173_disable_one(struct ad_sigma_delta *sd, unsigned int chan)
 {
+	struct ad7173_state *st = ad_sigma_delta_to_ad7173(sd);
+
+	if (st->channels[chan].cfg.openwire_comp_chan >= 0)
+		chan = st->channels[chan].cfg.openwire_comp_chan;
+
 	return ad_sd_write_reg(sd, AD7173_REG_CH(chan), 2, 0);
 }
 
@@ -968,6 +1051,12 @@ static int ad7173_read_raw(struct iio_dev *indio_dev,
 		if (ret < 0)
 			return ret;
 
+		if (ch->openwire_det_en) {
+			ret = ad4111_openwire_event(indio_dev, chan);
+			if (ret < 0)
+				return ret;
+		}
+
 		return IIO_VAL_INT;
 	case IIO_CHAN_INFO_SCALE:
 
@@ -1112,12 +1201,58 @@ static int ad7173_debug_reg_access(struct iio_dev *indio_dev, unsigned int reg,
 	return ad_sd_write_reg(&st->sd, reg, reg_size, writeval);
 }
 
+static int ad7173_write_event_config(struct iio_dev *indio_dev,
+				     const struct iio_chan_spec *chan,
+				     enum iio_event_type type,
+				     enum iio_event_direction dir,
+				     bool state)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct ad7173_channel *adchan = &st->channels[chan->address];
+
+	switch (type) {
+	case IIO_EV_TYPE_FAULT:
+		adchan->openwire_det_en = state;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int ad7173_read_event_config(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
+				     enum iio_event_type type, enum iio_event_direction dir)
+{
+	struct ad7173_state *st = iio_priv(indio_dev);
+	struct ad7173_channel *adchan = &st->channels[chan->address];
+
+	switch (type) {
+	case IIO_EV_TYPE_FAULT:
+		return adchan->openwire_det_en;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static const struct iio_event_spec ad4111_events[] = {
+	{
+		.type = IIO_EV_TYPE_FAULT,
+		.dir = IIO_EV_DIR_OPENWIRE,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE),
+		.mask_shared_by_all = BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
 static const struct iio_info ad7173_info = {
 	.read_raw = &ad7173_read_raw,
 	.write_raw = &ad7173_write_raw,
 	.debugfs_reg_access = &ad7173_debug_reg_access,
 	.validate_trigger = ad_sd_validate_trigger,
 	.update_scan_mode = ad7173_update_scan_mode,
+	.write_event_config = ad7173_write_event_config,
+	.read_event_config = ad7173_read_event_config,
 };
 
 static const struct iio_scan_type ad4113_scan_type = {
@@ -1321,6 +1456,19 @@ static int ad7173_validate_reference(struct ad7173_state *st, int ref_sel)
 	return 0;
 }
 
+static int ad7173_validate_openwire_ain_inputs(struct ad7173_state *st, bool differential,
+					       unsigned int ain0, unsigned int ain1)
+{
+	/*
+	 * If the channel is configured as differential,
+	 * the ad4111 requires specific ains to be used together
+	 */
+	if (differential)
+		return (ain0 % 2) ? (ain0 - 1) == ain1 : (ain0 + 1) == ain1;
+
+	return ain1 == AD4111_VINCOM_INPUT;
+}
+
 static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 {
 	struct ad7173_channel *chans_st_arr, *chan_st_priv;
@@ -1375,6 +1523,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.openwire_comp_chan = -1;
 		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
 		if (st->info->data_reg_only_16bit)
 			chan_arr[chan_index].scan_type = ad4113_scan_type;
@@ -1442,6 +1591,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 		chan_st_priv->chan_reg = chan_index;
 		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
 		chan_st_priv->cfg.odr = 0;
+		chan_st_priv->cfg.openwire_comp_chan = -1;
 
 		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
 		if (chan_st_priv->cfg.bipolar)
@@ -1456,6 +1606,17 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
 			chan_st_priv->cfg.input_buf = st->info->has_input_buf;
 			chan->channel2 = ain[1];
 			chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
+			if (st->info->has_openwire_det &&
+			    ad7173_validate_openwire_ain_inputs(st, chan->differential, ain[0], ain[1])) {
+				chan->event_spec = ad4111_events;
+				chan->num_event_specs = ARRAY_SIZE(ad4111_events);
+				chan_st_priv->cfg.openwire_thrsh_raw =
+					BIT(chan->scan_type.realbits - !!(chan_st_priv->cfg.bipolar))
+					* AD4111_OW_DET_THRSH_MV
+					/ ad7173_get_ref_voltage_milli(st, chan_st_priv->cfg.ref_sel);
+				if (chan->channel < st->info->num_voltage_in_div)
+					chan_st_priv->cfg.openwire_thrsh_raw /= AD4111_DIVIDER_RATIO;
+			}
 		}
 
 		if (st->info->data_reg_only_16bit)

-- 
2.47.1
Re: [PATCH RFC v3 2/2] iio: adc: ad7173: add openwire detection support for single conversions
Posted by David Lechner 10 months, 4 weeks ago 
On 1/16/25 9:01 AM, Guillaume Ranquet wrote:
> Some chips of the ad7173 family supports open wire detection.
> 
> Generate a level fault event whenever an external source is disconnected
> from the system input on single conversions.
> 
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
> ---

...

> @@ -1375,6 +1523,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.openwire_comp_chan = -1;

		^

>  		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>  		if (st->info->data_reg_only_16bit)
>  			chan_arr[chan_index].scan_type = ad4113_scan_type;
> @@ -1442,6 +1591,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>  		chan_st_priv->chan_reg = chan_index;
>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
>  		chan_st_priv->cfg.odr = 0;
> +		chan_st_priv->cfg.openwire_comp_chan = -1;

		^

Looks like the same line added twice in the same function.

>  
>  		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
>  		if (chan_st_priv->cfg.bipolar)
Re: [PATCH RFC v3 2/2] iio: adc: ad7173: add openwire detection support for single conversions
Posted by Guillaume Ranquet 10 months, 3 weeks ago 
On Sat, 18 Jan 2025 18:25, David Lechner <dlechner@baylibre.com> wrote:
>On 1/16/25 9:01 AM, Guillaume Ranquet wrote:
>> Some chips of the ad7173 family supports open wire detection.
>>
>> Generate a level fault event whenever an external source is disconnected
>> from the system input on single conversions.
>>
>> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
>> ---
>
>...
>
>> @@ -1375,6 +1523,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.openwire_comp_chan = -1;
>
>		^
>
>>  		st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>>  		if (st->info->data_reg_only_16bit)
>>  			chan_arr[chan_index].scan_type = ad4113_scan_type;
>> @@ -1442,6 +1591,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>>  		chan_st_priv->chan_reg = chan_index;
>>  		chan_st_priv->cfg.input_buf = st->info->has_input_buf;
>>  		chan_st_priv->cfg.odr = 0;
>> +		chan_st_priv->cfg.openwire_comp_chan = -1;
>
>		^
>
>Looks like the same line added twice in the same function.

Indeed, but the first one is for the temperature channel which the
parse_channel_config function handles separately than the rests of the
input that are into the `device_for_each_child_node_scoped()`

Thx,
Guillaume.
>
>>
>>  		chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
>>  		if (chan_st_priv->cfg.bipolar)
Re: [PATCH RFC v3 2/2] iio: adc: ad7173: add openwire detection support for single conversions
Posted by Jonathan Cameron 10 months, 4 weeks ago 
On Thu, 16 Jan 2025 16:01:47 +0100
Guillaume Ranquet <granquet@baylibre.com> wrote:

> Some chips of the ad7173 family supports open wire detection.
> 
> Generate a level fault event whenever an external source is disconnected
> from the system input on single conversions.
> 
> Signed-off-by: Guillaume Ranquet <granquet@baylibre.com>
Hi Guillaume.

In general this series looks fine to me.
A few things inline + maybe drop the RFC for v4.
There are some reviewers who will not take a close look until after that.
Not sure that applies to any of our regulars in IIO but it is appropriate
to drop it anyway at this point I think!

Jonathan

> ---
>  drivers/iio/adc/ad7173.c | 161 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 161 insertions(+)
> 
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 11ff148cb5a315d32485acf04b8d6f7d0fb6e5fa..d1cba93722673d2f7fd9239074d36e5274527557 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -35,6 +35,7 @@

> +/*
> + * Associative array of channel pairs for open wire detection
> + * The array is indexed by ain and gives the associated channel pair
> + * to perform the open wire detection with
> + * the channel pair [0] is for non differential and pair [1]
> + * is for differential inputs
> + */
> +static int openwire_ain_to_channel_pair[][2][2] = {
> +/*	AIN     Single    Differential */
> +	[0] = { {0, 15},  {1, 2}   },
> +	[1] = { {1, 2},   {2, 1}   },
> +	[2] = { {3, 4},   {5, 6}   },
> +	[3] = { {5, 6},   {6, 5}   },
> +	[4] = { {7, 8},   {9, 10}  },
> +	[5] = { {9, 10},  {10, 9}  },
> +	[6] = { {11, 12}, {13, 14} },
> +	[7] = { {13, 14}, {14, 13} },
> +};
> +
> +/*
> + * Openwire detection on ad4111 works by running the same input measurement
> + * on two different channels and compare if the difference between the two
> + * measurements exceeds a certain value (typical 300mV)
> + */
> +static int ad4111_openwire_event(struct iio_dev *indio_dev,
> +				  const struct iio_chan_spec *chan)
> +{
> +	struct ad7173_state *st = iio_priv(indio_dev);
> +	struct ad7173_channel *adchan = &st->channels[chan->address];
> +	struct ad7173_channel_config *cfg = &adchan->cfg;
> +	int ret, val1, val2;
> +
> +	ret = regmap_set_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, AD4111_GPIO_GP_OW_EN);

Given you need to do a v4 for some issues below, please also rewrap to sub 80 chars
where it doesn't hurt readability.


> +	if (ret)
> +		return ret;
> +
> +	adchan->cfg.openwire_comp_chan =
> +		openwire_ain_to_channel_pair[chan->channel][chan->differential][0];
> +
> +	ret = ad_sigma_delta_single_conversion(indio_dev, chan, &val1);
> +	if (ret < 0) {
> +		dev_err(&indio_dev->dev, "Error running ad_sigma_delta single conversion: %d", ret);
> +		goto out;
> +	}
> +
> +	adchan->cfg.openwire_comp_chan =
> +		openwire_ain_to_channel_pair[chan->channel][chan->differential][1];
> +
> +	ret = ad_sigma_delta_single_conversion(indio_dev, chan, &val2);
> +	if (ret < 0) {
> +		dev_err(&indio_dev->dev, "Error running ad_sigma_delta single conversion: %d", ret);
> +		goto out;
> +	}
> +
> +	if (abs(val1 - val2) > cfg->openwire_thrsh_raw)
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_VOLTAGE, chan->address,
> +						    IIO_EV_TYPE_FAULT, IIO_EV_DIR_OPENWIRE),
> +			       iio_get_time_ns(indio_dev));
> +
> +out:
> +	adchan->cfg.openwire_comp_chan = -1;
> +	regmap_clear_bits(st->reg_gpiocon_regmap, AD7173_REG_GPIO, AD4111_GPIO_GP_OW_EN);
> +	return ret;
> +}

...

> @@ -1112,12 +1201,58 @@ static int ad7173_debug_reg_access(struct iio_dev *indio_dev, unsigned int reg,
>  	return ad_sd_write_reg(&st->sd, reg, reg_size, writeval);
>  }
>  
> +static int ad7173_write_event_config(struct iio_dev *indio_dev,
> +				     const struct iio_chan_spec *chan,
> +				     enum iio_event_type type,
> +				     enum iio_event_direction dir,
> +				     bool state)
> +{
> +	struct ad7173_state *st = iio_priv(indio_dev);
> +	struct ad7173_channel *adchan = &st->channels[chan->address];
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_FAULT:
> +		adchan->openwire_det_en = state;

Fall through looks unlikely to be intended and if it were would
need marking.

		return 0; here and drop the return below.
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int ad7173_read_event_config(struct iio_dev *indio_dev, const struct iio_chan_spec *chan,
> +				     enum iio_event_type type, enum iio_event_direction dir)
> +{
> +	struct ad7173_state *st = iio_priv(indio_dev);
> +	struct ad7173_channel *adchan = &st->channels[chan->address];
> +
> +	switch (type) {
> +	case IIO_EV_TYPE_FAULT:
> +		return adchan->openwire_det_en;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
Unreachable so drop this.

> +}

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.