[PATCH 0/2] iio: temperature: add ADI MAX30210 SPI temperature sensor

[PATCH 0/2] iio: temperature: add ADI MAX30210 SPI temperature sensor

[John Erasmus Mari Geronimo]

This series adds support for the Analog Devices MAX30210
SPI temperature sensor to the IIO subsystem.

The MAX30210 is a high-accuracy digital temperature sensor
with SPI interface intended for medical and wearable
applications.

Patch 1 adds the device tree binding documentation.
Patch 2 adds the SPI driver implementation.

John Erasmus Mari Geronimo (2):
  dt-bindings: iio: temperature: add ADI MAX30210
  iio: temperature: add ADI MAX30210 driver

 .../iio/temperature/adi,max30210.yaml         |  71 ++
 MAINTAINERS                                   |   8 +
 drivers/iio/temperature/Kconfig               |  10 +
 drivers/iio/temperature/Makefile              |   1 +
 drivers/iio/temperature/max30210.c            | 758 ++++++++++++++++++
 5 files changed, 848 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
 create mode 100644 drivers/iio/temperature/max30210.c

-- 
2.34.1

[PATCH 1/2] dt-bindings: iio: temperature: add ADI MAX30210

[John Erasmus Mari Geronimo]

Add device tree binding documentation for the Analog Devices
MAX30210 temperature sensor.

Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
---
 .../iio/temperature/adi,max30210.yaml         | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml

diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
new file mode 100644
index 000000000000..80aeae23e0a5
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2026 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/adi,max30210.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices MAX30210 Low-Power I2C Digital Temperature Sensor
+
+maintainers:
+  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
+
+description: |
+  The MAX30210 operates from 1.7V to 2.0V supply voltage, and is a low-power,
+  high-accuracy digital temperature sensor with ±0.1°C accuracy from +20°C to
+  +50°C and ±0.15°C accuracy from -20°C to +85°C.
+  https://www.analog.com/media/en/technical-documentation/data-sheets/max30210.pdf
+
+properties:
+  compatible:
+    enum:
+      - adi,max30210
+
+  reg:
+    maxItems: 1
+
+  vdd-supply:
+    description: |
+      Analog Supply Voltage Input. Must have values in the interval (1.7V; 5.5V)
+      in order for the device to function correctly.
+
+  powerdown-gpios:
+    description: |
+      GPIO spec for CVT/PDB pin. Should be configured with GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  interrupts:
+    description: |
+      Connected to INT pin. Should be configured with type IRQ_TYPE_EDGE_BOTH.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+  - powerdown-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/pwm/pwm.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        status = "okay";
+
+        temperature-sensor@40 {
+            compatible = "adi,max30210";
+            reg = <0x40>;
+            vdd-supply = <&vdd>;
+            powerdown-gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
+
+            interrupt-parent = <&gpio>;
+            interrupts = <17 IRQ_TYPE_EDGE_BOTH>;
+        };
+    };
+...
-- 
2.34.1

[PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[John Erasmus Mari Geronimo]

MAX30210 ±0.1°C Accurate Ultra-Small Low-Power Digital Temperature Sensor

Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
---
 MAINTAINERS                        |   8 +
 drivers/iio/temperature/Kconfig    |  10 +
 drivers/iio/temperature/Makefile   |   1 +
 drivers/iio/temperature/max30210.c | 758 +++++++++++++++++++++++++++++
 4 files changed, 777 insertions(+)
 create mode 100644 drivers/iio/temperature/max30210.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1c75276404df..2abdbcf3a8e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1638,6 +1638,14 @@ W:	https://ez.analog.com/linux-software-drivers
 F:	Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
 F:	drivers/iio/dac/max22007.c
 
+ANALOG DEVICES INC MAX30210 DRIVER
+M:	John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
+L:	linux-iio@vger.kernel.org
+S:	Supported
+W:	https://ez.analog.com/linux-software-drivers
+F:	Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
+F:	drivers/iio/temperature/max30210.c
+
 ANALOG DEVICES INC ADA4250 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
 L:	linux-iio@vger.kernel.org
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 9328b2250ace..2d7cb50e2538 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -184,4 +184,14 @@ config MCP9600
 	  This driver can also be built as a module. If so, the module
 	  will be called mcp9600.
 
+config MAX30210
+	tristate "MAX30210 Low-Power I2C Digital Temperature Sensor"
+	depends on I2C
+	help
+	  If you say yes here you get support for MAX30210 low-power digital
+	  temperature sensor chip connected via I2C.
+
+	  This driver can also be build as a module. If so, the module
+	  will be called max30210.
+
 endmenu
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7..e5aad14dc09b 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_LTC2983) += ltc2983.o
 obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
 obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
 obj-$(CONFIG_MAX30208) += max30208.o
+obj-$(CONFIG_MAX30210) += max30210.o
 obj-$(CONFIG_MAX31856) += max31856.o
 obj-$(CONFIG_MAX31865) += max31865.o
 obj-$(CONFIG_MCP9600) += mcp9600.o
diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
new file mode 100644
index 000000000000..aaa3a26be131
--- /dev/null
+++ b/drivers/iio/temperature/max30210.c
@@ -0,0 +1,758 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Analog Devices MAX30210 I2C Temperature Sensor driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#include <asm/div64.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/debugfs.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/events.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/interrupt.h>
+#include <linux/log2.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/property.h>
+#include <linux/pwm.h>
+#include <linux/regmap.h>
+#include <linux/stat.h>
+#include <linux/string.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#define MAX30210_STATUS_REG		0x00
+#define MAX30210_INT_EN_REG		0x02
+#define MAX30210_FIFO_DATA_REG		0x08
+#define MAX30210_FIFO_CONF_1_REG	0x09
+#define MAX30210_FIFO_CONF_2_REG	0x0A
+#define MAX30210_SYS_CONF_REG		0x11
+#define MAX30210_PIN_CONF_REG		0x12
+#define MAX30210_TEMP_ALM_HI_REG	0x22
+#define MAX30210_TEMP_ALM_LO_REG	0x24
+#define MAX30210_TEMP_INC_THRESH_REG	0x26
+#define MAX30210_TEMP_DEC_THRESH_REG	0x27
+#define MAX30210_TEMP_CONF_1_REG	0x28
+#define MAX30210_TEMP_CONF_2_REG	0x29
+#define MAX30210_TEMP_CONV_REG		0x2A
+#define MAX30210_TEMP_DATA_REG		0x2B
+#define MAX30210_TEMP_SLOPE_REG		0x2D
+#define MAX30210_UNIQUE_ID_REG		0x30
+#define MAX30210_PART_ID_REG		0xFF
+
+#define MAX30210_A_FULL_MASK   BIT(7)
+#define MAX30210_TEMP_RDY_MASK BIT(6)
+#define MAX30210_TEMP_DEC_MASK BIT(5)
+#define MAX30210_TEMP_INC_MASK BIT(4)
+#define MAX30210_TEMP_LO_MASK  BIT(3)
+#define MAX30210_TEMP_HI_MASK  BIT(2)
+#define MAX30210_PWR_RDY_MASK  BIT(0)
+
+#define MAX30210_FLUSH_FIFO_MASK BIT(4)
+
+#define MAX30210_EXT_CNV_EN_MASK	BIT(7)
+#define MAX30210_EXT_CVT_ICFG_MASK	BIT(6)
+#define MAX30210_INT_FCFG_MASK		GENMASK(3, 2)
+#define MAX30210_INT_OCFG_MASK		GENMASK(1, 0)
+
+#define MAX30210_CHG_DET_EN_MASK	BIT(3)
+#define MAX30210_RATE_CHG_FILTER_MASK	GENMASK(2, 0)
+
+#define MAX30210_TEMP_PERIOD_MASK	GENMASK(3, 0)
+#define MAX30210_ALERT_MODE_MASK	BIT(7)
+
+#define MAX30210_AUTO_MASK	BIT(1)
+#define MAX30210_CONV_T_MASK	BIT(0)
+
+#define MAX30210_PART_ID		0x45
+#define MAX30210_FIFO_SIZE		64
+#define MAX30210_FIFO_INVAL_DATA	GENMASK(23, 0)
+#define MAX30210_WATERMARK_DEFAULT	(0x40 - 0x1F)
+
+#define MAX30210_INT_EN(state, mask)	((state) ? (mask) : 0x0)
+
+#define MAX30210_UNIQUE_ID_LEN		6
+#define MAX30210_EXT_CVT_FREQ_MIN	1
+#define MAX30210_EXT_CVT_FREQ_MAX	20
+
+struct max30210_state {
+	/*
+	 * Prevent simultaneous access to the i2c client.
+	 */
+	struct mutex lock;
+	struct regmap *regmap;
+	struct iio_trigger *trig;
+	struct gpio_desc *powerdown_gpio;
+	u8 watermark;
+	u8 data[3 * MAX30210_FIFO_SIZE]  __aligned(IIO_DMA_MINALIGN);
+};
+
+static const int samp_freq_avail[] = {
+	0, 15625,
+	0, 31250,
+	0, 62500,
+	0, 125000,
+	0, 250000,
+	0, 500000,
+	1, 0,
+	2, 0,
+	4, 0,
+	8, 0
+};
+
+static const struct regmap_config max30210_regmap = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MAX30210_PART_ID_REG,
+};
+
+static int max30210_read_temp(struct regmap *regmap, unsigned int reg,
+			      int *temp)
+{
+	u8 uval[2] __aligned(IIO_DMA_MINALIGN);
+	int ret;
+
+	ret = regmap_bulk_read(regmap, reg, uval, 2);
+	if (ret)
+		return ret;
+
+	*temp = sign_extend32(get_unaligned_be16(uval), 15);
+
+	return IIO_VAL_INT;
+}
+
+static int max30210_write_temp(struct regmap *regmap, unsigned int reg,
+			       int temp)
+{
+	u8 uval[2] __aligned(IIO_DMA_MINALIGN);
+
+	put_unaligned_be16(temp, uval);
+
+	return regmap_bulk_write(regmap, reg, uval, 2);
+}
+
+static void max30210_fifo_read(struct iio_dev *indio_dev)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	u32 samp;
+	int ret, i, j;
+
+	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
+			       st->data, 3 * st->watermark);
+	if (ret < 0)
+		return dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
+
+	for (i = 0; i < st->watermark; i++) {
+		samp = 0;
+
+		for (j = 0; j < 3; j++) {
+			samp <<= 8;
+			samp |= st->data[3 * i + j];
+		}
+
+		if (samp == MAX30210_FIFO_INVAL_DATA) {
+			dev_err(&indio_dev->dev, "Invalid data\n");
+			continue;
+		}
+
+		iio_push_to_buffers(indio_dev, &samp);
+	}
+}
+
+static irqreturn_t max30210_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int status;
+	int ret;
+
+	guard(mutex)(&st->lock);
+
+	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
+	if (ret) {
+		dev_err(&indio_dev->dev, "Status byte read error\n");
+		goto exit_irq;
+	}
+
+	if (status & MAX30210_PWR_RDY_MASK) {
+		dev_info(&indio_dev->dev, "power-on\n");
+		st->watermark = MAX30210_WATERMARK_DEFAULT;
+	}
+
+	if (status & MAX30210_A_FULL_MASK)
+		max30210_fifo_read(indio_dev);
+
+	if (status & MAX30210_TEMP_HI_MASK)
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_RISING),
+			       iio_get_time_ns(indio_dev));
+
+	if (status & MAX30210_TEMP_LO_MASK)
+		iio_push_event(indio_dev,
+			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
+						    IIO_EV_TYPE_THRESH,
+						    IIO_EV_DIR_FALLING),
+			       iio_get_time_ns(indio_dev));
+
+exit_irq:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static int max30210_reg_access(struct iio_dev *indio_dev, unsigned int reg,
+			       unsigned int writeval, unsigned int *readval)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+
+	if (!readval)
+		return regmap_write(st->regmap, reg, writeval);
+
+	return regmap_read(st->regmap, reg, readval);
+}
+
+static int max30210_validate_trigger(struct iio_dev *indio_dev,
+				     struct iio_trigger *trig)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+
+	if (st->trig != trig)
+		return -EINVAL;
+
+	return 0;
+}
+
+static int max30210_read_event(struct iio_dev *indio_dev,
+			       const struct iio_chan_spec *chan,
+			       enum iio_event_type type,
+			       enum iio_event_direction dir,
+			       enum iio_event_info info, int *val, int *val2)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+
+	if (info == IIO_EV_INFO_VALUE) {
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			switch (type) {
+			case IIO_EV_TYPE_THRESH:
+				return max30210_read_temp(st->regmap,
+							  MAX30210_TEMP_ALM_HI_REG, val);
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_FALLING:
+			switch (type) {
+			case IIO_EV_TYPE_THRESH:
+				return max30210_read_temp(st->regmap,
+							  MAX30210_TEMP_ALM_LO_REG, val);
+			default:
+				return -EINVAL;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int max30210_write_event(struct iio_dev *indio_dev,
+				const struct iio_chan_spec *chan,
+				enum iio_event_type type,
+				enum iio_event_direction dir,
+				enum iio_event_info info, int val, int val2)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+
+	if (info == IIO_EV_INFO_VALUE) {
+		switch (dir) {
+		case IIO_EV_DIR_RISING:
+			switch (type) {
+			case IIO_EV_TYPE_THRESH:
+				return max30210_write_temp(st->regmap,
+							   MAX30210_TEMP_ALM_HI_REG, val);
+			default:
+				return -EINVAL;
+			}
+			break;
+		case IIO_EV_DIR_FALLING:
+			switch (type) {
+			case IIO_EV_TYPE_THRESH:
+				return max30210_write_temp(st->regmap,
+							   MAX30210_TEMP_ALM_LO_REG, val);
+			default:
+				return -EINVAL;
+			}
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+
+	return -EINVAL;
+}
+
+static int max30210_read_event_config(struct iio_dev *indio_dev,
+				      const struct iio_chan_spec *chan,
+				      enum iio_event_type type,
+				      enum iio_event_direction dir)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(st->regmap, MAX30210_INT_EN_REG, &val);
+	if (ret)
+		return ret;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			return FIELD_GET(MAX30210_TEMP_HI_MASK, val);
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_DIR_FALLING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			return FIELD_GET(MAX30210_TEMP_LO_MASK, val);
+		default:
+			return -EINVAL;
+		}
+		break;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max30210_write_event_config(struct iio_dev *indio_dev,
+				       const struct iio_chan_spec *chan,
+				       enum iio_event_type type,
+				       enum iio_event_direction dir, int state)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int val;
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			val = MAX30210_INT_EN(state, MAX30210_TEMP_HI_MASK);
+
+			return regmap_update_bits(st->regmap,
+						  MAX30210_INT_EN_REG,
+						  MAX30210_TEMP_HI_MASK, val);
+		default:
+			return -EINVAL;
+		}
+		break;
+	case IIO_EV_DIR_FALLING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			val = MAX30210_INT_EN(state, MAX30210_TEMP_LO_MASK);
+
+			return regmap_update_bits(st->regmap,
+						  MAX30210_INT_EN_REG,
+						  MAX30210_TEMP_LO_MASK, val);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max30210_read_raw(struct iio_dev *indio_dev,
+			     struct iio_chan_spec const *chan, int *val,
+			     int *val2, long mask)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int uval;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		*val = 5;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
+		if (ret)
+			return ret;
+
+		uval = FIELD_GET(MAX30210_TEMP_PERIOD_MASK, uval);
+
+		*val = 8;
+
+		/**
+		 * register values 0x9 or above have the same sample
+		 * rate of 8Hz
+		 */
+		*val2 = uval >= 0x9 ? 1 : BIT(0x9 - uval);
+
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_RAW:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
+				   MAX30210_CONV_T_MASK);
+		if (ret)
+			goto release_dmode;
+
+		fsleep(8000);
+
+		ret = max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
+
+release_dmode:
+		iio_device_release_direct_mode(indio_dev);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max30210_read_avail(struct iio_dev *indio_dev,
+			       struct iio_chan_spec const *chan,
+			       const int **vals, int *type, int *length,
+			       long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*vals = samp_freq_avail;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(samp_freq_avail);
+
+		return IIO_AVAIL_LIST;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max30210_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan, int val,
+			      int val2, long mask)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	u64 data;
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			return ret;
+
+		/**
+		 * micro_value = val * 1000000 + val2
+		 * reg_value = ((micro_value * 64) / 1000000) - 1
+		 */
+		data = (val * MICRO + val2) << 6;
+		do_div(data, MICRO);
+
+		data = fls_long(data - 1);
+		data = FIELD_PREP(MAX30210_TEMP_PERIOD_MASK, data);
+
+		ret = regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
+					 MAX30210_TEMP_PERIOD_MASK,
+					 (unsigned int)data);
+
+		iio_device_release_direct_mode(indio_dev);
+		return ret;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int max30210_write_raw_get_fmt(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT_PLUS_MICRO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_trigger_ops max30210_trigger_ops = {
+	.validate_device = &iio_trigger_validate_own_device,
+};
+
+static int max30210_set_watermark(struct iio_dev *indio_dev, unsigned int val)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int reg;
+	int ret;
+
+	if (val < 1 || val > MAX30210_FIFO_SIZE)
+		return -EINVAL;
+
+	reg = MAX30210_FIFO_SIZE - val;
+
+	ret = regmap_write(st->regmap, MAX30210_FIFO_CONF_1_REG, reg);
+	if (ret)
+		return ret;
+
+	st->watermark = val;
+
+	return 0;
+}
+
+static ssize_t hwfifo_watermark_show(struct device *dev,
+				     struct device_attribute *devattr,
+				     char *buf)
+{
+	struct max30210_state *st = iio_priv(dev_to_iio_dev(dev));
+
+	return sysfs_emit(buf, "%d\n", st->watermark);
+}
+
+IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_min, "1");
+IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_max,
+			     __stringify(MAX30210_FIFO_SIZE));
+static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
+
+static const struct iio_dev_attr *max30210_fifo_attributes[] = {
+	&iio_dev_attr_hwfifo_watermark_min,
+	&iio_dev_attr_hwfifo_watermark_max,
+	&iio_dev_attr_hwfifo_watermark,
+	NULL,
+};
+
+static int max30210_buffer_preenable(struct iio_dev *indio_dev)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
+				 MAX30210_A_FULL_MASK, MAX30210_A_FULL_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+				 MAX30210_FLUSH_FIFO_MASK,
+				 MAX30210_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
+			   MAX30210_AUTO_MASK | MAX30210_CONV_T_MASK);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
+				 MAX30210_A_FULL_MASK, 0x0);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+				 MAX30210_FLUSH_FIFO_MASK,
+				 MAX30210_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct iio_buffer_setup_ops max30210_buffer_ops = {
+	.preenable = max30210_buffer_preenable,
+	.postdisable = max30210_buffer_postdisable,
+};
+
+static const struct iio_info max30210_info = {
+	.read_raw = max30210_read_raw,
+	.read_avail = max30210_read_avail,
+	.write_raw = max30210_write_raw,
+	.write_raw_get_fmt = max30210_write_raw_get_fmt,
+	.hwfifo_set_watermark = max30210_set_watermark,
+	.debugfs_reg_access = &max30210_reg_access,
+	.validate_trigger = &max30210_validate_trigger,
+	.read_event_value = max30210_read_event,
+	.write_event_value = max30210_write_event,
+	.write_event_config = max30210_write_event_config,
+	.read_event_config = max30210_read_event_config,
+};
+
+static const struct iio_event_spec max30210_events[] = {
+	{
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_RISING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
+	}, {
+		.type = IIO_EV_TYPE_THRESH,
+		.dir = IIO_EV_DIR_FALLING,
+		.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
+	},
+};
+
+static const struct iio_chan_spec max30210_channels = {
+	.type = IIO_TEMP,
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			      BIT(IIO_CHAN_INFO_SCALE) |
+			      BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+	.output = 0,
+	.scan_index = 0,
+	.event_spec = max30210_events,
+	.num_event_specs = ARRAY_SIZE(max30210_events),
+	.scan_type = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 32,
+		.shift = 8,
+		.endianness = IIO_BE,
+	},
+};
+
+static int max30210_setup(struct max30210_state *st, struct device *dev)
+{
+	unsigned int val;
+
+	/* Power down to reset device */
+	st->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(st->powerdown_gpio))
+		return dev_err_probe(dev, PTR_ERR(st->powerdown_gpio),
+				     "Failed to request powerdown GPIO.\n");
+
+	/* Power up device */
+	gpiod_set_value(st->powerdown_gpio, 0);
+
+	fsleep(700);
+
+	/* Clear status byte */
+	return regmap_read(st->regmap, MAX30210_STATUS_REG, &val);
+}
+
+static int max30210_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct iio_dev *indio_dev;
+	struct max30210_state *st;
+	int ret;
+
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
+		return -EOPNOTSUPP;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	st = iio_priv(indio_dev);
+
+	mutex_init(&st->lock);
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to enable vdd regulator.\n");
+
+	st->regmap = devm_regmap_init_i2c(client, &max30210_regmap);
+	if (IS_ERR(st->regmap))
+		return dev_err_probe(dev, PTR_ERR(st->regmap),
+				     "Failed to allocate regmap.\n");
+
+	ret = max30210_setup(st, dev);
+	if (ret)
+		return ret;
+
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = &max30210_channels;
+	indio_dev->num_channels = 1;
+	indio_dev->name = "max30210";
+	indio_dev->info = &max30210_info;
+
+	ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
+						  max30210_trigger_handler,
+						  IIO_BUFFER_DIRECTION_IN,
+						  &max30210_buffer_ops,
+						  max30210_fifo_attributes);
+	if (ret < 0)
+		return ret;
+
+	if (client->irq) {
+		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
+						  indio_dev->name,
+						  iio_device_id(indio_dev));
+		if (!st->trig)
+			return -ENOMEM;
+
+		st->trig->ops = &max30210_trigger_ops;
+		iio_trigger_set_drvdata(st->trig, indio_dev);
+		ret = devm_iio_trigger_register(dev, st->trig);
+		if (ret)
+			return ret;
+
+		indio_dev->trig = st->trig;
+		ret = devm_request_threaded_irq(dev, client->irq,
+						iio_trigger_generic_data_rdy_poll,
+						NULL, IRQF_TRIGGER_FALLING,
+						indio_dev->name, st->trig);
+		if (ret)
+			return ret;
+	}
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static const struct i2c_device_id max30210_id[] = {
+	{ "max30210", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, max30210_id);
+
+static const struct of_device_id max30210_of_match[] = {
+	{ .compatible = "adi,max30210" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, max30210_of_match);
+
+static struct i2c_driver max30210_driver = {
+	.driver = {
+		.name = "max30210",
+		.of_match_table = max30210_of_match,
+	},
+	.probe = max30210_probe,
+	.id_table = max30210_id,
+};
+module_i2c_driver(max30210_driver);
+
+MODULE_AUTHOR("John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com");
+MODULE_DESCRIPTION("MAX30210 low-power digital temperature sensor");
+MODULE_LICENSE("GPL");
-- 
2.34.1

Re: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[Andy Shevchenko]

On Fri, Feb 27, 2026 at 12:30:41AM +0800, John Erasmus Mari Geronimo wrote:
> MAX30210 ±0.1°C Accurate Ultra-Small Low-Power Digital Temperature Sensor

Not enough for the commit message.

...

> +#include <asm/div64.h>

linux/math64.h

> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>

> +#include <linux/debugfs.h>

Used?

> +#include <linux/delay.h>

> +#include <linux/errno.h>

You missed err.h

> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>

> +#include <linux/iio/buffer.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>

Wow! All of them are in use?

> +#include <linux/interrupt.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/stat.h>
> +#include <linux/string.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>

...

> +struct max30210_state {
> +	/*
> +	 * Prevent simultaneous access to the i2c client.
> +	 */
> +	struct mutex lock;

> +	struct regmap *regmap;


And if you swap them, won't the binary size be less?

> +	struct iio_trigger *trig;
> +	struct gpio_desc *powerdown_gpio;
> +	u8 watermark;
> +	u8 data[3 * MAX30210_FIFO_SIZE]  __aligned(IIO_DMA_MINALIGN);

Hmm... Don't we have a macro for this nowadays?

> +};

...

> +static const int samp_freq_avail[] = {

Why not 2D array?

> +	0, 15625,
> +	0, 31250,
> +	0, 62500,
> +	0, 125000,
> +	0, 250000,
> +	0, 500000,
> +	1, 0,
> +	2, 0,
> +	4, 0,
> +	8, 0

Leave trailing comma, it's not a terminator.

> +};

...

> +static int max30210_read_temp(struct regmap *regmap, unsigned int reg,
> +			      int *temp)
> +{
> +	u8 uval[2] __aligned(IIO_DMA_MINALIGN);

No way. This is variable on stack, not all CPUs / architectures allow this.
And actually why this alignment to begin with? Wouldn't

	__be16 val;

suffice?

> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, reg, uval, 2);

sizeof()

> +	if (ret)
> +		return ret;
> +
> +	*temp = sign_extend32(get_unaligned_be16(uval), 15);
> +
> +	return IIO_VAL_INT;
> +}

...

> +static void max30210_fifo_read(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	u32 samp;
> +	int ret, i, j;

Why are 'i' and 'j' signed?


> +	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
> +			       st->data, 3 * st->watermark);
> +	if (ret < 0)
> +		return dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
> +
> +	for (i = 0; i < st->watermark; i++) {

'i' is not used outside for-loop, hence

	for (unsigned int i = 0; i < st->watermark; i++) {

> +		samp = 0;
> +		for (j = 0; j < 3; j++) {
> +			samp <<= 8;
> +			samp |= st->data[3 * i + j];
> +		}

Reinventing get_unaligned_be32() if I'm not mistaken.

> +		if (samp == MAX30210_FIFO_INVAL_DATA) {
> +			dev_err(&indio_dev->dev, "Invalid data\n");
> +			continue;
> +		}
> +
> +		iio_push_to_buffers(indio_dev, &samp);
> +	}
> +}

...

> +static int max30210_setup(struct max30210_state *st, struct device *dev)
> +{
> +	unsigned int val;
> +
> +	/* Power down to reset device */
> +	st->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> +						     GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->powerdown_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->powerdown_gpio),
> +				     "Failed to request powerdown GPIO.\n");
> +
> +	/* Power up device */
> +	gpiod_set_value(st->powerdown_gpio, 0);

