[PATCH v12 1/3] dt-bindings: iio: adc: add max14001

[PATCH v12 1/3] dt-bindings: iio: adc: add max14001

[Marilene Andrade Garcia]

Add device-tree documentation for MAX14001/MAX14002 ADCs.
The MAX14001/MAX14002 are isolated, single-channel analog-to-digital
converters with programmable voltage comparators and inrush current
control optimized for configurable binary input applications.

They share the same features, but in the MAX14001 the inrush trigger
threshold, current magnitude, and current duration are all programmable,
whereas in the MAX14002 these parameters are fixed.

Co-developed-by: Kim Seer Paller <kimseer.paller@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Signed-off-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
---

Hello maintainers,
Thank you for reviewing v11 and for your suggestions.
I believe I have addressed the requested code changes in this v12.

Notes:
- I have not removed the “|” in the interrupt descriptions because the
descriptions were changed as requested, and I think they now need to keep it.

Changes since v11:
- Added spi-lsb-first in the example.
- Improved the interrupt descriptions.

Best regards,
Marilene Andrade Garcia

 .../bindings/iio/adc/adi,max14001.yaml        | 89 +++++++++++++++++++
 MAINTAINERS                                   |  8 ++
 2 files changed, 97 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml

diff --git a/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
new file mode 100644
index 000000000000..7b3a72dd0b0d
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
@@ -0,0 +1,89 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2023-2025 Analog Devices Inc.
+# Copyright 2023 Kim Seer Paller
+# Copyright 2025 Marilene Andrade Garcia
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/adi,max14001.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX14001-MAX14002 ADC
+
+maintainers:
+  - Kim Seer Paller <kimseer.paller@analog.com>
+  - Marilene Andrade Garcia <marilene.agarcia@gmail.com>
+
+description: |
+    Single channel 10 bit ADC with SPI interface.
+    Datasheet can be found here
+      https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf
+
+$ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    oneOf:
+      - const: adi,max14002
+      - items:
+          - const: adi,max14001
+          - const: adi,max14002
+
+  reg:
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 5000000
+
+  vdd-supply:
+    description:
+      Isolated DC-DC power supply input voltage.
+
+  vddl-supply:
+    description:
+      Logic power supply.
+
+  refin-supply:
+    description:
+      ADC voltage reference supply.
+
+  interrupts:
+    minItems: 1
+    items:
+      - description: |
+          cout: comparator output signal that asserts high on the COUT pin
+          when ADC readings exceed the upper threshold and low when readings
+          fall below the lower threshold.
+      - description: |
+          fault: when fault reporting is enabled, the FAULT pin is asserted
+          low whenever one of the monitored fault conditions occurs.
+
+  interrupt-names:
+    minItems: 1
+    items:
+      - const: cout
+      - const: fault
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - vddl-supply
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      max14001: adc@0 {
+        compatible = "adi,max14001", "adi,max14002";
+        reg = <0>;
+        spi-max-frequency = <5000000>;
+        spi-lsb-first;
+        vdd-supply = <&vdd>;
+        vddl-supply = <&vddl>;
+      };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index f090c2f6e63a..1e1a30f77a0c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14922,6 +14922,14 @@ S:	Orphan
 F:	drivers/video/fbdev/matrox/matroxfb_*
 F:	include/uapi/linux/matroxfb.h
 
+MAX14001/MAX14002 IIO ADC DRIVER
+M:	Kim Seer Paller <kimseer.paller@analog.com>
+M:	Marilene Andrade Garcia <marilene.agarcia@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
+
 MAX15301 DRIVER
 M:	Daniel Nilsson <daniel.nilsson@flex.com>
 L:	linux-hwmon@vger.kernel.org

base-commit: b9700f87939f0f477e5c00db817f54ab8a97702b
-- 
2.34.1

[PATCH v12 2/3] iio: adc: max14001: New driver

[Marilene Andrade Garcia]

The MAX14001/MAX14002 is configurable, isolated 10-bit ADCs for multi-range
binary inputs. In addition to ADC readings, the MAX14001/MAX14002 offers
more features, like a binary comparator, a filtered reading that can
provide the average of the last 2, 4, or 8 ADC readings, and an inrush
comparator that triggers the inrush current. There is also a fault feature
that can diagnose seven possible fault conditions. And an option to select
an external or internal ADC voltage reference.

MAX14001/MAX14002 features implemented so far:
- Raw ADC reading.
- Filtered ADC average reading with the default configuration.
- MV fault disable.
- Selection of external or internal ADC voltage reference, depending on
whether it is declared in the device tree.

Co-developed-by: Kim Seer Paller <kimseer.paller@analog.com>
Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Signed-off-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
---

