[PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P

[PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P

[Hardik Phalet]

This series adds an IIO driver for the QST QMC5883P, a 3-axis
anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC,
communicating over I2C. To my knowledge there is no existing
upstream driver for this device (see "Prior-art register-map check"
below).

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),
    selectable via IIO_CHAN_INFO_SCALE
  - Four output data rates (10, 50, 100, 200 Hz), selectable via
    IIO_CHAN_INFO_SAMP_FREQ
  - Four oversampling ratios (1, 2, 4, 8), selectable via
    IIO_CHAN_INFO_OVERSAMPLING_RATIO
  - Runtime PM with a 2 s autosuspend delay
  - System suspend/resume delegated to the runtime callbacks

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 300 us for POR
to complete, deassert reset, drop the register cache so subsequent
RMW writes read fresh values, then enter normal mode. 300 us
comfortably covers the 250 us POR time given in the datasheet.

Patches:
  1/5 - dt-bindings: vendor-prefixes: Add QST Corporation
  2/5 - dt-bindings: iio: magnetometer: QSTCORP QMC5883P
  3/5 - iio: magnetometer: add driver for QST QMC5883P
  4/5 - iio: magnetometer: qmc5883p: add oversampling ratio support
  5/5 - iio: magnetometer: qmc5883p: add PM support

Patches 4 and 5 are split out from the main driver so that the core
(1-3) can be reviewed and picked independently, per review feedback
on v2. 4/5 exposes the CTRL_1 OSR field via
IIO_CHAN_INFO_OVERSAMPLING_RATIO. 5/5 adds runtime PM that puts the
chip into MODE_SUSPEND when idle and wakes it to MODE_NORMAL on
demand.

Changes in v3
=============
Addressing review feedback on v2:

  - Moved the driver out of staging and into drivers/iio/magnetometer/
    (Greg Kroah-Hartman).

  - Changed the vendor prefix from "qst" to "qstcorp" to match the
    manufacturer's domain (qstcorp.com) (Krzysztof Kozlowski).

  - Subject of the binding patch no longer says "Add binding for";
    "dt-bindings:" already conveys that (Krzysztof Kozlowski).

  - Dropped the redundant last sentence of the binding commit message
    (Krzysztof Kozlowski).

  - VDD supply is now made required (Krzysztof Kozlowski).

  - Split runtime PM + system sleep handling out of the core driver
    patch into its own patch (5/5), so the core driver can be reviewed
    independently (David Lechner).

  - Split oversampling-ratio support out into its own patch (4/5)
    (David Lechner).

  - Dropped the custom downsampling_ratio sysfs attribute entirely.
    The datasheet describes OSR2 only as "another filter ... depth can
    be adjusted through OSR2", with no further characterisation, and
    no application note clarifying it. Without a precise definition
    of what the filter actually does it is not possible to map OSR2
    to any existing IIO ABI, so support for it is dropped from this
    series (David Lechner).

  - qmc5883p_verify_chip_id() -> qmc5883p_read_chip_id() no longer
    treats an ID mismatch as a probe failure; the chip-ID check is
    informational only (David Lechner).

  - qmc5883p_chip_init() no longer programs driver-chosen defaults
    for RNG/OSR/DSR/ODR. The hardware defaults are sufficient and the
    explicit writes were a development artifact (David Lechner).

  - Post-reset delay in qmc5883p_chip_init() uses fsleep() with a
    comment citing the 250 us POR time from the datasheet
    (David Lechner).

  - Timeout in regmap_read_poll_timeout() written as
    150 * (MICRO / MILLI) instead of 150000 (David Lechner).

  - Channel spec duplication collapsed behind a QMC5883P_CHAN(ch)
    macro (David Lechner).

  - qmc5883p_rf_init() moved up in probe, before the regulator and
    chip-ID reads, so the regmap fields are available by the time
    they are needed (David Lechner).

  - Trailing comma and extra whitespace in the of_device_id and
    i2c_device_id sentinel entries cleaned up (David Lechner).

  - Verified that there is no existing driver in drivers/iio/,
    drivers/hwmon/, drivers/input/, drivers/staging/iio/ or
    drivers/misc/ that matches the QMC5883P register map. Summary
    of candidates inspected is included in the "Testing" section
    below (Andy Shevchenko).

  - Waited ~10 days before sending v3 to allow time for review
    (Andy Shevchenko).

Additional v3 changes not directly from review:

  - Scale encoding changed from IIO_VAL_FRACTIONAL to
    IIO_VAL_INT_PLUS_NANO with a matching write_raw_get_fmt(),
    because the IIO core defaults sysfs writes to
    IIO_VAL_INT_PLUS_MICRO and was silently truncating nano-precision
    writes. The truncation on the 8 G and 2 G entries is documented
    in a comment above the scale table.

  - STATUS register marked precious (in addition to volatile) so
    regmap never reads it speculatively and clears DRDY/OVFL.

  - Added regcache_drop_region() after the soft-reset deassert, so
    subsequent RMW writes read fresh values rather than cached
    pre-reset values.

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).

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

Testing
=======
Hardware
  A GY-271 HM-246 breakout (this board is a QMC5883P, not a QMC5883L,
  despite what some vendors put on the silkscreen), connected over
  I2C bus 1 to a Raspberry Pi 4B running a mainline aarch64 kernel.
  The chip enumerates at address 0x2C via i2cdetect, and CHIP_ID
  (register 0x00) reads back 0x80 as expected.