All delays must be documented. Add a comment with the datasheet reference to
explain the value and need of the sleep.

> +	fsleep(700);

> +	/* Clear status byte */
> +	return regmap_read(st->regmap, MAX30210_STATUS_REG, &val);
> +}

...

> +static int max30210_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct max30210_state *st;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -EOPNOTSUPP;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);

> +	mutex_init(&st->lock);

	ret = devm_mutex_init(...);

> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to enable vdd regulator.\n");
> +
> +	st->regmap = devm_regmap_init_i2c(client, &max30210_regmap);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to allocate regmap.\n");
> +
> +	ret = max30210_setup(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &max30210_channels;
> +	indio_dev->num_channels = 1;
> +	indio_dev->name = "max30210";
> +	indio_dev->info = &max30210_info;
> +
> +	ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
> +						  max30210_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_IN,
> +						  &max30210_buffer_ops,
> +						  max30210_fifo_attributes);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (client->irq) {
> +		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +						  indio_dev->name,
> +						  iio_device_id(indio_dev));
> +		if (!st->trig)
> +			return -ENOMEM;
> +
> +		st->trig->ops = &max30210_trigger_ops;
> +		iio_trigger_set_drvdata(st->trig, indio_dev);
> +		ret = devm_iio_trigger_register(dev, st->trig);
> +		if (ret)
> +			return ret;
> +
> +		indio_dev->trig = st->trig;
> +		ret = devm_request_threaded_irq(dev, client->irq,
> +						iio_trigger_generic_data_rdy_poll,
> +						NULL, IRQF_TRIGGER_FALLING,
> +						indio_dev->name, st->trig);
> +		if (ret)
> +			return ret;
> +	}

> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

Wouldn't

	return devm_iio_device_register(dev, indio_dev);

suffice?

> +}

...

> +static const struct i2c_device_id max30210_id[] = {
> +	{ "max30210", 0 },

No ', 0' part.

> +	{ }
> +};

...

Can somebody at Analog start a common internal Wiki or other resources
and collect there typical requirements for the code in IIO? It will prevent
reviewers and maintainers from doing the same replies again and again.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: iio: temperature: add ADI MAX30210

[David Lechner]

When there is more than one patch in a series, please include a cover letter.

On 2/26/26 10:30 AM, John Erasmus Mari Geronimo wrote:
> Add device tree binding documentation for the Analog Devices
> MAX30210 temperature sensor.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> ---
>  .../iio/temperature/adi,max30210.yaml         | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> new file mode 100644
> index 000000000000..80aeae23e0a5
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2026 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/temperature/adi,max30210.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices MAX30210 Low-Power I2C Digital Temperature Sensor
> +
> +maintainers:
> +  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> +
> +description: |
> +  The MAX30210 operates from 1.7V to 2.0V supply voltage, and is a low-power,

This voltage range doesn't match the one below.

We don't really need all of the tech specs here anyway, we have the link
to the datasheet.

> +  high-accuracy digital temperature sensor with ±0.1°C accuracy from +20°C to
> +  +50°C and ±0.15°C accuracy from -20°C to +85°C.
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/max30210.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max30210
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |

"|" is not needed here or below. It is only needed to preserve formatting or if
the text contains ":".

> +      Analog Supply Voltage Input. Must have values in the interval (1.7V; 5.5V)
> +      in order for the device to function correctly.
> +
> +  powerdown-gpios:
> +    description: |
> +      GPIO spec for CVT/PDB pin. Should be configured with GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      Connected to INT pin. Should be configured with type IRQ_TYPE_EDGE_BOTH.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply

> +  - powerdown-gpios

Is this pin really required? Seems like it would still work with this pin
hard-wired by using autonomous mode.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/pwm/pwm.h>

pwm is unused.

> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";
> +
> +        temperature-sensor@40 {
> +            compatible = "adi,max30210";
> +            reg = <0x40>;
> +            vdd-supply = <&vdd>;
> +            powerdown-gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
> +
> +            interrupt-parent = <&gpio>;
> +            interrupts = <17 IRQ_TYPE_EDGE_BOTH>;
> +        };
> +    };
> +...

Re: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[David Lechner]

On 2/26/26 10:30 AM, John Erasmus Mari Geronimo wrote:
> MAX30210 ±0.1°C Accurate Ultra-Small Low-Power Digital Temperature Sensor
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> ---
>  MAINTAINERS                        |   8 +
>  drivers/iio/temperature/Kconfig    |  10 +
>  drivers/iio/temperature/Makefile   |   1 +
>  drivers/iio/temperature/max30210.c | 758 +++++++++++++++++++++++++++++
>  4 files changed, 777 insertions(+)
>  create mode 100644 drivers/iio/temperature/max30210.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1c75276404df..2abdbcf3a8e5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1638,6 +1638,14 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
>  F:	drivers/iio/dac/max22007.c
>  
> +ANALOG DEVICES INC MAX30210 DRIVER
> +M:	John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Supported
> +W:	https://ez.analog.com/linux-software-drivers
> +F:	Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml

The part above should be introduced in the first patch where the file
above is added.

> +F:	drivers/iio/temperature/max30210.c

Then only this part added in this patch.

> +
>  ANALOG DEVICES INC ADA4250 DRIVER
>  M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
>  L:	linux-iio@vger.kernel.org
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 9328b2250ace..2d7cb50e2538 100644
> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -184,4 +184,14 @@ config MCP9600
>  	  This driver can also be built as a module. If so, the module
>  	  will be called mcp9600.
>  
> +config MAX30210
> +	tristate "MAX30210 Low-Power I2C Digital Temperature Sensor"
> +	depends on I2C
> +	help
> +	  If you say yes here you get support for MAX30210 low-power digital
> +	  temperature sensor chip connected via I2C.
> +
> +	  This driver can also be build as a module. If so, the module
> +	  will be called max30210.
> +
>  endmenu
> diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
> index 07d6e65709f7..e5aad14dc09b 100644
> --- a/drivers/iio/temperature/Makefile
> +++ b/drivers/iio/temperature/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_LTC2983) += ltc2983.o
>  obj-$(CONFIG_HID_SENSOR_TEMP) += hid-sensor-temperature.o
>  obj-$(CONFIG_MAXIM_THERMOCOUPLE) += maxim_thermocouple.o
>  obj-$(CONFIG_MAX30208) += max30208.o
> +obj-$(CONFIG_MAX30210) += max30210.o

I suppose this has enough differences from max30208 and other chips
to justify a separate driver?

>  obj-$(CONFIG_MAX31856) += max31856.o
>  obj-$(CONFIG_MAX31865) += max31865.o
>  obj-$(CONFIG_MCP9600) += mcp9600.o
> diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
> new file mode 100644
> index 000000000000..aaa3a26be131
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,758 @@
> +// SPDX-License-Identifier: GPL-2.0

Prefer more precise GPL-2.0-only or GPL-2.0-or-later (your choice).

> +/*
> + * Analog Devices MAX30210 I2C Temperature Sensor driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
> +
> +#include <asm/div64.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/debugfs.h>
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/events.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_buffer.h>
> +#include <linux/iio/trigger_consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/log2.h>
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> +#include <linux/pwm.h>
> +#include <linux/regmap.h>
> +#include <linux/stat.h>
> +#include <linux/string.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +#define MAX30210_STATUS_REG		0x00
> +#define MAX30210_INT_EN_REG		0x02
> +#define MAX30210_FIFO_DATA_REG		0x08
> +#define MAX30210_FIFO_CONF_1_REG	0x09
> +#define MAX30210_FIFO_CONF_2_REG	0x0A
> +#define MAX30210_SYS_CONF_REG		0x11
> +#define MAX30210_PIN_CONF_REG		0x12
> +#define MAX30210_TEMP_ALM_HI_REG	0x22
> +#define MAX30210_TEMP_ALM_LO_REG	0x24
> +#define MAX30210_TEMP_INC_THRESH_REG	0x26
> +#define MAX30210_TEMP_DEC_THRESH_REG	0x27
> +#define MAX30210_TEMP_CONF_1_REG	0x28
> +#define MAX30210_TEMP_CONF_2_REG	0x29
> +#define MAX30210_TEMP_CONV_REG		0x2A
> +#define MAX30210_TEMP_DATA_REG		0x2B
> +#define MAX30210_TEMP_SLOPE_REG		0x2D
> +#define MAX30210_UNIQUE_ID_REG		0x30
> +#define MAX30210_PART_ID_REG		0xFF
> +
> +#define MAX30210_A_FULL_MASK   BIT(7)
> +#define MAX30210_TEMP_RDY_MASK BIT(6)
> +#define MAX30210_TEMP_DEC_MASK BIT(5)
> +#define MAX30210_TEMP_INC_MASK BIT(4)
> +#define MAX30210_TEMP_LO_MASK  BIT(3)
> +#define MAX30210_TEMP_HI_MASK  BIT(2)
> +#define MAX30210_PWR_RDY_MASK  BIT(0)
> +
> +#define MAX30210_FLUSH_FIFO_MASK BIT(4)
> +
> +#define MAX30210_EXT_CNV_EN_MASK	BIT(7)
> +#define MAX30210_EXT_CVT_ICFG_MASK	BIT(6)
> +#define MAX30210_INT_FCFG_MASK		GENMASK(3, 2)
> +#define MAX30210_INT_OCFG_MASK		GENMASK(1, 0)
> +
> +#define MAX30210_CHG_DET_EN_MASK	BIT(3)
> +#define MAX30210_RATE_CHG_FILTER_MASK	GENMASK(2, 0)
> +
> +#define MAX30210_TEMP_PERIOD_MASK	GENMASK(3, 0)
> +#define MAX30210_ALERT_MODE_MASK	BIT(7)
> +
> +#define MAX30210_AUTO_MASK	BIT(1)
> +#define MAX30210_CONV_T_MASK	BIT(0)
> +
> +#define MAX30210_PART_ID		0x45
> +#define MAX30210_FIFO_SIZE		64
> +#define MAX30210_FIFO_INVAL_DATA	GENMASK(23, 0)
> +#define MAX30210_WATERMARK_DEFAULT	(0x40 - 0x1F)
> +
> +#define MAX30210_INT_EN(state, mask)	((state) ? (mask) : 0x0)
> +
> +#define MAX30210_UNIQUE_ID_LEN		6
> +#define MAX30210_EXT_CVT_FREQ_MIN	1
> +#define MAX30210_EXT_CVT_FREQ_MAX	20
> +
> +struct max30210_state {
> +	/*
> +	 * Prevent simultaneous access to the i2c client.

This is currently only used in the interrupt handler, which is already
protected agains reentrancy. So we either don't need this or it should
be used in many more places to actually provide protection.

> +	 */
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	struct iio_trigger *trig;

Probably can be a local varaible when combined with another suggestion
later to use a default function implementation.

> +	struct gpio_desc *powerdown_gpio;

This is not used outside of max30210_setup(), so can be a local variable.

> +	u8 watermark;
> +	u8 data[3 * MAX30210_FIFO_SIZE]  __aligned(IIO_DMA_MINALIGN);

We should make this clear that this is FIFO data and not scan data.
Usually, we only see something like this for scan data, which would
need to be 4 bytes per sample rather than 3.

