Linux IIO development
 help / color / mirror / Atom feed
* [PATCH 0/2] iio: adc: ti-ads112c14: continuous mode support
@ 2026-07-24 20:13 David Lechner (TI)
  2026-07-24 20:13 ` [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support David Lechner (TI)
  2026-07-24 20:13 ` [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support David Lechner (TI)
  0 siblings, 2 replies; 7+ messages in thread
From: David Lechner (TI) @ 2026-07-24 20:13 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: Chris Hall, Patrick Edwards, Kurt Borja, linux-iio, linux-kernel,
	David Lechner (TI)

The TI ADS112C14 ADC has a continuous mode where it can trigger
conversions using its own clock and signals that data is ready using
the /DRDY line. This allows capturing data at higher rates since it
avoids the settling time between conversions. But it is limited to only
being able to sample one channel at a time.

Users will be able to select between continuous and single-shot mode
for buffered reads by selecting the trigger. If the "own" trigger from
the /DRDY interrupt is selected, continuous mode will be used. Or if
some other (software) trigger is selected, single-shot mode will be used
as was already implemented before this series.

Supporting open-drain /DRDY is deferred for future work. In simple cases
we have just done a `drive-open-drain` DT property for this, but on this
chip there is more than one pin (GPIO, FAULT interrupt) that could also
need to be open-drain and not all at the same time.

Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
David Lechner (TI) (2):
      iio: adc: ti-ads112c14: add DRDY interrupt support
      iio: adc: ti-ads112c14: add continuous mode support

 drivers/iio/adc/ti-ads112c14.c | 278 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 251 insertions(+), 27 deletions(-)
---
base-commit: 0b5e142ced4bcf20532da051934bd694d1bbd470
change-id: 20260724-iio-adc-ti-ads112c14-continuous-mode-dd1580754609
prerequisite-change-id: 20260714-iio-adc-ti-ads112c14-buffered-read-41e5d1ca0dc3:v2
prerequisite-patch-id: 8723aa599dcf5b836fb81cf1efb6c9096bf60b7a
prerequisite-patch-id: 12754335ccf210bcdeaa8a5d60b2280618d50993

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support
  2026-07-24 20:13 [PATCH 0/2] iio: adc: ti-ads112c14: continuous mode support David Lechner (TI)
@ 2026-07-24 20:13 ` David Lechner (TI)
  2026-07-27  1:34   ` Jonathan Cameron
  2026-07-24 20:13 ` [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support David Lechner (TI)
  1 sibling, 1 reply; 7+ messages in thread
From: David Lechner (TI) @ 2026-07-24 20:13 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: Chris Hall, Patrick Edwards, Kurt Borja, linux-iio, linux-kernel,
	David Lechner (TI)

Add handling for the DRDY interrupt to wait for data ready events rather
than polling (only when it is wired up).

Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---

Small note: the hard-coded 100 ms timeout will be replaced in a future
series with a dynamic value, so I didn't bother with a macro or comments
to explain why the value was chosen.

And passing indio_dev instead of data to irq is intentional as it will
be used in the next patch.
---
 drivers/iio/adc/ti-ads112c14.c | 82 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 75 insertions(+), 7 deletions(-)

diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
index 8ad8caee0ff7..57e301c15314 100644
--- a/drivers/iio/adc/ti-ads112c14.c
+++ b/drivers/iio/adc/ti-ads112c14.c
@@ -10,6 +10,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/cleanup.h>
+#include <linux/completion.h>
 #include <linux/crc8.h>
 #include <linux/delay.h>
 #include <linux/dev_printk.h>
@@ -19,6 +20,7 @@
 #include <linux/iio/iio.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
+#include <linux/interrupt.h>
 #include <linux/math64.h>
 #include <linux/minmax.h>
 #include <linux/module.h>
@@ -117,9 +119,15 @@
 #define   ADS112C14_GPIO_CFG_GPIO2_CFG			GENMASK(5, 4)
 #define   ADS112C14_GPIO_CFG_GPIO1_CFG			GENMASK(3, 2)
 #define   ADS112C14_GPIO_CFG_GPIO0_CFG			GENMASK(1, 0)
+#define     ADS112C14_GPIO_CFG_GPIO_CFG_DISABLED	  0
+#define     ADS112C14_GPIO_CFG_GPIO_CFG_INPUT		  1
+#define     ADS112C14_GPIO_CFG_GPIO_CFG_OUTPUT_PUSH_PULL  2
+#define     ADS112C14_GPIO_CFG_GPIO_CFG_OUTPUT_OPEN_DRAIN 3
 
 #define ADS112C14_REG_GPIO_DATA_OUTPUT			0x0C
 #define   ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC		BIT(7)
+#define     ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC_DAT_OUT  0
+#define     ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC_DRDY	  1
 #define   ADS112C14_GPIO_DATA_OUTPUT_GPIO2_SRC		BIT(6)
 #define   ADS112C14_GPIO_DATA_OUTPUT_GPIO3_DAT_OUT	BIT(3)
 #define   ADS112C14_GPIO_DATA_OUTPUT_GPIO2_DAT_OUT	BIT(2)
@@ -251,6 +259,8 @@ struct ads112c14_data {
 	struct regmap *regmap;
 	/* Synchronizes access to register value fields. */
 	struct mutex lock;
+	int drdy_irq;
+	struct completion drdy_completion;
 	bool i2c_crc_enabled;
 	u32 avdd_uV;
 	u32 ext_ref_uV;
@@ -265,6 +275,16 @@ struct ads112c14_data {
 						 ARRAY_SIZE(ads112c14_sys_mon_channels));
 };
 
+static irqreturn_t ads112c14_drdy_irq_handler(int irq, void *private)
+{
+	struct iio_dev *indio_dev = private;
+	struct ads112c14_data *data = iio_priv(indio_dev);
+
+	complete(&data->drdy_completion);
+
+	return IRQ_HANDLED;
+}
+
 static bool ads112c14_writeable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -601,17 +621,33 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
 			return ret;
 	}
 
+	if (data->drdy_irq) {
+		reinit_completion(&data->drdy_completion);
+		enable_irq(data->drdy_irq);
+	}
+
 	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
 			   ADS112C14_CONVERSION_CTRL_START);
-	if (ret)
+	if (ret) {
+		if (data->drdy_irq)
+			disable_irq(data->drdy_irq);
 		return ret;
+	}
 
-	ret = regmap_read_poll_timeout(data->regmap,
-				       ADS112C14_REG_STATUS_MSB, reg_val,
-				       FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
-				       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
-	if (ret)
-		return ret;
+	if (data->drdy_irq) {
+		ret = wait_for_completion_timeout(&data->drdy_completion,
+						  msecs_to_jiffies(100));
+		disable_irq(data->drdy_irq);
+		if (ret == 0)
+			return -ETIMEDOUT;
+	} else {
+		ret = regmap_read_poll_timeout(data->regmap,
+					       ADS112C14_REG_STATUS_MSB, reg_val,
+					       FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
+					       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * When doing buffered read, we don't check the CRC, but rather pass it
@@ -1392,6 +1428,38 @@ static int ads112c14_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
+	if (device_property_present(dev, "interrupts")) {
+		data->drdy_irq = fwnode_irq_get_byname(dev_fwnode(dev), "drdy");
+		if (data->drdy_irq < 0)
+			return dev_err_probe(dev, data->drdy_irq,
+					     "failed to get drdy interrupt\n");
+
+		/*
+		 * REVISIT: would probably need to implement a pin controller in
+		 * order to support open drain option here.
+		 */
+		ret = regmap_update_bits(data->regmap, ADS112C14_REG_GPIO_CFG,
+					 ADS112C14_GPIO_CFG_GPIO3_CFG,
+					 FIELD_PREP(ADS112C14_GPIO_CFG_GPIO3_CFG,
+						    ADS112C14_GPIO_CFG_GPIO_CFG_OUTPUT_PUSH_PULL));
+		if (ret)
+			return ret;
+
+		ret = regmap_update_bits(data->regmap, ADS112C14_REG_GPIO_DATA_OUTPUT,
+					 ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC,
+					 FIELD_PREP(ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC,
+						    ADS112C14_GPIO_DATA_OUTPUT_GPIO3_SRC_DRDY));
+		if (ret)
+			return ret;
+
+		init_completion(&data->drdy_completion);
+
+		ret = devm_request_irq(dev, data->drdy_irq, ads112c14_drdy_irq_handler,
+				       IRQF_NO_AUTOEN, dev_name(dev), indio_dev);
+		if (ret)
+			return ret;
+	}
+
 	ads112c14_populate_tables(data);
 
 	indio_dev->name = info->name;

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support
  2026-07-24 20:13 [PATCH 0/2] iio: adc: ti-ads112c14: continuous mode support David Lechner (TI)
  2026-07-24 20:13 ` [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support David Lechner (TI)
@ 2026-07-24 20:13 ` David Lechner (TI)
  2026-07-27  1:45   ` Jonathan Cameron
  1 sibling, 1 reply; 7+ messages in thread
From: David Lechner (TI) @ 2026-07-24 20:13 UTC (permalink / raw)
  To: Jonathan Cameron, Nuno Sá, Andy Shevchenko
  Cc: Chris Hall, Patrick Edwards, Kurt Borja, linux-iio, linux-kernel,
	David Lechner (TI)

Add support for continuous mode in the TI ADS112C14 ADC driver. In this
mode the ADC itself is starting each conversion, so we add a trigger
based on the DRDY interrupt to read each sample. This mode is also
limited in that only one channel can be enabled at a time since the
chip does not have a sequencer or simultaneous sampling capability.
Continuous mode will only be used when this new trigger is the current
trigger.

Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
---
 drivers/iio/adc/ti-ads112c14.c | 200 ++++++++++++++++++++++++++++++++++++-----
 1 file changed, 178 insertions(+), 22 deletions(-)

diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
index 57e301c15314..f3a2eba55362 100644
--- a/drivers/iio/adc/ti-ads112c14.c
+++ b/drivers/iio/adc/ti-ads112c14.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/bitfield.h>
+#include <linux/bitmap.h>
 #include <linux/cleanup.h>
 #include <linux/completion.h>
 #include <linux/crc8.h>
@@ -18,6 +19,7 @@
 #include <linux/i2c.h>
 #include <linux/iio/buffer.h>
 #include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
 #include <linux/iio/trigger_consumer.h>
 #include <linux/iio/triggered_buffer.h>
 #include <linux/interrupt.h>
@@ -257,6 +259,7 @@ struct ads112c14_measurement {
 struct ads112c14_data {
 	const struct ads112c14_chip_info *chip_info;
 	struct regmap *regmap;
+	struct iio_trigger *drdy_trig;
 	/* Synchronizes access to register value fields. */
 	struct mutex lock;
 	int drdy_irq;
@@ -280,11 +283,32 @@ static irqreturn_t ads112c14_drdy_irq_handler(int irq, void *private)
 	struct iio_dev *indio_dev = private;
 	struct ads112c14_data *data = iio_priv(indio_dev);
 
-	complete(&data->drdy_completion);
+	if (iio_trigger_using_own(indio_dev))
+		iio_trigger_poll(data->drdy_trig);
+	else
+		complete(&data->drdy_completion);
 
 	return IRQ_HANDLED;
 }
 
+static int ads112c14_trigger_set_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct ads112c14_data *data = iio_priv(indio_dev);
+
+	if (state)
+		enable_irq(data->drdy_irq);
+	else
+		disable_irq(data->drdy_irq);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops ads112c14_trigger_ops = {
+	.set_trigger_state = ads112c14_trigger_set_state,
+	.validate_device = iio_trigger_validate_own_device,
+};
+
 static bool ads112c14_writeable_reg(struct device *dev, unsigned int reg)
 {
 	switch (reg) {
@@ -601,6 +625,32 @@ static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
 	return 0;
 }
 
+static int ads112c14_prepare_channel(struct ads112c14_data *data,
+				     const struct iio_chan_spec *chan)
+{
+	if (chan->channel < ADS112C14_SYS_MON_CHANNEL_BASE)
+		return ads112c14_prepare_measurement_channel(data, chan);
+
+	return ads112c14_prepare_sys_mon_channel(data, chan);
+}
+
+static int ads112c14_scan_read(struct ads112c14_data *data, u8 *buf)
+{
+	struct i2c_client *client = to_i2c_client(regmap_get_device(data->regmap));
+	int ret;
+	u8 len;
+
+	len = BITS_TO_BYTES(data->chip_info->resolution_bits);
+	if (data->i2c_crc_enabled)
+		len += 1;
+
+	ret = i2c_smbus_read_i2c_block_data(client, ADS112C14_CMD_RDATA, len, buf);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int ads112c14_single_conversion(struct ads112c14_data *data,
 				       const struct iio_chan_spec *chan,
 				       u8 *buf, bool for_scan)
@@ -611,15 +661,9 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
 
 	guard(mutex)(&data->lock);
 
-	if (chan->channel < ADS112C14_SYS_MON_CHANNEL_BASE) {
-		ret = ads112c14_prepare_measurement_channel(data, chan);
-		if (ret)
-			return ret;
-	} else {
-		ret = ads112c14_prepare_sys_mon_channel(data, chan);
-		if (ret)
-			return ret;
-	}
+	ret = ads112c14_prepare_channel(data, chan);
+	if (ret)
+		return ret;
 
 	if (data->drdy_irq) {
 		reinit_completion(&data->drdy_completion);
@@ -655,17 +699,8 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
 	 * with CRC errors, but rather leave it to userspace to decide what to
 	 * do.
 	 */
-	if (for_scan) {
-		u8 len = BITS_TO_BYTES(data->chip_info->resolution_bits) +
-			 (data->i2c_crc_enabled ? 1 : 0);
-
-		ret = i2c_smbus_read_i2c_block_data(client, ADS112C14_CMD_RDATA,
-						    len, buf);
-		if (ret < 0)
-			return ret;
-
-		return 0;
-	}
+	if (for_scan)
+		return ads112c14_scan_read(data, buf);
 
 	return ads112c14_i2c_read_bytes(client, ADS112C14_CMD_RDATA, buf,
 					BITS_TO_BYTES(data->chip_info->resolution_bits),
@@ -934,10 +969,31 @@ static irqreturn_t ads112c14_trigger_handler(int irq, void *private)
 	struct iio_poll_func *pf = private;
 	struct iio_dev *indio_dev = pf->indio_dev;
 	struct ads112c14_data *data = iio_priv(indio_dev);
+	unsigned int scan_mask_len = iio_get_masklength(indio_dev);
 	u32 offset = 0;
 	u32 i;
 	int ret;
 
+	if (iio_trigger_using_own(indio_dev)) {
+		i = find_first_bit(indio_dev->active_scan_mask, scan_mask_len);
+		if (i >= scan_mask_len)
+			goto out;
+
+		ret = ads112c14_scan_read(data, (u8 *)&data->scan[0]);
+		if (ret) {
+			const struct iio_chan_spec *chan = &indio_dev->channels[i];
+
+			dev_err_once(indio_dev->dev.parent,
+				     "failed to read channel %d: %pe; additional errors will be suppressed\n",
+				     chan->channel, ERR_PTR(ret));
+			goto out;
+		}
+
+		iio_push_to_buffers_with_ts(indio_dev, data->scan,
+					    sizeof(data->scan), pf->timestamp);
+		goto out;
+	}
+
 	iio_for_each_active_channel(indio_dev, i) {
 		const struct iio_chan_spec *chan = &indio_dev->channels[i];
 
@@ -969,6 +1025,90 @@ static const struct iio_info ads112c14_info = {
 	.read_label = ads112c14_read_label,
 };
 
+static bool ads112c14_using_drdy_trigger(struct iio_dev *indio_dev)
+{
+	struct ads112c14_data *data = iio_priv(indio_dev);
+
+	return data->drdy_trig && indio_dev->trig == data->drdy_trig;
+}
+
+static bool ads112c14_validate_scan_mask(struct iio_dev *indio_dev,
+					 const unsigned long *mask)
+{
+	if (!ads112c14_using_drdy_trigger(indio_dev))
+		return true;
+
+	return bitmap_weight(mask, iio_get_masklength(indio_dev)) == 1;
+}
+
+static int ads112c14_buffer_postenable(struct iio_dev *indio_dev)
+{
+	struct ads112c14_data *data = iio_priv(indio_dev);
+	unsigned int scan_mask_len = iio_get_masklength(indio_dev);
+	unsigned int i;
+	const struct iio_chan_spec *chan;
+	int ret;
+
+	if (!ads112c14_using_drdy_trigger(indio_dev))
+		return 0;
+
+	i = find_first_bit(indio_dev->active_scan_mask, scan_mask_len);
+	if (i >= scan_mask_len)
+		return -EINVAL;
+
+	chan = &indio_dev->channels[i];
+
+	guard(mutex)(&data->lock);
+
+	ret = ads112c14_prepare_channel(data, chan);
+	if (ret)
+		return ret;
+
+	ret = regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
+				 ADS112C14_DEVICE_CFG_CONV_MODE,
+				 ADS112C14_DEVICE_CFG_CONV_MODE_CONTINUOUS);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
+			   ADS112C14_CONVERSION_CTRL_START);
+	if (ret) {
+		regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
+				   ADS112C14_DEVICE_CFG_CONV_MODE,
+				   ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int ads112c14_buffer_predisable(struct iio_dev *indio_dev)
+{
+	struct ads112c14_data *data = iio_priv(indio_dev);
+	int ret, ret2;
+
+	if (!ads112c14_using_drdy_trigger(indio_dev))
+		return 0;
+
+	guard(mutex)(&data->lock);
+
+	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
+			   ADS112C14_CONVERSION_CTRL_STOP);
+	ret2 = regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
+				  ADS112C14_DEVICE_CFG_CONV_MODE,
+				  ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);
+	if (ret2)
+		return ret2;
+
+	return ret;
+}
+
+static const struct iio_buffer_setup_ops ads112c14_buffer_setup_ops = {
+	.postenable = ads112c14_buffer_postenable,
+	.predisable = ads112c14_buffer_predisable,
+	.validate_scan_mask = ads112c14_validate_scan_mask,
+};
+
 static int ads112c14_populate_idac_mag(u32 current_nA, u8 *idac_mag)
 {
 	u32 current_uA = current_nA / (NANO / MICRO);
@@ -1458,6 +1598,21 @@ static int ads112c14_probe(struct i2c_client *client)
 				       IRQF_NO_AUTOEN, dev_name(dev), indio_dev);
 		if (ret)
 			return ret;
+
+		data->drdy_trig = devm_iio_trigger_alloc(dev, "%s-dev%d-drdy",
+							 info->name,
+							 iio_device_id(indio_dev));
+		if (!data->drdy_trig)
+			return -ENOMEM;
+
+		data->drdy_trig->ops = &ads112c14_trigger_ops;
+		iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
+
+		ret = devm_iio_trigger_register(dev, data->drdy_trig);
+		if (ret)
+			return ret;
+
+		indio_dev->trig = iio_trigger_get(data->drdy_trig);
 	}
 
 	ads112c14_populate_tables(data);
@@ -1468,7 +1623,8 @@ static int ads112c14_probe(struct i2c_client *client)
 
 	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
 					      iio_pollfunc_store_time,
-					      ads112c14_trigger_handler, NULL);
+					      ads112c14_trigger_handler,
+					      &ads112c14_buffer_setup_ops);
 	if (ret)
 		return ret;
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support
  2026-07-24 20:13 ` [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support David Lechner (TI)
@ 2026-07-27  1:34   ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-07-27  1:34 UTC (permalink / raw)
  To: David Lechner (TI)
  Cc: Nuno Sá, Andy Shevchenko, Chris Hall, Patrick Edwards,
	Kurt Borja, linux-iio, linux-kernel

On Fri, 24 Jul 2026 15:13:10 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add handling for the DRDY interrupt to wait for data ready events rather
> than polling (only when it is wired up).
> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>

I'm not sure the comment that I've made will still be true by end
of the various series you've posted but for now the complex
code flow would be simplified by duplicating a couple of lines
and having two helper functions to pick between dependent on whether
we have the drdy interrupt or not.

> ---
> 
> Small note: the hard-coded 100 ms timeout will be replaced in a future
> series with a dynamic value, so I didn't bother with a macro or comments
> to explain why the value was chosen.
> 
> And passing indio_dev instead of data to irq is intentional as it will
> be used in the next patch.
> ---
>  drivers/iio/adc/ti-ads112c14.c | 82 ++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 75 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> index 8ad8caee0ff7..57e301c15314 100644
> --- a/drivers/iio/adc/ti-ads112c14.c
> +++ b/drivers/iio/adc/ti-ads112c14.c

> +
>  static bool ads112c14_writeable_reg(struct device *dev, unsigned int reg)
>  {
>  	switch (reg) {
> @@ -601,17 +621,33 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
>  			return ret;
>  	}
>  
> +	if (data->drdy_irq) {
> +		reinit_completion(&data->drdy_completion);
> +		enable_irq(data->drdy_irq);
> +	}
> +
>  	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
>  			   ADS112C14_CONVERSION_CTRL_START);

Given this is basically the only shared code, maybe just split into
two helpers and chose between them?  The outer code in single_conversion
would be shared. I just mean this inner bit to establish we have data.


> -	if (ret)
> +	if (ret) {
> +		if (data->drdy_irq)
> +			disable_irq(data->drdy_irq);
>  		return ret;
> +	}
>  
> -	ret = regmap_read_poll_timeout(data->regmap,
> -				       ADS112C14_REG_STATUS_MSB, reg_val,
> -				       FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
> -				       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
> -	if (ret)
> -		return ret;
> +	if (data->drdy_irq) {
> +		ret = wait_for_completion_timeout(&data->drdy_completion,
> +						  msecs_to_jiffies(100));
> +		disable_irq(data->drdy_irq);
> +		if (ret == 0)
> +			return -ETIMEDOUT;
> +	} else {
> +		ret = regmap_read_poll_timeout(data->regmap,
> +					       ADS112C14_REG_STATUS_MSB, reg_val,
> +					       FIELD_GET(ADS112C14_STATUS_MSB_DRDY, reg_val),
> +					       1 * USEC_PER_MSEC, 100 * USEC_PER_MSEC);
> +		if (ret)
> +			return ret;
> +	}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support
  2026-07-24 20:13 ` [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support David Lechner (TI)
@ 2026-07-27  1:45   ` Jonathan Cameron
  2026-07-27 13:45     ` David Lechner
  0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2026-07-27  1:45 UTC (permalink / raw)
  To: David Lechner (TI)
  Cc: Nuno Sá, Andy Shevchenko, Chris Hall, Patrick Edwards,
	Kurt Borja, linux-iio, linux-kernel

On Fri, 24 Jul 2026 15:13:11 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:

> Add support for continuous mode in the TI ADS112C14 ADC driver. In this
> mode the ADC itself is starting each conversion, so we add a trigger
> based on the DRDY interrupt to read each sample. This mode is also
> limited in that only one channel can be enabled at a time since the
> chip does not have a sequencer or simultaneous sampling capability.
> Continuous mode will only be used when this new trigger is the current
> trigger.
> 
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
Hi David,

A few comments inline.

Thanks,

Jonathan

> ---
>  drivers/iio/adc/ti-ads112c14.c | 200 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 178 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> index 57e301c15314..f3a2eba55362 100644
> --- a/drivers/iio/adc/ti-ads112c14.c
> +++ b/drivers/iio/adc/ti-ads112c14.c

> @@ -601,6 +625,32 @@ static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
>  	return 0;
>  }
>  
> +static int ads112c14_prepare_channel(struct ads112c14_data *data,
> +				     const struct iio_chan_spec *chan)
> +{
> +	if (chan->channel < ADS112C14_SYS_MON_CHANNEL_BASE)
> +		return ads112c14_prepare_measurement_channel(data, chan);
> +
> +	return ads112c14_prepare_sys_mon_channel(data, chan);
> +}

Left these visible as referred to below.

> +
> +static int ads112c14_scan_read(struct ads112c14_data *data, u8 *buf)
> +{
> +	struct i2c_client *client = to_i2c_client(regmap_get_device(data->regmap));
> +	int ret;
> +	u8 len;
> +
> +	len = BITS_TO_BYTES(data->chip_info->resolution_bits);
> +	if (data->i2c_crc_enabled)
> +		len += 1;
> +
> +	ret = i2c_smbus_read_i2c_block_data(client, ADS112C14_CMD_RDATA, len, buf);
> +	if (ret < 0)
> +		return ret;
> +
> +	return 0;
> +}
> +
>  static int ads112c14_single_conversion(struct ads112c14_data *data,
>  				       const struct iio_chan_spec *chan,
>  				       u8 *buf, bool for_scan)
> @@ -611,15 +661,9 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
>  
>  	guard(mutex)(&data->lock);
>  
> -	if (chan->channel < ADS112C14_SYS_MON_CHANNEL_BASE) {
> -		ret = ads112c14_prepare_measurement_channel(data, chan);
> -		if (ret)
> -			return ret;
> -	} else {
> -		ret = ads112c14_prepare_sys_mon_channel(data, chan);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = ads112c14_prepare_channel(data, chan);

Minor obviously but maybe have these factoring out changes (to enable reuse
in the new code) as a precursor step?

> +	if (ret)
> +		return ret;
>  
>  	if (data->drdy_irq) {
>  		reinit_completion(&data->drdy_completion);
> @@ -655,17 +699,8 @@ static int ads112c14_single_conversion(struct ads112c14_data *data,
>  	 * with CRC errors, but rather leave it to userspace to decide what to
>  	 * do.
>  	 */
> -	if (for_scan) {
> -		u8 len = BITS_TO_BYTES(data->chip_info->resolution_bits) +
> -			 (data->i2c_crc_enabled ? 1 : 0);
> -
> -		ret = i2c_smbus_read_i2c_block_data(client, ADS112C14_CMD_RDATA,
> -						    len, buf);
> -		if (ret < 0)
> -			return ret;
> -
> -		return 0;
> -	}
> +	if (for_scan)
> +		return ads112c14_scan_read(data, buf);

Similar on perhaps factoring this out prior to the main change here

> +static bool ads112c14_validate_scan_mask(struct iio_dev *indio_dev,
> +					 const unsigned long *mask)
> +{
> +	if (!ads112c14_using_drdy_trigger(indio_dev))
> +		return true;
> +
> +	return bitmap_weight(mask, iio_get_masklength(indio_dev)) == 1;

Maybe use iio_validate_mask_onehot() as a kind of really obvious documentation
of this final line?

> +}
> +
> +static int ads112c14_buffer_postenable(struct iio_dev *indio_dev)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	unsigned int scan_mask_len = iio_get_masklength(indio_dev);
> +	unsigned int i;
> +	const struct iio_chan_spec *chan;
> +	int ret;

Might as well target reverse xmas tree where you can.

> +
> +	if (!ads112c14_using_drdy_trigger(indio_dev))
> +		return 0;
> +
> +	i = find_first_bit(indio_dev->active_scan_mask, scan_mask_len);
> +	if (i >= scan_mask_len)
> +		return -EINVAL;

> +
> +static int ads112c14_buffer_predisable(struct iio_dev *indio_dev)
> +{
> +	struct ads112c14_data *data = iio_priv(indio_dev);
> +	int ret, ret2;
> +
> +	if (!ads112c14_using_drdy_trigger(indio_dev))
> +		return 0;
> +
> +	guard(mutex)(&data->lock);
> +
> +	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> +			   ADS112C14_CONVERSION_CTRL_STOP);
> +	ret2 = regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> +				  ADS112C14_DEVICE_CFG_CONV_MODE,
> +				  ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);

This is odd looking code. Needs comments on why we should prefer returning
the error for the second call over that for the first.

> +	if (ret2)
> +		return ret2;
> +
> +	return ret;
> +}


>  static int ads112c14_populate_idac_mag(u32 current_nA, u8 *idac_mag)
>  {
>  	u32 current_uA = current_nA / (NANO / MICRO);
> @@ -1458,6 +1598,21 @@ static int ads112c14_probe(struct i2c_client *client)
>  				       IRQF_NO_AUTOEN, dev_name(dev), indio_dev);
>  		if (ret)
>  			return ret;
> +
> +		data->drdy_trig = devm_iio_trigger_alloc(dev, "%s-dev%d-drdy",
> +							 info->name,
> +							 iio_device_id(indio_dev));
> +		if (!data->drdy_trig)
> +			return -ENOMEM;
> +
> +		data->drdy_trig->ops = &ads112c14_trigger_ops;
> +		iio_trigger_set_drvdata(data->drdy_trig, indio_dev);
> +
> +		ret = devm_iio_trigger_register(dev, data->drdy_trig);
> +		if (ret)
> +			return ret;
> +
> +		indio_dev->trig = iio_trigger_get(data->drdy_trig);

Given you are intending to let the trigger choice select behaviour, I'd
not have any default trigger. It's not obvious that the devices own trigger
is the right 'default' given the limitations around its use!

Jonathan

>  	}

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support
  2026-07-27  1:45   ` Jonathan Cameron
@ 2026-07-27 13:45     ` David Lechner
  2026-07-27 21:18       ` Jonathan Cameron
  0 siblings, 1 reply; 7+ messages in thread
From: David Lechner @ 2026-07-27 13:45 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Nuno Sá, Andy Shevchenko, Chris Hall, Patrick Edwards,
	Kurt Borja, linux-iio, linux-kernel

On 7/26/26 8:45 PM, Jonathan Cameron wrote:
> On Fri, 24 Jul 2026 15:13:11 -0500
> "David Lechner (TI)" <dlechner@baylibre.com> wrote:
> 
>> Add support for continuous mode in the TI ADS112C14 ADC driver. In this
>> mode the ADC itself is starting each conversion, so we add a trigger
>> based on the DRDY interrupt to read each sample. This mode is also
>> limited in that only one channel can be enabled at a time since the
>> chip does not have a sequencer or simultaneous sampling capability.
>> Continuous mode will only be used when this new trigger is the current
>> trigger.
>>

...

>> +static int ads112c14_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> +	struct ads112c14_data *data = iio_priv(indio_dev);
>> +	int ret, ret2;
>> +
>> +	if (!ads112c14_using_drdy_trigger(indio_dev))
>> +		return 0;
>> +
>> +	guard(mutex)(&data->lock);
>> +
>> +	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
>> +			   ADS112C14_CONVERSION_CTRL_STOP);
>> +	ret2 = regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
>> +				  ADS112C14_DEVICE_CFG_CONV_MODE,
>> +				  ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);
> 
> This is odd looking code. Needs comments on why we should prefer returning
> the error for the second call over that for the first.

It is arbitrary, I just went with first error wins.

Other option would be to return early since the hardware is going to
be in a broken state anyway.

> 
>> +	if (ret2)
>> +		return ret2;
>> +
>> +	return ret;
>> +}
> 
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support
  2026-07-27 13:45     ` David Lechner
@ 2026-07-27 21:18       ` Jonathan Cameron
  0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2026-07-27 21:18 UTC (permalink / raw)
  To: David Lechner
  Cc: Nuno Sá, Andy Shevchenko, Chris Hall, Patrick Edwards,
	Kurt Borja, linux-iio, linux-kernel

On Mon, 27 Jul 2026 08:45:57 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 7/26/26 8:45 PM, Jonathan Cameron wrote:
> > On Fri, 24 Jul 2026 15:13:11 -0500
> > "David Lechner (TI)" <dlechner@baylibre.com> wrote:
> >   
> >> Add support for continuous mode in the TI ADS112C14 ADC driver. In this
> >> mode the ADC itself is starting each conversion, so we add a trigger
> >> based on the DRDY interrupt to read each sample. This mode is also
> >> limited in that only one channel can be enabled at a time since the
> >> chip does not have a sequencer or simultaneous sampling capability.
> >> Continuous mode will only be used when this new trigger is the current
> >> trigger.
> >>  
> 
> ...
> 
> >> +static int ads112c14_buffer_predisable(struct iio_dev *indio_dev)
> >> +{
> >> +	struct ads112c14_data *data = iio_priv(indio_dev);
> >> +	int ret, ret2;
> >> +
> >> +	if (!ads112c14_using_drdy_trigger(indio_dev))
> >> +		return 0;
> >> +
> >> +	guard(mutex)(&data->lock);
> >> +
> >> +	ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> >> +			   ADS112C14_CONVERSION_CTRL_STOP);
> >> +	ret2 = regmap_assign_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> >> +				  ADS112C14_DEVICE_CFG_CONV_MODE,
> >> +				  ADS112C14_DEVICE_CFG_CONV_MODE_SINGLE_SHOT);  
> > 
> > This is odd looking code. Needs comments on why we should prefer returning
> > the error for the second call over that for the first.  
> 
> It is arbitrary, I just went with first error wins.

?  Looks like second error wins to me!

> 
> Other option would be to return early since the hardware is going to
> be in a broken state anyway.
I'd go for that.

> 
> >   
> >> +	if (ret2)
> >> +		return ret2;
> >> +
> >> +	return ret;
> >> +}  
> > 
> >   


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-07-27 21:18 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24 20:13 [PATCH 0/2] iio: adc: ti-ads112c14: continuous mode support David Lechner (TI)
2026-07-24 20:13 ` [PATCH 1/2] iio: adc: ti-ads112c14: add DRDY interrupt support David Lechner (TI)
2026-07-27  1:34   ` Jonathan Cameron
2026-07-24 20:13 ` [PATCH 2/2] iio: adc: ti-ads112c14: add continuous mode support David Lechner (TI)
2026-07-27  1:45   ` Jonathan Cameron
2026-07-27 13:45     ` David Lechner
2026-07-27 21:18       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox