* [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393
@ 2026-05-10 19:10 Nikhil Gautam
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Nikhil Gautam @ 2026-05-10 19:10 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel, 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 RFC
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
initially I2C support for this driver
The driver has been tested on Raspberry Pi 5 hardware using an
actual MLX90393 sensor connected over I2C. Raw magnetic field and
temperature values were verified through the IIO sysfs interface.
A custom transport abstraction was used instead of regmap due to
the MLX90393 command/response protocol semantics and repeated-start
transfer behavior. This approach was discussed previously on the
mailing list and seemed preferable for this device.
Link: https://lore.kernel.org/linux-iio/20260424114818.1290b029@jic23-huawei/
The implementation intentionally focuses on a minimal and reviewable
feature set for the initial RFC submission. Support for SPI,
buffered capture, triggers, interrupts and runtime PM can be added
incrementally in follow-up work.
The DT binding schema has been validated using dt_binding_check and
the driver has been checked using checkpatch.pl --strict.
Feedback on the overall driver structure, transport abstraction,
IIO ABI usage and DT binding would be 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 | 39 +
drivers/iio/magnetometer/Kconfig | 10 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/mlx90393.h | 76 ++
drivers/iio/magnetometer/mlx90393_core.c | 724 ++++++++++++++++++
drivers/iio/magnetometer/mlx90393_i2c.c | 71 ++
6 files changed, 922 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
^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393
2026-05-10 19:10 [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393 Nikhil Gautam
@ 2026-05-10 19:10 ` Nikhil Gautam
2026-05-11 13:56 ` Jonathan Cameron
2026-05-11 21:42 ` sashiko-bot
2026-05-10 19:10 ` [RFC PATCH 2/2] iio: magnetometer: add support for " Nikhil Gautam
2026-05-11 13:51 ` [RFC PATCH 0/2] " Jonathan Cameron
2 siblings, 2 replies; 8+ messages in thread
From: Nikhil Gautam @ 2026-05-10 19:10 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel, 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 | 39 +++++++++++++++++++
1 file changed, 39 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..b99629ff2585
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
@@ -0,0 +1,39 @@
+# 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:
+ enum:
+ - melexis,mlx90393
+
+ reg:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ magnetometer@c {
+ compatible = "melexis,mlx90393";
+ reg = <0x0c>;
+ };
+ };
--
2.39.5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [RFC PATCH 2/2] iio: magnetometer: add support for Melexis MLX90393
2026-05-10 19:10 [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393 Nikhil Gautam
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
@ 2026-05-10 19:10 ` Nikhil Gautam
2026-05-11 14:23 ` Jonathan Cameron
2026-05-11 22:13 ` sashiko-bot
2026-05-11 13:51 ` [RFC PATCH 0/2] " Jonathan Cameron
2 siblings, 2 replies; 8+ messages in thread
From: Nikhil Gautam @ 2026-05-10 19:10 UTC (permalink / raw)
To: jic23
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel, 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>
---
drivers/iio/magnetometer/Kconfig | 10 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/mlx90393.h | 76 +++
drivers/iio/magnetometer/mlx90393_core.c | 724 +++++++++++++++++++++++
drivers/iio/magnetometer/mlx90393_i2c.c | 71 +++
5 files changed, 883 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/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..f3e04aed67de
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393.h
@@ -0,0 +1,76 @@
+/* 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/device.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 */
+#define MLX90393_CMD_SB 0x10
+#define MLX90393_CMD_SW 0x20
+#define MLX90393_CMD_SM 0x30
+#define MLX90393_CMD_RM 0x40
+#define MLX90393_CMD_RR 0x50
+#define MLX90393_CMD_WR 0x60
+#define MLX90393_CMD_EX 0x80
+#define MLX90393_CMD_HR 0xD0
+#define MLX90393_CMD_HS 0xE0
+#define MLX90393_CMD_RT 0xF0
+
+#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..c79f2b8c20d8
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_core.c
@@ -0,0 +1,724 @@
+// 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/device.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 {
+ struct device *dev;
+ struct mutex lock;
+ 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 +
+ 100;
+}
+
+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;
+ break;
+
+ case MLX90393_CMD_RT:
+ /* Reset acknowledge */
+ if (!(status & MLX90393_STATUS_RT))
+ return -EIO;
+ break;
+
+ default:
+ break;
+ }
+
+ 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, ®);
+ 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 cmd;
+ u8 rx[9];
+ int ret;
+ int tconv_us = mlx90393_get_tconv_us(data);
+
+ /* Start measurement */
+ cmd = MLX90393_CMD_SM | MLX90393_MEASURE_ALL;
+
+ ret = mlx90393_write_cmd(data, cmd);
+ if (ret)
+ return ret;
+
+ /* Wait conversion */
+ usleep_range(tconv_us, tconv_us + 1000);
+
+ /* Read measurement */
+ cmd = MLX90393_CMD_RM | MLX90393_MEASURE_ALL;
+
+ ret = mlx90393_read_cmd(data, cmd, 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]);
+ break;
+
+ case MLX90393_CHAN_X:
+ *val = sign_extend32(get_unaligned_be16(&rx[3]), 15);
+ break;
+
+ case MLX90393_CHAN_Y:
+ *val = sign_extend32(get_unaligned_be16(&rx[5]), 15);
+ break;
+
+ case MLX90393_CHAN_Z:
+ *val = sign_extend32(get_unaligned_be16(&rx[7]), 15);
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+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)
+{
+ int i;
+ u8 res;
+ enum mlx90393_axis_type axis;
+
+ axis = z_axis ? MLX90393_AXIS_TYPE_Z :
+ MLX90393_AXIS_TYPE_XY;
+
+ res = z_axis ? data->res_z : data->res_xy;
+
+ if (val != 0)
+ return -EINVAL;
+
+ for (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)
+{
+ int i;
+
+ for (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:
+ mutex_lock(&data->lock);
+ ret = mlx90393_set_scale(data, chan, val, val2);
+ mutex_unlock(&data->lock);
+ return ret;
+
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ mutex_lock(&data->lock);
+ if (chan->type == IIO_TEMP)
+ ret = mlx90393_set_temp_osr2(data, val);
+ else if (chan->type == IIO_MAGN)
+ ret = mlx90393_set_osr(data, val);
+ else
+ ret = -EINVAL;
+ mutex_unlock(&data->lock);
+ return ret;
+ 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:
+ mutex_lock(&data->lock);
+ ret = mlx90393_read_measurement(data, chan->address, val);
+ mutex_unlock(&data->lock);
+ 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) {
+ /* Datasheet: temperature offset */
+ *val = -45114;
+ return IIO_VAL_INT;
+ }
+ return -EINVAL;
+
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ if (chan->type == IIO_TEMP)
+ return mlx90393_get_temp_osr2(data, val);
+ if (chan->type == IIO_MAGN)
+ return mlx90393_get_osr(data, val);
+ 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;
+ int i;
+ u8 res;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ axis = chan->channel2 == IIO_MOD_Z;
+ res = axis ? data->res_z : data->res_xy;
+
+ for (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;
+ break;
+ 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;
+ }
+
+ break;
+ default:
+ return -EINVAL;
+ }
+ return IIO_AVAIL_LIST;
+}
+
+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;
+ u8 cmd;
+ u16 reg;
+
+ /* Exit mode */
+ cmd = MLX90393_CMD_EX;
+ ret = mlx90393_write_cmd(data, cmd);
+ if (ret)
+ return ret;
+
+ usleep_range(1000, 1500);
+
+ /* Reset device */
+ cmd = MLX90393_CMD_RT;
+ ret = mlx90393_write_cmd(data, cmd);
+ if (ret)
+ return ret;
+
+ /* Wait for device to reset */
+ usleep_range(5000, 6000);
+
+ ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, ®);
+ 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, ®);
+ 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);
+ mutex_init(&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) {
+ dev_err_probe(dev, ret, "failed to initialize device\n");
+ return ret;
+ }
+
+ 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..09d533a96907
--- /dev/null
+++ b/drivers/iio/magnetometer/mlx90393_i2c.c
@@ -0,0 +1,71 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <linux/module.h>
+#include <linux/i2c.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];
+
+ msgs[0].addr = client->addr;
+ msgs[0].flags = 0;
+ msgs[0].len = tx_len;
+ msgs[0].buf = (u8 *)tx;
+
+ msgs[1].addr = client->addr;
+ msgs[1].flags = I2C_M_RD;
+ msgs[1].len = rx_len;
+ msgs[1].buf = rx;
+
+ ret = i2c_transfer(client->adapter, msgs, 2);
+ if (ret != 2)
+ 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", 0 },
+ { }
+};
+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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393
2026-05-10 19:10 [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393 Nikhil Gautam
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
2026-05-10 19:10 ` [RFC PATCH 2/2] iio: magnetometer: add support for " Nikhil Gautam
@ 2026-05-11 13:51 ` Jonathan Cameron
2 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-11 13:51 UTC (permalink / raw)
To: Nikhil Gautam
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel
On Mon, 11 May 2026 00:40:08 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:
> Hi,
Hi Nikhil.
Thanks for your driver
Before I look at it, a quick comment on use of RFC.
Generally we only use that for drivers that either have a series
of specific open questions (driver structure doesn't count for IIO
as we have a 'lot' of examples!) and those questions should be
at the top of the RFC.
The other exception is where you know something is not ready for merge
due to some dependency or similar on other code. Occasionally done
as a quick hint on how we might solve a problem that is raised.
Anyhow, just send this without RFC. You'll get comments anyway!
Probably more of them as many reviewers are short on time so will
only briefly glance an an RFC if at all!
Jonathan
>
> 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 RFC
> 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
> initially I2C support for this driver
>
> The driver has been tested on Raspberry Pi 5 hardware using an
> actual MLX90393 sensor connected over I2C. Raw magnetic field and
> temperature values were verified through the IIO sysfs interface.
>
> A custom transport abstraction was used instead of regmap due to
> the MLX90393 command/response protocol semantics and repeated-start
> transfer behavior. This approach was discussed previously on the
> mailing list and seemed preferable for this device.
> Link: https://lore.kernel.org/linux-iio/20260424114818.1290b029@jic23-huawei/
>
> The implementation intentionally focuses on a minimal and reviewable
> feature set for the initial RFC submission. Support for SPI,
> buffered capture, triggers, interrupts and runtime PM can be added
> incrementally in follow-up work.
>
> The DT binding schema has been validated using dt_binding_check and
> the driver has been checked using checkpatch.pl --strict.
>
> Feedback on the overall driver structure, transport abstraction,
> IIO ABI usage and DT binding would be 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 | 39 +
> drivers/iio/magnetometer/Kconfig | 10 +
> drivers/iio/magnetometer/Makefile | 2 +
> drivers/iio/magnetometer/mlx90393.h | 76 ++
> drivers/iio/magnetometer/mlx90393_core.c | 724 ++++++++++++++++++
> drivers/iio/magnetometer/mlx90393_i2c.c | 71 ++
> 6 files changed, 922 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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
@ 2026-05-11 13:56 ` Jonathan Cameron
2026-05-11 21:42 ` sashiko-bot
1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-11 13:56 UTC (permalink / raw)
To: Nikhil Gautam
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel
On Mon, 11 May 2026 00:40:09 +0530
Nikhil Gautam <nikhilgtr@gmail.com> wrote:
> 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>
Hi Nikhil,
Add a MAINTAINERS entry as part of this patch and expand it in patch 2 to cover
the driver. That's much cleaner than doing it after the fact as a separate patch.
> ---
> .../iio/magnetometer/melexis,mlx90393.yaml | 39 +++++++++++++++++++
> 1 file changed, 39 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..b99629ff2585
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
> @@ -0,0 +1,39 @@
> +# 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:
> + enum:
> + - melexis,mlx90393
> +
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
Definitely require some power... Looks like vdd-supply and vddio-supply
Also binding in general should be as complete as possible, even if the
driver you are sending it with only supports some stuff to start with.
So looks like there is an interrupt and a trigger pin that can possibly
be used with a GPIO. Seems they may be the same pin from the pinout so
maybe don't bother with trig.
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + magnetometer@c {
> + compatible = "melexis,mlx90393";
> + reg = <0x0c>;
> + };
> + };
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/2] iio: magnetometer: add support for Melexis MLX90393
2026-05-10 19:10 ` [RFC PATCH 2/2] iio: magnetometer: add support for " Nikhil Gautam
@ 2026-05-11 14:23 ` Jonathan Cameron
2026-05-11 22:13 ` sashiko-bot
1 sibling, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-05-11 14:23 UTC (permalink / raw)
To: Nikhil Gautam
Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio,
devicetree, linux-kernel
On Mon, 11 May 2026 00:40:10 +0530
Nikhil Gautam <nikhilgtr@gmail.com> 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.
>
> Signed-off-by: Nikhil Gautam <nikhilgtr@gmail.com>
Hi Nikhil
Quick process reminder - even though I've reviewed this the day you
sent it out (coincidence of when I am catching up with backlog!),
wait to send a v2 for at least a week so there is plenty of time
for others to review.
Various comments inline,
Thanks,
Jonathan
> diff --git a/drivers/iio/magnetometer/mlx90393.h b/drivers/iio/magnetometer/mlx90393.h
> new file mode 100644
> index 000000000000..f3e04aed67de
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393.h
> @@ -0,0 +1,76 @@
> +/* 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/device.h>
As below - generally don't want to see device.h included in a new driver.
> +#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 */
> +#define MLX90393_CMD_SB 0x10
> +#define MLX90393_CMD_SW 0x20
> +#define MLX90393_CMD_SM 0x30
> +#define MLX90393_CMD_RM 0x40
> +#define MLX90393_CMD_RR 0x50
> +#define MLX90393_CMD_WR 0x60
> +#define MLX90393_CMD_EX 0x80
> +#define MLX90393_CMD_HR 0xD0
> +#define MLX90393_CMD_HS 0xE0
> +#define MLX90393_CMD_RT 0xF0
Maybe use a few more characters as those are pretty opaque acronyms!
Fair enough if they are straight from the datasheet but in that case can we have
some comments.
> diff --git a/drivers/iio/magnetometer/mlx90393_core.c b/drivers/iio/magnetometer/mlx90393_core.c
> new file mode 100644
> index 000000000000..c79f2b8c20d8
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_core.c
> @@ -0,0 +1,724 @@
> +// 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/device.h>
Generally not needed - try and find more specific relevant includes.
Check to see if you need more - this is a very short set for a driver
and we generally try to follow include what you use principles in IIO.
> +#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 {
> + struct device *dev;
> + struct mutex lock;
Locks should always have a comment saying what data they are protecting.
> + 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;
> +};
> +
> +/* 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},
{ 751, 1502, 3004, 6009 },
etc
As noted below I'd prefer this reformatted so you don't have
to copy it into that local static array. That way we can greatly
simplify avoiding some race conditions.
> + {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.
> + */
> +
> +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 +
> + 100;
If you are going to comment on the maths - match that layout in the
code rather than having so many line breaks.
> +}
> +
> +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;
> + break;
return 0;
similar reasoning to comment below.
> +
> + case MLX90393_CMD_RT:
> + /* Reset acknowledge */
> + if (!(status & MLX90393_STATUS_RT))
> + return -EIO;
> + break;
> +
> + default:
> + break;
> + }
> +
> + 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);
Wrap much less!
> + if (ret)
> + return ret;
> +
> + return mlx90393_check_status(tx[0], status);
> +}
> +
> +static int mlx90393_read_measurement(struct mlx90393_data *data,
> + enum mlx90393_channels chan, int *val)
> +{
> + u8 cmd;
> + u8 rx[9];
> + int ret;
> + int tconv_us = mlx90393_get_tconv_us(data);
> +
> + /* Start measurement */
> + cmd = MLX90393_CMD_SM | MLX90393_MEASURE_ALL;
> +
> + ret = mlx90393_write_cmd(data, cmd);
> + if (ret)
> + return ret;
> +
> + /* Wait conversion */
> + usleep_range(tconv_us, tconv_us + 1000);
fsleep() for all 'fuzzy' ranges like this.
> +
> + /* Read measurement */
> + cmd = MLX90393_CMD_RM | MLX90393_MEASURE_ALL;
Even here I'd put that value inline in the call rather than
having a local variable for it.
> +
> + ret = mlx90393_read_cmd(data, cmd, 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]);
> + break;
> +
> + case MLX90393_CHAN_X:
> + *val = sign_extend32(get_unaligned_be16(&rx[3]), 15);
> + break;
> +
> + case MLX90393_CHAN_Y:
> + *val = sign_extend32(get_unaligned_be16(&rx[5]), 15);
> + break;
> +
> + case MLX90393_CHAN_Z:
> + *val = sign_extend32(get_unaligned_be16(&rx[7]), 15);
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
Maybe just return early instead of break. Saves the reader scrolling
down to see what else is to be done.
> +}
> +
> +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;
Be consistent - here you use an if / else, below to ternary operators.
I don't much mind which (slightly preference for this) but mixing
this sort of thing up just makes for harder code to review.
> + } 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)
> +{
> + int i;
> + u8 res;
> + enum mlx90393_axis_type axis;
> +
> + axis = z_axis ? MLX90393_AXIS_TYPE_Z :
> + MLX90393_AXIS_TYPE_XY;
Go long if it helps readability and here I think it does + it's under
80 chars anyway!
axis = z_axis ? MLX90393_AXIS_TYPE_Z : MLX90393_AXIS_TYPE_XY;
> +
> + res = z_axis ? data->res_z : data->res_xy;
> +
> + if (val != 0)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(mlx90393_scale_table[0]); i++) {
> + if (mlx90393_scale_table[axis][i][res]
> + == val2) {
Here readability is hurt and it fits on one line under 80 chars anyway!
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);
I'd wrap these less
ret = mlx90393_update_bits(data, MLX90393_REG_CONF1,
> + MLX90393_CONF1_GAIN_SEL, gain);
For example. Same for other similar cases.
> + if (ret)
> + return ret;
> +
> + data->gain_sel = gain;
> +
> + return 0;
> +}
> +static int mlx90393_find_osr(int val, int *osr)
> +{
> + int i;
> +
> + for (i = 0; i < MLX90393_OSR_MAX; i++) {
For modern kernel code it's acceptable to do
for (unsigned int i = 0; i < MLX....)
and avoid needing to define the local variable separately above.
> + if (mlx90393_osr_avail[i] == val) {
> + *osr = i;
> + return 0;
> + }
> + }
> +
> + return -EINVAL;
> +}
> +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:
> + mutex_lock(&data->lock);
Less useful but I'd use a guard() here as well just for consistency.
> + ret = mlx90393_set_scale(data, chan, val, val2);
> + mutex_unlock(&data->lock);
> + return ret;
> +
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
Similar use of guard() + {} to described below. Then you can return
directly for each leg.
> + mutex_lock(&data->lock);
> + if (chan->type == IIO_TEMP)
> + ret = mlx90393_set_temp_osr2(data, val);
> + else if (chan->type == IIO_MAGN)
> + ret = mlx90393_set_osr(data, val);
> + else
> + ret = -EINVAL;
> + mutex_unlock(&data->lock);
> + return ret;
> + 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:
This is fine but maybe slightly nicer as:
case IIO_CHAN_INFO_RAW: {
guard(mutex)(&data->lock);
ret = mlx90393_read_measurement(data, chan->addrss, val)
if (ret)
return ret;
return IIO_VAL_INT;
}
> + mutex_lock(&data->lock);
> + ret = mlx90393_read_measurement(data, chan->address, val);
> + mutex_unlock(&data->lock);
> + 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) {
> + /* Datasheet: temperature offset */
> + *val = -45114;
> + return IIO_VAL_INT;
> + }
> + return -EINVAL;
I'd flip the logic .
if (chan->type != IIO_TEMP)
return -EINVAL;
> +
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + if (chan->type == IIO_TEMP)
Maybe a switch statement would be nicer and consistent at leasts
with scale above.
> + return mlx90393_get_temp_osr2(data, val);
> + if (chan->type == IIO_MAGN)
> + return mlx90393_get_osr(data, val);
> + 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;
> + int i;
> + u8 res;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + axis = chan->channel2 == IIO_MOD_Z;
> + res = axis ? data->res_z : data->res_xy;
> +
> + for (i = 0; i < MLX90393_GAIN_MAX; i++) {
> + scale_avail[i][0] = 0;
> + scale_avail[i][1] =
> + mlx90393_scale_table[axis][i][res];
If you support changing rest this can tear mid table write and you
get half of one table and half of the other. Use a lock.
However, can you just make that mlx90393_scale_table have a datalayout
that lets you pick between parts of that rather than having to copy it
in here?
> + }
> +
> + *vals = &scale_avail[0][0];
> + *type = IIO_VAL_INT_PLUS_NANO;
> + *length = MLX90393_GAIN_MAX * MLX90393_AXIS_MAX;
> + break;
> + 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;
> + }
> +
> + break;
> + default:
> + return -EINVAL;
> + }
> + return IIO_AVAIL_LIST;
> +}
> +
> +static int mlx90393_init(struct mlx90393_data *data)
> +{
> + int ret;
> + u8 cmd;
> + u16 reg;
> +
> + /* Exit mode */
> + cmd = MLX90393_CMD_EX;
Seems little benefit in the local variable use over
ret = mlx90393_write_cmd(data, MLX90393_CMD_EX);
> + ret = mlx90393_write_cmd(data, cmd);
> + if (ret)
> + return ret;
> +
> + usleep_range(1000, 1500);
fsleep() + a comment on why this particular sleep time.
> +
> + /* Reset device */
> + cmd = MLX90393_CMD_RT;
> + ret = mlx90393_write_cmd(data, cmd);
Similar to above.
> + if (ret)
> + return ret;
> +
> + /* Wait for device to reset */
> + usleep_range(5000, 6000);
and similar to above.
> +
> + ret = mlx90393_read_reg(data, MLX90393_REG_CONF1, ®);
> + 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, ®);
> + 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);
> + mutex_init(&data->lock);
ret = devm_mutex_init(&data->lock);
if (ret)
return ret;
Small advantage for lock debugging but now it's near free with that helper
it's nicer to do the version that actually marks the lock as disabled on exit.
> +
> + 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) {
> + dev_err_probe(dev, ret, "failed to initialize device\n");
> + return ret;
return dev_err_probe();
> + }
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
> diff --git a/drivers/iio/magnetometer/mlx90393_i2c.c b/drivers/iio/magnetometer/mlx90393_i2c.c
> new file mode 100644
> index 000000000000..09d533a96907
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_i2c.c
> @@ -0,0 +1,71 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/module.h>
> +#include <linux/i2c.h>
mod_devicetable.h for the id table structure definitions.
> +
> +#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];
> +
> + msgs[0].addr = client->addr;
> + msgs[0].flags = 0;
> + msgs[0].len = tx_len;
> + msgs[0].buf = (u8 *)tx;
> +
> + msgs[1].addr = client->addr;
> + msgs[1].flags = I2C_M_RD;
> + msgs[1].len = rx_len;
> + msgs[1].buf = rx;
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, 2);
sizeof(msgs)
> + if (ret != 2)
sizeof(msgs)
> + return ret < 0 ? ret : -EIO;
> +
> + return 0;
> +}
> +static int mlx90393_i2c_probe(struct i2c_client *client)
> +{
> + return mlx90393_core_probe(&client->dev,
> + &mlx90393_i2c_ops,
> + client);
return mlx90393_core_probe(&client->dev, &mlx90393_i2c_ops, client);
Definitely wrap less than you have and I'd probably just go a bit long on
this one and have it all on one line.
> +}
> +
> +static const struct i2c_device_id mlx90393_id[] = {
> + { "mlx90393", 0 },
{ "mlx90393" }
The , 0 doesn't add anything as if we do ever need to add
data it will be a pointer and not 0 anyway.
> + { }
> +};
> +MODULE_DEVICE_TABLE(i2c, mlx90393_id);
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add Melexis MLX90393
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
2026-05-11 13:56 ` Jonathan Cameron
@ 2026-05-11 21:42 ` sashiko-bot
1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-05-11 21:42 UTC (permalink / raw)
To: Nikhil Gautam; +Cc: robh, devicetree, conor+dt, krzk+dt
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The devicetree binding documents only the I2C interface for a device that supports both I2C and SPI, artificially restricting valid hardware descriptions.
- [Medium] Essential physical hardware connections, such as power supplies and interrupts, are omitted from the schema and actively forbidden by `additionalProperties: false`.
--
commit 8e952d4236837623faf2f1c5e7d285f977fcfe5a
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. It currently documents the I2C
interface for the device.
> 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 0000000000000..b99629ff25855
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/melexis,mlx90393.yaml
[ ... ]
> +properties:
> + compatible:
> + enum:
> + - melexis,mlx90393
> +
> + reg:
> + maxItems: 1
Are essential physical hardware connections, such as power supplies
(vdd-supply, vddio-supply) and optional interrupt pins missing here?
Because the schema enforces additionalProperties: false, any downstream board
devicetree that accurately models these physical connections to regulators or
interrupt controllers will fail schema validation. Could these properties be
explicitly listed to permit complete hardware descriptions, even if the
current driver does not yet use them?
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
Does this artificially restrict valid hardware descriptions by omitting the
SPI interface?
The commit message and subsequent driver commit (iio: magnetometer: add
support for Melexis MLX90393) explicitly state that the MLX90393 hardware
supports both I2C and SPI interfaces. However, the schema enforces
additionalProperties: false without referencing standard SPI properties like
spi-max-frequency.
Consequently, any valid board devicetree that connects the device via an
SPI bus will fail dtbs_check validation. Should the binding describe the
hardware component's full capabilities rather than just the limits of the
initial driver implementation?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260510191010.155380-1-nikhilgtr@gmail.com?part=1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 2/2] iio: magnetometer: add support for Melexis MLX90393
2026-05-10 19:10 ` [RFC PATCH 2/2] iio: magnetometer: add support for " Nikhil Gautam
2026-05-11 14:23 ` Jonathan Cameron
@ 2026-05-11 22:13 ` sashiko-bot
1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-05-11 22:13 UTC (permalink / raw)
To: Nikhil Gautam; +Cc: conor+dt, robh, krzk+dt, devicetree
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Data race on static `scale_avail` array in `mlx90393_read_avail`
- [High] `mlx90393_check_status` unconditionally rejects valid full-sensor measurements
- [High] Stack-allocated buffers are passed to I2C transfer, violating DMA safety rules
- [High] Unprotected state reads in `mlx90393_read_raw` lead to data races
- [Medium] IIO ABI violation: Magnetometer and temperature scales use incorrect units
- [Medium] `mlx90393_write_raw` improperly allows modifying the magnetometer gain via the temperature scale attribute
- [Medium] Magnetometer scale is exported as separate attributes despite utilizing a global hardware register
- [Low] Temperature channel exposes internal hardware register values for oversampling ratios
- [Low] Incorrect use of `usleep_range` for delays exceeding 200ms
--
commit b098ee066ea285eea196aa30f9837e81df63b980
Author: Nikhil Gautam <nikhilgtr@gmail.com>
iio: magnetometer: add support for Melexis MLX90393
This commit adds 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
> index 0000000000000..c79f2b8c20d8e
> --- /dev/null
> +++ b/drivers/iio/magnetometer/mlx90393_core.c
[ ... ]
> +static const int mlx90393_osr2_avail[MLX90393_OSR2_MAX] = {
> + 0, 1, 2, 3,
> +};
Is it intentional to expose internal hardware register index limits directly
to userspace instead of the actual physical oversampling ratios?
[ ... ]
> +#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), \
Does using info_mask_separate for IIO_CHAN_INFO_SCALE correctly represent
the hardware capabilities? Writing to the scale of one axis will alter the
global GAIN_SEL hardware register in mlx90393_set_scale(), altering the
scale for all axes.
[ ... ]
> +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;
If MLX90393_MEASURE_ALL requests all 4 channels (T, X, Y, Z), resulting in
4 words, does the 2-bit D[1:0] field truncate this to 00? This check might
unconditionally reject valid full-sensor measurements and return -EIO.
[ ... ]
> +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));
Are these stack-allocated buffers DMA-safe?
When passed to ops->xfer which calls i2c_transfer(), this can lead to memory
corruption or panics if the host controller maps buffers for DMA (especially
with CONFIG_VMAP_STACK=y).
[ ... ]
> +static int mlx90393_read_measurement(struct mlx90393_data *data,
> + enum mlx90393_channels chan, int *val)
> +{
> + u8 cmd;
> + u8 rx[9];
Can this stack allocated buffer also cause DMA safety issues?
> + int ret;
> + int tconv_us = mlx90393_get_tconv_us(data);
> +
> + /* Start measurement */
> + cmd = MLX90393_CMD_SM | MLX90393_MEASURE_ALL;
> +
> + ret = mlx90393_write_cmd(data, cmd);
> + if (ret)
> + return ret;
> +
> + /* Wait conversion */
> + usleep_range(tconv_us, tconv_us + 1000);
Depending on the configured oversampling ratio and digital filters, tconv_us
can exceed 200,000 microseconds. Is usleep_range() appropriate for delays
exceeding 20ms, or should msleep() be used here instead to avoid
unnecessarily taxing the hrtimer subsystem?
[ ... ]
> +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;
Does this scale comply with the IIO ABI requirements? Returning
IIO_VAL_INT_PLUS_NANO with these values produces Tesla rather than the Gauss
units required for magnetometers.
[ ... ]
> +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:
> + mutex_lock(&data->lock);
> + ret = mlx90393_set_scale(data, chan, val, val2);
> + mutex_unlock(&data->lock);
> + return ret;
Does this safely handle writes to the temperature scale?
The temperature channel exposes IIO_CHAN_INFO_SCALE, but mlx90393_set_scale()
modifies the GAIN_SEL hardware register, which might inadvertently corrupt the
magnetometer's sensitivity when a user writes to in_temp_scale.
[ ... ]
> +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:
> + mutex_lock(&data->lock);
> + ret = mlx90393_read_measurement(data, chan->address, val);
> + mutex_unlock(&data->lock);
> + 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);
Can this read race with concurrent sysfs writes?
mlx90393_get_scale() reads shared state like data->gain_sel without acquiring
data->lock, while mlx90393_write_raw() modifies these fields while holding the
lock.
> +
> + case IIO_TEMP:
> + /* Datasheet: 22124 millidegC/LSB */
> + *val = 0;
> + *val2 = 22124;
> + return IIO_VAL_INT_PLUS_MICRO;
Does returning IIO_VAL_INT_PLUS_MICRO with 22124 yield Celsius rather than
the milli-Celsius required for temperature channels by the IIO ABI?
> +
> + default:
> + return -EINVAL;
> + }
> +
> + case IIO_CHAN_INFO_OFFSET:
> + if (chan->type == IIO_TEMP) {
> + /* Datasheet: temperature offset */
> + *val = -45114;
> + return IIO_VAL_INT;
> + }
> + return -EINVAL;
> +
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + if (chan->type == IIO_TEMP)
> + return mlx90393_get_temp_osr2(data, val);
> + if (chan->type == IIO_MAGN)
> + return mlx90393_get_osr(data, val);
Can these reads of data->osr and data->osr2 also race with concurrent sysfs
writes, as data->lock is not acquired here?
[ ... ]
> +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;
> + int i;
> + u8 res;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + axis = chan->channel2 == IIO_MOD_Z;
> + res = axis ? data->res_z : data->res_xy;
> +
> + for (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];
Is it safe to use a global static array here?
Since scale_avail is dynamically populated based on the requested channel
axis, concurrent sysfs reads across different axes or device instances could
overwrite the array mid-operation, causing a data race and corrupted scale
values to be returned.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260510191010.155380-1-nikhilgtr@gmail.com?part=2
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-11 22:13 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-10 19:10 [RFC PATCH 0/2] iio: magnetometer: add support for Melexis MLX90393 Nikhil Gautam
2026-05-10 19:10 ` [RFC PATCH 1/2] dt-bindings: iio: magnetometer: add " Nikhil Gautam
2026-05-11 13:56 ` Jonathan Cameron
2026-05-11 21:42 ` sashiko-bot
2026-05-10 19:10 ` [RFC PATCH 2/2] iio: magnetometer: add support for " Nikhil Gautam
2026-05-11 14:23 ` Jonathan Cameron
2026-05-11 22:13 ` sashiko-bot
2026-05-11 13:51 ` [RFC PATCH 0/2] " Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox