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

[me]

On 2026-06-05 19:52, Andy Shevchenko wrote:
> On Fri, Jun 05, 2026 at 12:08:43PM +0200, Herman van Hazendonk 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-mg 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 driver
> > tolerates an unknown / unsupported value by falling back to the chip
> > default and warning, so the property is purely additive -- existing
> > in-tree DTSes are unaffected.
> >
> > Per-sensor mg ranges are listed in st_magn_sensors_settings[]. For
> > LSM303DLH the valid values are 1300, 1900, 2500, 4000, 4700, 5600
> > and 8100; for LSM303DLHC they are 1300, 1900, 2500, 4000, 4700, 5600,
> > 8100 (same code path); for LIS3MDL they are 4000, 8000, 12000, 16000;
> > and so on. Sensors with a fixed full-scale (fs.addr == 0) simply
> > ignore the property.
> >
> > 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.
>
> ...
>
> > +	{
>
> Oh, no. Use
>
> 	const char *propname;
> 	...
> 	propname = "st,fullscale-mg";
> 	if (device_property_present(..., propname)) {
> 		struct st_sensor_fullscale *fs = &mdata->sensor_settings->fs;
> 		u32 fs_mg;
>
> 		ret = device_property_read_u32(parent, propname, &fs_mg);
> 		if (ret)
> 			return ret;
>
> 		...
> 	}
>
> instead.

Hi Andy,

Done in v2: propname lifted to a local, presence check first, 
device_property_read_u32()
failure now return errs rather than silently skipping, and the dev_warn 
format string uses
%s, propname so the name is in exactly one place.
> > +		u32 fs_mg;
>
> > +		if (!device_property_read_u32(parent, "st,fullscale-mg",
> > +					      &fs_mg)) {
> > +			struct st_sensor_fullscale *fs =
> > +				&mdata->sensor_settings->fs;
> > +			int i;
> > +
> > +			for (i = 0; i < ST_SENSORS_FULLSCALE_AVL_MAX; i++) {
> > +				if (!fs->fs_avl[i].num)
> > +					break;
>
> This is strange. What's the point to go via the whole table? Does it 
> have gaps?
>
> > +				if (fs->fs_avl[i].num == fs_mg) {
> > +					mdata->current_fullscale =
> > +						&fs->fs_avl[i];
> > +					break;
> > +				}
> > +			}
> > +			if (mdata->current_fullscale->num != fs_mg)
> > +				dev_warn(parent,
> > +					 "st,fullscale-mg=%u not supported, using %u\n",
>
> Also use %s and propname.
>
> > +					 fs_mg, mdata->current_fullscale->num);
> > +		}
> > +	}
It's not gaps: fs_avl[] is a fixed-size array of 
ST_SENSORS_FULLSCALE_AVL_MAX (currently 8)
entries, but different magnetometers fill in different prefix lengths up 
to that cap.
LSM303DLH uses 7 entries; LSM303DLHC uses 7; LIS3MDL only uses 4.

The if (!fs->fs_avl[i].num) break; is the existing sentinel convention 
used elsewhere in this
file (see st_sensors_set_fullscale_by_gain() / st_sensors_match_fs()) so 
the loop terminates at
the first zero .num. I kept the same convention so future sensors with 
shorter ranges just work;
happy to switch to a ARRAY_SIZE() + is_terminator() helper if you'd 
prefer, but that would be a
wider cleanup than this patch warrants.

Thanks,
Herman