[PATCH v3 0/3] iio: light: isl29018: overflow/precision fix + cover-glass gain via DT

[PATCH v3 0/3] iio: light: isl29018: overflow/precision fix + cover-glass gain via DT

[Herman van Hazendonk]

v3:
 - Split the DT binding into its own commit per maintainer feedback
   on v2 (binding must precede the driver patch that consumes the
   new property).  The series is now 3 patches in dependency order:
   precision fix -> binding -> driver.

 - PATCH 1/3: tighten the precision fix.  v2 widened the intermediate
   arithmetic to u64 but still discarded the sub-lux remainder before
   applying calibscale, which collapsed to zero on low ranges with
   large cover-glass gain.  v3 keeps the remainder via div_u64_rem()
   so even small counts contribute.  Switching from '/' and '%' on
   u64 to div_u64()/div_u64_rem() also restores the ARM32 build,
   which v2 broke with __aeabi_uldivmod.

 - PATCH 3/3 (driver) addresses Andy's review of v2:
     * hoist 'struct device *dev = &client->dev;' so subsequent
       device_property_*() and devm_*() calls are uniform;
     * guard the property read with device_property_present() and
       surface the failure with dev_err_probe(), rather than silently
       falling back when device_property_read_u32() returns -EINVAL
       on a malformed value;
     * the silent fallback (calibscale = 1) only applies when the
       property is genuinely absent, which matches the tsl2563.c
       precedent.

PATCH 2/3 is the new standalone binding commit; the schema, default,
and rationale are unchanged from v2.

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 (3):
  iio: light: isl29018: fix overflow and precision in
    isl29018_read_lux()
  dt-bindings: iio: light: isl29018: add isil,cover-comp-gain
  iio: light: isl29018: support cover-glass gain compensation via DT

 .../bindings/iio/light/isl29018.yaml          | 13 ++++++++
 drivers/iio/light/isl29018.c                  | 31 ++++++++++++++-----
 2 files changed, 37 insertions(+), 7 deletions(-)


base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
-- 
2.43.0

[PATCH v3 1/3] iio: light: isl29018: fix overflow and precision in isl29018_read_lux()

[Herman van Hazendonk]

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.

Additionally, dividing lux_data * scale.uscale by 1000000 before
applying calibscale discards the fractional-lux remainder.  For low
ranges where scale.scale is zero, any reading below 1000000/scale.uscale
counts truncates to a data_x_range of zero, so the calibscale
multiplication cannot rescue it.  This creates a dead-band at low light
levels that is especially visible when a large cover-glass compensation
gain is in use.

Fix the overflows by widening the intermediate variables to u64 and
using div_u64() for the divisions (plain 64-bit division emits
__aeabi_uldivmod on ARM32, which is not available in kernel builds).
Preserve the uscale remainder across the first division so that the
calibscale multiplication captures the sub-lux contribution.  Clamp
the final result to INT_MAX before storing it in the signed int *lux
out parameter.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/iio/light/isl29018.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index b6ab726d1dae..f3312ad670d9 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/delay.h>
+#include <linux/math64.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
@@ -193,17 +194,22 @@ 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;
+	u32 uscale_rem;
+	u64 uscale_term, 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;
+	/* Retain the uscale remainder so calibscale captures sub-lux precision. */
+	uscale_term = (u64)lux_data * chip->scale.uscale;
+	data_x_range = (u64)lux_data * chip->scale.scale +
+		       div_u64_rem(uscale_term, 1000000, &uscale_rem);
+	result = data_x_range * chip->calibscale +
+		 div_u64((u64)uscale_rem * chip->calibscale, 1000000) +
+		 div_u64(data_x_range * chip->ucalibscale, 1000000);
+	*lux = (int)min_t(u64, result, INT_MAX);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v3 2/3] dt-bindings: iio: light: isl29018: add isil,cover-comp-gain

[Herman van Hazendonk]

Document the new optional property that seeds the ISL29018 calibration
scale factor at boot from firmware, allowing boards with tinted cover
glass to ship with correct luminance readings without a userspace helper.

The value is a positive integer (minimum 1, maximum 65535) that is
multiplied with the raw lux reading.  Userspace can still override it
at runtime through in_illuminance0_calibscale.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 .../devicetree/bindings/iio/light/isl29018.yaml     | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/Documentation/devicetree/bindings/iio/light/isl29018.yaml b/Documentation/devicetree/bindings/iio/light/isl29018.yaml
index 0ea278b07d1c..92ea2742bbd3 100644
--- a/Documentation/devicetree/bindings/iio/light/isl29018.yaml
+++ b/Documentation/devicetree/bindings/iio/light/isl29018.yaml
@@ -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
-- 
2.43.0

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

[Herman van Hazendonk]

Boards that mount the ISL29018 behind tinted or coated cover glass
experience optical loss that effectively reduces the sensor's apparent
sensitivity.  The existing in_illuminance0_calibscale sysfs attribute
can correct for this at runtime, but firmware knows the loss factor at
design time and there is no way to seed it without a userspace helper.

Add support for an optional "isil,cover-comp-gain" device-tree property
that initialises calibscale at probe time.  If the property is present
but cannot be read, probe returns an error via dev_err_probe() so the
root cause is visible in the log.  If absent, calibscale defaults to 1
(unity gain, matching the previous behaviour).  Userspace can still
override the value at runtime through the sysfs attribute.

The approach follows the precedent set by the TSL2563 driver.

Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
 drivers/iio/light/isl29018.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/light/isl29018.c b/drivers/iio/light/isl29018.c
index f3312ad670d9..9daacb9b02bc 100644
--- a/drivers/iio/light/isl29018.c
+++ b/drivers/iio/light/isl29018.c
@@ -711,10 +711,13 @@ static int isl29018_probe(struct i2c_client *client)
 	struct iio_dev *indio_dev;
 	const void *ddata = NULL;
 	const char *name;
+	struct device *dev;
 	int dev_id;
 	int err;
 
-	indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*chip));
+	dev = &client->dev;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*chip));
 	if (!indio_dev)
 		return -ENOMEM;
 
@@ -733,8 +736,16 @@ static int isl29018_probe(struct i2c_client *client)
 	mutex_init(&chip->lock);
 
 	chip->type = dev_id;
-	chip->calibscale = 1;
 	chip->ucalibscale = 0;
+	if (device_property_present(dev, "isil,cover-comp-gain")) {
+		err = device_property_read_u32(dev, "isil,cover-comp-gain",
+					       &chip->calibscale);
+		if (err)
+			return dev_err_probe(dev, err,
+					     "invalid isil,cover-comp-gain\n");
+	} else {
+		chip->calibscale = 1;
+	}
 	chip->int_time = ISL29018_INT_TIME_16;
 	chip->scale = isl29018_scales[chip->int_time][0];
 	chip->suspended = false;
-- 
2.43.0