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

John Erasmus Mari Geronimo posted 2 patches 1 month, 1 week ago
[PATCH v2 2/2] iio: temperature: add support for Analog Devices MAX30210
Posted by John Erasmus Mari Geronimo 1 month, 1 week ago
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
Posted by Jonathan Cameron 1 month, 1 week ago
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 2/2] iio: temperature: add support for Analog Devices MAX30210
Posted by David Lechner 1 month, 1 week ago
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
Posted by kernel test robot 1 month, 1 week ago
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
Posted by kernel test robot 1 month, 1 week ago
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