[PATCH v2 0/2] Add support for Analog Devices MAX30210

[PATCH v2 0/2] Add support for Analog Devices MAX30210

[John Erasmus Mari Geronimo]

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

[PATCH v2 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         | 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

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

[David Lechner]

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

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

[Jonathan Cameron]

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


> 

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

[Krzysztof Kozlowski]

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

[PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210

[John Erasmus Mari Geronimo]

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

Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210

[kernel test robot]

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

Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210

[kernel test robot]

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

Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210

[David Lechner]

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");

Re: [PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210

[Jonathan Cameron]

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

Re: [PATCH v2 0/2] Add support for Analog Devices MAX30210

[Andy Shevchenko]

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

[PATCH v3 0/2] Add support for Analog Devices MAX30210

[John Erasmus Mari Geronimo]

Add IIO support and device-tree bindings for the Analog Devices MAX30210
low-power digital temperature sensor.

Changes since v2:
- Fix FIFO byte-count handling and sample decoding.
- Fix IIO debug register read/write access.
- Validate event threshold values and sampling-frequency register values.
- Return the correct sampling-frequency format and reject direct reads while
  buffered capture is enabled.
- Clean up the Kconfig entry and device-tree supply declaration.

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         |  61 ++
 MAINTAINERS                                   |   8 +
 drivers/iio/temperature/Kconfig               |  10 +
 drivers/iio/temperature/Makefile              |   1 +
 drivers/iio/temperature/max30210.c            | 689 ++++++++++++++++++
 5 files changed, 769 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
 create mode 100644 drivers/iio/temperature/max30210.c

-- 
2.34.1

[PATCH v3 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         | 61 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 68 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 0000000000000..8bfd154bbc438
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
@@ -0,0 +1,61 @@
+# 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: true
+
+  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 396d4e76dccc2..6eb3cf66f431c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1653,6 +1653,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

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

[Krzysztof Kozlowski]

On Fri, Sep 11, 2026 at 06:12:16AM +0800, 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         | 61 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 68 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
> 

Do not attach (thread) your patchsets to some other threads (unrelated
or older versions). This buries them deep in the mailbox and might
interfere with applying entire sets. See also:
https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

[PATCH v3 2/2] iio: temperature: add support for Analog Devices MAX30210

[John Erasmus Mari Geronimo]

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 | 689 +++++++++++++++++++++++++++++
 4 files changed, 701 insertions(+)
 create mode 100644 drivers/iio/temperature/max30210.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6eb3cf66f431c..e479c64a284e0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1659,6 +1659,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 9328b2250aced..cf840f7dda765 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -152,6 +152,16 @@ config MAX30208
 	  This driver can also be built as a module. If so, the module
 	  will be called max30208.
 
+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 built as a module. If so, the module
+	  will be called max30210.
+
 config MAX31856
 	tristate "MAX31856 thermocouple sensor"
 	depends on SPI
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7f..e5aad14dc09b9 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 0000000000000..07ff85d6d4848
--- /dev/null
+++ b/drivers/iio/temperature/max30210.c
@@ -0,0 +1,689 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Analog Devices MAX30210 I2C Temperature Sensor driver
+ *
+ * Copyright 2026 Analog Devices Inc.
+ */
+
+#include <linux/bitfield.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/kfifo_buf.h>
+#include <linux/iio/sysfs.h>
+#include <linux/regmap.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_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_A_FULL_MASK		GENMASK(5, 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_BYTES_PER_SAMPLE		3
+#define MAX30210_FIFO_TEMP_MASK			GENMASK(15, 0)
+#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 */
+	u8 fifo_buf[MAX30210_FIFO_BYTES_PER_SAMPLE * 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,
+			       MAX30210_FIFO_BYTES_PER_SAMPLE * 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[MAX30210_FIFO_BYTES_PER_SAMPLE * i]);
+
+		if (raw == MAX30210_FIFO_INVAL_DATA) {
+			dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
+			continue;
+		}
+
+		s16 temp = (s16)FIELD_GET(MAX30210_FIFO_TEMP_MASK, raw);
+
+		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_read(st->regmap, reg, readval);
+
+	return regmap_write(st->regmap, reg, writeval);
+}
+
+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;
+
+	if (val < S16_MIN || val > S16_MAX)
+		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);
+
+		/*
+		 * TEMP_PERIOD is an index into the sampling frequency lookup
+		 * table. Clamp in case the register holds a reserved value.
+		 */
+		uval = min_t(unsigned int, 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_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_RAW: {
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		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])
+				continue;
+
+			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);
+	int ret;
+
+	if (val < 1 || val > MAX30210_FIFO_SIZE)
+		return -EINVAL;
+
+	/*
+	 * FIFO_A_FULL sets the number of empty FIFO slots remaining when the
+	 * A_FULL interrupt fires. For a watermark of N samples, set
+	 * FIFO_A_FULL = FIFO_SIZE - N.
+	 */
+	ret = regmap_write(st->regmap, MAX30210_FIFO_CONF_1_REG,
+			   FIELD_PREP(MAX30210_FIFOCONF1_A_FULL_MASK,
+				      MAX30210_FIFO_SIZE - val));
+	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;
+
+	/* Enable FIFO-full interrupt */
+	ret = regmap_set_bits(st->regmap, MAX30210_INT_EN_REG,
+			      MAX30210_STATUS_A_FULL_MASK);
+	if (ret)
+		return ret;
+
+	/* Flush FIFO before starting autonomous conversions */
+	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	/* Start autonomous temperature conversion */
+	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;
+
+	/* Stop autonomous conversion */
+	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
+	if (ret)
+		return ret;
+
+	/* Flush FIFO */
+	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
+			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
+	if (ret)
+		return ret;
+
+	/* Disable FIFO-full interrupt */
+	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_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,
+		.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_set_bits(st->regmap, MAX30210_SYS_CONF_REG,
+				      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_channel;
+	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)
+		return dev_err_probe(dev, -ENXIO, "Missing interrupt.\n");
+
+	ret = devm_request_threaded_irq(dev, client->irq, NULL,
+					max30210_irq_handler, IRQF_ONESHOT,
+					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

Re: [PATCH v3 2/2] iio: temperature: add support for Analog Devices MAX30210

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] The driver uses `regmap_bulk_read` to read from the FIFO, which increments the register address on fallback, corrupting the read.
- [High] Threshold events will never trigger unless the FIFO buffer is also enabled, because autonomous conversion is not managed correctly.
- [High] Use-after-free on IIO `event_interface` during device removal due to incorrect devm registration order.
- [Medium] The driver strictly requires an interrupt, contradicting the commit message claim that interrupt signaling is optional.
- [Low] Missing Kconfig dependencies for REGMAP_I2C, IIO_BUFFER, and IIO_KFIFO_BUF.
- [Low] Device is not powered down or disabled on driver removal, leading to power leaks.
- [Low] Missing `#include <linux/regulator/consumer.h>` causes implicit function declaration.
- [Low] Macro naming typo suggests writing to the wrong register.
--

commit cfe908c78eef329997beda55cb489b59b1cf8d93
Author: John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com>

iio: temperature: add support for Analog Devices MAX30210

This adds support for the Analog Devices MAX30210 I2C temperature sensor.
The driver integrates with the IIO framework, providing direct mode
temperature conversion, threshold events, and FIFO operation.

> --- a/drivers/iio/temperature/Kconfig
> +++ b/drivers/iio/temperature/Kconfig
> @@ -152,6 +152,16 @@ config MAX30208
>  	  This driver can also be built as a module. If so, the module
>  	  will be called max30208.
>  
> +config MAX30210
> +	tristate "Analog Devices MAX30210 temperature sensor"
> +	depends on I2C

[Severity: Low]
Should this select REGMAP_I2C, IIO_BUFFER, and IIO_KFIFO_BUF? The driver 
code calls devm_regmap_init_i2c() and devm_iio_kfifo_buffer_setup_ext(). 
Without these selections, could a build with CONFIG_MAX30210 enabled fail 
at compile or link time if the user doesn't manually enable them?

> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,689 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices MAX30210 I2C Temperature Sensor driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
[ ... ]
> +#include <linux/regmap.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>

[Severity: Low]
Should <linux/regulator/consumer.h> be included here? The probe function 
uses devm_regulator_get_enable(), which might cause an implicit function 
declaration error on some configurations without this header.

> +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,
> +			       MAX30210_FIFO_BYTES_PER_SAMPLE * st->watermark);

[Severity: High]
Could this corrupt data on SMBus-only adapters? The driver uses 
regmap_bulk_read() to read the FIFO data register but permits probing on 
adapters that only support I2C_FUNC_SMBUS_BYTE_DATA. 

On such adapters, regmap falls back to single-byte reads and automatically 
increments the register address for each byte. Instead of repeatedly 
reading the FIFO register, this would read sequential configuration 
registers and push their contents as temperature samples.

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

[Severity: High]
Will threshold events trigger if the IIO buffer is not enabled? 

The autonomous conversion bit (MAX30210_TEMPCONV_AUTO_MASK) is only set in 
max30210_buffer_preenable(). If a user configures and enables a threshold 
event via sysfs without enabling the buffer, does the device remain in 
standby and never sample the temperature?

> +static int max30210_buffer_preenable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	/* Enable FIFO-full interrupt */
> +	ret = regmap_set_bits(st->regmap, MAX30210_INT_EN_REG,
> +			      MAX30210_STATUS_A_FULL_MASK);
> +	if (ret)
> +		return ret;
> +
> +	/* Flush FIFO before starting autonomous conversions */
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);

[Severity: Low]
Is MAX30210_FIFOCONF1_FLUSH_FIFO_MASK correct here? It is being applied to 
MAX30210_FIFO_CONF_2_REG. Should this use a FIFOCONF2_ prefixed macro 
instead to avoid confusion?

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

