[PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[Hardik Phalet]

This series adds initial Linux support for the QST QMC5883P, a 3-axis
anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC that
communicates over I2C. To my knowledge there is no existing upstream
driver for this device.

The driver supports:
  - Raw magnetic field readings on X, Y, and Z axes
  - Four selectable full-scale ranges (±2 G, ±8 G, ±12 G, ±30 G)
  - Configurable output data rate (10, 50, 100, 200 Hz)
  - Configurable oversampling ratio (1, 2, 4, 8)
  - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
    attribute
  - Runtime PM with a 2 s autosuspend delay
  - System suspend/resume via pm_runtime_force_suspend/resume

Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
bit fields are accessed via regmap_field to avoid read-modify-write
races. The STATUS register is marked precious so regmap never reads
it speculatively and clears the DRDY/OVFL bits unexpectedly.

The init sequence on probe is: soft reset → wait 1 ms → deassert
reset → configure SET/RESET control → apply default ODR/OSR/DSR/RNG
→ enter normal mode. This ordering was determined empirically on
hardware to produce reliable, non-zero axis readings.

The driver is placed under drivers/staging/iio/magnetometer/ with a
TODO file tracking the remaining work before it can graduate:
  - Triggered buffer support (iio_triggered_buffer_setup)
  - DRDY interrupt support
  - Self-test implementation

Patches:
  1/4 - dt-bindings: vendor-prefixes: Add 'qst' for QST Corporation
  2/4 - dt-bindings: iio: magnetometer: Add binding for QST QMC5883P
  3/4 - staging: iio: magnetometer: Add QST QMC5883P driver
  4/4 - MAINTAINERS: Add entry for QST QMC5883P magnetometer driver

Testing
-------
Tested on a Raspberry Pi 4B running a mainline kernel (aarch64) with a
GY-271 HM-246 board connected via I2C bus 1. The chip was confirmed to
enumerate at address 0x2C via i2cdetect.

The driver was cross-compiled from Fedora (x86_64) targeting aarch64
and loaded as a module (qmc5883p.ko) with the Device Tree overlay
pointing at i2c1:0x2c.

Verification steps performed:
  - Chip ID register (0x00) reads back 0x80 on probe, confirming the
    correct device is present
  - All three axes (in_magn_x_raw, in_magn_y_raw, in_magn_z_raw) return
    non-zero, stable values when the board is held still and change
    appropriately when the board is rotated
  - in_magn_x_scale (and Y, Z) returns the expected fractional value for
    the default ±8 G range (1/37500000)
  - in_magn_sampling_frequency / _available, in_magn_oversampling_ratio /
    _available, and downsampling_ratio / downsampling_ratio_available all
    read and write correctly; the chip responds without error to each
    valid setting
  - Runtime PM: after 2 s of inactivity the device enters suspend mode
    (MODE = 0x00 confirmed via i2cdump); the next sysfs read correctly
    resumes the device and returns valid data
  - System suspend/resume (echo mem > /sys/power/state) leaves the
    driver in a consistent state; readings remain valid after resume
  - dt_binding_check passes for patch 2/4
  - Kernel builds cleanly with W=1 and no new warnings

Changes in v2:
  - Use get_unaligned_le16() from <linux/unaligned.h> instead of manual
    byte-shifting for deserialising axis data (review feedback)
  - Fix pm_runtime_* calls in downsampling_ratio_store() to use
    data->dev (the i2c parent device) instead of dev (the iio device),
    avoiding PM refcount imbalances (review feedback)
  - Replace manual pm_runtime_disable() devm action with
    devm_pm_runtime_enable(), which avoids a kcfi-violating function
    pointer cast (review feedback)
  - Move chip suspend into a devm action (qmc5883p_suspend_action)
    registered before devm_iio_device_register() so that devres LIFO
    ordering guarantees the IIO interface is fully unregistered before
    the hardware is put to sleep, closing a race window on removal
    (review feedback)
  - Drop qmc5883p_remove() and the .remove hook entirely, as the above
    devm action subsumes it (review feedback)
  - Remove the empty qmc5883p_runtime_idle() stub; passing NULL in
    RUNTIME_PM_OPS already provides the correct default behaviour
  - Add regulator support: use devm_regulator_get_enable_optional() for
    the vdd-supply documented in the dt-binding, with a 50 ms post-enable
    delay per datasheet section 5.3 (PSUP ramp + POR time) (review
    feedback)
  - Reinitialise the chip in qmc5883p_system_resume() via
    qmc5883p_chip_init() followed by regcache_mark_dirty() +
    regcache_sync(), so that the driver recovers correctly if the
    regulator was physically cut during system suspend and POR reset all
    registers (review feedback)

v1: https://lore.kernel.org/all/20260409162308.2590385-1-hardik.phalet@pm.me/

Hardik Phalet (4):
  dt-bindings: vendor-prefixes: Add QST Corporation
  dt-bindings: iio: magnetometer: Add binding for QST QMC5883P
  staging: iio: magnetometer: Add QST QMC5883P driver
  MAINTAINERS: Add entry for QST QMC5883P magnetometer driver

 .../iio/magnetometer/qst,qmc5883p.yaml        |  48 +
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/staging/iio/Kconfig                   |   1 +
 drivers/staging/iio/Makefile                  |   1 +
 drivers/staging/iio/magnetometer/Kconfig      |  20 +
 drivers/staging/iio/magnetometer/Makefile     |   7 +
 drivers/staging/iio/magnetometer/TODO         |   5 +
 drivers/staging/iio/magnetometer/qmc5883p.c   | 830 ++++++++++++++++++
 9 files changed, 921 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
 create mode 100644 drivers/staging/iio/magnetometer/Kconfig
 create mode 100644 drivers/staging/iio/magnetometer/Makefile
 create mode 100644 drivers/staging/iio/magnetometer/TODO
 create mode 100644 drivers/staging/iio/magnetometer/qmc5883p.c

-- 
2.53.0

[PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation

[Hardik Phalet]

Add the vendor prefix 'qst' for QST Corporation, a manufacturer of
MEMS sensors.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.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 5d2a7a8d3ac6..71a1b9087c5e 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1244,6 +1244,8 @@ patternProperties:
     description: Shenzhen QiShenglong Industrialist Co., Ltd.
   "^qnap,.*":
     description: QNAP Systems, Inc.
+  "^qst,.*":
+    description: QST Corporation
   "^quanta,.*":
     description: Quanta Computer Inc.
   "^radxa,.*":
-- 
2.53.0

[PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P

[Hardik Phalet]

Add the device tree binding document for the QST QMC5883P, a 3-axis
anisotropic magneto-resistive (AMR) sensor with a 16-bit ADC that
communicates over I2C. The binding exposes the required 'compatible'
and 'reg' properties along with an optional 'vdd-supply' for the
2.5 V–3.6 V VDD rail.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 .../iio/magnetometer/qst,qmc5883p.yaml        | 48 +++++++++++++++++++
 1 file changed, 48 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml b/Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
new file mode 100644
index 000000000000..84fec10d8b9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.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/qst,qmc5883p.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QST QMC5883P 3-axis magnetometer
+
+maintainers:
+  - Hardik Phalet <hardik.phalet@pm.me>
+
+description:
+  The QMC5883P is a 3-axis anisotropic magneto-resistive (AMR) sensor with a
+  16-bit ADC. It communicates over I2C (standard and fast modes) and is
+  targeted at compass, navigation, and industrial applications.
+
+properties:
+  compatible:
+    const: qst,qmc5883p
+
+  reg:
+    maxItems: 1
+    description: I2C address of the device; the default address is 0x2c.
+
+  vdd-supply:
+    description:
+      VDD power supply (2.5 V to 3.6 V). Powers all internal analog and
+      digital functional blocks.
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        magnetometer@2c {
+            compatible = "qst,qmc5883p";
+            reg = <0x2c>;
+            vdd-supply = <&vdd_3v3>;
+        };
+    };
+...
-- 
2.53.0

[PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[Hardik Phalet]

Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
over I2C. There is no existing upstream driver for this device.

The driver supports:
 - Raw magnetic field readings on X, Y, and Z axes
 - Four full-scale ranges (+/-2 G, +/-8 G, +/-12 G, +/-30 G)
 - Configurable output data rate (10, 50, 100, 200 Hz)
 - Configurable oversampling ratio (1, 2, 4, 8)
 - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
   attribute
 - Optional vdd-supply regulator management
 - Runtime PM with a 2 s autosuspend delay
 - System suspend/resume with full chip reinitialisation and regmap
   cache resync to handle regulator power-loss

Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
bit fields are accessed via regmap_field to avoid read-modify-write
races. The STATUS register is marked precious so regmap never reads
it speculatively and clears the DRDY/OVFL bits unexpectedly.

The probe-time init sequence is: soft reset, wait 1 ms, deassert
reset, configure SET/RESET control, apply default ODR/OSR/DSR/RNG,
enter normal mode. This ordering was determined empirically on
hardware to produce reliable, non-zero axis readings.

Cleanup is fully devm-managed: devm_pm_runtime_enable() handles
runtime PM teardown, and a devm action registered before
devm_iio_device_register() puts the chip to sleep on removal,
ensuring the IIO interface is unregistered before the hardware is
suspended.

The driver is placed under drivers/staging/iio/magnetometer/ with a
TODO file tracking the remaining work before it can graduate:
 - Triggered buffer support (iio_triggered_buffer_setup)
 - DRDY interrupt support
 - Self-test implementation

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 drivers/staging/iio/Kconfig                 |   1 +
 drivers/staging/iio/Makefile                |   1 +
 drivers/staging/iio/magnetometer/Kconfig    |  20 +
 drivers/staging/iio/magnetometer/Makefile   |   7 +
 drivers/staging/iio/magnetometer/TODO       |   5 +
 drivers/staging/iio/magnetometer/qmc5883p.c | 830 ++++++++++++++++++++
 6 files changed, 864 insertions(+)
 create mode 100644 drivers/staging/iio/magnetometer/Kconfig
 create mode 100644 drivers/staging/iio/magnetometer/Makefile
 create mode 100644 drivers/staging/iio/magnetometer/TODO
 create mode 100644 drivers/staging/iio/magnetometer/qmc5883p.c

diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
index a60631c1f449..d363e163d248 100644
--- a/drivers/staging/iio/Kconfig
+++ b/drivers/staging/iio/Kconfig
@@ -10,5 +10,6 @@ source "drivers/staging/iio/adc/Kconfig"
 source "drivers/staging/iio/addac/Kconfig"
 source "drivers/staging/iio/frequency/Kconfig"
 source "drivers/staging/iio/impedance-analyzer/Kconfig"
+source "drivers/staging/iio/magnetometer/Kconfig"
 
 endmenu
diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
index 628583535393..7dcbb75d43f0 100644
--- a/drivers/staging/iio/Makefile
+++ b/drivers/staging/iio/Makefile
@@ -8,3 +8,4 @@ obj-y += adc/
 obj-y += addac/
 obj-y += frequency/
 obj-y += impedance-analyzer/
+obj-y += magnetometer/
diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
new file mode 100644
index 000000000000..d631da9578a1
--- /dev/null
+++ b/drivers/staging/iio/magnetometer/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Magnetometer sensors
+#
+# When adding new entries keep the list in alphabetical order
+
+menu "Magnetometer sensors"
+
+config QMC5883P
+	tristate "QMC5883P 3-Axis Magnetometer"
+	depends on I2C
+	select REGMAP_I2C
+	help
+	  Say yes here to build support for QMC5883P I2C-based
+	  3-axis magnetometer chip.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called qmc5883p.
+
+endmenu
diff --git a/drivers/staging/iio/magnetometer/Makefile b/drivers/staging/iio/magnetometer/Makefile
new file mode 100644
index 000000000000..8e650f2e3b02
--- /dev/null
+++ b/drivers/staging/iio/magnetometer/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Makefile for staging industrial I/O Magnetometer sensor devices
+#
+# When adding new entries keep the list in alphabetical order
+
+obj-$(CONFIG_QMC5883P)	+= qmc5883p.o
diff --git a/drivers/staging/iio/magnetometer/TODO b/drivers/staging/iio/magnetometer/TODO
new file mode 100644
index 000000000000..6a8084c0dded
--- /dev/null
+++ b/drivers/staging/iio/magnetometer/TODO
@@ -0,0 +1,5 @@
+TODO
+====
+- Implement triggered buffer support (iio_triggered_buffer_setup)
+- Add interrupt (DRDY) support
+- Implement self-test (selftest regmap field is unused)
diff --git a/drivers/staging/iio/magnetometer/qmc5883p.c b/drivers/staging/iio/magnetometer/qmc5883p.c
new file mode 100644
index 000000000000..d9758f1e0f4d
--- /dev/null
+++ b/drivers/staging/iio/magnetometer/qmc5883p.c
@@ -0,0 +1,830 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * qmc5883p.c - QMC5883P magnetometer driver
+ *
+ * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
+ */
+
+#include <linux/array_size.h>
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/types.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/unaligned.h>
+
+/* Register definition */
+#define QMC5883P_REG_CHIP_ID 0x00
+#define QMC5883P_REG_X_LSB 0x01
+#define QMC5883P_REG_X_MSB 0x02
+#define QMC5883P_REG_Y_LSB 0x03
+#define QMC5883P_REG_Y_MSB 0x04
+#define QMC5883P_REG_Z_LSB 0x05
+#define QMC5883P_REG_Z_MSB 0x06
+#define QMC5883P_REG_STATUS 0x09
+#define QMC5883P_REG_CTRL_1 0x0A
+#define QMC5883P_REG_CTRL_2 0x0B
+
+/* Value definition */
+#define QMC5883P_MODE_SUSPEND 0x00
+#define QMC5883P_MODE_NORMAL 0x01
+#define QMC5883P_MODE_SINGLE 0x02
+#define QMC5883P_MODE_CONTINUOUS 0x03
+
+/* Output data rate */
+#define QMC5883P_ODR_10 0x00
+#define QMC5883P_ODR_50 0x01
+#define QMC5883P_ODR_100 0x02
+#define QMC5883P_ODR_200 0x03
+
+/* Oversampling rate */
+#define QMC5883P_OSR_8 0x00
+#define QMC5883P_OSR_4 0x01
+#define QMC5883P_OSR_2 0x02
+#define QMC5883P_OSR_1 0x03
+
+/* Downsampling rate */
+#define QMC5883P_DSR_1 0x00
+#define QMC5883P_DSR_2 0x01
+#define QMC5883P_DSR_4 0x02
+#define QMC5883P_DSR_8 0x03
+
+#define QMC5883P_RSTCTRL_SET_RESET \
+	0x00 /* Set and reset on, i.e. the offset of device is renewed */
+#define QMC5883P_RSTCTRL_SET_ONLY 0x01 /* Set only on */
+#define QMC5883P_RSTCTRL_OFF 0x02 /* Set and reset off */
+
+#define QMC5883P_RNG_30G 0x00
+#define QMC5883P_RNG_12G 0x01
+#define QMC5883P_RNG_08G 0x02
+#define QMC5883P_RNG_02G 0x03
+
+#define QMC5883P_DEFAULT_ODR QMC5883P_ODR_100
+#define QMC5883P_DEFAULT_OSR QMC5883P_OSR_4
+#define QMC5883P_DEFAULT_DSR QMC5883P_DSR_4
+#define QMC5883P_DEFAULT_RNG QMC5883P_RNG_08G
+
+#define QMC5883P_DRDY_POLL_US 1000
+
+#define QMC5883P_CHIP_ID 0x80
+
+#define QMC5883P_STATUS_DRDY BIT(0)
+#define QMC5883P_STATUS_OVFL BIT(1)
+
+/*
+ * Scale factors in T/LSB for IIO_VAL_FRACTIONAL (val/val2), derived from
+ * datasheet Table 2 sensitivities (LSB/G) converted to LSB/T (1 G = 1e-4 T):
+ *   sensitivity_T = sensitivity_G * 10000
+ *   scale = 1 / sensitivity_T
+ *
+ * Index matches register value: RNG<1:0> = 0b00..0b11
+ */
+static const int qmc5883p_scale[][2] = {
+	[QMC5883P_RNG_30G] = { 1, 10000000 },
+	[QMC5883P_RNG_12G] = { 1, 25000000 },
+	[QMC5883P_RNG_08G] = { 1, 37500000 },
+	[QMC5883P_RNG_02G] = { 1, 150000000 },
+};
+
+static const int qmc5883p_odr[] = {
+	[QMC5883P_ODR_10] = 10,
+	[QMC5883P_ODR_50] = 50,
+	[QMC5883P_ODR_100] = 100,
+	[QMC5883P_ODR_200] = 200,
+};
+
+static const int qmc5883p_osr[] = {
+	[QMC5883P_OSR_1] = 1,
+	[QMC5883P_OSR_2] = 2,
+	[QMC5883P_OSR_4] = 4,
+	[QMC5883P_OSR_8] = 8,
+};
+
+static const unsigned int qmc5883p_dsr[] = {
+	[QMC5883P_DSR_1] = 1,
+	[QMC5883P_DSR_2] = 2,
+	[QMC5883P_DSR_4] = 4,
+	[QMC5883P_DSR_8] = 8,
+};
+
+struct qmc5883p_rf {
+	struct regmap_field *osr;
+	struct regmap_field *dsr;
+	struct regmap_field *odr;
+	struct regmap_field *mode;
+	struct regmap_field *rng;
+	struct regmap_field *rstctrl;
+	struct regmap_field *sftrst;
+	struct regmap_field *selftest;
+	struct regmap_field *chip_id;
+};
+
+static const struct regmap_range qmc5883p_readable_ranges[] = {
+	regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_STATUS),
+	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
+};
+
+static const struct regmap_range qmc5883p_writable_ranges[] = {
+	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
+};
+
+/*
+ * Volatile registers: hardware updates these independently of the driver.
+ * regmap will never serve these from cache.
+ */
+static const struct regmap_range qmc5883p_volatile_ranges[] = {
+	regmap_reg_range(QMC5883P_REG_X_LSB, QMC5883P_REG_Z_MSB),
+	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
+};
+
+/*
+ * Precious registers: reading has a side effect (clears DRDY/OVFL bits).
+ * regmap will never read these speculatively.
+ */
+static const struct regmap_range qmc5883p_precious_ranges[] = {
+	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
+};
+
+static const struct regmap_access_table qmc5883p_readable_table = {
+	.yes_ranges = qmc5883p_readable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(qmc5883p_readable_ranges),
+};
+
+static const struct regmap_access_table qmc5883p_writable_table = {
+	.yes_ranges = qmc5883p_writable_ranges,
+	.n_yes_ranges = ARRAY_SIZE(qmc5883p_writable_ranges),
+};
+
+static const struct regmap_access_table qmc5883p_volatile_table = {
+	.yes_ranges = qmc5883p_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(qmc5883p_volatile_ranges),
+};
+
+static const struct regmap_access_table qmc5883p_precious_table = {
+	.yes_ranges = qmc5883p_precious_ranges,
+	.n_yes_ranges = ARRAY_SIZE(qmc5883p_precious_ranges),
+};
+
+static const struct regmap_config qmc5883p_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = 0x0B,
+	.cache_type = REGCACHE_RBTREE,
+	.rd_table = &qmc5883p_readable_table,
+	.wr_table = &qmc5883p_writable_table,
+	.volatile_table = &qmc5883p_volatile_table,
+	.precious_table = &qmc5883p_precious_table,
+};
+
+struct qmc5883p_data {
+	struct device *dev;
+	struct regmap *regmap;
+	struct mutex mutex; /* protects regmap and rf field accesses */
+	struct qmc5883p_rf rf;
+};
+
+enum qmc5883p_channels {
+	AXIS_X = 0,
+	AXIS_Y,
+	AXIS_Z,
+};
+
+static const struct reg_field qmc5883p_rf_osr =
+	REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
+static const struct reg_field qmc5883p_rf_dsr =
+	REG_FIELD(QMC5883P_REG_CTRL_1, 6, 7);
+static const struct reg_field qmc5883p_rf_odr =
+	REG_FIELD(QMC5883P_REG_CTRL_1, 2, 3);
+static const struct reg_field qmc5883p_rf_mode =
+	REG_FIELD(QMC5883P_REG_CTRL_1, 0, 1);
+static const struct reg_field qmc5883p_rf_rng =
+	REG_FIELD(QMC5883P_REG_CTRL_2, 2, 3);
+static const struct reg_field qmc5883p_rf_rstctrl =
+	REG_FIELD(QMC5883P_REG_CTRL_2, 0, 1);
+static const struct reg_field qmc5883p_rf_sftrst =
+	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
+static const struct reg_field qmc5883p_rf_selftest =
+	REG_FIELD(QMC5883P_REG_CTRL_2, 6, 6);
+static const struct reg_field qmc5883p_rf_chip_id =
+	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
+
+static int qmc5883p_rf_init(struct qmc5883p_data *data)
+{
+	struct regmap *regmap = data->regmap;
+	struct device *dev = data->dev;
+	struct qmc5883p_rf *rf = &data->rf;
+
+	rf->osr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_osr);
+	if (IS_ERR(rf->osr))
+		return PTR_ERR(rf->osr);
+
+	rf->dsr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_dsr);
+	if (IS_ERR(rf->dsr))
+		return PTR_ERR(rf->dsr);
+
+	rf->odr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_odr);
+	if (IS_ERR(rf->odr))
+		return PTR_ERR(rf->odr);
+
+	rf->mode = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_mode);
+	if (IS_ERR(rf->mode))
+		return PTR_ERR(rf->mode);
+
+	rf->rng = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rng);
+	if (IS_ERR(rf->rng))
+		return PTR_ERR(rf->rng);
+
+	rf->rstctrl = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rstctrl);
+	if (IS_ERR(rf->rstctrl))
+		return PTR_ERR(rf->rstctrl);
+
+	rf->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
+	if (IS_ERR(rf->sftrst))
+		return PTR_ERR(rf->sftrst);
+
+	rf->selftest =
+		devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_selftest);
+	if (IS_ERR(rf->selftest))
+		return PTR_ERR(rf->selftest);
+
+	rf->chip_id = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_chip_id);
+	if (IS_ERR(rf->chip_id))
+		return PTR_ERR(rf->chip_id);
+
+	return 0;
+}
+
+static int qmc5883p_verify_chip_id(struct qmc5883p_data *data)
+{
+	int ret, regval;
+
+	ret = regmap_field_read(data->rf.chip_id, &regval);
+	if (ret)
+		return dev_err_probe(data->dev, ret,
+				     "failed to read chip ID\n");
+
+	if (regval != QMC5883P_CHIP_ID)
+		return dev_err_probe(data->dev, -ENODEV,
+				     "unexpected chip ID 0x%02x, expected 0x%02x\n",
+				     regval, QMC5883P_CHIP_ID);
+	return ret;
+}
+
+static int qmc5883p_chip_init(struct qmc5883p_data *data)
+{
+	int ret;
+
+	ret = regmap_field_write(data->rf.sftrst, 1);
+	if (ret)
+		return ret;
+
+	usleep_range(1000, 2000);
+
+	ret = regmap_field_write(data->rf.sftrst, 0);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.rstctrl, QMC5883P_RSTCTRL_SET_RESET);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.rng, QMC5883P_DEFAULT_RNG);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.osr, QMC5883P_DEFAULT_OSR);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.dsr, QMC5883P_DEFAULT_DSR);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.odr, QMC5883P_DEFAULT_ODR);
+	if (ret)
+		return ret;
+
+	return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
+}
+
+/*
+ * qmc5883p_get_measure - read all three axes.
+ * Must be called with data->mutex held.
+ * Handles PM internally: resumes device, reads data, schedules autosuspend.
+ */
+static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
+				s16 *z)
+{
+	int ret;
+	u8 reg_data[6];
+	unsigned int status;
+
+	ret = pm_runtime_resume_and_get(data->dev);
+	if (ret < 0)
+		return ret;
+
+	/*
+	 * Poll the status register until DRDY is set or timeout.
+	 * Read the whole register in one shot so that OVFL is captured from
+	 * the same read: reading 0x09 clears both DRDY and OVFL, so a second
+	 * read would always see OVFL=0.
+	 * At ODR=10Hz one period is 100ms; use 150ms as a safe upper bound.
+	 */
+	ret = regmap_read_poll_timeout(data->regmap, QMC5883P_REG_STATUS,
+				       status, status & QMC5883P_STATUS_DRDY,
+				       QMC5883P_DRDY_POLL_US, 150000);
+	if (ret)
+		goto out;
+
+	if (status & QMC5883P_STATUS_OVFL) {
+		dev_warn_ratelimited(data->dev,
+				     "data overflow, consider reducing field range\n");
+		ret = -ERANGE;
+		goto out;
+	}
+
+	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
+			       ARRAY_SIZE(reg_data));
+	if (ret)
+		goto out;
+
+	*x = (s16)get_unaligned_le16(&reg_data[0]);
+	*y = (s16)get_unaligned_le16(&reg_data[2]);
+	*z = (s16)get_unaligned_le16(&reg_data[4]);
+
+out:
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+	return ret;
+}
+
+static int qmc5883p_write_scale(struct qmc5883p_data *data, int val, int val2)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(qmc5883p_scale); i++) {
+		if (qmc5883p_scale[i][0] == val && qmc5883p_scale[i][1] == val2)
+			return regmap_field_write(data->rf.rng, i);
+	}
+
+	return -EINVAL;
+}
+
+static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(qmc5883p_odr); i++) {
+		if (qmc5883p_odr[i] == val)
+			return regmap_field_write(data->rf.odr, i);
+	}
+
+	return -EINVAL;
+}
+
+static int qmc5883p_write_osr(struct qmc5883p_data *data, int val)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(qmc5883p_osr); i++) {
+		if (qmc5883p_osr[i] == val)
+			return regmap_field_write(data->rf.osr, i);
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t downsampling_ratio_show(struct device *dev,
+				       struct device_attribute *attr, char *buf)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	unsigned int regval;
+	int ret;
+
+	guard(mutex)(&data->mutex);
+
+	ret = regmap_field_read(data->rf.dsr, &regval);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", qmc5883p_dsr[regval]);
+}
+
+static ssize_t downsampling_ratio_store(struct device *dev,
+					struct device_attribute *attr,
+					const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	unsigned int val;
+	int i, ret, restore;
+
+	ret = kstrtouint(buf, 10, &val);
+	if (ret)
+		return ret;
+
+	guard(mutex)(&data->mutex);
+
+	ret = pm_runtime_resume_and_get(data->dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+	if (ret)
+		goto out;
+
+	ret = -EINVAL;
+	for (i = 0; i < ARRAY_SIZE(qmc5883p_dsr); i++) {
+		if (qmc5883p_dsr[i] == val) {
+			ret = regmap_field_write(data->rf.dsr, i);
+			break;
+		}
+	}
+
+	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
+	if (restore && !ret)
+		ret = restore;
+
+out:
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+	return ret ? ret : (ssize_t)len;
+}
+
+static int qmc5883p_read_raw(struct iio_dev *indio_dev,
+			     const struct iio_chan_spec *chan, int *val,
+			     int *val2, long mask)
+{
+	s16 x, y, z;
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	int ret;
+	unsigned int regval;
+
+	guard(mutex)(&data->mutex);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		ret = qmc5883p_get_measure(data, &x, &y, &z);
+		if (ret < 0)
+			return ret;
+		switch (chan->address) {
+		case AXIS_X:
+			*val = x;
+			break;
+		case AXIS_Y:
+			*val = y;
+			break;
+		case AXIS_Z:
+			*val = z;
+			break;
+		}
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_SCALE:
+		ret = regmap_field_read(data->rf.rng, &regval);
+		if (ret < 0)
+			return ret;
+		*val = qmc5883p_scale[regval][0];
+		*val2 = qmc5883p_scale[regval][1];
+		return IIO_VAL_FRACTIONAL;
+
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = regmap_field_read(data->rf.odr, &regval);
+		if (ret < 0)
+			return ret;
+		*val = qmc5883p_odr[regval];
+		return IIO_VAL_INT;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = regmap_field_read(data->rf.osr, &regval);
+		if (ret < 0)
+			return ret;
+		*val = qmc5883p_osr[regval];
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static int qmc5883p_write_raw(struct iio_dev *indio_dev,
+			      struct iio_chan_spec const *chan, int val,
+			      int val2, long mask)
+{
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	int ret, restore;
+
+	guard(mutex)(&data->mutex);
+
+	ret = pm_runtime_resume_and_get(data->dev);
+	if (ret)
+		return ret;
+
+	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+	if (ret)
+		goto out;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = qmc5883p_write_odr(data, val);
+		break;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		ret = qmc5883p_write_osr(data, val);
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		ret = qmc5883p_write_scale(data, val, val2);
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
+	if (restore && !ret)
+		ret = restore;
+
+out:
+	pm_runtime_mark_last_busy(data->dev);
+	pm_runtime_put_autosuspend(data->dev);
+	return ret;
+}
+
+/*
+ * qmc5883p_read_avail - expose available values to userspace.
+ *
+ * Creates the _available sysfs attributes automatically:
+ *   in_magn_sampling_frequency_available
+ *   in_magn_oversampling_ratio_available
+ *   in_magn_scale_available
+ */
+static int qmc5883p_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 = qmc5883p_odr;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(qmc5883p_odr);
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		*vals = qmc5883p_osr;
+		*type = IIO_VAL_INT;
+		*length = ARRAY_SIZE(qmc5883p_osr);
+		return IIO_AVAIL_LIST;
+
+	case IIO_CHAN_INFO_SCALE:
+		*vals = (const int *)qmc5883p_scale;
+		*type = IIO_VAL_FRACTIONAL;
+		*length = ARRAY_SIZE(qmc5883p_scale) * 2;
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
+		       downsampling_ratio_store, 0);
+static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");
+
+static struct attribute *qmc5883p_attributes[] = {
+	&iio_dev_attr_downsampling_ratio.dev_attr.attr,
+	&iio_const_attr_downsampling_ratio_available.dev_attr.attr, NULL
+};
+
+static const struct attribute_group qmc5883p_attribute_group = {
+	.attrs = qmc5883p_attributes,
+};
+
+static const struct iio_info qmc5883p_info = {
+	.attrs = &qmc5883p_attribute_group,
+	.read_raw = qmc5883p_read_raw,
+	.write_raw = qmc5883p_write_raw,
+	.read_avail = qmc5883p_read_avail,
+};
+
+static const struct iio_chan_spec qmc5883p_channels[] = {
+	{
+		.type = IIO_MAGN,
+		.channel2 = IIO_MOD_X,
+		.modified = 1,
+		.address = AXIS_X,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_type_available =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_MAGN,
+		.channel2 = IIO_MOD_Y,
+		.modified = 1,
+		.address = AXIS_Y,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_type_available =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+	{
+		.type = IIO_MAGN,
+		.channel2 = IIO_MOD_Z,
+		.modified = 1,
+		.address = AXIS_Z,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_type =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+		.info_mask_shared_by_type_available =
+			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+	},
+};
+
+static void qmc5883p_suspend_action(void *arg)
+{
+	struct qmc5883p_data *data = arg;
+
+	regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+}
+
+static int qmc5883p_probe(struct i2c_client *client)
+{
+	struct device *dev = &client->dev;
+	struct qmc5883p_data *data;
+	struct iio_dev *indio_dev;
+	struct regmap *regmap;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	regmap = devm_regmap_init_i2c(client, &qmc5883p_regmap_config);
+	if (IS_ERR(regmap))
+		return dev_err_probe(dev, PTR_ERR(regmap),
+				     "regmap initialization failed\n");
+
+	data = iio_priv(indio_dev);
+	data->dev = dev;
+	data->regmap = regmap;
+	mutex_init(&data->mutex);
+
+	ret = devm_regulator_get_enable_optional(dev, "vdd");
+	if (ret && ret != -ENODEV)
+		return dev_err_probe(dev, ret,
+				"failed to get vdd regulator\n");
+
+	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
+	fsleep(50000);
+
+	i2c_set_clientdata(client, indio_dev);
+
+	ret = qmc5883p_rf_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to initialize regmap fields\n");
+
+	ret = qmc5883p_verify_chip_id(data);
+	if (ret)
+		return ret;
+
+	ret = qmc5883p_chip_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to initialize chip\n");
+
+	indio_dev->name = "qmc5883p";
+	indio_dev->info = &qmc5883p_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->channels = qmc5883p_channels;
+	indio_dev->num_channels = ARRAY_SIZE(qmc5883p_channels);
+
+	pm_runtime_set_autosuspend_delay(dev, 2000);
+	pm_runtime_use_autosuspend(dev);
+
+	pm_runtime_set_active(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
+	pm_runtime_mark_last_busy(dev);
+
+	ret = devm_add_action_or_reset(dev, qmc5883p_suspend_action, data);
+	if (ret)
+		return ret;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to register IIO device\n");
+	return 0;
+}
+
+static int qmc5883p_runtime_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+
+	return regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+}
+
+static int qmc5883p_runtime_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
+	if (ret)
+		return ret;
+
+	usleep_range(10000, 11000);
+	return 0;
+}
+
+static int qmc5883p_system_suspend(struct device *dev)
+{
+	return pm_runtime_force_suspend(dev);
+}
+
+static int qmc5883p_system_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct qmc5883p_data *data = iio_priv(indio_dev);
+	int ret;
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * If the regulator was cut during system suspend, POR will have
+	 * reset all registers. Reinitialise the chip and resync the
+	 * regmap cache so that cached control register values are pushed
+	 * back to hardware.
+	 */
+	ret = qmc5883p_chip_init(data);
+	if (ret)
+		return ret;
+
+	regcache_mark_dirty(data->regmap);
+	return regcache_sync(data->regmap);
+}
+
+static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(qmc5883p_system_suspend, qmc5883p_system_resume)
+	RUNTIME_PM_OPS(qmc5883p_runtime_suspend, qmc5883p_runtime_resume, NULL)
+};
+
+static const struct of_device_id qmc5883p_of_match[] = {
+	{ .compatible = "qst,qmc5883p" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qmc5883p_of_match);
+
+static const struct i2c_device_id qmc5883p_id[] = {
+	{ "qmc5883p", 0 },
+	{},
+};
+MODULE_DEVICE_TABLE(i2c, qmc5883p_id);
+
+static struct i2c_driver qmc5883p_driver = {
+	.driver = {
+		.name = "qmc5883p",
+		.of_match_table = qmc5883p_of_match,
+		.pm = pm_ptr(&qmc5883p_dev_pm_ops),
+	},
+	.probe = qmc5883p_probe,
+	.id_table = qmc5883p_id,
+};
+module_i2c_driver(qmc5883p_driver);
+
+MODULE_AUTHOR("Hardik Phalet <hardik.phalet@pm.me>");
+MODULE_DESCRIPTION("QMC5883P magnetic sensor driver");
+MODULE_LICENSE("GPL");
-- 
2.53.0

[PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver

[Hardik Phalet]

Add a MAINTAINERS entry for the QST QMC5883P staging IIO driver,
covering the driver source and its device tree binding.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..d0b9bfceb283 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -20274,6 +20274,13 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
 F:	drivers/bus/fsl-mc/
 F:	include/uapi/linux/fsl_mc.h
 
+QST QMC5883P MAGNETOMETER DRIVER
+M:	Hardik Phalet <hardik.phalet@pm.me>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
+F:	drivers/staging/iio/magnetometer/
+
 QT1010 MEDIA DRIVER
 L:	linux-media@vger.kernel.org
 S:	Orphan
-- 
2.53.0

Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[Andy Shevchenko]

On Thu, Apr 09, 2026 at 09:07:11PM +0000, Hardik Phalet wrote:
> This series adds initial Linux support for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC that
> communicates over I2C. To my knowledge there is no existing upstream
> driver for this device.
> 
> The driver supports:
>   - Raw magnetic field readings on X, Y, and Z axes
>   - Four selectable full-scale ranges (±2 G, ±8 G, ±12 G, ±30 G)
>   - Configurable output data rate (10, 50, 100, 200 Hz)
>   - Configurable oversampling ratio (1, 2, 4, 8)
>   - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
>     attribute
>   - Runtime PM with a 2 s autosuspend delay
>   - System suspend/resume via pm_runtime_force_suspend/resume
> 
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
> 
> The init sequence on probe is: soft reset → wait 1 ms → deassert
> reset → configure SET/RESET control → apply default ODR/OSR/DSR/RNG
> → enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
> 
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
>   - Triggered buffer support (iio_triggered_buffer_setup)
>   - DRDY interrupt support
>   - Self-test implementation
> 
> Patches:
>   1/4 - dt-bindings: vendor-prefixes: Add 'qst' for QST Corporation
>   2/4 - dt-bindings: iio: magnetometer: Add binding for QST QMC5883P
>   3/4 - staging: iio: magnetometer: Add QST QMC5883P driver
>   4/4 - MAINTAINERS: Add entry for QST QMC5883P magnetometer driver
> 
> Testing
> -------
> Tested on a Raspberry Pi 4B running a mainline kernel (aarch64) with a
> GY-271 HM-246 board connected via I2C bus 1. The chip was confirmed to
> enumerate at address 0x2C via i2cdetect.
> 
> The driver was cross-compiled from Fedora (x86_64) targeting aarch64
> and loaded as a module (qmc5883p.ko) with the Device Tree overlay
> pointing at i2c1:0x2c.
> 
> Verification steps performed:
>   - Chip ID register (0x00) reads back 0x80 on probe, confirming the
>     correct device is present
>   - All three axes (in_magn_x_raw, in_magn_y_raw, in_magn_z_raw) return
>     non-zero, stable values when the board is held still and change
>     appropriately when the board is rotated
>   - in_magn_x_scale (and Y, Z) returns the expected fractional value for
>     the default ±8 G range (1/37500000)
>   - in_magn_sampling_frequency / _available, in_magn_oversampling_ratio /
>     _available, and downsampling_ratio / downsampling_ratio_available all
>     read and write correctly; the chip responds without error to each
>     valid setting
>   - Runtime PM: after 2 s of inactivity the device enters suspend mode
>     (MODE = 0x00 confirmed via i2cdump); the next sysfs read correctly
>     resumes the device and returns valid data
>   - System suspend/resume (echo mem > /sys/power/state) leaves the
>     driver in a consistent state; readings remain valid after resume
>   - dt_binding_check passes for patch 2/4
>   - Kernel builds cleanly with W=1 and no new warnings

This driver is rather huge. There are mistakes you made in the process, though:
- never send a new version for such a code (amount and complexity) earlier than
a week; give others a chance to review
- do not put driver to staging, why?
- the investigation is rather poor about existence of the driver — make sure
there is no compatible (by register layout) driver in IIO or even outside it
(for ADCs it might appear as HWMON [drivers/hwmon] or INPUT [drivers/input]
 in some cases)

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[Greg Kroah-Hartman]

On Thu, Apr 09, 2026 at 09:07:41PM +0000, Hardik Phalet wrote:
> Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
> magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
> over I2C. There is no existing upstream driver for this device.

Sorry, but no new iio drivers should be added to staging.  Take the time
to do it right and put it into drivers/iio/ from the beginning.
Otherwise this will just take more time and effort to get it into that
location in the end (i.e. doing it right is simpler/faster.)

thanks,

greg k-h

Re: [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation

[Krzysztof Kozlowski]

On Thu, Apr 09, 2026 at 09:07:20PM +0000, Hardik Phalet wrote:
> Add the vendor prefix 'qst' for QST Corporation, a manufacturer of
> MEMS sensors.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.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 5d2a7a8d3ac6..71a1b9087c5e 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -1244,6 +1244,8 @@ patternProperties:
>      description: Shenzhen QiShenglong Industrialist Co., Ltd.
>    "^qnap,.*":
>      description: QNAP Systems, Inc.
> +  "^qst,.*":

Website tells me qstcorp.com, so prefix is qstcorp. Unless it is
different company, but then just explain that in commit msg (e.g.
provide link to website).

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P

[Krzysztof Kozlowski]

On Thu, Apr 09, 2026 at 09:07:29PM +0000, Hardik Phalet wrote:
> Add the device tree binding document for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) sensor with a 16-bit ADC that
> communicates over I2C. The binding exposes the required 'compatible'
> and 'reg' properties along with an optional 'vdd-supply' for the
> 2.5 V–3.6 V VDD rail.

Drop last sentence. We can read the diff.

...

> +properties:
> +  compatible:
> +    const: qst,qmc5883p
> +
> +  reg:
> +    maxItems: 1
> +    description: I2C address of the device; the default address is 0x2c.
> +
> +  vdd-supply:
> +    description:
> +      VDD power supply (2.5 V to 3.6 V). Powers all internal analog and
> +      digital functional blocks.

Supply should be required. Devices need them to operate.

Best regards,
Krzysztof

Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P

[Krzysztof Kozlowski]

On Fri, Apr 10, 2026 at 09:55:24AM +0200, Krzysztof Kozlowski wrote:
> On Thu, Apr 09, 2026 at 09:07:29PM +0000, Hardik Phalet wrote:
> > Add the device tree binding document for the QST QMC5883P, a 3-axis
> > anisotropic magneto-resistive (AMR) sensor with a 16-bit ADC that
> > communicates over I2C. The binding exposes the required 'compatible'
> > and 'reg' properties along with an optional 'vdd-supply' for the
> > 2.5 V–3.6 V VDD rail.
> 
> Drop last sentence. We can read the diff.
> 
> ...
> 
> > +properties:
> > +  compatible:
> > +    const: qst,qmc5883p
> > +
> > +  reg:
> > +    maxItems: 1
> > +    description: I2C address of the device; the default address is 0x2c.
> > +
> > +  vdd-supply:
> > +    description:
> > +      VDD power supply (2.5 V to 3.6 V). Powers all internal analog and
> > +      digital functional blocks.
> 
> Supply should be required. Devices need them to operate.

Ah, and since I expect new version, also:

A nit, subject: drop second/last, redundant "binding for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

Best regards,
Krzysztof

Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[David Lechner]

On 4/9/26 4:07 PM, Hardik Phalet wrote:

For a series this be, please wait at least a week for more feedback
before submitting the next revision.

> This series adds initial Linux support for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC that
> communicates over I2C. To my knowledge there is no existing upstream
> driver for this device.
> 
> The driver supports:
>   - Raw magnetic field readings on X, Y, and Z axes
>   - Four selectable full-scale ranges (±2 G, ±8 G, ±12 G, ±30 G)
>   - Configurable output data rate (10, 50, 100, 200 Hz)
>   - Configurable oversampling ratio (1, 2, 4, 8)
>   - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs

What is the difference between oversampling and downsampling? I think
we have used some filter attribute for downsampling/decimation in some
other drivers so maybe that could be a good fit?

>     attribute
>   - Runtime PM with a 2 s autosuspend delay
>   - System suspend/resume via pm_runtime_force_suspend/resume
> 
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
> 
> The init sequence on probe is: soft reset → wait 1 ms → deassert
> reset → configure SET/RESET control → apply default ODR/OSR/DSR/RNG
> → enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
> 
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
>   - Triggered buffer support (iio_triggered_buffer_setup)
>   - DRDY interrupt support
>   - Self-test implementation

These are not reasons to have the driver in staging. It is fine
to have a driver that doesn't implement all functionality. We should
be able to add those features without breaking anything.

Re: [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver

[David Lechner]

On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add a MAINTAINERS entry for the QST QMC5883P staging IIO driver,
> covering the driver source and its device tree binding.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  MAINTAINERS | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa16..d0b9bfceb283 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -20274,6 +20274,13 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
>  F:	drivers/bus/fsl-mc/
>  F:	include/uapi/linux/fsl_mc.h
>  
> +QST QMC5883P MAGNETOMETER DRIVER
> +M:	Hardik Phalet <hardik.phalet@pm.me>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
> +F:	drivers/staging/iio/magnetometer/
> +
>  QT1010 MEDIA DRIVER
>  L:	linux-media@vger.kernel.org
>  S:	Orphan

This should be split up and added in the patches that actually
add the F: files. Most of it will go with the dt-bingings patch
and the one line added later with the driver.

Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[David Lechner]

On 4/9/26 4:07 PM, Hardik Phalet wrote:
> Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
> magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
> over I2C. There is no existing upstream driver for this device.
> 
> The driver supports:
>  - Raw magnetic field readings on X, Y, and Z axes
>  - Four full-scale ranges (+/-2 G, +/-8 G, +/-12 G, +/-30 G)
>  - Configurable output data rate (10, 50, 100, 200 Hz)
>  - Configurable oversampling ratio (1, 2, 4, 8)
>  - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
>    attribute
>  - Optional vdd-supply regulator management
>  - Runtime PM with a 2 s autosuspend delay
>  - System suspend/resume with full chip reinitialisation and regmap
>    cache resync to handle regulator power-loss

This is a little bit much to review all in one patch. Could be nice
to split out power management to a separate patch.

Oversampling/downsampling could be split to a separate patch or two
as well.

> 
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
> bit fields are accessed via regmap_field to avoid read-modify-write
> races. The STATUS register is marked precious so regmap never reads
> it speculatively and clears the DRDY/OVFL bits unexpectedly.
> 
> 
> The probe-time init sequence is: soft reset, wait 1 ms, deassert
> reset, configure SET/RESET control, apply default ODR/OSR/DSR/RNG,
> enter normal mode. This ordering was determined empirically on
> hardware to produce reliable, non-zero axis readings.
> 
> Cleanup is fully devm-managed: devm_pm_runtime_enable() handles
> runtime PM teardown, and a devm action registered before
> devm_iio_device_register() puts the chip to sleep on removal,
> ensuring the IIO interface is unregistered before the hardware is
> suspended.
> 
> The driver is placed under drivers/staging/iio/magnetometer/ with a
> TODO file tracking the remaining work before it can graduate:
>  - Triggered buffer support (iio_triggered_buffer_setup)
>  - DRDY interrupt support
>  - Self-test implementation
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  drivers/staging/iio/Kconfig                 |   1 +
>  drivers/staging/iio/Makefile                |   1 +
>  drivers/staging/iio/magnetometer/Kconfig    |  20 +
>  drivers/st
> aging/iio/magnetometer/Makefile   |   7 +
>  drivers/staging/iio/magnetometer/TODO       |   5 +
>  drivers/staging/iio/magnetometer/qmc5883p.c | 830 ++++++++++++++++++++
>  6 files changed, 864 insertions(+)
>  create mode 100644 drivers/staging/iio/magnetometer/Kconfig
>  create mode 100644 drivers/staging/iio/magnetometer/Makefile
>  create mode 100644 drivers/staging/iio/magnetometer/TODO
>  create mode 100644 drivers/staging/iio/magnetometer/qmc5883p.c
> 
> diff --git a/drivers/staging/iio/Kconfig b/drivers/staging/iio/Kconfig
> index a60631c1f449..d363e163d248 100644
> --- a/drivers/staging/iio/Kconfig
> +++ b/drivers/staging/iio/Kconfig
> @@ -10,5 +10,6 @@ source "drivers/staging/iio/adc/Kconfig"
>  source "drivers/staging/iio/addac/Kconfig"
>  source "drivers/staging/iio/frequency/Kconfig"
>  source "drivers/staging/iio/impedance-analyzer/Kconfig"
> +source "drivers/staging/iio/magnetometer/Kconfig"
>  
>  endmenu
> diff --git a/drivers/staging/iio/Makefile b/drivers/staging/iio/Makefile
> index 62
> 8583535393..7dcbb75d43f0 100644
> --- a/drivers/staging/iio/Makefile
> +++ b/drivers/staging/iio/Makefile
> @@ -8,3 +8,4 @@ obj-y += adc/
>  obj-y += addac/
>  obj-y += frequency/
>  obj-y += impedance-analyzer/
> +obj-y += magnetometer/
> diff --git a/drivers/staging/iio/magnetometer/Kconfig b/drivers/staging/iio/magnetometer/Kconfig
> new file mode 100644
> index 000000000000..d631da9578a1
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Kconfig
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Magnetometer sensors
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Magnetometer sensors"
> +
> +config QMC5883P
> +	tristate "QMC5883P 3-Axis Magnetometer"
> +	depends on I2C
> +	select REGMAP_I2C
> +	help
> +	  Say yes here to build support for QMC5883P I2C-based
> +	  3-axis magnetometer chip.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called qmc5883p.
> +
> +endmenu
> diff --git a/drivers/staging/iio/magnetometer/Ma
> kefile b/drivers/staging/iio/magnetometer/Makefile
> new file mode 100644
> index 000000000000..8e650f2e3b02
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/Makefile
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Makefile for staging industrial I/O Magnetometer sensor devices
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +obj-$(CONFIG_QMC5883P)	+= qmc5883p.o
> diff --git a/drivers/staging/iio/magnetometer/TODO b/drivers/staging/iio/magnetometer/TODO
> new file mode 100644
> index 000000000000..6a8084c0dded
> --- /dev/null
> +++ b/drivers/staging/iio/magnetometer/TODO
> @@ -0,0 +1,5 @@
> +TODO
> +====
> +- Implement triggered buffer support (iio_triggered_buffer_setup)
> +- Add interrupt (DRDY) support
> +- Implement self-test (selftest regmap field is unused)
> diff --git a/drivers/staging/iio/magnetometer/qmc5883p.c b/drivers/staging/iio/magnetometer/qmc5883p.c
> new file mode 100644
> index 000000000000..d9758f1e0f4d
> --- /dev/null
> +++ b/drivers/
> staging/iio/magnetometer/qmc5883p.c
> @@ -0,0 +1,830 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * qmc5883p.c - QMC5883P magnetometer driver
> + *
> + * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/types.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/unaligned.h>
> +
> +/* Register definition */
> +#define QMC5883P_REG_CHIP_ID 0x00
> +#define QMC5883P_REG_X_LSB 0x01
> +#define QMC5883P_REG_X_MSB 0x02
> +#define QMC5883P_REG_Y_LSB 0x03
> +#define QMC5883P_REG_Y_MSB 0x04
> +#define QMC58
> 83P_REG_Z_LSB 0x05

Were you manually editing the patch?

> +#define QMC5883P_REG_Z_MSB 0x06
> +#define QMC5883P_REG_STATUS 0x09
> +#define QMC5883P_REG_CTRL_1 0x0A
> +#define QMC5883P_REG_CTRL_2 0x0B
> +
> +/* Value definition */
> +#define QMC5883P_MODE_SUSPEND 0x00
> +#define QMC5883P_MODE_NORMAL 0x01
> +#define QMC5883P_MODE_SINGLE 0x02
> +#define QMC5883P_MODE_CONTINUOUS 0x03
> +
> +/* Output data rate */
> +#define QMC5883P_ODR_10 0x00
> +#define QMC5883P_ODR_50 0x01
> +#define QMC5883P_ODR_100 0x02
> +#define QMC5883P_ODR_200 0x03
> +
> +/* Oversampling rate */
> +#define QMC5883P_OSR_8 0x00
> +#define QMC5883P_OSR_4 0x01
> +#define QMC5883P_OSR_2 0x02
> +#define QMC5883P_OSR_1 0x03
> +
> +/* Downsampling rate */
> +#define QMC5883P_DSR_1 0x00
> +#define QMC5883P_DSR_2 0x01
> +#define QMC5883P_DSR_4 0x02
> +#define QMC5883P_DSR_8 0x03
> +
> +#define QMC5883P_RSTCTRL_SET_RESET \
> +	0x00 /* Set and reset on, i.e. the offset of device is renewed */
> +#define QMC5883P_RSTCTRL_SET_ONLY 0x01 /* Set only on */
> +#define QMC5883P_RSTCTRL_OFF 0x02 /* Set 
> and reset off */

Or maybe you mail client mangled the patch? These wraps are
happening in many places.

> +
> +#define QMC5883P_RNG_30G 0x00
> +#define QMC5883P_RNG_12G 0x01
> +#define QMC5883P_RNG_08G 0x02
> +#define QMC5883P_RNG_02G 0x03
> +
> +#define QMC5883P_DEFAULT_ODR QMC5883P_ODR_100
> +#define QMC5883P_DEFAULT_OSR QMC5883P_OSR_4
> +#define QMC5883P_DEFAULT_DSR QMC5883P_DSR_4
> +#define QMC5883P_DEFAULT_RNG QMC5883P_RNG_08G
> +
> +#define QMC5883P_DRDY_POLL_US 1000
> +
> +#define QMC5883P_CHIP_ID 0x80
> +
> +#define QMC5883P_STATUS_DRDY BIT(0)
> +#define QMC5883P_STATUS_OVFL BIT(1)
> +
> +/*
> + * Scale factors in T/LSB for IIO_VAL_FRACTIONAL (val/val2), derived from
> + * datasheet Table 2 sensitivities (LSB/G) converted to LSB/T (1 G = 1e-4 T):
> + *   sensitivity_T = sensitivity_G * 10000
> + *   scale = 1 / sensitivity_T
> + *
> + * Index matches register value: RNG<1:0> = 0b00..0b11
> + */
> +static const int qmc5883p_scale[][2] = {
> +	[QMC5883P_RNG_30G] = { 1, 10000000 },
> +	[QMC5883P_RNG_12G] = { 1, 25000000 },
> +	[QMC5883P_RNG_08G] = { 1, 37500000 },
> +	[QMC5883P_RNG_02G] = { 1, 15
> 0000000 },
> +};
> +
> +static const int qmc5883p_odr[] = {
> +	[QMC5883P_ODR_10] = 10,
> +	[QMC5883P_ODR_50] = 50,
> +	[QMC5883P_ODR_100] = 100,
> +	[QMC5883P_ODR_200] = 200,
> +};
> +
> +static const int qmc5883p_osr[] = {
> +	[QMC5883P_OSR_1] = 1,
> +	[QMC5883P_OSR_2] = 2,
> +	[QMC5883P_OSR_4] = 4,
> +	[QMC5883P_OSR_8] = 8,
> +};
> +
> +static const unsigned int qmc5883p_dsr[] = {
> +	[QMC5883P_DSR_1] = 1,
> +	[QMC5883P_DSR_2] = 2,
> +	[QMC5883P_DSR_4] = 4,
> +	[QMC5883P_DSR_8] = 8,
> +};
> +
> +struct qmc5883p_rf {
> +	struct regmap_field *osr;
> +	struct regmap_field *dsr;
> +	struct regmap_field *odr;
> +	struct regmap_field *mode;
> +	struct regmap_field *rng;
> +	struct regmap_field *rstctrl;
> +	struct regmap_field *sftrst;
> +	struct regmap_field *selftest;
> +	struct regmap_field *chip_id;
> +};
> +
> +static const struct regmap_range qmc5883p_readable_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_STATUS),
> +	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +static con
> st struct regmap_range qmc5883p_writable_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_CTRL_1, QMC5883P_REG_CTRL_2),
> +};
> +
> +/*
> + * Volatile registers: hardware updates these independently of the driver.
> + * regmap will never serve these from cache.
> + */
> +static const struct regmap_range qmc5883p_volatile_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_X_LSB, QMC5883P_REG_Z_MSB),
> +	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +/*
> + * Precious registers: reading has a side effect (clears DRDY/OVFL bits).
> + * regmap will never read these speculatively.
> + */
> +static const struct regmap_range qmc5883p_precious_ranges[] = {
> +	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_STATUS),
> +};
> +
> +static const struct regmap_access_table qmc5883p_readable_table = {
> +	.yes_ranges = qmc5883p_readable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_readable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_writable_table = {
> +	.yes_ranges = qmc5
> 883p_writable_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_writable_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_volatile_table = {
> +	.yes_ranges = qmc5883p_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_volatile_ranges),
> +};
> +
> +static const struct regmap_access_table qmc5883p_precious_table = {
> +	.yes_ranges = qmc5883p_precious_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(qmc5883p_precious_ranges),
> +};
> +
> +static const struct regmap_config qmc5883p_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0x0B,
> +	.cache_type = REGCACHE_RBTREE,
> +	.rd_table = &qmc5883p_readable_table,
> +	.wr_table = &qmc5883p_writable_table,
> +	.volatile_table = &qmc5883p_volatile_table,
> +	.precious_table = &qmc5883p_precious_table,
> +};
> +
> +struct qmc5883p_data {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct mutex mutex; /* protects regmap and rf field accesses */
> +	struct qmc5883p_rf rf;
> +};
> +
> +enum qmc5883p_channels {
> +	AXIS_X = 0
> ,
> +	AXIS_Y,
> +	AXIS_Z,
> +};
> +
> +static const struct reg_field qmc5883p_rf_osr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
> +static const struct reg_field qmc5883p_rf_dsr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 6, 7);
> +static const struct reg_field qmc5883p_rf_odr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 2, 3);
> +static const struct reg_field qmc5883p_rf_mode =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 0, 1);
> +static const struct reg_field qmc5883p_rf_rng =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 2, 3);
> +static const struct reg_field qmc5883p_rf_rstctrl =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 0, 1);
> +static const struct reg_field qmc5883p_rf_sftrst =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
> +static const struct reg_field qmc5883p_rf_selftest =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 6, 6);
> +static const struct reg_field qmc5883p_rf_chip_id =
> +	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
> +
> +static int qmc5883p_rf_init(struct qmc5883p_data *data)
> +{
> +	struct regmap *regmap = data->regmap;
> +	struct device *dev = d
> ata->dev;
> +	struct qmc5883p_rf *rf = &data->rf;
> +
> +	rf->osr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_osr);
> +	if (IS_ERR(rf->osr))
> +		return PTR_ERR(rf->osr);
> +
> +	rf->dsr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_dsr);
> +	if (IS_ERR(rf->dsr))
> +		return PTR_ERR(rf->dsr);
> +
> +	rf->odr = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_odr);
> +	if (IS_ERR(rf->odr))
> +		return PTR_ERR(rf->odr);
> +
> +	rf->mode = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_mode);
> +	if (IS_ERR(rf->mode))
> +		return PTR_ERR(rf->mode);
> +
> +	rf->rng = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rng);
> +	if (IS_ERR(rf->rng))
> +		return PTR_ERR(rf->rng);
> +
> +	rf->rstctrl = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_rstctrl);
> +	if (IS_ERR(rf->rstctrl))
> +		return PTR_ERR(rf->rstctrl);
> +
> +	rf->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
> +	if (IS_ERR(rf->sftrst))
> +		return PTR_ERR(rf->sftrst);
> +
> +	rf->selftest =
> +		devm_regmap_field_alloc(de
> v, regmap, qmc5883p_rf_selftest);
> +	if (IS_ERR(rf->selftest))
> +		return PTR_ERR(rf->selftest);
> +
> +	rf->chip_id = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_chip_id);
> +	if (IS_ERR(rf->chip_id))
> +		return PTR_ERR(rf->chip_id);
> +
> +	return 0;
> +}
> +
> +static int qmc5883p_verify_chip_id(struct qmc5883p_data *data)
> +{
> +	int ret, regval;
> +
> +	ret = regmap_field_read(data->rf.chip_id, &regval);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "failed to read chip ID\n");
> +
> +	if (regval != QMC5883P_CHIP_ID)
> +		return dev_err_probe(data->dev, -ENODEV,

We don't consider ID match an error. It has happened too many times
that there is a compatible part with a different ID. This can just
be dev_info() and return success.

> +				     "unexpected chip ID 0x%02x, expected 0x%02x\n",
> +				     regval, QMC5883P_CHIP_ID);
> +	return ret;
> +}
> +
> +static int qmc5883p_chip_init(struct qmc5883p_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_field_write(data->rf.sftrst, 1);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(1000, 2000);

Use fsleep() instead and add a comment explaining why this specific duration
was selected.

> +
> +	ret = regmap_field_write(data->rf.sftrst, 0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write
> (data->rf.rstctrl, QMC5883P_RSTCTRL_SET_RESET);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.rng, QMC5883P_DEFAULT_RNG);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.osr, QMC5883P_DEFAULT_OSR);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.dsr, QMC5883P_DEFAULT_DSR);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.odr, QMC5883P_DEFAULT_ODR);
> +	if (ret)
> +		return ret;