Prior-art register-map check (for Andy)
  I grepped drivers/iio/magnetometer/, drivers/hwmon/,
  drivers/input/misc/, drivers/staging/iio/ and drivers/misc/ for
  the distinctive offsets 0x09 (STATUS) and 0x0A (CTRL_1), narrowed
  to files containing both, and manually compared each candidate's
  register layout and control-bit encoding against the QMC5883P.

  Closest candidates:
    ak8975.c    - four register offsets coincide (0x00, 0x09, 0x0A,
                  0x0B) but the data registers sit at 0x03-0x08
                  (shifted +2 vs QMC5883P's 0x01-0x06), DRDY is in
                  ST1 at 0x02 rather than STATUS at 0x09, and CNTL
                  encodes a 4-bit mode only - no packed ODR/OSR/range
                  fields.
    hmc5843.c   - STATUS matches at 0x09, but 0x0A is a read-only ID
                  register, configuration spans 0x00-0x02 rather than
                  a single CTRL_1 byte, and data is MSB-first at 0x03
                  in X/Z/Y order.
    af8133j.c   - 0x0A and 0x0B carry mode and range, but STATUS is
                  at 0x02, data starts at 0x03, and the mode field is
                  2-valued (standby/work) rather than 4-valued.
    mmc35240.c  - data at 0x00-0x05 overlaps, but STATUS and control
                  land at 0x06-0x08.

  No overlap worth discussing: mmc5633, mag3110, tlv493d, tmag5273,
  bmc150_magn, rm3100, yamaha-yas530, st_magn, si7210, als31300. No
  magnetometer driver under drivers/hwmon/, drivers/input/misc/ or
  drivers/staging/.

  Conclusion: no existing driver can be extended to cover the
  QMC5883P without restructuring its register addressing and
  control-bit model. A new driver is warranted.

Functional testing on v3
  - Chip ID read: 0x80 (matches datasheet).
  - Raw axis reads: in_magn_{x,y,z}_raw return stable s16 values
    and track manual reorientation of the board.
  - Scale: all four ranges (+/-2/8/12/30 G) selectable via
    in_magn_scale; in_magn_scale_available lists all four; sysfs
    write-back round-trips cleanly at nano precision.
  - Sampling frequency: 10/50/100/200 Hz all selectable via
    in_magn_sampling_frequency; _available lists all four.
  - Oversampling ratio (patch 4): 1/2/4/8 selectable via
    in_magn_oversampling_ratio; _available lists all four.
  - DRDY polling: verified STATUS.DRDY asserts and clears on read,
    and that OVFL is captured in the same read as DRDY.
  - Soft reset: register state after qmc5883p_chip_init() matches
    the datasheet defaults; regcache_drop_region() confirmed by
    observing fresh values being read on the first post-reset RMW.
  - Runtime PM (patch 5): power/runtime_status transitions to
    "suspended" after the 2 s autosuspend delay (MODE_SUSPEND on
    the wire, verified by i2cdump); next sysfs read resumes the
    device and returns valid data.
  - System sleep: echo mem > /sys/power/state (s2idle on the Pi)
    followed by wake; readings are valid after resume.
  - Unbind: echo <dev> > /sys/bus/i2c/drivers/qmc5883p/unbind
    leaves the chip in MODE_SUSPEND, confirming the devm LIFO
    teardown order.
  - Build: CONFIG_QMC5883P=y and =m both clean; W=1 clean on
    aarch64; sparse clean; checkpatch --strict clean.
  - dt_binding_check: passes for patch 2/5.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
Hardik Phalet (5):
      dt-bindings: vendor-prefixes: Add QST Corporation
      dt-bindings: iio: magnetometer: QSTCORP QMC5883P
      iio: magnetometer: add driver for QST QMC5883P
      iio: magnetometer: qmc5883p: add oversampling ratio support
      iio: magnetometer: qmc5883p: add PM support

 .../iio/magnetometer/qstcorp,qmc5883p.yaml         |  48 ++
 .../devicetree/bindings/vendor-prefixes.yaml       |   2 +
 MAINTAINERS                                        |   7 +
 drivers/iio/magnetometer/Kconfig                   |  11 +
 drivers/iio/magnetometer/Makefile                  |   2 +
 drivers/iio/magnetometer/qmc5883p.c                | 673 +++++++++++++++++++++
 6 files changed, 743 insertions(+)
---
base-commit: d2a4ec19d2a2e54c23b5180e939994d3da4a6b91
change-id: 20260418-qmc5883p-driver-dcc74bd4a789

Best regards,
--  
Hardik Phalet <hardik.phalet@pm.me>

[PATCH v3 1/5] dt-bindings: vendor-prefixes: Add QST Corporation

[Hardik Phalet]

Add the vendor prefix 'qstcorp' 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 ee7fd3cfe203..4ecf438f1a4a 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -1337,6 +1337,8 @@ patternProperties:
     description: Shenzhen QiShenglong Industrialist Co., Ltd.
   "^qnap,.*":
     description: QNAP Systems, Inc.
+  "^qstcorp,.*":
+    description: QST Corporation
   "^quanta,.*":
     description: Quanta Computer Inc.
   "^radxa,.*":

-- 
2.53.0

[PATCH v3 2/5] dt-bindings: iio: magnetometer: QSTCORP 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.

Add a MAINTAINERS entry for the QSTCORP QMC5883P devicetree binding.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 .../iio/magnetometer/qstcorp,qmc5883p.yaml         | 48 ++++++++++++++++++++++
 MAINTAINERS                                        |  6 +++
 2 files changed, 54 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml
new file mode 100644
index 000000000000..72cc3fef2226
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,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/qstcorp,qmc5883p.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: QSTCORP 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: qstcorp,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
+  - vdd-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        magnetometer@2c {
+            compatible = "qstcorp,qmc5883p";
+            reg = <0x2c>;
+            vdd-supply = <&vdd_3v3>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 48fda1f8332e..d41f6b33d0e5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21554,6 +21554,12 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
 F:	drivers/bus/fsl-mc/
 F:	include/uapi/linux/fsl_mc.h
 
+QSTCORP QMC5883P MAGNETOMETER DRIVER
+M:	Hardik Phalet <hardik.phalet@pm.me>
+L:	linux-iio@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml
+
 QT1010 MEDIA DRIVER
 L:	linux-media@vger.kernel.org
 S:	Orphan

-- 
2.53.0

[PATCH v3 3/5] iio: magnetometer: add driver for QST QMC5883P

[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) selectable
   via IIO_CHAN_INFO_SCALE
 - Output data rate configurable via IIO_CHAN_INFO_SAMP_FREQ (10, 50,
   100, 200 Hz)
 - vdd-supply regulator management

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 300 us for POR
completion, deassert reset, then drop the register cache so subsequent
RMW writes read fresh values from the device. After reset the chip is in
MODE_SUSPEND per datasheet §6.2.4, and is left there; the first
userspace access will wake it via runtime PM (added in a follow-up
patch).

Cleanup is fully devm-managed via devm_regulator_get_enable() and
devm_iio_device_register().

Oversampling ratio and runtime PM are added in follow-up patches.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 MAINTAINERS                         |   1 +
 drivers/iio/magnetometer/Kconfig    |  11 +
 drivers/iio/magnetometer/Makefile   |   2 +
 drivers/iio/magnetometer/qmc5883p.c | 574 ++++++++++++++++++++++++++++++++++++
 4 files changed, 588 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index d41f6b33d0e5..2fbbe8831a7c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21559,6 +21559,7 @@ M:	Hardik Phalet <hardik.phalet@pm.me>
 L:	linux-iio@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml
+F:	drivers/iio/magnetometer/qmc5883p.c
 
 QT1010 MEDIA DRIVER
 L:	linux-media@vger.kernel.org
diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
index fb313e591e85..333c5e6f231d 100644
--- a/drivers/iio/magnetometer/Kconfig
+++ b/drivers/iio/magnetometer/Kconfig
@@ -298,4 +298,15 @@ config YAMAHA_YAS530
 	  To compile this driver as a module, choose M here: the module
 	  will be called yamaha-yas.
 
+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/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
index 5bd227f8c120..ff519a055d77 100644
--- a/drivers/iio/magnetometer/Makefile
+++ b/drivers/iio/magnetometer/Makefile
@@ -39,3 +39,5 @@ obj-$(CONFIG_SI7210)			+= si7210.o
 obj-$(CONFIG_TI_TMAG5273)		+= tmag5273.o
 
 obj-$(CONFIG_YAMAHA_YAS530)		+= yamaha-yas530.o
+
+obj-$(CONFIG_QMC5883P) += qmc5883p.o
diff --git a/drivers/iio/magnetometer/qmc5883p.c b/drivers/iio/magnetometer/qmc5883p.c
new file mode 100644
index 000000000000..e4a76ae7c2cf
--- /dev/null
+++ b/drivers/iio/magnetometer/qmc5883p.c
@@ -0,0 +1,574 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * qmc5883p.c - QMC5883P magnetometer driver
+ *
+ * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
+ *
+ * TODO: add triggered buffer support, PM, OSR, DSR
+ *
+ */
+
+#include <linux/array_size.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/iio/iio.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/units.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
+
+#define QMC5883P_RNG_30G 0x00
+#define QMC5883P_RNG_12G 0x01
+#define QMC5883P_RNG_08G 0x02
+#define QMC5883P_RNG_02G 0x03
+
+#define QMC5883P_DRDY_POLL_US 1000
+
+#define QMC5883P_CHIP_ID 0x80
+
+#define QMC5883P_STATUS_DRDY BIT(0)
+#define QMC5883P_STATUS_OVFL BIT(1)
+
+struct qmc5883p_rf {
+	struct regmap_field *osr;
+	struct regmap_field *odr;
+	struct regmap_field *mode;
+	struct regmap_field *rng;
+	struct regmap_field *sftrst;
+	struct regmap_field *chip_id;
+};
+
+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,
+};
+
+/*
+ * Scale factors in nT/LSB for IIO_VAL_INT_PLUS_NANO, derived from datasheet
+ * Table 2 sensitivities (LSB/G) converted to LSB/T (1 G = 1e-4 T):
+ *   sensitivity_T = sensitivity_G * 10000
+ *   scale_nT     = 1e9 / sensitivity_T
+ *
+ * The 8G and 2G entries truncate 26.666... and 6.666... nT/LSB respectively;
+ * IIO_VAL_INT_PLUS_NANO cannot carry the exact rationals, but the chosen
+ * values match what IIO_VAL_FRACTIONAL would have rendered and therefore
+ * round-trip cleanly through sysfs write back.
+ *
+ * Index matches register value: RNG<1:0> = 0b00..0b11
+ */
+static const int qmc5883p_scale[][2] = {
+	[QMC5883P_RNG_30G] = { 0, 100 },
+	[QMC5883P_RNG_12G] = { 0, 40 },
+	[QMC5883P_RNG_08G] = { 0, 26 },
+	[QMC5883P_RNG_02G] = { 0, 6 },
+};
+
+static const int qmc5883p_odr[] = {
+	[QMC5883P_ODR_10] = 10,
+	[QMC5883P_ODR_50] = 50,
+	[QMC5883P_ODR_100] = 100,
+	[QMC5883P_ODR_200] = 200,
+};
+
+static const struct regmap_range qmc5883p_readable_ranges[] = {
+	regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_Z_MSB),
+	regmap_reg_range(QMC5883P_REG_STATUS, 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),
+	regmap_reg_range(QMC5883P_REG_CTRL_2, QMC5883P_REG_CTRL_2),
+};
+
+/*
+ * 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,
+};
+
+static const struct reg_field qmc5883p_rf_osr =
+	REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
+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_sftrst =
+	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
+static const struct reg_field qmc5883p_rf_chip_id =
+	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
+
+/*
+ * qmc5883p_get_measure - read all three axes.
+ * Must be called with data->mutex held.
+ */
+static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
+				s16 *z)
+{
+	int ret;
+	u8 reg_data[6];
+	unsigned int status;
+
+	/*
+	 * 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,
+				       150 * (MICRO / MILLI));
+	if (ret)
+		return ret;
+
+	if (status & QMC5883P_STATUS_OVFL) {
+		dev_warn_ratelimited(data->dev,
+			"data overflow, consider reducing field range\n");
+		ret = -ERANGE;
+		return ret;
+	}
+
+	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
+			       ARRAY_SIZE(reg_data));
+	if (ret)
+		return ret;
+
+	*x = (s16)get_unaligned_le16(&reg_data[0]);
+	*y = (s16)get_unaligned_le16(&reg_data[2]);
+	*z = (s16)get_unaligned_le16(&reg_data[4]);
+
+	return ret;
+}
+
+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_INT_PLUS_NANO;
+
+	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;
+	}
+
+	return -EINVAL;
+}
+
+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_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 = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+	if (ret)
+		return ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = qmc5883p_write_odr(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;
+
+	return ret;
+}
+
+/*
+ * qmc5883p_read_avail - expose available values to userspace.
+ *
+ * Creates the _available sysfs attributes automatically:
+ *   in_magn_sampling_frequency_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_SCALE:
+		*vals = (const int *)qmc5883p_scale;
+		*type = IIO_VAL_INT_PLUS_NANO;
+		*length = ARRAY_SIZE(qmc5883p_scale) * 2;
+		return IIO_AVAIL_LIST;
+
+	default:
+		return -EINVAL;
+	}
+}
+
+/*
+ * Tell the IIO core how to parse sysfs writes. Without this, the core
+ * defaults to IIO_VAL_INT_PLUS_MICRO (6 fractional digits), which would
+ * silently truncate nano-scale writes like "0.000000040" to 0.
+ */
+static int qmc5883p_write_raw_get_fmt(struct iio_dev *indio_dev,
+				      struct iio_chan_spec const *chan,
+				      long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SCALE:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info qmc5883p_info = {
+	.read_raw = qmc5883p_read_raw,
+	.write_raw = qmc5883p_write_raw,
+	.write_raw_get_fmt = qmc5883p_write_raw_get_fmt,
+	.read_avail = qmc5883p_read_avail,
+};
+
+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->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->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
+	if (IS_ERR(rf->sftrst))
+		return PTR_ERR(rf->sftrst);
+
+	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_read_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)
+		dev_info(data->dev, "unexpected chip ID %#x, expected %#x\n",
+			regval, QMC5883P_CHIP_ID);
+
+	return 0;
+}
+
+#define QMC5883P_CHAN(ch)                                                 \
+	{                                                                 \
+		.type = IIO_MAGN,                                         \
+		.channel2 = IIO_MOD_##ch,                                 \
+		.modified = 1,                                            \
+		.address = AXIS_##ch,                                     \
+		.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), \
+		.info_mask_shared_by_type_available =                     \
+			BIT(IIO_CHAN_INFO_SAMP_FREQ),                     \
+	}
+
+static const struct iio_chan_spec qmc5883p_channels[] = {
+	QMC5883P_CHAN(X),
+	QMC5883P_CHAN(Y),
+	QMC5883P_CHAN(Z),
+};
+
+static int qmc5883p_chip_init(struct qmc5883p_data *data)
+{
+	int ret;
+
+	ret = regmap_field_write(data->rf.sftrst, 1);
+	if (ret)
+		return ret;
+
+	/*
+	 * The datasheet does not specify a post-reset delay, but POR
+	 * completion takes up to 250 microseconds. Use 300 microseconds
+	 * to be safe.
+	 */
+	fsleep(300);
+
+	ret = regmap_field_write(data->rf.sftrst, 0);
+	if (ret)
+		return ret;
+
+	/*
+	 * Soft reset restored every register to its default. Drop the cache
+	 * so subsequent RMW writes read fresh values from the device.
+	 */
+	regcache_drop_region(data->regmap, QMC5883P_REG_CHIP_ID,
+			     QMC5883P_REG_CTRL_2);
+
+	/* Chip is now in MODE_SUSPEND per datasheet §6.2.4. Leave it there. */
+	return 0;
+}
+
+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);
+	i2c_set_clientdata(client, indio_dev);
+	data->dev = dev;
+	data->regmap = regmap;
+
+	mutex_init(&data->mutex);
+
+	ret = qmc5883p_rf_init(data);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to initialize regmap fields\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);
+
+	ret = devm_regulator_get_enable(dev, "vdd");
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "failed to initialize vdd regulator\n");
+
+	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
+	fsleep(50 * (MICRO / MILLI) + 250);
+
+	ret = qmc5883p_read_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");
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+
+static const struct of_device_id qmc5883p_of_match[] = {
+	{ .compatible = "qstcorp,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,
+	},
+	.probe = qmc5883p_probe,
+	.id_table = qmc5883p_id,
+};
+module_i2c_driver(qmc5883p_driver);
+
+MODULE_AUTHOR("Hardik Phalet <hardik.phalet@pm.me>");
+MODULE_DESCRIPTION("QST QMC5883P 3-axis magnetometer driver");
+MODULE_LICENSE("GPL");

-- 
2.53.0

[PATCH v3 4/5] iio: magnetometer: qmc5883p: add oversampling ratio support

[Hardik Phalet]

Expose the CTRL_1 OSR field through IIO_CHAN_INFO_OVERSAMPLING_RATIO so
userspace can select among the four oversampling settings (1, 2, 4, 8)
supported by the device. Read, write and available handlers mirror the
existing SAMP_FREQ plumbing and use the already-present rf.osr regmap
field.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 drivers/iio/magnetometer/qmc5883p.c | 46 ++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/magnetometer/qmc5883p.c b/drivers/iio/magnetometer/qmc5883p.c
index e4a76ae7c2cf..d0e4a1a600b6 100644
--- a/drivers/iio/magnetometer/qmc5883p.c
+++ b/drivers/iio/magnetometer/qmc5883p.c
@@ -4,7 +4,7 @@
  *
  * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
  *
- * TODO: add triggered buffer support, PM, OSR, DSR
+ * TODO: add triggered buffer support, PM, DSR
  *
  */
 
@@ -119,6 +119,13 @@ static const int qmc5883p_odr[] = {
 	[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 struct regmap_range qmc5883p_readable_ranges[] = {
 	regmap_reg_range(QMC5883P_REG_CHIP_ID, QMC5883P_REG_Z_MSB),
 	regmap_reg_range(QMC5883P_REG_STATUS, QMC5883P_REG_CTRL_2),
@@ -277,6 +284,13 @@ static int qmc5883p_read_raw(struct iio_dev *indio_dev,
 			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;
@@ -306,6 +320,18 @@ static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
 	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 int qmc5883p_write_raw(struct iio_dev *indio_dev,
 			      struct iio_chan_spec const *chan, int val,
 			      int val2, long mask)
@@ -323,6 +349,9 @@ static int qmc5883p_write_raw(struct iio_dev *indio_dev,
 	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;
@@ -357,6 +386,12 @@ static int qmc5883p_read_avail(struct iio_dev *indio_dev,
 		*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_INT_PLUS_NANO;
@@ -382,6 +417,8 @@ static int qmc5883p_write_raw_get_fmt(struct iio_dev *indio_dev,
 		return IIO_VAL_INT_PLUS_NANO;
 	case IIO_CHAN_INFO_SAMP_FREQ:
 		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
+		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
 	}
@@ -452,9 +489,12 @@ static int qmc5883p_read_chip_id(struct qmc5883p_data *data)
 		.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), \
+		.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_SAMP_FREQ) |                    \
+			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),            \
 	}
 
 static const struct iio_chan_spec qmc5883p_channels[] = {

-- 
2.53.0

[PATCH v3 5/5] iio: magnetometer: qmc5883p: add PM support

[Hardik Phalet]

Add runtime PM with a 2 s autosuspend delay. Per datasheet §6.2.1
the chip continuously samples in MODE_NORMAL; putting it into
MODE_SUSPEND when idle drops current from up to 1180 uA to ~22 uA
(datasheet Table 2).

Wrap qmc5883p_get_measure() and qmc5883p_write_raw() with
pm_runtime_resume_and_get() / pm_runtime_put_autosuspend(), converting
early returns to a goto so the put is always paired.

System sleep is delegated to the runtime callbacks via
SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume).

A devm action is registered before devm_pm_runtime_enable() so that
LIFO teardown on unbind runs pm_runtime_disable() first (freezing PM
state) and then suspends the hardware via MODE_SUSPEND.

Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
---
 drivers/iio/magnetometer/qmc5883p.c | 69 ++++++++++++++++++++++++++++++++++---
 1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/magnetometer/qmc5883p.c b/drivers/iio/magnetometer/qmc5883p.c
index d0e4a1a600b6..0ff635924abf 100644
--- a/drivers/iio/magnetometer/qmc5883p.c
+++ b/drivers/iio/magnetometer/qmc5883p.c
@@ -4,7 +4,7 @@
  *
  * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
  *
- * TODO: add triggered buffer support, PM, DSR
+ * TODO: add triggered buffer support, DSR
  *
  */
 
@@ -15,6 +15,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
+#include <linux/pm_runtime.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/units.h>
@@ -208,6 +209,10 @@ static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
 	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
@@ -220,24 +225,26 @@ static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
 				       QMC5883P_DRDY_POLL_US,
 				       150 * (MICRO / MILLI));
 	if (ret)
-		return ret;
+		goto out;
 
 	if (status & QMC5883P_STATUS_OVFL) {
 		dev_warn_ratelimited(data->dev,
 			"data overflow, consider reducing field range\n");
 		ret = -ERANGE;
-		return ret;
+		goto out;
 	}
 
 	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
 			       ARRAY_SIZE(reg_data));
 	if (ret)
-		return 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_put_autosuspend(data->dev);
 	return ret;
 }
 
@@ -341,10 +348,14 @@ static int qmc5883p_write_raw(struct iio_dev *indio_dev,
 
 	guard(mutex)(&data->mutex);
 
-	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+	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);
@@ -364,6 +375,8 @@ static int qmc5883p_write_raw(struct iio_dev *indio_dev,
 	if (restore && !ret)
 		ret = restore;
 
+out:
+	pm_runtime_put_autosuspend(data->dev);
 	return ret;
 }
 
@@ -533,6 +546,18 @@ static int qmc5883p_chip_init(struct qmc5883p_data *data)
 	return 0;
 }
 
+static void qmc5883p_suspend_action(void *arg)
+{
+	struct qmc5883p_data *data = arg;
+
+	/*
+	 * PM is already disabled at this point (devm LIFO); put the hardware
+	 * into MODE_SUSPEND directly so the chip is not left sampling after
+	 * unbind.
+	 */
+	regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
+}
+
 static int qmc5883p_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -584,9 +609,42 @@ static int qmc5883p_probe(struct i2c_client *client)
 	if (ret)
 		return dev_err_probe(dev, ret, "failed to initialize chip\n");
 
+	ret = devm_add_action_or_reset(dev, qmc5883p_suspend_action, data);
+	if (ret)
+		return ret;
+
+	pm_runtime_set_autosuspend_delay(dev, 2000);
+	pm_runtime_use_autosuspend(dev);
+
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
+
 	return devm_iio_device_register(dev, indio_dev);
 }
 
+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);
+
+	return regmap_field_write(data->rf.mode, QMC5883P_MODE_NORMAL);
+}
+
+static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
+	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
+		RUNTIME_PM_OPS(qmc5883p_runtime_suspend,
+			       qmc5883p_runtime_resume, NULL)
+};
+
 static const struct of_device_id qmc5883p_of_match[] = {
 	{ .compatible = "qstcorp,qmc5883p" },
 	{ }
@@ -603,6 +661,7 @@ 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,

-- 
2.53.0

Re: [PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 10:32:09PM +0000, Hardik Phalet wrote:
> This series adds an IIO driver for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC,
> communicating over I2C. To my knowledge there is no existing
> upstream driver for this device (see "Prior-art register-map check"
> below).
> 
> 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),
>     selectable via IIO_CHAN_INFO_SCALE
>   - Four output data rates (10, 50, 100, 200 Hz), selectable via
>     IIO_CHAN_INFO_SAMP_FREQ
>   - Four oversampling ratios (1, 2, 4, 8), selectable via
>     IIO_CHAN_INFO_OVERSAMPLING_RATIO
>   - Runtime PM with a 2 s autosuspend delay
>   - System suspend/resume delegated to the runtime callbacks
> 
> Regmap with an rbtree cache is used throughout. CTRL_1 and CTRL_2

I'm not sure this paragraph answers the question "why not maple tree?"

> 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 300 us for POR
> to complete, deassert reset, drop the register cache so subsequent
> RMW writes read fresh values, then enter normal mode. 300 us
> comfortably covers the 250 us POR time given in the datasheet.
> 
> Patches:
>   1/5 - dt-bindings: vendor-prefixes: Add QST Corporation
>   2/5 - dt-bindings: iio: magnetometer: QSTCORP QMC5883P
>   3/5 - iio: magnetometer: add driver for QST QMC5883P
>   4/5 - iio: magnetometer: qmc5883p: add oversampling ratio support
>   5/5 - iio: magnetometer: qmc5883p: add PM support
> 
> Patches 4 and 5 are split out from the main driver so that the core
> (1-3) can be reviewed and picked independently, per review feedback
> on v2. 4/5 exposes the CTRL_1 OSR field via
> IIO_CHAN_INFO_OVERSAMPLING_RATIO. 5/5 adds runtime PM that puts the
> chip into MODE_SUSPEND when idle and wakes it to MODE_NORMAL on
> demand.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 3/5] iio: magnetometer: add driver for QST QMC5883P

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 10:32:43PM +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.
> 
> 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) selectable
>    via IIO_CHAN_INFO_SCALE
>  - Output data rate configurable via IIO_CHAN_INFO_SAMP_FREQ (10, 50,
>    100, 200 Hz)
>  - vdd-supply regulator management
> 
> 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 300 us for POR
> completion, deassert reset, then drop the register cache so subsequent
> RMW writes read fresh values from the device. After reset the chip is in
> MODE_SUSPEND per datasheet §6.2.4, and is left there; the first
> userspace access will wake it via runtime PM (added in a follow-up
> patch).
> 
> Cleanup is fully devm-managed via devm_regulator_get_enable() and
> devm_iio_device_register().
> 
> Oversampling ratio and runtime PM are added in follow-up patches.

...

> +/*
> + * qmc5883p.c - QMC5883P magnetometer driver

Do not add filename inside file. It makes often become an outdated (missed to
change) if the driver is renamed.

> + * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
> + *
> + * TODO: add triggered buffer support, PM, OSR, DSR

> + *

Drop this blank line.

> + */

...

> +#include <linux/array_size.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/unaligned.h>

Follow IWYU. Missing cleanup.h, types.h, maybe more, I haven't carefully checked.

...

> +#define QMC5883P_REG_CHIP_ID 0x00

It's better to read when it's a tab before value.

...

> +#define QMC5883P_DRDY_POLL_US 1000

(1 * USEC_PER_MSEC)

from time.h which gives better readability of what the units of the value.
Since it's used only once in the code, drop it, see below.

...

> +enum qmc5883p_channels {
> +	AXIS_X = 0,

Why? Is it Linux enum or HW-driven one? If the latter, you have to assign all
of them explicitly, otherwise it doesn't matter, no assignment is needed.

> +	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_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_sftrst =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
> +static const struct reg_field qmc5883p_rf_chip_id =
> +	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);

I would make these to be one-liners.

...

> +static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
> +				s16 *z)

Do a logical split.

static int qmc5883p_get_measure(struct qmc5883p_data *data,
				s16 *x, s16 *y, s16 *z)

> +{
> +	int ret;
> +	u8 reg_data[6];
> +	unsigned int status;
> +
> +	/*
> +	 * 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,
> +				       150 * (MICRO / MILLI));

MICRO/MILLI is used for µV to mV conversion (as we don't have it defined), but
here we have the constants in time.h (somewhere down).

				       1 * USEC_PER_MSEC, 150 * USEC_PER_MSEC);

> +	if (ret)
> +		return ret;
> +
> +	if (status & QMC5883P_STATUS_OVFL) {
> +		dev_warn_ratelimited(data->dev,
> +			"data overflow, consider reducing field range\n");
> +		ret = -ERANGE;

EOVERFLOW ? (ERANGE is fine, but the proposed might be slightly better...
or worse :-)

> +		return ret;
> +	}
> +
> +	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
> +			       ARRAY_SIZE(reg_data));

They are u8, sizeof() will suffice. Note, ARRAY_SIZE() may lead to interesting
bugs in some cases.

> +	if (ret)
> +		return ret;
> +
> +	*x = (s16)get_unaligned_le16(&reg_data[0]);
> +	*y = (s16)get_unaligned_le16(&reg_data[2]);
> +	*z = (s16)get_unaligned_le16(&reg_data[4]);

Why not defining the reg_data properly from the start?

	__le16 reg_data[3];

?

> +	return ret;
> +}

...

> +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;

Don't we have some struct for this? In any case you can define...

struct ...3d {
	s16 x;
	s16 y;
	s16 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);

...and use it here.

> +		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)

All these ' < 0' parts, are they needed / required?

> +			return ret;
> +		*val = qmc5883p_scale[regval][0];
> +		*val2 = qmc5883p_scale[regval][1];
> +		return IIO_VAL_INT_PLUS_NANO;
> +
> +	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;
> +	}
> +
> +	return -EINVAL;
> +}

...

> +static int qmc5883p_write_scale(struct qmc5883p_data *data, int val, int val2)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_scale); i++) {

	for (unsigned int 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)

As per above.

...

> +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 = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	if (ret)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = qmc5883p_write_odr(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;
> +
> +	return ret;

Why not simply do this

	if (ret)
		return ret;

	return restore;

?

> +}

...

> +static int qmc5883p_rf_init(struct qmc5883p_data *data)
> +{
> +	struct regmap *regmap = data->regmap;
> +	struct device *dev = data->dev;

Don't keep duplicates, either regmap from dev or dev from regmap may be
retrieved using the respective APIs.

> +	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->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->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
> +	if (IS_ERR(rf->sftrst))
> +		return PTR_ERR(rf->sftrst);
> +
> +	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_read_chip_id(struct qmc5883p_data *data)
> +{
> +	int ret, regval;

Why regval is signed? regmap API uses unsigned int all over the places...

> +	ret = regmap_field_read(data->rf.chip_id, &regval);
> +	if (ret)
> +		return dev_err_probe(data->dev, ret,
> +				     "failed to read chip ID\n");

Single line.

> +
> +	if (regval != QMC5883P_CHIP_ID)
> +		dev_info(data->dev, "unexpected chip ID %#x, expected %#x\n",
> +			regval, QMC5883P_CHIP_ID);
> +
> +	return 0;
> +}

...

> +#define QMC5883P_CHAN(ch)                                                 \
> +	{                                                                 \
> +		.type = IIO_MAGN,                                         \
> +		.channel2 = IIO_MOD_##ch,                                 \
> +		.modified = 1,                                            \
> +		.address = AXIS_##ch,                                     \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |            \
> +				      BIT(IIO_CHAN_INFO_SCALE),           \

The style of this is different to the below, I would follow below one.

		.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), \
> +		.info_mask_shared_by_type_available =                     \
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),                     \
> +	}

The  \ are indented with spaces! Please, use tabs.

...

> +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);
> +	i2c_set_clientdata(client, indio_dev);
> +	data->dev = dev;
> +	data->regmap = regmap;

> +	mutex_init(&data->mutex);

devm_mutex_init()

> +	ret = qmc5883p_rf_init(data);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to initialize regmap fields\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);
> +
> +	ret = devm_regulator_get_enable(dev, "vdd");
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "failed to initialize vdd regulator\n");
> +
> +	/* Datasheet specifies up to 50 ms supply ramp + 250 us POR time. */
> +	fsleep(50 * (MICRO / MILLI) + 250);

USEC_PER_MSEC

> +
> +	ret = qmc5883p_read_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");
> +
> +	return devm_iio_device_register(dev, indio_dev);
> +}

...

> +static const struct i2c_device_id qmc5883p_id[] = {
> +	{ "qmc5883p", 0 },

Drop ', 0' part.

> +	{ }
> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 4/5] iio: magnetometer: qmc5883p: add oversampling ratio support

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 10:32:56PM +0000, Hardik Phalet wrote:
> Expose the CTRL_1 OSR field through IIO_CHAN_INFO_OVERSAMPLING_RATIO so
> userspace can select among the four oversampling settings (1, 2, 4, 8)
> supported by the device. Read, write and available handlers mirror the
> existing SAMP_FREQ plumbing and use the already-present rf.osr regmap
> field.

...

>  		.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), \
> +		.info_mask_shared_by_type =                               \
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |                    \
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),            \

Use the same style from the start:

		.info_mask_shared_by_type =                               \
			BIT(IIO_CHAN_INFO_SAMP_FREQ);                    \

>  		.info_mask_shared_by_type_available =                     \
> -			BIT(IIO_CHAN_INFO_SAMP_FREQ),                     \
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ) |                    \
> +			BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),            \

In both cases this will become no '-' lines in the diff.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 5/5] iio: magnetometer: qmc5883p: add PM support

[Andy Shevchenko]

On Sun, Apr 19, 2026 at 10:33:06PM +0000, Hardik Phalet wrote:
> Add runtime PM with a 2 s autosuspend delay. Per datasheet §6.2.1
> the chip continuously samples in MODE_NORMAL; putting it into
> MODE_SUSPEND when idle drops current from up to 1180 uA to ~22 uA
> (datasheet Table 2).
> 
> Wrap qmc5883p_get_measure() and qmc5883p_write_raw() with
> pm_runtime_resume_and_get() / pm_runtime_put_autosuspend(), converting
> early returns to a goto so the put is always paired.
> 
> System sleep is delegated to the runtime callbacks via
> SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume).
> 
> A devm action is registered before devm_pm_runtime_enable() so that
> LIFO teardown on unbind runs pm_runtime_disable() first (freezing PM
> state) and then suspends the hardware via MODE_SUSPEND.

...

> +	ret = pm_runtime_resume_and_get(data->dev);
> +	if (ret < 0)
> +		return ret;

Don't you want to use PM_RUNTIME_ACQUIRE_AUTOSUSPEND()?

...

>  	guard(mutex)(&data->mutex);
>  
> -	ret = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);
> +	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;

No way, please, read what is written in the top of cleanup.h, also use
the advice I gave above.

> +out:
> +	pm_runtime_put_autosuspend(data->dev);
>  	return ret;
>  }

...

> +static const struct dev_pm_ops qmc5883p_dev_pm_ops = {
> +	SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend, pm_runtime_force_resume)
> +		RUNTIME_PM_OPS(qmc5883p_runtime_suspend,
> +			       qmc5883p_runtime_resume, NULL)

Besides broken indentation, use proper macro from DEFINE_PM_*() family:
DEFINE_RUNTIME_DEV_PM_OPS().

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/5] dt-bindings: iio: magnetometer: QSTCORP QMC5883P

[Jonathan Cameron]

On Sun, 19 Apr 2026 22:32:32 +0000
Hardik Phalet <hardik.phalet@pm.me> 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.
> 
> Add a MAINTAINERS entry for the QSTCORP QMC5883P devicetree binding.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>

Google fed me a break out board for this that has a drdy pin?
If so include that as an interrupt as the binding should attempt
to be as complete as possible from the start.

> ---
>  .../iio/magnetometer/qstcorp,qmc5883p.yaml         | 48 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml
> new file mode 100644
> index 000000000000..72cc3fef2226
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/magnetometer/qstcorp,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/qstcorp,qmc5883p.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: QSTCORP 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

If you are spinning again I'd drop the standard and fast modes thing.
It's rare that a device doesn't support those two.  High speed mode is another thing
entirely as that changes the protocol quite a bit (I think - it's been a while
since I spec dived on these).

> +  targeted at compass, navigation, and industrial applications.
> +
> +properties:
> +  compatible:
> +    const: qstcorp,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
> +  - vdd-supply
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    i2c {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        magnetometer@2c {
> +            compatible = "qstcorp,qmc5883p";
> +            reg = <0x2c>;
> +            vdd-supply = <&vdd_3v3>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 48fda1f8332e..d41f6b33d0e5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -21554,6 +21554,12 @@ F:	Documentation/networking/device_drivers/ethernet/freescale/dpaa2/overview.rst
>  F:	drivers/bus/fsl-mc/
>  F:	include/uapi/linux/fsl_mc.h
>  
> +QSTCORP QMC5883P MAGNETOMETER DRIVER
> +M:	Hardik Phalet <hardik.phalet@pm.me>
> +L:	linux-iio@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/magnetometer/qstcorp,qmc5883p.yaml
> +
>  QT1010 MEDIA DRIVER
>  L:	linux-media@vger.kernel.org
>  S:	Orphan
> 

Re: [PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P

[Jonathan Cameron]

On Sun, 19 Apr 2026 22:32:09 +0000
Hardik Phalet <hardik.phalet@pm.me> wrote:

> This series adds an IIO driver for the QST QMC5883P, a 3-axis
> anisotropic magneto-resistive (AMR) magnetometer with a 16-bit ADC,
> communicating over I2C. To my knowledge there is no existing
> upstream driver for this device (see "Prior-art register-map check"
> below).
> 
> 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),
>     selectable via IIO_CHAN_INFO_SCALE
>   - Four output data rates (10, 50, 100, 200 Hz), selectable via
>     IIO_CHAN_INFO_SAMP_FREQ
>   - Four oversampling ratios (1, 2, 4, 8), selectable via
>     IIO_CHAN_INFO_OVERSAMPLING_RATIO

I'm suspicious about this one based on a very quick read of the datasheet.
Conventional oversampling would involve running the internal sampling
engine at a multiple of the sampling frequency, and then averaging the
results.  The datasheet describes this as:
"Over sample Rate (OSR1) registers are used to control bandwidth of an
 internal digital filter. Larger OSR value leads to smaller filter bandwidth,
 less in-band noise and higher power consumption. It could be used to reach a
 good balance between noise and power. Four over sample ratios can be selected,
 8,4,2 or 1."

That sounds like a boxcar filter to me not oversampling (which would be
a combination of box car and reducing the output data rate).

If possible, can you enable the data ready output and put a scope on it
to see if that changes frequency when OSR or OSR2 are modified.

Trickier to do would be looking at the noise levels whilst playing with
these filters and see if they at least match with standard filter types.

If we can't figure these out, then it may be a case of picking something
that works well and hard coding that rather than letting userspace
change things in a fashion that might not match the ABI.

>   - Runtime PM with a 2 s autosuspend delay
>   - System suspend/resume delegated to the runtime callbacks

Re: [PATCH v3 1/5] dt-bindings: vendor-prefixes: Add QST Corporation

[Krzysztof Kozlowski]

On Sun, Apr 19, 2026 at 10:32:20PM +0000, Hardik Phalet wrote:
> Add the vendor prefix 'qstcorp' for QST Corporation, a manufacturer of

(and here goes few-word explanation why you have chosen qstcorp or just
the link to the website)

> MEMS sensors.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

Best regards,
Krzysztof

Re: [PATCH v3 2/5] dt-bindings: iio: magnetometer: QSTCORP QMC5883P

[Krzysztof Kozlowski]

On Sun, Apr 19, 2026 at 10:32:32PM +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.
> 
> Add a MAINTAINERS entry for the QSTCORP QMC5883P devicetree binding.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---
>  .../iio/magnetometer/qstcorp,qmc5883p.yaml         | 48 ++++++++++++++++++++++
>  MAINTAINERS                                        |  6 +++
>  2 files changed, 54 insertions(+)

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>

<form letter>
This is an automated instruction, just in case, because many review
tags are being ignored. If you know the process, just skip it entirely
(please do not feel offended by me posting it here - no bad intentions
intended, no patronizing, I just want to avoid wasted efforts). If you
do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions of patchset, under or above your Signed-off-by tag, unless
patch changed significantly (e.g. new properties added to the DT
bindings). Tag is "received", when provided in a message replied to you
on the mailing list. Tools like b4 can help here ('b4 trailers -u ...').
However, there's no need to repost patches *only* to add the tags. The
upstream maintainer will do that for tags received on the version they
apply.

https://elixir.bootlin.com/linux/v6.15/source/Documentation/process/submitting-patches.rst#L591
</form letter>

Best regards,
Krzysztof

Re: [PATCH v3 3/5] iio: magnetometer: add driver for QST QMC5883P

[Jonathan Cameron]

On Sun, 19 Apr 2026 22:32:43 +0000
Hardik Phalet <hardik.phalet@pm.me> 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) selectable
>    via IIO_CHAN_INFO_SCALE
>  - Output data rate configurable via IIO_CHAN_INFO_SAMP_FREQ (10, 50,
>    100, 200 Hz)
>  - vdd-supply regulator management
> 
> 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 300 us for POR
> completion, deassert reset, then drop the register cache so subsequent
> RMW writes read fresh values from the device. After reset the chip is in
> MODE_SUSPEND per datasheet §6.2.4, and is left there; the first
> userspace access will wake it via runtime PM (added in a follow-up
> patch).
> 
> Cleanup is fully devm-managed via devm_regulator_get_enable() and
> devm_iio_device_register().
> 
> Oversampling ratio and runtime PM are added in follow-up patches.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
Hi Hardik,

Welcome to IIO. Various comments inline.

Thanks,

Jonathan

> diff --git a/drivers/iio/magnetometer/Kconfig b/drivers/iio/magnetometer/Kconfig
> index fb313e591e85..333c5e6f231d 100644
> --- a/drivers/iio/magnetometer/Kconfig
> +++ b/drivers/iio/magnetometer/Kconfig
> @@ -298,4 +298,15 @@ config YAMAHA_YAS530
>  	  To compile this driver as a module, choose M here: the module
>  	  will be called yamaha-yas.
>  
> +config QMC5883P
Should be somewhere further up. In theory at least these files are in alphanumeric
order.

> +	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/iio/magnetometer/Makefile b/drivers/iio/magnetometer/Makefile
> index 5bd227f8c120..ff519a055d77 100644
> --- a/drivers/iio/magnetometer/Makefile
> +++ b/drivers/iio/magnetometer/Makefile
> @@ -39,3 +39,5 @@ obj-$(CONFIG_SI7210)			+= si7210.o
>  obj-$(CONFIG_TI_TMAG5273)		+= tmag5273.o
>  
>  obj-$(CONFIG_YAMAHA_YAS530)		+= yamaha-yas530.o
> +
> +obj-$(CONFIG_QMC5883P) += qmc5883p.o
Alphabetical order.

> diff --git a/drivers/iio/magnetometer/qmc5883p.c b/drivers/iio/magnetometer/qmc5883p.c
> new file mode 100644
> index 000000000000..e4a76ae7c2cf
> --- /dev/null
> +++ b/drivers/iio/magnetometer/qmc5883p.c
> @@ -0,0 +1,574 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * qmc5883p.c - QMC5883P magnetometer driver

No point in having the file name in the file.  The only thing that can
do is become wrong in the longer term!

> + *
> + * Copyright 2026 Hardik Phalet <hardik.phalet@pm.me>
> + *
> + * TODO: add triggered buffer support, PM, OSR, DSR
> + *
Blank line above doesn't add anything. Mind you I'm not really
sure driver todo lists add much in general. It's perfectly ok to
never support some of these features.

> + */
> +
> +#include <linux/array_size.h>
> +#include <linux/delay.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/units.h>
> +#include <linux/unaligned.h>
Seems a very short list.  e.g. dev_printk.h is missing.
Give this another look and follow the (unfortunately) rather fuzzy
version of Include What You Use principles.

> +/*
> + * Oversampling rate
Single line comment seems fine here.
Same for other ones above.


> + */
> +#define QMC5883P_OSR_8 0x00
> +#define QMC5883P_OSR_4 0x01
> +#define QMC5883P_OSR_2 0x02
> +#define QMC5883P_OSR_1 0x03

> +
> +struct qmc5883p_data {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct mutex mutex; /* protects regmap and rf field accesses */

Need more on that comment.  Why does it mater?  regmap has it's own internal
locks.  Are there some sequences of accesses that must be done without
anything else in between?

> +	struct qmc5883p_rf rf;
> +};
> +
> +enum qmc5883p_channels {

I don't think the named nature of the enum is used. If not, just make
it anonymous.

> +	AXIS_X = 0,
> +	AXIS_Y,
> +	AXIS_Z,
> +};
>
> +
> +static const struct regmap_config qmc5883p_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = 0x0B,
> +	.cache_type = REGCACHE_RBTREE,

Why this rather than MAPLE?  Unless you have strong reason just
go with REGCACHE_MAPLE so no one asks ;)


> +	.rd_table = &qmc5883p_readable_table,
> +	.wr_table = &qmc5883p_writable_table,
> +	.volatile_table = &qmc5883p_volatile_table,
> +	.precious_table = &qmc5883p_precious_table,
> +};
> +
> +static const struct reg_field qmc5883p_rf_osr =
> +	REG_FIELD(QMC5883P_REG_CTRL_1, 4, 5);
> +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_sftrst =
> +	REG_FIELD(QMC5883P_REG_CTRL_2, 7, 7);
> +static const struct reg_field qmc5883p_rf_chip_id =
> +	REG_FIELD(QMC5883P_REG_CHIP_ID, 0, 7);
> +
> +/*
> + * qmc5883p_get_measure - read all three axes.

I think that bit is obvious from the function signature so I'd drop it.

> + * Must be called with data->mutex held.

Instead of a comment, use markings on the function so the compiler / static
analysis can check it.  Look at __must_hold()  That acts as both a real

check and documentation.


> + */
> +static int qmc5883p_get_measure(struct qmc5883p_data *data, s16 *x, s16 *y,
> +				s16 *z)
> +{
> +	int ret;
> +	u8 reg_data[6];

	__le16 reg_data[3];

and use aligned accessors below.

> +	unsigned int status;
> +
> +	/*
> +	 * 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,
> +				       150 * (MICRO / MILLI));
> +	if (ret)
> +		return ret;
> +
> +	if (status & QMC5883P_STATUS_OVFL) {
> +		dev_warn_ratelimited(data->dev,
> +			"data overflow, consider reducing field range\n");
> +		ret = -ERANGE;
> +		return ret;
	
		return -ERANGE;

> +	}
> +
> +	ret = regmap_bulk_read(data->regmap, QMC5883P_REG_X_LSB, reg_data,
> +			       ARRAY_SIZE(reg_data));
> +	if (ret)
> +		return ret;
> +
> +	*x = (s16)get_unaligned_le16(&reg_data[0]);
> +	*y = (s16)get_unaligned_le16(&reg_data[2]);
> +	*z = (s16)get_unaligned_le16(&reg_data[4]);
> +
> +	return ret;
> +}
> +
> +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;

When nothing else gives an order to definitions go with reverse
xmas tree. 

> +
> +	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_INT_PLUS_NANO;
> +
> +	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;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +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);

Another sashiko one I might have missed.  Given there is only one field
for all channels, is having per channel scale appropriate?

Under the ABI it's not wrong to do that, as any ABI element being modified
is allowed to modify any other (that's there to handle complex interactions)
but it might not be the best we can do wrt to a user friendly interface.


> +	}
> +
> +	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 = regmap_field_write(data->rf.mode, QMC5883P_MODE_SUSPEND);

Sashiko was complaining about this and what mode we are in after reset
before calling this function. See below.  I think the solution
is probably to put it in normal mode in probe() and then deal with this
stuff as runtime pm later.


> +	if (ret)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		ret = qmc5883p_write_odr(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;
> +
> +	return ret;
> +}
> +
> +/*
> + * qmc5883p_read_avail - expose available values to userspace.
> + *
> + * Creates the _available sysfs attributes automatically:
> + *   in_magn_sampling_frequency_available
> + *   in_magn_scale_available

Probably standard enough for an IIO driver to not need a comment
that is more about what the core code is doing with this than what
this function does.

I'd drop the comment. It is the sort of thing that will become out
of date as a driver evolves and doesn't bring enough value to make
that risk worthwhile.

> + */
> +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_SCALE:
> +		*vals = (const int *)qmc5883p_scale;
> +		*type = IIO_VAL_INT_PLUS_NANO;
> +		*length = ARRAY_SIZE(qmc5883p_scale) * 2;
> +		return IIO_AVAIL_LIST;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +/*
> + * Tell the IIO core how to parse sysfs writes. Without this, the core
> + * defaults to IIO_VAL_INT_PLUS_MICRO (6 fractional digits), which would
> + * silently truncate nano-scale writes like "0.000000040" to 0.

Not sure this comment is really needed given this is a
pretty standard IIO thing.

> + */
> +static int qmc5883p_write_raw_get_fmt(struct iio_dev *indio_dev,
> +				      struct iio_chan_spec const *chan,
> +				      long mask)
> +{
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		return IIO_VAL_INT;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info qmc5883p_info = {
> +	.read_raw = qmc5883p_read_raw,
> +	.write_raw = qmc5883p_write_raw,
> +	.write_raw_get_fmt = qmc5883p_write_raw_get_fmt,
> +	.read_avail = qmc5883p_read_avail,
> +};
> +
> +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);

Might be worth looking at whether you can use
devm_regmap_field_bulk_alloc() without impacting the code readability too
much. I'm fine with this if that option looks too ugly by needing an
enum to index the fields.  Main positive is it would avoid the
repetition in this function.



> +	if (IS_ERR(rf->osr))
> +		return PTR_ERR(rf->osr);
> +
> +	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->sftrst = devm_regmap_field_alloc(dev, regmap, qmc5883p_rf_sftrst);
> +	if (IS_ERR(rf->sftrst))
> +		return PTR_ERR(rf->sftrst);
> +
> +	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;
> +}
> +

> +
> +#define QMC5883P_CHAN(ch)                                                 \
> +	{                                                                 \
> +		.type = IIO_MAGN,                                         \
> +		.channel2 = IIO_MOD_##ch,                                 \
> +		.modified = 1,                                            \
> +		.address = AXIS_##ch,                                     \
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |            \
> +				      BIT(IIO_CHAN_INFO_SCALE),           \
> +		.info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE), \

See above, whether scale is really separate of should be in shared_by_type.


> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +		.info_mask_shared_by_type_available =                     \
> +			BIT(IIO_CHAN_INFO_SAMP_FREQ),                     \
> +	}
> +
> +static const struct iio_chan_spec qmc5883p_channels[] = {
> +	QMC5883P_CHAN(X),
> +	QMC5883P_CHAN(Y),
> +	QMC5883P_CHAN(Z),
> +};
> +
> +static int qmc5883p_chip_init(struct qmc5883p_data *data)
> +{
> +	int ret;
> +
> +	ret = regmap_field_write(data->rf.sftrst, 1);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * The datasheet does not specify a post-reset delay, but POR
> +	 * completion takes up to 250 microseconds. Use 300 microseconds
> +	 * to be safe.

Very minor, but wrap comments up to 80 chars. That gives us:
(my email client has a ruler which makes these easy to spot!)

	 * The datasheet does not specify a post-reset delay, but POR completion
	 * takes up to 250 microseconds. Use 300 microseconds to be safe.
> +	 */
> +	fsleep(300);
> +
> +	ret = regmap_field_write(data->rf.sftrst, 0);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Soft reset restored every register to its default. Drop the cache
> +	 * so subsequent RMW writes read fresh values from the device.
> +	 */
> +	regcache_drop_region(data->regmap, QMC5883P_REG_CHIP_ID,
> +			     QMC5883P_REG_CTRL_2);
> +
> +	/* Chip is now in MODE_SUSPEND per datasheet §6.2.4. Leave it there. */

As below, sashiko raises concern that we can't read if if suspend mode so after
this patch the driver doesn't function right.  I haven't looked into it in detail
but sounds plausible.

> +	return 0;
> +}
> +
> +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);
> +	i2c_set_clientdata(client, indio_dev);

Add this when it is used. I'd guess in the runtime pm patch.
Note that shashiko raised some concerns about bisectibility and they
make me wonder if there are problems if runtime pm is simply disabled.
To handle that we normally make sure the device is in whatever mode is
needed to take a reading before we turn on runtime pm.

https://sashiko.dev/#/patchset/20260420-qmc5883p-driver-v3-0-da1e97088f8b%40pm.me

(Note this wasn't running on linux-iio until a few days ago so your
earlier versions weren't covered!)

> +	data->dev = dev;
> +	data->regmap = regmap;
> +
> +	mutex_init(&data->mutex);
For new code prefer
	ret = devm_mutex_init(dev, &data->mutex);
	if (ret)
		return ret;

It brings minor benefits for lock debugging hence I'm not inclined
to update old code, but we might as well use best practice in
new code.

> +
> +	ret = qmc5883p_rf_init(data);
> +	if (ret)
...


> +static const struct of_device_id qmc5883p_of_match[] = {
> +	{ .compatible = "qstcorp,qmc5883p" },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, qmc5883p_of_match);
> +
> +static const struct i2c_device_id qmc5883p_id[] = {
> +	{ "qmc5883p", 0 },

	{ "qmc5883p" },

Is enough.  Under the C spec .driver_data gets set to 0 anyway
but more important is it isn't used, so we shouldn't set it.  Any change
to add more parts will require changing that line anyway so we aren't
saving on future churn by setting it.

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

Re: [PATCH v3 4/5] iio: magnetometer: qmc5883p: add oversampling ratio support

[Jonathan Cameron]

On Sun, 19 Apr 2026 22:32:56 +0000
Hardik Phalet <hardik.phalet@pm.me> wrote:

> Expose the CTRL_1 OSR field through IIO_CHAN_INFO_OVERSAMPLING_RATIO so
> userspace can select among the four oversampling settings (1, 2, 4, 8)
> supported by the device. Read, write and available handlers mirror the
> existing SAMP_FREQ plumbing and use the already-present rf.osr regmap
> field.
> 
> Signed-off-by: Hardik Phalet <hardik.phalet@pm.me>
> ---

> @@ -306,6 +320,18 @@ static int qmc5883p_write_odr(struct qmc5883p_data *data, int val)
>  	return -EINVAL;
>  }
>  
> +static int qmc5883p_write_osr(struct qmc5883p_data *data, int val)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(qmc5883p_osr); i++) {

Whilst a fairly recent thing, it is now considered fine to do
	for (int i = 0; i < ... 

> +		if (qmc5883p_osr[i] == val)
> +			return regmap_field_write(data->rf.osr, i);
> +	}
> +
> +	return -EINVAL;
> +}