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

[Hardik Phalet]

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

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

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

Regards,
Hardik