Since we just reset the chip, why do we need to set everything to a
new default instead of using the chip's default? If there is a good
reason, add a comment, otherwise we can leave this out.

> +
> +	return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);

Does this start sampling? Seems like it could be out of place here.

> +}
> +
> +/*
> + * qmc5883p_get_measure - read all three axes.
> + * Must be called with data->mutex held.
> + * Handles PM internally: resumes device, reads data, schedules autosuspend.
> + */
> +static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
> +				s16 *z)
> +{
> +	int ret;
> +	u8 reg_data[6];
> +	unsigned int status;
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Poll the status register until DR
> DY is set or timeout.
> +	 * Read the whole register in one shot so that OVFL is captured from
> +	 * the same read: reading 0x09 clears both DRDY and OVFL, so a second
> +	 * read would always see OVFL=0.
> +	 * At ODR=10Hz one period is 100ms; use 150ms as a safe upper bound.
> +	 */
> +	ret = regmap_read_poll_timeout(data->regmap, QMC5883P_REG_STATUS,
> +				       status, status & QMC5883P_STATUS_DRDY,
> +				       QMC5883P_DRDY_POLL_US, 150000);

Numbers with lots of 0s are easier to read as 150 * (MICRO / MILLI).

> +	if (ret)
> +		goto out;
> +
> +	if (status & QMC5883P_STATUS_OVFL) {
> +		dev_warn_ratelimited(data->dev,
> +				     "data overflow, consider reducing field range\n");
> +		ret = -ERANGE;
> +		goto out;
> +	}
> +
> +	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
> +			       ARRAY_SIZE(reg_data));
> +	if (ret)
> +		goto out;
> +
> +	*x = (s16)get_unaligned_le16(&reg_data[0]);
> +	*y = (s16)get_unaligned_le16(&reg_data[2]);
> +	*z = (s16)get_unaligned_le16(&reg_data[4]);
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_
> autosuspend(data->dev);
> +	return ret;
> +}
> +
> +static int qmc5883p_write_scale(struct qmc5883p_data *data, int val, int val2)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_scale); i++) {
> +		if (qmc5883p_scale[i][0] == val && qmc5883p_scale[i][1] == val2)
> +			return regmap_field_write(data->rf.rng, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_odr); i++) {
> +		if (qmc5883p_odr[i] == val)
> +			return regmap_field_write(data->rf.odr, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_osr(struct qmc5883p_data *data, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_osr); i++) {
> +		if (qmc5883p_osr[i] == val)
> +			return regmap_field_write(data->rf.osr, i);
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static ssize_t downsampling_ratio_show(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct iio_dev *indio
> _dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	unsigned int regval;
> +	int ret;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = regmap_field_read(data->rf.dsr, &regval);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", qmc5883p_dsr[regval]);
> +}
> +
> +static ssize_t downsampling_ratio_store(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	unsigned int val;
> +	int i, ret, restore;
> +
> +	ret = kstrtouint(buf, 10, &val);
> +	if (ret)
> +		return ret;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		goto out;
> +
> +	ret = -EINVAL;
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_dsr); i++) {
> +		if (qmc5883p_dsr[i] == val) {
> +			ret = regmap_field_wr
> ite(data->rf.dsr, i);
> +			break;
> +		}
> +	}
> +
> +	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (restore && !ret)
> +		ret = restore;
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_autosuspend(data->dev);
> +	return ret ? ret : (ssize_t)len;
> +}
> +
> +static int qmc5883p_read_raw(struct iio_dev *indio_dev,
> +			     const struct iio_chan_spec *chan, int *val,
> +			     int *val2, long mask)
> +{
> +	s16 x, y, z;
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +	unsigned int regval;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = qmc5883p_get_measure(data, &x, &y, &z);
> +		if (ret < 0)
> +			return ret;
> +		switch (chan->address) {
> +		case AXIS_X:
> +			*val = x;
> +			break;
> +		case AXIS_Y:
> +			*val = y;
> +			break;
> +		case AXIS_Z:
> +			*val = z;
> +			break;
> +		}
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = regmap_field_read(data->rf.rng, &regval);
> +		if (
> ret < 0)
> +			return ret;
> +		*val = qmc5883p_scale[regval][0];
> +		*val2 = qmc5883p_scale[regval][1];
> +		return IIO_VAL_FRACTIONAL;
> +
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = regmap_field_read(data->rf.odr, &regval);
> +		if (ret < 0)
> +			return ret;
> +		*val = qmc5883p_odr[regval];
> +		return IIO_VAL_INT;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		ret = regmap_field_read(data->rf.osr, &regval);
> +		if (ret < 0)
> +			return ret;
> +		*val = qmc5883p_osr[regval];
> +		return IIO_VAL_INT;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int qmc5883p_write_raw(struct iio_dev *indio_dev,
> +			      struct iio_chan_spec const *chan, int val,
> +			      int val2, long mask)
> +{
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret, restore;
> +
> +	guard(mutex)(&data->mutex);
> +
> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		goto out;
> +
> +	switch (mask) {
> +	case IIO_
> CHAN_INFO_SAMP_FREQ:
> +		ret = qmc5883p_write_odr(data, val);
> +		break;
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		ret = qmc5883p_write_osr(data, val);
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = qmc5883p_write_scale(data, val, val2);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	restore = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (restore && !ret)
> +		ret = restore;
> +
> +out:
> +	pm_runtime_mark_last_busy(data->dev);
> +	pm_runtime_put_autosuspend(data->dev);
> +	return ret;
> +}
> +
> +/*
> + * qmc5883p_read_avail - expose available values to userspace.
> + *
> + * Creates the _available sysfs attributes automatically:
> + *   in_magn_sampling_frequency_available
> + *   in_magn_oversampling_ratio_available
> + *   in_magn_scale_available
> + */
> +static int qmc5883p_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 = qmc5883p_odr;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(qmc5883p_odr);
> +		return IIO_AVAIL_LIST;
> +
> +	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
> +		*vals = qmc5883p_osr;
> +		*type = IIO_VAL_INT;
> +		*length = ARRAY_SIZE(qmc5883p_osr);
> +		return IIO_AVAIL_LIST;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		*vals = (const int *)qmc5883p_scale;
> +		*type = IIO_VAL_FRACTIONAL;
> +		*length = ARRAY_SIZE(qmc5883p_scale) * 2;
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
> +		       downsampling_ratio_store, 0);
> +static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");

As mentioned in the cover letter, we'd like to know more about what
this actually does. If there is a good reason it doesn't fit with
any existing filter attribute, then we'll need a patch to document
the sysfs ABI as well.

> +
> +static struct attribute *qmc5883p_attributes[] = {
> +	&iio_dev_attr_downsampling_ratio.dev_attr.attr,
> +	&iio_const_attr_downsampling_ratio_available.dev_attr.attr, NULL
> +};
> +
> +static const struct attribute_group qmc5883p_attribute_group = {
> +	.attrs = qmc5883p_attribu
> tes,
> +};
> +
> +static const struct iio_info qmc5883p_info = {
> +	.attrs = &qmc5883p_attribute_group,
> +	.read_raw = qmc5883p_read_raw,
> +	.write_raw = qmc5883p_write_raw,
> +	.read_avail = qmc5883p_read_avail,
> +};
> +
> +static const struct iio_chan_spec qmc5883p_channels[] = {
> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_X,
> +		.modified = 1,
> +		.address = AXIS_X,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},

Could save some dupilcation by making a macro that takes X/Y/Z
as parameter. e.g.

#define QMC5883P_CHAN(ch) \
	...				\
	.channel2 = IIO_MOD_##ch,	\
	...				\
	.address = AXIS_##ch,		\
	...

> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_Y,
> +		.modified = 1,
> +		.address = AXIS_Y,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_IN
> FO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +	{
> +		.type = IIO_MAGN,
> +		.channel2 = IIO_MOD_Z,
> +		.modified = 1,
> +		.address = AXIS_Z,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE),
> +		.info_mask_shared_by_type =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +		.info_mask_shared_by_type_available =
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
> +	},
> +};
> +
> +static void qmc5883p_suspend_action(void *arg)
> +{
> +	struct qmc5883p_data *data = arg;
> +
> +	regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_probe(struct i2c_client *client)
> +{
> +	struct device *dev = &client->d
> ev;
> +	struct qmc5883p_data *data;
> +	struct iio_dev *indio_dev;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	regmap = devm_regmap_init_i2c(client, &qmc5883p_regmap_config);
> +	if (IS_ERR(regmap))
> +		return dev_err_probe(dev, PTR_ERR(regmap),
> +				     "regmap initialization failed\n");
> +
> +	data = iio_priv(indio_dev);
> +	data->dev = dev;
> +	data->regmap = regmap;
> +	mutex_init(&data->mutex);
> +
> +	ret = devm_regulator_get_enable_optional(dev, "vdd");
> +	if (ret && ret != -ENODEV)
> +		return dev_err_probe(dev, ret,
> +				"failed to get vdd regulator\n");
> +
> +	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
> +	fsleep(50000);
> +
> +	i2c_set_clientdata(client, indio_dev);
> +
> +	ret = qmc5883p_rf_init(data);

Would be more logical to move this up right after regmap is declared.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to initialize regmap fields\n");
> +
> +	ret = qmc5883p_verify_chip_id(data);
> +	if (ret)
> +		retur
> n ret;
> +
> +	ret = qmc5883p_chip_init(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize chip\n");
> +
> +	indio_dev->name = "qmc5883p";
> +	indio_dev->info = &qmc5883p_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = qmc5883p_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(qmc5883p_channels);
> +
> +	pm_runtime_set_autosuspend_delay(dev, 2000);
> +	pm_runtime_use_autosuspend(dev);
> +
> +	pm_runtime_set_active(dev);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;
> +
> +	pm_runtime_mark_last_busy(dev);
> +
> +	ret = devm_add_action_or_reset(dev, qmc5883p_suspend_action, data);
> +	if (ret)
> +		return ret;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);

Usually, we just return directly here. This pretty much doesn't ever fail.

> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to register IIO device\n");
> +	return 0;
> +}
> +
> +static int qmc5883p_runtime_suspend(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indi
> o_dev);
> +
> +	return regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +}
> +
> +static int qmc5883p_runtime_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
> +	if (ret)
> +		return ret;
> +
> +	usleep_range(10000, 11000);

Again, fsleep() and comment.

> +	return 0;
> +}
> +
> +static int qmc5883p_system_suspend(struct device *dev)
> +{
> +	return pm_runtime_force_suspend(dev);
> +}
> +
> +static int qmc5883p_system_resume(struct device *dev)
> +{
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +	struct qmc5883p_data *data = iio_priv(indio_dev);
> +	int ret;
> +
> +	ret = pm_runtime_force_resume(dev);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If the regulator was cut during system suspend, POR will have
> +	 * reset all registers. Reinitialise the chip and resync the
> +	 * regmap cache so that cached control register values are pushed
> +	 * back to har
> dware.
> +	 */
> +	ret = qmc5883p_chip_init(data);
> +	if (ret)
> +		return ret;
> +
> +	regcache_mark_dirty(data->regmap);
> +	return regcache_sync(data->regmap);
> +}
> +
> +static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(qmc5883p_system_suspend, qmc5883p_system_resume)
> +	RUNTIME_PM_OPS(qmc5883p_runtime_suspend, qmc5883p_runtime_resume, NULL)
> +};
> +
> +static const struct of_device_id qmc5883p_of_match[] = {
> +	{ .compatible = "qst,qmc5883p" },
> +	{}
> +};
> +MODULE_DEVICE_TABLE(of, qmc5883p_of_match);
> +
> +static const struct i2c_device_id qmc5883p_id[] = {
> +	{ "qmc5883p", 0 },
> +	{},

IIO style for this is:

	{ }

space between braces and no trailing comma.

> +};
> +MODULE_DEVICE_TABLE(i2c, qmc5883p_id);
> +

Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[Hardik Phalet]

On Sat Apr 11, 2026 at 1:32 AM IST, David Lechner wrote:
> On 4/9/26 4:07 PM, Hardik Phalet wrote:
>
> This is a little bit much to review all in one patch. Could be nice
> to split out power management to a separate patch.
>
> Oversampling/downsampling could be split to a separate patch or two
> as well.
>
Noted David. I will handle it in v3. I think downsampling can be another
patch.

>> +#define QMC5883P_REG_Y_MSB 0x04
>> +#define QMC58
>> 83P_REG_Z_LSB 0x05
>
> Were you manually editing the patch?
No. Maybe it's my email client (AERC). I will make sure the next version
is tight.

>
>> +#define QMC5883P_RSTCTRL_SET_RESET \
>> +	0x00 /* Set and reset on, i.e. the offset of device is renewed */
>> +#define QMC5883P_RSTCTRL_SET_ONLY 0x01 /* Set only on */
>> +#define QMC5883P_RSTCTRL_OFF 0x02 /* Set
>> and reset off */
>
> Or maybe you mail client mangled the patch? These wraps are
> happening in many places.
>
Yes. Exactly this.

>> +	if (regval != QMC5883P_CHIP_ID)
>> +		return dev_err_probe(data->dev, -ENODEV,
>
> We don't consider ID match an error. It has happened too many times
> that there is a compatible part with a different ID. This can just
> be dev_info() and return success.
>
Noted. Will handle in v3.

>> +static int qmc5883p_chip_init(struct qmc5883p_data *data)
>> +{
>> +	int ret;
>> +
>> +	ret = regmap_field_write(data->rf.sftrst, 1);
>> +	if (ret)
>> +		return ret;
>> +
>> +	usleep_range(1000, 2000);
>
> Use fsleep() instead and add a comment explaining why this specific duration
> was selected.
>
Noted, will handle in v3.

>> +	ret = regmap_field_write(data->rf.odr, QMC5883P_DEFAULT_ODR);
>> +	if (ret)
>> +		return ret;
>
> Since we just reset the chip, why do we need to set everything to a
> new default instead of using the chip's default? If there is a good
> reason, add a comment, otherwise we can leave this out.
>
These were simply more aligned with my usecase, so more of a development
artifact. Chip defaults are sufficient. I will leave this out. 

>> +
>> +	return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
>
> Does this start sampling? Seems like it could be out of place here.
>
Yes, normal mode starts sampling data. Your point makes sense, I will
remove it in the next version. Incorrect mental model, I apologise.

>> +	ret = regmap_read_poll_timeout(data->regmap, QMC5883P_REG_STATUS,
>> +				       status, status & QMC5883P_STATUS_DRDY,
>> +				       QMC5883P_DRDY_POLL_US, 150000);
>
> Numbers with lots of 0s are easier to read as 150 * (MICRO / MILLI).
>
My bad, will handle in v3.

>> +static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
>> +		       downsampling_ratio_store, 0);
>> +static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");
>
> As mentioned in the cover letter, we'd like to know more about what
> this actually does. If there is a good reason it doesn't fit with
> any existing filter attribute, then we'll need a patch to document
> the sysfs ABI as well.
>
In the device datasheet, OSR2("Down sampling ratio") is mentioned like this:
"Another filter is added for better noise performance; the depth can be
adjusted through OSR2". OSR2's defintion is called "down sampling ratio"
in a table. Nowhere else. I didn't know what attribute to map it to in
this case.

> Could save some dupilcation by making a macro that takes X/Y/Z
> as parameter. e.g.
>
> #define QMC5883P_CHAN(ch) \
> 	...				\
> 	.channel2 = IIO_MOD_##ch,	\
> 	...				\
> 	.address = AXIS_##ch,		\
> 	...
>
Noted. Will do this in v3.

>> +	ret = devm_regulator_get_enable_optional(dev, "vdd");
>> +	if (ret && ret != -ENODEV)
>> +		return dev_err_probe(dev, ret,
>> +				"failed to get vdd regulator\n");
>> +
>> +	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
>> +	fsleep(50000);
>> +
>> +	i2c_set_clientdata(client, indio_dev);
>> +
>> +	ret = qmc5883p_rf_init(data);
>
> Would be more logical to move this up right after regmap is declared.
>
Makes sense, I will move it up in v3.

>> +	ret = devm_iio_device_register(dev, indio_dev);
>
> Usually, we just return directly here. This pretty much doesn't ever fail.
>
Okay. Will remove the dev_err_probe() in v3. 

>> +	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
>> +	if (ret)
>> +		return ret;
>> +
>> +	usleep_range(10000, 11000);
>
> Again, fsleep() and comment.
>
Noted.


>> +static const struct i2c_device_id qmc5883p_id[] = {
>> +	{ "qmc5883p", 0 },
>> +	{},
>
> IIO style for this is:
>
> 	{ }
>
> space between braces and no trailing comma.
>
Noted. Will take it up in v3.

Thanks for the in-depth review David. I really appreciate it.

Regards,
Hardik

Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[Hardik Phalet]

On Fri Apr 10, 2026 at 10:58 AM IST, Greg Kroah-Hartman wrote:
> On Thu, Apr 09, 2026 at 09:07:41PM +0000, Hardik Phalet wrote:
>> Add an IIO driver for the QST QMC5883P, a 3-axis anisotropic
>> magneto-resistive (AMR) magnetometer with a 16-bit ADC, communicating
>> over I2C. There is no existing upstream driver for this device.
>
> Sorry, but no new iio drivers should be added to staging.  Take the time
> to do it right and put it into drivers/iio/ from the beginning.
> Otherwise this will just take more time and effort to get it into that
> location in the end (i.e. doing it right is simpler/faster.)
>
> thanks,
>
> greg k-h

Noted Greg, I will move it to /drivers/iio in the next version.
Thanks for the review. I appreciate your time.

Regards,
Hardik

Re: [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver

[Hardik Phalet]

On Sat Apr 11, 2026 at 1:02 AM IST, David Lechner wrote:
> On 4/9/26 4:07 PM, Hardik Phalet wrote:
>> Add a MAINTAINERS entry for the QST QMC5883P staging IIO driver,
>> covering the driver source and its device tree binding.
>>
>> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
>> ---
>>  MAINTAINERS | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index a92290fffa16..d0b9bfceb283 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -20274,6 +20274,13 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
>>  F:	drivers/bus/fsl-mc/
>>  F:	include/uapi/linux/fsl_mc.h
>>
>> +QST QMC5883P MAGNETOMETER DRIVER
>> +M:	Hardik Phalet <hardik.phalet@pm.me>
>> +L:	linux-iio@vger.kernel.org
>> +S:	Maintained
>> +F:	Documentation/devicetree/bindings/iio/magnetometer/qst,qmc5883p.yaml
>> +F:	drivers/staging/iio/magnetometer/
>> +
>>  QT1010 MEDIA DRIVER
>>  L:	linux-media@vger.kernel.org
>>  S:	Orphan
>
> This should be split up and added in the patches that actually
> add the F: files. Most of it will go with the dt-bingings patch
> and the one line added later with the driver.

Makes sense, David. Will do in the next patch series.

Regards,
Hardik

Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[Hardik Phalet]

On Sat Apr 11, 2026 at 12:56 AM IST, David Lechner wrote:
> On 4/9/26 4:07 PM, Hardik Phalet wrote:
>
> For a series this be, please wait at least a week for more feedback
> before submitting the next revision.
>
Noted. 

>> This series adds initial Linux support for the QST QMC5883P, a 3-axis
>> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC that
>> communicates over I2C. To my knowledge there is no existing upstream
>> driver for this device.
>>
>> The driver supports:
>>   - Raw magnetic field readings on X, Y, and Z axes
>>   - Four selectable full-scale ranges (±2 G, ±8 G, ±12 G, ±30 G)
>>   - Configurable output data rate (10, 50, 100, 200 Hz)
>>   - Configurable oversampling ratio (1, 2, 4, 8)
>>   - Configurable downsampling ratio (1, 2, 4, 8) via a custom sysfs
>
> What is the difference between oversampling and downsampling? I think
> we have used some filter attribute for downsampling/decimation in some
> other drivers so maybe that could be a good fit?
>
I mentioned my problem with it in my reply to your review for the third
patch in the series. Meanwhile, I will also have a look at how other
drivers are handling it.

>>     attribute
>>   - Runtime PM with a 2 s autosuspend delay
>>   - System suspend/resume via pm_runtime_force_suspend/resume
>>
>> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2
>> bit fields are accessed via regmap_field to avoid read-modify-write
>> races. The STATUS register is marked precious so regmap never reads
>> it speculatively and clears the DRDY/OVFL bits unexpectedly.
>>
>> The init sequence on probe is: soft reset → wait 1 ms → deassert
>> reset → configure SET/RESET control → apply default ODR/OSR/DSR/RNG
>> → enter normal mode. This ordering was determined empirically on
>> hardware to produce reliable, non-zero axis readings.
>>
>> The driver is placed under drivers/staging/iio/magnetometer/ with a
>> TODO file tracking the remaining work before it can graduate:
>>   - Triggered buffer support (iio_triggered_buffer_setup)
>>   - DRDY interrupt support
>>   - Self-test implementation
>
> These are not reasons to have the driver in staging. It is fine
> to have a driver that doesn't implement all functionality. We should
> be able to add those features without breaking anything.
Noted.

Regards,
Hardik

Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver

[Hardik Phalet]

On Fri Apr 10, 2026 at 10:06 AM IST, Andy Shevchenko wrote:
> On Thu, Apr 09, 2026 at 09:07:11PM +0000, Hardik Phalet wrote:
>> Verification steps performed:
>>   - Chip ID register (0x00) reads back 0x80 on probe, confirming the
>>     correct device is present
>>   - All three axes (in_magn_x_raw, in_magn_y_raw, in_magn_z_raw) return
>>     non-zero, stable values when the board is held still and change
>>     appropriately when the board is rotated
>>   - in_magn_x_scale (and Y, Z) returns the expected fractional value for
>>     the default ±8 G range (1/37500000)
>>   - in_magn_sampling_frequency / _available, in_magn_oversampling_ratio /
>>     _available, and downsampling_ratio / downsampling_ratio_available all
>>     read and write correctly; the chip responds without error to each
>>     valid setting
>>   - Runtime PM: after 2 s of inactivity the device enters suspend mode
>>     (MODE = 0x00 confirmed via i2cdump); the next sysfs read correctly
>>     resumes the device and returns valid data
>>   - System suspend/resume (echo mem > /sys/power/state) leaves the
>>     driver in a consistent state; readings remain valid after resume
>>   - dt_binding_check passes for patch 2/4
>>   - Kernel builds cleanly with W=1 and no new warnings
>
> This driver is rather huge. There are mistakes you made in the process, though:
> - never send a new version for such a code (amount and complexity) earlier than
> a week; give others a chance to review
Noted, Andy. I will take care of it from now on.

> - do not put driver to staging, why?
There are some functionality missing. But it has since been made clear
to me that that is not a valid reason. I will move to `drivers/iio`
in the next patch series.

> - the investigation is rather poor about existence of the driver — make sure
> there is no compatible (by register layout) driver in IIO or even outside it
> (for ADCs it might appear as HWMON [drivers/hwmon] or INPUT [drivers/input]
>  in some cases)
I will dig a bit deeper into this. But I could not find any thing
realted to the device I am working on (qmc5883p). 

There is a device hmc5843_core.c that already handles HMC5883L. But the
register layout is different, field ranges, gain encoding etc. are
different and I think warrants a separate driver. Other than that I
could not find something related to device I am working on. 

I will carefully go through HWMON and INPUT too, but do you have a
strategy to help me find somehting similar? It will be very useful.
Would `grep`-ing for 5883 and QST be a right starting point?

>
> --
> With Best Regards,
> Andy Shevchenko

Thank you for the review Andy, I appreciate your time!

Regards,
Hardik

- 

Re: [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation

[Hardik Phalet]

On Fri Apr 10, 2026 at 1:22 PM IST, Krzysztof Kozlowski wrote:
> On Thu, Apr 09, 2026 at 09:07:20PM +0000, Hardik Phalet wrote:
>> +  "^qst,.*":
>
> Website tells me qstcorp.com, so prefix is qstcorp. Unless it is
> different company, but then just explain that in commit msg (e.g.
> provide link to website).
No qstcorp.com is what I am referring to. I will change it in the next
commit.

>
> Best regards,
> Krzysztof

Regards,
Hardik

Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P

[Hardik Phalet]

On Fri Apr 10, 2026 at 1:25 PM IST, Krzysztof Kozlowski wrote:
> On Thu, Apr 09, 2026 at 09:07:29PM +0000, Hardik Phalet wrote:
>> Add the device tree binding document for the QST QMC5883P, a 3-axis
>> anisotropic magneto-resistive (AMR) sensor with a 16-bit ADC that
>> communicates over I2C. The binding exposes the required 'compatible'
>> and 'reg' properties along with an optional 'vdd-supply' for the
>> 2.5 V–3.6 V VDD rail.
>
> Drop last sentence. We can read the diff.
>
Noted.

> ...
>
>> +properties:
>> +  compatible:
>> +    const: qst,qmc5883p
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: I2C address of the device; the default address is 0x2c.
>> +
>> +  vdd-supply:
>> +    description:
>> +      VDD power supply (2.5 V to 3.6 V). Powers all internal analog and
>> +      digital functional blocks.
>
> Supply should be required. Devices need them to operate.
My bad, I will fix this in the next series.

>
> Best regards,
> Krzysztof

Regards,
Hardik

Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P

[Hardik Phalet]

On Fri Apr 10, 2026 at 1:36 PM IST, Krzysztof Kozlowski wrote:
> On Fri, Apr 10, 2026 at 09:55:24AM +0200, Krzysztof Kozlowski wrote:
>> On Thu, Apr 09, 2026 at 09:07:29PM +0000, Hardik Phalet wrote:
>> > Add the device tree binding document for the QST QMC5883P, a 3-axis
>> > anisotropic magneto-resistive (AMR) sensor with a 16-bit ADC that
>> > communicates over I2C. The binding exposes the required 'compatible'
>> > and 'reg' properties along with an optional 'vdd-supply' for the
>> > 2.5 V–3.6 V VDD rail.
>>
>> Drop last sentence. We can read the diff.
>>
>> ...
>>
>> > +properties:
>> > +  compatible:
>> > +    const: qst,qmc5883p
>> > +
>> > +  reg:
>> > +    maxItems: 1
>> > +    description: I2C address of the device; the default address is 0x2c.
>> > +
>> > +  vdd-supply:
>> > +    description:
>> > +      VDD power supply (2.5 V to 3.6 V). Powers all internal analog and
>> > +      digital functional blocks.
>>
>> Supply should be required. Devices need them to operate.
>
> Ah, and since I expect new version, also:
>
> A nit, subject: drop second/last, redundant "binding for". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.17-rc3/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
Noted. Thanks for sharing the link.

>
> Best regards,
> Krzysztof

Regards,
Hardik

Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver

[David Lechner]

On 4/12/26 4:54 AM, Hardik Phalet wrote:
> On Sat Apr 11, 2026 at 1:32 AM IST, David Lechner wrote:
>> On 4/9/26 4:07 PM, Hardik Phalet wrote:
>>
>> This is a little bit much to review all in one patch. Could be nice
>> to split out power management to a separate patch.
>>

...

>>> +static IIO_DEVICE_ATTR(downsampling_ratio, 0644, downsampling_ratio_show,
>>> +		       downsampling_ratio_store, 0);
>>> +static IIO_CONST_ATTR(downsampling_ratio_available, "1 2 4 8");
>>
>> As mentioned in the cover letter, we'd like to know more about what
>> this actually does. If there is a good reason it doesn't fit with
>> any existing filter attribute, then we'll need a patch to document
>> the sysfs ABI as well.
>>
> In the device datasheet, OSR2("Down sampling ratio") is mentioned like this:
> "Another filter is added for better noise performance; the depth can be
> adjusted through OSR2". OSR2's defintion is called "down sampling ratio"
> in a table. Nowhere else. I didn't know what attribute to map it to in
> this case.
> 
I wonder if there is an application note or something that explains it
in more detail if the datasheet does not.

We could always omit the feature if no one knows what it actually does.