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

[me@herrie.org]

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