[Severity: Low]
Does this leave the hardware drawing power indefinitely on driver unbind? 
The device is powered up by deasserting the powerdown GPIO, but there 
doesn't seem to be a corresponding devm action or .remove callback to 
re-assert it and put the device back into standby when the module is removed.

> +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 (!client->irq)
> +		return dev_err_probe(dev, -ENXIO, "Missing interrupt.\n");

[Severity: Medium]
Does this unconditionally require an interrupt? The commit message mentions 
"Optional interrupt-driven data ready signaling", but this check causes the 
probe to fail if no IRQ is provided. Should it fall back gracefully to 
direct polling mode instead?

> +	ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +					max30210_irq_handler, IRQF_ONESHOT,
> +					indio_dev->name, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

[Severity: High]
Can this cause a use-after-free on the IIO event_interface during device 
removal?

Because devm resources are cleaned up in LIFO order, 
devm_iio_device_register() will unregister first (which frees the 
event_interface), followed later by the IRQ handler shutdown. 

If an interrupt fires between these two steps, max30210_irq_handler() could 
call iio_push_event() and dereference the already freed event_interface.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789032019.git.johnerasmusmari.geronimo@analog.com?part=2

Re: [PATCH v3 2/2] iio: temperature: add support for Analog Devices MAX30210

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 06:12:17AM +0800, 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.

...

Shouldn't Kconfig select REGMAP_I2C?

...

The header block doesn't fully follow IWYU.

+ array_size.h // ARRAY_SIZE()
+ bitops.h // BIT(), sign_extend32(), et cetera

> +#include <linux/bitfield.h>

+ delay.h // fsleep()

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

It's better to group them out...

+ minmax.h // min*(), et cetera

> +#include <linux/regmap.h>

+ types.h // uXX, __le16, et cetera

> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +

...somewhere here.

...

> +struct max30210_state {
> +	struct regmap *regmap;

> +	u8 watermark;

I would swap to make it easier to read the array (which will be aligned).

> +	/* Raw FIFO byte buffer */
> +	u8 fifo_buf[MAX30210_FIFO_BYTES_PER_SAMPLE * MAX30210_FIFO_SIZE];
> +};

...

> +static const struct regmap_config max30210_regmap = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX30210_PART_ID_REG,

No cache? Why?

> +};

...

> +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,
> +			       MAX30210_FIFO_BYTES_PER_SAMPLE * 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[MAX30210_FIFO_BYTES_PER_SAMPLE * i]);
> +
> +		if (raw == MAX30210_FIFO_INVAL_DATA) {
> +			dev_err_ratelimited(&indio_dev->dev, "Invalid data\n");
> +			continue;
> +		}
> +
> +		s16 temp = (s16)FIELD_GET(MAX30210_FIFO_TEMP_MASK, raw);

Defined variable at the proper space, also better to use sign_extend() instead
of casting.

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

Imagine if this is an intermittent issue, this will flood the logs very quickly.

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

KILO? MILLI?

> +		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);
> +
> +		/*
> +		 * TEMP_PERIOD is an index into the sampling frequency lookup
> +		 * table. Clamp in case the register holds a reserved value.
> +		 */
> +		uval = min_t(unsigned int, uval, ARRAY_SIZE(max30210_samp_freq_avail) - 1);

No min_t(). Also add a new temporary variable for the value from the register,
so semantically uval wouldn't be overloaded.

> +		*val = max30210_samp_freq_avail[uval][0];
> +		*val2 = max30210_samp_freq_avail[uval][1];
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_RAW: {
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		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 */

10 * USEC_PER_MSEC (will require time.h to be included).

> +		if (ret)
> +			return ret;
> +
> +		return max30210_read_temp(st->regmap, MAX30210_TEMP_DATA_REG, val);
> +	}
> +	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)

Split logically...

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

(I am surprised that this is v3 and too many mistakes that linux-iio@ mailing
 list is full of. You may take your time and read the mailing list archives to
 understand the most common style and API mistakes in IIO contributions.)

> +{
> +	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])
> +				continue;
> +
> +			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 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);
> +}

^^^ (here, see below)

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

Move this closer to the callback.

...

> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)

> +		return dev_err_probe(dev, ret,
> +				     "Failed to enable vdd regulator.\n");

It's fine to have it on a single line.

...

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

C99 initialisers.

> +	{ }
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/2] iio: temperature: add support for Analog Devices MAX30210

[Jonathan Cameron]

On Fri, 11 Sep 2026 06:12:17 +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

Various comments inline

Jonathan

> diff --git a/drivers/iio/temperature/max30210.c b/drivers/iio/temperature/max30210.c
> new file mode 100644
> index 0000000000000..07ff85d6d4848
> --- /dev/null
> +++ b/drivers/iio/temperature/max30210.c
> @@ -0,0 +1,689 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Analog Devices MAX30210 I2C Temperature Sensor driver
> + *
> + * Copyright 2026 Analog Devices Inc.
> + */
> +
> +#include <linux/bitfield.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/kfifo_buf.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/regmap.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
> +
> +#define MAX30210_STATUS_REG		0x00
Up to you but I'm very much a fan of register address and fields together
in the code, using a bit of extra whitespace to make it obvious what
is going on. E.g. 
#define MAX30210_STATUS_REG		0x00
#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_INT_EN_REG		0x02
...

That means everything related to the register is one place
which saves me jumping around when reviewing.

> +#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_A_FULL_MASK		GENMASK(5, 0)
> +#define MAX30210_FIFOCONF1_FLUSH_FIFO_MASK	BIT(4)
> +
> +#define MAX30210_SYSCONF_RESET_MASK		BIT(0)

We often drop the MASK postfix for single bit flags where
the meaning is obvious like this one.
#define MAX30210_SYSCONF_RESET			BIT(0)
Is fine and feels more normal when you use it in the code.

Be careful though when doing this.  Sometimes a field is simply
one that takes only values 0 and 1 but is not representing a boolean.
Those ones need the _MASK postfix and use of FIELD_GET / FIELD_PREP.
A recent example of that was a register bank selector in another driver.
There were only two banks so it was one bit but the code was easier
to read with it being via FIELD_PREP(_BANK_MASK, 0) etc

> +
> +#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)

> +
> +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);
> +
> +		/*
> +		 * TEMP_PERIOD is an index into the sampling frequency lookup
> +		 * table. Clamp in case the register holds a reserved value.
> +		 */
> +		uval = min_t(unsigned int, uval, ARRAY_SIZE(max30210_samp_freq_avail) - 1);

min() without very good reasons.

> +
> +		*val = max30210_samp_freq_avail[uval][0];
> +		*val2 = max30210_samp_freq_avail[uval][1];
> +
> +		return IIO_VAL_INT_PLUS_MICRO;
> +	case IIO_CHAN_INFO_RAW: {
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;
> +
> +		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_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;

Not important but I'd move this check before the DIRECT_MODE claim
as it has nothing to do with device state.

> +
> +		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])
> +				continue;
> +
> +			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;
> +
> +	/* Enable FIFO-full interrupt */

More comments to think about whether they add anything useful over
the code.  If they do maybe the defines need their names to be
tweaked!  E.g. ALMOST_FULL would be clearer.  You don't need
to exactly match datasheets where space demands etc may have
lead to things getting too abbreviated to be useful!

> +	ret = regmap_set_bits(st->regmap, MAX30210_INT_EN_REG,
> +			      MAX30210_STATUS_A_FULL_MASK);
> +	if (ret)
> +		return ret;
> +
> +	/* Flush FIFO before starting autonomous conversions */
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	/* Start autonomous temperature conversion */
> +	return regmap_write(st->regmap, MAX30210_TEMP_CONV_REG,
> +			    MAX30210_TEMPCONV_AUTO_MASK | MAX30210_TEMPCONV_CONV_T_MASK);

If you do use MASK prefix, then I'd expect to always have FIELD_PREP()
involved.  So another reason to drop _MASK from these things
that have obvious meaning without.  Even then I'd be temped
to use FIELD_PREP simply to align with the off case I mention below.

> +}
> +
> +static int max30210_buffer_postdisable(struct iio_dev *indio_dev)
> +{
> +	struct max30210_state *st = iio_priv(indio_dev);
> +	int ret;
> +
> +	/* Stop autonomous conversion */

Not obvious why you'd write the whole register. Perhaps
specify that 0 as

FIELD_PREP(MAX30210_TEMP_CONV_AUTO, 0) |
FIELD_PREP(MAX30210_TEMP_CONV_T, 0),

Compiler can flatten that to 0.


> +	ret = regmap_write(st->regmap, MAX30210_TEMP_CONV_REG, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	/* Flush FIFO */
> +	ret = regmap_set_bits(st->regmap, MAX30210_FIFO_CONF_2_REG,
> +			      MAX30210_FIFOCONF1_FLUSH_FIFO_MASK);
> +	if (ret)
> +		return ret;
> +
> +	/* Disable FIFO-full interrupt */
> +	return regmap_clear_bits(st->regmap, MAX30210_INT_EN_REG,
> +				 MAX30210_STATUS_A_FULL_MASK);

Similar to below, the use of _MASK postfix for a flag like this to
me makes the code harder to read because I feel a need to go check
that it is just one bit.  Anyhow, that applies in lots of places.

> +}


> +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 */

That is kind of obvious from the code so I'd drop the comment.

> +	powerdown_gpio = devm_gpiod_get_optional(dev, "powerdown",
> +						 GPIOD_OUT_HIGH);
It is fine to go a bit long on lines if it helps readability.
Here the gain is minor but it's only a few chars long.

> +	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 */
Also obvious.  Check all your comments for ones that are saying things
that are naturally true.

> +		ret = regmap_set_bits(st->regmap, MAX30210_SYS_CONF_REG,
> +				      MAX30210_SYSCONF_RESET_MASK);

Comment at the top about use of _MASK postfix was from seeing this line.

> +		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 */

This is a good comment so keep it. Not obvious to anyone seeing the code
that the register is read to clear.

> +	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;
When I see one of these I immediate search for uses of smbus
functions to check it's right.  Obviously here they are all hidden
by regmap which got me wondering why it wasn't regmap doing this
check...  So 
https://elixir.bootlin.com/linux/v7.2.5/source/drivers/base/regmap/regmap-i2c.c#L350
for val_bits = 8 and reg_bits = 8 there are two possible paths.
	else if (config->val_bits == 8 && config->reg_bits == 8 &&
		 i2c_check_functionality(i2c->adapter,
					 I2C_FUNC_SMBUS_I2C_BLOCK))
		bus = &regmap_i2c_smbus_i2c_block;
	}
or
	else if (config->val_bits == 8 && config->reg_bits == 8 &&
		 i2c_check_functionality(i2c->adapter,
					 I2C_FUNC_SMBUS_BYTE_DATA))
		bus = &regmap_smbus_byte;
	}

The first one relies on auto increment behaviour for multi reg reads.
The second doesn't.  I guess this works because a device that doesn't
do autoincrement reads won't have a driver that issues them.

Anyhow upshot is it is already checked, so you don't need another check
in the driver.  At somepoint I'll do a sweep for these and remove
any that might get cut and paste into new drivers.

> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
> +	if (!indio_dev)
> +		return -ENOMEM;

Re: [PATCH v3 0/2] Add support for Analog Devices MAX30210

[Andy Shevchenko]

On Fri, Sep 11, 2026 at 06:12:15AM +0800, John Erasmus Mari Geronimo wrote:
> Add IIO support and device-tree bindings for the Analog Devices MAX30210
> low-power digital temperature sensor.

This doesn't explain why a brand new driver is required. We have already tons
of maxXXXX drivers in the same folder, do they not suit to host the support of
a new device?

> Changes since v2:
> - Fix FIFO byte-count handling and sample decoding.
> - Fix IIO debug register read/write access.
> - Validate event threshold values and sampling-frequency register values.
> - Return the correct sampling-frequency format and reject direct reads while
>   buffered capture is enabled.
> - Clean up the Kconfig entry and device-tree supply declaration.

Changelog would be good to have a link to the previous version on
lore.kernel.org.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/2] Add support for Analog Devices MAX30210

[Jonathan Cameron]

On Fri, 11 Sep 2026 06:12:15 +0800
John Erasmus Mari Geronimo <johnerasmusmari.geronimo@analog.com> wrote:

> Add IIO support and device-tree bindings for the Analog Devices MAX30210
> low-power digital temperature sensor.

Temperatures sensors submitted to IIO always need some descriptive text in
the cover letter to explain why they should be in IIO rather than hwmon.

Note I raised this on v1 and it's still not here.

Jonathan


> 
> Changes since v2:
> - Fix FIFO byte-count handling and sample decoding.
> - Fix IIO debug register read/write access.
> - Validate event threshold values and sampling-frequency register values.
> - Return the correct sampling-frequency format and reject direct reads while
>   buffered capture is enabled.
> - Clean up the Kconfig entry and device-tree supply declaration.
> 
> 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         |  61 ++
>  MAINTAINERS                                   |   8 +
>  drivers/iio/temperature/Kconfig               |  10 +
>  drivers/iio/temperature/Makefile              |   1 +
>  drivers/iio/temperature/max30210.c            | 689 ++++++++++++++++++
>  5 files changed, 769 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/temperature/adi,max30210.yaml
>  create mode 100644 drivers/iio/temperature/max30210.c
>