* [PATCH v2 0/4] Add QST QMC5883P magnetometer driver
@ 2026-04-09 21:07 Hardik Phalet
2026-04-09 21:07 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Hardik Phalet @ 2026-04-09 21:07 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell,
Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging,
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet @ 2026-04-09 21:07 ` Hardik Phalet 2026-04-10 7:52 ` Krzysztof Kozlowski 2026-04-09 21:07 ` [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P Hardik Phalet ` (4 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Hardik Phalet @ 2026-04-09 21:07 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation 2026-04-09 21:07 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet @ 2026-04-10 7:52 ` Krzysztof Kozlowski 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2026-04-10 7:52 UTC (permalink / raw) To: Hardik Phalet Cc: Greg Kroah-Hartman, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet 2026-04-09 21:07 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet @ 2026-04-09 21:07 ` Hardik Phalet 2026-04-10 7:55 ` Krzysztof Kozlowski 2026-04-09 21:07 ` [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver Hardik Phalet ` (3 subsequent siblings) 5 siblings, 1 reply; 13+ messages in thread From: Hardik Phalet @ 2026-04-09 21:07 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P 2026-04-09 21:07 ` [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P Hardik Phalet @ 2026-04-10 7:55 ` Krzysztof Kozlowski 2026-04-10 8:06 ` Krzysztof Kozlowski 0 siblings, 1 reply; 13+ messages in thread From: Krzysztof Kozlowski @ 2026-04-10 7:55 UTC (permalink / raw) To: Hardik Phalet Cc: Greg Kroah-Hartman, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P 2026-04-10 7:55 ` Krzysztof Kozlowski @ 2026-04-10 8:06 ` Krzysztof Kozlowski 0 siblings, 0 replies; 13+ messages in thread From: Krzysztof Kozlowski @ 2026-04-10 8:06 UTC (permalink / raw) To: Hardik Phalet Cc: Greg Kroah-Hartman, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet 2026-04-09 21:07 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet 2026-04-09 21:07 ` [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P Hardik Phalet @ 2026-04-09 21:07 ` Hardik Phalet 2026-04-10 5:28 ` Greg Kroah-Hartman 2026-04-10 20:02 ` David Lechner 2026-04-09 21:07 ` [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver Hardik Phalet ` (2 subsequent siblings) 5 siblings, 2 replies; 13+ messages in thread From: Hardik Phalet @ 2026-04-09 21:07 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging, 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, ®val); + 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(®_data[0]); + *y = (s16)get_unaligned_le16(®_data[2]); + *z = (s16)get_unaligned_le16(®_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, ®val); + 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, ®val); + 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, ®val); + 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, ®val); + 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver 2026-04-09 21:07 ` [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver Hardik Phalet @ 2026-04-10 5:28 ` Greg Kroah-Hartman 2026-04-10 20:02 ` David Lechner 1 sibling, 0 replies; 13+ messages in thread From: Greg Kroah-Hartman @ 2026-04-10 5:28 UTC (permalink / raw) To: Hardik Phalet Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver 2026-04-09 21:07 ` [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver Hardik Phalet 2026-04-10 5:28 ` Greg Kroah-Hartman @ 2026-04-10 20:02 ` David Lechner 1 sibling, 0 replies; 13+ messages in thread From: David Lechner @ 2026-04-10 20:02 UTC (permalink / raw) To: Hardik Phalet, Greg Kroah-Hartman Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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, ®val); > + 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(®_data[0]); > + *y = (s16)get_unaligned_le16(®_data[2]); > + *z = (s16)get_unaligned_le16(®_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, ®val); > + 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, ®val); > + 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, ®val); > + 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, ®val); > + 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); > + ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet ` (2 preceding siblings ...) 2026-04-09 21:07 ` [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver Hardik Phalet @ 2026-04-09 21:07 ` Hardik Phalet 2026-04-10 19:32 ` David Lechner 2026-04-10 4:36 ` [PATCH v2 0/4] Add " Andy Shevchenko 2026-04-10 19:26 ` David Lechner 5 siblings, 1 reply; 13+ messages in thread From: Hardik Phalet @ 2026-04-09 21:07 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver 2026-04-09 21:07 ` [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver Hardik Phalet @ 2026-04-10 19:32 ` David Lechner 0 siblings, 0 replies; 13+ messages in thread From: David Lechner @ 2026-04-10 19:32 UTC (permalink / raw) To: Hardik Phalet, Greg Kroah-Hartman Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet ` (3 preceding siblings ...) 2026-04-09 21:07 ` [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver Hardik Phalet @ 2026-04-10 4:36 ` Andy Shevchenko 2026-04-10 19:26 ` David Lechner 5 siblings, 0 replies; 13+ messages in thread From: Andy Shevchenko @ 2026-04-10 4:36 UTC (permalink / raw) To: Hardik Phalet Cc: Greg Kroah-Hartman, Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 0/4] Add QST QMC5883P magnetometer driver 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet ` (4 preceding siblings ...) 2026-04-10 4:36 ` [PATCH v2 0/4] Add " Andy Shevchenko @ 2026-04-10 19:26 ` David Lechner 5 siblings, 0 replies; 13+ messages in thread From: David Lechner @ 2026-04-10 19:26 UTC (permalink / raw) To: Hardik Phalet, Greg Kroah-Hartman Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley, Brigham Campbell, Shuah Khan, linux-iio, devicetree, linux-kernel, linux-staging 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-04-10 20:02 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-04-09 21:07 [PATCH v2 0/4] Add QST QMC5883P magnetometer driver Hardik Phalet 2026-04-09 21:07 ` [PATCH v2 1/4] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet 2026-04-10 7:52 ` Krzysztof Kozlowski 2026-04-09 21:07 ` [PATCH v2 2/4] dt-bindings: iio: magnetometer: Add binding for QST QMC5883P Hardik Phalet 2026-04-10 7:55 ` Krzysztof Kozlowski 2026-04-10 8:06 ` Krzysztof Kozlowski 2026-04-09 21:07 ` [PATCH v2 3/4] staging: iio: magnetometer: Add QST QMC5883P driver Hardik Phalet 2026-04-10 5:28 ` Greg Kroah-Hartman 2026-04-10 20:02 ` David Lechner 2026-04-09 21:07 ` [PATCH v2 4/4] MAINTAINERS: Add entry for QST QMC5883P magnetometer driver Hardik Phalet 2026-04-10 19:32 ` David Lechner 2026-04-10 4:36 ` [PATCH v2 0/4] Add " Andy Shevchenko 2026-04-10 19:26 ` David Lechner
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox