Devicetree
 help / color / mirror / Atom feed
From: Hardik Phalet <hardik.phalet@pm.me>
To: Jonathan Cameron <jic23@kernel.org>, Hardik Phalet <hardik.phalet@pm.me>
Cc: gregkh@linuxfoundation.org, andy@kernel.org, conor+dt@kernel.org,
	devicetree@vger.kernel.org, dlechner@baylibre.com,
	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 19:17:44 +0000	[thread overview]
Message-ID: <DIL6ZC7KKYMN.36KGJXKDRGFB0@pm.me> (raw)
In-Reply-To: <20260420144534.0e47c06a@jic23-huawei>

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.

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.
  (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 19:17 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 [this message]
2026-05-17 20:28     ` David Lechner
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=DIL6ZC7KKYMN.36KGJXKDRGFB0@pm.me \
    --to=hardik.phalet@pm.me \
    --cc=andy@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dlechner@baylibre.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hardik.phalet@gmail.com \
    --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