[PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[Vladislav Kulikov]

Add an IIO driver for the MEMSIC MMC5983MA 3-axis magnetometer over
I2C. The driver provides raw magnetic field readings with
per-measurement SET/RESET offset cancellation, giving 18-bit output
with a full-scale range of +/-8 Gauss.

Tested on a Raspberry Pi 2B with the sensor on I2C-1 at 0x30.

The initial driver implements the validated I2C single-measurement path.
Other chip features are left for future work:

- SPI transport: the binding describes SPI wiring, but driver support is
  left for follow-up validation of the SPI command and SET/RESET
  sequencing.
- Temperature channel: left until the temperature output behavior is
  better validated.
- Continuous measurement mode and Auto SET/RESET: left until the
  interaction between CMM, TM_M, Meas_M_Done, and SET/RESET sequencing
  is better understood.
- Saturation/self-test bits and BW/decimation tuning: not exposed until
  their behavior can be described reliably through stable IIO ABI.

The driver uses a conservative 500 us post-SET/RESET delay before
starting the following measurement. The datasheet describes a 500 ns
SET/RESET coil pulse, but testing showed that a longer software delay is
needed before taking the next measurement.

Changes since v1:
- DT binding:
  - added SPI bus support, interrupts, and vddio-supply
  - made vdd-supply required
  - switched to unevaluatedProperties with spi-peripheral-props ref
- Driver:
  - replaced scoped_guard() with guard(mutex) in a case block
  - added datasheet page references for timing values
  - changed product ID mismatch from probe failure to dev_info()
  - hardcoded the IIO device name
  - added trailing commas
  - added local struct device and regmap pointers in mmc5983_init()
- MAINTAINERS:
  - split binding and driver F: entries across the relevant patches

Thank you Jonathan for the review and quick response.

Vladislav Kulikov (2):
  dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA
  iio: magnetometer: add driver for MEMSIC MMC5983MA

 .../iio/magnetometer/memsic,mmc5983.yaml      |  65 ++++
 MAINTAINERS                                   |   7 +
 drivers/iio/magnetometer/Kconfig              |  11 +
 drivers/iio/magnetometer/Makefile             |   1 +
 drivers/iio/magnetometer/mmc5983.c            | 351 ++++++++++++++++++
 5 files changed, 435 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
 create mode 100644 drivers/iio/magnetometer/mmc5983.c


base-commit: 7fd2df204f342fc17d1a0bfcd474b24232fb0f32
-- 
2.43.0

[PATCH v2 1/2] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA

[Vladislav Kulikov]

Add a Devicetree binding for the MEMSIC MMC5983MA 3-axis
magnetometer.

MMC5983MA is not register-compatible with the existing MEMSIC
magnetometer drivers. It has a different register map, 18-bit output
data format, and I2C/SPI transport support.

Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com>
---
 .../iio/magnetometer/memsic,mmc5983.yaml      | 65 +++++++++++++++++++
 MAINTAINERS                                   |  6 ++
 2 files changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
new file mode 100644
index 000000000000..e144b4d9b0ca
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
@@ -0,0 +1,65 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/memsic,mmc5983.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MEMSIC MMC5983MA 3-axis magnetic sensor
+
+maintainers:
+  - Vladislav Kulikov <vlad.kulikov.c@gmail.com>
+
+properties:
+  compatible:
+    const: memsic,mmc5983
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply:
+    description: Regulator that provides power to the sensor
+
+  vddio-supply:
+    description: Regulator that provides power to the digital interface and INT pin
+
+required:
+  - compatible
+  - reg
+  - vdd-supply
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    // Example for I2C
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        magnetometer@30 {
+            compatible = "memsic,mmc5983";
+            reg = <0x30>;
+            vdd-supply = <&vdd_3v3_reg>;
+            vddio-supply = <&vdd_3v3_reg>;
+        };
+    };
+  - |
+    // Example for SPI
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        magnetometer@0 {
+            compatible = "memsic,mmc5983";
+            reg = <0>;
+            spi-max-frequency = <10000000>;
+            vdd-supply = <&vdd_3v3_reg>;
+            vddio-supply = <&vdd_3v3_reg>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 882214b0e7db..952fbf3020a4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17170,6 +17170,12 @@ F:	drivers/mtd/
 F:	include/linux/mtd/
 F:	include/uapi/mtd/
 
+MEMSIC MMC5983 MAGNETOMETER DRIVER
+M:	Vladislav Kulikov <vlad.kulikov.c@gmail.com>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
+
 MEN A21 WATCHDOG DRIVER
 M:	Johannes Thumshirn <morbidrsa@gmail.com>
 L:	linux-watchdog@vger.kernel.org
-- 
2.43.0

Re: [PATCH v2 1/2] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA

[Conor Dooley]

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

On Thu, May 07, 2026 at 08:50:31PM +0000, Vladislav Kulikov wrote:
> Add a Devicetree binding for the MEMSIC MMC5983MA 3-axis
> magnetometer.
> 
> MMC5983MA is not register-compatible with the existing MEMSIC
> magnetometer drivers. It has a different register map, 18-bit output
> data format, and I2C/SPI transport support.
> 
> Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> ---

Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable

>  .../iio/magnetometer/memsic,mmc5983.yaml      | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> new file mode 100644
> index 000000000000..e144b4d9b0ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/magnetometer/memsic,mmc5983.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MEMSIC MMC5983MA 3-axis magnetic sensor
> +
> +maintainers:
> +  - Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: memsic,mmc5983
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +  vddio-supply:
> +    description: Regulator that provides power to the digital interface and INT pin
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    // Example for I2C
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        magnetometer@30 {
> +            compatible = "memsic,mmc5983";
> +            reg = <0x30>;
> +            vdd-supply = <&vdd_3v3_reg>;
> +            vddio-supply = <&vdd_3v3_reg>;
> +        };
> +    };
> +  - |
> +    // Example for SPI
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        magnetometer@0 {
> +            compatible = "memsic,mmc5983";
> +            reg = <0>;
> +            spi-max-frequency = <10000000>;
> +            vdd-supply = <&vdd_3v3_reg>;
> +            vddio-supply = <&vdd_3v3_reg>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 882214b0e7db..952fbf3020a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17170,6 +17170,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
>  
> +MEMSIC MMC5983 MAGNETOMETER DRIVER
> +M:	Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> +
>  MEN A21 WATCHDOG DRIVER
>  M:	Johannes Thumshirn <morbidrsa@gmail.com>
>  L:	linux-watchdog@vger.kernel.org
> -- 
> 2.43.0
> 

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

Re: [PATCH v2 1/2] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA

[David Lechner]

On 5/7/26 3:50 PM, Vladislav Kulikov wrote:
> Add a Devicetree binding for the MEMSIC MMC5983MA 3-axis
> magnetometer.
> 
> MMC5983MA is not register-compatible with the existing MEMSIC
> magnetometer drivers. It has a different register map, 18-bit output
> data format, and I2C/SPI transport support.
> 
> Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> ---
>  .../iio/magnetometer/memsic,mmc5983.yaml      | 65 +++++++++++++++++++
>  MAINTAINERS                                   |  6 ++
>  2 files changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> new file mode 100644
> index 000000000000..e144b4d9b0ca
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> @@ -0,0 +1,65 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/magnetometer/memsic,mmc5983.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MEMSIC MMC5983MA 3-axis magnetic sensor
> +
> +maintainers:
> +  - Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> +
> +properties:
> +  compatible:
> +    const: memsic,mmc5983
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vdd-supply:
> +    description: Regulator that provides power to the sensor
> +
> +  vddio-supply:
> +    description: Regulator that provides power to the digital interface and INT pin
> +
> +required:
> +  - compatible
> +  - reg
> +  - vdd-supply
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    // Example for I2C

If we do another revision, we could drop these comments.
They are redundant since the yaml already says both
"examples" and "i2c".

> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        magnetometer@30 {
> +            compatible = "memsic,mmc5983";
> +            reg = <0x30>;
> +            vdd-supply = <&vdd_3v3_reg>;
> +            vddio-supply = <&vdd_3v3_reg>;
> +        };
> +    };
> +  - |
> +    // Example for SPI

same

> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        magnetometer@0 {
> +            compatible = "memsic,mmc5983";
> +            reg = <0>;
> +            spi-max-frequency = <10000000>;
> +            vdd-supply = <&vdd_3v3_reg>;
> +            vddio-supply = <&vdd_3v3_reg>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 882214b0e7db..952fbf3020a4 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -17170,6 +17170,12 @@ F:	drivers/mtd/
>  F:	include/linux/mtd/
>  F:	include/uapi/mtd/
>  
> +MEMSIC MMC5983 MAGNETOMETER DRIVER
> +M:	Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
> +
>  MEN A21 WATCHDOG DRIVER
>  M:	Johannes Thumshirn <morbidrsa@gmail.com>
>  L:	linux-watchdog@vger.kernel.org

[PATCH v2 2/2] iio: magnetometer: add driver for MEMSIC MMC5983MA

[Vladislav Kulikov]

Add support for the MEMSIC MMC5983MA 3-axis magnetometer. The driver
provides raw magnetic field readings via IIO sysfs with SET/RESET
offset cancellation for each measurement.

Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com>
---
 MAINTAINERS                        |   1 +
 drivers/iio/magnetometer/Kconfig   |  11 +
 drivers/iio/magnetometer/Makefile  |   1 +
 drivers/iio/magnetometer/mmc5983.c | 351 +++++++++++++++++++++++++++++
 4 files changed, 364 insertions(+)
 create mode 100644 drivers/iio/magnetometer/mmc5983.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 952fbf3020a4..b1d9d7b586a1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17175,6 +17175,7 @@ M:	Vladislav Kulikov <vlad.kulikov.c@gmail.com>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml
+F:	drivers/iio/magnetometer/mmc5983.c
 
 MEN A21 WATCHDOG DRIVER
 M:	Johannes Thumshirn <morbidrsa@gmail.com>
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index fb313e591e85..ea2697fb5ab6 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -151,6 +151,17 @@ config MMC5633
 	  To compile this driver as a module, choose M here: the module
 	  will be called mmc5633
 
+config MMC5983
+	tristate "MEMSIC MMC5983 3-axis magnetic sensor"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for the MEMSIC MMC5983 3-axis
+	  magnetic sensor.
+
+	  To compile this driver as a module, choose M here: the module
+	  will be called mmc5983
+
 config IIO_ST_MAGN_3AXIS
 	tristate "STMicroelectronics magnetometers 3-Axis Driver"
 	depends on (I2C || SPI_MASTER) && SYSFS
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 5bd227f8c120..7fd9b3fd914e 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_MAG3110)	+= mag3110.o
 obj-$(CONFIG_HID_SENSOR_MAGNETOMETER_3D) += hid-sensor-magn-3d.o
 obj-$(CONFIG_MMC35240)	+= mmc35240.o
 obj-$(CONFIG_MMC5633)	+= mmc5633.o
+obj-$(CONFIG_MMC5983)	+= mmc5983.o
 
 obj-$(CONFIG_IIO_ST_MAGN_3AXIS) += st_magn.o
 st_magn-y := st_magn_core.o
diff --git a/drivers/iio/magnetometer/mmc5983.c b/drivers/iio/magnetometer/mmc5983.c
new file mode 100644
index 000000000000..3b06164fad15
--- /dev/null
+++ b/drivers/iio/magnetometer/mmc5983.c
@@ -0,0 +1,351 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MMC5983 - MEMSIC 3-axis Magnetic Sensor
+ *
+ * Copyright (c) 2026, Vlad Kulikov <vlad.kulikov.c@gmail.com>
+ *
+ * IIO driver for MMC5983
+ */
+
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#define MMC5983_REG_XOUT0	0x00
+#define MMC5983_REG_XOUT1	0x01
+#define MMC5983_REG_YOUT0	0x02
+#define MMC5983_REG_YOUT1	0x03
+#define MMC5983_REG_ZOUT0	0x04
+#define MMC5983_REG_ZOUT1	0x05
+#define MMC5983_REG_XYZOUT2	0x06
+
+#define MMC5983_REG_STATUS	0x08
+
+#define MMC5983_REG_CTRL0	0x09
+#define MMC5983_REG_CTRL1	0x0A
+#define MMC5983_REG_CTRL2	0x0B
+#define MMC5983_REG_CTRL3	0x0C
+
+#define MMC5983_REG_ID		0x2F
+
+#define MMC5983_PRODUCT_ID	0x30
+
+#define MMC5983_STATUS_MEAS_M_DONE_BIT	BIT(0)
+#define MMC5983_STATUS_OTP_RD_DONE_BIT	BIT(4)
+
+#define MMC5983_CTRL0_TM_M_BIT		BIT(0)
+#define MMC5983_CTRL0_SET_BIT		BIT(3)
+#define MMC5983_CTRL0_RESET_BIT		BIT(4)
+#define MMC5983_CTRL0_OTP_RD_BIT	BIT(6)
+
+#define MMC5983_CTRL1_SW_RST_BIT	BIT(7)
+
+enum mmc5983_axis {
+	MMC5983_AXIS_X,
+	MMC5983_AXIS_Y,
+	MMC5983_AXIS_Z,
+};
+
+struct mmc5983_data {
+	struct regmap *regmap;
+	/* Protects chip access during SET/RESET measurement sequence */
+	struct mutex mutex;
+};
+
+#define MMC5983_CHANNEL(_axis) { \
+	.type = IIO_MAGN, \
+	.modified = 1, \
+	.channel2 = IIO_MOD_##_axis, \
+	.address = MMC5983_AXIS_##_axis, \
+	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+}
+
+static const struct iio_chan_spec mmc5983_channels[] = {
+	MMC5983_CHANNEL(X),
+	MMC5983_CHANNEL(Y),
+	MMC5983_CHANNEL(Z),
+};
+
+static int mmc5983_take_measurement(struct mmc5983_data *data, int m[3])
+{
+	unsigned int status;
+	u8 buf[7];
+	int ret;
+
+	ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
+			   MMC5983_CTRL0_TM_M_BIT);
+	if (ret)
+		return ret;
+
+	/*
+	 * Datasheet page 15: measurement time is 8 ms at BW=00 (default,
+	 * slowest setting). Use a 50 ms timeout for margin.
+	 */
+	ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS,
+				       status,
+				       status & MMC5983_STATUS_MEAS_M_DONE_BIT,
+				       10000, 50000);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(data->regmap, MMC5983_REG_XOUT0, buf,
+			       sizeof(buf));
+	if (ret)
+		return ret;
+
+	m[0] = (buf[0] << 10) | (buf[1] << 2) | ((buf[6] >> 6) & 0x3);
+	m[1] = (buf[2] << 10) | (buf[3] << 2) | ((buf[6] >> 4) & 0x3);
+	m[2] = (buf[4] << 10) | (buf[5] << 2) | ((buf[6] >> 2) & 0x3);
+
+	return 0;
+}
+
+static int mmc5983_read_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan, int *val,
+			     int *val2, long mask)
+{
+	struct mmc5983_data *data = iio_priv(indio_dev);
+	int m1[3], m2[3];
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW: {
+		guard(mutex)(&data->mutex);
+
+		ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
+				   MMC5983_CTRL0_SET_BIT);
+		if (ret)
+			return ret;
+
+		/*
+		 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
+		 * Vendor sample code waits 500 us before the next operation.
+		 */
+		fsleep(500);
+
+		ret = mmc5983_take_measurement(data, m1);
+		if (ret)
+			return ret;
+
+		ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
+				   MMC5983_CTRL0_RESET_BIT);
+		if (ret)
+			return ret;
+
+		/*
+		 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
+		 * Vendor sample code waits 500 us before the next operation.
+		 */
+		fsleep(500);
+
+		ret = mmc5983_take_measurement(data, m2);
+		if (ret)
+			return ret;
+
+		*val = (m1[chan->address] - m2[chan->address]) / 2;
+		return IIO_VAL_INT;
+	}
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = 61035;
+		return IIO_VAL_INT_PLUS_NANO;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info mmc5983_info = {
+	.read_raw = mmc5983_read_raw,
+};
+
+static bool mmc5983_is_writeable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MMC5983_REG_CTRL0:
+	case MMC5983_REG_CTRL1:
+	case MMC5983_REG_CTRL2:
+	case MMC5983_REG_CTRL3:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool mmc5983_is_readable_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MMC5983_REG_XOUT0:
+	case MMC5983_REG_XOUT1:
+	case MMC5983_REG_YOUT0:
+	case MMC5983_REG_YOUT1:
+	case MMC5983_REG_ZOUT0:
+	case MMC5983_REG_ZOUT1:
+	case MMC5983_REG_XYZOUT2:
+	case MMC5983_REG_STATUS:
+	case MMC5983_REG_CTRL0:
+	case MMC5983_REG_CTRL1:
+	case MMC5983_REG_CTRL2:
+	case MMC5983_REG_CTRL3:
+	case MMC5983_REG_ID:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool mmc5983_is_volatile_reg(struct device *dev, unsigned int reg)
+{
+	switch (reg) {
+	case MMC5983_REG_XOUT0:
+	case MMC5983_REG_XOUT1:
+	case MMC5983_REG_YOUT0:
+	case MMC5983_REG_YOUT1:
+	case MMC5983_REG_ZOUT0:
+	case MMC5983_REG_ZOUT1:
+	case MMC5983_REG_XYZOUT2:
+	case MMC5983_REG_STATUS:
+	case MMC5983_REG_CTRL0:
+	case MMC5983_REG_CTRL1:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static const struct regmap_config mmc5983_regmap_config = {
+	.name = "mmc5983_regmap",
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = MMC5983_REG_ID,
+	.writeable_reg = mmc5983_is_writeable_reg,
+	.readable_reg = mmc5983_is_readable_reg,
+	.volatile_reg = mmc5983_is_volatile_reg,
+};
+
+static int mmc5983_init(struct mmc5983_data *data)
+{
+	struct regmap *regmap = data->regmap;
+	struct device *dev = regmap_get_device(regmap);
+	unsigned int reg_id, status;
+	int ret;
+
+	ret = regmap_read(regmap, MMC5983_REG_ID, &reg_id);
+	if (ret)
+		return dev_err_probe(dev, ret, "Error reading product id\n");
+
+	if (reg_id != MMC5983_PRODUCT_ID)
+		dev_info(dev, "unexpected product id 0x%02x\n", reg_id);
+
+	ret = regmap_write(regmap, MMC5983_REG_CTRL1, MMC5983_CTRL1_SW_RST_BIT);
+	if (ret)
+		return ret;
+
+	/* Datasheet page 15: power-on time after SW_RST is 10 ms */
+	fsleep(10000);
+
+	ret = regmap_write(regmap, MMC5983_REG_CTRL0, MMC5983_CTRL0_OTP_RD_BIT);
+	if (ret)
+		return ret;
+
+	/*
+	 * Datasheet page 15: OTP read completes and self-clears. No separate
+	 * OTP refresh timeout is specified, so use the 10 ms power-on time as
+	 * a conservative upper bound.
+	 */
+	ret = regmap_read_poll_timeout(regmap, MMC5983_REG_STATUS, status,
+				       status & MMC5983_STATUS_OTP_RD_DONE_BIT,
+				       1000, 10000);
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, MMC5983_REG_CTRL0, MMC5983_CTRL0_SET_BIT);
+	if (ret)
+		return ret;
+
+	/*
+	 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
+	 * Vendor sample code waits 500 us before the next operation.
+	 */
+	fsleep(500);
+
+	ret = regmap_write(regmap, MMC5983_REG_CTRL0, MMC5983_CTRL0_RESET_BIT);
+	if (ret)
+		return ret;
+
+	/*
+	 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
+	 * Vendor sample code waits 500 us before the next operation.
+	 */
+	fsleep(500);
+
+	return 0;
+}
+
+static int mmc5983_probe(struct i2c_client *i2c)
+{
+	struct device *dev = &i2c->dev;
+	struct mmc5983_data *data;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	data = iio_priv(indio_dev);
+
+	ret = devm_mutex_init(dev, &data->mutex);
+	if (ret)
+		return ret;
+
+	data->regmap = devm_regmap_init_i2c(i2c, &mmc5983_regmap_config);
+	if (IS_ERR(data->regmap))
+		return dev_err_probe(dev, PTR_ERR(data->regmap),
+				     "failed to allocate register map\n");
+
+	indio_dev->info = &mmc5983_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->name = "mmc5983";
+	indio_dev->channels = mmc5983_channels;
+	indio_dev->num_channels = ARRAY_SIZE(mmc5983_channels);
+
+	ret = mmc5983_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "mmc5983 chip init failed\n");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id mmc5983_of_match[] = {
+	{ .compatible = "memsic,mmc5983" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mmc5983_of_match);
+
+static const struct i2c_device_id mmc5983_id[] = {
+	{ "mmc5983" },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, mmc5983_id);
+
+static struct i2c_driver mmc5983_driver = {
+	.driver = {
+		.name = "mmc5983",
+		.of_match_table = mmc5983_of_match,
+	},
+	.probe = mmc5983_probe,
+	.id_table = mmc5983_id,
+};
+module_i2c_driver(mmc5983_driver);
+
+MODULE_AUTHOR("Vladislav Kulikov <vlad.kulikov.c@gmail.com>");
+MODULE_DESCRIPTION("MEMSIC MMC5983 magnetic sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.43.0

Re: [PATCH v2 2/2] iio: magnetometer: add driver for MEMSIC MMC5983MA

[David Lechner]

On 5/7/26 3:50 PM, Vladislav Kulikov wrote:
> Add support for the MEMSIC MMC5983MA 3-axis magnetometer. The driver
> provides raw magnetic field readings via IIO sysfs with SET/RESET
> offset cancellation for each measurement.
> 
> Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com>
> ---

I gave my RB tag already, but if we do another revision, a couple of
suggestions.

> +static int mmc5983_take_measurement(struct mmc5983_data *data, int m[3])
> +{
> +	unsigned int status;
> +	u8 buf[7];
> +	int ret;
> +
> +	ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
> +			   MMC5983_CTRL0_TM_M_BIT);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Datasheet page 15: measurement time is 8 ms at BW=00 (default,
> +	 * slowest setting). Use a 50 ms timeout for margin.
> +	 */
> +	ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS,
> +				       status,
> +				       status & MMC5983_STATUS_MEAS_M_DONE_BIT,
> +				       10000, 50000);

I wouldn't mind seeing 10 * KILO, 50 * KILO here to make it easier to read.

> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_bulk_read(data->regmap, MMC5983_REG_XOUT0, buf,
> +			       sizeof(buf));
> +	if (ret)
> +		return ret;
> +
> +	m[0] = (buf[0] << 10) | (buf[1] << 2) | ((buf[6] >> 6) & 0x3);
> +	m[1] = (buf[2] << 10) | (buf[3] << 2) | ((buf[6] >> 4) & 0x3);
> +	m[2] = (buf[4] << 10) | (buf[5] << 2) | ((buf[6] >> 2) & 0x3);
> +
> +	return 0;
> +}
> +
> +static int mmc5983_read_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	struct mmc5983_data *data = iio_priv(indio_dev);
> +	int m1[3], m2[3];
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW: {
> +		guard(mutex)(&data->mutex);
> +
> +		ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
> +				   MMC5983_CTRL0_SET_BIT);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
> +		 * Vendor sample code waits 500 us before the next operation.
> +		 */
> +		fsleep(500);
> +
> +		ret = mmc5983_take_measurement(data, m1);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_write(data->regmap, MMC5983_REG_CTRL0,
> +				   MMC5983_CTRL0_RESET_BIT);
> +		if (ret)
> +			return ret;
> +
> +		/*
> +		 * Datasheet page 15: SET/RESET coil pulse is 500 ns.
> +		 * Vendor sample code waits 500 us before the next operation.
> +		 */
> +		fsleep(500);

It looks like this SET/RESET sequence is also repeated during init. Maybe
refactor that out into a separate function.

> +
> +		ret = mmc5983_take_measurement(data, m2);
> +		if (ret)
> +			return ret;
> +
> +		*val = (m1[chan->address] - m2[chan->address]) / 2;
> +		return IIO_VAL_INT;
> +	}
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = 61035;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +

Re: [PATCH v2 2/2] iio: magnetometer: add driver for MEMSIC MMC5983MA

[Andy Shevchenko]

On Sat, May 09, 2026 at 05:09:33PM -0500, David Lechner wrote:
> On 5/7/26 3:50 PM, Vladislav Kulikov wrote:
> > Add support for the MEMSIC MMC5983MA 3-axis magnetometer. The driver
> > provides raw magnetic field readings via IIO sysfs with SET/RESET
> > offset cancellation for each measurement.

...

> > +	ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS,
> > +				       status,
> > +				       status & MMC5983_STATUS_MEAS_M_DONE_BIT,
> > +				       10000, 50000);
> 
> I wouldn't mind seeing 10 * KILO, 50 * KILO here to make it easier to read.

No, it should be USEC_PER_MSEC (requires time.h to be included).

> > +	if (ret)
> > +		return ret;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[Andy Shevchenko]

On Thu, May 07, 2026 at 08:50:30PM +0000, Vladislav Kulikov wrote:
> Add an IIO driver for the MEMSIC MMC5983MA 3-axis magnetometer over
> I2C. The driver provides raw magnetic field readings with
> per-measurement SET/RESET offset cancellation, giving 18-bit output
> with a full-scale range of +/-8 Gauss.
> 
> Tested on a Raspberry Pi 2B with the sensor on I2C-1 at 0x30.
> 
> The initial driver implements the validated I2C single-measurement path.
> Other chip features are left for future work:
> 
> - SPI transport: the binding describes SPI wiring, but driver support is
>   left for follow-up validation of the SPI command and SET/RESET
>   sequencing.
> - Temperature channel: left until the temperature output behavior is
>   better validated.
> - Continuous measurement mode and Auto SET/RESET: left until the
>   interaction between CMM, TM_M, Meas_M_Done, and SET/RESET sequencing
>   is better understood.
> - Saturation/self-test bits and BW/decimation tuning: not exposed until
>   their behavior can be described reliably through stable IIO ABI.
> 
> The driver uses a conservative 500 us post-SET/RESET delay before
> starting the following measurement. The datasheet describes a 500 ns
> SET/RESET coil pulse, but testing showed that a longer software delay is
> needed before taking the next measurement.

My comment from v1 still applies. Note, when sending a new version of a driver
like this, give approximately as many days as hundreds of LoC in it.
The bare minimum 24h anyway.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[Vlad]

On Fri, May 8, 2026 at 1:03 PM Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
> My comment from v1 still applies. Note, when sending a new version of a driver
> like this, give approximately as many days as hundreds of LoC in it.
> The bare minimum 24h anyway.

Thanks, understood about the revision cadence. I will wait longer before
sending the next version.

I did include a short explanation in patch 1/2, but I see that it should
also be visible in the cover letter for a new driver series. Unless you
think the existing note in patch 1/2 is sufficient, I will add a
dedicated "why a new driver" section to the cover letter in v3.

Best regards,
Vladi

Re: [PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[Andy Shevchenko]

On Fri, May 08, 2026 at 01:40:17PM +0300, Vlad wrote:
> On Fri, May 8, 2026 at 1:03 PM Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> > My comment from v1 still applies. Note, when sending a new version of a driver
> > like this, give approximately as many days as hundreds of LoC in it.
> > The bare minimum 24h anyway.
> 
> Thanks, understood about the revision cadence. I will wait longer before
> sending the next version.
> 
> I did include a short explanation in patch 1/2, but I see that it should
> also be visible in the cover letter for a new driver series. Unless you
> think the existing note in patch 1/2 is sufficient, I will add a
> dedicated "why a new driver" section to the cover letter in v3.

I'm waiting for this before reviewing. I.o.w. it's crucial to me to even
start the review round. It's up to you what to do on this.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/2] iio: magnetometer: add MEMSIC MMC5983MA driver

[David Lechner]

On 5/7/26 3:50 PM, Vladislav Kulikov wrote:
> Add an IIO driver for the MEMSIC MMC5983MA 3-axis magnetometer over
> I2C. The driver provides raw magnetic field readings with
> per-measurement SET/RESET offset cancellation, giving 18-bit output
> with a full-scale range of +/-8 Gauss.
> 
I wish all new drivers looked this good. :-)

Reviewed-by: David Lechner <dlechner@baylibre.com>