Hello maintainers,
Thank you for reviewing v11 and for your suggestions.
I believe I have addressed most of the requested code changes in this v12.
There were some discussions about a few of them, and I tried to follow the
path that you seemed to agree with.

I have one remaining question related to the max_register attribute of the
regmap. The register regions that can be accessed are 0x00–0x0c and
0x13–0x1a. As suggested, I used max_register to set the upper limit of the
register region that can be accessed (0x1a). Beyond this address, there is
a reserved region that should not be used (0x1b–0x1f). However, there is 
also a reserved region that should not be used between addresses 0x0d–0x12.
Should I use something to mark this region in the regmap?

Notes:
As suggested by Andy, I have chosen to use the code "if (ret == -ENODEV)" 
rather than "if (ret < 0)" on line 312, because it produces a slightly smaller
max14001.o file compared to the other approach (10640 bytes vs. 10648 bytes).
Additionally, as mentioned, it is more explicit check.

As suggested by David, I added support for SPI_LSB_FIRST, and I also used a
union to avoid clang compiler warnings related to casts between __le16,
__be16, and u16. Thank you for the code examples.

I tested it on the Raspberry Pi modified kernel version rpi-6.12 with
Raspberry Pi 5 hardware, using the MAX14001PMB evaluation board, and it
seems to work fine.

Main changes since v11:
- I think I fixed the alphabetical order in the files pointed.
- Fixed small issues in the include files.
- Removed the mutex since regmap has a lock mechanism (also removed the 
mutex include).
- Added support for SPI_LSB_FIRST in case it is used in a device tree file.

Best regards,
Marilene Andrade Garcia

 MAINTAINERS                |   1 +
 drivers/iio/adc/Kconfig    |  10 +
 drivers/iio/adc/Makefile   |   1 +
 drivers/iio/adc/max14001.c | 371 +++++++++++++++++++++++++++++++++++++
 4 files changed, 383 insertions(+)
 create mode 100644 drivers/iio/adc/max14001.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1e1a30f77a0c..da024038d6a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14929,6 +14929,7 @@ L:	linux-iio@vger.kernel.org
 S:	Maintained
 W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml
+F:	drivers/iio/adc/max14001.c
 
 MAX15301 DRIVER
 M:	Daniel Nilsson <daniel.nilsson@flex.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 58a14e6833f6..7c5dc06408a4 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1020,6 +1020,16 @@ config MAX1363
 	  To compile this driver as a module, choose M here: the module will be
 	  called max1363.
 
+config MAX14001
+	tristate "Analog Devices MAX14001/MAX14002 ADC driver"
+	depends on SPI
+	help
+	  Say yes here to build support for Analog Devices MAX14001/MAX14002
+	  Configurable, Isolated 10-bit ADCs for Multi-Range Binary Inputs.
+
+	  To compile this driver as a module, choose M here: the module will be
+	  called max14001.
+
 config MAX34408
 	tristate "Maxim max34408/max344089 ADC driver"
 	depends on I2C
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d008f78dc010..4241178c1098 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -89,6 +89,7 @@ obj-$(CONFIG_MAX11205) += max11205.o
 obj-$(CONFIG_MAX11410) += max11410.o
 obj-$(CONFIG_MAX1241) += max1241.o
 obj-$(CONFIG_MAX1363) += max1363.o
+obj-$(CONFIG_MAX14001) += max14001.o
 obj-$(CONFIG_MAX34408) += max34408.o
 obj-$(CONFIG_MAX77541_ADC) += max77541-adc.o
 obj-$(CONFIG_MAX9611) += max9611.o