(This is why Andy thought we shuold be using IIO_DECLARE_BUFFER_WITH_TS()
here, but it isn't actually the case.)

> +};
> +
> +static const int samp_freq_avail[] = {
> +	0, 15625,
> +	0, 31250,
> +	0, 62500,
> +	0, 125000,
> +	0, 250000,
> +	0, 500000,
> +	1, 0,
> +	2, 0,
> +	4, 0,
> +	8, 0
> +};
> +
> +static const struct regmap_config max30210_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX30210_PART_ID_REG,
> +};
> +
> +static int max30210_read_temp(struct regmap *regmap, unsigned int reg,
> +			      int *temp)
> +{
> +	u8 uval[2] __aligned(IIO_DMA_MINALIGN);
> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, reg, uval, 2);
> +	if (ret)
> +		return ret;
> +
> +	*temp = sign_extend32(get_unaligned_be16(uval), 15);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int max30210_write_temp(struct regmap *regmap, unsigned int reg,
> +			       int temp)
> +{
> +	u8 uval[2] __aligned(IIO_DMA_MINALIGN);
> +
> +	put_unaligned_be16(temp, uval);
> +
> +	return regmap_bulk_write(regmap, reg, uval, 2);
> +}
> +
> +static void max30210_fifo_read(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	u32 samp;
> +	int ret, i, j;
> +
> +	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
> +			       st->data, 3 * st->watermark);
> +	if (ret < 0)
> +		return dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
> +
> +	for (i = 0; i < st->watermark; i++) {
> +		samp = 0;
> +
> +		for (j = 0; j < 3; j++) {
> +			samp <<= 8;
> +			samp |= st->data[3 * i + j];
> +		}

Can we not just do a 3 byte memcpy() here?

Or if this is making it CPU-endian, then use get_unaligned_be24()
and fix IIO_BE to be IIO_CPU in the scan data.

> +
> +		if (samp == MAX30210_FIFO_INVAL_DATA) {
> +			dev_err(&indio_dev->dev, "Invalid data\n");

This error could get quite noisy. There is a rate-limited version
of dev_err() we could use.

> +			continue;
> +		}
> +
> +		iio_push_to_buffers(indio_dev, &samp);
> +	}
> +}
> +
> +static irqreturn_t max30210_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int status;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Status byte read error\n");
> +		goto exit_irq;
> +	}
> +
> +	if (status & MAX30210_PWR_RDY_MASK) {
> +		dev_info(&indio_dev->dev, "power-on\n");

dev_dbg(). Although I question that we really need this at all. There is
a time delay in the setup function, so it seems like nothing would be
waiting for this.

> +		st->watermark = MAX30210_WATERMARK_DEFAULT;

Would make more sense to set the default during probe.

> +	}
> +
> +	if (status & MAX30210_A_FULL_MASK)
> +		max30210_fifo_read(indio_dev);
> +
> +	if (status & MAX30210_TEMP_HI_MASK)
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +			       iio_get_time_ns(indio_dev));
> +
> +	if (status & MAX30210_TEMP_LO_MASK)
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_FALLING),
> +			       iio_get_time_ns(indio_dev));
> +
> +exit_irq:
> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}
> +
> +static int max30210_reg_access(struct iio_dev *indio_dev, unsigned int reg,
> +			       unsigned int writeval, unsigned int *readval)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (!readval)
> +		return regmap_write(st->regmap, reg, writeval);
> +
> +	return regmap_read(st->regmap, reg, readval);
> +}
> +
> +static int max30210_validate_trigger(struct iio_dev *indio_dev,
> +				     struct iio_trigger *trig)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (st->trig != trig)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int max30210_read_event(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       enum iio_event_type type,
> +			       enum iio_event_direction dir,
> +			       enum iio_event_info info, int *val, int *val2)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (info == IIO_EV_INFO_VALUE) {
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_read_temp(st->regmap,
> +							  MAX30210_TEMP_ALM_HI_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;

break is unreachable and should be omitted.

> +		case IIO_EV_DIR_FALLING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_read_temp(st->regmap,
> +							  MAX30210_TEMP_ALM_LO_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;

Same.

> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return -EINVAL;

Just return early after the if to avoid indenting the switch statement so much.

> +}
> +
> +static int max30210_write_event(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info, int val, int val2)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (info == IIO_EV_INFO_VALUE) {
> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_write_temp(st->regmap,
> +							   MAX30210_TEMP_ALM_HI_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		case IIO_EV_DIR_FALLING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_write_temp(st->regmap,
> +							   MAX30210_TEMP_ALM_LO_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return -EINVAL;

Same.

> +}
> +
> +static int max30210_read_event_config(struct iio_dev *indio_dev,
> +				      const struct iio_chan_spec *chan,
> +				      enum iio_event_type type,
> +				      enum iio_event_direction dir)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, MAX30210_INT_EN_REG, &val);
> +	if (ret)
> +		return ret;
> +
> +	switch (dir) {
> +	case IIO_EV_DIR_RISING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			return FIELD_GET(MAX30210_TEMP_HI_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_EV_DIR_FALLING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			return FIELD_GET(MAX30210_TEMP_LO_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max30210_write_event_config(struct iio_dev *indio_dev,
> +				       const struct iio_chan_spec *chan,
> +				       enum iio_event_type type,
> +				       enum iio_event_direction dir, int state)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int val;
> +
> +	switch (dir) {
> +	case IIO_EV_DIR_RISING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			val = MAX30210_INT_EN(state, MAX30210_TEMP_HI_MASK);
> +
> +			return regmap_update_bits(st->regmap,
> +						  MAX30210_INT_EN_REG,
> +						  MAX30210_TEMP_HI_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +		break;
> +	case IIO_EV_DIR_FALLING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			val = MAX30210_INT_EN(state, MAX30210_TEMP_LO_MASK);
> +
> +			return regmap_update_bits(st->regmap,
> +						  MAX30210_INT_EN_REG,
> +						  MAX30210_TEMP_LO_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max30210_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int uval;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 5;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
> +		if (ret)
> +			return ret;
> +
> +		uval = FIELD_GET(MAX30210_TEMP_PERIOD_MASK, uval);
> +
> +		*val = 8;
> +
> +		/**
> +		 * register values 0x9 or above have the same sample
> +		 * rate of 8Hz
> +		 */
> +		*val2 = uval >= 0x9 ? 1 : BIT(0x9 - uval);
> +
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(indio_dev);

If the buffer is enabled, can we just skip setting the converion mode
and read the TEMP_DATA register to get the last value measured?

Also, iio_device_claim_direct_mode() doesn't exist anymore, so I wonder
which kernel version was used to compile and test this patch.

We now have IIO_DEV_ACQUIRE_DIRECT_MODE() that can be used in cases
like this to avoid the goto.


> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +				   MAX30210_CONV_T_MASK);
> +		if (ret)
> +			goto release_dmode;
> +
> +		fsleep(8000);
> +
> +		ret = max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +
> +release_dmode:
> +		iio_device_release_direct_mode(indio_dev);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max30210_read_avail(struct iio_dev *indio_dev,
> +			       struct iio_chan_spec const *chan,
> +			       const int **vals, int *type, int *length,
> +			       long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*vals = samp_freq_avail;
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = ARRAY_SIZE(samp_freq_avail);
> +
> +		return IIO_AVAIL_LIST;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max30210_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan, int val,
> +			      int val2, long mask)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	u64 data;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		/**
> +		 * micro_value = val * 1000000 + val2
> +		 * reg_value = ((micro_value * 64) / 1000000) - 1
> +		 */
> +		data = (val * MICRO + val2) << 6;

This will end up with wierd resutls if val or val2 is negative.

Can we write << 6 as * 64 to match the comment? The compiler is smart enough
to make the optimization for us.

> +		do_div(data, MICRO);
> +
> +		data = fls_long(data - 1);
> +		data = FIELD_PREP(MAX30210_TEMP_PERIOD_MASK, data);
> +
> +		ret = regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
> +					 MAX30210_TEMP_PERIOD_MASK,
> +					 (unsigned int)data);
> +
> +		iio_device_release_direct_mode(indio_dev);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int max30210_write_raw_get_fmt(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return IIO_VAL_INT_PLUS_MICRO;

This is the default, so we should not need this function at all.

> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_trigger_ops max30210_trigger_ops = {
> +	.validate_device = &iio_trigger_validate_own_device,
> +};
> +
> +static int max30210_set_watermark(struct iio_dev *indio_dev, unsigned int val)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int reg;
> +	int ret;
> +
> +	if (val < 1 || val > MAX30210_FIFO_SIZE)
> +		return -EINVAL;
> +
> +	reg = MAX30210_FIFO_SIZE - val;
> +
> +	ret = regmap_write(st->regmap, MAX30210_FIFO_CONF_1_REG, reg);
> +	if (ret)
> +		return ret;
> +
> +	st->watermark = val;
> +
> +	return 0;
> +}
> +
> +static ssize_t hwfifo_watermark_show(struct device *dev,
> +				     struct device_attribute *devattr,
> +				     char *buf)
> +{
> +	struct max30210_state *st = iio_priv(dev_to_iio_dev(dev));
> +
> +	return sysfs_emit(buf, "%d\n", st->watermark);
> +}
> +
> +IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_min, "1");
> +IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_max,
> +			     __stringify(MAX30210_FIFO_SIZE));
> +static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
> +
> +static const struct iio_dev_attr *max30210_fifo_attributes[] = {
> +	&iio_dev_attr_hwfifo_watermark_min,
> +	&iio_dev_attr_hwfifo_watermark_max,
> +	&iio_dev_attr_hwfifo_watermark,
> +	NULL,
> +};
> +
> +static int max30210_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_A_FULL_MASK, MAX30210_A_FULL_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +				 MAX30210_FLUSH_FIFO_MASK,
> +				 MAX30210_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +			   MAX30210_AUTO_MASK | MAX30210_CONV_T_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +

Unwinding should be the revese order of the setup. If there is a reason
for it to be in a different order, it needs a comment to explain.

> +	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_A_FULL_MASK, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +				 MAX30210_FLUSH_FIFO_MASK,
> +				 MAX30210_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops max30210_buffer_ops = {
> +	.preenable = max30210_buffer_preenable,
> +	.postdisable = max30210_buffer_postdisable,
> +};
> +
> +static const struct iio_info max30210_info = {
> +	.read_raw = max30210_read_raw,
> +	.read_avail = max30210_read_avail,
> +	.write_raw = max30210_write_raw,
> +	.write_raw_get_fmt = max30210_write_raw_get_fmt,
> +	.hwfifo_set_watermark = max30210_set_watermark,
> +	.debugfs_reg_access = &max30210_reg_access,

> +	.validate_trigger = &max30210_validate_trigger,

Can we just use iio_validate_own_trigger()?

> +	.read_event_value = max30210_read_event,
> +	.write_event_value = max30210_write_event,
> +	.write_event_config = max30210_write_event_config,
> +	.read_event_config = max30210_read_event_config,
> +};
> +
> +static const struct iio_event_spec max30210_events[] = {
> +	{
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_RISING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
> +	}, {
> +		.type = IIO_EV_TYPE_THRESH,
> +		.dir = IIO_EV_DIR_FALLING,
> +		.mask_separate = BIT(IIO_EV_INFO_VALUE) | BIT(IIO_EV_INFO_ENABLE),
> +	},
> +};
> +
> +static const struct iio_chan_spec max30210_channels = {
> +	.type = IIO_TEMP,
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			      BIT(IIO_CHAN_INFO_SCALE) |
> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	.output = 0,
> +	.scan_index = 0,
> +	.event_spec = max30210_events,
> +	.num_event_specs = ARRAY_SIZE(max30210_events),
> +	.scan_type = {
> +		.sign = 's',
> +		.realbits = 16,
> +		.storagebits = 32,
> +		.shift = 8,
> +		.endianness = IIO_BE,
> +	},
> +};
> +
> +static int max30210_setup(struct max30210_state *st, struct device *dev)
> +{
> +	unsigned int val;
> +
> +	/* Power down to reset device */
> +	st->powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> +						     GPIOD_OUT_HIGH);
> +	if (IS_ERR(st->powerdown_gpio))
> +		return dev_err_probe(dev, PTR_ERR(st->powerdown_gpio),
> +				     "Failed to request powerdown GPIO.\n");
> +
> +	/* Power up device */
> +	gpiod_set_value(st->powerdown_gpio, 0);

Usually, if the gpio is not wired up, we would reset the device via
a software reset. It looks like this has one in the SYSTEM_CONFIGURATION
register.

> +
> +	fsleep(700);
> +
> +	/* Clear status byte */
> +	return regmap_read(st->regmap, MAX30210_STATUS_REG, &val);
> +}
> +

Re: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[kernel test robot]

Hi John,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v7.0-rc1 next-20260226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Erasmus-Mari-Geronimo/dt-bindings-iio-temperature-add-ADI-MAX30210/20260227-013306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260226163041.169786-3-johnerasmusmari.geronimo%40analog.com
patch subject: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver
config: nios2-allmodconfig (https://download.01.org/0day-ci/archive/20260227/202602270554.gpbYaUtd-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260227/202602270554.gpbYaUtd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602270554.gpbYaUtd-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iio/temperature/max30210.c: In function 'max30210_read_raw':
>> drivers/iio/temperature/max30210.c:412:23: error: implicit declaration of function 'iio_device_claim_direct_mode'; did you mean 'iio_device_claim_direct'? [-Werror=implicit-function-declaration]
     412 |                 ret = iio_device_claim_direct_mode(indio_dev);
         |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                       iio_device_claim_direct
>> drivers/iio/temperature/max30210.c:426:17: error: implicit declaration of function 'iio_device_release_direct_mode'; did you mean 'iio_device_release_direct'? [-Werror=implicit-function-declaration]
     426 |                 iio_device_release_direct_mode(indio_dev);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                 iio_device_release_direct
   drivers/iio/temperature/max30210.c: At top level:
>> drivers/iio/temperature/max30210.c:604:31: error: initialization of 'int (*)(struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type,  enum iio_event_direction,  bool)' {aka 'int (*)(struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type,  enum iio_event_direction,  _Bool)'} from incompatible pointer type 'int (*)(struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type,  enum iio_event_direction,  int)' [-Werror=incompatible-pointer-types]
     604 |         .write_event_config = max30210_write_event_config,
         |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/iio/temperature/max30210.c:604:31: note: (near initialization for 'max30210_info.write_event_config')
   cc1: some warnings being treated as errors


vim +412 drivers/iio/temperature/max30210.c

   381	
   382	static int max30210_read_raw(struct iio_dev *indio_dev,
   383				     struct iio_chan_spec const *chan, int *val,
   384				     int *val2, long mask)
   385	{
   386		struct max30210_state *st = iio_priv(indio_dev);
   387		unsigned int uval;
   388		int ret;
   389	
   390		switch (mask) {
   391		case IIO_CHAN_INFO_SCALE:
   392			*val = 5;
   393	
   394			return IIO_VAL_INT;
   395		case IIO_CHAN_INFO_SAMP_FREQ:
   396			ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
   397			if (ret)
   398				return ret;
   399	
   400			uval = FIELD_GET(MAX30210_TEMP_PERIOD_MASK, uval);
   401	
   402			*val = 8;
   403	
   404			/**
   405			 * register values 0x9 or above have the same sample
   406			 * rate of 8Hz
   407			 */
   408			*val2 = uval >= 0x9 ? 1 : BIT(0x9 - uval);
   409	
   410			return IIO_VAL_FRACTIONAL;
   411		case IIO_CHAN_INFO_RAW:
 > 412			ret = iio_device_claim_direct_mode(indio_dev);
   413			if (ret)
   414				return ret;
   415	
   416			ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
   417					   MAX30210_CONV_T_MASK);
   418			if (ret)
   419				goto release_dmode;
   420	
   421			fsleep(8000);
   422	
   423			ret = max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
   424	
   425	release_dmode:
 > 426			iio_device_release_direct_mode(indio_dev);
   427			return ret;
   428		default:
   429			return -EINVAL;
   430		}
   431	}
   432	
   433	static int max30210_read_avail(struct iio_dev *indio_dev,
   434				       struct iio_chan_spec const *chan,
   435				       const int **vals, int *type, int *length,
   436				       long mask)
   437	{
   438		switch (mask) {
   439		case IIO_CHAN_INFO_SAMP_FREQ:
   440			*vals = samp_freq_avail;
   441			*type = IIO_VAL_INT_PLUS_MICRO;
   442			*length = ARRAY_SIZE(samp_freq_avail);
   443	
   444			return IIO_AVAIL_LIST;
   445		default:
   446			return -EINVAL;
   447		}
   448	}
   449	
   450	static int max30210_write_raw(struct iio_dev *indio_dev,
   451				      struct iio_chan_spec const *chan, int val,
   452				      int val2, long mask)
   453	{
   454		struct max30210_state *st = iio_priv(indio_dev);
   455		u64 data;
   456		int ret;
   457	
   458		switch (mask) {
   459		case IIO_CHAN_INFO_SAMP_FREQ:
   460			ret = iio_device_claim_direct_mode(indio_dev);
   461			if (ret)
   462				return ret;
   463	
   464			/**
   465			 * micro_value = val * 1000000 + val2
   466			 * reg_value = ((micro_value * 64) / 1000000) - 1
   467			 */
   468			data = (val * MICRO + val2) << 6;
   469			do_div(data, MICRO);
   470	
   471			data = fls_long(data - 1);
   472			data = FIELD_PREP(MAX30210_TEMP_PERIOD_MASK, data);
   473	
   474			ret = regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
   475						 MAX30210_TEMP_PERIOD_MASK,
   476						 (unsigned int)data);
   477	
   478			iio_device_release_direct_mode(indio_dev);
   479			return ret;
   480		default:
   481			return -EINVAL;
   482		}
   483	}
   484	
   485	static int max30210_write_raw_get_fmt(struct iio_dev *indio_dev,
   486					      struct iio_chan_spec const *chan,
   487					      long mask)
   488	{
   489		switch (mask) {
   490		case IIO_CHAN_INFO_SAMP_FREQ:
   491			return IIO_VAL_INT_PLUS_MICRO;
   492		default:
   493			return -EINVAL;
   494		}
   495	}
   496	
   497	static const struct iio_trigger_ops max30210_trigger_ops = {
   498		.validate_device = &iio_trigger_validate_own_device,
   499	};
   500	
   501	static int max30210_set_watermark(struct iio_dev *indio_dev, unsigned int val)
   502	{
   503		struct max30210_state *st = iio_priv(indio_dev);
   504		unsigned int reg;
   505		int ret;
   506	
   507		if (val < 1 || val > MAX30210_FIFO_SIZE)
   508			return -EINVAL;
   509	
   510		reg = MAX30210_FIFO_SIZE - val;
   511	
   512		ret = regmap_write(st->regmap, MAX30210_FIFO_CONF_1_REG, reg);
   513		if (ret)
   514			return ret;
   515	
   516		st->watermark = val;
   517	
   518		return 0;
   519	}
   520	
   521	static ssize_t hwfifo_watermark_show(struct device *dev,
   522					     struct device_attribute *devattr,
   523					     char *buf)
   524	{
   525		struct max30210_state *st = iio_priv(dev_to_iio_dev(dev));
   526	
   527		return sysfs_emit(buf, "%d\n", st->watermark);
   528	}
   529	
   530	IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_min, "1");
   531	IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_max,
   532				     __stringify(MAX30210_FIFO_SIZE));
   533	static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
   534	
   535	static const struct iio_dev_attr *max30210_fifo_attributes[] = {
   536		&iio_dev_attr_hwfifo_watermark_min,
   537		&iio_dev_attr_hwfifo_watermark_max,
   538		&iio_dev_attr_hwfifo_watermark,
   539		NULL,
   540	};
   541	
   542	static int max30210_buffer_preenable(struct iio_dev *indio_dev)
   543	{
   544		struct max30210_state *st = iio_priv(indio_dev);
   545		int ret;
   546	
   547		ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
   548					 MAX30210_A_FULL_MASK, MAX30210_A_FULL_MASK);
   549		if (ret)
   550			return ret;
   551	
   552		ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
   553					 MAX30210_FLUSH_FIFO_MASK,
   554					 MAX30210_FLUSH_FIFO_MASK);
   555		if (ret)
   556			return ret;
   557	
   558		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
   559				   MAX30210_AUTO_MASK | MAX30210_CONV_T_MASK);
   560		if (ret)
   561			return ret;
   562	
   563		return 0;
   564	}
   565	
   566	static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
   567	{
   568		struct max30210_state *st = iio_priv(indio_dev);
   569		int ret;
   570	
   571		ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
   572					 MAX30210_A_FULL_MASK, 0x0);
   573		if (ret)
   574			return ret;
   575	
   576		ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
   577					 MAX30210_FLUSH_FIFO_MASK,
   578					 MAX30210_FLUSH_FIFO_MASK);
   579		if (ret)
   580			return ret;
   581	
   582		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
   583		if (ret)
   584			return ret;
   585	
   586		return 0;
   587	}
   588	
   589	static const struct iio_buffer_setup_ops max30210_buffer_ops = {
   590		.preenable = max30210_buffer_preenable,
   591		.postdisable = max30210_buffer_postdisable,
   592	};
   593	
   594	static const struct iio_info max30210_info = {
   595		.read_raw = max30210_read_raw,
   596		.read_avail = max30210_read_avail,
   597		.write_raw = max30210_write_raw,
   598		.write_raw_get_fmt = max30210_write_raw_get_fmt,
   599		.hwfifo_set_watermark = max30210_set_watermark,
   600		.debugfs_reg_access = &max30210_reg_access,
   601		.validate_trigger = &max30210_validate_trigger,
   602		.read_event_value = max30210_read_event,
   603		.write_event_value = max30210_write_event,
 > 604		.write_event_config = max30210_write_event_config,
   605		.read_event_config = max30210_read_event_config,
   606	};
   607	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[kernel test robot]

Hi John,

kernel test robot noticed the following build errors:

[auto build test ERROR on jic23-iio/togreg]
[also build test ERROR on linus/master v7.0-rc1 next-20260226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Erasmus-Mari-Geronimo/dt-bindings-iio-temperature-add-ADI-MAX30210/20260227-013306
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git togreg
patch link:    https://lore.kernel.org/r/20260226163041.169786-3-johnerasmusmari.geronimo%40analog.com
patch subject: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20260227/202602270610.Batqc2is-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project 9a109fbb6e184ec9bcce10615949f598f4c974a9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260227/202602270610.Batqc2is-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602270610.Batqc2is-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/temperature/max30210.c:412:9: error: call to undeclared function 'iio_device_claim_direct_mode'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     412 |                 ret = iio_device_claim_direct_mode(indio_dev);
         |                       ^
   drivers/iio/temperature/max30210.c:412:9: note: did you mean 'iio_device_claim_direct'?
   include/linux/iio/iio.h:687:20: note: 'iio_device_claim_direct' declared here
     687 | static inline bool iio_device_claim_direct(struct iio_dev *indio_dev)
         |                    ^
>> drivers/iio/temperature/max30210.c:426:3: error: call to undeclared function 'iio_device_release_direct_mode'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     426 |                 iio_device_release_direct_mode(indio_dev);
         |                 ^
   drivers/iio/temperature/max30210.c:460:9: error: call to undeclared function 'iio_device_claim_direct_mode'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     460 |                 ret = iio_device_claim_direct_mode(indio_dev);
         |                       ^
   drivers/iio/temperature/max30210.c:478:3: error: call to undeclared function 'iio_device_release_direct_mode'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     478 |                 iio_device_release_direct_mode(indio_dev);
         |                 ^
>> drivers/iio/temperature/max30210.c:604:24: error: incompatible function pointer types initializing 'int (*)(struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type, enum iio_event_direction, bool)' (aka 'int (*)(struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type, enum iio_event_direction, _Bool)') with an expression of type 'int (struct iio_dev *, const struct iio_chan_spec *, enum iio_event_type, enum iio_event_direction, int)' [-Wincompatible-function-pointer-types]
     604 |         .write_event_config = max30210_write_event_config,
         |                               ^~~~~~~~~~~~~~~~~~~~~~~~~~~
   5 errors generated.


vim +/iio_device_claim_direct_mode +412 drivers/iio/temperature/max30210.c

   381	
   382	static int max30210_read_raw(struct iio_dev *indio_dev,
   383				     struct iio_chan_spec const *chan, int *val,
   384				     int *val2, long mask)
   385	{
   386		struct max30210_state *st = iio_priv(indio_dev);
   387		unsigned int uval;
   388		int ret;
   389	
   390		switch (mask) {
   391		case IIO_CHAN_INFO_SCALE:
   392			*val = 5;
   393	
   394			return IIO_VAL_INT;
   395		case IIO_CHAN_INFO_SAMP_FREQ:
   396			ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
   397			if (ret)
   398				return ret;
   399	
   400			uval = FIELD_GET(MAX30210_TEMP_PERIOD_MASK, uval);
   401	
   402			*val = 8;
   403	
   404			/**
   405			 * register values 0x9 or above have the same sample
   406			 * rate of 8Hz
   407			 */
   408			*val2 = uval >= 0x9 ? 1 : BIT(0x9 - uval);
   409	
   410			return IIO_VAL_FRACTIONAL;
   411		case IIO_CHAN_INFO_RAW:
 > 412			ret = iio_device_claim_direct_mode(indio_dev);
   413			if (ret)
   414				return ret;
   415	
   416			ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
   417					   MAX30210_CONV_T_MASK);
   418			if (ret)
   419				goto release_dmode;
   420	
   421			fsleep(8000);
   422	
   423			ret = max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
   424	
   425	release_dmode:
 > 426			iio_device_release_direct_mode(indio_dev);
   427			return ret;
   428		default:
   429			return -EINVAL;
   430		}
   431	}
   432	
   433	static int max30210_read_avail(struct iio_dev *indio_dev,
   434				       struct iio_chan_spec const *chan,
   435				       const int **vals, int *type, int *length,
   436				       long mask)
   437	{
   438		switch (mask) {
   439		case IIO_CHAN_INFO_SAMP_FREQ:
   440			*vals = samp_freq_avail;
   441			*type = IIO_VAL_INT_PLUS_MICRO;
   442			*length = ARRAY_SIZE(samp_freq_avail);
   443	
   444			return IIO_AVAIL_LIST;
   445		default:
   446			return -EINVAL;
   447		}
   448	}
   449	
   450	static int max30210_write_raw(struct iio_dev *indio_dev,
   451				      struct iio_chan_spec const *chan, int val,
   452				      int val2, long mask)
   453	{
   454		struct max30210_state *st = iio_priv(indio_dev);
   455		u64 data;
   456		int ret;
   457	
   458		switch (mask) {
   459		case IIO_CHAN_INFO_SAMP_FREQ:
   460			ret = iio_device_claim_direct_mode(indio_dev);
   461			if (ret)
   462				return ret;
   463	
   464			/**
   465			 * micro_value = val * 1000000 + val2
   466			 * reg_value = ((micro_value * 64) / 1000000) - 1
   467			 */
   468			data = (val * MICRO + val2) << 6;
   469			do_div(data, MICRO);
   470	
   471			data = fls_long(data - 1);
   472			data = FIELD_PREP(MAX30210_TEMP_PERIOD_MASK, data);
   473	
   474			ret = regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
   475						 MAX30210_TEMP_PERIOD_MASK,
   476						 (unsigned int)data);
   477	
   478			iio_device_release_direct_mode(indio_dev);
   479			return ret;
   480		default:
   481			return -EINVAL;
   482		}
   483	}
   484	
   485	static int max30210_write_raw_get_fmt(struct iio_dev *indio_dev,
   486					      struct iio_chan_spec const *chan,
   487					      long mask)
   488	{
   489		switch (mask) {
   490		case IIO_CHAN_INFO_SAMP_FREQ:
   491			return IIO_VAL_INT_PLUS_MICRO;
   492		default:
   493			return -EINVAL;
   494		}
   495	}
   496	
   497	static const struct iio_trigger_ops max30210_trigger_ops = {
   498		.validate_device = &iio_trigger_validate_own_device,
   499	};
   500	
   501	static int max30210_set_watermark(struct iio_dev *indio_dev, unsigned int val)
   502	{
   503		struct max30210_state *st = iio_priv(indio_dev);
   504		unsigned int reg;
   505		int ret;
   506	
   507		if (val < 1 || val > MAX30210_FIFO_SIZE)
   508			return -EINVAL;
   509	
   510		reg = MAX30210_FIFO_SIZE - val;
   511	
   512		ret = regmap_write(st->regmap, MAX30210_FIFO_CONF_1_REG, reg);
   513		if (ret)
   514			return ret;
   515	
   516		st->watermark = val;
   517	
   518		return 0;
   519	}
   520	
   521	static ssize_t hwfifo_watermark_show(struct device *dev,
   522					     struct device_attribute *devattr,
   523					     char *buf)
   524	{
   525		struct max30210_state *st = iio_priv(dev_to_iio_dev(dev));
   526	
   527		return sysfs_emit(buf, "%d\n", st->watermark);
   528	}
   529	
   530	IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_min, "1");
   531	IIO_STATIC_CONST_DEVICE_ATTR(hwfifo_watermark_max,
   532				     __stringify(MAX30210_FIFO_SIZE));
   533	static IIO_DEVICE_ATTR_RO(hwfifo_watermark, 0);
   534	
   535	static const struct iio_dev_attr *max30210_fifo_attributes[] = {
   536		&iio_dev_attr_hwfifo_watermark_min,
   537		&iio_dev_attr_hwfifo_watermark_max,
   538		&iio_dev_attr_hwfifo_watermark,
   539		NULL,
   540	};
   541	
   542	static int max30210_buffer_preenable(struct iio_dev *indio_dev)
   543	{
   544		struct max30210_state *st = iio_priv(indio_dev);
   545		int ret;
   546	
   547		ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
   548					 MAX30210_A_FULL_MASK, MAX30210_A_FULL_MASK);
   549		if (ret)
   550			return ret;
   551	
   552		ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
   553					 MAX30210_FLUSH_FIFO_MASK,
   554					 MAX30210_FLUSH_FIFO_MASK);
   555		if (ret)
   556			return ret;
   557	
   558		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
   559				   MAX30210_AUTO_MASK | MAX30210_CONV_T_MASK);
   560		if (ret)
   561			return ret;
   562	
   563		return 0;
   564	}
   565	
   566	static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
   567	{
   568		struct max30210_state *st = iio_priv(indio_dev);
   569		int ret;
   570	
   571		ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
   572					 MAX30210_A_FULL_MASK, 0x0);
   573		if (ret)
   574			return ret;
   575	
   576		ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
   577					 MAX30210_FLUSH_FIFO_MASK,
   578					 MAX30210_FLUSH_FIFO_MASK);
   579		if (ret)
   580			return ret;
   581	
   582		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
   583		if (ret)
   584			return ret;
   585	
   586		return 0;
   587	}
   588	
   589	static const struct iio_buffer_setup_ops max30210_buffer_ops = {
   590		.preenable = max30210_buffer_preenable,
   591		.postdisable = max30210_buffer_postdisable,
   592	};
   593	
   594	static const struct iio_info max30210_info = {
   595		.read_raw = max30210_read_raw,
   596		.read_avail = max30210_read_avail,
   597		.write_raw = max30210_write_raw,
   598		.write_raw_get_fmt = max30210_write_raw_get_fmt,
   599		.hwfifo_set_watermark = max30210_set_watermark,
   600		.debugfs_reg_access = &max30210_reg_access,
   601		.validate_trigger = &max30210_validate_trigger,
   602		.read_event_value = max30210_read_event,
   603		.write_event_value = max30210_write_event,
 > 604		.write_event_config = max30210_write_event_config,
   605		.read_event_config = max30210_read_event_config,
   606	};
   607	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/2] dt-bindings: iio: temperature: add ADI MAX30210

[Krzysztof Kozlowski]

On Fri, Feb 27, 2026 at 12:30:40AM +0800, John Erasmus Mari Geronimo wrote:
> +
> +maintainers:
> +  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> +
> +description: |
> +  The MAX30210 operates from 1.7V to 2.0V supply voltage, and is a low-power,
> +  high-accuracy digital temperature sensor with ±0.1°C accuracy from +20°C to
> +  +50°C and ±0.15°C accuracy from -20°C to +85°C.
> +  https://www.analog.com/media/en/technical-documentation/data-sheets/max30210.pdf
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,max30210
> +
> +  reg:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: |

Do not need '|' unless you need to preserve formatting. Same in other
places.

> +      Analog Supply Voltage Input. Must have values in the interval (1.7V; 5.5V)
> +      in order for the device to function correctly.
> +
> +  powerdown-gpios:
> +    description: |
> +      GPIO spec for CVT/PDB pin. Should be configured with GPIO_ACTIVE_LOW.

Should be configured depending on wiring, because the flag includes any
inverters. Just say it is active low,


> +    maxItems: 1
> +
> +  interrupts:
> +    description: |
> +      Connected to INT pin. Should be configured with type IRQ_TYPE_EDGE_BOTH.

No, just say it is interrupt triggered by raising and falling edges.

> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +  - powerdown-gpios
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +    #include <dt-bindings/pwm/pwm.h>

Where do you use this header?

> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        status = "okay";

Drop.

> +
> +        temperature-sensor@40 {
> +            compatible = "adi,max30210";
> +            reg = <0x40>;
> +            vdd-supply = <&vdd>;
> +            powerdown-gpios = <&gpio 12 GPIO_ACTIVE_LOW>;
> +
> +            interrupt-parent = <&gpio>;
> +            interrupts = <17 IRQ_TYPE_EDGE_BOTH>;
> +        };
> +    };
> +...
> -- 
> 2.34.1
> 

Re: [PATCH 1/2] dt-bindings: iio: temperature: add ADI MAX30210

[Jonathan Cameron]

On Thu, 26 Feb 2026 12:33:38 -0600
David Lechner <dlechner@baylibre.com> wrote:

> When there is more than one patch in a series, please include a cover letter.
Looks like different patches got different CC lists.

For a small series like this just +CC the same folk on all emails
including the cover letter that David didn't get

Thanks,

Jonathan

Re: [PATCH 2/2] iio: temperature: add ADI MAX30210 driver

[Jonathan Cameron]

On Fri, 27 Feb 2026 00:30:41 +0800
John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com> wrote:

> MAX30210 ±0.1°C Accurate Ultra-Small Low-Power Digital Temperature Sensor

For a temperature sensor, this description doesn't give enough detail
on why you are proposing an IIO driver rather than a hwmon one.

Please add more for v2.  Focus on what the part is and features you are supporting
that don't have a path to being supporting in hwmon. E.g. the fifo.

The other thing to add a brief note on is why this support cannot be easily
added to an existing driver.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
Various additional comments inline from me.

Welcome to IIO!

Jonathan


> diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
> new file mode 100644
> index 000000000000..aaa3a26be131
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,758 @@

> +
> +#define MAX30210_STATUS_REG		0x00
> +#define MAX30210_INT_EN_REG		0x02
> +#define MAX30210_FIFO_DATA_REG		0x08
> +#define MAX30210_FIFO_CONF_1_REG	0x09
> +#define MAX30210_FIFO_CONF_2_REG	0x0A
> +#define MAX30210_SYS_CONF_REG		0x11
> +#define MAX30210_PIN_CONF_REG		0x12
> +#define MAX30210_TEMP_ALM_HI_REG	0x22
> +#define MAX30210_TEMP_ALM_LO_REG	0x24
> +#define MAX30210_TEMP_INC_THRESH_REG	0x26
> +#define MAX30210_TEMP_DEC_THRESH_REG	0x27
> +#define MAX30210_TEMP_CONF_1_REG	0x28
> +#define MAX30210_TEMP_CONF_2_REG	0x29
> +#define MAX30210_TEMP_CONV_REG		0x2A
> +#define MAX30210_TEMP_DATA_REG		0x2B
> +#define MAX30210_TEMP_SLOPE_REG		0x2D
> +#define MAX30210_UNIQUE_ID_REG		0x30
> +#define MAX30210_PART_ID_REG		0xFF
> +
> +#define MAX30210_A_FULL_MASK   BIT(7)

I'm very keen that field names reflect which register they are in.
That makes it much easier to review whether they are being correctly
used.  Usual way to do that is have a prefix that incorporates enough 
of the register name to work out that mapping.

> +#define MAX30210_TEMP_RDY_MASK BIT(6)
> +#define MAX30210_TEMP_DEC_MASK BIT(5)
> +#define MAX30210_TEMP_INC_MASK BIT(4)
> +#define MAX30210_TEMP_LO_MASK  BIT(3)
> +#define MAX30210_TEMP_HI_MASK  BIT(2)
> +#define MAX30210_PWR_RDY_MASK  BIT(0)
> +
> +#define MAX30210_FLUSH_FIFO_MASK BIT(4)
> +
> +#define MAX30210_EXT_CNV_EN_MASK	BIT(7)
> +#define MAX30210_EXT_CVT_ICFG_MASK	BIT(6)
> +#define MAX30210_INT_FCFG_MASK		GENMASK(3, 2)
> +#define MAX30210_INT_OCFG_MASK		GENMASK(1, 0)
> +
> +#define MAX30210_CHG_DET_EN_MASK	BIT(3)
> +#define MAX30210_RATE_CHG_FILTER_MASK	GENMASK(2, 0)
> +
> +#define MAX30210_TEMP_PERIOD_MASK	GENMASK(3, 0)
> +#define MAX30210_ALERT_MODE_MASK	BIT(7)
> +
> +#define MAX30210_AUTO_MASK	BIT(1)
> +#define MAX30210_CONV_T_MASK	BIT(0)
> +
> +#define MAX30210_PART_ID		0x45
> +#define MAX30210_FIFO_SIZE		64
> +#define MAX30210_FIFO_INVAL_DATA	GENMASK(23, 0)
> +#define MAX30210_WATERMARK_DEFAULT	(0x40 - 0x1F)
> +
> +#define MAX30210_INT_EN(state, mask)	((state) ? (mask) : 0x0)
Use regmap_assign_bits() to replace this macro.


> +
> +struct max30210_state {
> +	/*
> +	 * Prevent simultaneous access to the i2c client.
Why does that matter?  I'd imagine you have some read / modify write
sequences or need to not change the mode whilst something else is going
on?

> +	 */
> +	struct mutex lock;
> +	struct regmap *regmap;
> +	struct iio_trigger *trig;
> +	struct gpio_desc *powerdown_gpio;
> +	u8 watermark;
> +	u8 data[3 * MAX30210_FIFO_SIZE]  __aligned(IIO_DMA_MINALIGN);
> +};

> +static int max30210_read_temp(struct regmap *regmap, unsigned int reg,
> +			      int *temp)
> +{
> +	u8 uval[2] __aligned(IIO_DMA_MINALIGN);

As below, forcing alignment on the stack doesn't work for this purpose.
Needs to be on the heap. Put it next to data in the _state structure.

However, this is an i2c driver. I2C doesn't have any such requirements
on buffer alignment because it always bounce buffers data if needed.


> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, reg, uval, 2);
> +	if (ret)
> +		return ret;
> +
> +	*temp = sign_extend32(get_unaligned_be16(uval), 15);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int max30210_write_temp(struct regmap *regmap, unsigned int reg,
> +			       int temp)
> +{
> +	u8 uval[2] __aligned(IIO_DMA_MINALIGN);

You've miss understood the purpose of IIO_DMA_MINALIGN.
Make sure to look into that but the short answer is you can't do it on the stack.

> +
> +	put_unaligned_be16(temp, uval);
> +
> +	return regmap_bulk_write(regmap, reg, uval, 2);
Use a __be16 type for the buffer and then sizeof() for the 2

> +}
> +
> +static void max30210_fifo_read(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	u32 samp;
> +	int ret, i, j;
> +
> +	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
> +			       st->data, 3 * st->watermark);
> +	if (ret < 0)
> +		return dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
> +
> +	for (i = 0; i < st->watermark; i++) {

		u32 samp = 0;

> +		samp = 0;
> +
> +		for (j = 0; j < 3; j++) {
> +			samp <<= 8;
> +			samp |= st->data[3 * i + j];

Looks like get_unaligned_be24() or similar. Use that instead of opencoding
the endian conversion.  However, you are claiming this is a big endian channel
and this is an endian conversion so something is wrong.  If you keep it as
a big endian channel, this probably wants to be simply memcpy()




> +		}
> +
> +		if (samp == MAX30210_FIFO_INVAL_DATA) {
> +			dev_err(&indio_dev->dev, "Invalid data\n");
> +			continue;
> +		}
> +
> +		iio_push_to_buffers(indio_dev, &samp);
> +	}
> +}
> +
> +static irqreturn_t max30210_trigger_handler(int irq, void *p)
> +{
> +	struct iio_poll_func *pf = p;
> +	struct iio_dev *indio_dev = pf->indio_dev;
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int status;
> +	int ret;
> +
> +	guard(mutex)(&st->lock);
> +
> +	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Status byte read error\n");
> +		goto exit_irq;
> +	}
> +
> +	if (status & MAX30210_PWR_RDY_MASK) {
> +		dev_info(&indio_dev->dev, "power-on\n");

dev_dbg() at most.

> +		st->watermark = MAX30210_WATERMARK_DEFAULT;
> +	}
> +
> +	if (status & MAX30210_A_FULL_MASK)
> +		max30210_fifo_read(indio_dev);
> +
> +	if (status & MAX30210_TEMP_HI_MASK)
This is unusual.  If you have a single interrupt for both trigger
and thresholds then you can't use iio_trigger_generic_data_rdy_poll()

The main interrupt handler needs to work out what has happened then
ultimately use iio_trigger_poll_nested() to fire of the data capture.

However, there is a hardware fifo going on here. So what benefit
is the trigger providing?  Triggers are optional and often not appropriate
when there are hardware fifos present because it is not useful to use
them to capture data from other devices etc.  So just drop the trigger
here and have this as the main irq handler.

> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_RISING),
> +			       iio_get_time_ns(indio_dev));
> +
> +	if (status & MAX30210_TEMP_LO_MASK)
> +		iio_push_event(indio_dev,
> +			       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
> +						    IIO_EV_TYPE_THRESH,
> +						    IIO_EV_DIR_FALLING),
> +			       iio_get_time_ns(indio_dev));
> +
> +exit_irq:

Whilst this is not buggy the advice (see cleanup.h comments) is never
combine gotos and guard() / __free() in one function. There are some evil corner
case and GCC at least doesn't catch them all.  Various ways to refactor
the code to avoid the mix.

> +	iio_trigger_notify_done(indio_dev->trig);
> +	return IRQ_HANDLED;
> +}

> +
> +static int max30210_validate_trigger(struct iio_dev *indio_dev,
> +				     struct iio_trigger *trig)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (st->trig != trig)
> +		return -EINVAL;
Can you use iio_validate_own_trigger()?
They should both have the same parent.

> +
> +	return 0;

> +static int max30210_write_event(struct iio_dev *indio_dev,
> +				const struct iio_chan_spec *chan,
> +				enum iio_event_type type,
> +				enum iio_event_direction dir,
> +				enum iio_event_info info, int val, int val2)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	if (info == IIO_EV_INFO_VALUE) {
Can flip the logic to reduce indent.
	if (info != IIO_EV_INFO_VALUE)
		return -EINVAL;

	switch()

> +		switch (dir) {
> +		case IIO_EV_DIR_RISING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_write_temp(st->regmap,
> +							   MAX30210_TEMP_ALM_HI_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;

As below (check for other instances of this)

> +		case IIO_EV_DIR_FALLING:
> +			switch (type) {
> +			case IIO_EV_TYPE_THRESH:
> +				return max30210_write_temp(st->regmap,
> +							   MAX30210_TEMP_ALM_LO_REG, val);
> +			default:
> +				return -EINVAL;
> +			}
> +			break;

Can't get here so drop the break.

> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}

> +
> +static int max30210_read_raw(struct iio_dev *indio_dev,
> +			     struct iio_chan_spec const *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int uval;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 5;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_read(st->regmap, MAX30210_TEMP_CONF_2_REG, &uval);
> +		if (ret)
> +			return ret;
> +
> +		uval = FIELD_GET(MAX30210_TEMP_PERIOD_MASK, uval);
> +
> +		*val = 8;
> +
> +		/**

/* 
See below and run a W=1 build which will probably complain about this.

> +		 * register values 0x9 or above have the same sample
> +		 * rate of 8Hz
> +		 */
> +		*val2 = uval >= 0x9 ? 1 : BIT(0x9 - uval);
> +
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_device_claim_direct_mode(indio_dev);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +				   MAX30210_CONV_T_MASK);
> +		if (ret)
> +			goto release_dmode;
> +
> +		fsleep(8000);

Add a spec reference for this.

> +
> +		ret = max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +
> +release_dmode:

Labels for gotos inside a switch are not a good thing to do for readability.
We have the new ACQUIRE() stuff that David mentioned, but if that isn't appropriate
I'd suggest factoring out some of the code here so you can avoid the goto.

> +		iio_device_release_direct_mode(indio_dev);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +}
> +
> +static int max30210_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan, int val,
> +			      int val2, long mask)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	u64 data;
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = iio_device_claim_direct_mode(indio_dev);

Others pointed out this needs to be based on upstream where that
particular function has gone away.

> +		if (ret)
> +			return ret;
> +
> +		/**
Not kernel-doc style so /*

> +		 * micro_value = val * 1000000 + val2
> +		 * reg_value = ((micro_value * 64) / 1000000) - 1

Except that's not the reg_val, because you then call fls() on it.

> +		 */
> +		data = (val * MICRO + val2) << 6;
> +		do_div(data, MICRO);
> +
> +		data = fls_long(data - 1);

This is unusual enough I'd add a comment on the maths.

> +		data = FIELD_PREP(MAX30210_TEMP_PERIOD_MASK, data);

Use a different variable of the appropriate type to store the result.
That way no need for a cast below.

> +
> +		ret = regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
> +					 MAX30210_TEMP_PERIOD_MASK,
> +					 (unsigned int)data);
> +
> +		iio_device_release_direct_mode(indio_dev);
> +		return ret;
> +	default:
> +		return -EINVAL;
> +	}
> +}


> +
> +static const struct iio_dev_attr *max30210_fifo_attributes[] = {
> +	&iio_dev_attr_hwfifo_watermark_min,
> +	&iio_dev_attr_hwfifo_watermark_max,
> +	&iio_dev_attr_hwfifo_watermark,
> +	NULL,

No comma for a null terminator.  Doesn't add anything useful and
makes it easier to put things after this (which is obviously a bug).

> +};
> +
> +static int max30210_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_A_FULL_MASK, MAX30210_A_FULL_MASK);

regmap_set_bits() just avoids repeating the mask.


> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +				 MAX30210_FLUSH_FIFO_MASK,
> +				 MAX30210_FLUSH_FIFO_MASK);

set bits is fine here.  I assume it auto-clears?


> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +			   MAX30210_AUTO_MASK | MAX30210_CONV_T_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

return regmap_write()

> +}
> +
> +static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_A_FULL_MASK, 0x0);
Tiny bit simpler as
	ret = regmap_clear_bits(st->regmap, MAX30210_INT_EN_REG,
				MAX30210_A_FULL_MASK);

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +				 MAX30210_FLUSH_FIFO_MASK,
> +				 MAX30210_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);

return regmap_write();

I'm a bit surprised by the ordering in here (I haven't looked at the datasheet).
Mostly we'd expect the tear down of settings to be the reverse of setup. Here
that probably means that final write belongs before the flushing of the fifo.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static const struct iio_buffer_setup_ops max30210_buffer_ops = {
> +	.preenable = max30210_buffer_preenable,
> +	.postdisable = max30210_buffer_postdisable,
> +};
> +
> +static const struct iio_info max30210_info = {
> +	.read_raw = max30210_read_raw,
> +	.read_avail = max30210_read_avail,
> +	.write_raw = max30210_write_raw,
> +	.write_raw_get_fmt = max30210_write_raw_get_fmt,
> +	.hwfifo_set_watermark = max30210_set_watermark,
> +	.debugfs_reg_access = &max30210_reg_access,

Why & for some function pointers and not others?

> +	.validate_trigger = &max30210_validate_trigger,
> +	.read_event_value = max30210_read_event,
> +	.write_event_value = max30210_write_event,
> +	.write_event_config = max30210_write_event_config,
> +	.read_event_config = max30210_read_event_config,
> +};

> +
> +static const struct iio_chan_spec max30210_channels = {
> +	.type = IIO_TEMP,
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +			      BIT(IIO_CHAN_INFO_SCALE) |
> +			      BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	.info_mask_separate_available = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> +	.output = 0,

0 is a natural default for a boolean type thing like .output, so
no need to specify it. Let the C spec guarantees around zeroing all
fields deal with it for you.

> +	.scan_index = 0,
> +	.event_spec = max30210_events,
> +	.num_event_specs = ARRAY_SIZE(max30210_events),
> +	.scan_type = {
> +		.sign = 's',
> +		.realbits = 16,
> +		.storagebits = 32,
> +		.shift = 8,
> +		.endianness = IIO_BE,
> +	},
> +};

> +static int max30210_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->dev;
> +	struct iio_dev *indio_dev;
> +	struct max30210_state *st;
> +	int ret;
> +
> +	if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +		return -EOPNOTSUPP;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	st = iio_priv(indio_dev);
> +
> +	mutex_init(&st->lock);

	ret = devm_mutex_init(&st->lock);
	if (ret)
		return ret;

Look at what that does.  We don't care a lot about lock lifetime debugging
but given it's easy to do, why not enable it for the driver.


> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to enable vdd regulator.\n");
> +
> +	st->regmap = devm_regmap_init_i2c(client, &max30210_regmap);
> +	if (IS_ERR(st->regmap))
> +		return dev_err_probe(dev, PTR_ERR(st->regmap),
> +				     "Failed to allocate regmap.\n");
> +
> +	ret = max30210_setup(st, dev);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = &max30210_channels;
> +	indio_dev->num_channels = 1;
> +	indio_dev->name = "max30210";
> +	indio_dev->info = &max30210_info;
> +
> +	ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
> +						  max30210_trigger_handler,
> +						  IIO_BUFFER_DIRECTION_IN,
> +						  &max30210_buffer_ops,
> +						  max30210_fifo_attributes);
> +	if (ret < 0)
> +		return ret;

For consistency, if (ret) should be fine here.  I don't think any
IIO core calls return positive integers.

> +
> +	if (client->irq) {
> +		st->trig = devm_iio_trigger_alloc(dev, "%s-dev%d",
> +						  indio_dev->name,
> +						  iio_device_id(indio_dev));
> +		if (!st->trig)
> +			return -ENOMEM;
> +
> +		st->trig->ops = &max30210_trigger_ops;
> +		iio_trigger_set_drvdata(st->trig, indio_dev);
> +		ret = devm_iio_trigger_register(dev, st->trig);
> +		if (ret)
> +			return ret;
> +
> +		indio_dev->trig = st->trig;
> +		ret = devm_request_threaded_irq(dev, client->irq,
> +						iio_trigger_generic_data_rdy_poll,
> +						NULL, IRQF_TRIGGER_FALLING,

So we have an unfortunate history of drivers that specify the interrupt
polarity that we can't fix, but for new drivers, that is a job for firmware
not the driver (because there may be an inverter in the path or similar).
Hence most likely flags here should be IRQF_NO_THREAD.

There is no thread, so devm_request_irq() is enough.
However, note that iio_trigger_generic_data_ready_poll() is ultimately kicking
of activity on an interrupt chip (software one) that is buried in the
IIO core. That can't be done correctly from a thread.  Hence the IRQF_NO_THREAD.

> +						indio_dev->name, st->trig);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return 0;

	return devm_iio_device_register();

> +}
> +
> +static const struct i2c_device_id max30210_id[] = {
> +	{ "max30210", 0 },
No point in the 0, so  just
	{ "max30210" },

> +	{ }
> +};
> +MODULE_DEVICE_TABLE(i2c, max30210_id);