[PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393

[PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393

[Nikhil Gautam]

Hi,

This series adds initial Industrial I/O subsystem support for the
Melexis MLX90393 3-axis magnetometer and temperature sensor.

The MLX90393 supports both I2C and SPI interfaces. This series
implements support for the I2C interface while keeping the driver
structure transport-independent to simplify future SPI support.

Currently supported features:

* Raw magnetic field measurements for X/Y/Z axes
* Raw temperature measurements
* Configurable gain/scale selection
* Configurable oversampling ratio
* Direct mode operation through the IIO subsystem
* I2C interface support

The driver has been tested on Raspberry Pi 5 hardware using an
MLX90393 sensor connected over I2C. Magnetic field and temperature
measurements were verified through the IIO sysfs interface.

Previous Submission:
Link: https://lore.kernel.org/linux-iio/20260510191010.155380-1-nikhilgtr@gmail.com/

Changes in v2:
[DT]
- Extended the DT binding to document power supply regulators and
  optional interrupt and trigger GPIOs.
[IIO]
- Removed the RFC tag based on reviewer feedback.
- Added a MAINTAINERS entry as part of the initial submission and
  expanded it in the driver patch.
- Reworked the scale availability implementation to simplify the
  data layout and eliminate the need for constructing a temporary
  table, avoiding potential race conditions.
- Replaced usleep_range() with fsleep() where appropriate and
  documented initialization delays.
- Simplified helper functions and improved error handling by
  returning directly where appropriate.
- Reduced unnecessary local variables and line wrapping to improve
  readability and align with kernel coding style.
- Added comments for lock protection and command definitions to
  improve code clarity.
- Switched to devm_mutex_init() and cleaned up include usage in
  accordance with the "include what you use" principle.
- Improved consistency across the driver, including conditional
  handling, switch statements, formatting, and general code style.
- Addressed all review comments from Jonathan Cameron.

I would like to thank Jonathan Cameron for the prompt and thorough
review of the previous revision. The detailed feedback on both the
submission process and the implementation has significantly improved
the quality and maintainability of this series.

Further review and comments are greatly appreciated.

Thanks,
Nikhil Gautam

Nikhil Gautam (2):
  dt-bindings: iio: magnetometer: add Melexis MLX90393
  iio: magnetometer: add support for Melexis MLX90393

 .../iio/magnetometer/melexis,mlx90393.yaml    |  55 ++
 MAINTAINERS                                   |   7 +
 drivers/iio/magnetometer/Kconfig              |  10 +
 drivers/iio/magnetometer/Makefile             |   2 +
 drivers/iio/magnetometer/mlx90393.h           |  74 ++
 drivers/iio/magnetometer/mlx90393_core.c      | 681 ++++++++++++++++++
 drivers/iio/magnetometer/mlx90393_i2c.c       |  72 ++
 7 files changed, 901 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
 create mode 100644 drivers/iio/magnetometer/mlx90393.h
 create mode 100644 drivers/iio/magnetometer/mlx90393_core.c
 create mode 100644 drivers/iio/magnetometer/mlx90393_i2c.c

-- 
2.39.5

[PATCH v2 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393

[Nikhil Gautam]

Add devicetree bindings for the Melexis MLX90393
3-axis magnetometer and temperature sensor.

The device supports magnetic field and temperature
measurements over I2C and SPI interfaces.

This initial binding documents the I2C interface.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 .../iio/magnetometer/melexis,mlx90393.yaml    | 55 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 61 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
new file mode 100644
index 000000000000..79e7e4a124b6
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/melexis,mlx90393.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Melexis MLX90393 magnetometer sensor
+
+maintainers:
+  - Nikhil Gautam <nikhilgtr@gmail.com>
+
+description:
+  Melexis MLX90393 3-axis magnetometer and temperature sensor.
+
+properties:
+  compatible:
+    const: melexis,mlx90393
+
+  reg:
+    maxItems: 1
+
+  vdd-supply: true
+  vddio-supply: true
+
+  interrupts:
+    maxItems: 1
+
+  trigger-gpios:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        magnetometer@c {
+            compatible = "melexis,mlx90393";
+            reg = <0x0c>;
+
+            interrupt-parent = <&gpio>;
+            interrupts = <17 IRQ_TYPE_EDGE_RISING>;
+
+            trigger-gpios = <&gpio 18 GPIO_ACTIVE_HIGH>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..e9ddcd12feb5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24926,6 +24926,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
 F:	drivers/iio/magnetometer/tmag5273.c
 
+MELEXIS MLX90393 MAGNETOMETER DRIVER
+M:	Nikhil Gautam <nikhilgtr@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
+
 TI TRF7970A NFC DRIVER
 M:	Mark Greer <mgreer@animalcreek.com>
 L:	linux-wireless@vger.kernel.org
-- 
2.39.5

Re: [PATCH v2 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The new entry for 'MELEXIS MLX90393 MAGNETOMETER DRIVER' in the MAINTAINERS file is inserted out of alphabetical order.
--

commit 756049ed43f6fd027532ae2aea78e2889e87ad8e
Author: Nikhil Gautam <nikhilgtr@gmail.com>

dt-bindings: iio: magnetometer: add Melexis MLX90393
    
This commit adds devicetree bindings for the Melexis MLX90393 3-axis
magnetometer and temperature sensor. The initial binding documents the
I2C interface.

[ ... ]

> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa163..e9ddcd12feb5e 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -24926,6 +24926,12 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/magnetometer/ti,tmag5273.yaml
>  F:	drivers/iio/magnetometer/tmag5273.c
>  
> +MELEXIS MLX90393 MAGNETOMETER DRIVER
> +M:	Nikhil Gautam <nikhilgtr@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
> +
>  TI TRF7970A NFC DRIVER

[Severity: Low]
This isn't a bug, but should this new entry be placed earlier in the file? 
The MAINTAINERS list is generally sorted alphabetically, and the "MELEXIS" 
driver block is currently being inserted in the "T" section, between 
"TI TMAG5273" and "TI TRF7970A".

>  M:	Mark Greer <mgreer@animalcreek.com>
>  L:	linux-wireless@vger.kernel.org

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618160141.11409-1-nikhilgtr@gmail.com?part=1

[PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Nikhil Gautam]

Add Industrial I/O subsystem support for the Melexis
MLX90393 3-axis magnetometer and temperature sensor.

The driver currently supports:

raw magnetic field measurements
raw temperature measurements
configurable gain/scale selection
configurable oversampling ratio
direct mode operation

The MLX90393 supports both I2C and SPI interfaces. This
initial implementation adds support for the I2C interface.

The driver is structured around a shared sensor core with
a small transport abstraction layer to simplify future SPI
support without duplicating sensor logic.

Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
---
 MAINTAINERS                              |   1 +
 drivers/iio/magnetometer/Kconfig         |  10 +
 drivers/iio/magnetometer/Makefile        |   2 +
 drivers/iio/magnetometer/mlx90393.h      |  74 +++
 drivers/iio/magnetometer/mlx90393_core.c | 681 +++++++++++++++++++++++
 drivers/iio/magnetometer/mlx90393_i2c.c  |  72 +++
 6 files changed, 840 insertions(+)
 create mode 100644 drivers/iio/magnetometer/mlx90393.h
 create mode 100644 drivers/iio/magnetometer/mlx90393_core.c
 create mode 100644 drivers/iio/magnetometer/mlx90393_i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e9ddcd12feb5..ef7eb6fec0c2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24931,6 +24931,7 @@ M:	Nikhil Gautam <nikhilgtr@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
+F:	drivers/iio/magnetometer/mlx90393*
 
 TI TRF7970A NFC DRIVER
 M:	Mark Greer <mgreer@animalcreek.com>
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index 3debf1320ad1..e6b74e7e3317 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -128,6 +128,16 @@ config HID_SENSOR_MAGNETOMETER_3D
 	  Say yes here to build support for the HID SENSOR
 	  Magnetometer 3D.
 
+config MLX90393
+	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
+	depends on I2C
+	help
+	  Say yes here to build support for the MELEXIS MLX90393 3-axis
+	  magnetometer.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mlx90393.
+
 config MMC35240
 	tristate "MEMSIC MMC35240 3-axis magnetic sensor"
 	select REGMAP_I2C
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 9297723a97d8..542c89d38a59 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -14,6 +14,8 @@ obj-$(CONFIG_BMC150_MAGN_SPI) += bmc150_magn_spi.o
 
 obj-$(CONFIG_MAG3110)	+= mag3110.o
 obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
+obj-$(CONFIG_MLX90393)		+= mlx90393_core.o
+obj-$(CONFIG_MLX90393)		+= mlx90393_i2c.o
 obj-$(CONFIG_MMC35240)	+= mmc35240.o
 
 obj-$(CONFIG_IIO_ST_MAGN_3AXIS) += st_magn.o
diff --git a/drivers/iio/magnetometer/mlx90393.h b/drivers/iio/magnetometer/mlx90393.h
new file mode 100644
index 000000000000..b3356f9521f8
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * MLX90393 magnetometer & temperature sensor driver
+ *
+ * Copyright (c) 2026 Nikhil Gautam <nikhilgtr@gmail.com>
+ */
+
+#ifndef MLX90393_H
+#define MLX90393_H
+
+#include <linux/bitops.h>
+#include <linux/bits.h>
+#include <linux/types.h>
+
+#define MLX90393_AXIS_MAX		2
+#define MLX90393_GAIN_MAX		8
+#define MLX90393_RES_MAX		4
+#define MLX90393_OSR2_MAX		4
+#define MLX90393_OSR_MAX		4
+
+#define MLX90393_CMD_MASK	GENMASK(7, 4)
+
+/* Commands (datasheet, Table 11 - Command List) */
+#define MLX90393_CMD_SB		0x10	/* Start Burst Mode */
+#define MLX90393_CMD_SW		0x20	/* Start Wake-up on Change Mode */
+#define MLX90393_CMD_SM		0x30	/* Start Single Measurement Mode */
+#define MLX90393_CMD_RM		0x40	/* Read Measurement */
+#define MLX90393_CMD_RR		0x50	/* Read Register */
+#define MLX90393_CMD_WR		0x60	/* Write Register */
+#define MLX90393_CMD_EX		0x80	/* Exit Mode */
+#define MLX90393_CMD_HR		0xD0	/* Memory Recall */
+#define MLX90393_CMD_HS		0xE0	/* Memory Store */
+#define MLX90393_CMD_RT		0xF0	/* Reset Device */
+
+#define MLX90393_MEASURE_Z	BIT(0)
+#define MLX90393_MEASURE_Y	BIT(1)
+#define MLX90393_MEASURE_X	BIT(2)
+#define MLX90393_MEASURE_TEMP	BIT(3)
+
+#define MLX90393_MEASURE_ALL \
+	(MLX90393_MEASURE_TEMP | MLX90393_MEASURE_X | \
+	MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)
+
+#define MLX90393_NUM_CHANNELS	4
+
+#define MLX90393_STATUS_RESP	GENMASK(1, 0)
+#define MLX90393_STATUS_RT	BIT(2)
+#define MLX90393_STATUS_ERROR	BIT(4)
+
+#define MLX90393_REG_CONF1	0x00
+#define MLX90393_REG_CONF2	0x01
+#define MLX90393_REG_CONF3	0x02
+#define MLX90393_REG_CONF4	0x03
+
+#define MLX90393_CONF1_GAIN_SEL		GENMASK(6, 4)
+#define MLX90393_CONF1_HALLCONF		GENMASK(3, 0)
+
+#define MLX90393_CONF3_OSR		GENMASK(1, 0)
+#define MLX90393_CONF3_DIG_FILT		GENMASK(4, 2)
+#define MLX90393_CONF3_RES_X		GENMASK(6, 5)
+#define MLX90393_CONF3_RES_Y		GENMASK(8, 7)
+#define MLX90393_CONF3_RES_Z		GENMASK(10, 9)
+#define MLX90393_CONF3_OSR2		GENMASK(12, 11)
+
+struct mlx90393_transfer_ops {
+	int (*xfer)(void *context, const u8 *tx, int tx_len,
+		    u8 *rx, int rx_len);
+};
+
+int mlx90393_core_probe(struct device *dev,
+			const struct mlx90393_transfer_ops *ops,
+			void *context);
+
+#endif
diff --git a/drivers/iio/magnetometer/mlx90393_core.c b/drivers/iio/magnetometer/mlx90393_core.c
new file mode 100644
index 000000000000..0ad4a30c0be9
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_core.c
@@ -0,0 +1,681 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MLX90393 magnetometer & temperature sensor driver
+ *
+ * Copyright (c) 2026 Nikhil Gautam <nikhilgtr@gmail.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/unaligned.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#include "mlx90393.h"
+
+struct mlx90393_data {
+	/* Protects sensor configuration and measurement operations */
+	struct mutex lock;
+	struct device *dev;
+	void *bus_context;
+	const struct mlx90393_transfer_ops *ops;
+	u8 gain_sel;
+	u8 hallconf;
+
+	u8 res_xy;
+	u8 res_z;
+
+	u8 dig_filt;
+	u8 osr;
+	u8 osr2;
+};
+
+enum mlx90393_channels {
+	MLX90393_CHAN_X,
+	MLX90393_CHAN_Y,
+	MLX90393_CHAN_Z,
+	MLX90393_CHAN_TEMP,
+};
+
+enum mlx90393_axis_type {
+	MLX90393_AXIS_TYPE_XY,
+	MLX90393_AXIS_TYPE_Z,
+};
+
+/* Datasheet: Table no.17 */
+static const int mlx90393_scale_table[MLX90393_AXIS_MAX]
+				[MLX90393_GAIN_MAX]
+				[MLX90393_RES_MAX] = {
+	/* XY axis */
+	{
+		{ 751, 1502, 3004, 6009},
+		{ 601, 1202, 2403, 4840},
+		{ 451, 901, 1803, 3605},
+		{ 376, 751, 1502, 3004},
+		{ 300, 601, 1202, 2403},
+		{ 250, 501, 1001, 2003},
+		{ 200, 401, 801, 1602},
+		{ 150, 300, 601, 1202},
+	},
+	/* Z axis */
+	{
+		{ 1210, 2420, 4840, 9680},
+		{ 968, 1936, 3872, 7744},
+		{ 726, 1452, 2904, 5808},
+		{ 605, 1210, 2420, 4840},
+		{ 484, 968, 1936, 3872},
+		{ 403, 807, 1613, 3227},
+		{ 323, 645, 1291, 2581},
+		{ 242, 484, 968, 1936},
+	}
+};
+
+static const int mlx90393_osr2_avail[MLX90393_OSR2_MAX] = {
+	0, 1, 2, 3,
+};
+
+static const int mlx90393_osr_avail[MLX90393_OSR_MAX] = {
+	1, 2, 4, 8,
+};
+
+#define MLX90393_CHAN(idx, axis, addr) { \
+	.type = IIO_MAGN, \
+	.modified = 1, \
+	.channel = idx, \
+	.address = addr, \
+	.channel2 = IIO_MOD_##axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
+		BIT(IIO_CHAN_INFO_SCALE), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),\
+	.info_mask_separate_available = \
+		BIT(IIO_CHAN_INFO_SCALE),  \
+	.info_mask_shared_by_type_available = \
+		BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+}
+
+static const struct iio_chan_spec mlx90393_channels[] = {
+	MLX90393_CHAN(0, X, MLX90393_CHAN_X),
+	MLX90393_CHAN(1, Y, MLX90393_CHAN_Y),
+	MLX90393_CHAN(2, Z, MLX90393_CHAN_Z),
+	{
+		.type = IIO_TEMP,
+		.address = MLX90393_CHAN_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+			BIT(IIO_CHAN_INFO_OFFSET) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_separate_available =
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+	},
+};
+
+/*
+ * Calculate total conversion time in microseconds.
+ *
+ * Formula derived from datasheet timing equations.
+ */
+
+static int mlx90393_get_tconv_us(struct mlx90393_data *data)
+{
+	const int osr = data->osr;
+	const int osr2 = data->osr2;
+	const int df = data->dig_filt;
+
+	int tconvm;
+	int tconvt;
+
+	int m = 3; /* X,Y,Z */
+
+	/*
+	 * Datasheet:
+	 * TCONVM = 67 + 64 * 2^OSR * (2 + 2^DIG_FILT)
+	 */
+	tconvm = 67 + (64 * BIT(osr) * (2 + BIT(df)));
+
+	/*
+	 * Datasheet:
+	 * TCONVT = 67 + 192 * 2^OSR2
+	 */
+	tconvt = 67 + (192 * BIT(osr2));
+	/*
+	 * Total conversion time:
+	 * TSTBY + TACTIVE + m * TCONVM + TCONVT + TCONV_END
+	 */
+	return 220 + 360 + (m * tconvm) + tconvt + 1100;
+}
+
+static int mlx90393_xfer(struct mlx90393_data *data,
+			 const u8 *tx, int tx_len,
+			 u8 *rx, int rx_len)
+{
+	return data->ops->xfer(data->bus_context,
+			tx, tx_len,
+			rx, rx_len);
+}
+
+static int mlx90393_check_status(u8 cmd, u8 status)
+{
+	/* Always validate error bit */
+	if (status & MLX90393_STATUS_ERROR)
+		return -EIO;
+
+	switch (cmd & MLX90393_CMD_MASK) {
+	case MLX90393_CMD_RM:
+		/*
+		 * D1:D0 indicates response availability
+		 * 00 means invalid/no measurement
+		 */
+		if ((status & MLX90393_STATUS_RESP) == 0)
+			return -EIO;
+		return 0;
+	case MLX90393_CMD_RT:
+		/* Reset acknowledge */
+		if (!(status & MLX90393_STATUS_RT))
+			return -EIO;
+		return 0;
+	default:
+		return 0;
+	}
+}
+
+static int mlx90393_write_cmd(struct mlx90393_data *data, u8 cmd)
+{
+	u8 status;
+	int ret;
+
+	ret = mlx90393_xfer(data, &cmd, 1, &status, 1);
+	if (ret)
+		return ret;
+
+	return mlx90393_check_status(cmd, status);
+}
+
+static int mlx90393_read_cmd(struct mlx90393_data *data, u8 cmd, u8 *rx,
+			     int rx_len)
+{
+	int ret;
+
+	ret = mlx90393_xfer(data, &cmd, 1, rx, rx_len);
+	if (ret)
+		return ret;
+
+	return mlx90393_check_status(cmd, rx[0]);
+}
+
+static int mlx90393_read_reg(struct mlx90393_data *data, u8 reg, u16 *val)
+{
+	u8 tx[2];
+	u8 rx[3];
+	int ret;
+
+	tx[0] = MLX90393_CMD_RR;
+	/* Register address is encoded in bits [7:2] */
+	tx[1] = reg << 2;
+
+	ret = mlx90393_xfer(data, tx, sizeof(tx), rx, sizeof(rx));
+	if (ret)
+		return ret;
+
+	ret = mlx90393_check_status(tx[0], rx[0]);
+	if (ret)
+		return ret;
+
+	*val = get_unaligned_be16(&rx[1]);
+
+	return 0;
+}
+
+static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)
+{
+	u8 tx[4];
+	u8 status;
+	int ret;
+
+	tx[0] = MLX90393_CMD_WR;
+	put_unaligned_be16(val, &tx[1]);
+	/* Register address is encoded in bits [7:2] */
+	tx[3] = reg << 2;
+
+	ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
+	if (ret)
+		return ret;
+
+	return mlx90393_check_status(tx[0], status);
+}
+
+static int mlx90393_update_bits(struct mlx90393_data *data, u8 reg_addr,
+				u16 mask, u16 val)
+{
+	u16 reg;
+	int ret;
+
+	ret = mlx90393_read_reg(data, reg_addr, &reg);
+	if (ret)
+		return ret;
+
+	reg &= ~mask;
+	reg |= (val << __ffs(mask)) & mask;
+
+	return mlx90393_write_reg(data, reg_addr, reg);
+}
+
+static int mlx90393_read_measurement(struct mlx90393_data *data,
+				     enum mlx90393_channels chan, int *val)
+{
+	u8 rx[9];
+	int ret;
+
+	/* Start measurement */
+	ret = mlx90393_write_cmd(data, MLX90393_CMD_SM | MLX90393_MEASURE_ALL);
+	if (ret)
+		return ret;
+
+	/* Wait conversion */
+	fsleep(mlx90393_get_tconv_us(data));
+
+	/* Read measurement */
+	ret = mlx90393_read_cmd(data, MLX90393_CMD_RM | MLX90393_MEASURE_ALL,
+				rx, sizeof(rx));
+	if (ret)
+		return ret;
+	/*
+	 * Measurement response layout:
+	 * [status][temp][x][y][z]
+	 */
+
+	switch (chan) {
+	case MLX90393_CHAN_TEMP:
+		*val = get_unaligned_be16(&rx[1]);
+		return 0;
+
+	case MLX90393_CHAN_X:
+		*val = sign_extend32(get_unaligned_be16(&rx[3]), 15);
+		return 0;
+
+	case MLX90393_CHAN_Y:
+		*val = sign_extend32(get_unaligned_be16(&rx[5]), 15);
+		return 0;
+
+	case MLX90393_CHAN_Z:
+		*val = sign_extend32(get_unaligned_be16(&rx[7]), 15);
+		return 0;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90393_get_scale(struct mlx90393_data *data,
+			      const struct iio_chan_spec *chan,
+			      int *val, int *val2)
+{
+	enum mlx90393_axis_type axis;
+	u8 res;
+
+	if (chan->channel2 == IIO_MOD_Z) {
+		axis = MLX90393_AXIS_TYPE_Z;
+		res = data->res_z;
+	} else {
+		axis = MLX90393_AXIS_TYPE_XY;
+		res = data->res_xy;
+	}
+
+	/*
+	 * Convert:
+	 * µT × 1000 → nT
+	 */
+	*val = 0;
+	*val2 = mlx90393_scale_table[axis][data->gain_sel][res];
+
+	return IIO_VAL_INT_PLUS_NANO;
+}
+
+static int mlx90393_find_scale(struct mlx90393_data *data, bool z_axis,
+			       int val, int val2,
+			       int *gain)
+{
+	u8 res;
+	enum mlx90393_axis_type axis;
+
+	if (z_axis) {
+		axis = MLX90393_AXIS_TYPE_Z;
+		res = data->res_z;
+	} else {
+		axis = MLX90393_AXIS_TYPE_XY;
+		res = data->res_xy;
+	}
+
+	if (val != 0)
+		return -EINVAL;
+
+	for (unsigned int i = 0; i < ARRAY_SIZE(mlx90393_scale_table[0]); i++)
+		if (mlx90393_scale_table[axis][i][res] == val2) {
+			*gain = i;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static int mlx90393_set_scale(struct mlx90393_data *data,
+			      const struct iio_chan_spec *chan,
+			      int val, int val2)
+{
+	bool z_axis;
+	int gain;
+	int ret;
+
+	z_axis = chan->channel2 == IIO_MOD_Z;
+
+	ret = mlx90393_find_scale(data, z_axis, val, val2, &gain);
+	if (ret)
+		return ret;
+
+	ret = mlx90393_update_bits(data, MLX90393_REG_CONF1, MLX90393_CONF1_GAIN_SEL,
+				   gain);
+	if (ret)
+		return ret;
+
+	data->gain_sel = gain;
+
+	return 0;
+}
+
+static int mlx90393_get_osr(struct mlx90393_data *data, int *val)
+{
+	*val = mlx90393_osr_avail[data->osr];
+
+	return IIO_VAL_INT;
+}
+
+static int mlx90393_find_osr(int val, int *osr)
+{
+	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
+		if (mlx90393_osr_avail[i] == val) {
+			*osr = i;
+			return 0;
+		}
+
+	return -EINVAL;
+}
+
+static int mlx90393_get_temp_osr2(struct mlx90393_data *data, int *val)
+{
+	*val = mlx90393_osr2_avail[data->osr2];
+	return IIO_VAL_INT;
+}
+
+static int mlx90393_set_osr(struct mlx90393_data *data, int val)
+{
+	int osr;
+	int ret;
+
+	ret = mlx90393_find_osr(val, &osr);
+	if (ret)
+		return ret;
+
+	if (osr == data->osr)
+		return 0;
+
+	ret = mlx90393_update_bits(data, MLX90393_REG_CONF3, MLX90393_CONF3_OSR,
+				   osr);
+	if (ret)
+		return ret;
+
+	data->osr = osr;
+	return 0;
+}
+
+static int mlx90393_set_temp_osr2(struct mlx90393_data *data, int val)
+{
+	int ret;
+
+	if (val < 0 || val >= MLX90393_OSR2_MAX)
+		return -EINVAL;
+
+	if (val == data->osr2)
+		return 0;
+
+	ret = mlx90393_update_bits(data, MLX90393_REG_CONF3, MLX90393_CONF3_OSR2,
+				   val);
+	if (ret)
+		return ret;
+
+	data->osr2 = val;
+
+	return 0;
+}
+
+static int mlx90393_write_raw_get_fmt(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return IIO_VAL_INT;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90393_write_raw(struct iio_dev *indio_dev,
+			      const struct iio_chan_spec *chan,
+			      int val, int val2,
+			      long mask)
+{
+	struct mlx90393_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE: {
+		guard(mutex)(&data->lock);
+		ret = mlx90393_set_scale(data, chan, val, val2);
+		return ret;
+	}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
+		guard(mutex)(&data->lock);
+		switch (chan->type) {
+		case IIO_TEMP:
+			return mlx90393_set_temp_osr2(data, val);
+
+		case IIO_MAGN:
+			return mlx90393_set_osr(data, val);
+
+		default:
+			return -EINVAL;
+		}
+	}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90393_read_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan,
+			     int *val, int *val2, long mask)
+{
+	struct mlx90393_data *data = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&data->lock);
+		ret = mlx90393_read_measurement(data, chan->address, val);
+		if (ret)
+			return ret;
+
+		return IIO_VAL_INT;
+		}
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->type) {
+		case IIO_MAGN:
+			return mlx90393_get_scale(data, chan, val, val2);
+
+		case IIO_TEMP:
+			/* Datasheet: 22124 millidegC/LSB */
+			*val = 0;
+			*val2 = 22124;
+			return IIO_VAL_INT_PLUS_MICRO;
+
+		default:
+			return -EINVAL;
+		}
+
+	case IIO_CHAN_INFO_OFFSET:
+		if (chan->type != IIO_TEMP)
+			return -EINVAL;
+
+		/* Datasheet: temperature offset */
+		*val = -45114;
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		switch (chan->type) {
+		case IIO_TEMP:
+			return mlx90393_get_temp_osr2(data, val);
+		case IIO_MAGN:
+			return mlx90393_get_osr(data, val);
+		default:
+			return -EINVAL;
+		}
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mlx90393_read_avail(struct iio_dev *indio_dev,
+			       const struct iio_chan_spec *chan,
+			       const int **vals,
+			       int *type,
+			       int *length,
+			       long mask)
+{
+	struct mlx90393_data *data = iio_priv(indio_dev);
+	static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];
+	enum mlx90393_axis_type axis;
+	u8 res;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE: {
+		guard(mutex)(&data->lock);
+		axis = chan->channel2 == IIO_MOD_Z;
+		res = axis ? data->res_z : data->res_xy;
+
+		for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
+			scale_avail[i][0] = 0;
+			scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
+		}
+
+		*vals = &scale_avail[0][0];
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
+		return IIO_AVAIL_LIST;
+	}
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		if (chan->type == IIO_TEMP) {
+			*vals = mlx90393_osr2_avail;
+			*type = IIO_VAL_INT;
+			*length = MLX90393_OSR2_MAX;
+		} else {
+			*vals = mlx90393_osr_avail;
+			*type = IIO_VAL_INT;
+			*length = MLX90393_OSR_MAX;
+		}
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EINVAL;
+	}
+	return -EINVAL;
+}
+
+static const struct iio_info mlx90393_info = {
+	.read_raw = mlx90393_read_raw,
+	.write_raw = mlx90393_write_raw,
+	.read_avail = mlx90393_read_avail,
+	.write_raw_get_fmt = mlx90393_write_raw_get_fmt,
+};
+
+static int mlx90393_init(struct mlx90393_data *data)
+{
+	int ret;
+	u16 reg;
+
+	/* Exit mode */
+	ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
+	if (ret)
+		return ret;
+
+	/* Wait for device comes out of reset */
+	fsleep(1000);
+
+	/* Reset device */
+	ret = mlx90393_write_cmd(data, MLX90393_CMD_RT);
+	if (ret)
+		return ret;
+
+	/* Wait for device to reset */
+	fsleep(6000);
+
+	ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, &reg);
+	if (ret)
+		return ret;
+
+	data->gain_sel = FIELD_GET(MLX90393_CONF1_GAIN_SEL, reg);
+	data->hallconf = FIELD_GET(MLX90393_CONF1_HALLCONF, reg);
+
+	ret = mlx90393_read_reg(data, MLX90393_REG_CONF3, &reg);
+	if (ret)
+		return ret;
+
+	data->res_xy = FIELD_GET(MLX90393_CONF3_RES_X, reg);
+	data->res_z = FIELD_GET(MLX90393_CONF3_RES_Z, reg);
+	data->dig_filt = FIELD_GET(MLX90393_CONF3_DIG_FILT, reg);
+	data->osr = FIELD_GET(MLX90393_CONF3_OSR, reg);
+	data->osr2 = FIELD_GET(MLX90393_CONF3_OSR2, reg);
+
+	return 0;
+}
+
+int mlx90393_core_probe(struct device *dev,
+			const struct mlx90393_transfer_ops *ops,
+			void *context)
+{
+	struct iio_dev *indio_dev;
+	struct mlx90393_data *data;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+	devm_mutex_init(dev, &data->lock);
+
+	data->dev = dev;
+	data->ops = ops;
+	data->bus_context = context;
+
+	indio_dev->name = "mlx90393";
+	indio_dev->info = &mlx90393_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = mlx90393_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mlx90393_channels);
+
+	ret = mlx90393_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize device\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL_GPL(mlx90393_core_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Nikhil Gautam <nikhilgtr@gmail.com>");
+MODULE_DESCRIPTION("MLX90393 magnetometer sensor driver");
diff --git a/drivers/iio/magnetometer/mlx90393_i2c.c b/drivers/iio/magnetometer/mlx90393_i2c.c
new file mode 100644
index 000000000000..52233b6295c2
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_i2c.c
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+
+#include "mlx90393.h"
+
+/*
+ * MLX90393 commands use repeated-start transfers where
+ * every command is followed by a status/data response.
+ */
+static int mlx90393_i2c_xfer(void *context,
+			     const u8 *tx, int tx_len,
+			     u8 *rx, int rx_len)
+{
+	struct i2c_client *client = context;
+	int ret;
+	struct i2c_msg msgs[2] = {
+		[0] = {
+			.addr = client->addr,
+			.len = tx_len,
+			.buf = (u8 *)tx,
+		},
+		[1] = {
+			.addr = client->addr,
+			.flags = I2C_M_RD,
+			.len = rx_len,
+			.buf = rx,
+		},
+	};
+
+	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+	if (ret != ARRAY_SIZE(msgs))
+		return ret < 0 ? ret : -EIO;
+
+	return 0;
+}
+
+static const struct mlx90393_transfer_ops mlx90393_i2c_ops = {
+	.xfer = mlx90393_i2c_xfer,
+};
+
+static int mlx90393_i2c_probe(struct i2c_client *client)
+{
+	return mlx90393_core_probe(&client->dev, &mlx90393_i2c_ops, client);
+}
+
+static const struct i2c_device_id mlx90393_id[] = {
+	{ "mlx90393" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mlx90393_id);
+
+static const struct of_device_id mlx90393_of_match[] = {
+	{ .compatible = "melexis,mlx90393" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mlx90393_of_match);
+
+static struct i2c_driver mlx90393_i2c_driver = {
+	.driver = {
+		.name = "mlx90393",
+		.of_match_table = mlx90393_of_match,
+	},
+	.probe = mlx90393_i2c_probe,
+};
+
+module_i2c_driver(mlx90393_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Nikhil Gautam <nikhilgtr@gmail.com>");
+MODULE_DESCRIPTION("MLX90393 magnetometer sensor driver");
-- 
2.39.5

Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Data race on statically allocated `scale_avail` array.
- [High] DMA safety violation due to stack-allocated transfer buffers.
- [High] Writing to the temperature scale attribute corrupts the magnetometer gain configuration.
- [Medium] IIO scale values violate ABI standard (Tesla and Celsius instead of Gauss and milli-Celsius).
- [Medium] Missing `id_table` assignment in the I2C driver structure.
- [Medium] Temperature channel exposes raw register exponent values instead of true oversampling ratios.
- [Medium] Lockless state reads in `mlx90393_read_raw` lead to data races.
--

commit bdec68e57f45f6e35e5c0c76d37b8ee5efa9691b
Author: Nikhil Gautam <nikhilgtr@gmail.com>

iio: magnetometer: add support for Melexis MLX90393

Add Industrial I/O subsystem support for the Melexis
MLX90393 3-axis magnetometer and temperature sensor.

> diff --git a/drivers/iio/magnetometer/mlx90393_core.c b/drivers/iio/magnetometer/mlx90393_core.c
> new file mode 100644
> index 0000000000000..0ad4a30c0be96
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_core.c

[ ... ]

> +static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)
> +{
> +	u8 tx[4];
> +	u8 status;
> +	int ret;
> +
> +	tx[0] = MLX90393_CMD_WR;
> +	put_unaligned_be16(val, &tx[1]);
> +	/* Register address is encoded in bits [7:2] */
> +	tx[3] = reg << 2;
> +
> +	ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);

[Severity: High]
Is it safe to allocate the transfer buffers on the stack here and in
mlx90393_read_measurement()? 

If the I2C host adapter uses DMA and CONFIG_VMAP_STACK is enabled, mapping
stack memory for DMA operations will fail. Should these buffers be
dynamically allocated to ensure DMA safety?

[ ... ]

> +static int mlx90393_get_scale(struct mlx90393_data *data,
> +			      const struct iio_chan_spec *chan,
> +			      int *val, int *val2)
> +{
> +	enum mlx90393_axis_type axis;
> +	u8 res;
> +
> +	if (chan->channel2 == IIO_MOD_Z) {
> +		axis = MLX90393_AXIS_TYPE_Z;
> +		res = data->res_z;
> +	} else {
> +		axis = MLX90393_AXIS_TYPE_XY;
> +		res = data->res_xy;
> +	}
> +
> +	/*
> +	 * Convert:
> +	 * µT × 1000 → nT
> +	 */
> +	*val = 0;
> +	*val2 = mlx90393_scale_table[axis][data->gain_sel][res];
> +
> +	return IIO_VAL_INT_PLUS_NANO;
> +}

[Severity: Medium]
Does this return the scale in Gauss as required by the IIO ABI for
in_magn_scale? Using IIO_VAL_INT_PLUS_NANO with the current scale table yields
Tesla, which is off by a factor of 10,000 compared to Gauss.

[ ... ]

> +static int mlx90393_write_raw(struct iio_dev *indio_dev,
> +			      const struct iio_chan_spec *chan,
> +			      int val, int val2,
> +			      long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		ret = mlx90393_set_scale(data, chan, val, val2);
> +		return ret;
> +	}

[Severity: High]
Can writing to the temperature scale attribute corrupt the magnetometer gain
configuration?

The IIO_TEMP channel exposes a writable scale attribute, but this case does
not check chan->type before calling mlx90393_set_scale(). Because the
temperature channel's channel2 field is 0, it evaluates as an XY magnetometer
channel and will reconfigure the global gain_sel register.

[ ... ]

> +static int mlx90393_read_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan,
> +			     int *val, int *val2, long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&data->lock);
> +		ret = mlx90393_read_measurement(data, chan->address, val);
> +		if (ret)
> +			return ret;
> +
> +		return IIO_VAL_INT;
> +		}
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->type) {
> +		case IIO_MAGN:
> +			return mlx90393_get_scale(data, chan, val, val2);

[Severity: Medium]
Is a lock needed here?

Configuration variables like gain_sel, osr, and osr2 are updated while
holding data->lock in mlx90393_write_raw(), but mlx90393_read_raw() reads
these properties without acquiring the lock.

Can concurrent updates result in stale or inconsistently-read configurations?

> +
> +		case IIO_TEMP:
> +			/* Datasheet: 22124 millidegC/LSB */
> +			*val = 0;
> +			*val2 = 22124;
> +			return IIO_VAL_INT_PLUS_MICRO;

[Severity: Medium]
Does this return the temperature scale in milli-Celsius as required by the IIO
ABI? Returning 22124 with IIO_VAL_INT_PLUS_MICRO yields Celsius, which is off
by a factor of 1,000.

[ ... ]

> +static int mlx90393_read_avail(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       const int **vals,
> +			       int *type,
> +			       int *length,
> +			       long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);
> +	static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];

[Severity: High]
Is it safe to use a statically allocated local array for scale_avail?

The lock is released before the IIO core iterates over the returned pointer.
Can concurrent sysfs reads across different channels or multiple device
instances clobber the shared array and return corrupted lists to userspace?

> +	enum mlx90393_axis_type axis;
> +	u8 res;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		axis = chan->channel2 == IIO_MOD_Z;
> +		res = axis ? data->res_z : data->res_xy;
> +
> +		for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
> +			scale_avail[i][0] = 0;
> +			scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
> +		}
> +
> +		*vals = &scale_avail[0][0];
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		*length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
> +		return IIO_AVAIL_LIST;
> +	}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		if (chan->type == IIO_TEMP) {
> +			*vals = mlx90393_osr2_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR2_MAX;

[Severity: Medium]
Does this expose raw register exponent values instead of true oversampling
ratios for the temperature channel?

Returning the register exponents (0, 1, 2, 3) directly violates the IIO ABI
which dictates reflecting the actual number of hardware samples applied.

[ ... ]

> diff --git a/drivers/iio/magnetometer/mlx90393_i2c.c b/drivers/iio/magnetometer/mlx90393_i2c.c
> new file mode 100644
> index 0000000000000..52233b6295c2b
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_i2c.c

[ ... ]

> +static struct i2c_driver mlx90393_i2c_driver = {
> +	.driver = {
> +		.name = "mlx90393",
> +		.of_match_table = mlx90393_of_match,
> +	},
> +	.probe = mlx90393_i2c_probe,
> +};

[Severity: Medium]
Is the id_table assignment missing here?

The driver defines the mlx90393_id table and exports it, but it is not
assigned to the id_table field of this structure. Could this prevent the
driver from binding on systems relying on legacy board-file setups or sysfs
new_device instantiation?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618160141.11409-1-nikhilgtr@gmail.com?part=2

Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Uwe Kleine-König]

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

Hello,

On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> +static const struct i2c_device_id mlx90393_id[] = {
> +	{ "mlx90393" },
> +	{ }

Please make that:

	{ .name = "mlx90393" },


> +};
> +MODULE_DEVICE_TABLE(i2c, mlx90393_id);
> +
> +static const struct of_device_id mlx90393_of_match[] = {
> +	{ .compatible = "melexis,mlx90393" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, mlx90393_of_match);
> +
> +static struct i2c_driver mlx90393_i2c_driver = {
> +	.driver = {
> +		.name = "mlx90393",
> +		.of_match_table = mlx90393_of_match,
> +	},
> +	.probe = mlx90393_i2c_probe,

I guess you want

	.id_table = mlx90393_id,

here.

Best regards
Uwe

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

Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Nikhil Gautam]

On 18-06-2026 10:55 pm, Uwe Kleine-König wrote:
> Hello,
>
> On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> Please make that:
>
> 	{ .name = "mlx90393" },
Thanks for pointing this out. I'll update it in v3.
> I guess you want
>
> 	.id_table = mlx90393_id,
Yes, that's correct. I'll add the missing .id_table entry in the next 
version.

Thanks for the review!

Best regards,
Nikhil

Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Andy Shevchenko]

On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
> Add Industrial I/O subsystem support for the Melexis
> MLX90393 3-axis magnetometer and temperature sensor.
> 
> The driver currently supports:
> 
> raw magnetic field measurements
> raw temperature measurements
> configurable gain/scale selection
> configurable oversampling ratio
> direct mode operation
> 
> The MLX90393 supports both I2C and SPI interfaces. This
> initial implementation adds support for the I2C interface.
> 
> The driver is structured around a shared sensor core with
> a small transport abstraction layer to simplify future SPI
> support without duplicating sensor logic.

...

> +config MLX90393
> +	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
> +	depends on I2C

Why not a regmap?

> +	help
> +	  Say yes here to build support for the MELEXIS MLX90393 3-axis
> +	  magnetometer.
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called mlx90393.
> +
>  config MMC35240
>  	tristate "MEMSIC MMC35240 3-axis magnetic sensor"
>  	select REGMAP_I2C
> diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
> index 9297723a97d8..542c89d38a59 100644
> --- a/drivers/iio/magnetometer/Makefile
> +++ b/drivers/iio/magnetometer/Makefile
> @@ -14,6 +14,8 @@ obj-$(CONFIG_BMC150_MAGN_SPI) += bmc150_magn_spi.o
>  
>  obj-$(CONFIG_MAG3110)	+= mag3110.o
>  obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
> +obj-$(CONFIG_MLX90393)		+= mlx90393_core.o
> +obj-$(CONFIG_MLX90393)		+= mlx90393_i2c.o
>  obj-$(CONFIG_MMC35240)	+= mmc35240.o

...

> +#ifndef MLX90393_H
> +#define MLX90393_H

> +#include <linux/bitops.h>

> +#include <linux/bits.h>

Not required, it's covered by bitops.h.

> +#include <linux/types.h>

...

> +#define MLX90393_MEASURE_ALL \
> +	(MLX90393_MEASURE_TEMP | MLX90393_MEASURE_X | \
> +	MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)

