Re: [PATCH v2 3/3] iio: magnetometer: st_magn: honour st,fullscale-milligauss DT property

[Jonathan Cameron]

On Tue, 16 Jun 2026 15:02:06 +0200
Herman van Hazendonk <github.com@xxxxxxxxxx> wrote:

> The ST magnetometer core's common probe hardcodes fs_avl[0] -- the
> highest-sensitivity full-scale supported by the chip -- as the
> starting range. For the LSM303DLH that is +/-1.3 G; for the
> LSM303DLHC and LSM303DLM it is +/-2 G; for the LIS3MDL it is +/-4 G.
> 
> That is the right default for "minimal noise floor at a desk", but
> it leaves no margin for boards that pick up appreciable DC bias from
> nearby PCB structures. On the HP TouchPad (apq8060 / tenderloin) the
> LSM303DLH magnetometer is mounted close enough to the surrounding
> power planes that X reads back as the chip's 0xF000 overflow
> sentinel (== -4096 raw, the value the chip publishes when the ADC
> saturates) on every sample at the chip-default range, while Y and Z
> fall well within the +/-1.3 G window.
> 
> Parse the st,fullscale-milligauss device-tree property (documented
> separately in dt-bindings/iio/st,st-sensors.yaml) in the
> magnetometer common probe to select the initial fs_avl entry by its
> mg value. The DT binding pins the accepted value set per compatible
> via allOf/if-then enum clauses, so a malformed mg value fails
> dt_binding_check rather than reaching the driver. Sensors with a
> fixed full-scale (fs.addr == 0: LSM303AGR, LIS2MDL, IIS2MDC) have no
> register to switch and the property is rejected outright for them
> in the binding; the parse block is additionally gated on fs.addr as
> defence in depth against stale DTBs.
> 
> Per-sensor mg ranges are listed in st_magn_sensors_settings[]. For
> LSM303DLH and LSM303DLHC/DLM the valid values are 1300, 1900, 2500,
> 4000, 4700, 5600 and 8100; for LIS3MDL, LSM9DS1-magn and LSM303C-magn
> they are 4000, 8000, 12000, 16000.
> 
> Empirical scale sweep on the HP TouchPad confirmed that on this
> board any fs_avl >= 1 produces non-saturated X readings:
> 
>     scale (0.001 G/LSB)  | X raw    Y raw    Z raw
>     --------------------+-------------------------------
>             1.100        | -4096    44       46    (X saturated)
>             0.855        |  -547    37       37    (clean)
>             0.670        |  -433    94      103    (clean)
>             0.450        |  -266    44       71    (clean)
>             0.400        |  -235    34       65    (clean)
>             0.330        |  -196    27       56    (clean)
>             0.230        |  -145    15       40    (clean)
> 
> 2500 mg is the natural choice for tenderloin: comfortably outside
> the saturation regime while keeping useful precision for compass
> applications.
> 
> Assisted-by: Claude:claude-opus-4-7 sparse smatch clang-analyzer coccinelle checkpatch
> Assisted-by: Sashiko:claude-opus-4-7
Hmm. First time I remember seeing Sashiko credited like this. Seems like pretty much
every patch series of any complexity would end up crediting sashiko.
Out of curiosity were you just looking at reports, or were you running it locally to
help with development?

One thing inline.
> Signed-off-by: Herman van Hazendonk <github.com@xxxxxxxxxx>
> ---
>  drivers/iio/magnetometer/st_magn_core.c | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
> 
> diff --git a/drivers/iio/magnetometer/st_magn_core.c b/drivers/iio/magnetometer/st_magn_core.c
> index ef348d316c00..6f369e8dddea 100644
> --- a/drivers/iio/magnetometer/st_magn_core.c
> +++ b/drivers/iio/magnetometer/st_magn_core.c
> @@ -10,6 +10,7 @@
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
> +#include <linux/property.h>
>  #include <linux/sysfs.h>
>  #include <linux/iio/iio.h>
>  #include <linux/iio/sysfs.h>
> @@ -608,6 +609,7 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
>  	struct st_sensor_data *mdata = iio_priv(indio_dev);
>  	struct device *parent = indio_dev->dev.parent;
>  	struct st_sensors_platform_data *pdata = dev_get_platdata(parent);
> +	const char *propname;
>  	int err;
>  
>  	indio_dev->modes = INDIO_DIRECT_MODE;
> @@ -628,6 +630,36 @@ int st_magn_common_probe(struct iio_dev *indio_dev)
>  	mdata->current_fullscale = &mdata->sensor_settings->fs.fs_avl[0];
>  	mdata->odr = mdata->sensor_settings->odr.odr_avl[0].hz;
>  
> +	/*
> +	 * Skip fixed-FS chips (fs.addr == 0): no register to switch.
> +	 * The binding rejects the property on these compatibles too;
> +	 * the gate guards stale DTBs.
Isn't it optional?  If so they aren't necessarily stale, people
may have relied on the default - which I now notice isn't specified
in the dt-binding (just replied to that patch).

> +	 */
> +	propname = "st,fullscale-milligauss";
> +	if (mdata->sensor_settings->fs.addr &&
> +	    device_property_present(parent, propname)) {