diff --git a/drivers/iio/adc/max14001.c b/drivers/iio/adc/max14001.c
new file mode 100644
index 000000000000..52584c24fb08
--- /dev/null
+++ b/drivers/iio/adc/max14001.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+/*
+ * Analog Devices MAX14001/MAX14002 ADC driver
+ *
+ * Copyright (C) 2023-2025 Analog Devices Inc.
+ * Copyright (C) 2023 Kim Seer Paller <kimseer.paller@analog.com>
+ * Copyright (c) 2025 Marilene Andrade Garcia <marilene.agarcia@gmail.com>
+ *
+ * Datasheet: https://www.analog.com/media/en/technical-documentation/data-sheets/MAX14001-MAX14002.pdf
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitrev.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/spi/spi.h>
+#include <linux/types.h>
+#include <linux/units.h>
+#include <asm/byteorder.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+
+/* MAX14001 Registers Address */
+#define MAX14001_REG_ADC		0x00
+#define MAX14001_REG_FADC		0x01
+#define MAX14001_REG_FLAGS		0x02
+#define MAX14001_REG_FLTEN		0x03
+#define MAX14001_REG_THL		0x04
+#define MAX14001_REG_THU		0x05
+#define MAX14001_REG_INRR		0x06
+#define MAX14001_REG_INRT		0x07
+#define MAX14001_REG_INRP		0x08
+#define MAX14001_REG_CFG		0x09
+#define MAX14001_REG_ENBL		0x0A
+#define MAX14001_REG_ACT		0x0B
+#define MAX14001_REG_WEN		0x0C
+
+#define MAX14001_REG_VERIFICATION(x)	((x) + 0x10)
+
+#define MAX14001_REG_CFG_BIT_EXRF	BIT(5)
+
+#define MAX14001_REG_WEN_VALUE_WRITE	0x294
+
+#define MAX14001_MASK_ADDR		GENMASK(15, 11)
+#define MAX14001_MASK_WR		BIT(10)
+#define MAX14001_MASK_DATA		GENMASK(9, 0)
+
+struct max14001_state {
+	const struct max14001_chip_info *chip_info;
+	struct spi_device *spi;
+	struct regmap *regmap;
+	int vref_mV;
+	bool spi_hw_has_lsb_first;
+
+	/*
+	 * The following buffers will be bit-reversed during device
+	 * communication, because the device transmits and receives data
+	 * LSB-first.
+	 * DMA (thus cache coherency maintenance) requires the transfer
+	 * buffers to live in their own cache lines.
+	 */
+	union {
+		__be16 be;
+		__le16 le;
+	} spi_tx_buffer __aligned(IIO_DMA_MINALIGN);
+
+	union {
+		__be16 be;
+		__le16 le;
+	} spi_rx_buffer;
+};
+
+struct max14001_chip_info {
+	const char *name;
+};
+
+static int max14001_read(void *context, unsigned int reg, unsigned int *val)
+{
+	struct max14001_state *st = context;
+	struct spi_transfer xfers[] = {
+		{
+			.tx_buf = &st->spi_tx_buffer,
+			.len = sizeof(st->spi_tx_buffer),
+			.cs_change = 1,
+		}, {
+			.rx_buf = &st->spi_rx_buffer,
+			.len = sizeof(st->spi_rx_buffer),
+		},
+	};
+	int ret;
+	unsigned int addr, data;
+
+	/*
+	 * Prepare SPI transmit buffer 16 bit-value and reverse bit order
+	 * to align with the LSB-first input on SDI port in order to meet
+	 * the device communication requirements. If the controller supports
+	 * SPI_LSB_FIRST, this step will be handled by the SPI code.
+	 */
+	addr = FIELD_PREP(MAX14001_MASK_ADDR, reg);
+
+	if (st->spi_hw_has_lsb_first)
+		st->spi_tx_buffer.le = cpu_to_le16(addr);
+	else
+		st->spi_tx_buffer.be = cpu_to_be16(bitrev16(addr));
+
+	ret = spi_sync_transfer(st->spi, xfers, ARRAY_SIZE(xfers));
+	if (ret)
+		return ret;
+
+	/*
+	 * Convert received 16-bit value to cpu-endian format and reverse
+	 * bit order. If the controller supports SPI_LSB_FIRST, this step
+	 * will be handled by the SPI code.
+	 */
+	if (st->spi_hw_has_lsb_first)
+		data = le16_to_cpu(st->spi_rx_buffer.le);
+	else
+		data = bitrev16(be16_to_cpu(st->spi_rx_buffer.be));
+
+	*val = FIELD_GET(MAX14001_MASK_DATA, data);
+
+	return 0;
+}
+
+static int max14001_write(struct max14001_state *st, unsigned int reg, unsigned int val)
+{
+	unsigned int addr;
+
+	/*
+	 * Prepare SPI transmit buffer 16 bit-value and reverse bit order
+	 * to align with the LSB-first input on SDI port in order to meet
+	 * the device communication requirements. If the controller supports
+	 * SPI_LSB_FIRST, this step will be handled by the SPI code.
+	 */
+	addr = FIELD_PREP(MAX14001_MASK_ADDR, reg) |
+	       FIELD_PREP(MAX14001_MASK_WR, 1) |
+	       FIELD_PREP(MAX14001_MASK_DATA, val);
+
+	if (st->spi_hw_has_lsb_first)
+		st->spi_tx_buffer.le = cpu_to_le16(addr);
+	else
+		st->spi_tx_buffer.be = cpu_to_be16(bitrev16(addr));
+
+	return spi_write(st->spi, &st->spi_tx_buffer, sizeof(st->spi_tx_buffer));
+}
+
+static int max14001_write_single_reg(void *context, unsigned int reg, unsigned int val)
+{
+	struct max14001_state *st = context;
+	int ret;
+
+	/* Enable writing to the SPI register */
+	ret = max14001_write(st, MAX14001_REG_WEN, MAX14001_REG_WEN_VALUE_WRITE);
+	if (ret)
+		return ret;
+
+	/* Writing data into SPI register */
+	ret = max14001_write(st, reg, val);
+	if (ret)
+		return ret;
+
+	/* Disable writing to the SPI register */
+	return max14001_write(st, MAX14001_REG_WEN, 0);
+}
+
+static int max14001_write_verification_reg(struct max14001_state *st, unsigned int reg)
+{
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, reg, &val);
+	if (ret)
+		return ret;
+
+	return max14001_write(st, MAX14001_REG_VERIFICATION(reg), val);
+}
+
+static int max14001_disable_mv_fault(struct max14001_state *st)
+{
+	unsigned int reg;
+	int ret;
+
+	/* Enable writing to the SPI registers */
+	ret = max14001_write(st, MAX14001_REG_WEN, MAX14001_REG_WEN_VALUE_WRITE);
+	if (ret)
+		return ret;
+
+	/*
+	 * Reads all registers and writes the values to their appropriate
+	 * verification registers to clear the Memory Validation fault.
+	 */
+	for (reg = MAX14001_REG_FLTEN; reg <= MAX14001_REG_ENBL; reg++) {
+		ret = max14001_write_verification_reg(st, reg);
+		if (ret)
+			return ret;
+	}
+
+	/* Disable writing to the SPI registers */
+	return max14001_write(st, MAX14001_REG_WEN, 0);
+}
+
+static int max14001_debugfs_reg_access(struct iio_dev *indio_dev,
+				       unsigned int reg, unsigned int writeval,
+				       unsigned int *readval)
+{
+	struct max14001_state *st = iio_priv(indio_dev);
+
+	if (readval)
+		return regmap_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+static int max14001_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan,
+			     int *val, int *val2, long mask)
+{
+	struct max14001_state *st = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_AVERAGE_RAW:
+		ret = regmap_read(st->regmap, MAX14001_REG_FADC, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = st->vref_mV;
+		*val2 = 10;
+
+		return IIO_VAL_FRACTIONAL_LOG2;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct regmap_config max14001_regmap_config = {
+	.reg_read = max14001_read,
+	.reg_write = max14001_write_single_reg,
+	.max_register = MAX14001_REG_VERIFICATION(MAX14001_REG_ENBL),
+};
+
+static const struct iio_info max14001_info = {
+	.read_raw = max14001_read_raw,
+	.debugfs_reg_access = max14001_debugfs_reg_access,
+};
+
+static const struct iio_chan_spec max14001_channel[] = {
+	{
+		.type = IIO_VOLTAGE,
+		.indexed = 1,
+		.channel = 0,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_AVERAGE_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+static int max14001_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev;
+	struct max14001_state *st;
+	int ret = 0;
+	bool use_ext_vrefin = false;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+	st->spi = spi;
+	st->spi_hw_has_lsb_first = spi->mode & SPI_LSB_FIRST;
+	st->chip_info = spi_get_device_match_data(spi);
+
+	indio_dev->name = st->chip_info->name;
+	indio_dev->info = &max14001_info;
+	indio_dev->channels = max14001_channel;
+	indio_dev->num_channels = ARRAY_SIZE(max14001_channel);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+
+	st->regmap = devm_regmap_init(dev, NULL, st, &max14001_regmap_config);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap), "Failed to initialize regmap\n");
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable Vdd supply\n");
+
+	ret = devm_regulator_get_enable(dev, "vddl");
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to enable Vddl supply\n");
+
+	ret = devm_regulator_get_enable_read_voltage(dev, "refin");
+	if (ret < 0 && ret != -ENODEV)
+		return dev_err_probe(dev, ret, "Failed to get REFIN voltage\n");
+
+	if (ret == -ENODEV)
+		ret = 1250000;
+	else
+		use_ext_vrefin = true;
+	st->vref_mV = ret / (MICRO / MILLI);
+
+	if (use_ext_vrefin) {
+		/*
+		 * Configure the MAX14001/MAX14002 to use an external voltage
+		 * reference source by setting the bit 5 of the configuration register.
+		 */
+		ret = regmap_set_bits(st->regmap, MAX14001_REG_CFG,
+				      MAX14001_REG_CFG_BIT_EXRF);
+		if (ret)
+			return dev_err_probe(dev, ret,
+			       "Failed to set External REFIN in Configuration Register\n");
+	}
+
+	ret = max14001_disable_mv_fault(st);
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to disable MV Fault\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static struct max14001_chip_info max14001_chip_info = {
+	.name = "max14001",
+};
+
+static struct max14001_chip_info max14002_chip_info = {
+	.name = "max14002",
+};
+
+static const struct spi_device_id max14001_id_table[] = {
+	{ "max14001", (kernel_ulong_t)&max14001_chip_info },
+	{ "max14002", (kernel_ulong_t)&max14002_chip_info },
+	{ }
+};
+
+static const struct of_device_id max14001_of_match[] = {
+	{ .compatible = "adi,max14001", .data = &max14001_chip_info },
+	{ .compatible = "adi,max14002", .data = &max14002_chip_info },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max14001_of_match);
+
+static struct spi_driver max14001_driver = {
+	.driver = {
+		.name = "max14001",
+		.of_match_table = max14001_of_match,
+	},
+	.probe = max14001_probe,
+	.id_table = max14001_id_table,
+};
+module_spi_driver(max14001_driver);
+
+MODULE_AUTHOR("Kim Seer Paller <kimseer.paller@analog.com>");
+MODULE_AUTHOR("Marilene Andrade Garcia <marilene.agarcia@gmail.com>");
+MODULE_DESCRIPTION("Analog Devices MAX14001/MAX14002 ADCs driver");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[David Lechner]

On Mon, Sep 29, 2025 at 7:59 AM Marilene Andrade Garcia
<marilene.agarcia@gmail.com> wrote:
>

This is looking very nice. Just one question still about the averaging
feature and a few suggestions to really polish it up.

> I have one remaining question related to the max_register attribute of the
> regmap. The register regions that can be accessed are 0x00–0x0c and
> 0x13–0x1a. As suggested, I used max_register to set the upper limit of the
> register region that can be accessed (0x1a). Beyond this address, there is
> a reserved region that should not be used (0x1b–0x1f). However, there is
> also a reserved region that should not be used between addresses 0x0d–0x12.
> Should I use something to mark this region in the regmap?

There is struct regmap_access_table that can be used with .rd_table
and .wr_table in the regmap config.

> +static int max14001_read(void *context, unsigned int reg, unsigned int *val)
> +{
> +       struct max14001_state *st = context;
> +       struct spi_transfer xfers[] = {
> +               {
> +                       .tx_buf = &st->spi_tx_buffer,
> +                       .len = sizeof(st->spi_tx_buffer),
> +                       .cs_change = 1,
> +               }, {
> +                       .rx_buf = &st->spi_rx_buffer,
> +                       .len = sizeof(st->spi_rx_buffer),
> +               },
> +       };
> +       int ret;
> +       unsigned int addr, data;
> +
> +       /*
> +        * Prepare SPI transmit buffer 16 bit-value and reverse bit order
> +        * to align with the LSB-first input on SDI port in order to meet
> +        * the device communication requirements. If the controller supports
> +        * SPI_LSB_FIRST, this step will be handled by the SPI code.

s/code/controller/ or s/code/hardware/

> +        */
> +       addr = FIELD_PREP(MAX14001_MASK_ADDR, reg);
> +

...

> +static int max14001_write_single_reg(void *context, unsigned int reg, unsigned int val)
> +{
> +       struct max14001_state *st = context;
> +       int ret;
> +
> +       /* Enable writing to the SPI register */

Always nice to put `.` at the end of the sentence in comments.

> +       ret = max14001_write(st, MAX14001_REG_WEN, MAX14001_REG_WEN_VALUE_WRITE);
> +       if (ret)
> +               return ret;
> +

...

> +static int max14001_read_raw(struct iio_dev *indio_dev,
> +                            struct iio_chan_spec const *chan,
> +                            int *val, int *val2, long mask)
> +{
> +       struct max14001_state *st = iio_priv(indio_dev);
> +       int ret;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
> +               if (ret)
> +                       return ret;
> +
> +               return IIO_VAL_INT;
> +       case IIO_CHAN_INFO_AVERAGE_RAW:
> +               ret = regmap_read(st->regmap, MAX14001_REG_FADC, val);

I don't remember... did you give a reason why this should not be a
separate channel? Or just use REG_FADC as the raw value and forget
about REG_ADC? In any case we would want another attribute to control
the filter window size.

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

...

> +static int max14001_probe(struct spi_device *spi)
> +{
> +       struct device *dev = &spi->dev;
> +       struct iio_dev *indio_dev;
> +       struct max14001_state *st;
> +       int ret = 0;
> +       bool use_ext_vrefin = false;
> +
> +       indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       st = iio_priv(indio_dev);
> +       st->spi = spi;
> +       st->spi_hw_has_lsb_first = spi->mode & SPI_LSB_FIRST;
> +       st->chip_info = spi_get_device_match_data(spi);

To be extra-safe, we should check that this does not return NULL. I
think most drivers will return -EINVAL in that case.

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[Andy Shevchenko]

On Wed, Oct 1, 2025 at 5:03 PM David Lechner <dlechner@baylibre.com> wrote:
> On Mon, Sep 29, 2025 at 7:59 AM Marilene Andrade Garcia
> <marilene.agarcia@gmail.com> wrote:

...

> > +       /* Enable writing to the SPI register */
>
> Always nice to put `.` at the end of the sentence in comments.

FWIW, for one-line comments we allow different styles, the main
requirement is to be consistent with chosen style across the driver in
question.

1) "bla bla bla"
2) "Foo bar baz"
3) "Happy comment."

Each of all three is okay.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[Marilene Andrade Garcia]

On 01/10/2025 11:03, David Lechner wrote:
> On Mon, Sep 29, 2025 at 7:59 AM Marilene Andrade Garcia
> <marilene.agarcia@gmail.com> wrote:
>>
> 
...
>> +static int max14001_read_raw(struct iio_dev *indio_dev,
>> +                            struct iio_chan_spec const *chan,
>> +                            int *val, int *val2, long mask)
>> +{
>> +       struct max14001_state *st = iio_priv(indio_dev);
>> +       int ret;
>> +
>> +       switch (mask) {
>> +       case IIO_CHAN_INFO_RAW:
>> +               ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
>> +               if (ret)
>> +                       return ret;
>> +
>> +               return IIO_VAL_INT;
>> +       case IIO_CHAN_INFO_AVERAGE_RAW:
>> +               ret = regmap_read(st->regmap, MAX14001_REG_FADC, val);
> 
> I don't remember... did you give a reason why this should not be a
> separate channel? Or just use REG_FADC as the raw value and forget
> about REG_ADC? In any case we would want another attribute to control
> the filter window size.
...

Hello David,

Thank you for the review and suggestions.
Sorry for not adding any comments about that in v12. From what I 
understood from our previous conversation, for now the code could have 
one channel to keep things simple, since we’re not sure if anyone will 
actually need to read both the filtered and unfiltered data at the same 
time.

I was thinking of sending a separate set of commits to address that 
after this one gets merged, as it will involve new code changes related 
to adding a function to configure how many ADC readings are included in 
the mean calculation, and adding a new attribute to sysfs.

Since both IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_AVERAGE_RAW are currently 
returning the same value, I could drop IIO_CHAN_INFO_AVERAGE_RAW in v13 
and add it back in the next series of commits to implement the related 
feature.

I would like to know your thoughts about it, because if you prefer, I 
could change my plans and implement it in v13.

Best Regards,
Marilene

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[David Lechner]

On 10/5/25 6:25 PM, Marilene Andrade Garcia wrote:
> On 01/10/2025 11:03, David Lechner wrote:
>> On Mon, Sep 29, 2025 at 7:59 AM Marilene Andrade Garcia
>> <marilene.agarcia@gmail.com> wrote:
>>>
>>
> ...
>>> +static int max14001_read_raw(struct iio_dev *indio_dev,
>>> +                            struct iio_chan_spec const *chan,
>>> +                            int *val, int *val2, long mask)
>>> +{
>>> +       struct max14001_state *st = iio_priv(indio_dev);
>>> +       int ret;
>>> +
>>> +       switch (mask) {
>>> +       case IIO_CHAN_INFO_RAW:
>>> +               ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
>>> +               if (ret)
>>> +                       return ret;
>>> +
>>> +               return IIO_VAL_INT;
>>> +       case IIO_CHAN_INFO_AVERAGE_RAW:
>>> +               ret = regmap_read(st->regmap, MAX14001_REG_FADC, val);
>>
>> I don't remember... did you give a reason why this should not be a
>> separate channel? Or just use REG_FADC as the raw value and forget
>> about REG_ADC? In any case we would want another attribute to control
>> the filter window size.
> ...
> 
> Hello David,
> 
> Thank you for the review and suggestions.
> Sorry for not adding any comments about that in v12. From what I understood from our previous conversation, for now the code could have one channel to keep things simple, since we’re not sure if anyone will actually need to read both the filtered and unfiltered data at the same time.
> 
> I was thinking of sending a separate set of commits to address that after this one gets merged, as it will involve new code changes related to adding a function to configure how many ADC readings are included in the mean calculation, and adding a new attribute to sysfs.
> 
> Since both IIO_CHAN_INFO_RAW and IIO_CHAN_INFO_AVERAGE_RAW are currently returning the same value, I could drop IIO_CHAN_INFO_AVERAGE_RAW in v13 and add it back in the next series of commits to implement the related feature.
> 
> I would like to know your thoughts about it, because if you prefer, I could change my plans and implement it in v13.
> 
> Best Regards,
> Marilene
> 

Since everything else looks good, I think Marcelo's suggestion to drop
averaging from this series and add it in a later series is the best advice.

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[Jonathan Cameron]

On Mon, 29 Sep 2025 02:59:37 -0300
Marilene Andrade Garcia <marilene.agarcia@gmail.com> wrote:

> The MAX14001/MAX14002 is configurable, isolated 10-bit ADCs for multi-range
> binary inputs. In addition to ADC readings, the MAX14001/MAX14002 offers
> more features, like a binary comparator, a filtered reading that can
> provide the average of the last 2, 4, or 8 ADC readings, and an inrush
> comparator that triggers the inrush current. There is also a fault feature
> that can diagnose seven possible fault conditions. And an option to select
> an external or internal ADC voltage reference.
> 
> MAX14001/MAX14002 features implemented so far:
> - Raw ADC reading.
> - Filtered ADC average reading with the default configuration.
> - MV fault disable.
> - Selection of external or internal ADC voltage reference, depending on
> whether it is declared in the device tree.
> 
> Co-developed-by: Kim Seer Paller <kimseer.paller@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> Signed-off-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
> ---
> 
> Hello maintainers,
> Thank you for reviewing v11 and for your suggestions.
> I believe I have addressed most of the requested code changes in this v12.
> There were some discussions about a few of them, and I tried to follow the
> path that you seemed to agree with.
> 
> I have one remaining question related to the max_register attribute of the
> regmap. The register regions that can be accessed are 0x00–0x0c and
> 0x13–0x1a. As suggested, I used max_register to set the upper limit of the
> register region that can be accessed (0x1a). Beyond this address, there is
> a reserved region that should not be used (0x1b–0x1f). However, there is 
> also a reserved region that should not be used between addresses 0x0d–0x12.
> Should I use something to mark this region in the regmap?
regmap allows specification of which registers readable and which writeable.
If this is a concern then you could do that.  I'd not worry too much though
as those regions are only accessed by the debugfs interface and that provides
many other foot guns!

Just one trivial comment to add to David's more detailed review and questions.

> 
> Notes:
> As suggested by Andy, I have chosen to use the code "if (ret == -ENODEV)" 
> rather than "if (ret < 0)" on line 312, because it produces a slightly smaller
> max14001.o file compared to the other approach (10640 bytes vs. 10648 bytes).
> Additionally, as mentioned, it is more explicit check.
> 
> As suggested by David, I added support for SPI_LSB_FIRST, and I also used a
> union to avoid clang compiler warnings related to casts between __le16,
> __be16, and u16. Thank you for the code examples.
> 
> I tested it on the Raspberry Pi modified kernel version rpi-6.12 with
> Raspberry Pi 5 hardware, using the MAX14001PMB evaluation board, and it
> seems to work fine.
> 
> Main changes since v11:
> - I think I fixed the alphabetical order in the files pointed.
> - Fixed small issues in the include files.
> - Removed the mutex since regmap has a lock mechanism (also removed the 
> mutex include).
> - Added support for SPI_LSB_FIRST in case it is used in a device tree file.


> diff --git a/drivers/iio/adc/max14001.c b/drivers/iio/adc/max14001.c
> new file mode 100644
> index 000000000000..52584c24fb08
> --- /dev/null
> +++ b/drivers/iio/adc/max14001.c

...


> +static int max14001_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev;
> +	struct max14001_state *st;
> +	int ret = 0;

Set before use I think in all paths below. So can drop init here.

> +	bool use_ext_vrefin = false;

Re: [PATCH v12 2/3] iio: adc: max14001: New driver

[Marcelo Schmitt]

Hi Marilene,

...
> I have one remaining question related to the max_register attribute of the
> regmap. The register regions that can be accessed are 0x00–0x0c and
> 0x13–0x1a. As suggested, I used max_register to set the upper limit of the
> register region that can be accessed (0x1a). Beyond this address, there is
> a reserved region that should not be used (0x1b–0x1f). However, there is 
> also a reserved region that should not be used between addresses 0x0d–0x12.
> Should I use something to mark this region in the regmap?
> 

Yes, use rd_table and wr_table as suggested by David.
If you haven't implemented it already, I think the reg ranges would look like

static const struct regmap_range max14001_regmap_rd_range[] = {
	regmap_reg_range(MAX14001_REG_ADC, MAX14001_REG_ENBL),
	regmap_reg_range(MAX14001_REG_WEN, MAX14001_REG_WEN),
	regmap_reg_range(MAX14001_REG_VERIFICATION(MAX14001_REG_FLTEN),
			 MAX14001_REG_VERIFICATION(MAX14001_REG_ENBL)),
};

static const struct regmap_range max14001_regmap_wr_range[] = {
	regmap_reg_range(MAX14001_REG_FLTEN, MAX14001_REG_WEN),
	regmap_reg_range(MAX14001_REG_VERIFICATION(MAX14001_REG_FLTEN),
			 MAX14001_REG_VERIFICATION(MAX14001_REG_ENBL)),
};

see drivers/iio/adc/ad4030.c for an example.

> 
> I tested it on the Raspberry Pi modified kernel version rpi-6.12 with
> Raspberry Pi 5 hardware, using the MAX14001PMB evaluation board, and it
> seems to work fine.
> 

I finally managed to give this driver a try.

max14001_disable_mv_fault() does make the memory fault flag (MV) clear out.
Both register read and register write work fine through debugfs.
Though, even after I plugged a bench top power supply to the ADC inputs, I read
0x80 from the FLAGS (0x02) register. That flag indicates "input voltage detected
without input current" so maybe I missed or miss-connected some inputs.

The _scale looks good. Eval board data sheet says each ADC input is DC offset by
VREF/2 (1.25 / 2 == 0.625 Volts), and I do get readings that float between
0.623779 and 0.629883 volts. We know that the PMB board also significantly
attenuates the signal. At 10V input, I start getting data that scales to
0.631104 volts (_raw == 517). And it looks the same if I go to 20V. Interesting
thing is, the readings peak only after I disable the power supply output. One of
the eval board user guides [1] has an example connection diagram to an AC
voltage/current circuit, but no example connecting to a DC circuit. So, maybe
I'm just being silly trying to test this with a DC supply. Though, despite the
outcome of my rudimentary tests [2], the driver provides a proper _scale and
grabs sample data from the correct register.

[1]: https://www.analog.com/media/en/reference-design-documentation/design-notes/ds25-isolated-adc-with-integrated-dc-dc-converter-simplifies-field-side-circuitry.pdf
[2]: https://wiki.analog.com/resources/tools-software/linux-drivers/iio-adc/max14001

Thus,
Tested-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>


About the data averaging, my suggestion is to remove _AVERAGE_RAW stuff from the
initial driver support patch and (optionally) implement it on a follow up patch.
It will make it easier, faster, and safer to get this driver merged without the
averaging thing. max14001 averaging feature is uncommon. Even though we have
in_Y_mean_raw documented in IIO ABI, adding more interfaces is a commitment.
Once it's there and people start using it, we cannot change. 'We don't break
user space'. So, the Linux kernel community may want to think about it
thoroughly before committing to new ABI. The max14001 driver don't need to be on
hold until a decision about max14001 averaging interface is made.

> +static int max14001_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct max14001_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = regmap_read(st->regmap, MAX14001_REG_ADC, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_AVERAGE_RAW:
Drop/remove IIO_CHAN_INFO_AVERAGE_RAW from the initial driver patch.
Optionally, add IIO_CHAN_INFO_AVERAGE_RAW on a follow up patch.

> +		ret = regmap_read(st->regmap, MAX14001_REG_FADC, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
...
> +static const struct iio_chan_spec max14001_channel[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.indexed = 1,
> +		.channel = 0,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_AVERAGE_RAW) |
Same reasoning here. Can be introduced in a separate patch.

> +				      BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};

With the averaging related stuff dropped from the initial driver support,
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>

btw, well done.

Re: [PATCH v12 1/3] dt-bindings: iio: adc: add max14001

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 544 bytes --]

On Mon, Sep 29, 2025 at 02:58:38AM -0300, Marilene Andrade Garcia wrote:

>  .../bindings/iio/adc/adi,max14001.yaml        | 89 +++++++++++++++++++
>  MAINTAINERS                                   |  8 ++
>  2 files changed, 97 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,max14001.yaml

FWIW, odd to call the file 14001 when 14002 is the compatible that is
permitted standalone.

> +      max14001: adc@0 {

Drop the label, it's not used.
With those,
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v12 1/3] dt-bindings: iio: adc: add max14001

[Marcelo Schmitt]

On 09/29, Marilene Andrade Garcia wrote:
> Add device-tree documentation for MAX14001/MAX14002 ADCs.
> The MAX14001/MAX14002 are isolated, single-channel analog-to-digital
> converters with programmable voltage comparators and inrush current
> control optimized for configurable binary input applications.
> 
> They share the same features, but in the MAX14001 the inrush trigger
> threshold, current magnitude, and current duration are all programmable,
> whereas in the MAX14002 these parameters are fixed.
> 
> Co-developed-by: Kim Seer Paller <kimseer.paller@analog.com>
> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
> Signed-off-by: Marilene Andrade Garcia <marilene.agarcia@gmail.com>
> ---
> 
No strong opinion about the dt doc file name.
With Conor's suggesting of dropping the label applied,
Reviewed-by: Marcelo Schmitt <marcelo.schmitt1@gmail.com>