Split logically.

#define MLX90393_MEASURE_ALL \
	(MLX90393_MEASURE_TEMP | \
	 MLX90393_MEASURE_X | MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)

Or just a (long) single line.

...

> +	int (*xfer)(void *context, const u8 *tx, int tx_len,
> +		    u8 *rx, int rx_len);

One line, it's only 81 characters.

> +};
> +
> +int mlx90393_core_probe(struct device *dev,

You want forward declaration for struct device.

> +			const struct mlx90393_transfer_ops *ops,
> +			void *context);
> +

...

+ array_size.h
+ bitfield.h // FIELD_GET()

> +#include <linux/delay.h>

+ errno.h // -Exxx

> +#include <linux/module.h>
> +#include <linux/mutex.h>

+ types.h // uXX

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

IWYU, please (just pointed out a few missing above, there are more).

...

> +/* Datasheet: Table no.17 */
> +static const int mlx90393_scale_table[MLX90393_AXIS_MAX]
> +				[MLX90393_GAIN_MAX]
> +				[MLX90393_RES_MAX] = {

This is broken indentation.

> +	/* XY axis */
> +	{
> +		{ 751, 1502, 3004, 6009},
> +		{ 601, 1202, 2403, 4840},
> +		{ 451, 901, 1803, 3605},
> +		{ 376, 751, 1502, 3004},
> +		{ 300, 601, 1202, 2403},
> +		{ 250, 501, 1001, 2003},
> +		{ 200, 401, 801, 1602},
> +		{ 150, 300, 601, 1202},
> +	},
> +	/* Z axis */
> +	{
> +		{ 1210, 2420, 4840, 9680},
> +		{ 968, 1936, 3872, 7744},
> +		{ 726, 1452, 2904, 5808},
> +		{ 605, 1210, 2420, 4840},
> +		{ 484, 968, 1936, 3872},
> +		{ 403, 807, 1613, 3227},
> +		{ 323, 645, 1291, 2581},
> +		{ 242, 484, 968, 1936},
> +	}
> +};

