* [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver
@ 2026-05-07 12:47 Vladislav Kulikov
2026-05-07 12:47 ` [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA Vladislav Kulikov
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Vladislav Kulikov @ 2026-05-07 12:47 UTC (permalink / raw)
To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt
Cc: linux-iio, devicetree, linux-kernel, 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 following chip features are intentionally left out of the initial
driver because the public datasheet does not provide enough detail to
expose them confidently through stable IIO ABI, or because they still
need more validation:
- SPI transport: deferred because SET/RESET polarity behavior has been
reported to differ between I2C and SPI, especially around SET/RESET
timing and/or SPI mode.
- Temperature channel: deferred until the temperature output behavior is
better validated.
- Continuous measurement mode and Auto SET/RESET: deferred because the
datasheet does not clearly define the interaction between CMM, TM_M,
Meas_M_Done, and SET/RESET sequencing.
- Saturation/self-test bits: deferred because the applied test field
strength and bit lifetime are not specified.
- BW/decimation filter tuning: only the documented measurement timing is
used; no filter response is exposed because the filter topology and
coefficients are not documented.
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 existing sample code and practical testing
indicate that a longer software delay is needed before taking the next
measurement.
Vladislav Kulikov (3):
dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA
iio: magnetometer: add driver for MEMSIC MMC5983MA
MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver
.../iio/magnetometer/memsic,mmc5983.yaml | 38 ++
MAINTAINERS | 7 +
drivers/iio/magnetometer/Kconfig | 11 +
drivers/iio/magnetometer/Makefile | 1 +
drivers/iio/magnetometer/mmc5983.c | 330 ++++++++++++++++++
5 files changed, 387 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
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA 2026-05-07 12:47 [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Vladislav Kulikov @ 2026-05-07 12:47 ` Vladislav Kulikov 2026-05-07 16:46 ` Jonathan Cameron 2026-05-07 12:47 ` [PATCH 2/3] iio: magnetometer: add driver for " Vladislav Kulikov ` (2 subsequent siblings) 3 siblings, 1 reply; 8+ messages in thread From: Vladislav Kulikov @ 2026-05-07 12:47 UTC (permalink / raw) To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt Cc: linux-iio, devicetree, linux-kernel, Vladislav Kulikov Add device tree binding documentation for the MEMSIC MMC5983MA 3-axis magnetometer connected via I2C. Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com> --- .../iio/magnetometer/memsic,mmc5983.yaml | 38 +++++++++++++++++++ 1 file changed, 38 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..bbe2aa597f75 --- /dev/null +++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml @@ -0,0 +1,38 @@ +# 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 + + vdd-supply: + description: Regulator that provides power to the sensor + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + magnetometer@30 { + compatible = "memsic,mmc5983"; + reg = <0x30>; + vdd-supply = <&vdd_3v3_reg>; + }; + }; -- 2.43.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA 2026-05-07 12:47 ` [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA Vladislav Kulikov @ 2026-05-07 16:46 ` Jonathan Cameron 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2026-05-07 16:46 UTC (permalink / raw) To: Vladislav Kulikov Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio, devicetree, linux-kernel On Thu, 7 May 2026 12:47:22 +0000 Vladislav Kulikov <vlad.kulikov.c@gmail.com> wrote: > Add device tree binding documentation for the MEMSIC MMC5983MA > 3-axis magnetometer connected via I2C. HI Vladislav, Usual question to answer. How is this different from existing memsic magnetometers that are supported? Even if the driver is different is there a reason to have a separate binding? Note that same question will apply to the driver - just provide some brief notes on how it is different enough from existing devices. The others are all in trivial bindings. They maybe should not be! A few other comments inline. Whilst you comment you've left the SPI side out, it would be good to still have a DT binding even if the driver doesn't support it. > > Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com> > --- > .../iio/magnetometer/memsic,mmc5983.yaml | 38 +++++++++++++++++++ > 1 file changed, 38 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..bbe2aa597f75 > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/magnetometer/memsic,mmc5983.yaml > @@ -0,0 +1,38 @@ > +# 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 Binding should try to be as complete as possible, even if the driver doesn't yet use some features. Looks like we have an interrupt to describe. Also vddio is missing based on the datasheet google found for me: https://media.digikey.com/pdf/Data%20Sheets/MEMSIC%20PDFs/MMC5983MA_RevA_4-3-19.pdf > + > + vdd-supply: > + description: Regulator that provides power to the sensor > + > +required: > + - compatible > + - reg I'm guessing it doesn't work well without a power supply - vdd-supply should be here. > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + magnetometer@30 { > + compatible = "memsic,mmc5983"; > + reg = <0x30>; > + vdd-supply = <&vdd_3v3_reg>; > + }; > + }; ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/3] iio: magnetometer: add driver for MEMSIC MMC5983MA 2026-05-07 12:47 [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Vladislav Kulikov 2026-05-07 12:47 ` [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA Vladislav Kulikov @ 2026-05-07 12:47 ` Vladislav Kulikov 2026-05-07 17:00 ` Jonathan Cameron 2026-05-07 12:47 ` [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver Vladislav Kulikov 2026-05-08 9:19 ` [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Andy Shevchenko 3 siblings, 1 reply; 8+ messages in thread From: Vladislav Kulikov @ 2026-05-07 12:47 UTC (permalink / raw) To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt Cc: linux-iio, devicetree, linux-kernel, 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> --- drivers/iio/magnetometer/Kconfig | 11 + drivers/iio/magnetometer/Makefile | 1 + drivers/iio/magnetometer/mmc5983.c | 330 +++++++++++++++++++++++++++++ 3 files changed, 342 insertions(+) create mode 100644 drivers/iio/magnetometer/mmc5983.c 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..7f2f5133a472 --- /dev/null +++ b/drivers/iio/magnetometer/mmc5983.c @@ -0,0 +1,330 @@ +// 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; + + ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS, + status, + status & MMC5983_STATUS_MEAS_M_DONE_BIT, + 5000, 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 = -EBUSY; + + switch (mask) { + case IIO_CHAN_INFO_RAW: + scoped_guard(mutex, &data->mutex) { + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, + MMC5983_CTRL0_SET_BIT); + if (ret) + break; + + fsleep(500); + + ret = mmc5983_take_measurement(data, m1); + if (ret) + break; + + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, + MMC5983_CTRL0_RESET_BIT); + if (ret) + break; + + fsleep(500); + + ret = mmc5983_take_measurement(data, m2); + if (ret) + break; + + *val = (m1[chan->address] - m2[chan->address]) / 2; + ret = IIO_VAL_INT; + } + return ret; + 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) +{ + unsigned int reg_id, status; + int ret; + + ret = regmap_read(data->regmap, MMC5983_REG_ID, ®_id); + if (ret) + return dev_err_probe(regmap_get_device(data->regmap), ret, + "Error reading product id\n"); + + if (reg_id != MMC5983_PRODUCT_ID) + return dev_err_probe(regmap_get_device(data->regmap), -ENODEV, + "wrong product id 0x%02x\n", reg_id); + + ret = regmap_write(data->regmap, MMC5983_REG_CTRL1, + MMC5983_CTRL1_SW_RST_BIT); + if (ret) + return ret; + + fsleep(10000); + + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, + MMC5983_CTRL0_OTP_RD_BIT); + if (ret) + return ret; + + ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS, + status, + status & MMC5983_STATUS_OTP_RD_DONE_BIT, + 1000, 10000); + if (ret) + return ret; + + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, + MMC5983_CTRL0_SET_BIT); + if (ret) + return ret; + + fsleep(500); + + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, + MMC5983_CTRL0_RESET_BIT); + if (ret) + return ret; + + 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 = i2c->name; + 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] iio: magnetometer: add driver for MEMSIC MMC5983MA 2026-05-07 12:47 ` [PATCH 2/3] iio: magnetometer: add driver for " Vladislav Kulikov @ 2026-05-07 17:00 ` Jonathan Cameron 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2026-05-07 17:00 UTC (permalink / raw) To: Vladislav Kulikov Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio, devicetree, linux-kernel On Thu, 7 May 2026 12:47:23 +0000 Vladislav Kulikov <vlad.kulikov.c@gmail.com> 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> Hi A few comments inline but mostly a nice clean driver. Thanks, Jonathan > diff --git a/drivers/iio/magnetometer/mmc5983.c b/drivers/iio/magnetometer/mmc5983.c > new file mode 100644 > index 000000000000..7f2f5133a472 > --- /dev/null > +++ b/drivers/iio/magnetometer/mmc5983.c > +enum mmc5983_axis { > + MMC5983_AXIS_X, > + MMC5983_AXIS_Y, > + MMC5983_AXIS_Z May get more entries so add trailing comma to reduce future churn > +}; > + > +struct mmc5983_data { > + struct regmap *regmap; > + /* Protects chip access during SET/RESET measurement sequence */ > + struct mutex mutex; > +}; > + > +static const struct iio_chan_spec mmc5983_channels[] = { > + MMC5983_CHANNEL(X), > + MMC5983_CHANNEL(Y), > + MMC5983_CHANNEL(Z) We may well get channels after this so add a trailing , > +}; > + > +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 = -EBUSY; > + > + switch (mask) { > + case IIO_CHAN_INFO_RAW: > + scoped_guard(mutex, &data->mutex) { To avoid the intent and need for that return at the end (due to compilers being slow to catch on to for loops that are always run once!) case IIO_CHAN_INFO_RAW: { guard(mutex)(&data->mutex); ret = ... ... reutrn IIO_VAL_INT; } > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, > + MMC5983_CTRL0_SET_BIT); > + if (ret) > + break; > + > + fsleep(500); > + > + ret = mmc5983_take_measurement(data, m1); > + if (ret) > + break; > + > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, > + MMC5983_CTRL0_RESET_BIT); > + if (ret) > + break; > + > + fsleep(500); > + > + ret = mmc5983_take_measurement(data, m2); > + if (ret) > + break; > + > + *val = (m1[chan->address] - m2[chan->address]) / 2; > + ret = IIO_VAL_INT; > + } > + return ret; > + case IIO_CHAN_INFO_SCALE: > + *val = 0; > + *val2 = 61035; > + return IIO_VAL_INT_PLUS_NANO; > + default: > + return -EINVAL; > + } > +} > + > +static int mmc5983_init(struct mmc5983_data *data) > +{ > + unsigned int reg_id, status; > + int ret; > + > + ret = regmap_read(data->regmap, MMC5983_REG_ID, ®_id); > + if (ret) > + return dev_err_probe(regmap_get_device(data->regmap), ret, Have struct device *dev = regmap_get_device(data->regmap); at top of function to avoid repeats of this. Then we should near enough to 80 chars that at least some of these error messages become one liners. Given lots of uses of regmap might be useful to have a local variable for that as well. If a given line is a tiny bit over 80 chars and that helps readability then that is fine in IIO these days. With these local variables I suspect that applies quite a lot in here! > + "Error reading product id\n"); > + > + if (reg_id != MMC5983_PRODUCT_ID) This one is a bit subtle but we never fail to probe on a missmatched product ID. The reason is DT fallback compatibles. If in future memsic release a new device that has a different product ID but which is a strict superset (or identical) in interface etc, then it will have a fallback compatible listed in the dt-binding. Those allow old kernels to still support the device based on that compatibility. That doesn't work if the product ID failing to match is a hard failure. So trust the firmware and just print an dev_info() to indicate you have a part you don't recognise rather than failing. > + return dev_err_probe(regmap_get_device(data->regmap), -ENODEV, > + "wrong product id 0x%02x\n", reg_id); > + > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL1, > + MMC5983_CTRL1_SW_RST_BIT); > + if (ret) > + return ret; > + > + fsleep(10000); As below. Would expect a comment on why this time. > + > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, > + MMC5983_CTRL0_OTP_RD_BIT); > + if (ret) > + return ret; > + > + ret = regmap_read_poll_timeout(data->regmap, MMC5983_REG_STATUS, > + status, > + status & MMC5983_STATUS_OTP_RD_DONE_BIT, > + 1000, 10000); Why these times? Spec reference. > + if (ret) > + return ret; > + > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, > + MMC5983_CTRL0_SET_BIT); > + if (ret) > + return ret; > + > + fsleep(500); As below. > + > + ret = regmap_write(data->regmap, MMC5983_REG_CTRL0, > + MMC5983_CTRL0_RESET_BIT); > + if (ret) > + return ret; > + > + fsleep(500); Add a spec reference ideally that justifies this value. Sometimes we get 'slow' parts that need longer. Hence it is good to know where to find the numbers. > + > + 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 = i2c->name; Use a hard coded name. This can provide annoying fragile dependent on exactly how the driver was bound / firmware etc and for now there is only one right answer wrt to what the part number is. > + 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); > +} ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver 2026-05-07 12:47 [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Vladislav Kulikov 2026-05-07 12:47 ` [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA Vladislav Kulikov 2026-05-07 12:47 ` [PATCH 2/3] iio: magnetometer: add driver for " Vladislav Kulikov @ 2026-05-07 12:47 ` Vladislav Kulikov 2026-05-07 16:47 ` Jonathan Cameron 2026-05-08 9:19 ` [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Andy Shevchenko 3 siblings, 1 reply; 8+ messages in thread From: Vladislav Kulikov @ 2026-05-07 12:47 UTC (permalink / raw) To: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt Cc: linux-iio, devicetree, linux-kernel, Vladislav Kulikov Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com> --- MAINTAINERS | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 882214b0e7db..b1d9d7b586a1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -17170,6 +17170,13 @@ 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 +F: drivers/iio/magnetometer/mmc5983.c + MEN A21 WATCHDOG DRIVER M: Johannes Thumshirn <morbidrsa@gmail.com> L: linux-watchdog@vger.kernel.org -- 2.43.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver 2026-05-07 12:47 ` [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver Vladislav Kulikov @ 2026-05-07 16:47 ` Jonathan Cameron 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2026-05-07 16:47 UTC (permalink / raw) To: Vladislav Kulikov Cc: dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio, devicetree, linux-kernel On Thu, 7 May 2026 12:47:24 +0000 Vladislav Kulikov <vlad.kulikov.c@gmail.com> wrote: > Signed-off-by: Vladislav Kulikov <vlad.kulikov.c@gmail.com> Hi Vladislav My preference is for the Maintainers entry to be added with the first patch in the series (dt binding only at that point) and then the other files added in the patches that add them. That way it's always valid and their maintainers are fully documented as part of the patches rather than at the end like this. Jonathan > --- > MAINTAINERS | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 882214b0e7db..b1d9d7b586a1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -17170,6 +17170,13 @@ 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 > +F: drivers/iio/magnetometer/mmc5983.c > + > MEN A21 WATCHDOG DRIVER > M: Johannes Thumshirn <morbidrsa@gmail.com> > L: linux-watchdog@vger.kernel.org ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver 2026-05-07 12:47 [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Vladislav Kulikov ` (2 preceding siblings ...) 2026-05-07 12:47 ` [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver Vladislav Kulikov @ 2026-05-08 9:19 ` Andy Shevchenko 3 siblings, 0 replies; 8+ messages in thread From: Andy Shevchenko @ 2026-05-08 9:19 UTC (permalink / raw) To: Vladislav Kulikov Cc: jic23, dlechner, nuno.sa, andy, robh, krzk+dt, conor+dt, linux-iio, devicetree, linux-kernel On Thu, May 07, 2026 at 12:47:21PM +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 following chip features are intentionally left out of the initial > driver because the public datasheet does not provide enough detail to > expose them confidently through stable IIO ABI, or because they still > need more validation: > > - SPI transport: deferred because SET/RESET polarity behavior has been > reported to differ between I2C and SPI, especially around SET/RESET > timing and/or SPI mode. > - Temperature channel: deferred until the temperature output behavior is > better validated. > - Continuous measurement mode and Auto SET/RESET: deferred because the > datasheet does not clearly define the interaction between CMM, TM_M, > Meas_M_Done, and SET/RESET sequencing. > - Saturation/self-test bits: deferred because the applied test field > strength and bit lifetime are not specified. > - BW/decimation filter tuning: only the documented measurement timing is > used; no filter response is exposed because the filter topology and > coefficients are not documented. > > 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 existing sample code and practical testing > indicate that a longer software delay is needed before taking the next > measurement. Missed section for a new driver. Id est answer the question "Why a new brand driver? Do we have something similar in IIO already to be expanded to cover this HW part?" -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-05-08 9:19 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-05-07 12:47 [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Vladislav Kulikov 2026-05-07 12:47 ` [PATCH 1/3] dt-bindings: iio: magnetometer: add MEMSIC MMC5983MA Vladislav Kulikov 2026-05-07 16:46 ` Jonathan Cameron 2026-05-07 12:47 ` [PATCH 2/3] iio: magnetometer: add driver for " Vladislav Kulikov 2026-05-07 17:00 ` Jonathan Cameron 2026-05-07 12:47 ` [PATCH 3/3] MAINTAINERS: add entry for MEMSIC MMC5983MA magnetometer driver Vladislav Kulikov 2026-05-07 16:47 ` Jonathan Cameron 2026-05-08 9:19 ` [PATCH 0/3] iio: magnetometer: add MEMSIC MMC5983MA driver Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox