Re: [PATCH v2 2/2] iio: light: isl29018: support cover-glass gain compensation via DT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Truncating the intermediate lux value before applying the calibration scale (`cover-comp-gain`) causes massive precision loss, rendering the sensor wildly inaccurate for large gains.
--

commit 602cf7dfe3433fd4740e367b31d5116c92d21c70
Author: Herman van Hazendonk <github.com@herrie.org>

iio: light: isl29018: support cover-glass gain compensation via DT

This commit adds a new device-tree property isil,cover-comp-gain to the
isl29018 sensor driver. It allows initializing the calibscale parameter
directly from firmware, compensating for optical loss when the sensor is
mounted behind tinted cover glass.

> diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
> index 927018c531e8d..1d6a011c92625 100644
> --- a/drivers/iio/light/isl29018.c
> +++ b/drivers/iio/light/isl29018.c
[ ... ]
> @@ -728,7 +728,16 @@ static int isl29018_probe(struct i2c_client *client)
>  	mutex_init(&chip->lock);
>  
>  	chip->type = dev_id;
> +	/*
> +	 * Allow boards that mount the sensor behind tinted / coated cover
> +	 * glass to bake the optical-loss compensation into firmware via
> +	 * "isil,cover-comp-gain", following the precedent set by
> +	 * tsl2563.c. The value seeds calibscale (default 1), so userspace
> +	 * can still retune through in_illuminance0_calibscale.
> +	 */
>  	chip->calibscale = 1;
> +	device_property_read_u32(&client->dev, "isil,cover-comp-gain",
> +				 &chip->calibscale);

[Severity: High]
This is a pre-existing issue, but adding the isil,cover-comp-gain
device-tree property encourages using large calibration gains from boot, which
exposes an existing flaw.

In isl29018_read_lux(), the division by 1000000 drops the fractional part of
the base lux reading before applying the calibration scale:

drivers/iio/light/isl29018.c:isl29018_read_lux() {
    ...
    data_x_range = (u64)lux_data * chip->scale.scale +
                   (u64)lux_data * chip->scale.uscale / 1000000;
    result = data_x_range * chip->calibscale +
             data_x_range * chip->ucalibscale / 1000000;
    ...
}

With a dark cover glass requiring a large gain (for example, 100), the sensor
operates at very low raw lux levels. Dropping the fractional part before
multiplying by calibscale causes all raw readings below 1 lux to truncate to
0, resulting in an output of 0 lux.

When the raw reading reaches 1.0, the output will abruptly jump to 100 lux.

Could the division by 1000000 be delayed until after the calibscale
multiplication to preserve accuracy?

>  	chip->ucalibscale = 0;
>  	chip->int_time = ISL29018_INT_TIME_16;
>  	chip->scale = isl29018_scales[chip->int_time][0];

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604064925.3097108-1-github.com@herrie.org?part=2