[PATCH v6 0/2] iio: Add support for TI ADS1X18 ADCs

[PATCH v6 0/2] iio: Add support for TI ADS1X18 ADCs

[Kurt Borja]

Hi,

This series adds a new driver for TI ADS1X18 SPI devices.

This is my first time contributing to the IIO subsystem and making
dt-bindings documentation, so (don't) go easy on me :p.

As explained in Patch 2 changelog, the DRDY interrupt line is shared
with the MOSI pin. This awkward quirk is also found on some Analog
Devices sigma-delta SPI ADCs, so the interrupt and trigger design is
inspired by those.

Thank you in advance for your reviews.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
v2:
  - [Patch 1]:
    - Move MAINTAINERS change here
    - Use generic node names: ads1118@0 -> adc@0
    - Rename file to ti,ads1118.yaml -> ti,ads1018.yaml
    - Drop ti,gain and ti,datarate
    - Add spi-cpha and spi-max-frecuency properties as they are fixed in
      all models
    - Add vdd-supply
    - Make interrupts and drdy-gpios optional properties

  - [Patch 2]:
    - Update probe based on dt-bindings changes
    - Rename file to ti-ads1x18.c -> ti-ads1018.c
    - Rework ads1018_oneshot(), instead of waiting for IRQ wait an
      appropriate delay before reading again
    - Only alloc and register a trigger if we have an IRQ line
    - Drop ads1x18->msg_lock in favor of IIO API locks
    - Read conver before enabling and after disabling IRQ to ensure CS
      state is correct
    - Add ads1018_read_locked() which takes an additional argument
      `hold_cs` to explicitly control CS state in trigger and buffer
    - Fix ADS1X18_CHANNELS_MAX limit 9 -> 10
    - Call iio_trigger_notify_done() in all IRQ handler paths
    - Drop unused includes
    - Drop BIT_U16 and GENMASK_U16 macros
    - Drop unnecessary named defines
    - Use u8 types in ads1018_chan_data
    - Rename some struct members for clarity
    - Move tx_buf and rx_buf to the end of struct ads1018
    - Rework channel handling to just make everything visible and add
      ADS1018_VOLT_DIFF_CHAN
    - Use .scan_index instead of .address in IIO channels
    
  - v1: https://lore.kernel.org/r/20251121-ads1x18-v1-0-86db080fc9a4@gmail.com

---
v3:
  - [Patch 1]:
    - Use unevaluatedProperties: false
    - Drop #address-cells and #size-cells

  - [Patch 2]:
    - Add kernel-doc to internal API
    - Drop bits.h and bitops.h includes
    - Add types.h include
    - Use unsigned type for data_rate_mode_to_hz
    - Rename __ads1018_read_raw() -> ads1018_read_raw_unlocked()
    - Rename __ads1018_write_raw() -> ads1018_write_raw_unlocked()
    - Rename ads1018_read_locked -> ads1018_read_unlocked() for
      consistency
    - Let ads1018_read_unlocked() take NULL cnv pointers
    - Add ads1018_set_trigger_{enable,disable}()
    - Refactor ads1018_write_raw_unlocked() loop matching
    - Invert ads1018_trigger_handler() logic to follow traditional error
      handling pattern
    - Refactor ads1018_trigger_setup() cleaner
    - Make ADS1018_FSR_TO_SCALE() calculation be 32-bit compatible
    - Some additionall minor cleanups

  - Link to v2: https://lore.kernel.org/r/20251127-ads1x18-v2-0-2ebfd780b633@gmail.com

---
v4:
  - [Patch 2]:
    - Replaced <linux/byteorder/generic.h> -> <asm/byteorder.h>
    - Dropped ADS1018_CFG_DEFAULT
    - Fixed long lines
    - Added Andy's remark on ADS1018_FSR_TO_SCALE() kernel-doc
      description.
    - Fixed wrong argument on iio_trigger_notify_done():
      ads1018->indio_trig -> indio_dev->trig
    - Renamed argument in channel macros _addr -> _index
    - Changed return type of ads1018_calc_delay() to u32
    - Mention @cnv is optional in ads1018_read_unlocked()
    - Use 16-bit transmission cycle in ads1018_oneshot()
    - Dropped spi_set_drvdata()
    - Use full resolution in ADS1018_FSR_TO_SCALE() and subtract 1
      inside macro
    - Rename ads1018_read_locked() -> ads1018_spi_read_exclusive() for
      clarity
    - Minor style changes

  - Link to v3: https://lore.kernel.org/r/20251128-ads1x18-v3-0-a6ebab815b2d@gmail.com

---
v5:
  - [Patch 2]:
    - Fix ADS1018_FSR_TO_SCALE() long description
    - In ADS1018_FSR_TO_SCALE() subtract 6 from BIT() argument instead
      of shifting the value

  - Link to v4: https://lore.kernel.org/r/20251202-ads1x18-v4-0-8c3580bc273f@gmail.com

---
v6:
  - [Patch 2]:
    - Actually make the changes described above. Sorry for the noise :(.

  - Link to v5: https://lore.kernel.org/r/20251204-ads1x18-v5-0-b6243de766d1@gmail.com

---
Kurt Borja (2):
      dt-bindings: iio: adc: Add TI ADS1018/ADS1118
      iio: adc: Add ti-ads1018 driver

 .../devicetree/bindings/iio/adc/ti,ads1018.yaml    |  82 ++
 MAINTAINERS                                        |   7 +
 drivers/iio/adc/Kconfig                            |  12 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/ti-ads1018.c                       | 826 +++++++++++++++++++++
 5 files changed, 928 insertions(+)
---
base-commit: f9e05791642810a0cf6237d39fafd6fec5e0b4bb
change-id: 20251012-ads1x18-0d0779d06690

-- 
 ~ Kurt

[PATCH v6 1/2] dt-bindings: iio: adc: Add TI ADS1018/ADS1118

[Kurt Borja]

Add documentation for Texas Instruments ADS1018 and ADS1118
analog-to-digital converters.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 .../devicetree/bindings/iio/adc/ti,ads1018.yaml    | 82 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 ++
 2 files changed, 88 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
new file mode 100644
index 000000000000..93c9b2921a54
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
@@ -0,0 +1,82 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads1018.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI ADS1018/ADS1118 SPI analog to digital converter
+
+maintainers:
+  - Kurt Borja <kuurtb@gmail.com>
+
+description: |
+  The ADS1018/ADS1118 is a precision, low-power, 12-bit or 16-bit, noise-free,
+  analog-to-digital converter (ADC). It integrates a programmable gain amplifier
+  (PGA), voltage reference, oscillator and high-accuracy temperature sensor.
+
+  Datasheets:
+    - ADS1018: https://www.ti.com/lit/ds/symlink/ads1018.pdf
+    - ADS1118: https://www.ti.com/lit/ds/symlink/ads1118.pdf
+
+properties:
+  compatible:
+    enum:
+      - ti,ads1018
+      - ti,ads1118
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+
+  spi-max-frequency:
+    maximum: 4000000
+
+  spi-cpha: true
+
+  interrupts:
+    description: DOUT/DRDY (Data Out/Data Ready) line.
+    maxItems: 1
+
+  drdy-gpios:
+    description:
+      Extra GPIO line connected to DOUT/DRDY (Data Out/Data Ready). This allows
+      distinguishing between interrupts triggered by the data-ready signal and
+      interrupts triggered by an SPI transfer.
+    maxItems: 1
+
+  '#io-channel-cells':
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@0 {
+            compatible = "ti,ads1118";
+            reg = <0>;
+
+            spi-max-frequency = <4000000>;
+            spi-cpha;
+
+            vdd-supply = <&vdd_3v3_reg>;
+
+            interrupts-extended = <&gpio 14 IRQ_TYPE_EDGE_FALLING>;
+            drdy-gpios = <&gpio 14 GPIO_ACTIVE_LOW>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 31d98efb1ad1..3d5295b5d6eb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25646,6 +25646,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/ti,ads1119.yaml
 F:	drivers/iio/adc/ti-ads1119.c
 
+TI ADS1018 ADC DRIVER
+M:	Kurt Borja <kuurtb@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
+
 TI ADS7924 ADC DRIVER
 M:	Hugo Villeneuve <hvilleneuve@dimonoff.com>
 L:	linux-iio@vger.kernel.org

-- 
2.52.0

[PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Kurt Borja]

Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
analog-to-digital converters.

These chips' MOSI pin is shared with a data-ready interrupt. Defining
this interrupt in devicetree is optional, therefore we only create an
IIO trigger if one is found.

Handling this interrupt requires some considerations. When enabling the
trigger the CS line is tied low (active), thus we need to hold
spi_bus_lock() too, to avoid state corruption. This is done inside the
set_trigger_state() callback, to let users use other triggers without
wasting a bus lock.

Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 MAINTAINERS                  |   1 +
 drivers/iio/adc/Kconfig      |  12 +
 drivers/iio/adc/Makefile     |   1 +
 drivers/iio/adc/ti-ads1018.c | 826 +++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 840 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 3d5295b5d6eb..b3822cbff2c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -25651,6 +25651,7 @@ M:	Kurt Borja <kuurtb@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
+F:	drivers/iio/adc/ti-ads1018.c
 
 TI ADS7924 ADC DRIVER
 M:	Hugo Villeneuve <hvilleneuve@dimonoff.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58da8255525e..aa3f7023c64b 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1664,6 +1664,18 @@ config TI_ADS1015
 	  This driver can also be built as a module. If so, the module will be
 	  called ti-ads1015.
 
+config TI_ADS1018
+       tristate "Texas Instruments ADS1018 ADC"
+       depends on SPI
+       select IIO_BUFFER
+       select IIO_TRIGGERED_BUFFER
+       help
+         If you say yes here you get support for Texas Instruments ADS1018 and
+         ADS1118 ADC chips.
+
+         This driver can also be built as a module. If so, the module will be
+         called ti-ads1018.
+
 config TI_ADS1100
 	tristate "Texas Instruments ADS1100 and ADS1000 ADC"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7cc8f9a12f76..72ef79becdec 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -145,6 +145,7 @@ obj-$(CONFIG_TI_ADC12138) += ti-adc12138.o
 obj-$(CONFIG_TI_ADC128S052) += ti-adc128s052.o
 obj-$(CONFIG_TI_ADC161S626) += ti-adc161s626.o
 obj-$(CONFIG_TI_ADS1015) += ti-ads1015.o
+obj-$(CONFIG_TI_ADS1018) += ti-ads1018.o
 obj-$(CONFIG_TI_ADS1100) += ti-ads1100.o
 obj-$(CONFIG_TI_ADS1119) += ti-ads1119.o
 obj-$(CONFIG_TI_ADS124S08) += ti-ads124s08.o
diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
new file mode 100644
index 000000000000..419e789bd0eb
--- /dev/null
+++ b/drivers/iio/adc/ti-ads1018.c
@@ -0,0 +1,826 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Texas Instruments ADS1018 ADC driver
+ *
+ * Copyright (C) 2025 Kurt Borja <kuurtb@gmail.com>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitmap.h>
+#include <linux/dev_printk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/interrupt.h>
+#include <linux/math.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+
+#include <asm/byteorder.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>
+
+#define ADS1018_CFG_OS_TRIG		BIT(15)
+#define ADS1018_CFG_TS_MODE_EN		BIT(4)
+#define ADS1018_CFG_PULL_UP		BIT(3)
+#define ADS1018_CFG_NOP			BIT(1)
+#define ADS1018_CFG_VALID		(ADS1018_CFG_PULL_UP | ADS1018_CFG_NOP)
+
+#define ADS1018_CFG_MUX_MASK		GENMASK(14, 12)
+
+#define ADS1018_CFG_PGA_MASK		GENMASK(11, 9)
+#define ADS1018_PGA_DEFAULT		2
+
+#define ADS1018_CFG_MODE_MASK		BIT(8)
+#define ADS1018_MODE_CONTINUOUS		0
+#define ADS1018_MODE_ONESHOT		1
+
+#define ADS1018_CFG_DRATE_MASK		GENMASK(7, 5)
+#define ADS1018_DRATE_DEFAULT		4
+
+#define ADS1018_CHANNELS_MAX		10
+
+struct ads1018_chan_data {
+	u8 pga_mode;
+	u8 data_rate_mode;
+};
+
+struct ads1018_chip_info {
+	const char *name;
+
+	const struct iio_chan_spec *channels;
+	unsigned long num_channels;
+
+	/* IIO_VAL_INT */
+	const unsigned int *data_rate_mode_to_hz;
+	unsigned long num_data_rate_mode_to_hz;
+
+	/* IIO_VAL_INT_PLUS_NANO */
+	const unsigned int (*pga_mode_to_gain)[2];
+	unsigned long num_pga_mode_to_gain;
+
+	/* IIO_VAL_INT_PLUS_MICRO */
+	const int temp_scale[2];
+};
+
+struct ads1018 {
+	struct spi_device *spi;
+	struct iio_trigger *indio_trig;
+
+	struct gpio_desc *drdy_gpiod;
+	int drdy_irq;
+
+	bool restore_mode;
+
+	struct ads1018_chan_data chan_data[ADS1018_CHANNELS_MAX];
+	const struct ads1018_chip_info *chip_info;
+
+	struct spi_message msg_read;
+	struct spi_transfer xfer;
+	__be16 tx_buf[2] __aligned(IIO_DMA_MINALIGN);
+	__be16 rx_buf[2];
+};
+
+#define ADS1018_VOLT_DIFF_CHAN(_index, _chan, _chan2, _realbits) {		\
+	.type = IIO_VOLTAGE,							\
+	.channel = _chan,							\
+	.channel2 = _chan2,							\
+	.scan_index = _index,							\
+	.scan_type = {								\
+		.sign = 's',							\
+		.realbits = _realbits,						\
+		.storagebits = 16,						\
+		.shift = 16 - _realbits,					\
+		.endianness = IIO_BE,						\
+	},									\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
+			      BIT(IIO_CHAN_INFO_SCALE) |			\
+			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.indexed = true,							\
+	.differential = true,							\
+}
+
+#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
+	.type = IIO_VOLTAGE,							\
+	.channel = _chan,							\
+	.scan_index = _index,							\
+	.scan_type = {								\
+		.sign = 's',							\
+		.realbits = _realbits,						\
+		.storagebits = 16,						\
+		.shift = 16 - _realbits,					\
+		.endianness = IIO_BE,						\
+	},									\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
+			      BIT(IIO_CHAN_INFO_SCALE) |			\
+			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),		\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+	.indexed = true,							\
+}
+
+#define ADS1018_TEMP_CHAN(_index, _realbits) {					\
+	.type = IIO_TEMP,							\
+	.scan_index = _index,							\
+	.scan_type = {								\
+		.sign = 's',							\
+		.realbits = _realbits,						\
+		.storagebits = 16,						\
+		.shift = 16 - _realbits,					\
+		.endianness = IIO_BE,						\
+	},									\
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
+			      BIT(IIO_CHAN_INFO_SCALE) |			\
+			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
+	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
+}
+
+static const struct iio_chan_spec ads1118_iio_channels[] = {
+	ADS1018_VOLT_DIFF_CHAN(0, 0, 1, 16),
+	ADS1018_VOLT_DIFF_CHAN(1, 0, 3, 16),
+	ADS1018_VOLT_DIFF_CHAN(2, 1, 3, 16),
+	ADS1018_VOLT_DIFF_CHAN(3, 2, 3, 16),
+	ADS1018_VOLT_CHAN(4, 0, 16),
+	ADS1018_VOLT_CHAN(5, 1, 16),
+	ADS1018_VOLT_CHAN(6, 2, 16),
+	ADS1018_VOLT_CHAN(7, 3, 16),
+	ADS1018_TEMP_CHAN(8, 14),
+	IIO_CHAN_SOFT_TIMESTAMP(9),
+};
+
+static const struct iio_chan_spec ads1018_iio_channels[] = {
+	ADS1018_VOLT_DIFF_CHAN(0, 0, 1, 12),
+	ADS1018_VOLT_DIFF_CHAN(1, 0, 3, 12),
+	ADS1018_VOLT_DIFF_CHAN(2, 1, 3, 12),
+	ADS1018_VOLT_DIFF_CHAN(3, 2, 3, 12),
+	ADS1018_VOLT_CHAN(4, 0, 12),
+	ADS1018_VOLT_CHAN(5, 1, 12),
+	ADS1018_VOLT_CHAN(6, 2, 12),
+	ADS1018_VOLT_CHAN(7, 3, 12),
+	ADS1018_TEMP_CHAN(8, 12),
+	IIO_CHAN_SOFT_TIMESTAMP(9),
+};
+
+/**
+ * ads1018_get_data_rate_mode - Get current data-rate mode for a channel.
+ * @ads1018: Device data
+ * @address: Channel address
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Return: Current data-rate mode for the channel at @address.
+ */
+static u8 ads1018_get_data_rate_mode(struct ads1018 *ads1018, unsigned int address)
+{
+	return ads1018->chan_data[address].data_rate_mode;
+}
+
+/**
+ * ads1018_get_pga_mode - Get current PGA mode for a channel.
+ * @ads1018: Device data
+ * @address: Channel address
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Return: Current PGA mode for the channel at @address.
+ */
+static u8 ads1018_get_pga_mode(struct ads1018 *ads1018, unsigned int address)
+{
+	return ads1018->chan_data[address].pga_mode;
+}
+
+/**
+ * ads1018_set_data_rate_mode - Set a data-rate mode for a channel.
+ * @ads1018: Device data
+ * @address: Channel address
+ * @val: New data-rate mode for channel at @address.
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Lazily set a new data-rate mode for a channel.
+ */
+static void ads1018_set_data_rate_mode(struct ads1018 *ads1018,
+				       unsigned int address, u8 val)
+{
+	ads1018->chan_data[address].data_rate_mode = val;
+}
+
+/**
+ * ads1018_set_pga_mode - Set a PGA mode for a channel.
+ * @ads1018: Device data
+ * @address: Channel address
+ * @val: New PGA mode for channel at @address.
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Lazily set a new PGA mode for a channel.
+ */
+static void ads1018_set_pga_mode(struct ads1018 *ads1018,
+				 unsigned int address, u8 val)
+{
+	ads1018->chan_data[address].pga_mode = val;
+}
+
+/**
+ * ads1018_calc_delay - Calculates a suitable delay for a single-shot reading
+ * @ads1018: Device data
+ *
+ * Calculates an appropriate delay for a single shot reading, assuming the
+ * device's maximum data-rate is used.
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Return: Delay in microseconds (Always greater than 0).
+ */
+static u32 ads1018_calc_delay(struct ads1018 *ads1018)
+{
+	const struct ads1018_chip_info *chip_info = ads1018->chip_info;
+	unsigned long max_drate_mode = chip_info->num_data_rate_mode_to_hz - 1;
+	unsigned int hz = chip_info->data_rate_mode_to_hz[max_drate_mode];
+
+	/*
+	 * Calculate the worst-case sampling rate on the maximum data-rate
+	 * mode by subtracting 10% error specified in the datasheet.
+	 */
+	hz -= DIV_ROUND_UP(hz, 10);
+
+	/*
+	 * Then calculate time per sample in microseconds.
+	 */
+	return DIV_ROUND_UP(MICROHZ_PER_HZ, hz);
+}
+
+/**
+ * ads1018_spi_read_exclusive - Reads a conversion value from the device
+ * @ads1018: Device data
+ * @cnv: ADC Conversion value (optional)
+ * @hold_cs: Keep CS line asserted after the SPI transfer
+ *
+ * Reads the most recent ADC conversion value, without updating the
+ * device's configuration.
+ *
+ * Context: Expects iio_device_claim_buffer_mode() is held and SPI bus
+ *	    *exclusive* use.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int ads1018_spi_read_exclusive(struct ads1018 *ads1018, __be16 *cnv,
+				      bool hold_cs)
+{
+	int ret;
+
+	ads1018->xfer.cs_change = hold_cs;
+
+	ret = spi_sync_locked(ads1018->spi, &ads1018->msg_read);
+	if (ret)
+		return ret;
+
+	if (cnv)
+		*cnv = ads1018->rx_buf[0];
+
+	return 0;
+}
+
+/**
+ * ads1018_single_shot - Performs a one-shot reading sequence
+ * @ads1018: Device data
+ * @cfg: New configuration for the device
+ * @cnv: Conversion value
+ *
+ * Writes a new configuration, waits an appropriate delay (assuming the new
+ * configuration uses the maximum data-rate) and then reads the most recent
+ * conversion.
+ *
+ * Context: Expects iio_device_claim_direct() is held.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+static int ads1018_single_shot(struct ads1018 *ads1018, u16 cfg, u16 *cnv)
+{
+	struct spi_transfer xfer[2] = {
+		{
+			.tx_buf = ads1018->tx_buf,
+			.len = sizeof(ads1018->tx_buf[0]),
+			.delay = {
+				.value = ads1018_calc_delay(ads1018),
+				.unit = SPI_DELAY_UNIT_USECS,
+			},
+			.cs_change = 1, /* 16-bit mode requires CS de-assert */
+		},
+		{
+			.rx_buf = ads1018->rx_buf,
+			.len = sizeof(ads1018->rx_buf[0]),
+		},
+	};
+	int ret;
+
+	ads1018->tx_buf[0] = cpu_to_be16(cfg);
+
+	ret = spi_sync_transfer(ads1018->spi, xfer, ARRAY_SIZE(xfer));
+	if (ret)
+		return ret;
+
+	*cnv = be16_to_cpu(ads1018->rx_buf[0]);
+
+	return 0;
+}
+
+static int
+ads1018_read_raw_unlocked(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, int *val, int *val2,
+			  long mask)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	const struct ads1018_chip_info *chip_info = ads1018->chip_info;
+	u8 drate_mode = ads1018_get_data_rate_mode(ads1018, chan->scan_index);
+	u8 pga_mode = ads1018_get_pga_mode(ads1018, chan->scan_index);
+	u8 max_drate_mode = chip_info->num_data_rate_mode_to_hz - 1;
+	u16 cnv, cfg;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		cfg = ADS1018_CFG_VALID | ADS1018_CFG_OS_TRIG;
+		cfg |= FIELD_PREP(ADS1018_CFG_MUX_MASK, chan->scan_index);
+		cfg |= FIELD_PREP(ADS1018_CFG_PGA_MASK, pga_mode);
+		cfg |= FIELD_PREP(ADS1018_CFG_MODE_MASK, ADS1018_MODE_ONESHOT);
+		cfg |= FIELD_PREP(ADS1018_CFG_DRATE_MASK, max_drate_mode);
+
+		if (chan->type == IIO_TEMP)
+			cfg |= ADS1018_CFG_TS_MODE_EN;
+
+		ret = ads1018_single_shot(ads1018, cfg, &cnv);
+		if (ret)
+			return ret;
+
+		cnv >>= chan->scan_type.shift;
+		*val = sign_extend32(cnv, chan->scan_type.realbits - 1);
+
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_VOLTAGE:
+			*val = chip_info->pga_mode_to_gain[pga_mode][0];
+			*val2 = chip_info->pga_mode_to_gain[pga_mode][1];
+			return IIO_VAL_INT_PLUS_NANO;
+
+		case IIO_TEMP:
+			*val = chip_info->temp_scale[0];
+			*val2 = chip_info->temp_scale[1];
+			return IIO_VAL_INT_PLUS_MICRO;
+
+		default:
+			return -EOPNOTSUPP;
+		}
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = chip_info->data_rate_mode_to_hz[drate_mode];
+		return IIO_VAL_INT;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+ads1018_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+		 int *val, int *val2, long mask)
+{
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+	ret = ads1018_read_raw_unlocked(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+
+	return ret;
+}
+
+static int
+ads1018_read_avail(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+		   const int **vals, int *type, int *length, long mask)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*vals = (const int *)ads1018->chip_info->pga_mode_to_gain;
+		*length = ads1018->chip_info->num_pga_mode_to_gain * 2;
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*type = IIO_VAL_INT;
+		*vals = ads1018->chip_info->data_rate_mode_to_hz;
+		*length = ads1018->chip_info->num_data_rate_mode_to_hz;
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+ads1018_write_raw_unlocked(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan, int val, int val2,
+			   long mask)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	const struct ads1018_chip_info *info = ads1018->chip_info;
+	unsigned int i;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		for (i = 0; i < info->num_pga_mode_to_gain; i++) {
+			if (val == info->pga_mode_to_gain[i][0] &&
+			    val2 == info->pga_mode_to_gain[i][1])
+				break;
+		}
+		if (i == info->num_pga_mode_to_gain)
+			return -EINVAL;
+
+		ads1018_set_pga_mode(ads1018, chan->scan_index, i);
+		return 0;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		for (i = 0; i < info->num_data_rate_mode_to_hz; i++) {
+			if (val == info->data_rate_mode_to_hz[i])
+				break;
+		}
+		if (i == info->num_data_rate_mode_to_hz)
+			return -EINVAL;
+
+		ads1018_set_data_rate_mode(ads1018, chan->scan_index, i);
+		return 0;
+
+	default:
+		return -EOPNOTSUPP;
+	}
+}
+
+static int
+ads1018_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
+		  int val, int val2, long mask)
+{
+	int ret;
+
+	if (!iio_device_claim_direct(indio_dev))
+		return -EBUSY;
+	ret = ads1018_write_raw_unlocked(indio_dev, chan, val, val2, mask);
+	iio_device_release_direct(indio_dev);
+
+	return ret;
+}
+
+static int
+ads1018_write_raw_get_fmt(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return IIO_VAL_INT_PLUS_MICRO;
+	}
+}
+
+static const struct iio_info ads1018_iio_info = {
+	.read_raw = ads1018_read_raw,
+	.read_avail = ads1018_read_avail,
+	.write_raw = ads1018_write_raw,
+	.write_raw_get_fmt = ads1018_write_raw_get_fmt,
+};
+
+static void ads1018_set_trigger_enable(struct ads1018 *ads1018)
+{
+	spi_bus_lock(ads1018->spi->controller);
+	ads1018_spi_read_exclusive(ads1018, NULL, true);
+	enable_irq(ads1018->drdy_irq);
+}
+
+static void ads1018_set_trigger_disable(struct ads1018 *ads1018)
+{
+	disable_irq(ads1018->drdy_irq);
+	ads1018_spi_read_exclusive(ads1018, NULL, false);
+	spi_bus_unlock(ads1018->spi->controller);
+}
+
+static int ads1018_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+	struct ads1018 *ads1018 = iio_trigger_get_drvdata(trig);
+
+	/*
+	 * We need to lock the SPI bus and tie CS low (hold_cs) to catch
+	 * data-ready interrupts, otherwise the MISO line enters a Hi-Z state.
+	 */
+
+	if (state)
+		ads1018_set_trigger_enable(ads1018);
+	else
+		ads1018_set_trigger_disable(ads1018);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops ads1018_trigger_ops = {
+	.set_trigger_state = ads1018_set_trigger_state,
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static int ads1018_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	const struct ads1018_chip_info *chip_info = ads1018->chip_info;
+	unsigned int pga, drate, addr;
+	u16 cfg;
+
+	addr = find_first_bit(indio_dev->active_scan_mask, iio_get_masklength(indio_dev));
+	pga = ads1018_get_pga_mode(ads1018, addr);
+	drate = ads1018_get_data_rate_mode(ads1018, addr);
+
+	cfg = ADS1018_CFG_VALID;
+	cfg |= FIELD_PREP(ADS1018_CFG_MUX_MASK, addr);
+	cfg |= FIELD_PREP(ADS1018_CFG_PGA_MASK, pga);
+	cfg |= FIELD_PREP(ADS1018_CFG_MODE_MASK, ADS1018_MODE_CONTINUOUS);
+	cfg |= FIELD_PREP(ADS1018_CFG_DRATE_MASK, drate);
+
+	if (chip_info->channels[addr].type == IIO_TEMP)
+		cfg |= ADS1018_CFG_TS_MODE_EN;
+
+	ads1018->tx_buf[0] = cpu_to_be16(cfg);
+	ads1018->tx_buf[1] = 0;
+
+	return spi_write(ads1018->spi, ads1018->tx_buf, sizeof(ads1018->tx_buf));
+}
+
+static int ads1018_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	u16 cfg;
+
+	cfg = ADS1018_CFG_VALID;
+	cfg |= FIELD_PREP(ADS1018_CFG_MODE_MASK, ADS1018_MODE_ONESHOT);
+
+	ads1018->tx_buf[0] = cpu_to_be16(cfg);
+	ads1018->tx_buf[1] = 0;
+
+	return spi_write(ads1018->spi, ads1018->tx_buf, sizeof(ads1018->tx_buf));
+}
+
+static const struct iio_buffer_setup_ops ads1018_buffer_ops = {
+	.preenable = ads1018_buffer_preenable,
+	.postdisable = ads1018_buffer_postdisable,
+	.validate_scan_mask = iio_validate_scan_mask_onehot,
+};
+
+static irqreturn_t ads1018_irq_handler(int irq, void *dev_id)
+{
+	struct ads1018 *ads1018 = dev_id;
+
+	/*
+	 * We need to check if the "drdy" pin is actually active or if it's a
+	 * pending interrupt triggered by the SPI transfer.
+	 */
+	if (!gpiod_get_value(ads1018->drdy_gpiod))
+		return IRQ_HANDLED;
+
+	iio_trigger_poll(ads1018->indio_trig);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t ads1018_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	struct {
+		__be16 conv;
+		aligned_s64 ts;
+	} scan = {};
+	int ret;
+
+	if (iio_device_claim_buffer_mode(indio_dev))
+		goto out_notify_done;
+
+	if (iio_trigger_using_own(indio_dev)) {
+		disable_irq(ads1018->drdy_irq);
+		ret = ads1018_spi_read_exclusive(ads1018, &scan.conv, true);
+		enable_irq(ads1018->drdy_irq);
+	} else {
+		ret = spi_read(ads1018->spi, ads1018->rx_buf, sizeof(ads1018->rx_buf));
+		scan.conv = ads1018->rx_buf[0];
+	}
+	if (ret)
+		goto out_release_buffer;
+
+	iio_push_to_buffers_with_ts(indio_dev, &scan, sizeof(scan), pf->timestamp);
+
+out_release_buffer:
+	iio_device_release_buffer_mode(indio_dev);
+out_notify_done:
+	iio_trigger_notify_done(indio_dev->trig);
+
+	return IRQ_HANDLED;
+}
+
+static int ads1018_trigger_setup(struct iio_dev *indio_dev)
+{
+	struct ads1018 *ads1018 = iio_priv(indio_dev);
+	struct spi_device *spi = ads1018->spi;
+	struct device *dev = &spi->dev;
+	const char *con_id = "drdy";
+	int ret;
+
+	ads1018->drdy_gpiod = devm_gpiod_get_optional(dev, con_id, GPIOD_IN);
+	if (IS_ERR(ads1018->drdy_gpiod))
+		return dev_err_probe(dev, PTR_ERR(ads1018->drdy_gpiod),
+				     "Failed to get %s GPIO.\n", con_id);
+
+	/* First try to get IRQ from SPI core, then from GPIO */
+	if (spi->irq > 0)
+		ads1018->drdy_irq = spi->irq;
+	else if (ads1018->drdy_gpiod)
+		ads1018->drdy_irq = gpiod_to_irq(ads1018->drdy_gpiod);
+	if (ads1018->drdy_irq < 0)
+		return dev_err_probe(dev, ads1018->drdy_irq,
+				     "Failed to get IRQ from %s GPIO.\n", con_id);
+
+	/* An IRQ line is only an optional requirement for the IIO trigger */
+	if (ads1018->drdy_irq == 0)
+		return 0;
+
+	ads1018->indio_trig = devm_iio_trigger_alloc(dev, "%s-dev%d-%s",
+						     indio_dev->name,
+						     iio_device_id(indio_dev),
+						     con_id);
+	if (!ads1018->indio_trig)
+		return -ENOMEM;
+
+	iio_trigger_set_drvdata(ads1018->indio_trig, ads1018);
+	ads1018->indio_trig->ops = &ads1018_trigger_ops;
+
+	ret = devm_iio_trigger_register(dev, ads1018->indio_trig);
+	if (ret)
+		return ret;
+
+	/*
+	 * The "data-ready" IRQ line is shared with the MOSI pin, thus we need
+	 * to keep it disabled until we actually request data.
+	 */
+	return devm_request_irq(dev, ads1018->drdy_irq, ads1018_irq_handler,
+				IRQF_NO_AUTOEN, ads1018->chip_info->name, ads1018);
+}
+
+static int ads1018_spi_probe(struct spi_device *spi)
+{
+	const struct ads1018_chip_info *info = spi_get_device_match_data(spi);
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct ads1018 *ads1018;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*ads1018));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ads1018 = iio_priv(indio_dev);
+	ads1018->spi = spi;
+	ads1018->chip_info = info;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->name = info->name;
+	indio_dev->info = &ads1018_iio_info;
+	indio_dev->channels = info->channels;
+	indio_dev->num_channels = info->num_channels;
+
+	for (unsigned int i = 0; i < ADS1018_CHANNELS_MAX; i++) {
+		ads1018->chan_data[i].data_rate_mode = ADS1018_DRATE_DEFAULT;
+		ads1018->chan_data[i].pga_mode = ADS1018_PGA_DEFAULT;
+	}
+
+	ads1018->xfer.rx_buf = ads1018->rx_buf;
+	ads1018->xfer.len = sizeof(ads1018->rx_buf);
+	spi_message_init_with_transfers(&ads1018->msg_read, &ads1018->xfer, 1);
+
+	ret = ads1018_trigger_setup(indio_dev);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
+					      iio_pollfunc_store_time,
+					      ads1018_trigger_handler,
+					      &ads1018_buffer_ops);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(&spi->dev, indio_dev);
+}
+
+/**
+ * ADS1018_FSR_TO_SCALE - Converts FSR into scale
+ * @_fsr: Full-scale range in millivolts
+ * @_res: ADC resolution
+ *
+ * The macro is crafted to avoid potential overflows on 32-bit machines. This
+ * imposes restrictions on the possible values for @_fsr (less than 274878),
+ * and @_res (greater than or equal to 7 bits).
+ *
+ * Return: Scale in IIO_VAL_INT_PLUS_NANO format
+ */
+#define ADS1018_FSR_TO_SCALE(_fsr, _res) \
+	{ 0, ((_fsr) * (MICRO >> 6)) / (BIT((_res) - 6 - 1)) }
+
+static const unsigned int ads1018_gain_table[][2] = {
+	ADS1018_FSR_TO_SCALE(6144, 12),
+	ADS1018_FSR_TO_SCALE(4096, 12),
+	ADS1018_FSR_TO_SCALE(2048, 12),
+	ADS1018_FSR_TO_SCALE(1024, 12),
+	ADS1018_FSR_TO_SCALE(512, 12),
+	ADS1018_FSR_TO_SCALE(256, 12),
+};
+
+static const unsigned int ads1118_gain_table[][2] = {
+	ADS1018_FSR_TO_SCALE(6144, 16),
+	ADS1018_FSR_TO_SCALE(4096, 16),
+	ADS1018_FSR_TO_SCALE(2048, 16),
+	ADS1018_FSR_TO_SCALE(1024, 16),
+	ADS1018_FSR_TO_SCALE(512, 16),
+	ADS1018_FSR_TO_SCALE(256, 16),
+};
+
+static const unsigned int ads1018_data_rate_table[] = {
+	128, 250, 490, 920, 1600, 2400, 3300,
+};
+
+static const unsigned int ads1118_data_rate_table[] = {
+	8, 16, 32, 64, 128, 250, 475, 860,
+};
+
+static const struct ads1018_chip_info ads1018_chip_info = {
+	.name = "ads1018",
+
+	.channels = ads1018_iio_channels,
+	.num_channels = ARRAY_SIZE(ads1018_iio_channels),
+
+	.pga_mode_to_gain = ads1018_gain_table,
+	.num_pga_mode_to_gain = ARRAY_SIZE(ads1018_gain_table),
+
+	.data_rate_mode_to_hz = ads1018_data_rate_table,
+	.num_data_rate_mode_to_hz = ARRAY_SIZE(ads1018_data_rate_table),
+
+	.temp_scale = { 0, 125000 },
+};
+
+static const struct ads1018_chip_info ads1118_chip_info = {
+	.name = "ads1118",
+
+	.channels = ads1118_iio_channels,
+	.num_channels = ARRAY_SIZE(ads1118_iio_channels),
+
+	.pga_mode_to_gain = ads1118_gain_table,
+	.num_pga_mode_to_gain = ARRAY_SIZE(ads1118_gain_table),
+
+	.data_rate_mode_to_hz = ads1118_data_rate_table,
+	.num_data_rate_mode_to_hz = ARRAY_SIZE(ads1118_data_rate_table),
+
+	.temp_scale = { 0, 31250 },
+};
+
+static const struct of_device_id ads1018_of_match[] = {
+	{ .compatible = "ti,ads1018", .data = &ads1018_chip_info },
+	{ .compatible = "ti,ads1118", .data = &ads1118_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, ads1018_of_match);
+
+static const struct spi_device_id ads1018_spi_match[] = {
+	{ "ads1018", (kernel_ulong_t)&ads1018_chip_info },
+	{ "ads1118", (kernel_ulong_t)&ads1118_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(spi, ads1018_spi_match);
+
+static struct spi_driver ads1018_spi_driver = {
+	.driver = {
+		.name = "ads1018",
+		.of_match_table = ads1018_of_match,
+	},
+	.probe = ads1018_spi_probe,
+	.id_table = ads1018_spi_match,
+};
+
+module_spi_driver(ads1018_spi_driver);
+
+MODULE_DESCRIPTION("Texas Instruments ADS1018 ADC Driver");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Kurt Borja <kuurtb@gmail.com>");

-- 
2.52.0

Re: [PATCH v6 1/2] dt-bindings: iio: adc: Add TI ADS1018/ADS1118

[Jonathan Cameron]

On Thu, 04 Dec 2025 13:01:27 -0500
Kurt Borja <kuurtb@gmail.com> wrote:

> Add documentation for Texas Instruments ADS1018 and ADS1118
> analog-to-digital converters.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
A couple of trivial things that aren't worth a respin.

Thanks,

Jonathan

> ---
>  .../devicetree/bindings/iio/adc/ti,ads1018.yaml    | 82 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 ++
>  2 files changed, 88 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> new file mode 100644
> index 000000000000..93c9b2921a54
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1018.yaml
> @@ -0,0 +1,82 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads1018.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI ADS1018/ADS1118 SPI analog to digital converter
> +
> +maintainers:
> +  - Kurt Borja <kuurtb@gmail.com>
> +
> +description: |
> +  The ADS1018/ADS1118 is a precision, low-power, 12-bit or 16-bit, noise-free,
If you spin again: 12-bit/16-bit
to align with ADS1018/ADS1118 earlier in the line.
Otherwise it sounds like each can do either mode.

I'd also drop the noise-free.  To me that seems overly optimistic marketing and
it doesn't add anything useful.


> +  analog-to-digital converter (ADC). It integrates a programmable gain amplifier
> +  (PGA), voltage reference, oscillator and high-accuracy temperature sensor.
> +
> +  Datasheets:
> +    - ADS1018: https://www.ti.com/lit/ds/symlink/ads1018.pdf
> +    - ADS1118: https://www.ti.com/lit/ds/symlink/ads1118.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,ads1018
> +      - ti,ads1118
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply: true
> +
> +  spi-max-frequency:
> +    maximum: 4000000
> +
> +  spi-cpha: true
> +
> +  interrupts:
> +    description: DOUT/DRDY (Data Out/Data Ready) line.
> +    maxItems: 1
> +
> +  drdy-gpios:
> +    description:
> +      Extra GPIO line connected to DOUT/DRDY (Data Out/Data Ready). This allows
> +      distinguishing between interrupts triggered by the data-ready signal and
> +      interrupts triggered by an SPI transfer.
> +    maxItems: 1
> +
> +  '#io-channel-cells':
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        adc@0 {
> +            compatible = "ti,ads1118";
> +            reg = <0>;
> +
> +            spi-max-frequency = <4000000>;
> +            spi-cpha;
> +
> +            vdd-supply = <&vdd_3v3_reg>;
> +
> +            interrupts-extended = <&gpio 14 IRQ_TYPE_EDGE_FALLING>;
> +            drdy-gpios = <&gpio 14 GPIO_ACTIVE_LOW>;
> +        };
> +    };

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Jonathan Cameron]

On Thu, 04 Dec 2025 13:01:28 -0500
Kurt Borja <kuurtb@gmail.com> wrote:

> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> analog-to-digital converters.
> 
> These chips' MOSI pin is shared with a data-ready interrupt. Defining
> this interrupt in devicetree is optional, therefore we only create an
> IIO trigger if one is found.
> 
> Handling this interrupt requires some considerations. When enabling the
> trigger the CS line is tied low (active), thus we need to hold
> spi_bus_lock() too, to avoid state corruption. This is done inside the
> set_trigger_state() callback, to let users use other triggers without
> wasting a bus lock.
> 
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
Hi Kurt

Generally in a good state.  A few minor things inline.
I didn't understand the delay calculation comments.

Jonathan


> diff --git a/drivers/iio/adc/ti-ads1018.c b/drivers/iio/adc/ti-ads1018.c
> new file mode 100644
> index 000000000000..419e789bd0eb
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads1018.c
> @@ -0,0 +1,826 @@

> +struct ads1018_chip_info {
> +	const char *name;
> +
> +	const struct iio_chan_spec *channels;
> +	unsigned long num_channels;
> +
> +	/* IIO_VAL_INT */
> +	const unsigned int *data_rate_mode_to_hz;
> +	unsigned long num_data_rate_mode_to_hz;
(unlike pga mode this one is fine as there are variable numbers
 of these)
> +
> +	/* IIO_VAL_INT_PLUS_NANO */
> +	const unsigned int (*pga_mode_to_gain)[2];
> +	unsigned long num_pga_mode_to_gain;

Maybe this is a case of premature addition of flexibility given
this is always 6 so far and you could just embed the data in here
rather than access via a pointer.

> +
> +	/* IIO_VAL_INT_PLUS_MICRO */
> +	const int temp_scale[2];
> +};


> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
> +	.type = IIO_VOLTAGE,							\
> +	.channel = _chan,							\
> +	.scan_index = _index,							\
> +	.scan_type = {								\
> +		.sign = 's',							\
> +		.realbits = _realbits,						\
> +		.storagebits = 16,						\
> +		.shift = 16 - _realbits,					\
> +		.endianness = IIO_BE,						\
> +	},									\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\

What motivates per channel sampling frequency?

Given you have to write it each time you configure I guess it doesn't matter much
either way.

> +	.info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),		\
> +	.info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.indexed = true,							\
> +}

> +/**
> + * ads1018_get_data_rate_mode - Get current data-rate mode for a channel.
> + * @ads1018: Device data
> + * @address: Channel address
> + *
> + * Context: Expects iio_device_claim_direct() is held.
> + *
> + * Return: Current data-rate mode for the channel at @address.
> + */
> +static u8 ads1018_get_data_rate_mode(struct ads1018 *ads1018, unsigned int address)
> +{
> +	return ads1018->chan_data[address].data_rate_mode;
> +}
> +
> +/**
> + * ads1018_get_pga_mode - Get current PGA mode for a channel.
> + * @ads1018: Device data
> + * @address: Channel address
> + *
> + * Context: Expects iio_device_claim_direct() is held.
> + *
> + * Return: Current PGA mode for the channel at @address.
> + */
> +static u8 ads1018_get_pga_mode(struct ads1018 *ads1018, unsigned int address)
> +{
> +	return ads1018->chan_data[address].pga_mode;
> +}
> +
> +/**
> + * ads1018_set_data_rate_mode - Set a data-rate mode for a channel.
> + * @ads1018: Device data
> + * @address: Channel address
> + * @val: New data-rate mode for channel at @address.
> + *
> + * Context: Expects iio_device_claim_direct() is held.
> + *
> + * Lazily set a new data-rate mode for a channel.

I'm not sure these tiny access helpers add anything much. Maybe
just get/set the value directly inline?  The field name in the
structure makes it fairly clear what is going on.

> + */
> +static void ads1018_set_data_rate_mode(struct ads1018 *ads1018,
> +				       unsigned int address, u8 val)
> +{
> +	ads1018->chan_data[address].data_rate_mode = val;
> +}
> +
> +/**
> + * ads1018_set_pga_mode - Set a PGA mode for a channel.
> + * @ads1018: Device data
> + * @address: Channel address
> + * @val: New PGA mode for channel at @address.
> + *
> + * Context: Expects iio_device_claim_direct() is held.
> + *
> + * Lazily set a new PGA mode for a channel.
> + */
> +static void ads1018_set_pga_mode(struct ads1018 *ads1018,
> +				 unsigned int address, u8 val)
> +{
> +	ads1018->chan_data[address].pga_mode = val;
> +}
> +
> +/**
> + * ads1018_calc_delay - Calculates a suitable delay for a single-shot reading
> + * @ads1018: Device data
> + *
> + * Calculates an appropriate delay for a single shot reading, assuming the
> + * device's maximum data-rate is used.
> + *
> + * Context: Expects iio_device_claim_direct() is held.

What in here changes if we are in buffered mode?
We have no reason to call it but why does that matter?

> + *
> + * Return: Delay in microseconds (Always greater than 0).
> + */
> +static u32 ads1018_calc_delay(struct ads1018 *ads1018)
> +{
> +	const struct ads1018_chip_info *chip_info = ads1018->chip_info;
> +	unsigned long max_drate_mode = chip_info->num_data_rate_mode_to_hz - 1;
> +	unsigned int hz = chip_info->data_rate_mode_to_hz[max_drate_mode];
> +
> +	/*
> +	 * Calculate the worst-case sampling rate on the maximum data-rate
> +	 * mode by subtracting 10% error specified in the datasheet.
> +	 */
> +	hz -= DIV_ROUND_UP(hz, 10);
> +
> +	/*
> +	 * Then calculate time per sample in microseconds.
> +	 */

Single line comment syntax appropriate here.

> +	return DIV_ROUND_UP(MICROHZ_PER_HZ, hz);
> +}

> +/**
> + * ads1018_single_shot - Performs a one-shot reading sequence
> + * @ads1018: Device data
> + * @cfg: New configuration for the device
> + * @cnv: Conversion value
> + *
> + * Writes a new configuration, waits an appropriate delay (assuming the new
> + * configuration uses the maximum data-rate) and then reads the most recent

I'm lost on this.  Normally the longest delay is governed by the minimum data rate.
I.e. Samples take longer when running few per second, so we wait longer.

I think this is meant to mean the delay needed for a sample at the minimum expected
rate for this configuration.

> + * conversion.
> + *
> + * Context: Expects iio_device_claim_direct() is held.
> + *
> + * Return: 0 on success, negative errno on error.
> + */
> +static int ads1018_single_shot(struct ads1018 *ads1018, u16 cfg, u16 *cnv)
> +{
> +	struct spi_transfer xfer[2] = {
> +		{
> +			.tx_buf = ads1018->tx_buf,
> +			.len = sizeof(ads1018->tx_buf[0]),
> +			.delay = {
> +				.value = ads1018_calc_delay(ads1018),
> +				.unit = SPI_DELAY_UNIT_USECS,
> +			},
> +			.cs_change = 1, /* 16-bit mode requires CS de-assert */
> +		},
> +		{
> +			.rx_buf = ads1018->rx_buf,
> +			.len = sizeof(ads1018->rx_buf[0]),
> +		},
> +	};
> +	int ret;
> +
> +	ads1018->tx_buf[0] = cpu_to_be16(cfg);
> +
> +	ret = spi_sync_transfer(ads1018->spi, xfer, ARRAY_SIZE(xfer));
> +	if (ret)
> +		return ret;
> +
> +	*cnv = be16_to_cpu(ads1018->rx_buf[0]);
> +
> +	return 0;
> +}
> +
> +static int
> +ads1018_read_raw_unlocked(struct iio_dev *indio_dev,

Similar comment to below.  I think unlocked is emphasising the wrong property.

> +			  struct iio_chan_spec const *chan, int *val, int *val2,
> +			  long mask)
> +{

> +static int
> +ads1018_read_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +		 int *val, int *val2, long mask)
> +{
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +	ret = ads1018_read_raw_unlocked(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +
> +	return ret;
> +}

> +
> +static int
> +ads1018_write_raw_unlocked(struct iio_dev *indio_dev,

Similar to the naming discussion on the ACQUIRE RFC I'm not sure
using locked here is really descriptive of more than an internal
detail of how we prevent mode switching. I'd prefer something like
ads1018_write_raw_direct_claimed() or ads1018_write_raw_direct_mode()
(the absence of any other write_raw_*** would indicate this is the only
valid one perhaps).

Also this isn't the unlocked version, it's the one that doesn't take
the lock.


> +			   struct iio_chan_spec const *chan, int val, int val2,
> +			   long mask)
> +{
...

> +
> +static int
> +ads1018_write_raw(struct iio_dev *indio_dev, struct iio_chan_spec const *chan,
> +		  int val, int val2, long mask)
> +{
> +	int ret;
> +
> +	if (!iio_device_claim_direct(indio_dev))
> +		return -EBUSY;
> +	ret = ads1018_write_raw_unlocked(indio_dev, chan, val, val2, mask);
> +	iio_device_release_direct(indio_dev);
> +
> +	return ret;
> +}

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Kurt Borja]

On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:
> On Thu, 04 Dec 2025 13:01:28 -0500
> Kurt Borja <kuurtb@gmail.com> wrote:
>
>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>> analog-to-digital converters.
>> 
>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
>> this interrupt in devicetree is optional, therefore we only create an
>> IIO trigger if one is found.
>> 
>> Handling this interrupt requires some considerations. When enabling the
>> trigger the CS line is tied low (active), thus we need to hold
>> spi_bus_lock() too, to avoid state corruption. This is done inside the
>> set_trigger_state() callback, to let users use other triggers without
>> wasting a bus lock.
>> 
>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>

...

>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
>> +	.type = IIO_VOLTAGE,							\
>> +	.channel = _chan,							\
>> +	.scan_index = _index,							\
>> +	.scan_type = {								\
>> +		.sign = 's',							\
>> +		.realbits = _realbits,						\
>> +		.storagebits = 16,						\
>> +		.shift = 16 - _realbits,					\
>> +		.endianness = IIO_BE,						\
>> +	},									\
>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>
> What motivates per channel sampling frequency?
>
> Given you have to write it each time you configure I guess it doesn't matter much
> either way.

I guess making it shared by all is simpler too, so I'll go with that.

...

>> +/**
>> + * ads1018_calc_delay - Calculates a suitable delay for a single-shot reading
>> + * @ads1018: Device data
>> + *
>> + * Calculates an appropriate delay for a single shot reading, assuming the
>> + * device's maximum data-rate is used.
>> + *
>> + * Context: Expects iio_device_claim_direct() is held.
>
> What in here changes if we are in buffered mode?
> We have no reason to call it but why does that matter?

Yep, I just pasted this mindlessly. I'll remove it.

...

>> +/**
>> + * ads1018_single_shot - Performs a one-shot reading sequence
>> + * @ads1018: Device data
>> + * @cfg: New configuration for the device
>> + * @cnv: Conversion value
>> + *
>> + * Writes a new configuration, waits an appropriate delay (assuming the new
>> + * configuration uses the maximum data-rate) and then reads the most recent
>
> I'm lost on this.  Normally the longest delay is governed by the minimum data rate.
> I.e. Samples take longer when running few per second, so we wait longer.

We are using the minimum data rate on the maximum data-rate mode. I
should have added "mode" there.

>
> I think this is meant to mean the delay needed for a sample at the minimum expected
> rate for this configuration.

Yes, I think this was too confusing.

I'll add a `hz` argument to ads1018_calc_delay() and pass the frequency
of the maximum data-rate mode when preparing the config.

I think this will make the intent more explicit.

...

>> +static int
>> +ads1018_write_raw_unlocked(struct iio_dev *indio_dev,
>
> Similar to the naming discussion on the ACQUIRE RFC I'm not sure
> using locked here is really descriptive of more than an internal
> detail of how we prevent mode switching. I'd prefer something like
> ads1018_write_raw_direct_claimed() or ads1018_write_raw_direct_mode()
> (the absence of any other write_raw_*** would indicate this is the only
> valid one perhaps).
>
> Also this isn't the unlocked version, it's the one that doesn't take
> the lock.

I'll go with ads1018_{read,write}_raw_direct_mode().

>

Ack to everything else, including bindings stuff. Thanks, Jonathan!


-- 
 ~ Kurt

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[David Lechner]

On 12/7/25 10:02 AM, Kurt Borja wrote:
> On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:
>> On Thu, 04 Dec 2025 13:01:28 -0500
>> Kurt Borja <kuurtb@gmail.com> wrote:
>>
>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>>> analog-to-digital converters.
>>>
>>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
>>> this interrupt in devicetree is optional, therefore we only create an
>>> IIO trigger if one is found.
>>>
>>> Handling this interrupt requires some considerations. When enabling the
>>> trigger the CS line is tied low (active), thus we need to hold
>>> spi_bus_lock() too, to avoid state corruption. This is done inside the
>>> set_trigger_state() callback, to let users use other triggers without
>>> wasting a bus lock.
>>>
>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> 
> ...
> 
>>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
>>> +	.type = IIO_VOLTAGE,							\
>>> +	.channel = _chan,							\
>>> +	.scan_index = _index,							\
>>> +	.scan_type = {								\
>>> +		.sign = 's',							\
>>> +		.realbits = _realbits,						\
>>> +		.storagebits = 16,						\
>>> +		.shift = 16 - _realbits,					\
>>> +		.endianness = IIO_BE,						\
>>> +	},									\
>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
>>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
>>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\
>>
>> What motivates per channel sampling frequency?
>>
>> Given you have to write it each time you configure I guess it doesn't matter much
>> either way.
> 
> I guess making it shared by all is simpler too, so I'll go with that.
> 
Just keep in mind that if there is ever some use case we don't know
about that would require a different rate per channel, we can't change
it without breaking usespace. Once the decision is made, we are
locked in. Keeping it per-channel seems more future-proof to me.

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Jonathan Cameron]

On Sun, 7 Dec 2025 11:12:51 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 12/7/25 10:02 AM, Kurt Borja wrote:
> > On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:  
> >> On Thu, 04 Dec 2025 13:01:28 -0500
> >> Kurt Borja <kuurtb@gmail.com> wrote:
> >>  
> >>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> >>> analog-to-digital converters.
> >>>
> >>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
> >>> this interrupt in devicetree is optional, therefore we only create an
> >>> IIO trigger if one is found.
> >>>
> >>> Handling this interrupt requires some considerations. When enabling the
> >>> trigger the CS line is tied low (active), thus we need to hold
> >>> spi_bus_lock() too, to avoid state corruption. This is done inside the
> >>> set_trigger_state() callback, to let users use other triggers without
> >>> wasting a bus lock.
> >>>
> >>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>  
> > 
> > ...
> >   
> >>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
> >>> +	.type = IIO_VOLTAGE,							\
> >>> +	.channel = _chan,							\
> >>> +	.scan_index = _index,							\
> >>> +	.scan_type = {								\
> >>> +		.sign = 's',							\
> >>> +		.realbits = _realbits,						\
> >>> +		.storagebits = 16,						\
> >>> +		.shift = 16 - _realbits,					\
> >>> +		.endianness = IIO_BE,						\
> >>> +	},									\
> >>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> >>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
> >>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\  
> >>
> >> What motivates per channel sampling frequency?
> >>
> >> Given you have to write it each time you configure I guess it doesn't matter much
> >> either way.  
> > 
> > I guess making it shared by all is simpler too, so I'll go with that.
> >   
> Just keep in mind that if there is ever some use case we don't know
> about that would require a different rate per channel, we can't change
> it without breaking usespace. Once the decision is made, we are
> locked in. Keeping it per-channel seems more future-proof to me.

Only way I can think of that might cause that to matter would be
if the complex dance to avoid the onehot buffer restriction is added.
Given you gave this response I went looking and that might make
sense as an enhancement as the SPI protocol would allow a crafted message
sequence to do this efficiently.  Extension of figure 15 where first message
sets config and after that they read out channel and set config for next one.

Given that is sane, I agree with you that we should probably keep these separate.
I doubt anyone will use different sampling frequencies even if possible but you
never know.

Jonathan

> 

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Kurt Borja]

On Sun Dec 7, 2025 at 2:56 PM -05, Jonathan Cameron wrote:
> On Sun, 7 Dec 2025 11:12:51 -0600
> David Lechner <dlechner@baylibre.com> wrote:
>
>> On 12/7/25 10:02 AM, Kurt Borja wrote:
>> > On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:  
>> >> On Thu, 04 Dec 2025 13:01:28 -0500
>> >> Kurt Borja <kuurtb@gmail.com> wrote:
>> >>  
>> >>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>> >>> analog-to-digital converters.
>> >>>
>> >>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
>> >>> this interrupt in devicetree is optional, therefore we only create an
>> >>> IIO trigger if one is found.
>> >>>
>> >>> Handling this interrupt requires some considerations. When enabling the
>> >>> trigger the CS line is tied low (active), thus we need to hold
>> >>> spi_bus_lock() too, to avoid state corruption. This is done inside the
>> >>> set_trigger_state() callback, to let users use other triggers without
>> >>> wasting a bus lock.
>> >>>
>> >>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>  
>> > 
>> > ...
>> >   
>> >>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
>> >>> +	.type = IIO_VOLTAGE,							\
>> >>> +	.channel = _chan,							\
>> >>> +	.scan_index = _index,							\
>> >>> +	.scan_type = {								\
>> >>> +		.sign = 's',							\
>> >>> +		.realbits = _realbits,						\
>> >>> +		.storagebits = 16,						\
>> >>> +		.shift = 16 - _realbits,					\
>> >>> +		.endianness = IIO_BE,						\
>> >>> +	},									\
>> >>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
>> >>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
>> >>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\  
>> >>
>> >> What motivates per channel sampling frequency?
>> >>
>> >> Given you have to write it each time you configure I guess it doesn't matter much
>> >> either way.  
>> > 
>> > I guess making it shared by all is simpler too, so I'll go with that.
>> >   
>> Just keep in mind that if there is ever some use case we don't know
>> about that would require a different rate per channel, we can't change
>> it without breaking usespace. Once the decision is made, we are
>> locked in. Keeping it per-channel seems more future-proof to me.
>
> Only way I can think of that might cause that to matter would be
> if the complex dance to avoid the onehot buffer restriction is added.
> Given you gave this response I went looking and that might make
> sense as an enhancement as the SPI protocol would allow a crafted message
> sequence to do this efficiently.  Extension of figure 15 where first message
> sets config and after that they read out channel and set config for next one.

This is possible, yes. But would the timestamp even make sense in this
case? Even in the fastest sampling rate, we would have to wait at least
1 ms for each channel and the timestamp would become stale.

That was my reasoning for using the onehot restriction.

Is that acceptable? Or maybe we would need to disallow the timestamp
channel if more than one channel is selected?

>
> Given that is sane, I agree with you that we should probably keep these separate.
> I doubt anyone will use different sampling frequencies even if possible but you
> never know.

I'll leave it as is then.

>
> Jonathan
>
>> 


-- 
 ~ Kurt

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[David Lechner]

On 12/7/25 10:06 PM, Kurt Borja wrote:
> On Sun Dec 7, 2025 at 2:56 PM -05, Jonathan Cameron wrote:
>> On Sun, 7 Dec 2025 11:12:51 -0600
>> David Lechner <dlechner@baylibre.com> wrote:
>>
>>> On 12/7/25 10:02 AM, Kurt Borja wrote:
>>>> On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:  
>>>>> On Thu, 04 Dec 2025 13:01:28 -0500
>>>>> Kurt Borja <kuurtb@gmail.com> wrote:
>>>>>  
>>>>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>>>>>> analog-to-digital converters.
>>>>>>
>>>>>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
>>>>>> this interrupt in devicetree is optional, therefore we only create an
>>>>>> IIO trigger if one is found.
>>>>>>
>>>>>> Handling this interrupt requires some considerations. When enabling the
>>>>>> trigger the CS line is tied low (active), thus we need to hold
>>>>>> spi_bus_lock() too, to avoid state corruption. This is done inside the
>>>>>> set_trigger_state() callback, to let users use other triggers without
>>>>>> wasting a bus lock.
>>>>>>
>>>>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>  
>>>>
>>>> ...
>>>>   
>>>>>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
>>>>>> +	.type = IIO_VOLTAGE,							\
>>>>>> +	.channel = _chan,							\
>>>>>> +	.scan_index = _index,							\
>>>>>> +	.scan_type = {								\
>>>>>> +		.sign = 's',							\
>>>>>> +		.realbits = _realbits,						\
>>>>>> +		.storagebits = 16,						\
>>>>>> +		.shift = 16 - _realbits,					\
>>>>>> +		.endianness = IIO_BE,						\
>>>>>> +	},									\
>>>>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
>>>>>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
>>>>>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\  
>>>>>
>>>>> What motivates per channel sampling frequency?
>>>>>
>>>>> Given you have to write it each time you configure I guess it doesn't matter much
>>>>> either way.  
>>>>
>>>> I guess making it shared by all is simpler too, so I'll go with that.
>>>>   
>>> Just keep in mind that if there is ever some use case we don't know
>>> about that would require a different rate per channel, we can't change
>>> it without breaking usespace. Once the decision is made, we are
>>> locked in. Keeping it per-channel seems more future-proof to me.
>>
>> Only way I can think of that might cause that to matter would be
>> if the complex dance to avoid the onehot buffer restriction is added.
>> Given you gave this response I went looking and that might make
>> sense as an enhancement as the SPI protocol would allow a crafted message
>> sequence to do this efficiently.  Extension of figure 15 where first message
>> sets config and after that they read out channel and set config for next one.
> 
> This is possible, yes. But would the timestamp even make sense in this
> case? Even in the fastest sampling rate, we would have to wait at least
> 1 ms for each channel and the timestamp would become stale.
> 
> That was my reasoning for using the onehot restriction.
> 
> Is that acceptable? Or maybe we would need to disallow the timestamp
> channel if more than one channel is selected?

Yes. We have pretty much the same situation with timestamps on every
other ADC. The timestamp is usually when one full set of samples is
triggered. Not when the actual individual conversions are performed.

> 
>>
>> Given that is sane, I agree with you that we should probably keep these separate.
>> I doubt anyone will use different sampling frequencies even if possible but you
>> never know.
> 
> I'll leave it as is then.
> 
>>
>> Jonathan
>>
>>>
> 
> 

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Kurt Borja]

On Mon Dec 8, 2025 at 11:00 AM -05, David Lechner wrote:
> On 12/7/25 10:06 PM, Kurt Borja wrote:
>> On Sun Dec 7, 2025 at 2:56 PM -05, Jonathan Cameron wrote:
>>> On Sun, 7 Dec 2025 11:12:51 -0600
>>> David Lechner <dlechner@baylibre.com> wrote:
>>>
>>>> On 12/7/25 10:02 AM, Kurt Borja wrote:
>>>>> On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:  
>>>>>> On Thu, 04 Dec 2025 13:01:28 -0500
>>>>>> Kurt Borja <kuurtb@gmail.com> wrote:
>>>>>>  
>>>>>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
>>>>>>> analog-to-digital converters.
>>>>>>>
>>>>>>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
>>>>>>> this interrupt in devicetree is optional, therefore we only create an
>>>>>>> IIO trigger if one is found.
>>>>>>>
>>>>>>> Handling this interrupt requires some considerations. When enabling the
>>>>>>> trigger the CS line is tied low (active), thus we need to hold
>>>>>>> spi_bus_lock() too, to avoid state corruption. This is done inside the
>>>>>>> set_trigger_state() callback, to let users use other triggers without
>>>>>>> wasting a bus lock.
>>>>>>>
>>>>>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>  
>>>>>
>>>>> ...
>>>>>   
>>>>>>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
>>>>>>> +	.type = IIO_VOLTAGE,							\
>>>>>>> +	.channel = _chan,							\
>>>>>>> +	.scan_index = _index,							\
>>>>>>> +	.scan_type = {								\
>>>>>>> +		.sign = 's',							\
>>>>>>> +		.realbits = _realbits,						\
>>>>>>> +		.storagebits = 16,						\
>>>>>>> +		.shift = 16 - _realbits,					\
>>>>>>> +		.endianness = IIO_BE,						\
>>>>>>> +	},									\
>>>>>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
>>>>>>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
>>>>>>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\  
>>>>>>
>>>>>> What motivates per channel sampling frequency?
>>>>>>
>>>>>> Given you have to write it each time you configure I guess it doesn't matter much
>>>>>> either way.  
>>>>>
>>>>> I guess making it shared by all is simpler too, so I'll go with that.
>>>>>   
>>>> Just keep in mind that if there is ever some use case we don't know
>>>> about that would require a different rate per channel, we can't change
>>>> it without breaking usespace. Once the decision is made, we are
>>>> locked in. Keeping it per-channel seems more future-proof to me.
>>>
>>> Only way I can think of that might cause that to matter would be
>>> if the complex dance to avoid the onehot buffer restriction is added.
>>> Given you gave this response I went looking and that might make
>>> sense as an enhancement as the SPI protocol would allow a crafted message
>>> sequence to do this efficiently.  Extension of figure 15 where first message
>>> sets config and after that they read out channel and set config for next one.
>> 
>> This is possible, yes. But would the timestamp even make sense in this
>> case? Even in the fastest sampling rate, we would have to wait at least
>> 1 ms for each channel and the timestamp would become stale.
>> 
>> That was my reasoning for using the onehot restriction.
>> 
>> Is that acceptable? Or maybe we would need to disallow the timestamp
>> channel if more than one channel is selected?
>
> Yes. We have pretty much the same situation with timestamps on every
> other ADC. The timestamp is usually when one full set of samples is
> triggered. Not when the actual individual conversions are performed.

This is good to know for future patches or drivers. Thanks!


-- 
 ~ Kurt

Re: [PATCH v6 2/2] iio: adc: Add ti-ads1018 driver

[Jonathan Cameron]

On Tue, 09 Dec 2025 23:08:33 -0500
"Kurt Borja" <kuurtb@gmail.com> wrote:

> On Mon Dec 8, 2025 at 11:00 AM -05, David Lechner wrote:
> > On 12/7/25 10:06 PM, Kurt Borja wrote:  
> >> On Sun Dec 7, 2025 at 2:56 PM -05, Jonathan Cameron wrote:  
> >>> On Sun, 7 Dec 2025 11:12:51 -0600
> >>> David Lechner <dlechner@baylibre.com> wrote:
> >>>  
> >>>> On 12/7/25 10:02 AM, Kurt Borja wrote:  
> >>>>> On Sat Dec 6, 2025 at 3:07 PM -05, Jonathan Cameron wrote:    
> >>>>>> On Thu, 04 Dec 2025 13:01:28 -0500
> >>>>>> Kurt Borja <kuurtb@gmail.com> wrote:
> >>>>>>    
> >>>>>>> Add ti-ads1018 driver for Texas Instruments ADS1018 and ADS1118 SPI
> >>>>>>> analog-to-digital converters.
> >>>>>>>
> >>>>>>> These chips' MOSI pin is shared with a data-ready interrupt. Defining
> >>>>>>> this interrupt in devicetree is optional, therefore we only create an
> >>>>>>> IIO trigger if one is found.
> >>>>>>>
> >>>>>>> Handling this interrupt requires some considerations. When enabling the
> >>>>>>> trigger the CS line is tied low (active), thus we need to hold
> >>>>>>> spi_bus_lock() too, to avoid state corruption. This is done inside the
> >>>>>>> set_trigger_state() callback, to let users use other triggers without
> >>>>>>> wasting a bus lock.
> >>>>>>>
> >>>>>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>    
> >>>>>
> >>>>> ...
> >>>>>     
> >>>>>>> +#define ADS1018_VOLT_CHAN(_index, _chan, _realbits) {				\
> >>>>>>> +	.type = IIO_VOLTAGE,							\
> >>>>>>> +	.channel = _chan,							\
> >>>>>>> +	.scan_index = _index,							\
> >>>>>>> +	.scan_type = {								\
> >>>>>>> +		.sign = 's',							\
> >>>>>>> +		.realbits = _realbits,						\
> >>>>>>> +		.storagebits = 16,						\
> >>>>>>> +		.shift = 16 - _realbits,					\
> >>>>>>> +		.endianness = IIO_BE,						\
> >>>>>>> +	},									\
> >>>>>>> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |				\
> >>>>>>> +			      BIT(IIO_CHAN_INFO_SCALE) |			\
> >>>>>>> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),			\    
> >>>>>>
> >>>>>> What motivates per channel sampling frequency?
> >>>>>>
> >>>>>> Given you have to write it each time you configure I guess it doesn't matter much
> >>>>>> either way.    
> >>>>>
> >>>>> I guess making it shared by all is simpler too, so I'll go with that.
> >>>>>     
> >>>> Just keep in mind that if there is ever some use case we don't know
> >>>> about that would require a different rate per channel, we can't change
> >>>> it without breaking usespace. Once the decision is made, we are
> >>>> locked in. Keeping it per-channel seems more future-proof to me.  
> >>>
> >>> Only way I can think of that might cause that to matter would be
> >>> if the complex dance to avoid the onehot buffer restriction is added.
> >>> Given you gave this response I went looking and that might make
> >>> sense as an enhancement as the SPI protocol would allow a crafted message
> >>> sequence to do this efficiently.  Extension of figure 15 where first message
> >>> sets config and after that they read out channel and set config for next one.  
> >> 
> >> This is possible, yes. But would the timestamp even make sense in this
> >> case? Even in the fastest sampling rate, we would have to wait at least
> >> 1 ms for each channel and the timestamp would become stale.
> >> 
> >> That was my reasoning for using the onehot restriction.
> >> 
> >> Is that acceptable? Or maybe we would need to disallow the timestamp
> >> channel if more than one channel is selected?  
> >
> > Yes. We have pretty much the same situation with timestamps on every
> > other ADC. The timestamp is usually when one full set of samples is
> > triggered. Not when the actual individual conversions are performed.  
> 
> This is good to know for future patches or drivers. Thanks!
In theory we have the ABI to describe the relative timing, but meh, it's
rarely useful to do so. Hence we don't use it except in very odd corner cases.
See docs for in_voltageY_convdelay in Documentation/ABI/testing/sysfs-bus-iio

I've vaguely thought about suggesting we use that more widely but no one has
asked for it.  Use cases that I came across are things like 
complex filters across multiple channels (think of orientation tracking
if they were accelerometer axis).

So generally probably not a good idea to describe it at all. :)

Jonathan


> 
>