* [PATCH 0/3] iio: magnetometer: add driver for QST QMC5883L Sensor
@ 2026-06-12 12:45 Siratul Islam
2026-06-12 12:45 ` [PATCH 1/3] dt-bindings: add entry for qstcorp Siratul Islam
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Siratul Islam @ 2026-06-12 12:45 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-iio, devicetree, linux-kernel,
Siratul Islam
This patch series introduces the QST QMC5883L 3-Axis Magnetic Sensor
driver. It is a simple device with minimal magnetometer functionalities.
Commonly used as (software incompatible) replacement for the
Honeywell HMC5883L sensor.
This driver implements the basic functionalities of the QMC5883L sensor,
and intentionally leaves out some features like DRDY interrupt pin support
and power management for simplicity, both of which will be addressed
in future patches.
There was an attempt to introduce this device about an year ago but
the author seems to have abandoned the patch series. Since the device
is simple enough, I decided to start from scratch.
Note: I also noticed a patch for the QMC5883P variant. Despite similar
naming, the sensors are different including different register maps,
so these devices are not compatible with each other.
Siratul Islam (3):
dt-bindings: add entry for qstcorp
dt-bindings: iio: magnetometer: add QST QMC5883L Sensor
iio: magnetometer: add driver for QST QMC5883L Sensor
.../iio/magnetometer/qstcorp,qmc5883l.yaml | 48 ++
.../devicetree/bindings/vendor-prefixes.yaml | 2 +
MAINTAINERS | 7 +
drivers/iio/magnetometer/Kconfig | 11 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/qmc5883l.c | 512 ++++++++++++++++++
6 files changed, 582 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
create mode 100644 drivers/iio/magnetometer/qmc5883l.c
--
2.54.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/3] dt-bindings: add entry for qstcorp
2026-06-12 12:45 [PATCH 0/3] iio: magnetometer: add driver for QST QMC5883L Sensor Siratul Islam
@ 2026-06-12 12:45 ` Siratul Islam
2026-06-12 12:45 ` [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor Siratul Islam
2026-06-12 12:45 ` [PATCH 3/3] iio: magnetometer: add driver for " Siratul Islam
2 siblings, 0 replies; 8+ messages in thread
From: Siratul Islam @ 2026-06-12 12:45 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-iio, devicetree, linux-kernel,
Siratul Islam
Add an entry for QST Corporation Limited
Signed-off-by: Siratul Islam <email@sirat.me>
---
Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 28784d66ae7b..11aac47f90ce 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1355,6 +1355,8 @@ patternProperties:
description: Shenzhen QiShenglong Industrialist Co., Ltd.
"^qnap,.*":
description: QNAP Systems, Inc.
+ "^qstcorp,.*":
+ description: QST Corporation Limited
"^quanta,.*":
description: Quanta Computer Inc.
"^radxa,.*":
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor
2026-06-12 12:45 [PATCH 0/3] iio: magnetometer: add driver for QST QMC5883L Sensor Siratul Islam
2026-06-12 12:45 ` [PATCH 1/3] dt-bindings: add entry for qstcorp Siratul Islam
@ 2026-06-12 12:45 ` Siratul Islam
2026-06-12 13:13 ` Joshua Crofts
2026-06-12 12:45 ` [PATCH 3/3] iio: magnetometer: add driver for " Siratul Islam
2 siblings, 1 reply; 8+ messages in thread
From: Siratul Islam @ 2026-06-12 12:45 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-iio, devicetree, linux-kernel,
Siratul Islam
Add devicetree binding for the QST QMC5883L 3-Axis Magnetic Sensor
connected via i2c.
Interrupt not implemented in driver but kept in the binding for future
addition.
Used enum so that more driver could use this binding
Signed-off-by: Siratul Islam <email@sirat.me>
---
.../iio/magnetometer/qstcorp,qmc5883l.yaml | 48 +++++++++++++++++++
MAINTAINERS | 6 +++
2 files changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
diff --git a/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
new file mode 100644
index 000000000000..238cc7e22b89
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
@@ -0,0 +1,48 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/magnetometer/qstcorp,qmc5883l.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QST QMC5883L 3-Axis Magnetic Sensor
+
+maintainers:
+ - Siratul Islam <email@sirat.me>
+
+properties:
+ compatible:
+ enum:
+ - qstcorp,qmc5883l
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ vdd-supply: true
+
+ vddio-supply: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - vdd-supply
+ - vddio-supply
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ magnetometer@d {
+ compatible = "qstcorp,qmc5883l";
+ reg = <0x0d>;
+ vdd-supply = <&vdd_3v3_reg>;
+ vddio-supply = <&vdd_3v3_reg>;
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index e035a3be797c..310074b34072 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21787,6 +21787,12 @@ F: Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
F: drivers/bus/fsl-mc/
F: include/uapi/linux/fsl_mc.h
+QST QMC5883L 3-Axis Magnetic Sensor
+M: Siratul Islam <email@sirat.me>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
+
QT1010 MEDIA DRIVER
L: linux-media@vger.kernel.org
S: Orphan
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
2026-06-12 12:45 [PATCH 0/3] iio: magnetometer: add driver for QST QMC5883L Sensor Siratul Islam
2026-06-12 12:45 ` [PATCH 1/3] dt-bindings: add entry for qstcorp Siratul Islam
2026-06-12 12:45 ` [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor Siratul Islam
@ 2026-06-12 12:45 ` Siratul Islam
2026-06-12 12:56 ` sashiko-bot
2026-06-12 13:49 ` Joshua Crofts
2 siblings, 2 replies; 8+ messages in thread
From: Siratul Islam @ 2026-06-12 12:45 UTC (permalink / raw)
To: jic23, robh, krzk+dt, conor+dt
Cc: dlechner, nuno.sa, andy, linux-iio, devicetree, linux-kernel,
Siratul Islam
Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
connected via i2c.
Signed-off-by: Siratul Islam <email@sirat.me>
---
MAINTAINERS | 1 +
drivers/iio/magnetometer/Kconfig | 11 +
drivers/iio/magnetometer/Makefile | 2 +
drivers/iio/magnetometer/qmc5883l.c | 512 ++++++++++++++++++++++++++++
4 files changed, 526 insertions(+)
create mode 100644 drivers/iio/magnetometer/qmc5883l.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 310074b34072..f94da70b91af 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21792,6 +21792,7 @@ M: Siratul Islam <email@sirat.me>
L: linux-iio@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883l.yaml
+F: drivers/iio/magnetometer/qmc5883l.c
QT1010 MEDIA DRIVER
L: linux-media@vger.kernel.org
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index fb313e591e85..615564174086 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -198,6 +198,17 @@ config INFINEON_TLV493D
To compile this driver as a module, choose M here: the module
will be called tlv493d.
+config QMC5883L
+ tristate "QST QMC5883L 3-Axis Magnetic Sensor"
+ depends on I2C
+ select REGMAP_I2C
+ help
+ Say Y here to add support driver for QST QMC5883L 3-Axis
+ Magnetic Sensor.
+
+ To compile this driver as a module, choose M here: the
+ module will be called qmc5883l.
+
config SENSORS_HMC5843
tristate
select IIO_BUFFER
diff --git a/drivers/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 5bd227f8c120..552682555d86 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -26,6 +26,8 @@ obj-$(CONFIG_IIO_ST_MAGN_SPI_3AXIS) += st_magn_spi.o
obj-$(CONFIG_INFINEON_TLV493D) += tlv493d.o
+obj-$(CONFIG_QMC5883L) += qmc5883l.o
+
obj-$(CONFIG_SENSORS_HMC5843) += hmc5843_core.o
obj-$(CONFIG_SENSORS_HMC5843_I2C) += hmc5843_i2c.o
obj-$(CONFIG_SENSORS_HMC5843_SPI) += hmc5843_spi.o
diff --git a/drivers/iio/magnetometer/qmc5883l.c b/drivers/iio/magnetometer/qmc5883l.c
new file mode 100644
index 000000000000..055e51570635
--- /dev/null
+++ b/drivers/iio/magnetometer/qmc5883l.c
@@ -0,0 +1,512 @@
+// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+/*
+ * Support for QST QMC5883L 3-Axis Magnetic Sensor on i2c bus.
+ *
+ * Copyright (C) 2026 Siratul Islam <email@sirat.me>
+ *
+ * Datasheet available at
+ * <https://www.qstcorp.com/upload/pdf/202512/13-52-04%20QMC5883L%20Datasheet%20Rev.%20B.pdf>
+ *
+ * Default 7-bit i2c slave address 0x0D.
+ *
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/time.h>
+
+#include <asm/byteorder.h>
+
+#define QMC5883L_REG_X_LSB 0x00
+#define QMC5883L_REG_STATUS1 0x06
+#define QMC5883L_REG_CTRL1 0x09
+#define QMC5883L_REG_CTRL2 0x0A
+#define QMC5883L_REG_SET_RESET 0x0B
+#define QMC5883L_REG_ID 0x0D
+
+#define QMC5883L_CHIP_ID 0xFF
+
+#define QMC5883L_MODE_MASK GENMASK(1, 0)
+#define QMC5883L_ODR_MASK GENMASK(3, 2)
+#define QMC5883L_RNG_MASK GENMASK(5, 4)
+#define QMC5883L_OSR_MASK GENMASK(7, 6)
+
+#define QMC5883L_MODE_STANDBY FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x00)
+#define QMC5883L_MODE_CONT FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x01)
+
+#define QMC5883L_ODR_10HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x00)
+#define QMC5883L_ODR_50HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x01)
+#define QMC5883L_ODR_100HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x02)
+#define QMC5883L_ODR_200HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x03)
+
+#define QMC5883L_RNG_2G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x00)
+#define QMC5883L_RNG_8G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x01)
+
+#define QMC5883L_OSR_512 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x00)
+#define QMC5883L_OSR_256 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x01)
+#define QMC5883L_OSR_128 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x02)
+#define QMC5883L_OSR_64 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x03)
+
+#define QMC5883L_STATUS_DRDY BIT(0)
+#define QMC5883L_STATUS_OVL BIT(1)
+
+#define QMC5883L_SET_RESET_VAL BIT(0)
+#define QMC5883L_INT_DISABLE BIT(0)
+#define QMC5883L_SOFT_RESET BIT(7)
+
+#define QMC5883L_SCALE_2G 83333
+#define QMC5883L_SCALE_8G 333333
+
+/* POR completion time max per datasheet */
+#define QMC5883L_PORT_US 350
+
+struct qmc5883l_data {
+ struct regmap *regmap;
+ struct mutex mutex; /* update and read regmap data */
+ u8 range;
+ u8 odr;
+ u8 osr;
+};
+
+enum qmc5883l_chan {
+ QMC5883L_AXIS_X,
+ QMC5883L_AXIS_Y,
+ QMC5883L_AXIS_Z,
+};
+
+static const int qmc5883l_odr_avail[] = { 10, 50, 100, 200 };
+
+static const int qmc5883l_osr_avail[] = { 512, 256, 128, 64 };
+
+static const int qmc5883l_rng_avail[] = {
+ 0, QMC5883L_SCALE_2G, /* 2G */
+ 0, QMC5883L_SCALE_8G, /* 8G */
+};
+
+static int qmc5883l_take_measurement(struct iio_dev *indio_dev, int index,
+ int *val)
+{
+ struct qmc5883l_data *data = iio_priv(indio_dev);
+ unsigned int status;
+ __le16 buf[3];
+ int ret;
+
+ scoped_guard(mutex, &data->mutex)
+ {
+ /* 50ms headroom over the slowest ODR (10Hz) */
+ ret = regmap_read_poll_timeout(data->regmap,
+ QMC5883L_REG_STATUS1, status,
+ (status & QMC5883L_STATUS_DRDY),
+ 2 * USEC_PER_MSEC,
+ 150 * USEC_PER_MSEC);
+ if (ret)
+ return ret;
+
+ if (status & QMC5883L_STATUS_OVL)
+ return -ERANGE;
+
+ ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
+ sizeof(buf));
+ if (ret)
+ return ret;
+
+ *val = (s16)le16_to_cpu(buf[index]);
+ }
+
+ return 0;
+}
+
+static int qmc5883l_read_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int *val,
+ int *val2, long mask)
+{
+ struct qmc5883l_data *data = iio_priv(indio_dev);
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ if (!iio_device_claim_direct(indio_dev))
+ return -EBUSY;
+ ret = qmc5883l_take_measurement(indio_dev, chan->address, val);
+ iio_device_release_direct(indio_dev);
+ if (ret)
+ return ret;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ scoped_guard(mutex, &data->mutex)
+ {
+ *val = 0;
+ *val2 = data->range == QMC5883L_RNG_2G ?
+ QMC5883L_SCALE_2G :
+ QMC5883L_SCALE_8G;
+ }
+ return IIO_VAL_INT_PLUS_NANO;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ scoped_guard(mutex, &data->mutex)
+ {
+ switch (data->odr) {
+ case QMC5883L_ODR_200HZ:
+ *val = 200;
+ break;
+ case QMC5883L_ODR_100HZ:
+ *val = 100;
+ break;
+ case QMC5883L_ODR_50HZ:
+ *val = 50;
+ break;
+ case QMC5883L_ODR_10HZ:
+ *val = 10;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ scoped_guard(mutex, &data->mutex)
+ {
+ switch (data->osr) {
+ case QMC5883L_OSR_64:
+ *val = 64;
+ break;
+ case QMC5883L_OSR_128:
+ *val = 128;
+ break;
+ case QMC5883L_OSR_256:
+ *val = 256;
+ break;
+ case QMC5883L_OSR_512:
+ *val = 512;
+ break;
+ default:
+ return -EINVAL;
+ }
+ }
+ return IIO_VAL_INT;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int qmc5883l_write_raw(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan, int val,
+ int val2, long mask)
+{
+ struct qmc5883l_data *data = iio_priv(indio_dev);
+ u8 rng;
+ u8 osr;
+ u8 odr;
+ int ret;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ if (val != 0)
+ return -EINVAL;
+
+ switch (val2) {
+ case QMC5883L_SCALE_2G:
+ rng = QMC5883L_RNG_2G;
+ break;
+ case QMC5883L_SCALE_8G:
+ rng = QMC5883L_RNG_8G;
+ break;
+ default:
+ return -EINVAL;
+ }
+ scoped_guard(mutex, &data->mutex)
+ {
+ ret = regmap_update_bits(data->regmap,
+ QMC5883L_REG_CTRL1,
+ QMC5883L_RNG_MASK, rng);
+ if (ret)
+ return ret;
+ data->range = rng;
+ }
+ break;
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ switch (val) {
+ case 200:
+ odr = QMC5883L_ODR_200HZ;
+ break;
+ case 100:
+ odr = QMC5883L_ODR_100HZ;
+ break;
+ case 50:
+ odr = QMC5883L_ODR_50HZ;
+ break;
+ case 10:
+ odr = QMC5883L_ODR_10HZ;
+ break;
+ default:
+ return -EINVAL;
+ }
+ scoped_guard(mutex, &data->mutex)
+ {
+ ret = regmap_update_bits(data->regmap,
+ QMC5883L_REG_CTRL1,
+ QMC5883L_ODR_MASK, odr);
+ if (ret)
+ return ret;
+ data->odr = odr;
+ }
+ break;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ switch (val) {
+ case 64:
+ osr = QMC5883L_OSR_64;
+ break;
+ case 128:
+ osr = QMC5883L_OSR_128;
+ break;
+ case 256:
+ osr = QMC5883L_OSR_256;
+ break;
+ case 512:
+ osr = QMC5883L_OSR_512;
+ break;
+ default:
+ return -EINVAL;
+ }
+ scoped_guard(mutex, &data->mutex)
+ {
+ ret = regmap_update_bits(data->regmap,
+ QMC5883L_REG_CTRL1,
+ QMC5883L_OSR_MASK, osr);
+ if (ret)
+ return ret;
+ data->osr = osr;
+ }
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int qmc5883l_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_SAMP_FREQ:
+ *vals = qmc5883l_odr_avail;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(qmc5883l_odr_avail);
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+ *vals = qmc5883l_osr_avail;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(qmc5883l_osr_avail);
+ return IIO_AVAIL_LIST;
+ case IIO_CHAN_INFO_SCALE:
+ *vals = qmc5883l_rng_avail;
+ *type = IIO_VAL_INT_PLUS_NANO;
+ *length = ARRAY_SIZE(qmc5883l_rng_avail);
+ return IIO_AVAIL_LIST;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int qmc5883l_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;
+ default:
+ return IIO_VAL_INT;
+ }
+}
+
+static const struct iio_info qmc5883l_info = {
+ .read_raw = qmc5883l_read_raw,
+ .write_raw = qmc5883l_write_raw,
+ .read_avail = qmc5883l_read_avail,
+ .write_raw_get_fmt = qmc5883l_write_raw_get_fmt,
+};
+
+static int qmc5883l_init(struct qmc5883l_data *data)
+{
+ unsigned int reg;
+ int ret;
+
+ ret = regmap_read(data->regmap, QMC5883L_REG_ID, ®);
+ if (ret)
+ return ret;
+
+ /* Not failing because rev 1.0 had this register reserved */
+ if (reg != QMC5883L_CHIP_ID)
+ dev_warn(regmap_get_device(data->regmap),
+ "unknown chip id: 0x%02x, continuing\n", reg);
+
+ ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
+ QMC5883L_SOFT_RESET);
+ if (ret)
+ return ret;
+
+ fsleep(QMC5883L_PORT_US);
+
+ /* DRDY pin no used in this version of the driver */
+ ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
+ QMC5883L_INT_DISABLE);
+ if (ret)
+ return ret;
+
+ ret = regmap_write(data->regmap, QMC5883L_REG_SET_RESET,
+ QMC5883L_SET_RESET_VAL);
+ if (ret)
+ return ret;
+
+ data->odr = QMC5883L_ODR_50HZ;
+ data->range = QMC5883L_RNG_2G;
+ data->osr = QMC5883L_OSR_64;
+
+ ret = regmap_write(data->regmap, QMC5883L_REG_CTRL1,
+ (QMC5883L_MODE_CONT | data->odr | data->range |
+ data->osr));
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static bool qmc5883l_volatile_reg(struct device *dev, unsigned int reg)
+{
+ return reg <= QMC5883L_REG_STATUS1;
+}
+
+static bool qmc5883l_writable_reg(struct device *dev, unsigned int reg)
+{
+ switch (reg) {
+ case QMC5883L_REG_CTRL1:
+ case QMC5883L_REG_CTRL2:
+ case QMC5883L_REG_SET_RESET:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static const struct regmap_config qmc5883l_regmap_config = {
+ .reg_bits = 8,
+ .val_bits = 8,
+ .max_register = QMC5883L_REG_ID,
+ .cache_type = REGCACHE_MAPLE,
+ .volatile_reg = qmc5883l_volatile_reg,
+ .writeable_reg = qmc5883l_writable_reg
+};
+
+#define QMC5883L_CHANNEL(_axis) \
+ { \
+ .type = IIO_MAGN, \
+ .modified = 1, \
+ .channel2 = IIO_MOD_##_axis, \
+ .address = QMC5883L_AXIS_##_axis, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ .info_mask_shared_by_type_available = \
+ BIT(IIO_CHAN_INFO_SCALE) | \
+ BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
+ }
+
+static const struct iio_chan_spec qmc5883l_channels[] = {
+ QMC5883L_CHANNEL(X),
+ QMC5883L_CHANNEL(Y),
+ QMC5883L_CHANNEL(Z),
+};
+
+static int qmc5883l_probe(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct regmap *regmap;
+ struct qmc5883l_data *data;
+ struct device *dev = &client->dev;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ regmap = devm_regmap_init_i2c(client, &qmc5883l_regmap_config);
+ if (IS_ERR(regmap)) {
+ return dev_err_probe(dev, PTR_ERR(regmap),
+ "regmap initialization failed\n");
+ }
+
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable VDD regulator\n");
+
+ ret = devm_regulator_get_enable(dev, "vddio");
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Failed to enable VDDIO regulator\n");
+
+ fsleep(QMC5883L_PORT_US);
+
+ data = iio_priv(indio_dev);
+ data->regmap = regmap;
+ ret = devm_mutex_init(dev, &data->mutex);
+ if (ret)
+ return ret;
+
+ indio_dev->name = "qmc5883l";
+ indio_dev->info = &qmc5883l_info;
+ indio_dev->channels = qmc5883l_channels;
+ indio_dev->num_channels = ARRAY_SIZE(qmc5883l_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = qmc5883l_init(data);
+ if (ret)
+ return dev_err_probe(dev, ret, "qmc5883l init failed\n");
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id qmc5883l_match[] = {
+ { .compatible = "qstcorp,qmc5883l" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, qmc5883l_match);
+
+static const struct i2c_device_id qmc5883l_id[] = {
+ { "qmc5883l" },
+ { },
+};
+MODULE_DEVICE_TABLE(i2c, qmc5883l_id);
+
+static struct i2c_driver qmc5883l_driver = {
+ .driver = {
+ .name = "qmc5883l",
+ .of_match_table = qmc5883l_match,
+ },
+ .id_table = qmc5883l_id,
+ .probe = qmc5883l_probe,
+};
+
+module_i2c_driver(qmc5883l_driver);
+
+MODULE_DESCRIPTION("QST QMC5883L 3-Axis Magnetic Sensor driver");
+MODULE_AUTHOR("Siratul Islam <email@sirat.me>");
+MODULE_LICENSE("Dual BSD/GPL");
--
2.54.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
2026-06-12 12:45 ` [PATCH 3/3] iio: magnetometer: add driver for " Siratul Islam
@ 2026-06-12 12:56 ` sashiko-bot
2026-06-12 13:49 ` Joshua Crofts
1 sibling, 0 replies; 8+ messages in thread
From: sashiko-bot @ 2026-06-12 12:56 UTC (permalink / raw)
To: Siratul Islam; +Cc: devicetree, conor+dt, robh
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Returning early with `-ERANGE` when the overflow flag (OVL) is set prevents the data registers from being read, leaving the sensor permanently stuck in an overflow state.
- [Medium] The device is left in continuous measurement mode on driver removal or initialization failure, causing an unnecessary power drain.
--
commit 8aa498258e2b8bddd48bf3c46ef8555a7376ea6e
Author: Siratul Islam <email@sirat.me>
iio: magnetometer: add driver for QST QMC5883L Sensor
This commit adds a new I2C driver for the QST QMC5883L 3-Axis Magnetic Sensor,
including continuous measurement mode support and scaling options.
> diff --git a/drivers/iio/magnetometer/qmc5883l.c b/drivers/iio/magnetometer/qmc5883l.c
> new file mode 100644
> index 0000000000000..055e51570635c
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc5883l.c
[ ... ]
> +static int qmc5883l_take_measurement(struct iio_dev *indio_dev, int index,
> + int *val)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + unsigned int status;
> + __le16 buf[3];
> + int ret;
> +
> + scoped_guard(mutex, &data->mutex)
> + {
> + /* 50ms headroom over the slowest ODR (10Hz) */
> + ret = regmap_read_poll_timeout(data->regmap,
> + QMC5883L_REG_STATUS1, status,
> + (status & QMC5883L_STATUS_DRDY),
> + 2 * USEC_PER_MSEC,
> + 150 * USEC_PER_MSEC);
> + if (ret)
> + return ret;
> +
> + if (status & QMC5883L_STATUS_OVL)
> + return -ERANGE;
[Severity: High]
If we return -ERANGE here when the overflow flag (OVL) is set, does the
sensor get permanently stuck in an overflow state?
In typical I2C magnetometers, the Data Ready (DRDY) and Overflow (OVL)
status bits are only cleared by reading the data registers. By returning
early without reading the data registers via regmap_bulk_read(), the DRDY
and OVL flags might remain set indefinitely.
On subsequent measurement attempts, regmap_read_poll_timeout() will return
immediately and this check will instantly fail again, potentially locking up
the sensor until a reset.
> +
> + ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
> + sizeof(buf));
[ ... ]
> +static int qmc5883l_init(struct qmc5883l_data *data)
> +{
> + unsigned int reg;
> + int ret;
> +
[ ... ]
> + data->odr = QMC5883L_ODR_50HZ;
> + data->range = QMC5883L_RNG_2G;
> + data->osr = QMC5883L_OSR_64;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL1,
> + (QMC5883L_MODE_CONT | data->odr | data->range |
> + data->osr));
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
[ ... ]
> +static int qmc5883l_probe(struct i2c_client *client)
> +{
[ ... ]
> + indio_dev->name = "qmc5883l";
> + indio_dev->info = &qmc5883l_info;
> + indio_dev->channels = qmc5883l_channels;
> + indio_dev->num_channels = ARRAY_SIZE(qmc5883l_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = qmc5883l_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "qmc5883l init failed\n");
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
[Severity: Medium]
Is there a missing devm teardown callback to power down the sensor when the
driver is unbound or if registration fails?
In qmc5883l_init(), the sensor is placed into continuous sampling mode. If
devm_iio_device_register() fails, or when the driver is removed, there
doesn't appear to be a registered cleanup action (e.g., via
devm_add_action_or_reset()) to place the device back into standby mode.
Although the devm core disables the VDD/VDDIO regulators during teardown,
these regulators might be shared with other components on the board. If the
shared supply remains active, the sensor could continue continuous sampling
and unnecessarily drain power.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260612124557.13750-1-email@sirat.me?part=3
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor
2026-06-12 12:45 ` [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor Siratul Islam
@ 2026-06-12 13:13 ` Joshua Crofts
2026-06-12 13:45 ` Jonathan Cameron
0 siblings, 1 reply; 8+ messages in thread
From: Joshua Crofts @ 2026-06-12 13:13 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
On Fri, 12 Jun 2026 18:45:26 +0600
Siratul Islam <email@sirat.me> wrote:
> +QST QMC5883L 3-Axis Magnetic Sensor
> +M: Siratul Islam <email@sirat.me>
> +L: linux-iio@vger.kernel.org
There's no point in having the IIO list in your MAINTAINERS
entry, get_maintainer.pl would return it automatically based
on the driver file's path.
--
Kind regards
CJD
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor
2026-06-12 13:13 ` Joshua Crofts
@ 2026-06-12 13:45 ` Jonathan Cameron
0 siblings, 0 replies; 8+ messages in thread
From: Jonathan Cameron @ 2026-06-12 13:45 UTC (permalink / raw)
To: Joshua Crofts
Cc: Siratul Islam, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
On Fri, 12 Jun 2026 15:13:24 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> On Fri, 12 Jun 2026 18:45:26 +0600
> Siratul Islam <email@sirat.me> wrote:
>
> > +QST QMC5883L 3-Axis Magnetic Sensor
> > +M: Siratul Islam <email@sirat.me>
> > +L: linux-iio@vger.kernel.org
>
> There's no point in having the IIO list in your MAINTAINERS
> entry, get_maintainer.pl would return it automatically based
> on the driver file's path.
True, but I'm not sure there is a standard convention for whether
lists should be added in this case or not.
One of those things where we should decide on an answer perhaps
and stick to it. Andy, Nuno, David, Dt folk what do you think?
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] iio: magnetometer: add driver for QST QMC5883L Sensor
2026-06-12 12:45 ` [PATCH 3/3] iio: magnetometer: add driver for " Siratul Islam
2026-06-12 12:56 ` sashiko-bot
@ 2026-06-12 13:49 ` Joshua Crofts
1 sibling, 0 replies; 8+ messages in thread
From: Joshua Crofts @ 2026-06-12 13:49 UTC (permalink / raw)
To: Siratul Islam
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-iio, devicetree, linux-kernel
On Fri, 12 Jun 2026 18:45:27 +0600
Siratul Islam <email@sirat.me> wrote:
> Add driver for the QST QMC5883L 3-Axis Magnetic Sensor
> connected via i2c.
>
> Signed-off-by: Siratul Islam <email@sirat.me>
> ---
Hi Siratul,
various comments inline. I've probably missed a few things
as I only took a quick look so feel free to call me out on that!
Josh
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc5883l.c
> @@ -0,0 +1,512 @@
> +// SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +/*
> + * Support for QST QMC5883L 3-Axis Magnetic Sensor on i2c bus.
Nitpick, but I'd write it I2C with capital letters.
> + *
> + * Copyright (C) 2026 Siratul Islam <email@sirat.me>
> + *
> + * Datasheet available at
> + * <https://www.qstcorp.com/upload/pdf/202512/13-52-04%20QMC5883L%20Datasheet%20Rev.%20B.pdf>
> + *
> + * Default 7-bit i2c slave address 0x0D.
You also have this as a macro, consider removing this comment.
> + *
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
Move IIO specific headers below generic linux headers, i. e.
#include <linux/x.h>
#include <linux/y.h>
#include <linux/iio/z.h>
> +#include <linux/i2c.h>
> +#include <linux/kernel.h>
No, refrain from using kernel.h, include the actual headers
instead of relying on this (on second thought, I think you've
actually included all the required headers, so you definitely
don't need this).
> +#include <linux/module.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/time.h>
> +
> +#include <asm/byteorder.h>
> +
> +#define QMC5883L_REG_X_LSB 0x00
> +#define QMC5883L_REG_STATUS1 0x06
> +#define QMC5883L_REG_CTRL1 0x09
> +#define QMC5883L_REG_CTRL2 0x0A
> +#define QMC5883L_REG_SET_RESET 0x0B
> +#define QMC5883L_REG_ID 0x0D
> +
> +#define QMC5883L_CHIP_ID 0xFF
> +
> +#define QMC5883L_MODE_MASK GENMASK(1, 0)
> +#define QMC5883L_ODR_MASK GENMASK(3, 2)
> +#define QMC5883L_RNG_MASK GENMASK(5, 4)
> +#define QMC5883L_OSR_MASK GENMASK(7, 6)
> +
> +#define QMC5883L_MODE_STANDBY FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x00)
> +#define QMC5883L_MODE_CONT FIELD_PREP_CONST(QMC5883L_MODE_MASK, 0x01)
> +
> +#define QMC5883L_ODR_10HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x00)
> +#define QMC5883L_ODR_50HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x01)
> +#define QMC5883L_ODR_100HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x02)
> +#define QMC5883L_ODR_200HZ FIELD_PREP_CONST(QMC5883L_ODR_MASK, 0x03)
> +
> +#define QMC5883L_RNG_2G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x00)
> +#define QMC5883L_RNG_8G FIELD_PREP_CONST(QMC5883L_RNG_MASK, 0x01)
> +
> +#define QMC5883L_OSR_512 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x00)
> +#define QMC5883L_OSR_256 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x01)
> +#define QMC5883L_OSR_128 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x02)
> +#define QMC5883L_OSR_64 FIELD_PREP_CONST(QMC5883L_OSR_MASK, 0x03)
> +
> +#define QMC5883L_STATUS_DRDY BIT(0)
> +#define QMC5883L_STATUS_OVL BIT(1)
> +
> +#define QMC5883L_SET_RESET_VAL BIT(0)
> +#define QMC5883L_INT_DISABLE BIT(0)
> +#define QMC5883L_SOFT_RESET BIT(7)
> +
> +#define QMC5883L_SCALE_2G 83333
> +#define QMC5883L_SCALE_8G 333333
> +
> +/* POR completion time max per datasheet */
> +#define QMC5883L_PORT_US 350
If it's POR completion, why does the macro contain PORT instead?
> +
> +struct qmc5883l_data {
> + struct regmap *regmap;
> + struct mutex mutex; /* update and read regmap data */
> + u8 range;
> + u8 odr;
> + u8 osr;
> +};
> +
> +enum qmc5883l_chan {
> + QMC5883L_AXIS_X,
> + QMC5883L_AXIS_Y,
> + QMC5883L_AXIS_Z,
> +};
> +
> +static const int qmc5883l_odr_avail[] = { 10, 50, 100, 200 };
> +
> +static const int qmc5883l_osr_avail[] = { 512, 256, 128, 64 };
> +
> +static const int qmc5883l_rng_avail[] = {
> + 0, QMC5883L_SCALE_2G, /* 2G */
These comments are redundant IMO, you're already mentioning
the value in the macro name.
> + 0, QMC5883L_SCALE_8G, /* 8G */
> +};
> +
> +static int qmc5883l_take_measurement(struct iio_dev *indio_dev, int index,
> + int *val)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + unsigned int status;
> + __le16 buf[3];
> + int ret;
> +
> + scoped_guard(mutex, &data->mutex)
Why scoped_guard? I don't see any other possible code paths in this
function etc., just regular guard() should suffice.
> + {
> + /* 50ms headroom over the slowest ODR (10Hz) */
> + ret = regmap_read_poll_timeout(data->regmap,
> + QMC5883L_REG_STATUS1, status,
> + (status & QMC5883L_STATUS_DRDY),
> + 2 * USEC_PER_MSEC,
> + 150 * USEC_PER_MSEC);
> + if (ret)
> + return ret;
> +
> + if (status & QMC5883L_STATUS_OVL)
> + return -ERANGE;
Sashiko has a remark:
If we return -ERANGE here when the overflow flag (OVL) is set, does the
sensor get permanently stuck in an overflow state?
In typical I2C magnetometers, the Data Ready (DRDY) and Overflow (OVL)
status bits are only cleared by reading the data registers. By returning
early without reading the data registers via regmap_bulk_read(), the DRDY
and OVL flags might remain set indefinitely.
On subsequent measurement attempts, regmap_read_poll_timeout() will return
immediately and this check will instantly fail again, potentially locking up
the sensor until a reset.
> +
> + ret = regmap_bulk_read(data->regmap, QMC5883L_REG_X_LSB, buf,
> + sizeof(buf));
> + if (ret)
> + return ret;
> +
> + *val = (s16)le16_to_cpu(buf[index]);
> + }
> +
> + return 0;
> +}
> +
> +static int qmc5883l_read_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int *val,
I'd put val on the same line as val2 and mask, more logical separation.
> + int *val2, long mask)
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> + ret = qmc5883l_take_measurement(indio_dev, chan->address, val);
> + iio_device_release_direct(indio_dev);
> + if (ret)
> + return ret;
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_SCALE:
Again, you can probably just add guard() before the switch.
> + scoped_guard(mutex, &data->mutex)
> + {
> + *val = 0;
> + *val2 = data->range == QMC5883L_RNG_2G ?
> + QMC5883L_SCALE_2G :
> + QMC5883L_SCALE_8G;
> + }
> + return IIO_VAL_INT_PLUS_NANO;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + scoped_guard(mutex, &data->mutex)
> + {
> + switch (data->odr) {
> + case QMC5883L_ODR_200HZ:
> + *val = 200;
> + break;
> + case QMC5883L_ODR_100HZ:
> + *val = 100;
> + break;
> + case QMC5883L_ODR_50HZ:
> + *val = 50;
> + break;
> + case QMC5883L_ODR_10HZ:
> + *val = 10;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> + return IIO_VAL_INT;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + scoped_guard(mutex, &data->mutex)
> + {
> + switch (data->osr) {
> + case QMC5883L_OSR_64:
> + *val = 64;
> + break;
> + case QMC5883L_OSR_128:
> + *val = 128;
> + break;
> + case QMC5883L_OSR_256:
> + *val = 256;
> + break;
> + case QMC5883L_OSR_512:
> + *val = 512;
> + break;
> + default:
> + return -EINVAL;
> + }
> + }
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int qmc5883l_write_raw(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan, int val,
> + int val2, long mask)
Same comment with val as previous function.
> +{
> + struct qmc5883l_data *data = iio_priv(indio_dev);
> + u8 rng;
> + u8 osr;
> + u8 odr;
> + int ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_SCALE:
> + if (val != 0)
> + return -EINVAL;
> +
> + switch (val2) {
> + case QMC5883L_SCALE_2G:
> + rng = QMC5883L_RNG_2G;
> + break;
> + case QMC5883L_SCALE_8G:
> + rng = QMC5883L_RNG_8G;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
Again, guard() is probably good enough.
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_RNG_MASK, rng);
> + if (ret)
> + return ret;
> + data->range = rng;
> + }
> + break;
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + switch (val) {
> + case 200:
> + odr = QMC5883L_ODR_200HZ;
> + break;
> + case 100:
> + odr = QMC5883L_ODR_100HZ;
> + break;
> + case 50:
> + odr = QMC5883L_ODR_50HZ;
> + break;
> + case 10:
> + odr = QMC5883L_ODR_10HZ;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_ODR_MASK, odr);
> + if (ret)
> + return ret;
> + data->odr = odr;
> + }
> + break;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + switch (val) {
> + case 64:
> + osr = QMC5883L_OSR_64;
> + break;
> + case 128:
> + osr = QMC5883L_OSR_128;
> + break;
> + case 256:
> + osr = QMC5883L_OSR_256;
> + break;
> + case 512:
> + osr = QMC5883L_OSR_512;
> + break;
> + default:
> + return -EINVAL;
> + }
> + scoped_guard(mutex, &data->mutex)
> + {
> + ret = regmap_update_bits(data->regmap,
> + QMC5883L_REG_CTRL1,
> + QMC5883L_OSR_MASK, osr);
> + if (ret)
> + return ret;
> + data->osr = osr;
> + }
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int qmc5883l_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_SAMP_FREQ:
> + *vals = qmc5883l_odr_avail;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883l_odr_avail);
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> + *vals = qmc5883l_osr_avail;
> + *type = IIO_VAL_INT;
> + *length = ARRAY_SIZE(qmc5883l_osr_avail);
> + return IIO_AVAIL_LIST;
> + case IIO_CHAN_INFO_SCALE:
> + *vals = qmc5883l_rng_avail;
> + *type = IIO_VAL_INT_PLUS_NANO;
> + *length = ARRAY_SIZE(qmc5883l_rng_avail);
> + return IIO_AVAIL_LIST;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int qmc5883l_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;
> + default:
> + return IIO_VAL_INT;
> + }
> +}
> +
> +static const struct iio_info qmc5883l_info = {
> + .read_raw = qmc5883l_read_raw,
> + .write_raw = qmc5883l_write_raw,
> + .read_avail = qmc5883l_read_avail,
> + .write_raw_get_fmt = qmc5883l_write_raw_get_fmt,
> +};
> +
> +static int qmc5883l_init(struct qmc5883l_data *data)
> +{
> + unsigned int reg;
> + int ret;
struct regmap *regmap = data->regmap;
Makes your lines shorter.
> + ret = regmap_read(data->regmap, QMC5883L_REG_ID, ®);
> + if (ret)
> + return ret;
> +
> + /* Not failing because rev 1.0 had this register reserved */
> + if (reg != QMC5883L_CHIP_ID)
> + dev_warn(regmap_get_device(data->regmap),
> + "unknown chip id: 0x%02x, continuing\n", reg);
I recall a conversation about using dev_warn() or dev_info() for checking
the chip ID, up to personal preference. Nevertheless, your error messages
should follow the same format, so capital letter at the beginning (nitpick).
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_SOFT_RESET);
> + if (ret)
> + return ret;
> +
> + fsleep(QMC5883L_PORT_US);
> +
> + /* DRDY pin no used in this version of the driver */
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL2,
> + QMC5883L_INT_DISABLE);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_SET_RESET,
> + QMC5883L_SET_RESET_VAL);
> + if (ret)
> + return ret;
> +
> + data->odr = QMC5883L_ODR_50HZ;
> + data->range = QMC5883L_RNG_2G;
> + data->osr = QMC5883L_OSR_64;
> +
> + ret = regmap_write(data->regmap, QMC5883L_REG_CTRL1,
> + (QMC5883L_MODE_CONT | data->odr | data->range |
> + data->osr));
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> +static bool qmc5883l_volatile_reg(struct device *dev, unsigned int reg)
> +{
> + return reg <= QMC5883L_REG_STATUS1;
> +}
> +
> +static bool qmc5883l_writable_reg(struct device *dev, unsigned int reg)
> +{
> + switch (reg) {
> + case QMC5883L_REG_CTRL1:
> + case QMC5883L_REG_CTRL2:
> + case QMC5883L_REG_SET_RESET:
> + return true;
> + default:
> + return false;
> + }
> +}
> +
> +static const struct regmap_config qmc5883l_regmap_config = {
> + .reg_bits = 8,
> + .val_bits = 8,
> + .max_register = QMC5883L_REG_ID,
> + .cache_type = REGCACHE_MAPLE,
> + .volatile_reg = qmc5883l_volatile_reg,
> + .writeable_reg = qmc5883l_writable_reg
> +};
> +
> +#define QMC5883L_CHANNEL(_axis) \
> + { \
> + .type = IIO_MAGN, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_##_axis, \
> + .address = QMC5883L_AXIS_##_axis, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_type = \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + .info_mask_shared_by_type_available = \
> + BIT(IIO_CHAN_INFO_SCALE) | \
> + BIT(IIO_CHAN_INFO_SAMP_FREQ) | \
> + BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO), \
> + }
> +
> +static const struct iio_chan_spec qmc5883l_channels[] = {
> + QMC5883L_CHANNEL(X),
> + QMC5883L_CHANNEL(Y),
> + QMC5883L_CHANNEL(Z),
> +};
> +
> +static int qmc5883l_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct regmap *regmap;
> + struct qmc5883l_data *data;
> + struct device *dev = &client->dev;
Reverse Christmas tree order.
> + int ret;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + regmap = devm_regmap_init_i2c(client, &qmc5883l_regmap_config);
> + if (IS_ERR(regmap)) {
No point in adding brackets if this is a single line if statement
(checkpatch.pl should warn about this IMO).
> + return dev_err_probe(dev, PTR_ERR(regmap),
> + "regmap initialization failed\n");
> + }
> +
> + ret = devm_regulator_get_enable(dev, "vdd");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable VDD regulator\n");
> +
> + ret = devm_regulator_get_enable(dev, "vddio");
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "Failed to enable VDDIO regulator\n");
> +
> + fsleep(QMC5883L_PORT_US);
> +
> + data = iio_priv(indio_dev);
> + data->regmap = regmap;
Blank line here.
> + ret = devm_mutex_init(dev, &data->mutex);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = "qmc5883l";
> + indio_dev->info = &qmc5883l_info;
> + indio_dev->channels = qmc5883l_channels;
> + indio_dev->num_channels = ARRAY_SIZE(qmc5883l_channels);
> + indio_dev->modes = INDIO_DIRECT_MODE;
> +
> + ret = qmc5883l_init(data);
> + if (ret)
> + return dev_err_probe(dev, ret, "qmc5883l init failed\n");
> +
> + return devm_iio_device_register(dev, indio_dev);
> +}
You're using devm_* functions, however I don't see any callback on driver
unbinding or a potential error during probing. Consider adding a power off
callback.
--
Kind regards
CJD
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-12 13:49 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-12 12:45 [PATCH 0/3] iio: magnetometer: add driver for QST QMC5883L Sensor Siratul Islam
2026-06-12 12:45 ` [PATCH 1/3] dt-bindings: add entry for qstcorp Siratul Islam
2026-06-12 12:45 ` [PATCH 2/3] dt-bindings: iio: magnetometer: add QST QMC5883L Sensor Siratul Islam
2026-06-12 13:13 ` Joshua Crofts
2026-06-12 13:45 ` Jonathan Cameron
2026-06-12 12:45 ` [PATCH 3/3] iio: magnetometer: add driver for " Siratul Islam
2026-06-12 12:56 ` sashiko-bot
2026-06-12 13:49 ` Joshua Crofts
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox