public inbox for linux-iio@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add support for Analog Devices MAX30210
@ 2026-03-04 12:25 John Erasmus Mari Geronimo
  2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: John Erasmus Mari Geronimo @ 2026-03-04 12:25 UTC (permalink / raw)
  To: linux-iio, jic23; +Cc: dlechner, nuno.sa, andy, linux-kernel

This series adds support for the Analog Devices MAX30210 I2C
temperature sensor.

The device provides a 16-bit temperature output and includes
features such as a hardware FIFO, programmable sampling rate,
threshold alarms, and interrupt support. These capabilities map
naturally to the IIO subsystem, particularly the buffered interface
and event framework.

Patch 1 adds the devicetree binding.
Patch 2 adds the IIO driver implementation.

Changes in v2:
- Improve commit message to better describe device features and driver
  support.
- Simplify devicetree binding description and clean up property
  descriptions.
- Add MAINTAINERS entry for MAX30210 driver.
- Rename register field macros to include register prefix for clarity.
- Remove incorrect IIO_DMA_MINALIGN usage and switch to proper __be16
  handling.
- Replace manual FIFO byte assembly with get_unaligned_be24().
- Use dev_err_ratelimited() for invalid FIFO data messages.
- Simplify event read/write handlers by reducing nested conditionals.
- Fix write_event_config prototype to use bool state.
- Replace MAX30210_INT_EN macro with regmap_assign_bits().
- Replace deprecated iio_device_claim_direct_mode() usage with
  IIO_DEV_ACQUIRE_DIRECT_MODE().
- Implement conversion completion polling using regmap_read_poll_timeout()
  instead of fixed delay.
- Rework sampling frequency handling using lookup table instead of
  bit math.
- Remove trigger infrastructure and switch to IRQ-driven FIFO handling.
- Replace triggered buffer with kfifo buffer implementation.
- Use regmap_set_bits() and regmap_clear_bits() helpers where appropriate.
- Improve FIFO buffer handling and push 16-bit temperature samples.
- Simplify buffer enable/disable paths.
- Remove unnecessary fields and clean up state structure.
- Add optional hardware reset via powerdown GPIO with software reset
  fallback.
- Adjust scan type to match 16-bit temperature samples.
- Miscellaneous code cleanups and style fixes.

John Erasmus Mari Geronimo (2):
  dt-bindings: iio: temperature: add ADI MAX30210
  iio: temperature: add support for Analog Devices MAX30210

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


base-commit: 70a9ae59c5b1f2f5501e78e2d85bfeefd153f854
-- 
2.34.1


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

* [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210
  2026-03-04 12:25 [PATCH v2 0/2] Add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
@ 2026-03-04 12:25 ` John Erasmus Mari Geronimo
  2026-03-05  0:11   ` David Lechner
  2026-03-05  7:00   ` Krzysztof Kozlowski
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
  2026-03-04 13:42 ` [PATCH v2 0/2] Add " Andy Shevchenko
  2 siblings, 2 replies; 11+ messages in thread
From: John Erasmus Mari Geronimo @ 2026-03-04 12:25 UTC (permalink / raw)
  To: linux-iio, jic23; +Cc: dlechner, nuno.sa, andy, linux-kernel

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         | 62 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 69 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..66867880a20f
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
@@ -0,0 +1,62 @@
+# 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 Temperature Sensor
+
+maintainers:
+  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
+
+description: |
+  The MAX30210 is a temperature sensor with an I2C interface.
+  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.
+
+  powerdown-gpios:
+    description: GPIO connected to the CVT/PDB pin (active low).
+    maxItems: 1
+
+  interrupts:
+    description: Connected to INT pin. Interrupt triggered on both rising and falling edges.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        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>;
+        };
+    };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 1c75276404df..09345b9f32ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1638,6 +1638,13 @@ 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
+
 ANALOG DEVICES INC ADA4250 DRIVER
 M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
 L:	linux-iio@vger.kernel.org
-- 
2.34.1


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

* [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
  2026-03-04 12:25 [PATCH v2 0/2] Add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
  2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
@ 2026-03-04 12:25 ` John Erasmus Mari Geronimo
  2026-03-05  0:34   ` kernel test robot
                     ` (3 more replies)
  2026-03-04 13:42 ` [PATCH v2 0/2] Add " Andy Shevchenko
  2 siblings, 4 replies; 11+ messages in thread
From: John Erasmus Mari Geronimo @ 2026-03-04 12:25 UTC (permalink / raw)
  To: linux-iio, jic23; +Cc: dlechner, nuno.sa, andy, linux-kernel

Add support for the Analog Devices MAX30210 I2C temperature
sensor.

The driver uses regmap for register access and integrates with
the IIO framework. It supports:

- Direct mode temperature conversion
- Configurable sampling frequency
- Threshold events
- FIFO operation with IIO kfifo buffer support
- Optional interrupt-driven data ready signaling

The device provides 16-bit signed temperature data and a
64-sample FIFO.

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

diff --git a/MAINTAINERS b/MAINTAINERS
index 09345b9f32ed..2abdbcf3a8e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1644,6 +1644,7 @@ 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>
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 9328b2250ace..b396757eb761 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 "Analog Devices MAX30210 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..839ed9830957
--- /dev/null
+++ b/drivers/iio/temperature/max30210.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices MAX30210 I2C Temperature Sensor driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#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/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
+#include <linux/regmap.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_STATUS_A_FULL_MASK   BIT(7)
+#define MAX30210_STATUS_TEMP_RDY_MASK BIT(6)
+#define MAX30210_STATUS_TEMP_DEC_MASK BIT(5)
+#define MAX30210_STATUS_TEMP_INC_MASK BIT(4)
+#define MAX30210_STATUS_TEMP_LO_MASK  BIT(3)
+#define MAX30210_STATUS_TEMP_HI_MASK  BIT(2)
+#define MAX30210_STATUS_PWR_RDY_MASK  BIT(0)
+
+#define MAX30210_FIFOCONF1_FLUSH_FIFO_MASK BIT(4)
+
+#define MAX30210_SYSCONF_RESET_MASK		BIT(0)
+
+#define MAX30210_PINCONF_EXT_CNV_EN_MASK	BIT(7)
+#define MAX30210_PINCONF_EXT_CVT_ICFG_MASK	BIT(6)
+#define MAX30210_PINCONF_INT_FCFG_MASK		GENMASK(3, 2)
+#define MAX30210_PINCONF_INT_OCFG_MASK		GENMASK(1, 0)
+
+#define MAX30210_TEMPCONF1_CHG_DET_EN_MASK	BIT(3)
+#define MAX30210_TEMPCONF1_RATE_CHG_FILTER_MASK	GENMASK(2, 0)
+
+#define MAX30210_TEMPCONF2_TEMP_PERIOD_MASK	GENMASK(3, 0)
+#define MAX30210_TEMPCONF2_ALERT_MODE_MASK	BIT(7)
+
+#define MAX30210_TEMPCONV_AUTO_MASK	BIT(1)
+#define MAX30210_TEMPCONV_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_UNIQUE_ID_LEN		6
+#define MAX30210_EXT_CVT_FREQ_MIN	1
+#define MAX30210_EXT_CVT_FREQ_MAX	20
+
+struct max30210_state {
+	struct regmap *regmap;
+	u8 watermark;
+	/* Raw FIFO byte buffer (3 bytes per sample) */
+	u8 fifo_buf[3 * MAX30210_FIFO_SIZE];
+};
+
+static const int max30210_samp_freq_avail[][2] = {
+	{ 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)
+{
+	__be16 val;
+	int ret;
+
+	ret = regmap_bulk_read(regmap, reg, &val, sizeof(val));
+	if (ret)
+		return ret;
+
+	*temp = sign_extend32(be16_to_cpu(val), 15);
+
+	return IIO_VAL_INT;
+}
+
+static int max30210_write_temp(struct regmap *regmap, unsigned int reg,
+			       int temp)
+{
+	__be16 val;
+
+	val = cpu_to_be16(temp);
+
+	return regmap_bulk_write(regmap, reg, &val, sizeof(val));
+}
+
+static void max30210_fifo_read(struct iio_dev *indio_dev)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
+			       st->fifo_buf, 3 * st->watermark);
+	if (ret) {
+		dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
+		return;
+	}
+
+	for (unsigned int i = 0; i < st->watermark; i++) {
+		u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
+
+		if (raw == MAX30210_FIFO_INVAL_DATA) {
+			dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
+			continue;
+		}
+
+		s16 temp = (s16)(raw & 0xFFFF);
+
+		iio_push_to_buffers(indio_dev, &temp);
+	}
+}
+
+static irqreturn_t max30210_irq_handler(int irq, void *dev_id)
+{
+	struct iio_dev *indio_dev = dev_id;
+	struct max30210_state *st = iio_priv(indio_dev);
+	unsigned int status;
+	int ret;
+
+	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
+	if (ret) {
+		dev_err(&indio_dev->dev, "Status read failed\n");
+		return IRQ_NONE;
+	}
+
+	if (status & MAX30210_STATUS_A_FULL_MASK)
+		max30210_fifo_read(indio_dev);
+
+	if (status & MAX30210_STATUS_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_STATUS_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));
+
+	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_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)
+		return -EINVAL;
+
+	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;
+		}
+	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;
+		}
+	default:
+		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)
+		return -EINVAL;
+
+	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;
+		}
+	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;
+		}
+	default:
+		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_STATUS_TEMP_HI_MASK, val);
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_DIR_FALLING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			return FIELD_GET(MAX30210_STATUS_TEMP_LO_MASK, val);
+		default:
+			return -EINVAL;
+		}
+	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, bool state)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+
+	switch (dir) {
+	case IIO_EV_DIR_RISING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			return regmap_assign_bits(st->regmap, MAX30210_INT_EN_REG,
+						  MAX30210_STATUS_TEMP_HI_MASK, state);
+		default:
+			return -EINVAL;
+		}
+	case IIO_EV_DIR_FALLING:
+		switch (type) {
+		case IIO_EV_TYPE_THRESH:
+			return regmap_assign_bits(st->regmap, MAX30210_INT_EN_REG,
+						  MAX30210_STATUS_TEMP_LO_MASK, state);
+		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;
+		*val2 = 1000;
+
+		return IIO_VAL_FRACTIONAL;
+	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_TEMPCONF2_TEMP_PERIOD_MASK, uval);
+
+		if (uval >= ARRAY_SIZE(max30210_samp_freq_avail))
+			uval = ARRAY_SIZE(max30210_samp_freq_avail) - 1;
+
+		*val = max30210_samp_freq_avail[uval][0];
+		*val2 = max30210_samp_freq_avail[uval][1];
+
+		return IIO_VAL_FRACTIONAL;
+	case IIO_CHAN_INFO_RAW: {
+		if (iio_buffer_enabled(indio_dev))
+			return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
+
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
+		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
+				   MAX30210_TEMPCONV_CONV_T_MASK);
+		if (ret)
+			return ret;
+
+		/*
+		 * Wait until CONVERT_T auto-clears.
+		 * Datasheet:
+		 *   tBIAS_WU = 260 µs
+		 *   tINT     = 8 ms
+		 *
+		 * Worst-case conversion ≈ 8.26 ms.
+		 * Use 10 ms timeout for margin.
+		 */
+		ret = regmap_read_poll_timeout(st->regmap, MAX30210_TEMP_CONV_REG, uval,
+					       !(uval & MAX30210_TEMPCONV_CONV_T_MASK),
+					       500,          /* poll every 500 µs */
+					       10000);       /* 10 ms timeout */
+		if (ret)
+			return ret;
+
+		return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
+	}
+	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 = &max30210_samp_freq_avail[0][0];
+		*type = IIO_VAL_INT_PLUS_MICRO;
+		*length = ARRAY_SIZE(max30210_samp_freq_avail) * 2;
+
+		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);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ: {
+		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
+		if (IIO_DEV_ACQUIRE_FAILED(claim))
+			return -EBUSY;
+
+		if (val < 0 || val2 < 0)
+			return -EINVAL;
+
+		for (unsigned int i = 0; i < ARRAY_SIZE(max30210_samp_freq_avail); i++) {
+			if (val == max30210_samp_freq_avail[i][0] &&
+			    val2 == max30210_samp_freq_avail[i][1]) {
+				return regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
+						MAX30210_TEMPCONF2_TEMP_PERIOD_MASK,
+						FIELD_PREP(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, i));
+			}
+		}
+
+		return -EINVAL;
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+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_set_bits(st->regmap, MAX30210_INT_EN_REG, MAX30210_STATUS_A_FULL_MASK);
+	if (ret)
+		return ret;
+
+	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	return regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
+			    MAX30210_TEMPCONV_AUTO_MASK | MAX30210_TEMPCONV_CONV_T_MASK);
+}
+
+static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
+{
+	struct max30210_state *st = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
+	if (ret)
+		return ret;
+
+	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	return regmap_clear_bits(st->regmap, MAX30210_INT_EN_REG,
+				 MAX30210_STATUS_A_FULL_MASK);
+}
+
+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,
+	.hwfifo_set_watermark = max30210_set_watermark,
+	.debugfs_reg_access = max30210_reg_access,
+	.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),
+	.scan_index = 0,
+	.event_spec = max30210_events,
+	.num_event_specs = ARRAY_SIZE(max30210_events),
+	.scan_type = {
+		.sign = 's',
+		.realbits = 16,
+		.storagebits = 16,
+		.shift = 0,
+		.endianness = IIO_CPU,
+	},
+};
+
+static int max30210_setup(struct max30210_state *st, struct device *dev)
+{
+	struct gpio_desc *powerdown_gpio;
+	unsigned int val;
+	int ret;
+
+	/* Optional hardware reset via powerdown GPIO */
+	powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
+						 GPIOD_OUT_HIGH);
+	if (IS_ERR(powerdown_gpio))
+		return dev_err_probe(dev, PTR_ERR(powerdown_gpio),
+				     "failed to request powerdown GPIO\n");
+
+	if (powerdown_gpio) {
+		/* Deassert powerdown to power up device */
+		gpiod_set_value(powerdown_gpio, 0);
+	} else {
+		/* Software reset fallback */
+		ret = regmap_update_bits(st->regmap, MAX30210_SYS_CONF_REG,
+					 MAX30210_SYSCONF_RESET_MASK,
+					 MAX30210_SYSCONF_RESET_MASK);
+		if (ret)
+			return ret;
+	}
+
+	/* Datasheet Figure 6:
+	 * tPU max = 700 µs after power-up or reset before device is ready.
+	 */
+	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);
+
+	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");
+
+	st->watermark = MAX30210_WATERMARK_DEFAULT;
+
+	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_kfifo_buffer_setup_ext(dev, indio_dev, &max30210_buffer_ops,
+					      max30210_fifo_attributes);
+	if (ret)
+		return ret;
+
+	if (client->irq) {
+		ret = devm_request_irq(dev, client->irq, max30210_irq_handler, IRQF_NO_THREAD,
+				       indio_dev->name, indio_dev);
+		if (ret)
+			return ret;
+	}
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct i2c_device_id max30210_id[] = {
+	{ "max30210" },
+	{ }
+};
+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


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

* Re: [PATCH v2 0/2] Add support for Analog Devices MAX30210
  2026-03-04 12:25 [PATCH v2 0/2] Add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
  2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
@ 2026-03-04 13:42 ` Andy Shevchenko
  2 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2026-03-04 13:42 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo
  Cc: linux-iio, jic23, dlechner, nuno.sa, andy, linux-kernel

On Wed, Mar 04, 2026 at 08:25:07PM +0800, John Erasmus Mari Geronimo wrote:
> This series adds support for the Analog Devices MAX30210 I2C
> temperature sensor.
> 
> The device provides a 16-bit temperature output and includes
> features such as a hardware FIFO, programmable sampling rate,
> threshold alarms, and interrupt support. These capabilities map
> naturally to the IIO subsystem, particularly the buffered interface
> and event framework.

Neither cover letter, nor the code patch describes why we need a brand new
driver. The folder has, for example, max30208.c which might be considered
(no, I haven't checked myself).

-- 
With Best Regards,
Andy Shevchenko



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

* Re: [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210
  2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
@ 2026-03-05  0:11   ` David Lechner
  2026-03-07 12:21     ` Jonathan Cameron
  2026-03-05  7:00   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 11+ messages in thread
From: David Lechner @ 2026-03-05  0:11 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo, linux-iio, jic23; +Cc: nuno.sa, andy, linux-kernel

On 3/4/26 6:25 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         | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 69 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..66867880a20f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> @@ -0,0 +1,62 @@
> +# 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 Temperature Sensor
> +
> +maintainers:
> +  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> +
> +description: |
> +  The MAX30210 is a temperature sensor with an I2C interface.
> +  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.

The description makes it sound like there could be other supplies,
but there aren't. It is the "everything" supply, so we can just
call it the power supply.

> +
> +  powerdown-gpios:
> +    description: GPIO connected to the CVT/PDB pin (active low).
> +    maxItems: 1
> +
> +  interrupts:
> +    description: Connected to INT pin. Interrupt triggered on both rising and falling edges.
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        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>;
> +        };
> +    };
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1c75276404df..09345b9f32ed 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1638,6 +1638,13 @@ W:	https://ez.analog.com/linux-software-drivers
>  F:	Documentation/devicetree/bindings/iio/dac/adi,max22007.yaml
>  F:	drivers/iio/dac/max22007.c

It looks like MAX22007 is not in alphabetical order. Let's fix it
first so we can put this new one in the right place.

>  
> +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
> +
>  ANALOG DEVICES INC ADA4250 DRIVER
>  M:	Antoniu Miclaus <antoniu.miclaus@analog.com>
>  L:	linux-iio@vger.kernel.org


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

* Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
@ 2026-03-05  0:34   ` kernel test robot
  2026-03-05  0:45   ` kernel test robot
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2026-03-05  0:34 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo, linux-iio, jic23
  Cc: llvm, oe-kbuild-all, dlechner, nuno.sa, andy, linux-kernel

Hi John,

kernel test robot noticed the following build errors:

[auto build test ERROR on 70a9ae59c5b1f2f5501e78e2d85bfeefd153f854]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Erasmus-Mari-Geronimo/dt-bindings-iio-temperature-add-ADI-MAX30210/20260304-202920
base:   70a9ae59c5b1f2f5501e78e2d85bfeefd153f854
patch link:    https://lore.kernel.org/r/20260304122509.67931-3-johnerasmusmari.geronimo%40analog.com
patch subject: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
config: sparc64-allmodconfig (https://download.01.org/0day-ci/archive/20260305/202603050819.rN8MJLQm-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/20260305/202603050819.rN8MJLQm-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/202603050819.rN8MJLQm-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/iio/temperature/max30210.c:136:13: error: call to undeclared function 'get_unaligned_be24'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     136 |                 u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
         |                           ^
>> drivers/iio/temperature/max30210.c:272:11: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     272 |                         return FIELD_GET(MAX30210_STATUS_TEMP_HI_MASK, val);
         |                                ^
   drivers/iio/temperature/max30210.c:279:11: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     279 |                         return FIELD_GET(MAX30210_STATUS_TEMP_LO_MASK, val);
         |                                ^
   drivers/iio/temperature/max30210.c:336:10: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     336 |                 uval = FIELD_GET(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, uval);
         |                        ^
>> drivers/iio/temperature/max30210.c:418:7: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     418 |                                                 FIELD_PREP(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, i));
         |                                                 ^
   5 errors generated.


vim +/get_unaligned_be24 +136 drivers/iio/temperature/max30210.c

   122	
   123	static void max30210_fifo_read(struct iio_dev *indio_dev)
   124	{
   125		struct max30210_state *st = iio_priv(indio_dev);
   126		int ret;
   127	
   128		ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
   129				       st->fifo_buf, 3 * st->watermark);
   130		if (ret) {
   131			dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
   132			return;
   133		}
   134	
   135		for (unsigned int i = 0; i < st->watermark; i++) {
 > 136			u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
   137	
   138			if (raw == MAX30210_FIFO_INVAL_DATA) {
   139				dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
   140				continue;
   141			}
   142	
   143			s16 temp = (s16)(raw & 0xFFFF);
   144	
   145			iio_push_to_buffers(indio_dev, &temp);
   146		}
   147	}
   148	
   149	static irqreturn_t max30210_irq_handler(int irq, void *dev_id)
   150	{
   151		struct iio_dev *indio_dev = dev_id;
   152		struct max30210_state *st = iio_priv(indio_dev);
   153		unsigned int status;
   154		int ret;
   155	
   156		ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
   157		if (ret) {
   158			dev_err(&indio_dev->dev, "Status read failed\n");
   159			return IRQ_NONE;
   160		}
   161	
   162		if (status & MAX30210_STATUS_A_FULL_MASK)
   163			max30210_fifo_read(indio_dev);
   164	
   165		if (status & MAX30210_STATUS_TEMP_HI_MASK)
   166			iio_push_event(indio_dev,
   167				       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
   168							    IIO_EV_TYPE_THRESH,
   169							    IIO_EV_DIR_RISING),
   170				       iio_get_time_ns(indio_dev));
   171	
   172		if (status & MAX30210_STATUS_TEMP_LO_MASK)
   173			iio_push_event(indio_dev,
   174				       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
   175							    IIO_EV_TYPE_THRESH,
   176							    IIO_EV_DIR_FALLING),
   177				       iio_get_time_ns(indio_dev));
   178	
   179		return IRQ_HANDLED;
   180	}
   181	
   182	static int max30210_reg_access(struct iio_dev *indio_dev, unsigned int reg,
   183				       unsigned int writeval, unsigned int *readval)
   184	{
   185		struct max30210_state *st = iio_priv(indio_dev);
   186	
   187		if (!readval)
   188			return regmap_write(st->regmap, reg, writeval);
   189	
   190		return regmap_read(st->regmap, reg, readval);
   191	}
   192	
   193	static int max30210_read_event(struct iio_dev *indio_dev,
   194				       const struct iio_chan_spec *chan,
   195				       enum iio_event_type type,
   196				       enum iio_event_direction dir,
   197				       enum iio_event_info info, int *val, int *val2)
   198	{
   199		struct max30210_state *st = iio_priv(indio_dev);
   200	
   201		if (info != IIO_EV_INFO_VALUE)
   202			return -EINVAL;
   203	
   204		switch (dir) {
   205		case IIO_EV_DIR_RISING:
   206			switch (type) {
   207			case IIO_EV_TYPE_THRESH:
   208				return max30210_read_temp(st->regmap, MAX30210_TEMP_ALM_HI_REG, val);
   209			default:
   210				return -EINVAL;
   211			}
   212		case IIO_EV_DIR_FALLING:
   213			switch (type) {
   214			case IIO_EV_TYPE_THRESH:
   215				return max30210_read_temp(st->regmap, MAX30210_TEMP_ALM_LO_REG, val);
   216			default:
   217				return -EINVAL;
   218			}
   219		default:
   220			return -EINVAL;
   221		}
   222	}
   223	
   224	static int max30210_write_event(struct iio_dev *indio_dev,
   225					const struct iio_chan_spec *chan,
   226					enum iio_event_type type,
   227					enum iio_event_direction dir,
   228					enum iio_event_info info, int val, int val2)
   229	{
   230		struct max30210_state *st = iio_priv(indio_dev);
   231	
   232		if (info != IIO_EV_INFO_VALUE)
   233			return -EINVAL;
   234	
   235		switch (dir) {
   236		case IIO_EV_DIR_RISING:
   237			switch (type) {
   238			case IIO_EV_TYPE_THRESH:
   239				return max30210_write_temp(st->regmap, MAX30210_TEMP_ALM_HI_REG, val);
   240			default:
   241				return -EINVAL;
   242			}
   243		case IIO_EV_DIR_FALLING:
   244			switch (type) {
   245			case IIO_EV_TYPE_THRESH:
   246				return max30210_write_temp(st->regmap, MAX30210_TEMP_ALM_LO_REG, val);
   247			default:
   248				return -EINVAL;
   249			}
   250		default:
   251			return -EINVAL;
   252		}
   253	}
   254	
   255	static int max30210_read_event_config(struct iio_dev *indio_dev,
   256					      const struct iio_chan_spec *chan,
   257					      enum iio_event_type type,
   258					      enum iio_event_direction dir)
   259	{
   260		struct max30210_state *st = iio_priv(indio_dev);
   261		unsigned int val;
   262		int ret;
   263	
   264		ret = regmap_read(st->regmap, MAX30210_INT_EN_REG, &val);
   265		if (ret)
   266			return ret;
   267	
   268		switch (dir) {
   269		case IIO_EV_DIR_RISING:
   270			switch (type) {
   271			case IIO_EV_TYPE_THRESH:
 > 272				return FIELD_GET(MAX30210_STATUS_TEMP_HI_MASK, val);
   273			default:
   274				return -EINVAL;
   275			}
   276		case IIO_EV_DIR_FALLING:
   277			switch (type) {
   278			case IIO_EV_TYPE_THRESH:
   279				return FIELD_GET(MAX30210_STATUS_TEMP_LO_MASK, val);
   280			default:
   281				return -EINVAL;
   282			}
   283		default:
   284			return -EINVAL;
   285		}
   286	}
   287	

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

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

* Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
  2026-03-05  0:34   ` kernel test robot
@ 2026-03-05  0:45   ` kernel test robot
  2026-03-05  0:56   ` David Lechner
  2026-03-07 12:38   ` Jonathan Cameron
  3 siblings, 0 replies; 11+ messages in thread
From: kernel test robot @ 2026-03-05  0:45 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo, linux-iio, jic23
  Cc: oe-kbuild-all, dlechner, nuno.sa, andy, linux-kernel

Hi John,

kernel test robot noticed the following build errors:

[auto build test ERROR on 70a9ae59c5b1f2f5501e78e2d85bfeefd153f854]

url:    https://github.com/intel-lab-lkp/linux/commits/John-Erasmus-Mari-Geronimo/dt-bindings-iio-temperature-add-ADI-MAX30210/20260304-202920
base:   70a9ae59c5b1f2f5501e78e2d85bfeefd153f854
patch link:    https://lore.kernel.org/r/20260304122509.67931-3-johnerasmusmari.geronimo%40analog.com
patch subject: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
config: i386-randconfig-r071-20260305 (https://download.01.org/0day-ci/archive/20260305/202603050822.weVzrjhU-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
smatch: v0.5.0-9004-gb810ac53
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260305/202603050822.weVzrjhU-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/202603050822.weVzrjhU-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iio/temperature/max30210.c: In function 'max30210_fifo_read':
>> drivers/iio/temperature/max30210.c:136:27: error: implicit declaration of function 'get_unaligned_be24' [-Wimplicit-function-declaration]
     136 |                 u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
         |                           ^~~~~~~~~~~~~~~~~~
   drivers/iio/temperature/max30210.c: In function 'max30210_read_event_config':
>> drivers/iio/temperature/max30210.c:272:32: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     272 |                         return FIELD_GET(MAX30210_STATUS_TEMP_HI_MASK, val);
         |                                ^~~~~~~~~
   drivers/iio/temperature/max30210.c: In function 'max30210_write_raw':
>> drivers/iio/temperature/max30210.c:418:49: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     418 |                                                 FIELD_PREP(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, i));
         |                                                 ^~~~~~~~~~


vim +/get_unaligned_be24 +136 drivers/iio/temperature/max30210.c

   122	
   123	static void max30210_fifo_read(struct iio_dev *indio_dev)
   124	{
   125		struct max30210_state *st = iio_priv(indio_dev);
   126		int ret;
   127	
   128		ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
   129				       st->fifo_buf, 3 * st->watermark);
   130		if (ret) {
   131			dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
   132			return;
   133		}
   134	
   135		for (unsigned int i = 0; i < st->watermark; i++) {
 > 136			u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
   137	
   138			if (raw == MAX30210_FIFO_INVAL_DATA) {
   139				dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
   140				continue;
   141			}
   142	
   143			s16 temp = (s16)(raw & 0xFFFF);
   144	
   145			iio_push_to_buffers(indio_dev, &temp);
   146		}
   147	}
   148	
   149	static irqreturn_t max30210_irq_handler(int irq, void *dev_id)
   150	{
   151		struct iio_dev *indio_dev = dev_id;
   152		struct max30210_state *st = iio_priv(indio_dev);
   153		unsigned int status;
   154		int ret;
   155	
   156		ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
   157		if (ret) {
   158			dev_err(&indio_dev->dev, "Status read failed\n");
   159			return IRQ_NONE;
   160		}
   161	
   162		if (status & MAX30210_STATUS_A_FULL_MASK)
   163			max30210_fifo_read(indio_dev);
   164	
   165		if (status & MAX30210_STATUS_TEMP_HI_MASK)
   166			iio_push_event(indio_dev,
   167				       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
   168							    IIO_EV_TYPE_THRESH,
   169							    IIO_EV_DIR_RISING),
   170				       iio_get_time_ns(indio_dev));
   171	
   172		if (status & MAX30210_STATUS_TEMP_LO_MASK)
   173			iio_push_event(indio_dev,
   174				       IIO_UNMOD_EVENT_CODE(IIO_TEMP, 0,
   175							    IIO_EV_TYPE_THRESH,
   176							    IIO_EV_DIR_FALLING),
   177				       iio_get_time_ns(indio_dev));
   178	
   179		return IRQ_HANDLED;
   180	}
   181	
   182	static int max30210_reg_access(struct iio_dev *indio_dev, unsigned int reg,
   183				       unsigned int writeval, unsigned int *readval)
   184	{
   185		struct max30210_state *st = iio_priv(indio_dev);
   186	
   187		if (!readval)
   188			return regmap_write(st->regmap, reg, writeval);
   189	
   190		return regmap_read(st->regmap, reg, readval);
   191	}
   192	
   193	static int max30210_read_event(struct iio_dev *indio_dev,
   194				       const struct iio_chan_spec *chan,
   195				       enum iio_event_type type,
   196				       enum iio_event_direction dir,
   197				       enum iio_event_info info, int *val, int *val2)
   198	{
   199		struct max30210_state *st = iio_priv(indio_dev);
   200	
   201		if (info != IIO_EV_INFO_VALUE)
   202			return -EINVAL;
   203	
   204		switch (dir) {
   205		case IIO_EV_DIR_RISING:
   206			switch (type) {
   207			case IIO_EV_TYPE_THRESH:
   208				return max30210_read_temp(st->regmap, MAX30210_TEMP_ALM_HI_REG, val);
   209			default:
   210				return -EINVAL;
   211			}
   212		case IIO_EV_DIR_FALLING:
   213			switch (type) {
   214			case IIO_EV_TYPE_THRESH:
   215				return max30210_read_temp(st->regmap, MAX30210_TEMP_ALM_LO_REG, val);
   216			default:
   217				return -EINVAL;
   218			}
   219		default:
   220			return -EINVAL;
   221		}
   222	}
   223	
   224	static int max30210_write_event(struct iio_dev *indio_dev,
   225					const struct iio_chan_spec *chan,
   226					enum iio_event_type type,
   227					enum iio_event_direction dir,
   228					enum iio_event_info info, int val, int val2)
   229	{
   230		struct max30210_state *st = iio_priv(indio_dev);
   231	
   232		if (info != IIO_EV_INFO_VALUE)
   233			return -EINVAL;
   234	
   235		switch (dir) {
   236		case IIO_EV_DIR_RISING:
   237			switch (type) {
   238			case IIO_EV_TYPE_THRESH:
   239				return max30210_write_temp(st->regmap, MAX30210_TEMP_ALM_HI_REG, val);
   240			default:
   241				return -EINVAL;
   242			}
   243		case IIO_EV_DIR_FALLING:
   244			switch (type) {
   245			case IIO_EV_TYPE_THRESH:
   246				return max30210_write_temp(st->regmap, MAX30210_TEMP_ALM_LO_REG, val);
   247			default:
   248				return -EINVAL;
   249			}
   250		default:
   251			return -EINVAL;
   252		}
   253	}
   254	
   255	static int max30210_read_event_config(struct iio_dev *indio_dev,
   256					      const struct iio_chan_spec *chan,
   257					      enum iio_event_type type,
   258					      enum iio_event_direction dir)
   259	{
   260		struct max30210_state *st = iio_priv(indio_dev);
   261		unsigned int val;
   262		int ret;
   263	
   264		ret = regmap_read(st->regmap, MAX30210_INT_EN_REG, &val);
   265		if (ret)
   266			return ret;
   267	
   268		switch (dir) {
   269		case IIO_EV_DIR_RISING:
   270			switch (type) {
   271			case IIO_EV_TYPE_THRESH:
 > 272				return FIELD_GET(MAX30210_STATUS_TEMP_HI_MASK, val);
   273			default:
   274				return -EINVAL;
   275			}
   276		case IIO_EV_DIR_FALLING:
   277			switch (type) {
   278			case IIO_EV_TYPE_THRESH:
   279				return FIELD_GET(MAX30210_STATUS_TEMP_LO_MASK, val);
   280			default:
   281				return -EINVAL;
   282			}
   283		default:
   284			return -EINVAL;
   285		}
   286	}
   287	

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

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

* Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
  2026-03-05  0:34   ` kernel test robot
  2026-03-05  0:45   ` kernel test robot
@ 2026-03-05  0:56   ` David Lechner
  2026-03-07 12:38   ` Jonathan Cameron
  3 siblings, 0 replies; 11+ messages in thread
From: David Lechner @ 2026-03-05  0:56 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo, linux-iio, jic23; +Cc: nuno.sa, andy, linux-kernel

On 3/4/26 6:25 AM, John Erasmus Mari Geronimo wrote:
> Add support for the Analog Devices MAX30210 I2C temperature
> sensor.
> 
> The driver uses regmap for register access and integrates with
> the IIO framework. It supports:
> 
> - Direct mode temperature conversion
> - Configurable sampling frequency
> - Threshold events
> - FIFO operation with IIO kfifo buffer support
> - Optional interrupt-driven data ready signaling
> 
> The device provides 16-bit signed temperature data and a
> 64-sample FIFO.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> ---
>  MAINTAINERS                        |   1 +
>  drivers/iio/temperature/Kconfig    |  10 +
>  drivers/iio/temperature/Makefile   |   1 +
>  drivers/iio/temperature/max30210.c | 664 +++++++++++++++++++++++++++++
>  4 files changed, 676 insertions(+)
>  create mode 100644 drivers/iio/temperature/max30210.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 09345b9f32ed..2abdbcf3a8e5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1644,6 +1644,7 @@ 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>
> diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
> index 9328b2250ace..b396757eb761 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.
>  

Let's keep these in order too. MAX30... goes before MAX31...

> +config MAX30210
> +	tristate "Analog Devices MAX30210 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..839ed9830957
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,664 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices MAX30210 I2C Temperature Sensor driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
> +

Looks like we are missing some headers here. We try to include
what is actually used in the file rather than relying on includes
from other includes.

> +#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/kfifo_buf.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regmap.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_STATUS_A_FULL_MASK   BIT(7)
> +#define MAX30210_STATUS_TEMP_RDY_MASK BIT(6)
> +#define MAX30210_STATUS_TEMP_DEC_MASK BIT(5)
> +#define MAX30210_STATUS_TEMP_INC_MASK BIT(4)
> +#define MAX30210_STATUS_TEMP_LO_MASK  BIT(3)
> +#define MAX30210_STATUS_TEMP_HI_MASK  BIT(2)
> +#define MAX30210_STATUS_PWR_RDY_MASK  BIT(0)
> +
> +#define MAX30210_FIFOCONF1_FLUSH_FIFO_MASK BIT(4)
> +
> +#define MAX30210_SYSCONF_RESET_MASK		BIT(0)
> +
> +#define MAX30210_PINCONF_EXT_CNV_EN_MASK	BIT(7)
> +#define MAX30210_PINCONF_EXT_CVT_ICFG_MASK	BIT(6)
> +#define MAX30210_PINCONF_INT_FCFG_MASK		GENMASK(3, 2)
> +#define MAX30210_PINCONF_INT_OCFG_MASK		GENMASK(1, 0)
> +
> +#define MAX30210_TEMPCONF1_CHG_DET_EN_MASK	BIT(3)
> +#define MAX30210_TEMPCONF1_RATE_CHG_FILTER_MASK	GENMASK(2, 0)
> +
> +#define MAX30210_TEMPCONF2_TEMP_PERIOD_MASK	GENMASK(3, 0)
> +#define MAX30210_TEMPCONF2_ALERT_MODE_MASK	BIT(7)
> +
> +#define MAX30210_TEMPCONV_AUTO_MASK	BIT(1)
> +#define MAX30210_TEMPCONV_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_UNIQUE_ID_LEN		6
> +#define MAX30210_EXT_CVT_FREQ_MIN	1
> +#define MAX30210_EXT_CVT_FREQ_MAX	20
> +
> +struct max30210_state {
> +	struct regmap *regmap;
> +	u8 watermark;
> +	/* Raw FIFO byte buffer (3 bytes per sample) */

This 3 is used a few other places in the code, so might be nice to have
a macro for that.

> +	u8 fifo_buf[3 * MAX30210_FIFO_SIZE];
> +};
> +
> +static const int max30210_samp_freq_avail[][2] = {
> +	{ 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)
> +{
> +	__be16 val;
> +	int ret;
> +
> +	ret = regmap_bulk_read(regmap, reg, &val, sizeof(val));
> +	if (ret)
> +		return ret;
> +
> +	*temp = sign_extend32(be16_to_cpu(val), 15);
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int max30210_write_temp(struct regmap *regmap, unsigned int reg,
> +			       int temp)
> +{
> +	__be16 val;
> +
> +	val = cpu_to_be16(temp);
> +
> +	return regmap_bulk_write(regmap, reg, &val, sizeof(val));
> +}
> +
> +static void max30210_fifo_read(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
> +			       st->fifo_buf, 3 * st->watermark);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
> +		return;
> +	}
> +
> +	for (unsigned int i = 0; i < st->watermark; i++) {
> +		u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
> +
> +		if (raw == MAX30210_FIFO_INVAL_DATA) {
> +			dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
> +			continue;
> +		}
> +
> +		s16 temp = (s16)(raw & 0xFFFF);

Can use FIELD_GET().

> +
> +		iio_push_to_buffers(indio_dev, &temp);
> +	}
> +}
> +
> +static irqreturn_t max30210_irq_handler(int irq, void *dev_id)
> +{
> +	struct iio_dev *indio_dev = dev_id;
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	unsigned int status;
> +	int ret;
> +
> +	ret = regmap_read(st->regmap, MAX30210_STATUS_REG, &status);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Status read failed\n");
> +		return IRQ_NONE;
> +	}
> +
> +	if (status & MAX30210_STATUS_A_FULL_MASK)
> +		max30210_fifo_read(indio_dev);
> +
> +	if (status & MAX30210_STATUS_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_STATUS_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));
> +
> +	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)

Usual way is to swap these to avoid the !.

> +		return regmap_write(st->regmap, reg, writeval);
> +
> +	return regmap_read(st->regmap, reg, readval);
> +}
> +
> +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)
> +		return -EINVAL;
> +
> +	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;
> +		}
> +	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;
> +		}
> +	default:
> +		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)
> +		return -EINVAL;

No range checking on val?

> +
> +	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;
> +		}
> +	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;
> +		}
> +	default:
> +		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_STATUS_TEMP_HI_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_EV_DIR_FALLING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			return FIELD_GET(MAX30210_STATUS_TEMP_LO_MASK, val);
> +		default:
> +			return -EINVAL;
> +		}
> +	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, bool state)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +
> +	switch (dir) {
> +	case IIO_EV_DIR_RISING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			return regmap_assign_bits(st->regmap, MAX30210_INT_EN_REG,
> +						  MAX30210_STATUS_TEMP_HI_MASK, state);
> +		default:
> +			return -EINVAL;
> +		}
> +	case IIO_EV_DIR_FALLING:
> +		switch (type) {
> +		case IIO_EV_TYPE_THRESH:
> +			return regmap_assign_bits(st->regmap, MAX30210_INT_EN_REG,
> +						  MAX30210_STATUS_TEMP_LO_MASK, state);
> +		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;
> +		*val2 = 1000;
> +
> +		return IIO_VAL_FRACTIONAL;
> +	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_TEMPCONF2_TEMP_PERIOD_MASK, uval);
> +

Could use a comment here to explain code below. It makes sense when you
read the datasheet, but looks odd without that context.

> +		if (uval >= ARRAY_SIZE(max30210_samp_freq_avail))
> +			uval = ARRAY_SIZE(max30210_samp_freq_avail) - 1;
> +
> +		*val = max30210_samp_freq_avail[uval][0];
> +		*val2 = max30210_samp_freq_avail[uval][1];
> +
> +		return IIO_VAL_FRACTIONAL;
> +	case IIO_CHAN_INFO_RAW: {

Techicnally, there is a race condition here. We could be exiting buffer mode
here. So this looks like one of those rare cases where we should use
IIO_DEV_GUARD_CURRENT_MODE().

Typically we just don't allow reading raw during buffered read though
unless there is a real-world use case for it.

> +		if (iio_buffer_enabled(indio_dev))
> +			return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +				   MAX30210_TEMPCONV_CONV_T_MASK);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Wait until CONVERT_T auto-clears.
> +		 * Datasheet:
> +		 *   tBIAS_WU = 260 µs
> +		 *   tINT     = 8 ms
> +		 *
> +		 * Worst-case conversion ≈ 8.26 ms.
> +		 * Use 10 ms timeout for margin.
> +		 */
> +		ret = regmap_read_poll_timeout(st->regmap, MAX30210_TEMP_CONV_REG, uval,
> +					       !(uval & MAX30210_TEMPCONV_CONV_T_MASK),
> +					       500,          /* poll every 500 µs */
> +					       10000);       /* 10 ms timeout */
> +		if (ret)
> +			return ret;
> +
> +		return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +	}
> +	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 = &max30210_samp_freq_avail[0][0];
> +		*type = IIO_VAL_INT_PLUS_MICRO;
> +		*length = ARRAY_SIZE(max30210_samp_freq_avail) * 2;
> +
> +		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);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		if (val < 0 || val2 < 0)
> +			return -EINVAL;
> +
> +		for (unsigned int i = 0; i < ARRAY_SIZE(max30210_samp_freq_avail); i++) {
> +			if (val == max30210_samp_freq_avail[i][0] &&
> +			    val2 == max30210_samp_freq_avail[i][1]) {
> +				return regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
> +						MAX30210_TEMPCONF2_TEMP_PERIOD_MASK,
> +						FIELD_PREP(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, i));
> +			}
> +		}
> +
> +		return -EINVAL;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +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;

This looks a bit odd and could use some explanation.

And even if it technically doesn't need it, would still expect a FIELD_PREP()
here since there are some reserved bits.

> +
> +	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;

The register names are a bit cryptic. I wouldn't mind a high-level comment
explaining the intention here.

> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_INT_EN_REG, MAX30210_STATUS_A_FULL_MASK);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +			    MAX30210_TEMPCONV_AUTO_MASK | MAX30210_TEMPCONV_CONV_T_MASK);
> +}
> +
> +static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_clear_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_STATUS_A_FULL_MASK);
> +}
> +
> +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,
> +	.hwfifo_set_watermark = max30210_set_watermark,
> +	.debugfs_reg_access = max30210_reg_access,
> +	.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),
> +	.scan_index = 0,
> +	.event_spec = max30210_events,
> +	.num_event_specs = ARRAY_SIZE(max30210_events),
> +	.scan_type = {
> +		.sign = 's',
> +		.realbits = 16,
> +		.storagebits = 16,
> +		.shift = 0,
> +		.endianness = IIO_CPU,
> +	},
> +};
> +
> +static int max30210_setup(struct max30210_state *st, struct device *dev)
> +{
> +	struct gpio_desc *powerdown_gpio;
> +	unsigned int val;
> +	int ret;
> +
> +	/* Optional hardware reset via powerdown GPIO */
> +	powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> +						 GPIOD_OUT_HIGH);
> +	if (IS_ERR(powerdown_gpio))
> +		return dev_err_probe(dev, PTR_ERR(powerdown_gpio),
> +				     "failed to request powerdown GPIO\n");
> +
> +	if (powerdown_gpio) {
> +		/* Deassert powerdown to power up device */
> +		gpiod_set_value(powerdown_gpio, 0);
> +	} else {
> +		/* Software reset fallback */
> +		ret = regmap_update_bits(st->regmap, MAX30210_SYS_CONF_REG,

regmap_set_bits()

> +					 MAX30210_SYSCONF_RESET_MASK,
> +					 MAX30210_SYSCONF_RESET_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +

Use IIO style multi-line comments:

	/*
	 * Datasheet Figure 6:

> +	/* Datasheet Figure 6:
> +	 * tPU max = 700 µs after power-up or reset before device is ready.
> +	 */
> +	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);
> +
> +	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");
> +
> +	st->watermark = MAX30210_WATERMARK_DEFAULT;
> +
> +	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_kfifo_buffer_setup_ext(dev, indio_dev, &max30210_buffer_ops,
> +					      max30210_fifo_attributes);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq) {

The way the driver is written currently, the interrup is required.

Either we need to reutrn error on !client->irq or we need to have a second
max30210_info that excludes buffer and events.

> +		ret = devm_request_irq(dev, client->irq, max30210_irq_handler, IRQF_NO_THREAD,
> +				       indio_dev->name, indio_dev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +
> +static const struct i2c_device_id max30210_id[] = {
> +	{ "max30210" },
> +	{ }
> +};
> +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");


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

* Re: [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210
  2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
  2026-03-05  0:11   ` David Lechner
@ 2026-03-05  7:00   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 11+ messages in thread
From: Krzysztof Kozlowski @ 2026-03-05  7:00 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo, linux-iio, jic23
  Cc: dlechner, nuno.sa, andy, linux-kernel

On 04/03/2026 13:25, 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>

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.

This is odd considering previously you sent it to maintainers.


Best regards,
Krzysztof

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

* Re: [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210
  2026-03-05  0:11   ` David Lechner
@ 2026-03-07 12:21     ` Jonathan Cameron
  0 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-03-07 12:21 UTC (permalink / raw)
  To: David Lechner
  Cc: John Erasmus Mari Geronimo, linux-iio, nuno.sa, andy,
	linux-kernel

On Wed, 4 Mar 2026 18:11:22 -0600
David Lechner <dlechner@baylibre.com> wrote:

> On 3/4/26 6:25 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         | 62 +++++++++++++++++++
> >  MAINTAINERS                                   |  7 +++
> >  2 files changed, 69 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..66867880a20f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> > @@ -0,0 +1,62 @@
> > +# 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 Temperature Sensor
> > +
> > +maintainers:
> > +  - John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
> > +
> > +description: |
> > +  The MAX30210 is a temperature sensor with an I2C interface.
> > +  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.  
> 
> The description makes it sound like there could be other supplies,
> but there aren't. It is the "everything" supply, so we can just
> call it the power supply.

I think

   vdd-supply: true

is also fine when it's a simple as this.

Jonathan


> 


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

* Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
  2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
                     ` (2 preceding siblings ...)
  2026-03-05  0:56   ` David Lechner
@ 2026-03-07 12:38   ` Jonathan Cameron
  3 siblings, 0 replies; 11+ messages in thread
From: Jonathan Cameron @ 2026-03-07 12:38 UTC (permalink / raw)
  To: John Erasmus Mari Geronimo
  Cc: linux-iio, dlechner, nuno.sa, andy, linux-kernel

On Wed, 4 Mar 2026 20:25:09 +0800
John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com> wrote:

> Add support for the Analog Devices MAX30210 I2C temperature
> sensor.
> 
> The driver uses regmap for register access and integrates with
> the IIO framework. It supports:
> 
> - Direct mode temperature conversion
> - Configurable sampling frequency
> - Threshold events
> - FIFO operation with IIO kfifo buffer support
> - Optional interrupt-driven data ready signaling
> 
> The device provides 16-bit signed temperature data and a
> 64-sample FIFO.
> 
> Signed-off-by: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>
Hi John

A few additional comment from me, but overall this is coming together nicely.

Thanks,

Jonathan

> diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
> new file mode 100644
> index 000000000000..839ed9830957
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c

> +static void max30210_fifo_read(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_bulk_read(st->regmap, MAX30210_FIFO_DATA_REG,
> +			       st->fifo_buf, 3 * st->watermark);
> +	if (ret) {
> +		dev_err(&indio_dev->dev, "Failed to read from fifo.\n");
> +		return;
> +	}
> +
> +	for (unsigned int i = 0; i < st->watermark; i++) {
> +		u32 raw = get_unaligned_be24(&st->fifo_buf[3 * i]);
> +
> +		if (raw == MAX30210_FIFO_INVAL_DATA) {
Whilst this aligns with how the datasheet describes it, the
only bit that is definitely different for invalid data is bit 7 of the
tag.  So could just check that. The code to get the temperature value then
just becomes  memcpy() with the channel described as bit endian.
I don't mind if you prefer it this way though, just thought I'd raise
the possibility.

> +			dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
> +			continue;
> +		}
> +
> +		s16 temp = (s16)(raw & 0xFFFF);
> +
> +		iio_push_to_buffers(indio_dev, &temp);
> +	}
> +}


> +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;
> +		*val2 = 1000;
> +
> +		return IIO_VAL_FRACTIONAL;
> +	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_TEMPCONF2_TEMP_PERIOD_MASK, uval);
> +
> +		if (uval >= ARRAY_SIZE(max30210_samp_freq_avail))
> +			uval = ARRAY_SIZE(max30210_samp_freq_avail) - 1;
		uval = min(uval, ARRAY_SIZE(max30210_samp_freq_avail) - 1);

perhaps?

> +
> +		*val = max30210_samp_freq_avail[uval][0];
> +		*val2 = max30210_samp_freq_avail[uval][1];
> +
> +		return IIO_VAL_FRACTIONAL;


> +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);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		if (val < 0 || val2 < 0)
> +			return -EINVAL;
> +
> +		for (unsigned int i = 0; i < ARRAY_SIZE(max30210_samp_freq_avail); i++) {
> +			if (val == max30210_samp_freq_avail[i][0] &&
> +			    val2 == max30210_samp_freq_avail[i][1]) {

Flip the logic to reduce indent.
			if (val != max...[0] ||
			    val != max...[1])
				continue;

			return....

> +				return regmap_update_bits(st->regmap, MAX30210_TEMP_CONF_2_REG,
> +						MAX30210_TEMPCONF2_TEMP_PERIOD_MASK,
> +						FIELD_PREP(MAX30210_TEMPCONF2_TEMP_PERIOD_MASK, i));
> +			}
> +		}
> +
> +		return -EINVAL;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

> +static int max30210_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_INT_EN_REG, MAX30210_STATUS_A_FULL_MASK);
I would wrap this one. Won't hurt readability and keeps it inline with the wrapping
on the clear in postdisable()

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +			    MAX30210_TEMPCONV_AUTO_MASK | MAX30210_TEMPCONV_CONV_T_MASK);
> +}
> +
> +static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	return regmap_clear_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_STATUS_A_FULL_MASK);
> +}

> +
> +static const struct iio_chan_spec max30210_channels = {

only one. So max30210_channel

> +	.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),
> +	.scan_index = 0,
> +	.event_spec = max30210_events,
> +	.num_event_specs = ARRAY_SIZE(max30210_events),
> +	.scan_type = {
> +		.sign = 's',
> +		.realbits = 16,
> +		.storagebits = 16,
> +		.shift = 0,

A zero shift is kind of considered the obvious default, so we normally don't
bother setting it explicitly.

> +		.endianness = IIO_CPU,
> +	},
> +};
> +
> +static int max30210_setup(struct max30210_state *st, struct device *dev)
> +{
> +	struct gpio_desc *powerdown_gpio;
> +	unsigned int val;
> +	int ret;
> +
> +	/* Optional hardware reset via powerdown GPIO */
> +	powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> +						 GPIOD_OUT_HIGH);
> +	if (IS_ERR(powerdown_gpio))
> +		return dev_err_probe(dev, PTR_ERR(powerdown_gpio),
> +				     "failed to request powerdown GPIO\n");
> +
> +	if (powerdown_gpio) {
> +		/* Deassert powerdown to power up device */
> +		gpiod_set_value(powerdown_gpio, 0);
> +	} else {
> +		/* Software reset fallback */
> +		ret = regmap_update_bits(st->regmap, MAX30210_SYS_CONF_REG,
> +					 MAX30210_SYSCONF_RESET_MASK,
> +					 MAX30210_SYSCONF_RESET_MASK);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	/* Datasheet Figure 6:
	/*
	 * Datasheet..

> +	 * tPU max = 700 µs after power-up or reset before device is ready.
> +	 */
> +	fsleep(700);
> +
> +	/* Clear status byte */
> +	return regmap_read(st->regmap, MAX30210_STATUS_REG, &val);
> +}
> +

> +static int max30210_probe(struct i2c_client *client)
> +{

...

> +	ret = devm_iio_kfifo_buffer_setup_ext(dev, indio_dev, &max30210_buffer_ops,
> +					      max30210_fifo_attributes);
> +	if (ret)
> +		return ret;
> +
> +	if (client->irq) {
> +		ret = devm_request_irq(dev, client->irq, max30210_irq_handler, IRQF_NO_THREAD,

You are doing a bunch of bus accesses in your handler and I can't see anything int here
that wouldn't work in a thread. So I think this is backwards. All the work should be
done in an interrupt thread.

> +				       indio_dev->name, indio_dev);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

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

end of thread, other threads:[~2026-03-07 12:38 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-04 12:25 [PATCH v2 0/2] Add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
2026-03-04 12:25 ` [PATCH v2 1/2] dt-bindings: iio: temperature: add ADI MAX30210 John Erasmus Mari Geronimo
2026-03-05  0:11   ` David Lechner
2026-03-07 12:21     ` Jonathan Cameron
2026-03-05  7:00   ` Krzysztof Kozlowski
2026-03-04 12:25 ` [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210 John Erasmus Mari Geronimo
2026-03-05  0:34   ` kernel test robot
2026-03-05  0:45   ` kernel test robot
2026-03-05  0:56   ` David Lechner
2026-03-07 12:38   ` Jonathan Cameron
2026-03-04 13:42 ` [PATCH v2 0/2] Add " Andy Shevchenko

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