...

> +/*
> + * Calculate total conversion time in microseconds.
> + *
> + * Formula derived from datasheet timing equations.

Which formula? Where is datasheet? What if I have no access to it?
Always repeat the important details in the comment in the code.

> + */

> +

Unneeded blank line.

> +static int mlx90393_get_tconv_us(struct mlx90393_data *data)
> +{
> +	const int osr = data->osr;
> +	const int osr2 = data->osr2;
> +	const int df = data->dig_filt;
> +
> +	int tconvm;
> +	int tconvt;
> +
> +	int m = 3; /* X,Y,Z */
> +
> +	/*
> +	 * Datasheet:

What chapter/section/table name? Page number?

> +	 * TCONVM = 67 + 64 * 2^OSR * (2 + 2^DIG_FILT)

What does this cryptic message mean? Please, accompany this with more English
plain text.

> +	 */
> +	tconvm = 67 + (64 * BIT(osr) * (2 + BIT(df)));
> +
> +	/*
> +	 * Datasheet:
> +	 * TCONVT = 67 + 192 * 2^OSR2
> +	 */
> +	tconvt = 67 + (192 * BIT(osr2));
> +	/*
> +	 * Total conversion time:
> +	 * TSTBY + TACTIVE + m * TCONVM + TCONVT + TCONV_END
> +	 */
> +	return 220 + 360 + (m * tconvm) + tconvt + 1100;
> +}

...

> +static int mlx90393_xfer(struct mlx90393_data *data,
> +			 const u8 *tx, int tx_len,
> +			 u8 *rx, int rx_len)
> +{
> +	return data->ops->xfer(data->bus_context,
> +			tx, tx_len,
> +			rx, rx_len);

It's perfectly one line.

Also you might want to have

	if (!...->xfer)
		return -E...;

> +}

...

> +static int mlx90393_check_status(u8 cmd, u8 status)
> +{
> +	/* Always validate error bit */
> +	if (status & MLX90393_STATUS_ERROR)
> +		return -EIO;
> +
> +	switch (cmd & MLX90393_CMD_MASK) {
> +	case MLX90393_CMD_RM:
> +		/*
> +		 * D1:D0 indicates response availability
> +		 * 00 means invalid/no measurement
> +		 */
> +		if ((status & MLX90393_STATUS_RESP) == 0)
> +			return -EIO;
> +		return 0;
> +	case MLX90393_CMD_RT:
> +		/* Reset acknowledge */
> +		if (!(status & MLX90393_STATUS_RT))

For sake of consistency you might want to also compare to 0 here.

> +			return -EIO;
> +		return 0;
> +	default:
> +		return 0;
> +	}
> +}

...

> +static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)

Here the variable is named 'reg' there is 'reg_addr'. As I can see the code is
full of inconsistencies (like 2+ people with different style guidelines wrote
it). Please. take your time and check the code and make it consistent.

> +{
> +	u8 tx[4];
> +	u8 status;
> +	int ret;
> +
> +	tx[0] = MLX90393_CMD_WR;
> +	put_unaligned_be16(val, &tx[1]);
> +	/* Register address is encoded in bits [7:2] */
> +	tx[3] = reg << 2;
> +
> +	ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
> +	if (ret)
> +		return ret;
> +
> +	return mlx90393_check_status(tx[0], status);
> +}
> +
> +static int mlx90393_update_bits(struct mlx90393_data *data, u8 reg_addr,
> +				u16 mask, u16 val)
> +{
> +	u16 reg;
> +	int ret;
> +
> +	ret = mlx90393_read_reg(data, reg_addr, &reg);
> +	if (ret)
> +		return ret;

> +	reg &= ~mask;
> +	reg |= (val << __ffs(mask)) & mask;

bitfield.h has macros for this.


> +	return mlx90393_write_reg(data, reg_addr, reg);
> +}

...

> +static int mlx90393_find_osr(int val, int *osr)
> +{
> +	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
> +		if (mlx90393_osr_avail[i] == val) {
> +			*osr = i;
> +			return 0;
> +		}

Missing {}.

> +	return -EINVAL;
> +}

...

> +static int mlx90393_get_temp_osr2(struct mlx90393_data *data, int *val)
> +{
> +	*val = mlx90393_osr2_avail[data->osr2];

Missing blank line.

> +	return IIO_VAL_INT;
> +}

...

> +static int mlx90393_write_raw(struct iio_dev *indio_dev,
> +			      const struct iio_chan_spec *chan,
> +			      int val, int val2,
> +			      long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);

> +	int ret;

Not needed, return directly.

> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		ret = mlx90393_set_scale(data, chan, val, val2);
> +		return ret;
> +	}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO: {
> +		guard(mutex)(&data->lock);
> +		switch (chan->type) {
> +		case IIO_TEMP:
> +			return mlx90393_set_temp_osr2(data, val);
> +
> +		case IIO_MAGN:
> +			return mlx90393_set_osr(data, val);
> +
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +			/* Datasheet: 22124 millidegC/LSB */
> +		/* Datasheet: temperature offset */

Again, at least put a page, better to have Section/Table/et cetera title.

...

> +static int mlx90393_read_avail(struct iio_dev *indio_dev,
> +			       const struct iio_chan_spec *chan,
> +			       const int **vals,
> +			       int *type,
> +			       int *length,
> +			       long mask)
> +{
> +	struct mlx90393_data *data = iio_priv(indio_dev);
> +	static int scale_avail[MLX90393_GAIN_MAX][MLX90393_AXIS_MAX];
> +	enum mlx90393_axis_type axis;
> +	u8 res;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE: {
> +		guard(mutex)(&data->lock);
> +		axis = chan->channel2 == IIO_MOD_Z;
> +		res = axis ? data->res_z : data->res_xy;
> +
> +		for (unsigned int i = 0; i < MLX90393_GAIN_MAX; i++) {
> +			scale_avail[i][0] = 0;
> +			scale_avail[i][1] = mlx90393_scale_table[axis][i][res];
> +		}
> +
> +		*vals = &scale_avail[0][0];
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		*length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
> +		return IIO_AVAIL_LIST;
> +	}
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		if (chan->type == IIO_TEMP) {
> +			*vals = mlx90393_osr2_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR2_MAX;
> +		} else {
> +			*vals = mlx90393_osr_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR_MAX;
> +		}
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}

> +	return -EINVAL;

Besides missing blank line, this is actually a dead code.

> +}

...

> +static int mlx90393_init(struct mlx90393_data *data)
> +{
> +	int ret;
> +	u16 reg;
> +
> +	/* Exit mode */
> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for device comes out of reset */

Datasheet? Empirical?

> +	fsleep(1000);

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

> +	/* Reset device */
> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_RT);
> +	if (ret)
> +		return ret;
> +
> +	/* Wait for device to reset */
> +	fsleep(6000);

As per above.

> +	ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, &reg);
> +	if (ret)
> +		return ret;
> +
> +	data->gain_sel = FIELD_GET(MLX90393_CONF1_GAIN_SEL, reg);
> +	data->hallconf = FIELD_GET(MLX90393_CONF1_HALLCONF, reg);
> +
> +	ret = mlx90393_read_reg(data, MLX90393_REG_CONF3, &reg);
> +	if (ret)
> +		return ret;
> +
> +	data->res_xy = FIELD_GET(MLX90393_CONF3_RES_X, reg);
> +	data->res_z = FIELD_GET(MLX90393_CONF3_RES_Z, reg);
> +	data->dig_filt = FIELD_GET(MLX90393_CONF3_DIG_FILT, reg);
> +	data->osr = FIELD_GET(MLX90393_CONF3_OSR, reg);
> +	data->osr2 = FIELD_GET(MLX90393_CONF3_OSR2, reg);
> +
> +	return 0;
> +}

...

> +int mlx90393_core_probe(struct device *dev,
> +			const struct mlx90393_transfer_ops *ops,
> +			void *context)
> +{
> +	struct iio_dev *indio_dev;
> +	struct mlx90393_data *data;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	data = iio_priv(indio_dev);
> +	devm_mutex_init(dev, &data->lock);

Nonsense. If we don't check the return code of devm_*(), there is a little
reason to use it in the first place. But then one should not use devm further.
Easy fix: check for returned errors.

> +	data->dev = dev;
> +	data->ops = ops;
> +	data->bus_context = context;
> +
> +	indio_dev->name = "mlx90393";
> +	indio_dev->info = &mlx90393_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mlx90393_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(mlx90393_channels);
> +
> +	ret = mlx90393_init(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize device\n");
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}
> +EXPORT_SYMBOL_GPL(mlx90393_core_probe);

Make it namespaces.

...

+ array_size.h
+ errno.h

> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/mod_devicetable.h>

...and so on.

Same, follow the IWYU principle.

...

> +/*
> + * MLX90393 commands use repeated-start transfers where
> + * every command is followed by a status/data response.
> + */
> +static int mlx90393_i2c_xfer(void *context,
> +			     const u8 *tx, int tx_len,
> +			     u8 *rx, int rx_len)
> +{
> +	struct i2c_client *client = context;
> +	int ret;
> +	struct i2c_msg msgs[2] = {
> +		[0] = {
> +			.addr = client->addr,
> +			.len = tx_len,
> +			.buf = (u8 *)tx,
> +		},
> +		[1] = {
> +			.addr = client->addr,
> +			.flags = I2C_M_RD,
> +			.len = rx_len,
> +			.buf = rx,
> +		},
> +	};
> +
> +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> +	if (ret != ARRAY_SIZE(msgs))
> +		return ret < 0 ? ret : -EIO;

Please, make this to be the regular pattern

	if (ret < 0)
		return ret;
	if (ret != ARRAY_SIZE(msgs))
		return -EIO;

> +	return 0;
> +}

...

> +static struct i2c_driver mlx90393_i2c_driver = {
> +	.driver = {
> +		.name = "mlx90393",
> +		.of_match_table = mlx90393_of_match,
> +	},
> +	.probe = mlx90393_i2c_probe,
> +};

> +

Remove this blank line.

> +module_i2c_driver(mlx90393_i2c_driver);

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/2] iio: magnetometer: add support for Melexis MLX90393

[Nikhil Gautam]

On 19-06-2026 12:56 am, Andy Shevchenko wrote:
> On Thu, Jun 18, 2026 at 09:31:41PM +0530, Nikhil Gautam wrote:
Hi Andy,

Thank you very much for taking the time to do such a thorough review of 
this patch series.

Your detailed comments and suggestions are very helpful.
I'll address the issues you've pointed out, update the cover letter to 
better explain the design decisions,
and incorporate the requested coding style and API changes in the next 
revision.

I appreciate your review and feedback.
>> +config MLX90393
>> +	tristate "MELEXIS MLX90393 3-axis magnetometer sensor"
>> +	depends on I2C
> Why not a regmap?
The MLX90393 uses both register-based and command-based transactions.
Since regmap does not naturally model the command-based interface,
using it would require workarounds such as virtual registers or bypass 
paths.
A custom transport abstraction is therefore simpler and better suited 
for this device.

I already discussed this on thread, Link : 
https://lore.kernel.org/linux-iio/20260423121834.4244-1-nikhilgtr@gmail.com/ 

>> +#ifndef MLX90393_H
>> +#define MLX90393_H
>>
>> +#include <linux/bitops.h>
>> +#include <linux/bits.h>
> Not required, it's covered by bitops.h.
Noted. Thanks

>> +#include <linux/types.h>
> ...
>
>> +#define MLX90393_MEASURE_ALL \
>> +	(MLX90393_MEASURE_TEMP | MLX90393_MEASURE_X | \
>> +	MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)
> Split logically.
>
> #define MLX90393_MEASURE_ALL \
> 	(MLX90393_MEASURE_TEMP | \
> 	 MLX90393_MEASURE_X | MLX90393_MEASURE_Y | MLX90393_MEASURE_Z)
>
> Or just a (long) single line.
Agreed. Thanks

>> +	int (*xfer)(void *context, const u8 *tx, int tx_len,
>> +		    u8 *rx, int rx_len);
> One line, it's only 81 characters.
Agreed Thanks

>> +};
>> +
>> +int mlx90393_core_probe(struct device *dev,
> You want forward declaration for struct device.
Agreed, Thanks

> + array_size.h
> + bitfield.h // FIELD_GET()
>
>> +#include <linux/delay.h>
> + errno.h // -Exxx
>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
> + types.h // uXX
>
>> +#include <linux/unaligned.h>
>> +#include <linux/units.h>
> IWYU, please (just pointed out a few missing above, there are more).
Sure, will do it.

>> +/* Datasheet: Table no.17 */
>> +static const int mlx90393_scale_table[MLX90393_AXIS_MAX]
>> +				[MLX90393_GAIN_MAX]
>> +				[MLX90393_RES_MAX] = {
> This is broken indentation.
Noted, Thanks

>> +/*
>> + * Calculate total conversion time in microseconds.
>> + *
>> + * Formula derived from datasheet timing equations.
> Which formula? Where is datasheet? What if I have no access to it?
> Always repeat the important details in the comment in the code.
Got it, will add all details from datasheet

>> + */
>> +
> Unneeded blank line.
Noted, Thanks

>> +static int mlx90393_get_tconv_us(struct mlx90393_data *data)
>> +{
>> +	const int osr = data->osr;
>> +	const int osr2 = data->osr2;
>> +	const int df = data->dig_filt;
>> +
>> +	int tconvm;
>> +	int tconvt;
>> +
>> +	int m = 3; /* X,Y,Z */
>> +
>> +	/*
>> +	 * Datasheet:
> What chapter/section/table name? Page number?
Sure will add.

>> +	 * TCONVM = 67 + 64 * 2^OSR * (2 + 2^DIG_FILT)
> What does this cryptic message mean? Please, accompany this with more English
> plain text.
Agreed, Thanks

>> +static int mlx90393_xfer(struct mlx90393_data *data,
>> +			 const u8 *tx, int tx_len,
>> +			 u8 *rx, int rx_len)
>> +{
>> +	return data->ops->xfer(data->bus_context,
>> +			tx, tx_len,
>> +			rx, rx_len);
> It's perfectly one line.
>
> Also you might want to have
>
> 	if (!...->xfer)
> 		return -E...;
>
>> +}
Noted, Thanks

>> +static int mlx90393_check_status(u8 cmd, u8 status)
>> +{
>> +	/* Always validate error bit */
>> +	if (status & MLX90393_STATUS_ERROR)
>> +		return -EIO;
>> +
>> +	switch (cmd & MLX90393_CMD_MASK) {
>> +	case MLX90393_CMD_RM:
>> +		/*
>> +		 * D1:D0 indicates response availability
>> +		 * 00 means invalid/no measurement
>> +		 */
>> +		if ((status & MLX90393_STATUS_RESP) == 0)
>> +			return -EIO;
>> +		return 0;
>> +	case MLX90393_CMD_RT:
>> +		/* Reset acknowledge */
>> +		if (!(status & MLX90393_STATUS_RT))
> For sake of consistency you might want to also compare to 0 here.
Thanks, I'll make that change.

>> +static int mlx90393_write_reg(struct mlx90393_data *data, u8 reg, u16 val)
> Here the variable is named 'reg' there is 'reg_addr'. As I can see the code is
> full of inconsistencies (like 2+ people with different style guidelines wrote
> it). Please. take your time and check the code and make it consistent.
Got it, will make it consistent across whole file.

>> +{
>> +	u8 tx[4];
>> +	u8 status;
>> +	int ret;
>> +
>> +	tx[0] = MLX90393_CMD_WR;
>> +	put_unaligned_be16(val, &tx[1]);
>> +	/* Register address is encoded in bits [7:2] */
>> +	tx[3] = reg << 2;
>> +
>> +	ret = mlx90393_xfer(data, tx, sizeof(tx), &status, 1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return mlx90393_check_status(tx[0], status);
>> +}
>> +
>> +static int mlx90393_update_bits(struct mlx90393_data *data, u8 reg_addr,
>> +				u16 mask, u16 val)
>> +{
>> +	u16 reg;
>> +	int ret;
>> +
>> +	ret = mlx90393_read_reg(data, reg_addr, &reg);
>> +	if (ret)
>> +		return ret;
>>
>> +	reg &= ~mask;
>> +	reg |= (val << __ffs(mask)) & mask;
> bitfield.h has macros for this.
Noted, Thanks
>> +static int mlx90393_find_osr(int val, int *osr)
>> +{
>> +	for (unsigned int i = 0; i < MLX90393_OSR_MAX;  i++)
>> +		if (mlx90393_osr_avail[i] == val) {
>> +			*osr = i;
>> +			return 0;
>> +		}
> Missing {}.
Agreed, removed intentionally as single statement, will add as per 
guidelines all the places
where needed

>> +static int mlx90393_get_temp_osr2(struct mlx90393_data *data, int *val)
>> +{
>> +	*val = mlx90393_osr2_avail[data->osr2];
> Missing blank line.
Noted, Thanks

>> int mlx90393_write_raw(struct iio_dev *indio_dev,
>> +			      const struct iio_chan_spec *chan,
>> +			      int val, int val2,
>> +			      long mask)
>> +{
>> +	struct mlx90393_data *data = iio_priv(indio_dev);
>> +	int ret;
> Not needed, return directly.
Agreed.

>> +			/* Datasheet: 22124 millidegC/LSB */
>> +		/* Datasheet: temperature offset */
> Again, at least put a page, better to have Section/Table/et cetera title.
Sure, will do it

> +			*vals = mlx90393_osr_avail;
> +			*type = IIO_VAL_INT;
> +			*length = MLX90393_OSR_MAX;
> +		}
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}
>> +	return -EINVAL;
> Besides missing blank line, this is actually a dead code.
Correct, this will never be called. will fix

>> +static int mlx90393_init(struct mlx90393_data *data)
>> +{
>> +	int ret;
>> +	u16 reg;
>> +
>> +	/* Exit mode */
>> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Wait for device comes out of reset */
> Datasheet? Empirical?
Agreed, Thanks

>> +	fsleep(1000);
> 1 * USEC_PER_MSEC
> (will require time.h to be included).
Noted, Thanks

>> +	/* Reset device */
>> +	ret = mlx90393_write_cmd(data, MLX90393_CMD_RT);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Wait for device to reset */
>> +	fsleep(6000);
> As per above.
This delay is not 6000ms it is 1500ms as per datasheet, forgot to update 
in code
Thanks for pointing out, will update all places delay requirement with 
datasheet reference.

>> +int mlx90393_core_probe(struct device *dev,
>> +			const struct mlx90393_transfer_ops *ops,
>> +			void *context)
>> +{
>> +	struct iio_dev *indio_dev;
>> +	struct mlx90393_data *data;
>> +	int ret;
>> +
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	data = iio_priv(indio_dev);
>> +	devm_mutex_init(dev, &data->lock);
> Nonsense. If we don't check the return code of devm_*(), there is a little
> reason to use it in the first place. But then one should not use devm further.
> Easy fix: check for returned errors.
Agreed, I'll fix this in the next revision.

>> +	data->dev = dev;
>> +	data->ops = ops;
>> +	data->bus_context = context;
>> +
>> +	indio_dev->name = "mlx90393";
>> +	indio_dev->info = &mlx90393_info;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	indio_dev->channels = mlx90393_channels;
>> +	indio_dev->num_channels = ARRAY_SIZE(mlx90393_channels);
>> +
>> +	ret = mlx90393_init(data);
>> +	if (ret)
>> +		return dev_err_probe(dev, ret, "failed to initialize device\n");
>> +
>> +	return devm_iio_device_register(dev, indio_dev);
>> +}
>> +EXPORT_SYMBOL_GPL(mlx90393_core_probe);
> Make it namespaces.
Okay, will do it
> + array_size.h
> + errno.h
>
>> +#include <linux/module.h>
>> +#include <linux/i2c.h>
>> +#include <linux/mod_devicetable.h>
> ...and so on.
>
> Same, follow the IWYU principle.
Got it, will take care
>> +/*
>> + * MLX90393 commands use repeated-start transfers where
>> + * every command is followed by a status/data response.
>> + */
>> +static int mlx90393_i2c_xfer(void *context,
>> +			     const u8 *tx, int tx_len,
>> +			     u8 *rx, int rx_len)
>> +{
>> +	struct i2c_client *client = context;
>> +	int ret;
>> +	struct i2c_msg msgs[2] = {
>> +		[0] = {
>> +			.addr = client->addr,
>> +			.len = tx_len,
>> +			.buf = (u8 *)tx,
>> +		},
>> +		[1] = {
>> +			.addr = client->addr,
>> +			.flags = I2C_M_RD,
>> +			.len = rx_len,
>> +			.buf = rx,
>> +		},
>> +	};
>> +
>> +	ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
>> +	if (ret != ARRAY_SIZE(msgs))
>> +		return ret < 0 ? ret : -EIO;
> Please, make this to be the regular pattern
>
> 	if (ret < 0)
> 		return ret;
> 	if (ret != ARRAY_SIZE(msgs))
> 		return -EIO;
>
>> +	return 0;
>> +}
Thanks, I'll make that change.
>> +static struct i2c_driver mlx90393_i2c_driver = {
>> +	.driver = {
>> +		.name = "mlx90393",
>> +		.of_match_table = mlx90393_of_match,
>> +	},
>> +	.probe = mlx90393_i2c_probe,
>> +};
> Remove this blank line.
Noted
>> +module_i2c_driver(mlx90393_i2c_driver);
>>

Thanks & Regards,
Nikhil

Re: [PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393

[Andy Shevchenko]

On Thu, Jun 18, 2026 at 09:31:39PM +0530, Nikhil Gautam wrote:
> Hi,
> 
> This series adds initial Industrial I/O subsystem support for the
> Melexis MLX90393 3-axis magnetometer and temperature sensor.
> 
> The MLX90393 supports both I2C and SPI interfaces. This series
> implements support for the I2C interface while keeping the driver
> structure transport-independent to simplify future SPI support.
> 
> Currently supported features:
> 
> * Raw magnetic field measurements for X/Y/Z axes
> * Raw temperature measurements
> * Configurable gain/scale selection
> * Configurable oversampling ratio
> * Direct mode operation through the IIO subsystem
> * I2C interface support
> 
> The driver has been tested on Raspberry Pi 5 hardware using an
> MLX90393 sensor connected over I2C. Magnetic field and temperature
> measurements were verified through the IIO sysfs interface.

This doesn't answer to two important questions:
- why do we need a brand new driver?
  Can't one of the existing be updated to cover this HW?

- where to find the datasheet? Any Links or other means to get it?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] iio: magnetometer: add support for Melexis MLX90393

[Nikhil Gautam]

On 19-06-2026 12:29 am, Andy Shevchenko wrote:
> On Thu, Jun 18, 2026 at 09:31:39PM +0530, Nikhil Gautam wrote:
>> Hi,
>>
>> This series adds initial Industrial I/O subsystem support for the
>> Melexis MLX90393 3-axis magnetometer and temperature sensor.
>>
>> The MLX90393 supports both I2C and SPI interfaces. This series
>> implements support for the I2C interface while keeping the driver
>> structure transport-independent to simplify future SPI support.
>>
>> Currently supported features:
>>
>> * Raw magnetic field measurements for X/Y/Z axes
>> * Raw temperature measurements
>> * Configurable gain/scale selection
>> * Configurable oversampling ratio
>> * Direct mode operation through the IIO subsystem
>> * I2C interface support
>>
>> The driver has been tested on Raspberry Pi 5 hardware using an
>> MLX90393 sensor connected over I2C. Magnetic field and temperature
>> measurements were verified through the IIO sysfs interface.
> This doesn't answer to two important questions:
> - why do we need a brand new driver?
>    Can't one of the existing be updated to cover this HW?
>
> - where to find the datasheet? Any Links or other means to get it?

Hi Andy,

I looked at the existing magnetometer drivers in the IIO subsystem,
but none support the MLX90393 or a sufficiently similar register map and 
command protocol.
The MLX90393 uses its own command-based interface and device-specific 
configuration and conversion logic,
so extending an existing driver would require substantial changes with 
little code reuse.
Therefore, I chose to implement it as a separate driver.

The datasheet is publicly available from Melexis:
Link: 
https://media.melexis.com/-/media/files/documents/datasheets/mlx90393-datasheet-melexis.pdf

I'll include the datasheet link and clarify the rationale for introducing
a new driver in the cover letter of the next revision.

Thanks,
Nikhil