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

PATCH Superseded Herman van Hazendonk 5 participants 11 messages Jun 4 – Jun 8

Subsystems (2)

Device tree bindings IIO

lore

Share & tools

lore

Herman van Hazendonk

author

2026-06-04 05:47 UTC #0 lore raw

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

The ISL29018/ISL29023 is commonly mounted behind a tinted or coated cover glass on consumer devices (HP TouchPad being one example). In that configuration the visible-light photodiode reads roughly 1/100th of actual ambient lux, causing downstream consumers (brightness daemons, display managers) to permanently classify the environment as near-dark and pin the backlight at a fraction of its range.

The driver already exposes in_illuminance0_calibscale for runtime compensation, but that requires udev rules or userspace re-application after every reboot. The optical loss is a board-level hardware constant and belongs in firmware.

This series adds an "isil,cover-comp-gain" DT property that seeds calibscale at probe time, following the precedent established by tsl2563.c (amstaos,cover-comp-gain) for the same class of problem. The default stays 1 so existing systems are unaffected, and userspace can still override the value through sysfs afterwards.

A follow-up DTS patch enabling this for the HP TouchPad will be sent separately to the ARM/DTS tree once this driver change is upstream.

Herman van Hazendonk (1):
iio: light: isl29018: support cover-glass gain compensation via DT

 .../devicetree/bindings/iio/light/isl29018.yaml     | 13 +++++++++++++
 drivers/iio/light/isl29018.c                        |  9 +++++++++
 2 files changed, 22 insertions(+)

Signature · 1 line

2.43.0

Herman van Hazendonk

author

2026-06-04 05:47 UTC #1 lore raw

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

Boards that mount the sensor under a tinted or coated cover glass need to compensate for the optical loss before downstream consumers can map the reading onto a useful lux range. The driver already exposes a runtime knob through in_illuminance0_calibscale, but every user has to re-apply it after every reboot (or rely on a board-specific udev rule), and a power-of-two cover gain like 100x is a hardware constant of the board rather than a policy choice that belongs in userspace.

Add an "isil,cover-comp-gain" device-tree property that seeds calibscale at probe, mirroring the pattern tsl2563.c already uses for the same class of problem (amstaos,cover-comp-gain). The default stays 1 so existing systems are unaffected, and userspace can still re-tune via the sysfs attribute afterwards.

Signed-off-by Herman van Hazendonk <[email protected]>

---
 .../devicetree/bindings/iio/light/isl29018.yaml     | 13 +++++++++++++
 drivers/iio/light/isl29018.c                        |  9 +++++++++
 2 files changed, 22 insertions(+)

Documentation/devicetree/bindings/iio/light/isl29018.yaml +13 -0

index 0ea278b07d1c..92ea2742bbd3 100644
@@ -34,6 +34,19 @@ properties:
   vcc-supply:
     description: Regulator that provides power to the sensor
 
+  isil,cover-comp-gain:
+    description: |
+      Multiplier applied to the ambient-light reading at startup to
+      compensate for optical loss in the board's cover glass. Boards
+      that mount the sensor under a tinted or coated window typically
+      need a value between a few and a few hundred. The value seeds
+      in_illuminance0_calibscale, so it can still be retuned at
+      runtime through sysfs.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 65535
+    default: 1
+
 required:
   - compatible
   - reg

drivers/iio/light/isl29018.c +9 -0

index b6ab726d1dae..427dbb639141 100644
@@ -727,7 +727,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);
 	chip->ucalibscale = 0;
 	chip->int_time = ISL29018_INT_TIME_16;
 	chip->scale = isl29018_scales[chip->int_time][0];

Signature · 1 line

2.43.0

[email protected]

AI review

2026-06-04 05:57 UTC #2 lore raw

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

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

Pre-existing issues:
- [High] Multiple 32-bit integer overflows in isl29018_read_lux() lead to incorrect, wrapped, or negative sensor readings. --

commit 190a907a89f0aeaf207c50457cf9f79e29e324c2
Author: Herman van Hazendonk <[email protected]>

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

This commit introduces a new device-tree property, "isil,cover-comp-gain", to seed the in_illuminance0_calibscale value at probe time. This allows hardware platforms to compensate for optical loss from tinted or coated cover glass over the sensor. The default value is set to 1 to maintain compatibility with existing systems.

[ ... ]

Quoted 7 lines

drivers/iio/light/isl29018.c +0 -0

@@ -727,7 +727,16 @@ static int isl29018_probe(struct i2c_client *client)
 	mutex_init(&chip->lock);
 
 	chip->type = dev_id;

drivers/iio/light/isl29018.c static int isl29018_probe(struct i2c_client *client) · line 739

Quoted 10 lines

drivers/iio/light/isl29018.c +9 -0

+	/*
+	 * 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 does setting a large calibscale here exacerbate an integer overflow in isl29018_read_lux()?

In isl29018_read_lux(), the calculations for data_x_range and the final *lux value rely on 32-bit unsigned arithmetic:

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

The multiplication lux_data * chip->scale.uscale can reach ~64 billion (e.g., 65535 * 976,562), overflowing UINT_MAX (4,294,967,295) and losing precision.

Furthermore, the subsequent multiplication data_x_range * chip->calibscale easily exceeds 2 billion if calibscale is seeded to a high value (like 100, as the commit message suggests). Because the result is stored in the signed int *lux, exceeding 2 billion causes the value to wrap, resulting in massive negative lux values being reported to userspace.

Could these 32-bit multiplications be updated to use 64-bit math or scaled down to prevent overflow in bright light environments?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Andy Shevchenko

2026-06-04 07:18 UTC #3 lore raw

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

In reply to Herman van Hazendonk

Quoted 8 lines · Herman van Hazendonk

Boards that mount the sensor under a tinted or coated cover glass need to compensate for the optical loss before downstream consumers can map the reading onto a useful lux range. The driver already exposes a runtime knob through in_illuminance0_calibscale, but every user has to re-apply it after every reboot (or rely on a board-specific udev rule), and a power-of-two cover gain like 100x is a hardware constant of the board rather than a policy choice that belongs in userspace.

Quoted 5 lines

Add an "isil,cover-comp-gain" device-tree property that seeds calibscale at probe, mirroring the pattern tsl2563.c already uses for the same class of problem (amstaos,cover-comp-gain). The default stays 1 so existing systems are unaffected, and userspace can still re-tune via the sysfs attribute afterwards.

Haven't DT schema patches needed to go separately?

...

drivers/iio/light/isl29018.c static int isl29018_probe(struct i2c_client *client) · line 739

Quoted 10 lines

drivers/iio/light/isl29018.c +9 -0

+	/*
+	 * 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);

With

	struct device *dev = &client->dev;

at the top of the function this will be made shorter.

Additionally the above approach won't be accurate when property is present in DT but by some reason can't be retrieved. It will puzzle the user. That's why the robust approach is

	struct device *dev = &client->dev;
	const char *propname;
	...

	propname = "isil,cover-comp-gain";
	if (device_property_present(dev, &chip->calibscale)) {
		ret = device_property_read_u32(dev, propname, &chip->calibscale);
		if (ret)
			return ret; // optionally dev_err_probe()
	} else {
		chip->calibscale = 1;
	}

Taking the expansion of this pattern we might need to actually introduce a full set of device_property_read_*_optional() to make this churn less required. Perhaps for v7.3.

Signature · 2 lines

With Best Regards,
Andy Shevchenko

Joshua Crofts

2026-06-04 07:22 UTC #4 lore raw

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

Quoted 10 lines · Andy Shevchenko

Quoted 8 lines · Herman van Hazendonk
Boards that mount the sensor under a tinted or coated cover glass need to compensate for the optical loss before downstream consumers can map the reading onto a useful lux range. The driver already exposes a runtime knob through in_illuminance0_calibscale, but every user has to re-apply it after every reboot (or rely on a board-specific udev rule), and a power-of-two cover gain like 100x is a hardware constant of the board rather than a policy choice that belongs in userspace.

Quoted 7 lines

Quoted 5 lines
Add an "isil,cover-comp-gain" device-tree property that seeds calibscale at probe, mirroring the pattern tsl2563.c already uses for the same class of problem (amstaos,cover-comp-gain). The default stays 1 so existing systems are unaffected, and userspace can still re-tune via the sysfs attribute afterwards.
Haven't DT schema patches needed to go separately?

Yes, with the correct subject line/commit message title as well. (e.g. dt-bindings: iio: light: isl29018: add support cover gain comp)

Signature · 3 lines

Kind regards

CJD

Herman van Hazendonk

author

2026-06-04 06:49 UTC #5 lore raw

v2 cover iio: light: isl29018: overflow fix + cover-glass gain via DT

In reply to Herman van Hazendonk

v2: split into two patches after review feedback on v1.

PATCH 1/2 fixes a pre-existing 32-bit overflow in isl29018_read_lux() that the reviewer correctly identified: lux_data * scale.uscale can reach ~64 billion at full-scale 16-bit readings (65535 × 976562), overflowing UINT_MAX before the /1000000 division. A second overflow occurs when the result is multiplied by calibscale and stored in the signed int *lux. Both are fixed by widening intermediate arithmetic to u64 and clamping the output.

PATCH 2/2 is unchanged from v1: it adds the "isil,cover-comp-gain" DT property to seed calibscale at probe time, following the precedent set by tsl2563.c (amstaos,cover-comp-gain). The default stays 1 so existing systems are unaffected. The overflow fix in patch 1 is a natural prerequisite because patch 2 makes large calibscale values straightforward to deploy from firmware.

A follow-up DTS patch enabling this for the HP TouchPad will be sent separately to the ARM/DTS tree once the driver change is upstream.

Herman van Hazendonk (2):
iio: light: isl29018: fix 32-bit overflow in isl29018_read_lux()
iio: light: isl29018: support cover-glass gain compensation via DT

 .../bindings/iio/light/isl29018.yaml          | 13 ++++++++++++
 drivers/iio/light/isl29018.c                  | 20 ++++++++++++++-----
 2 files changed, 28 insertions(+), 5 deletions(-)

base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff

Signature · 1 line

2.43.0

Herman van Hazendonk

author

2026-06-04 06:49 UTC #6 lore raw

PATCH v2 1/2 iio: light: isl29018: fix 32-bit overflow in isl29018_read_lux()

The intermediate calculations in isl29018_read_lux() use 32-bit arithmetic throughout, which overflows in two distinct ways:

1. lux_data * chip->scale.uscale — at 16-bit integration time and the 1000 fc range, scale.uscale is 976562. A full-scale 16-bit reading (65535) gives 65535 * 976562 ≈ 64 billion, far beyond UINT_MAX. The value wraps before the / 1000000 division can save it, producing a wildly wrong data_x_range.

2. data_x_range * chip->calibscale — even after a correct data_x_range, multiplying by a calibscale of a few hundred (reasonable for a deeply tinted cover glass) pushes the product past INT_MAX, causing *lux to wrap negative.

Fix both by widening the intermediate variables to u64 and clamping the final result to INT_MAX before storing it in the signed int *lux out parameter.

Signed-off-by Herman van Hazendonk <[email protected]>

---
 drivers/iio/light/isl29018.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

drivers/iio/light/isl29018.c +6 -5

index b6ab726d1dae..eafdfd9c4635 100644
@@ -193,17 +193,18 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
 static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
 {
 	int lux_data;
-	unsigned int data_x_range;
+	u64 data_x_range, result;
 
 	lux_data = isl29018_read_sensor_input(chip,
 					      ISL29018_CMD1_OPMODE_ALS_ONCE);
 	if (lux_data < 0)
 		return lux_data;
 
-	data_x_range = lux_data * chip->scale.scale +
-		       lux_data * chip->scale.uscale / 1000000;
-	*lux = data_x_range * chip->calibscale +
-	       data_x_range * chip->ucalibscale / 1000000;
+	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;
+	*lux = (int)min_t(u64, result, INT_MAX);
 
 	return 0;
 }

Signature · 1 line

2.43.0

[email protected]

AI review

2026-06-04 06:58 UTC #7 lore raw

Re: [PATCH v2 1/2] iio: light: isl29018: fix 32-bit overflow in isl29018_read_lux()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] The patch introduces 64-bit divisions using the standard `/` operator, which will cause a linker…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces 64-bit divisions using the standard / operator, which will cause a linker error (undefined reference to __udivdi3) on 32-bit architectures. --

commit 37588d2a24184ed61a15690c807e6b4b16aa4002
Author: Herman van Hazendonk <[email protected]>

iio: light: isl29018: fix 32-bit overflow in isl29018_read_lux()

This commit addresses an overflow in isl29018_read_lux() by changing intermediate calculation variables to u64. It then clamps the final result to INT_MAX before storing it in the signed output variable.

Quoted 10 lines

drivers/iio/light/isl29018.c +1 -1

index 1b4c184230489..927018c531e8d 100644
@@ -193,17 +193,18 @@ static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode)
 static int isl29018_read_lux(struct isl29018_chip *chip, int *lux)
 {
 	int lux_data;
-	unsigned int data_x_range;
+	u64 data_x_range, result;

[ ... ]

drivers/iio/light/isl29018.c static int isl29018_read_sensor_input(struct isl29018_chip *chip, int mode) · line 206

Quoted 8 lines

drivers/iio/light/isl29018.c +4 -4

-	data_x_range = lux_data * chip->scale.scale +
-		       lux_data * chip->scale.uscale / 1000000;
-	*lux = data_x_range * chip->calibscale +
-	       data_x_range * chip->ucalibscale / 1000000;
+	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;

[Severity: Low]
Will this cause a linker error on 32-bit architectures?

Since data_x_range and the result of the multiplications are now u64, using the native division operator (/) for 1000000 will result in an undefined reference to __udivdi3 when compiling for 32-bit platforms.

Should this use div_u64() from <linux/math64.h> instead?

Quoted 4 lines

drivers/iio/light/isl29018.c +1 -0

+	*lux = (int)min_t(u64, result, INT_MAX);
 
 	return 0;
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Herman van Hazendonk

author

2026-06-04 06:49 UTC #8 lore raw

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

In reply to Herman van Hazendonk

Signed-off-by Herman van Hazendonk <[email protected]>

---
 .../devicetree/bindings/iio/light/isl29018.yaml     | 13 +++++++++++++
 drivers/iio/light/isl29018.c                        |  9 +++++++++
 2 files changed, 22 insertions(+)

Documentation/devicetree/bindings/iio/light/isl29018.yaml +13 -0

index 0ea278b07d1c..92ea2742bbd3 100644
@@ -34,6 +34,19 @@ properties:
   vcc-supply:
     description: Regulator that provides power to the sensor
 
+  isil,cover-comp-gain:
+    description: |
+      Multiplier applied to the ambient-light reading at startup to
+      compensate for optical loss in the board's cover glass. Boards
+      that mount the sensor under a tinted or coated window typically
+      need a value between a few and a few hundred. The value seeds
+      in_illuminance0_calibscale, so it can still be retuned at
+      runtime through sysfs.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 1
+    maximum: 65535
+    default: 1
+
 required:
   - compatible
   - reg

drivers/iio/light/isl29018.c +9 -0

index eafdfd9c4635..6d2f1e9f59b5 100644
@@ -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);
 	chip->ucalibscale = 0;
 	chip->int_time = ISL29018_INT_TIME_16;
 	chip->scale = isl29018_scales[chip->int_time][0];

Signature · 1 line

2.43.0

[email protected]

AI review

2026-06-04 07:00 UTC #9 lore raw

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

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 <[email protected]>

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.

Quoted 4 lines

drivers/iio/light/isl29018.c +0 -0
index 927018c531e8d..1d6a011c92625 100644

[ ... ]

Quoted 4 lines

drivers/iio/light/isl29018.c +0 -0

@@ -728,7 +728,16 @@ static int isl29018_probe(struct i2c_client *client)
 	mutex_init(&chip->lock);
 
 	chip->type = dev_id;

drivers/iio/light/isl29018.c static int isl29018_probe(struct i2c_client *client) · line 740

Quoted 10 lines

drivers/iio/light/isl29018.c +9 -0

+	/*
+	 * 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?

Quoted 3 lines

drivers/iio/light/isl29018.c +0 -0

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

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Krzysztof Kozlowski

2026-06-08 08:13 UTC #10 lore raw

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

In reply to Herman van Hazendonk

Quoted 14 lines · Herman van Hazendonk

Boards that mount the sensor under a tinted or coated cover glass need to compensate for the optical loss before downstream consumers can map the reading onto a useful lux range. The driver already exposes a runtime knob through in_illuminance0_calibscale, but every user has to re-apply it after every reboot (or rely on a board-specific udev rule), and a power-of-two cover gain like 100x is a hardware constant of the board rather than a policy choice that belongs in userspace.
Add an "isil,cover-comp-gain" device-tree property that seeds calibscale at probe, mirroring the pattern tsl2563.c already uses for the same class of problem (amstaos,cover-comp-gain). The default stays 1 so existing systems are unaffected, and userspace can still re-tune via the sysfs attribute afterwards.

Quoted 5 lines

Signed-off-by Herman van Hazendonk <[email protected]>

---
 .../devicetree/bindings/iio/light/isl29018.yaml     | 13 +++++++++++++
 drivers/iio/light/isl29018.c                        |  9 +++++++++
 2 files changed, 22 insertions(+)

Do not attach (thread) your patchsets to some other threads (unrelated or older versions). This buries them deep in the mailbox and might interfere with applying entire sets. See also:
https://elixir.bootlin.com/linux/v6.16-rc2/source/Documentation/process/submitting-patches.rst#L830

Please run scripts/checkpatch.pl on the patches and fix reported warnings. After that, run also 'scripts/checkpatch.pl --strict' on the patches and (probably) fix more warnings. Some warnings can be ignored, especially from --strict run, but the code here looks like it needs a fix. Feel free to get in touch if the warning is not clear.

Best regards,
Krzysztof