Devicetree
 help / color / mirror / Atom feed
From: David Lechner <dlechner@baylibre.com>
To: Hardik Phalet <hardik.phalet@pm.me>, Jonathan Cameron <jic23@kernel.org>
Cc: gregkh@linuxfoundation.org, andy@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, krzk+dt@kernel.org,
	linux-iio@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-staging@lists.linux.dev, me@brighamcampbell.com,
	nuno.sa@analog.com, robh@kernel.org, skhan@linuxfoundation.org,
	Hardik Phalet <hardik.phalet@gmail.com>
Subject: Re: [PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P
Date: Sun, 17 May 2026 15:28:22 -0500	[thread overview]
Message-ID: <166a6262-0ad4-4957-9729-863e2694e81d@baylibre.com> (raw)
In-Reply-To: <DIL6ZC7KKYMN.36KGJXKDRGFB0@pm.me>

On 5/17/26 2:17 PM, Hardik Phalet wrote:
> On Mon Apr 20, 2026 at 7:15 PM IST, Jonathan Cameron wrote:
>> 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.
> 
> I could not arrange a oscilloscope. So wrote a small standalone tool that drives
> the chip directly over /dev/i2c-N (skipping the driver) on a Pi 4 and
> characterised all three fields in CTRL_1 with timing + noise measurements.
> Summary:
> 
>   Field    Bits     Role per measurement
>   ODR      [3:2]    output rate, accurate to ~3% of nominal in Normal Mode
>   OSR1     [5:4]    low-pass filter (bandwidth control), no rate change
>   OSR2     [7:6]    N-sample averaging, no rate change (real oversampling)
> 
> 1. DRDY interval vs each field (median of 500 samples per setting,
>    ms; SCHED_FIFO, pinned CPU):
> 
>    vary ODR (OSR1=1, OSR2=1):
>      ODR=10 Hz  -> 100.9 ms       ODR=100 Hz -> 10.30 ms
>      ODR=50 Hz  ->  20.5 ms       ODR=200 Hz ->  5.14 ms
> 
>    vary OSR1 (ODR=200 Hz, OSR2=1):
>      OSR1=1 -> 5.142 ms           OSR1=4 -> 5.139 ms
>      OSR1=2 -> 5.143 ms           OSR1=8 -> 5.141 ms
> 
>    vary OSR2 (ODR=200 Hz, OSR1=1):
>      OSR2=1 -> 5.142 ms           OSR2=4 -> 5.142 ms
>      OSR2=2 -> 5.140 ms           OSR2=8 -> 5.141 ms
> 
>    ODR is the only knob that changes the rate.
> 
> 2. Per-axis noise stddev (2000 samples, sensor still, ratios vs N=1):
> 
>    vary OSR1:                   vary OSR2:
>      OSR1=1 -> 1.000              OSR2=1 -> 1.000
>      OSR1=2 -> 0.723              OSR2=2 -> 0.706
>      OSR1=4 -> 0.544              OSR2=4 -> 0.494
>      OSR1=8 -> 0.416              OSR2=8 -> 0.333
> 
>    Reference for N-sample averaging (1/sqrt(N)):
>      1.000 / 0.707 / 0.500 / 0.354
> 
>    OSR2 matches within measurement noise; OSR1 trails the curve,
>    which is the expected signature of a bandwidth-limiting filter
>    on non-white input noise.
> 
> So to my understanding, OSR2 is true oversampling. OSR1 is a filter.

The datasheet says that power consumption changes based on OSR1,
so that makes me think that OSR1 is really oversampling since it
would take more energy to do more conversions in the same amount
of time. (Note 1 on table 2.1)

Another chip by the same mfg that looks like it has the same OSR1/
OSR2 has a bit more info in the same table [1]. It looks like OSR2
affects the sensitivity (and calls OSR2 a low pass filter instead
of "down sampling rate").

[1] https://www.qstcorp.com/upload/pdf/202601/CF382A94E1424763B3DE87DC967757FC.pdf

> 
> v4 plan, pending your input on (c):
> 
>   (a) Add an OSR2 regmap_field and expose it as
>       IIO_CHAN_INFO_OVERSAMPLING_RATIO with values {1, 2, 4, 8}.
>   (b) Drop the OVERSAMPLING_RATIO mapping on OSR1.
>   (c) For OSR1, two options:
>         i)  expose as IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY.
>             The public datasheet does not give cutoff frequencies,
>             so I'd have to either characterise them empirically and
>             document them in the driver, or use placeholder values
>             derived from the OSR1 setting alone (which feels wrong).
>         ii) hard-code OSR1=1 in chip_init and not expose it. Loses
>             the noise-vs-bandwidth tradeoff but keeps the ABI clean.
>       Slight preference for (ii) unless you'd rather see the filter
>       knob exposed.

It sounds like we already figured out that OSR2 is just a moving average
so might as well implement IIO_CHAN_INFO_LOW_PASS_FILTER_3DB_FREQUENCY.

>   (d) Keep ODR mapped to IIO_CHAN_INFO_SAMP_FREQ as in v3.
> 
> One other finding worth mentioning: the datasheet's setup examples
> (sec 7.1, 7.2, 7.3) all write 0x06 to register 0x29 before entering
> an active mode, with the comment "Define the sign for X Y and Z axis".
> This register is not listed in the documented register map but the
> write is required for axes Y and Z to come out with the documented
> orientation. v4 will add this write to chip_init().
> 
> If you have other measurements you'd like to see before v4, happy to
> run them.
> 
>>
>>>   - Runtime PM with a 2 s autosuspend delay
>>>   - System suspend/resume delegated to the runtime callbacks
> 
> Best regards,
> Hardik


  reply	other threads:[~2026-05-17 20:28 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-19 22:32 [PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P Hardik Phalet
2026-04-19 22:32 ` [PATCH v3 1/5] dt-bindings: vendor-prefixes: Add QST Corporation Hardik Phalet
2026-04-20 14:08   ` Krzysztof Kozlowski
2026-04-19 22:32 ` [PATCH v3 2/5] dt-bindings: iio: magnetometer: QSTCORP QMC5883P Hardik Phalet
2026-04-20 13:37   ` Jonathan Cameron
2026-04-20 14:10   ` Krzysztof Kozlowski
2026-04-19 22:32 ` [PATCH v3 3/5] iio: magnetometer: add driver for QST QMC5883P Hardik Phalet
2026-04-20  9:43   ` Andy Shevchenko
2026-04-20 14:22   ` Jonathan Cameron
2026-04-19 22:32 ` [PATCH v3 4/5] iio: magnetometer: qmc5883p: add oversampling ratio support Hardik Phalet
2026-04-20  9:45   ` Andy Shevchenko
2026-04-20 14:25   ` Jonathan Cameron
2026-04-19 22:33 ` [PATCH v3 5/5] iio: magnetometer: qmc5883p: add PM support Hardik Phalet
2026-04-20  9:52   ` Andy Shevchenko
2026-04-20  9:18 ` [PATCH v3 0/5] iio: magnetometer: add driver for QST QMC5883P Andy Shevchenko
2026-04-20 13:45 ` Jonathan Cameron
2026-05-17 19:17   ` Hardik Phalet
2026-05-17 20:28     ` David Lechner [this message]
2026-05-18 13:22       ` Jonathan Cameron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=166a6262-0ad4-4957-9729-863e2694e81d@baylibre.com \
    --to=dlechner@baylibre.com \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=hardik.phalet@gmail.com \
    --cc=hardik.phalet@pm.me \
    --cc=jic23@kernel.org \
    --cc=krzk+dt@kernel.org \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=me@brighamcampbell.com \
    --cc=nuno.sa@analog.com \
    --cc=robh@kernel.org \
    --cc=skhan@linuxfoundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox